From bee347eeebe9ca6bf5bd6696c00885b6ccf245ad Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Wed, 16 May 2018 13:03:59 -0700 Subject: [PATCH] moveToNewFile: Don't move imports --- src/compiler/core.ts | 17 +++++ src/services/codefixes/fixUnreachableCode.ts | 15 +---- src/services/refactors/moveToNewFile.ts | 64 +++++++++++++++---- .../fourslash/moveToNewFile_exportImport.ts | 8 +-- .../fourslash/moveToNewFile_moveImport.ts | 5 +- 5 files changed, 76 insertions(+), 33 deletions(-) diff --git a/src/compiler/core.ts b/src/compiler/core.ts index 3e8e33e301719..fb4324eb0a45f 100644 --- a/src/compiler/core.ts +++ b/src/compiler/core.ts @@ -696,6 +696,23 @@ namespace ts { return false; } + /** Calls the callback with (start, afterEnd) index pairs for each range where 'pred' is true. */ + export function getRangesWhere(arr: ReadonlyArray, pred: (t: T) => boolean, cb: (start: number, afterEnd: number) => void): void { + let start: number | undefined; + for (let i = 0; i < arr.length; i++) { + if (pred(arr[i])) { + start = start === undefined ? i : start; + } + else { + if (start !== undefined) { + cb(start, i); + start = undefined; + } + } + } + if (start !== undefined) cb(start, arr.length); + } + export function concatenate(array1: T[], array2: T[]): T[]; export function concatenate(array1: ReadonlyArray, array2: ReadonlyArray): ReadonlyArray; export function concatenate(array1: T[], array2: T[]): T[] { diff --git a/src/services/codefixes/fixUnreachableCode.ts b/src/services/codefixes/fixUnreachableCode.ts index 5b075d6af1fdf..96241c2263c6d 100644 --- a/src/services/codefixes/fixUnreachableCode.ts +++ b/src/services/codefixes/fixUnreachableCode.ts @@ -71,19 +71,6 @@ namespace ts.codefix { // Calls 'cb' with the start and end of each range where 'pred' is true. function split(arr: ReadonlyArray, pred: (t: T) => boolean, cb: (start: T, end: T) => void): void { - let start: T | undefined; - for (let i = 0; i < arr.length; i++) { - const value = arr[i]; - if (pred(value)) { - start = start || value; - } - else { - if (start) { - cb(start, arr[i - 1]); - start = undefined; - } - } - } - if (start) cb(start, arr[arr.length - 1]); + getRangesWhere(arr, pred, (start, afterEnd) => cb(arr[start], arr[afterEnd - 1])); } } diff --git a/src/services/refactors/moveToNewFile.ts b/src/services/refactors/moveToNewFile.ts index 2f5a27f87f781..621bf4eda2d78 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[] { - if (!context.preferences.allowTextChangesInNewFiles || getStatementsToMove(context) === undefined) return undefined; + if (!context.preferences.allowTextChangesInNewFiles || getFirstAndLastStatementToMove(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 getStatementsToMove(context: RefactorContext): ReadonlyArray | undefined { + function getFirstAndLastStatementToMove(context: RefactorContext): { readonly first: number, readonly afterLast: number } | undefined { const { file } = context; const range = createTextRangeFromSpan(getRefactorContextSpan(context)); const { statements } = file; @@ -28,12 +28,12 @@ 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 statements.slice(startNodeIndex, afterEndNodeIndex === -1 ? statements.length : afterEndNodeIndex); + return { first: startNodeIndex, afterLast: afterEndNodeIndex === -1 ? statements.length : afterEndNodeIndex }; } - function doChange(oldFile: SourceFile, program: Program, toMove: ReadonlyArray, changes: textChanges.ChangeTracker, host: LanguageServiceHost): void { + function doChange(oldFile: SourceFile, program: Program, toMove: ToMove, changes: textChanges.ChangeTracker, host: LanguageServiceHost): void { const checker = program.getTypeChecker(); - const usage = getUsageInfo(oldFile, toMove, checker); + const usage = getUsageInfo(oldFile, toMove.all, checker); const currentDirectory = getDirectoryPath(oldFile.fileName); const extension = extensionFromPath(oldFile.fileName); @@ -46,6 +46,42 @@ namespace ts.refactor { addNewFileToTsconfig(program, changes, oldFile.fileName, newFileNameWithExtension, hostGetCanonicalFileName(host)); } + interface StatementRange { + readonly first: Statement; + readonly last: Statement; + } + interface ToMove { + readonly all: ReadonlyArray; + readonly ranges: ReadonlyArray; + } + + // 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 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 }; + } + + function isPureImport(node: Node): boolean { + switch (node.kind) { + case SyntaxKind.ImportDeclaration: + return true; + case SyntaxKind.ImportEqualsDeclaration: + return !hasModifier(node, ModifierFlags.Export); + case SyntaxKind.VariableStatement: + return (node as VariableStatement).declarationList.declarations.every(d => isRequireCall(d.initializer, /*checkArgumentIsStringLiteralLike*/ true)); + default: + return false; + } + } + function addNewFileToTsconfig(program: Program, changes: textChanges.ChangeTracker, oldFileName: string, newFileNameWithExtension: string, getCanonicalFileName: GetCanonicalFileName): void { const cfg = program.getCompilerOptions().configFile; if (!cfg) return; @@ -62,13 +98,13 @@ namespace ts.refactor { } function getNewStatements( - oldFile: SourceFile, usage: UsageInfo, changes: textChanges.ChangeTracker, toMove: ReadonlyArray, program: Program, newModuleName: string, + oldFile: SourceFile, usage: UsageInfo, changes: textChanges.ChangeTracker, toMove: ToMove, program: Program, newModuleName: string, ): ReadonlyArray { const checker = program.getTypeChecker(); if (!oldFile.externalModuleIndicator && !oldFile.commonJsModuleIndicator) { - changes.deleteNodeRange(oldFile, first(toMove), last(toMove)); - return toMove; + deleteMovedStatements(oldFile, toMove.ranges, changes); + return toMove.all; } const useEs6ModuleSyntax = !!oldFile.externalModuleIndicator; @@ -77,17 +113,23 @@ namespace ts.refactor { changes.insertNodeBefore(oldFile, oldFile.statements[0], importsFromNewFile, /*blankLineBetween*/ true); } - deleteUnusedOldImports(oldFile, toMove, changes, usage.unusedImportsFromOldFile, checker); - changes.deleteNodeRange(oldFile, first(toMove), last(toMove)); + deleteUnusedOldImports(oldFile, toMove.all, changes, usage.unusedImportsFromOldFile, checker); + deleteMovedStatements(oldFile, toMove.ranges, changes); updateImportsInOtherFiles(changes, program, oldFile, usage.movedSymbols, newModuleName); return [ ...getNewFileImportsAndAddExportInOldFile(oldFile, usage.oldImportsNeededByNewFile, usage.newFileImportsFromOldFile, changes, checker, useEs6ModuleSyntax), - ...addExports(oldFile, toMove, usage.oldFileImportsFromNewFile, useEs6ModuleSyntax), + ...addExports(oldFile, toMove.all, usage.oldFileImportsFromNewFile, useEs6ModuleSyntax), ]; } + function deleteMovedStatements(sourceFile: SourceFile, moved: ReadonlyArray, changes: textChanges.ChangeTracker) { + for (const { first, last } of moved) { + changes.deleteNodeRange(sourceFile, first, last); + } + } + function deleteUnusedOldImports(oldFile: SourceFile, toMove: ReadonlyArray, changes: textChanges.ChangeTracker, toDelete: ReadonlySymbolSet, checker: TypeChecker) { for (const statement of oldFile.statements) { if (contains(toMove, statement)) continue; diff --git a/tests/cases/fourslash/moveToNewFile_exportImport.ts b/tests/cases/fourslash/moveToNewFile_exportImport.ts index a8ac3a66a0124..657aa2f807598 100644 --- a/tests/cases/fourslash/moveToNewFile_exportImport.ts +++ b/tests/cases/fourslash/moveToNewFile_exportImport.ts @@ -9,14 +9,12 @@ verify.moveToNewFile({ newFileContents: { "/a.ts": -`import { M } from "./M"; - -export namespace N { export const x = 0; } +`export namespace N { export const x = 0; } +import M = N; M;`, - "/M.ts": + "/O.ts": `import { N } from "./a"; -export import M = N; export import O = N;`, }, }); diff --git a/tests/cases/fourslash/moveToNewFile_moveImport.ts b/tests/cases/fourslash/moveToNewFile_moveImport.ts index a96a187f633f6..08a8940307068 100644 --- a/tests/cases/fourslash/moveToNewFile_moveImport.ts +++ b/tests/cases/fourslash/moveToNewFile_moveImport.ts @@ -5,14 +5,13 @@ ////a;|] ////b; -//verify.noMoveToNewFile(); verify.moveToNewFile({ newFileContents: { "/a.ts": -`b;`, +`import { b } from "m"; +b;`, "/newFile.ts": `import { a } from "m"; -import { a, b } from "m"; a;`, } });