Skip to content

Commit 176e35b

Browse files
author
Andy
authored
moveToNewFile: Don't move imports (microsoft#24177)
1 parent aa7e2b0 commit 176e35b

5 files changed

Lines changed: 76 additions & 33 deletions

File tree

‎src/compiler/core.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -696,6 +696,23 @@ namespace ts {
696696
return false;
697697
}
698698

699+
/** Calls the callback with (start, afterEnd) index pairs for each range where 'pred' is true. */
700+
export function getRangesWhere<T>(arr: ReadonlyArray<T>, pred: (t: T) => boolean, cb: (start: number, afterEnd: number) => void): void {
701+
let start: number | undefined;
702+
for (let i = 0; i < arr.length; i++) {
703+
if (pred(arr[i])) {
704+
start = start === undefined ? i : start;
705+
}
706+
else {
707+
if (start !== undefined) {
708+
cb(start, i);
709+
start = undefined;
710+
}
711+
}
712+
}
713+
if (start !== undefined) cb(start, arr.length);
714+
}
715+
699716
export function concatenate<T>(array1: T[], array2: T[]): T[];
700717
export function concatenate<T>(array1: ReadonlyArray<T>, array2: ReadonlyArray<T>): ReadonlyArray<T>;
701718
export function concatenate<T>(array1: T[], array2: T[]): T[] {

‎src/services/codefixes/fixUnreachableCode.ts‎

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -71,19 +71,6 @@ namespace ts.codefix {
7171

7272
// Calls 'cb' with the start and end of each range where 'pred' is true.
7373
function split<T>(arr: ReadonlyArray<T>, pred: (t: T) => boolean, cb: (start: T, end: T) => void): void {
74-
let start: T | undefined;
75-
for (let i = 0; i < arr.length; i++) {
76-
const value = arr[i];
77-
if (pred(value)) {
78-
start = start || value;
79-
}
80-
else {
81-
if (start) {
82-
cb(start, arr[i - 1]);
83-
start = undefined;
84-
}
85-
}
86-
}
87-
if (start) cb(start, arr[arr.length - 1]);
74+
getRangesWhere(arr, pred, (start, afterEnd) => cb(arr[start], arr[afterEnd - 1]));
8875
}
8976
}

‎src/services/refactors/moveToNewFile.ts‎

Lines changed: 53 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ namespace ts.refactor {
33
const refactorName = "Move to a new file";
44
registerRefactor(refactorName, {
55
getAvailableActions(context): ApplicableRefactorInfo[] {
6-
if (!context.preferences.allowTextChangesInNewFiles || getStatementsToMove(context) === undefined) return undefined;
6+
if (!context.preferences.allowTextChangesInNewFiles || getFirstAndLastStatementToMove(context) === undefined) return undefined;
77
const description = getLocaleSpecificMessage(Diagnostics.Move_to_a_new_file);
88
return [{ name: refactorName, description, actions: [{ name: refactorName, description }] }];
99
},
@@ -15,7 +15,7 @@ namespace ts.refactor {
1515
}
1616
});
1717

18-
function getStatementsToMove(context: RefactorContext): ReadonlyArray<Statement> | undefined {
18+
function getFirstAndLastStatementToMove(context: RefactorContext): { readonly first: number, readonly afterLast: number } | undefined {
1919
const { file } = context;
2020
const range = createTextRangeFromSpan(getRefactorContextSpan(context));
2121
const { statements } = file;
@@ -28,12 +28,12 @@ namespace ts.refactor {
2828
// Can't be partially into the next node
2929
if (afterEndNodeIndex !== -1 && (afterEndNodeIndex === 0 || statements[afterEndNodeIndex].getStart(file) < range.end)) return undefined;
3030

31-
return statements.slice(startNodeIndex, afterEndNodeIndex === -1 ? statements.length : afterEndNodeIndex);
31+
return { first: startNodeIndex, afterLast: afterEndNodeIndex === -1 ? statements.length : afterEndNodeIndex };
3232
}
3333

34-
function doChange(oldFile: SourceFile, program: Program, toMove: ReadonlyArray<Statement>, changes: textChanges.ChangeTracker, host: LanguageServiceHost): void {
34+
function doChange(oldFile: SourceFile, program: Program, toMove: ToMove, changes: textChanges.ChangeTracker, host: LanguageServiceHost): void {
3535
const checker = program.getTypeChecker();
36-
const usage = getUsageInfo(oldFile, toMove, checker);
36+
const usage = getUsageInfo(oldFile, toMove.all, checker);
3737

3838
const currentDirectory = getDirectoryPath(oldFile.fileName);
3939
const extension = extensionFromPath(oldFile.fileName);
@@ -46,6 +46,42 @@ namespace ts.refactor {
4646
addNewFileToTsconfig(program, changes, oldFile.fileName, newFileNameWithExtension, hostGetCanonicalFileName(host));
4747
}
4848

49+
interface StatementRange {
50+
readonly first: Statement;
51+
readonly last: Statement;
52+
}
53+
interface ToMove {
54+
readonly all: ReadonlyArray<Statement>;
55+
readonly ranges: ReadonlyArray<StatementRange>;
56+
}
57+
58+
// 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.
59+
function getStatementsToMove(context: RefactorContext): ToMove | undefined {
60+
const { statements } = context.file;
61+
const { first, afterLast } = getFirstAndLastStatementToMove(context)!;
62+
const all: Statement[] = [];
63+
const ranges: StatementRange[] = [];
64+
const rangeToMove = statements.slice(first, afterLast);
65+
getRangesWhere(rangeToMove, s => !isPureImport(s), (start, afterEnd) => {
66+
for (let i = start; i < afterEnd; i++) all.push(rangeToMove[i]);
67+
ranges.push({ first: rangeToMove[start], last: rangeToMove[afterEnd - 1] });
68+
});
69+
return { all, ranges };
70+
}
71+
72+
function isPureImport(node: Node): boolean {
73+
switch (node.kind) {
74+
case SyntaxKind.ImportDeclaration:
75+
return true;
76+
case SyntaxKind.ImportEqualsDeclaration:
77+
return !hasModifier(node, ModifierFlags.Export);
78+
case SyntaxKind.VariableStatement:
79+
return (node as VariableStatement).declarationList.declarations.every(d => isRequireCall(d.initializer, /*checkArgumentIsStringLiteralLike*/ true));
80+
default:
81+
return false;
82+
}
83+
}
84+
4985
function addNewFileToTsconfig(program: Program, changes: textChanges.ChangeTracker, oldFileName: string, newFileNameWithExtension: string, getCanonicalFileName: GetCanonicalFileName): void {
5086
const cfg = program.getCompilerOptions().configFile;
5187
if (!cfg) return;
@@ -62,13 +98,13 @@ namespace ts.refactor {
6298
}
6399

64100
function getNewStatements(
65-
oldFile: SourceFile, usage: UsageInfo, changes: textChanges.ChangeTracker, toMove: ReadonlyArray<Statement>, program: Program, newModuleName: string,
101+
oldFile: SourceFile, usage: UsageInfo, changes: textChanges.ChangeTracker, toMove: ToMove, program: Program, newModuleName: string,
66102
): ReadonlyArray<Statement> {
67103
const checker = program.getTypeChecker();
68104

69105
if (!oldFile.externalModuleIndicator && !oldFile.commonJsModuleIndicator) {
70-
changes.deleteNodeRange(oldFile, first(toMove), last(toMove));
71-
return toMove;
106+
deleteMovedStatements(oldFile, toMove.ranges, changes);
107+
return toMove.all;
72108
}
73109

74110
const useEs6ModuleSyntax = !!oldFile.externalModuleIndicator;
@@ -77,17 +113,23 @@ namespace ts.refactor {
77113
changes.insertNodeBefore(oldFile, oldFile.statements[0], importsFromNewFile, /*blankLineBetween*/ true);
78114
}
79115

80-
deleteUnusedOldImports(oldFile, toMove, changes, usage.unusedImportsFromOldFile, checker);
81-
changes.deleteNodeRange(oldFile, first(toMove), last(toMove));
116+
deleteUnusedOldImports(oldFile, toMove.all, changes, usage.unusedImportsFromOldFile, checker);
117+
deleteMovedStatements(oldFile, toMove.ranges, changes);
82118

83119
updateImportsInOtherFiles(changes, program, oldFile, usage.movedSymbols, newModuleName);
84120

85121
return [
86122
...getNewFileImportsAndAddExportInOldFile(oldFile, usage.oldImportsNeededByNewFile, usage.newFileImportsFromOldFile, changes, checker, useEs6ModuleSyntax),
87-
...addExports(oldFile, toMove, usage.oldFileImportsFromNewFile, useEs6ModuleSyntax),
123+
...addExports(oldFile, toMove.all, usage.oldFileImportsFromNewFile, useEs6ModuleSyntax),
88124
];
89125
}
90126

127+
function deleteMovedStatements(sourceFile: SourceFile, moved: ReadonlyArray<StatementRange>, changes: textChanges.ChangeTracker) {
128+
for (const { first, last } of moved) {
129+
changes.deleteNodeRange(sourceFile, first, last);
130+
}
131+
}
132+
91133
function deleteUnusedOldImports(oldFile: SourceFile, toMove: ReadonlyArray<Statement>, changes: textChanges.ChangeTracker, toDelete: ReadonlySymbolSet, checker: TypeChecker) {
92134
for (const statement of oldFile.statements) {
93135
if (contains(toMove, statement)) continue;

‎tests/cases/fourslash/moveToNewFile_exportImport.ts‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,12 @@
99
verify.moveToNewFile({
1010
newFileContents: {
1111
"/a.ts":
12-
`import { M } from "./M";
13-
14-
export namespace N { export const x = 0; }
12+
`export namespace N { export const x = 0; }
13+
import M = N;
1514
M;`,
1615

17-
"/M.ts":
16+
"/O.ts":
1817
`import { N } from "./a";
19-
export import M = N;
2018
export import O = N;`,
2119
},
2220
});

‎tests/cases/fourslash/moveToNewFile_moveImport.ts‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,13 @@
55
////a;|]
66
////b;
77

8-
//verify.noMoveToNewFile();
98
verify.moveToNewFile({
109
newFileContents: {
1110
"/a.ts":
12-
`b;`,
11+
`import { b } from "m";
12+
b;`,
1313
"/newFile.ts":
1414
`import { a } from "m";
15-
import { a, b } from "m";
1615
a;`,
1716
}
1817
});

0 commit comments

Comments
 (0)