Skip to content

Commit

Permalink
fix autoimports crash: generate namespace and other module symbol imp…
Browse files Browse the repository at this point in the history 
…orts (#60333)
  • Loading branch information
@iisaduan
iisaduan authored Nov 1, 2024
1 parent 32513a7 commit 3e61718
Show file tree
Hide file tree
Showing 21 changed files with 2,361 additions and 22 deletions.
78 changes: 76 additions & 2 deletions src/services/codefixes/importFixes.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,12 +80,15 @@ import {
ImportSpecifier,
insertImports,
InternalSymbolName,
isDefaultImport,
isExternalModuleReference,
isFullSourceFile,
isIdentifier,
isImportable,
isImportClause,
isImportDeclaration,
isImportEqualsDeclaration,
isImportSpecifier,
isIntrinsicJsxName,
isJSDocImportTag,
isJsxClosingElement,
Expand Down Expand Up @@ -228,6 +231,7 @@ export interface ImportAdder {
addImportFromDiagnostic: (diagnostic: DiagnosticWithLocation, context: CodeFixContextBase) => void;
addImportFromExportedSymbol: (exportedSymbol: Symbol, isValidTypeOnlyUseSite?: boolean, referenceImport?: ImportOrRequireAliasDeclaration) => void;
addImportForNonExistentExport: (exportName: string, exportingFileName: string, exportKind: ExportKind, exportedMeanings: SymbolFlags, isImportUsageValidAsTypeOnly: boolean) => void;
addImportForModuleSymbol: (symbolAlias: Symbol, isValidTypeOnlyUseSite: boolean, referenceImport: ImportOrRequireAliasDeclaration) => void;
addImportForUnresolvedIdentifier: (context: CodeFixContextBase, symbolToken: Identifier, useAutoImportProvider: boolean) => void;
addVerbatimImport: (declaration: AnyImportOrRequireStatement | ImportOrRequireAliasDeclaration) => void;
removeExistingImport: (declaration: ImportOrRequireAliasDeclaration) => void;
Expand Down Expand Up @@ -257,7 +261,7 @@ function createImportAdderWorker(sourceFile: SourceFile | FutureSourceFile, prog
type NewImportsKey = `${0 | 1}|${string}`;
/** Use `getNewImportEntry` for access */
const newImports = new Map<NewImportsKey, Mutable<ImportsCollection & { useRequire: boolean; }>>();
return { addImportFromDiagnostic, addImportFromExportedSymbol, writeFixes, hasFixes, addImportForUnresolvedIdentifier, addImportForNonExistentExport, removeExistingImport, addVerbatimImport };
return { addImportFromDiagnostic, addImportFromExportedSymbol, addImportForModuleSymbol, writeFixes, hasFixes, addImportForUnresolvedIdentifier, addImportForNonExistentExport, removeExistingImport, addVerbatimImport };

function addVerbatimImport(declaration: AnyImportOrRequireStatement | ImportOrRequireAliasDeclaration) {
verbatimImports.add(declaration);
Expand All @@ -276,7 +280,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(exportedSymbol.parent, "Expected exported symbol to have module symbol as parent");
const symbolName = getNameForExportedSymbol(exportedSymbol, getEmitScriptTarget(compilerOptions));
const checker = program.getTypeChecker();
const symbol = checker.getMergedSymbol(skipAlias(exportedSymbol, checker));
Expand Down Expand Up @@ -317,6 +321,74 @@ function createImportAdderWorker(sourceFile: SourceFile | FutureSourceFile, prog
}
}

function addImportForModuleSymbol(symbolAlias: Symbol, isValidTypeOnlyUseSite: boolean, referenceImport: ImportOrRequireAliasDeclaration) {
// Adds import for module, import alias will be symbolAlias.name
const checker = program.getTypeChecker();
const moduleSymbol = checker.getAliasedSymbol(symbolAlias);
Debug.assert(moduleSymbol.flags & SymbolFlags.Module, "Expected symbol to be a module");
const moduleSpecifierResolutionHost = createModuleSpecifierResolutionHost(program, host);
const moduleSpecifierResult = moduleSpecifiers.getModuleSpecifiersWithCacheInfo(moduleSymbol, checker, compilerOptions, sourceFile, moduleSpecifierResolutionHost, preferences, /*options*/ undefined, /*forAutoImport*/ true);

const useRequire = shouldUseRequire(sourceFile, program);

// Copy the type-only status from the reference import
let addAsTypeOnly = getAddAsTypeOnly(
isValidTypeOnlyUseSite,
/*isForNewImportDeclaration*/ true,
/*symbol*/ undefined,
symbolAlias.flags,
program.getTypeChecker(),
compilerOptions,
);
addAsTypeOnly = addAsTypeOnly === AddAsTypeOnly.Allowed && isTypeOnlyImportDeclaration(referenceImport) ? AddAsTypeOnly.Required : AddAsTypeOnly.Allowed;

// Copy the kind of import
const importKind = isImportDeclaration(referenceImport) ?
isDefaultImport(referenceImport) ? ImportKind.Default : ImportKind.Namespace :
isImportSpecifier(referenceImport) ? ImportKind.Named :
isImportClause(referenceImport) && !!referenceImport.name ? ImportKind.Default : ImportKind.Namespace;

const exportInfo = [{
symbol: symbolAlias,
moduleSymbol,
moduleFileName: moduleSymbol.declarations?.[0]?.getSourceFile()?.fileName,
exportKind: ExportKind.Module,
targetFlags: symbolAlias.flags,
isFromPackageJson: false,
}];

const existingFix = getImportFixForSymbol(
sourceFile,
exportInfo,
program,
/*position*/ undefined,
!!isValidTypeOnlyUseSite,
useRequire,
host,
preferences,
);

let fix: FixAddNewImport | ImportFixWithModuleSpecifier;
if (existingFix && importKind !== ImportKind.Namespace) {
fix = {
...existingFix,
addAsTypeOnly,
importKind,
};
}
else {
fix = {
kind: ImportFixKind.AddNew,
moduleSpecifierKind: existingFix !== undefined ? existingFix.moduleSpecifierKind : moduleSpecifierResult.kind,
moduleSpecifier: existingFix !== undefined ? existingFix.moduleSpecifier : first(moduleSpecifierResult.moduleSpecifiers),
importKind,
addAsTypeOnly,
useRequire,
};
}
addImport({ fix, symbolName: symbolAlias.name, errorIdentifierText: undefined });
}

function addImportForNonExistentExport(exportName: string, exportingFileName: string, exportKind: ExportKind, exportedMeanings: SymbolFlags, isImportUsageValidAsTypeOnly: boolean) {
const exportingSourceFile = program.getSourceFile(exportingFileName);
const useRequire = shouldUseRequire(sourceFile, program);
Expand Down Expand Up @@ -1452,6 +1524,8 @@ export function getImportKind(importingFile: SourceFile | FutureSourceFile, expo
return getExportEqualsImportKind(importingFile, program.getCompilerOptions(), !!forceImportKeyword);
case ExportKind.UMD:
return getUmdImportKind(importingFile, program, !!forceImportKeyword);
case ExportKind.Module:
return ImportKind.Namespace;
default:
return Debug.assertNever(exportKind);
}
Expand Down
1 change: 1 addition & 0 deletions src/services/exportInfoMap.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ export const enum ExportKind {
Default,
ExportEquals,
UMD,
Module,
}

/** @internal */
Expand Down
4 changes: 4 additions & 0 deletions src/services/refactors/helpers.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,10 @@ export function addTargetFileImports(
if (checker.isUnknownSymbol(targetSymbol)) {
importAdder.addVerbatimImport(Debug.checkDefined(declaration ?? findAncestor(symbol.declarations?.[0], isAnyImportOrRequireStatement)));
}
else if (targetSymbol.parent === undefined) {
Debug.assert(declaration !== undefined, "expected module symbol to have a declaration");
importAdder.addImportForModuleSymbol(symbol, isValidTypeOnlyUseSite, declaration);
}
else {
importAdder.addImportFromExportedSymbol(targetSymbol, isValidTypeOnlyUseSite, declaration);
}
Expand Down
32 changes: 16 additions & 16 deletions tests/baselines/reference/tsserver/fourslashServer/pasteEdits_blankTargetFile.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,28 +22,28 @@ console.log(abc);
console.log("abc");
//// [/home/src/workspaces/project/c.ts]
//// [/home/src/workspaces/project/folder/c.ts]
//// [/home/src/workspaces/project/tsconfig.json]
{ "files": ["c.ts", "a.ts", "b.ts"] }
{ "files": ["folder/c.ts", "a.ts", "b.ts"] }
Info seq [hh:mm:ss:mss] request:
{
"seq": 0,
"type": "request",
"arguments": {
"file": "/home/src/workspaces/project/c.ts"
"file": "/home/src/workspaces/project/folder/c.ts"
},
"command": "open"
}
Info seq [hh:mm:ss:mss] getConfigFileNameForFile:: File: /home/src/workspaces/project/c.ts ProjectRootPath: undefined:: Result: /home/src/workspaces/project/tsconfig.json
Info seq [hh:mm:ss:mss] getConfigFileNameForFile:: File: /home/src/workspaces/project/folder/c.ts ProjectRootPath: undefined:: Result: /home/src/workspaces/project/tsconfig.json
Info seq [hh:mm:ss:mss] Creating ConfiguredProject: /home/src/workspaces/project/tsconfig.json, currentDirectory: /home/src/workspaces/project
Info seq [hh:mm:ss:mss] FileWatcher:: Added:: WatchInfo: /home/src/workspaces/project/tsconfig.json 2000 undefined Project: /home/src/workspaces/project/tsconfig.json WatchType: Config file
Info seq [hh:mm:ss:mss] Config: /home/src/workspaces/project/tsconfig.json : {
"rootNames": [
"/home/src/workspaces/project/c.ts",
"/home/src/workspaces/project/folder/c.ts",
"/home/src/workspaces/project/a.ts",
"/home/src/workspaces/project/b.ts"
],
Expand All @@ -58,7 +58,7 @@ Info seq [hh:mm:ss:mss] event:
"event": "projectLoadingStart",
"body": {
"projectName": "/home/src/workspaces/project/tsconfig.json",
"reason": "Creating possible configured project for /home/src/workspaces/project/c.ts to open"
"reason": "Creating possible configured project for /home/src/workspaces/project/folder/c.ts to open"
}
}
Info seq [hh:mm:ss:mss] FileWatcher:: Added:: WatchInfo: /home/src/workspaces/project/a.ts 500 undefined WatchType: Closed Script info
Expand All @@ -81,7 +81,7 @@ Info seq [hh:mm:ss:mss] Files (6)
/home/src/tslibs/TS/Lib/lib.d.ts Text-1 lib.d.ts-Text
/home/src/tslibs/TS/Lib/lib.decorators.d.ts Text-1 lib.decorators.d.ts-Text
/home/src/tslibs/TS/Lib/lib.decorators.legacy.d.ts Text-1 lib.decorators.legacy.d.ts-Text
/home/src/workspaces/project/c.ts SVC-1-0 ""
/home/src/workspaces/project/folder/c.ts SVC-1-0 ""
/home/src/workspaces/project/a.ts Text-1 "export const abc = 10;"
/home/src/workspaces/project/b.ts Text-1 "import { abc } from \"./a\";\n\nconsole.log(abc);\n\n\nconsole.log(\"abc\");"
Expand All @@ -92,7 +92,7 @@ Info seq [hh:mm:ss:mss] Files (6)
Library referenced via 'decorators' from file '../../tslibs/TS/Lib/lib.d.ts'
../../tslibs/TS/Lib/lib.decorators.legacy.d.ts
Library referenced via 'decorators.legacy' from file '../../tslibs/TS/Lib/lib.d.ts'
c.ts
folder/c.ts
Part of 'files' list in tsconfig.json
a.ts
Part of 'files' list in tsconfig.json
Expand All @@ -116,7 +116,7 @@ Info seq [hh:mm:ss:mss] event:
"type": "event",
"event": "configFileDiag",
"body": {
"triggerFile": "/home/src/workspaces/project/c.ts",
"triggerFile": "/home/src/workspaces/project/folder/c.ts",
"configFile": "/home/src/workspaces/project/tsconfig.json",
"diagnostics": []
}
Expand All @@ -126,7 +126,7 @@ Info seq [hh:mm:ss:mss] Files (6)
Info seq [hh:mm:ss:mss] -----------------------------------------------
Info seq [hh:mm:ss:mss] Open files:
Info seq [hh:mm:ss:mss] FileName: /home/src/workspaces/project/c.ts ProjectRootPath: undefined
Info seq [hh:mm:ss:mss] FileName: /home/src/workspaces/project/folder/c.ts ProjectRootPath: undefined
Info seq [hh:mm:ss:mss] Projects: /home/src/workspaces/project/tsconfig.json
Info seq [hh:mm:ss:mss] response:
{
Expand Down Expand Up @@ -191,7 +191,7 @@ ScriptInfos::
version: Text-1
containingProjects: 1
/home/src/workspaces/project/tsconfig.json
/home/src/workspaces/project/c.ts (Open) *new*
/home/src/workspaces/project/folder/c.ts (Open) *new*
version: SVC-1-0
containingProjects: 1
/home/src/workspaces/project/tsconfig.json *default*
Expand Down Expand Up @@ -242,7 +242,7 @@ Info seq [hh:mm:ss:mss] request:
"seq": 2,
"type": "request",
"arguments": {
"file": "/home/src/workspaces/project/c.ts",
"file": "/home/src/workspaces/project/folder/c.ts",
"pastedText": [
"console.log(abc);"
],
Expand Down Expand Up @@ -283,7 +283,7 @@ Info seq [hh:mm:ss:mss] Files (6)
/home/src/tslibs/TS/Lib/lib.d.ts Text-1 lib.d.ts-Text
/home/src/tslibs/TS/Lib/lib.decorators.d.ts Text-1 lib.decorators.d.ts-Text
/home/src/tslibs/TS/Lib/lib.decorators.legacy.d.ts Text-1 lib.decorators.legacy.d.ts-Text
/home/src/workspaces/project/c.ts SVC-1-1 "console.log(abc);"
/home/src/workspaces/project/folder/c.ts SVC-1-1 "console.log(abc);"
/home/src/workspaces/project/a.ts Text-1 "export const abc = 10;"
/home/src/workspaces/project/b.ts Text-1 "import { abc } from \"./a\";\n\nconsole.log(abc);\n\n\nconsole.log(\"abc\");"

Expand All @@ -303,7 +303,7 @@ Info seq [hh:mm:ss:mss] response:
"body": {
"edits": [
{
"fileName": "/home/src/workspaces/project/c.ts",
"fileName": "/home/src/workspaces/project/folder/c.ts",
"textChanges": [
{
"start": {
Expand All @@ -314,7 +314,7 @@ Info seq [hh:mm:ss:mss] response:
"line": 1,
"offset": 1
},
"newText": "import { abc } from \"./a\";\n\n"
"newText": "import { abc } from \"../a\";\n\n"
},
{
"start": {
Expand Down Expand Up @@ -362,7 +362,7 @@ ScriptInfos::
version: Text-1
containingProjects: 1
/home/src/workspaces/project/tsconfig.json
/home/src/workspaces/project/c.ts (Open) *changed*
/home/src/workspaces/project/folder/c.ts (Open) *changed*
version: SVC-1-2 *changed*
containingProjects: 1
/home/src/workspaces/project/tsconfig.json *default*
Loading

0 comments on commit 3e61718

Please sign in to comment.