From b876e639bc24a5f00dec3f0e1dad168b9ca5d3c2 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Mon, 1 Feb 2021 12:48:07 -0800 Subject: [PATCH 1/2] Fix completions crash on transient exported property named default --- src/harness/fourslashImpl.ts | 2 +- src/services/codefixes/importFixes.ts | 12 +++++------ src/services/utilities.ts | 6 +++--- ...completionsImport_weirdDefaultSynthesis.ts | 20 +++++++++++++++++++ 4 files changed, 29 insertions(+), 11 deletions(-) create mode 100644 tests/cases/fourslash/completionsImport_weirdDefaultSynthesis.ts diff --git a/src/harness/fourslashImpl.ts b/src/harness/fourslashImpl.ts index aa708bfcf4c4f..8fd2f7f2447b8 100644 --- a/src/harness/fourslashImpl.ts +++ b/src/harness/fourslashImpl.ts @@ -2802,7 +2802,7 @@ namespace FourSlash { const matchingName = completions?.filter(e => e.name === options.name); const detailMessage = matchingName?.length ? `\n Found ${matchingName.length} with name '${options.name}' from source(s) ${matchingName.map(e => `'${e.source}'`).join(", ")}.` - : ""; + : ` (In fact, there were no completions with name '${options.name}' at all.)`; return this.raiseError(`No completions were found for the given name, source, and preferences.` + detailMessage); } const codeActions = details.codeActions; diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index e47c7f79b53cf..be3a0e84ede18 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -262,7 +262,7 @@ namespace ts.codefix { } const defaultInfo = getDefaultLikeExportInfo(importingFile, moduleSymbol, checker, compilerOptions); - if (defaultInfo && (defaultInfo.name === symbolName || moduleSymbolToValidIdentifier(moduleSymbol, compilerOptions.target) === symbolName) && skipAlias(defaultInfo.symbol, checker) === exportedSymbol) { + if (defaultInfo && getNameForExportedSymbol(defaultInfo.symbol, compilerOptions.target) === symbolName && skipAlias(defaultInfo.symbol, checker) === exportedSymbol) { result.push({ moduleSymbol, importKind: defaultInfo.kind, exportedSymbolIsTypeOnly: isTypeOnlySymbol(defaultInfo.symbol, checker) }); } @@ -581,7 +581,7 @@ namespace ts.codefix { const compilerOptions = program.getCompilerOptions(); const defaultInfo = getDefaultLikeExportInfo(sourceFile, moduleSymbol, checker, compilerOptions); - if (defaultInfo && (defaultInfo.name === symbolName || moduleSymbolToValidIdentifier(moduleSymbol, compilerOptions.target) === symbolName) && symbolHasMeaning(defaultInfo.symbolForMeaning, currentTokenMeaning)) { + if (defaultInfo && getNameForExportedSymbol(defaultInfo.symbol, compilerOptions.target) === symbolName && symbolHasMeaning(defaultInfo.symbolForMeaning, currentTokenMeaning)) { addSymbol(moduleSymbol, defaultInfo.symbol, defaultInfo.kind, checker); } @@ -636,7 +636,7 @@ namespace ts.codefix { return allowSyntheticDefaults ? ImportKind.Default : ImportKind.CommonJS; } - function getDefaultExportInfoWorker(defaultExport: Symbol, moduleSymbol: Symbol, checker: TypeChecker, compilerOptions: CompilerOptions): { readonly symbolForMeaning: Symbol, readonly name: string } | undefined { + function getDefaultExportInfoWorker(defaultExport: Symbol, _moduleSymbol: Symbol, checker: TypeChecker, compilerOptions: CompilerOptions): { readonly symbolForMeaning: Symbol, readonly name: string } | undefined { const localSymbol = getLocalSymbolForExportDefault(defaultExport); if (localSymbol) return { symbolForMeaning: localSymbol, name: localSymbol.name }; @@ -658,15 +658,13 @@ namespace ts.codefix { defaultExport.escapedName !== InternalSymbolName.ExportEquals) { return { symbolForMeaning: defaultExport, name: defaultExport.getName() }; } - return { symbolForMeaning: defaultExport, name: moduleSymbolToValidIdentifier(moduleSymbol, compilerOptions.target) }; + return { symbolForMeaning: defaultExport, name: getNameForExportedSymbol(defaultExport, compilerOptions.target) }; } function getNameForExportDefault(symbol: Symbol): string | undefined { return symbol.declarations && firstDefined(symbol.declarations, declaration => { if (isExportAssignment(declaration)) { - if (isIdentifier(declaration.expression)) { - return declaration.expression.text; - } + return tryCast(skipOuterExpressions(declaration.expression), isIdentifier)?.text; } else if (isExportSpecifier(declaration)) { Debug.assert(declaration.name.text === InternalSymbolName.Default, "Expected the specifier to be a default export"); diff --git a/src/services/utilities.ts b/src/services/utilities.ts index 8344c3e60ff87..1fb8a23043ef7 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -2880,10 +2880,10 @@ namespace ts { return isArray(valueOrArray) ? first(valueOrArray) : valueOrArray; } - export function getNameForExportedSymbol(symbol: Symbol, scriptTarget: ScriptTarget) { - if (symbol.escapedName === InternalSymbolName.ExportEquals || symbol.escapedName === InternalSymbolName.Default) { + export function getNameForExportedSymbol(symbol: Symbol, scriptTarget: ScriptTarget | undefined) { + if (!(symbol.flags & SymbolFlags.Transient) && (symbol.escapedName === InternalSymbolName.ExportEquals || symbol.escapedName === InternalSymbolName.Default)) { // Name of "export default foo;" is "foo". Name of "export default 0" is the filename converted to camelCase. - return firstDefined(symbol.declarations, d => isExportAssignment(d) && isIdentifier(d.expression) ? d.expression.text : undefined) + return firstDefined(symbol.declarations, d => isExportAssignment(d) ? tryCast(skipOuterExpressions(d.expression), isIdentifier)?.text : undefined) || codefix.moduleSymbolToValidIdentifier(getSymbolParentOrFail(symbol), scriptTarget); } return symbol.name; diff --git a/tests/cases/fourslash/completionsImport_weirdDefaultSynthesis.ts b/tests/cases/fourslash/completionsImport_weirdDefaultSynthesis.ts new file mode 100644 index 0000000000000..b8d3558da3e0f --- /dev/null +++ b/tests/cases/fourslash/completionsImport_weirdDefaultSynthesis.ts @@ -0,0 +1,20 @@ +/// + +// @Filename: /collection.ts +//// class Collection { +//// public static readonly default: typeof Collection = Collection; +//// } +//// export = Collection as typeof Collection & { default: typeof Collection }; + +// @Filename: /index.ts +//// Colle/**/ + +verify.applyCodeActionFromCompletion("", { + name: "Collection", + source: "/collection", + description: `Import 'Collection' from module "./collection"`, + preferences: { + includeCompletionsForModuleExports: true + }, + newFileContent: `import Collection = require("./collection");\n\nColle` +}); From 174ec4759e8101414d57ffc57be81b9f9be690ae Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Mon, 1 Feb 2021 13:28:30 -0800 Subject: [PATCH 2/2] Revert simplification, both conditions were needed --- src/services/codefixes/importFixes.ts | 10 +++++----- src/services/completions.ts | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index be3a0e84ede18..8ecdb840348cc 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -262,7 +262,7 @@ namespace ts.codefix { } const defaultInfo = getDefaultLikeExportInfo(importingFile, moduleSymbol, checker, compilerOptions); - if (defaultInfo && getNameForExportedSymbol(defaultInfo.symbol, compilerOptions.target) === symbolName && skipAlias(defaultInfo.symbol, checker) === exportedSymbol) { + if (defaultInfo && (defaultInfo.name === symbolName || moduleSymbolToValidIdentifier(moduleSymbol, compilerOptions.target) === symbolName) && skipAlias(defaultInfo.symbol, checker) === exportedSymbol) { result.push({ moduleSymbol, importKind: defaultInfo.kind, exportedSymbolIsTypeOnly: isTypeOnlySymbol(defaultInfo.symbol, checker) }); } @@ -581,7 +581,7 @@ namespace ts.codefix { const compilerOptions = program.getCompilerOptions(); const defaultInfo = getDefaultLikeExportInfo(sourceFile, moduleSymbol, checker, compilerOptions); - if (defaultInfo && getNameForExportedSymbol(defaultInfo.symbol, compilerOptions.target) === symbolName && symbolHasMeaning(defaultInfo.symbolForMeaning, currentTokenMeaning)) { + if (defaultInfo && (defaultInfo.name === symbolName || moduleSymbolToValidIdentifier(moduleSymbol, compilerOptions.target) === symbolName) && symbolHasMeaning(defaultInfo.symbolForMeaning, currentTokenMeaning)) { addSymbol(moduleSymbol, defaultInfo.symbol, defaultInfo.kind, checker); } @@ -600,7 +600,7 @@ namespace ts.codefix { const exported = getDefaultLikeExportWorker(importingFile, moduleSymbol, checker, compilerOptions); if (!exported) return undefined; const { symbol, kind } = exported; - const info = getDefaultExportInfoWorker(symbol, moduleSymbol, checker, compilerOptions); + const info = getDefaultExportInfoWorker(symbol, checker, compilerOptions); return info && { symbol, kind, ...info }; } @@ -636,7 +636,7 @@ namespace ts.codefix { return allowSyntheticDefaults ? ImportKind.Default : ImportKind.CommonJS; } - function getDefaultExportInfoWorker(defaultExport: Symbol, _moduleSymbol: Symbol, checker: TypeChecker, compilerOptions: CompilerOptions): { readonly symbolForMeaning: Symbol, readonly name: string } | undefined { + function getDefaultExportInfoWorker(defaultExport: Symbol, checker: TypeChecker, compilerOptions: CompilerOptions): { readonly symbolForMeaning: Symbol, readonly name: string } | undefined { const localSymbol = getLocalSymbolForExportDefault(defaultExport); if (localSymbol) return { symbolForMeaning: localSymbol, name: localSymbol.name }; @@ -650,7 +650,7 @@ namespace ts.codefix { // but we can still offer completions for it. // - `aliased.parent` will be undefined if the module is exporting `globalThis.something`, // or another expression that resolves to a global. - return getDefaultExportInfoWorker(aliased, aliased.parent, checker, compilerOptions); + return getDefaultExportInfoWorker(aliased, checker, compilerOptions); } } diff --git a/src/services/completions.ts b/src/services/completions.ts index e7b1a8e86bdd4..11bebd931a969 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -732,7 +732,7 @@ namespace ts.Completions { exportedSymbol, moduleSymbol, sourceFile, - getNameForExportedSymbol(symbol, compilerOptions.target!), + getNameForExportedSymbol(symbol, compilerOptions.target), host, program, formatContext,