From 5c6c6ab6ebdd370a9a98a9e293ac3dedfcf53a97 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Tue, 22 May 2018 15:01:03 -0700 Subject: [PATCH] moveToNewFile: Don't provide refactor if selection is just imports --- src/services/refactors/moveToNewFile.ts | 15 +++++++-------- .../fourslash/moveToNewFile_selectionOnImports.ts | 9 +++++++++ 2 files changed, 16 insertions(+), 8 deletions(-) create mode 100644 tests/cases/fourslash/moveToNewFile_selectionOnImports.ts diff --git a/src/services/refactors/moveToNewFile.ts b/src/services/refactors/moveToNewFile.ts index ddccee1acba58..b415b9048ef0a 100644 --- a/src/services/refactors/moveToNewFile.ts +++ b/src/services/refactors/moveToNewFile.ts @@ -3,7 +3,7 @@ namespace ts.refactor { const refactorName = "Move to a new file"; registerRefactor(refactorName, { getAvailableActions(context): ApplicableRefactorInfo[] | undefined { - if (!context.preferences.allowTextChangesInNewFiles || getFirstAndLastStatementToMove(context) === undefined) return undefined; + if (!context.preferences.allowTextChangesInNewFiles || getStatementsToMove(context) === undefined) return undefined; const description = getLocaleSpecificMessage(Diagnostics.Move_to_a_new_file); return [{ name: refactorName, description, actions: [{ name: refactorName, description }] }]; }, @@ -15,7 +15,7 @@ namespace ts.refactor { } }); - function getFirstAndLastStatementToMove(context: RefactorContext): { readonly first: number, readonly afterLast: number } | undefined { + function getRangeToMove(context: RefactorContext): ReadonlyArray | undefined { const { file } = context; const range = createTextRangeFromSpan(getRefactorContextSpan(context)); const { statements } = file; @@ -25,7 +25,7 @@ namespace ts.refactor { const startStatement = statements[startNodeIndex]; if (isNamedDeclaration(startStatement) && startStatement.name && rangeContainsRange(startStatement.name, range)) { - return { first: startNodeIndex, afterLast: startNodeIndex + 1 }; + return [statements[startNodeIndex]]; } // Can't only partially include the start node or be partially into the next node @@ -34,7 +34,7 @@ namespace ts.refactor { // Can't be partially into the next node if (afterEndNodeIndex !== -1 && (afterEndNodeIndex === 0 || statements[afterEndNodeIndex].getStart(file) < range.end)) return undefined; - return { first: startNodeIndex, afterLast: afterEndNodeIndex === -1 ? statements.length : afterEndNodeIndex }; + return statements.slice(startNodeIndex, afterEndNodeIndex === -1 ? statements.length : afterEndNodeIndex); } function doChange(oldFile: SourceFile, program: Program, toMove: ToMove, changes: textChanges.ChangeTracker, host: LanguageServiceHost): void { @@ -63,16 +63,15 @@ namespace ts.refactor { // Filters imports out of the range of statements to move. Imports will be copied to the new file anyway, and may still be needed in the old file. function getStatementsToMove(context: RefactorContext): ToMove | undefined { - const { statements } = context.file; - const { first, afterLast } = getFirstAndLastStatementToMove(context)!; + const rangeToMove = getRangeToMove(context); + if (rangeToMove === undefined) return undefined; const all: Statement[] = []; const ranges: StatementRange[] = []; - const rangeToMove = statements.slice(first, afterLast); getRangesWhere(rangeToMove, s => !isPureImport(s), (start, afterEnd) => { for (let i = start; i < afterEnd; i++) all.push(rangeToMove[i]); ranges.push({ first: rangeToMove[start], last: rangeToMove[afterEnd - 1] }); }); - return { all, ranges }; + return all.length === 0 ? undefined : { all, ranges }; } function isPureImport(node: Node): boolean { diff --git a/tests/cases/fourslash/moveToNewFile_selectionOnImports.ts b/tests/cases/fourslash/moveToNewFile_selectionOnImports.ts new file mode 100644 index 0000000000000..0884428398d48 --- /dev/null +++ b/tests/cases/fourslash/moveToNewFile_selectionOnImports.ts @@ -0,0 +1,9 @@ +/// + +// @Filename: /a.ts +////export {}; +////[|import foo from "./foo";|] + +// No refactor available if every statement in the range is an import, since we don't move those. + +verify.noMoveToNewFile();