Skip to content

Commit ce4862c

Browse files
authored
Updating move to file (microsoft#58257)
1 parent 8d2e2d5 commit ce4862c

28 files changed

Lines changed: 273 additions & 508 deletions

src/services/codefixes/importFixes.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -404,6 +404,15 @@ function createImportAdderWorker(sourceFile: SourceFile | FutureSourceFile, prog
404404
entry.namedImports.set(symbolName, reduceAddAsTypeOnlyValues(prevValue, addAsTypeOnly));
405405
break;
406406
case ImportKind.CommonJS:
407+
if (compilerOptions.verbatimModuleSyntax) {
408+
const prevValue = (entry.namedImports ||= new Map()).get(symbolName);
409+
entry.namedImports.set(symbolName, reduceAddAsTypeOnlyValues(prevValue, addAsTypeOnly));
410+
}
411+
else {
412+
Debug.assert(entry.namespaceLikeImport === undefined || entry.namespaceLikeImport.name === symbolName, "Namespacelike import shoudl be missing or match symbolName");
413+
entry.namespaceLikeImport = { importKind, name: symbolName, addAsTypeOnly };
414+
}
415+
break;
407416
case ImportKind.Namespace:
408417
Debug.assert(entry.namespaceLikeImport === undefined || entry.namespaceLikeImport.name === symbolName, "Namespacelike import shoudl be missing or match symbolName");
409418
entry.namespaceLikeImport = { importKind, name: symbolName, addAsTypeOnly };

src/services/refactors/helpers.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,15 @@
1+
import {
2+
codefix,
3+
Debug,
4+
findAncestor,
5+
isAnyImportOrRequireStatement,
6+
Program,
7+
skipAlias,
8+
SourceFile,
9+
Symbol,
10+
TypeChecker,
11+
} from "../_namespaces/ts";
12+
import { addImportsForMovedSymbols } from "./moveToFile";
113
/**
214
* Returned by refactor functions when some error message needs to be surfaced to users.
315
*
@@ -26,3 +38,30 @@ export function refactorKindBeginsWith(known: string, requested: string | undefi
2638
if (!requested) return true;
2739
return known.substr(0, requested.length) === requested;
2840
}
41+
42+
/** @internal */
43+
export function addTargetFileImports(
44+
oldFile: SourceFile,
45+
importsToCopy: Map<Symbol, [boolean, codefix.ImportOrRequireAliasDeclaration | undefined]>,
46+
targetFileImportsFromOldFile: Map<Symbol, boolean>,
47+
checker: TypeChecker,
48+
program: Program,
49+
importAdder: codefix.ImportAdder,
50+
) {
51+
/**
52+
* Recomputing the imports is preferred with importAdder because it manages multiple import additions for a file and writes then to a ChangeTracker,
53+
* but sometimes it fails because of unresolved imports from files, or when a source file is not available for the target file (in this case when creating a new file).
54+
* So in that case, fall back to copying the import verbatim.
55+
*/
56+
importsToCopy.forEach(([isValidTypeOnlyUseSite, declaration], symbol) => {
57+
const targetSymbol = skipAlias(symbol, checker);
58+
if (checker.isUnknownSymbol(targetSymbol)) {
59+
importAdder.addVerbatimImport(Debug.checkDefined(declaration ?? findAncestor(symbol.declarations?.[0], isAnyImportOrRequireStatement)));
60+
}
61+
else {
62+
importAdder.addImportFromExportedSymbol(targetSymbol, isValidTypeOnlyUseSite, declaration);
63+
}
64+
});
65+
66+
addImportsForMovedSymbols(targetFileImportsFromOldFile, oldFile.fileName, importAdder, program);
67+
}

src/services/refactors/moveToFile.ts

Lines changed: 139 additions & 287 deletions
Large diffs are not rendered by default.
Lines changed: 11 additions & 129 deletions
Original file line numberDiff line numberDiff line change
@@ -1,56 +1,31 @@
11
import {
2-
append,
32
ApplicableRefactorInfo,
3+
codefix,
4+
createFutureSourceFile,
45
Debug,
56
Diagnostics,
67
emptyArray,
7-
fileShouldUseJavaScriptRequire,
8-
getBaseFileName,
98
getLineAndCharacterOfPosition,
109
getLocaleSpecificMessage,
11-
getQuotePreference,
12-
hasSyntacticModifier,
1310
hostGetCanonicalFileName,
14-
Identifier,
15-
insertImports,
16-
isPrologueDirective,
1711
LanguageServiceHost,
1812
last,
19-
ModifierFlags,
20-
nodeSeenTracker,
13+
ModuleKind,
2114
Program,
22-
QuotePreference,
15+
RefactorContext,
2316
RefactorEditInfo,
2417
SourceFile,
25-
Symbol,
26-
SyntaxKind,
27-
takeWhile,
2818
textChanges,
29-
TypeChecker,
3019
UserPreferences,
3120
} from "../_namespaces/ts";
3221
import {
33-
addExports,
34-
addExportToChanges,
3522
addNewFileToTsconfig,
3623
createNewFileName,
37-
createOldFileImportsFromTargetFile,
38-
deleteMovedStatements,
39-
deleteUnusedOldImports,
40-
filterImport,
41-
forEachImportInStatement,
24+
getNewStatementsAndRemoveFromOldFile,
4225
getStatementsToMove,
43-
getTopLevelDeclarationStatement,
4426
getUsageInfo,
45-
isTopLevelDeclaration,
46-
makeImportOrRequire,
47-
moduleSpecifierFromImport,
48-
nameOfTopLevelDeclaration,
4927
registerRefactor,
50-
SupportedImportStatement,
5128
ToMove,
52-
updateImportsInOtherFiles,
53-
UsageInfo,
5429
} from "../_namespaces/ts.refactor";
5530

5631
const refactorName = "Move to a new file";
@@ -81,112 +56,19 @@ registerRefactor(refactorName, {
8156
getEditsForAction: function getRefactorEditsToMoveToNewFile(context, actionName): RefactorEditInfo {
8257
Debug.assert(actionName === refactorName, "Wrong refactor invoked");
8358
const statements = Debug.checkDefined(getStatementsToMove(context));
84-
const edits = textChanges.ChangeTracker.with(context, t => doChange(context.file, context.program, statements, t, context.host, context.preferences));
59+
const edits = textChanges.ChangeTracker.with(context, t => doChange(context.file, context.program, statements, t, context.host, context, context.preferences));
8560
return { edits, renameFilename: undefined, renameLocation: undefined };
8661
},
8762
});
8863

89-
function doChange(oldFile: SourceFile, program: Program, toMove: ToMove, changes: textChanges.ChangeTracker, host: LanguageServiceHost, preferences: UserPreferences): void {
64+
function doChange(oldFile: SourceFile, program: Program, toMove: ToMove, changes: textChanges.ChangeTracker, host: LanguageServiceHost, context: RefactorContext, preferences: UserPreferences): void {
9065
const checker = program.getTypeChecker();
9166
const usage = getUsageInfo(oldFile, toMove.all, checker);
9267
const newFilename = createNewFileName(oldFile, program, host, toMove);
68+
const newSourceFile = createFutureSourceFile(newFilename, oldFile.externalModuleIndicator ? ModuleKind.ESNext : oldFile.commonJsModuleIndicator ? ModuleKind.CommonJS : undefined, program, host);
9369

94-
// If previous file was global, this is easy.
95-
changes.createNewFile(oldFile, newFilename, getNewStatementsAndRemoveFromOldFile(oldFile, usage, changes, toMove, program, host, newFilename, preferences));
96-
70+
const importAdderForOldFile = codefix.createImportAdder(oldFile, context.program, context.preferences, context.host);
71+
const importAdderForNewFile = codefix.createImportAdder(newSourceFile, context.program, context.preferences, context.host);
72+
getNewStatementsAndRemoveFromOldFile(oldFile, newSourceFile, usage, changes, toMove, program, host, preferences, importAdderForNewFile, importAdderForOldFile);
9773
addNewFileToTsconfig(program, changes, oldFile.fileName, newFilename, hostGetCanonicalFileName(host));
9874
}
99-
100-
function getNewStatementsAndRemoveFromOldFile(
101-
oldFile: SourceFile,
102-
usage: UsageInfo,
103-
changes: textChanges.ChangeTracker,
104-
toMove: ToMove,
105-
program: Program,
106-
host: LanguageServiceHost,
107-
newFilename: string,
108-
preferences: UserPreferences,
109-
) {
110-
const checker = program.getTypeChecker();
111-
const prologueDirectives = takeWhile(oldFile.statements, isPrologueDirective);
112-
if (oldFile.externalModuleIndicator === undefined && oldFile.commonJsModuleIndicator === undefined && usage.oldImportsNeededByTargetFile.size === 0) {
113-
deleteMovedStatements(oldFile, toMove.ranges, changes);
114-
return [...prologueDirectives, ...toMove.all];
115-
}
116-
117-
const useEsModuleSyntax = !fileShouldUseJavaScriptRequire(newFilename, program, host, !!oldFile.commonJsModuleIndicator);
118-
const quotePreference = getQuotePreference(oldFile, preferences);
119-
const importsFromNewFile = createOldFileImportsFromTargetFile(oldFile, usage.oldFileImportsFromTargetFile, newFilename, program, host, useEsModuleSyntax, quotePreference);
120-
if (importsFromNewFile) {
121-
insertImports(changes, oldFile, importsFromNewFile, /*blankLineBetween*/ true, preferences);
122-
}
123-
124-
deleteUnusedOldImports(oldFile, toMove.all, changes, usage.unusedImportsFromOldFile, checker);
125-
deleteMovedStatements(oldFile, toMove.ranges, changes);
126-
updateImportsInOtherFiles(changes, program, host, oldFile, usage.movedSymbols, newFilename, quotePreference);
127-
128-
const imports = getNewFileImportsAndAddExportInOldFile(oldFile, usage.oldImportsNeededByTargetFile, usage.targetFileImportsFromOldFile, changes, checker, program, host, useEsModuleSyntax, quotePreference);
129-
const body = addExports(oldFile, toMove.all, usage.oldFileImportsFromTargetFile, useEsModuleSyntax);
130-
if (imports.length && body.length) {
131-
return [
132-
...prologueDirectives,
133-
...imports,
134-
SyntaxKind.NewLineTrivia as const,
135-
...body,
136-
];
137-
}
138-
139-
return [
140-
...prologueDirectives,
141-
...imports,
142-
...body,
143-
];
144-
}
145-
146-
function getNewFileImportsAndAddExportInOldFile(
147-
oldFile: SourceFile,
148-
importsToCopy: Map<Symbol, boolean>,
149-
newFileImportsFromOldFile: Set<Symbol>,
150-
changes: textChanges.ChangeTracker,
151-
checker: TypeChecker,
152-
program: Program,
153-
host: LanguageServiceHost,
154-
useEsModuleSyntax: boolean,
155-
quotePreference: QuotePreference,
156-
): readonly SupportedImportStatement[] {
157-
const copiedOldImports: SupportedImportStatement[] = [];
158-
for (const oldStatement of oldFile.statements) {
159-
forEachImportInStatement(oldStatement, i => {
160-
append(copiedOldImports, filterImport(i, moduleSpecifierFromImport(i), name => importsToCopy.has(checker.getSymbolAtLocation(name)!)));
161-
});
162-
}
163-
164-
// Also, import things used from the old file, and insert 'export' modifiers as necessary in the old file.
165-
let oldFileDefault: Identifier | undefined;
166-
const oldFileNamedImports: string[] = [];
167-
const markSeenTop = nodeSeenTracker(); // Needed because multiple declarations may appear in `const x = 0, y = 1;`.
168-
newFileImportsFromOldFile.forEach(symbol => {
169-
if (!symbol.declarations) {
170-
return;
171-
}
172-
for (const decl of symbol.declarations) {
173-
if (!isTopLevelDeclaration(decl)) continue;
174-
const name = nameOfTopLevelDeclaration(decl);
175-
if (!name) continue;
176-
177-
const top = getTopLevelDeclarationStatement(decl);
178-
if (markSeenTop(top)) {
179-
addExportToChanges(oldFile, top, name, changes, useEsModuleSyntax);
180-
}
181-
if (hasSyntacticModifier(decl, ModifierFlags.Default)) {
182-
oldFileDefault = name;
183-
}
184-
else {
185-
oldFileNamedImports.push(name.text);
186-
}
187-
}
188-
});
189-
190-
append(copiedOldImports, makeImportOrRequire(oldFile, oldFileDefault, oldFileNamedImports, getBaseFileName(oldFile.fileName), program, host, useEsModuleSyntax, quotePreference));
191-
return copiedOldImports;
192-
}

src/services/utilities.ts

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2507,11 +2507,6 @@ export function moduleResolutionUsesNodeModules(moduleResolution: ModuleResoluti
25072507
|| moduleResolution === ModuleResolutionKind.Bundler;
25082508
}
25092509

2510-
/** @internal */
2511-
export function makeImportIfNecessary(defaultImport: Identifier | undefined, namedImports: readonly ImportSpecifier[] | undefined, moduleSpecifier: string, quotePreference: QuotePreference): ImportDeclaration | undefined {
2512-
return defaultImport || namedImports && namedImports.length ? makeImport(defaultImport, namedImports, moduleSpecifier, quotePreference) : undefined;
2513-
}
2514-
25152510
/** @internal */
25162511
export function makeImport(defaultImport: Identifier | undefined, namedImports: readonly ImportSpecifier[] | undefined, moduleSpecifier: string | Expression, quotePreference: QuotePreference, isTypeOnly?: boolean): ImportDeclaration {
25172512
return factory.createImportDeclaration(

tests/baselines/reference/tsserver/fourslashServer/moveToFile_emptyTargetFile.js

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -241,13 +241,13 @@ Info seq [hh:mm:ss:mss] response:
241241
]
242242
},
243243
{
244-
"name": "Move to a new file",
245-
"description": "Move to a new file",
244+
"name": "Move to file",
245+
"description": "Move to file",
246246
"actions": [
247247
{
248-
"name": "Move to a new file",
249-
"description": "Move to a new file",
250-
"kind": "refactor.move.newFile",
248+
"name": "Move to file",
249+
"description": "Move to file",
250+
"kind": "refactor.move.file",
251251
"range": {
252252
"start": {
253253
"line": 1,
@@ -262,13 +262,13 @@ Info seq [hh:mm:ss:mss] response:
262262
]
263263
},
264264
{
265-
"name": "Move to file",
266-
"description": "Move to file",
265+
"name": "Move to a new file",
266+
"description": "Move to a new file",
267267
"actions": [
268268
{
269-
"name": "Move to file",
270-
"description": "Move to file",
271-
"kind": "refactor.move.file",
269+
"name": "Move to a new file",
270+
"description": "Move to a new file",
271+
"kind": "refactor.move.newFile",
272272
"range": {
273273
"start": {
274274
"line": 1,

tests/baselines/reference/tsserver/getApplicableRefactors/returns-the-affected-range-of-text-for-'move-to-file'-and-'move-to-new-file'-refactors.js

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -107,13 +107,13 @@ Info seq [hh:mm:ss:mss] response:
107107
{
108108
"response": [
109109
{
110-
"name": "Move to a new file",
111-
"description": "Move to a new file",
110+
"name": "Move to file",
111+
"description": "Move to file",
112112
"actions": [
113113
{
114-
"name": "Move to a new file",
115-
"description": "Move to a new file",
116-
"kind": "refactor.move.newFile",
114+
"name": "Move to file",
115+
"description": "Move to file",
116+
"kind": "refactor.move.file",
117117
"range": {
118118
"start": {
119119
"line": 1,
@@ -128,13 +128,13 @@ Info seq [hh:mm:ss:mss] response:
128128
]
129129
},
130130
{
131-
"name": "Move to file",
132-
"description": "Move to file",
131+
"name": "Move to a new file",
132+
"description": "Move to a new file",
133133
"actions": [
134134
{
135-
"name": "Move to file",
136-
"description": "Move to file",
137-
"kind": "refactor.move.file",
135+
"name": "Move to a new file",
136+
"description": "Move to a new file",
137+
"kind": "refactor.move.newFile",
138138
"range": {
139139
"start": {
140140
"line": 1,

tests/cases/fourslash/moveToFile_blankExistingFile.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,6 @@ verify.moveToFile({
2020
"/bar.ts":
2121
`//
2222
import { p } from './a';
23-
24-
2523
import { b } from './other';
2624
2725

tests/cases/fourslash/moveToFile_consistentQuoteStyle.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ verify.moveToFile({
2121

2222
"/bar.ts":
2323
`import { a } from './a';
24-
2524
import { b } from './other';
2625
2726
export const tt = 2;

tests/cases/fourslash/moveToFile_differentDirectories2.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ y;`,
2424

2525
"/src/dir2/bar.ts":
2626
`import { a } from '../dir1/a';
27-
2827
import { b } from '../dir1/other';
2928
3029

0 commit comments

Comments
 (0)