From eb962faf97b7f95d51c1359ba76326ff182098db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20Burzy=C5=84ski?= Date: Mon, 21 Oct 2024 19:11:36 +0200 Subject: [PATCH] Fixed crashes when moving namespace imports to other files in refactorings --- src/services/codefixes/importFixes.ts | 5 ++- src/services/exportInfoMap.ts | 13 ++++++++ .../unittests/tsserver/autoImportProvider.ts | 5 ++- .../fourslash/moveToFile_namespaceImport1.ts | 26 +++++++++++++++ .../fourslash/moveToFile_namespaceImport2.ts | 26 +++++++++++++++ .../fourslash/moveToFile_namespaceImport3.ts | 30 +++++++++++++++++ .../fourslash/moveToFile_namespaceImport4.ts | 29 +++++++++++++++++ .../fourslash/moveToFile_namespaceImport5.ts | 32 +++++++++++++++++++ .../fourslash/moveToFile_namespaceImport6.ts | 29 +++++++++++++++++ .../fourslash/moveToFile_namespaceImport7.ts | 29 +++++++++++++++++ .../fourslash/moveToFile_namespaceImport8.ts | 28 ++++++++++++++++ ...t.ts => moveToNewFile_namespaceImport1.ts} | 0 .../moveToNewFile_namespaceImport2.ts | 23 +++++++++++++ .../moveToNewFile_namespaceImport3.ts | 24 ++++++++++++++ 14 files changed, 297 insertions(+), 2 deletions(-) create mode 100644 tests/cases/fourslash/moveToFile_namespaceImport1.ts create mode 100644 tests/cases/fourslash/moveToFile_namespaceImport2.ts create mode 100644 tests/cases/fourslash/moveToFile_namespaceImport3.ts create mode 100644 tests/cases/fourslash/moveToFile_namespaceImport4.ts create mode 100644 tests/cases/fourslash/moveToFile_namespaceImport5.ts create mode 100644 tests/cases/fourslash/moveToFile_namespaceImport6.ts create mode 100644 tests/cases/fourslash/moveToFile_namespaceImport7.ts create mode 100644 tests/cases/fourslash/moveToFile_namespaceImport8.ts rename tests/cases/fourslash/{moveToNewFile_namespaceImport.ts => moveToNewFile_namespaceImport1.ts} (100%) create mode 100644 tests/cases/fourslash/moveToNewFile_namespaceImport2.ts create mode 100644 tests/cases/fourslash/moveToNewFile_namespaceImport3.ts diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index 0387702de5eef..ea0794b65a324 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -81,6 +81,7 @@ import { insertImports, InternalSymbolName, isExternalModuleReference, + isExternalModuleSymbol, isFullSourceFile, isIdentifier, isImportable, @@ -276,7 +277,7 @@ function createImportAdderWorker(sourceFile: SourceFile | FutureSourceFile, prog } function addImportFromExportedSymbol(exportedSymbol: Symbol, isValidTypeOnlyUseSite?: boolean, referenceImport?: ImportOrRequireAliasDeclaration) { - const moduleSymbol = Debug.checkDefined(exportedSymbol.parent); + const moduleSymbol = Debug.checkDefined(isExternalModuleSymbol(exportedSymbol) ? exportedSymbol : exportedSymbol.parent); const symbolName = getNameForExportedSymbol(exportedSymbol, getEmitScriptTarget(compilerOptions)); const checker = program.getTypeChecker(); const symbol = checker.getMergedSymbol(skipAlias(exportedSymbol, checker)); @@ -1447,6 +1448,8 @@ export function getImportKind(importingFile: SourceFile | FutureSourceFile, expo return ImportKind.Named; case ExportKind.Default: return ImportKind.Default; + case ExportKind.Module: + return ImportKind.Namespace; case ExportKind.ExportEquals: return getExportEqualsImportKind(importingFile, program.getCompilerOptions(), !!forceImportKeyword); case ExportKind.UMD: diff --git a/src/services/exportInfoMap.ts b/src/services/exportInfoMap.ts index f1f88c6ad42ef..85725ec8e8121 100644 --- a/src/services/exportInfoMap.ts +++ b/src/services/exportInfoMap.ts @@ -73,6 +73,7 @@ export const enum ExportKind { Named, Default, ExportEquals, + Module, UMD, } @@ -557,6 +558,18 @@ export function getExportInfoMap(importingFile: SourceFile | FutureSourceFile, h if (++moduleCount % 100 === 0) cancellationToken?.throwIfCancellationRequested(); const seenExports = new Set<__String>(); const checker = program.getTypeChecker(); + if (isImportableSymbol(moduleSymbol, checker)) { + cache.add( + importingFile.path, + moduleSymbol, + moduleSymbol.escapedName, + moduleSymbol, + moduleFile, + ExportKind.Module, + isFromPackageJson, + checker, + ); + } const defaultInfo = getDefaultLikeExportInfo(moduleSymbol, checker); // Note: I think we shouldn't actually see resolved module symbols here, but weird merges // can cause it to happen: see 'completionsImport_mergedReExport.ts' diff --git a/src/testRunner/unittests/tsserver/autoImportProvider.ts b/src/testRunner/unittests/tsserver/autoImportProvider.ts index cc3d1e052045b..b37196e76a89e 100644 --- a/src/testRunner/unittests/tsserver/autoImportProvider.ts +++ b/src/testRunner/unittests/tsserver/autoImportProvider.ts @@ -323,9 +323,12 @@ describe("unittests:: tsserver:: autoImportProvider::", () => { assert.lengthOf(info, 1); seenSymbolNames.add(symbolName); }); - assert.equal(seenSymbolNames.size, 2); + assert.equal(seenSymbolNames.size, 5); assert.ok(seenSymbolNames.has("Stats")); assert.ok(seenSymbolNames.has("Volume")); + assert.ok(seenSymbolNames.has('"/user/username/projects/project/index"')); + assert.ok(seenSymbolNames.has('"/user/username/projects/project/node_modules/@types/node/index"')); + assert.ok(seenSymbolNames.has('"/user/username/projects/project/node_modules/memfs/lib/index"')); baselineTsserverLogs("autoImportProvider", "Shared source files between AutoImportProvider and main program", session); }); }); diff --git a/tests/cases/fourslash/moveToFile_namespaceImport1.ts b/tests/cases/fourslash/moveToFile_namespaceImport1.ts new file mode 100644 index 0000000000000..7467c683748fd --- /dev/null +++ b/tests/cases/fourslash/moveToFile_namespaceImport1.ts @@ -0,0 +1,26 @@ +/// + +// @Filename: /a.ts +//// export type ExecutionPoint = string; +//// +// @Filename: /b.ts +//// import * as A from "./a"; +//// +//// [|async function pointToFrameExecutionPoint(point: A.ExecutionPoint) {}|] +//// +// @Filename: /point.ts +//// + +verify.moveToFile({ + newFileContents: { + "/b.ts": ` +`, + "/point.ts": +`import * as A from "./a"; + + +async function pointToFrameExecutionPoint(point: A.ExecutionPoint) { } +`, + }, + interactiveRefactorArguments: { targetFile: "/point.ts" }, +}); diff --git a/tests/cases/fourslash/moveToFile_namespaceImport2.ts b/tests/cases/fourslash/moveToFile_namespaceImport2.ts new file mode 100644 index 0000000000000..8b02f59dcba7f --- /dev/null +++ b/tests/cases/fourslash/moveToFile_namespaceImport2.ts @@ -0,0 +1,26 @@ +/// + +// @Filename: /a.ts +//// export type ExecutionPoint = string; +//// +// @Filename: /b.ts +//// import * as P from "./a"; +//// +//// [|async function fn(point: typeof P) {}|] +//// +// @Filename: /c.ts +//// + +verify.moveToFile({ + newFileContents: { + "/b.ts": ` +`, + "/c.ts": +`import * as P from "./a"; + + +async function fn(point: typeof P) { } +`, + }, + interactiveRefactorArguments: { targetFile: "/c.ts" }, +}); diff --git a/tests/cases/fourslash/moveToFile_namespaceImport3.ts b/tests/cases/fourslash/moveToFile_namespaceImport3.ts new file mode 100644 index 0000000000000..d65e8a5225e3c --- /dev/null +++ b/tests/cases/fourslash/moveToFile_namespaceImport3.ts @@ -0,0 +1,30 @@ +/// + +// @Filename: /a.ts +//// export type ExecutionPoint = string; +//// +// @Filename: /b.ts +//// import * as A from "./a"; +//// +//// [|async function fn1(point: A.ExecutionPoint) {}|] +//// +//// async function fn2(point: A.ExecutionPoint) {} +//// +// @Filename: /point.ts +//// + +verify.moveToFile({ + newFileContents: { + "/b.ts": `import * as A from "./a"; + +async function fn2(point: A.ExecutionPoint) {} +`, + "/point.ts": +`import * as A from "./a"; + + +async function fn1(point: A.ExecutionPoint) { } +`, + }, + interactiveRefactorArguments: { targetFile: "/point.ts" }, +}); diff --git a/tests/cases/fourslash/moveToFile_namespaceImport4.ts b/tests/cases/fourslash/moveToFile_namespaceImport4.ts new file mode 100644 index 0000000000000..abf5b9e961333 --- /dev/null +++ b/tests/cases/fourslash/moveToFile_namespaceImport4.ts @@ -0,0 +1,29 @@ +/// + +// @Filename: /a.ts +//// export type ExecutionPoint = string; +//// +// @Filename: /b.ts +//// import * as A from "./a"; +//// +//// [|async function fn1(point: A.ExecutionPoint) {}|] +//// +// @Filename: /point.ts +//// import { ExecutionPoint } from "./a"; +//// +//// async function fn2(point: ExecutionPoint) {} + +verify.moveToFile({ + newFileContents: { + "/b.ts": ` +`, + "/point.ts": +`import * as A from "./a"; +import { ExecutionPoint } from "./a"; + +async function fn2(point: ExecutionPoint) {} +async function fn1(point: A.ExecutionPoint) { } +`, + }, + interactiveRefactorArguments: { targetFile: "/point.ts" }, +}); diff --git a/tests/cases/fourslash/moveToFile_namespaceImport5.ts b/tests/cases/fourslash/moveToFile_namespaceImport5.ts new file mode 100644 index 0000000000000..09002fd7ae660 --- /dev/null +++ b/tests/cases/fourslash/moveToFile_namespaceImport5.ts @@ -0,0 +1,32 @@ +/// + +// @Filename: /a.ts +//// export type ExecutionPoint = string; +//// +// @Filename: /b.ts +//// export type ExecutionPoint = { point: string }; +//// +// @Filename: /c.ts +//// import * as A from "./a"; +//// +//// [|async function fn1(point: A.ExecutionPoint) {}|] +//// +// @Filename: /point.ts +//// import * as A from "./c"; +//// +//// async function fn2(point: A.ExecutionPoint) {} + +verify.moveToFile({ + newFileContents: { + "/c.ts": ` +`, + "/point.ts": +`import * as A from "./a"; +import * as A from "./c"; + +async function fn2(point: A.ExecutionPoint) {} +async function fn1(point: A.ExecutionPoint) { } +`, + }, + interactiveRefactorArguments: { targetFile: "/point.ts" }, +}); diff --git a/tests/cases/fourslash/moveToFile_namespaceImport6.ts b/tests/cases/fourslash/moveToFile_namespaceImport6.ts new file mode 100644 index 0000000000000..af2d521e7adc1 --- /dev/null +++ b/tests/cases/fourslash/moveToFile_namespaceImport6.ts @@ -0,0 +1,29 @@ +/// + +// @Filename: /a.ts +//// export type ExecutionPoint = string; +//// +// @Filename: /b.ts +//// import * as A from "./a"; +//// +//// [|async function fn1(point: A.ExecutionPoint) {}|] +//// +// @Filename: /point.ts +//// import * as A from "./a"; +//// +//// async function fn2(point: A.ExecutionPoint) {} + +verify.moveToFile({ + newFileContents: { + "/b.ts": ` +`, + "/point.ts": +`import * as A from "./a"; +import * as A from "./a"; + +async function fn2(point: A.ExecutionPoint) {} +async function fn1(point: A.ExecutionPoint) { } +`, + }, + interactiveRefactorArguments: { targetFile: "/point.ts" }, +}); diff --git a/tests/cases/fourslash/moveToFile_namespaceImport7.ts b/tests/cases/fourslash/moveToFile_namespaceImport7.ts new file mode 100644 index 0000000000000..5162f608d1f5d --- /dev/null +++ b/tests/cases/fourslash/moveToFile_namespaceImport7.ts @@ -0,0 +1,29 @@ +/// + +// @Filename: /a.ts +//// export type ExecutionPoint = string; +//// +// @Filename: /b.ts +//// import * as A from "./a"; +//// +//// [|async function fn1(point: A.ExecutionPoint) {}|] +//// +// @Filename: /point.ts +//// import * as A1 from "./a"; +//// +//// async function fn2(point: A1.ExecutionPoint) {} + +verify.moveToFile({ + newFileContents: { + "/b.ts": ` +`, + "/point.ts": +`import * as A from "./a"; +import * as A1 from "./a"; + +async function fn2(point: A1.ExecutionPoint) {} +async function fn1(point: A.ExecutionPoint) { } +`, + }, + interactiveRefactorArguments: { targetFile: "/point.ts" }, +}); diff --git a/tests/cases/fourslash/moveToFile_namespaceImport8.ts b/tests/cases/fourslash/moveToFile_namespaceImport8.ts new file mode 100644 index 0000000000000..fa9b2e4bc5a17 --- /dev/null +++ b/tests/cases/fourslash/moveToFile_namespaceImport8.ts @@ -0,0 +1,28 @@ +/// + +// @Filename: /a.ts +//// export type ExecutionPoint = string; +//// +// @Filename: /b.ts +//// import * as A from "./a"; +//// +//// [|async function fn1(point: A.ExecutionPoint) {}|] +//// +// @Filename: /point.ts +//// type A = {}; +//// export {}; + +verify.moveToFile({ + newFileContents: { + "/b.ts": ` +`, + "/point.ts": +`import * as A from "./a"; + +type A = {}; +export {}; +async function fn1(point: A.ExecutionPoint) { } +`, + }, + interactiveRefactorArguments: { targetFile: "/point.ts" }, +}); diff --git a/tests/cases/fourslash/moveToNewFile_namespaceImport.ts b/tests/cases/fourslash/moveToNewFile_namespaceImport1.ts similarity index 100% rename from tests/cases/fourslash/moveToNewFile_namespaceImport.ts rename to tests/cases/fourslash/moveToNewFile_namespaceImport1.ts diff --git a/tests/cases/fourslash/moveToNewFile_namespaceImport2.ts b/tests/cases/fourslash/moveToNewFile_namespaceImport2.ts new file mode 100644 index 0000000000000..0d9ac04171d58 --- /dev/null +++ b/tests/cases/fourslash/moveToNewFile_namespaceImport2.ts @@ -0,0 +1,23 @@ +/// + +// @Filename: /a.ts +//// export type ExecutionPoint = string; +//// +// @Filename: /b.ts +//// import * as A from "./a"; +//// +//// [|async function pointToFrameExecutionPoint(point: A.ExecutionPoint) {}|] +//// + +verify.moveToNewFile({ + newFileContents: { + "/b.ts": ` +`, + "/pointToFrameExecutionPoint.ts": +`import * as A from "./a"; + + +async function pointToFrameExecutionPoint(point: A.ExecutionPoint) { } +`, + }, +}); diff --git a/tests/cases/fourslash/moveToNewFile_namespaceImport3.ts b/tests/cases/fourslash/moveToNewFile_namespaceImport3.ts new file mode 100644 index 0000000000000..ee55a3c155573 --- /dev/null +++ b/tests/cases/fourslash/moveToNewFile_namespaceImport3.ts @@ -0,0 +1,24 @@ +/// + +// @Filename: /a.ts +//// export type ExecutionPoint = string; +//// +// @Filename: /b.ts +//// import * as A from "./a"; +//// +//// [|async function fn(a: typeof A) {}|] +//// + +verify.moveToNewFile({ + newFileContents: { + "/b.ts": +` +`, + "/fn.ts": +`import * as A from "./a"; + + +async function fn(a: typeof A) { } +`, + }, +});