From 72be7156d2e8cdabda4180302f4edb0804e76245 Mon Sep 17 00:00:00 2001 From: Andy Date: Tue, 10 Jul 2018 10:47:43 -0700 Subject: [PATCH] Support completions for unique symbol exported from module (#25537) --- src/services/completions.ts | 32 ++++++++++++------- src/services/findAllReferences.ts | 2 +- src/services/utilities.ts | 3 +- .../completionsUniqueSymbol_import.ts | 22 +++++++++++-- 4 files changed, 42 insertions(+), 17 deletions(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index c049c1e4dae..8939fea2012 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -2,12 +2,20 @@ namespace ts.Completions { export type Log = (message: string) => void; - type SymbolOriginInfo = { type: "this-type" } | { type: "symbol-member" } | SymbolOriginInfoExport; + const enum SymbolOriginInfoKind { ThisType, SymbolMemberNoExport, SymbolMemberExport, Export } + type SymbolOriginInfo = { kind: SymbolOriginInfoKind.ThisType } | { kind: SymbolOriginInfoKind.SymbolMemberNoExport } | SymbolOriginInfoExport; interface SymbolOriginInfoExport { - type: "export"; + kind: SymbolOriginInfoKind.SymbolMemberExport | SymbolOriginInfoKind.Export; moduleSymbol: Symbol; isDefaultExport: boolean; } + function originIsSymbolMember(origin: SymbolOriginInfo): boolean { + return origin.kind === SymbolOriginInfoKind.SymbolMemberExport || origin.kind === SymbolOriginInfoKind.SymbolMemberNoExport; + } + function originIsExport(origin: SymbolOriginInfo): origin is SymbolOriginInfoExport { + return origin.kind === SymbolOriginInfoKind.SymbolMemberExport || origin.kind === SymbolOriginInfoKind.Export; + } + /** * Map from symbol id -> SymbolOriginInfo. * Only populated for symbols that come from other modules. @@ -214,12 +222,12 @@ namespace ts.Completions { let insertText: string | undefined; let replacementSpan: TextSpan | undefined; - if (origin && origin.type === "this-type") { + if (origin && origin.kind === SymbolOriginInfoKind.ThisType) { insertText = needsConvertPropertyAccess ? `this[${quote(name, preferences)}]` : `this.${name}`; } // We should only have needsConvertPropertyAccess if there's a property access to convert. But see #21790. // Somehow there was a global with a non-identifier name. Hopefully someone will complain about getting a "foo bar" global completion and provide a repro. - else if ((origin && origin.type === "symbol-member" || needsConvertPropertyAccess) && propertyAccessToConvert) { + else if ((origin && originIsSymbolMember(origin) || needsConvertPropertyAccess) && propertyAccessToConvert) { insertText = needsConvertPropertyAccess ? `[${quote(name, preferences)}]` : `[${name}]`; const dot = findChildOfKind(propertyAccessToConvert, SyntaxKind.DotToken, sourceFile)!; // If the text after the '.' starts with this name, write over it. Else, add new text. @@ -253,7 +261,7 @@ namespace ts.Completions { kindModifiers: SymbolDisplay.getSymbolModifiers(symbol), sortText: "0", source: getSourceFromOrigin(origin), - hasAction: trueOrUndefined(!!origin && origin.type === "export"), + hasAction: trueOrUndefined(!!origin && originIsExport(origin)), isRecommended: trueOrUndefined(isRecommendedCompletionMatch(symbol, recommendedCompletion, typeChecker)), insertText, replacementSpan, @@ -283,7 +291,7 @@ namespace ts.Completions { } function getSourceFromOrigin(origin: SymbolOriginInfo | undefined): string | undefined { - return origin && origin.type === "export" ? stripQuotes(origin.moduleSymbol.name) : undefined; + return origin && originIsExport(origin) ? stripQuotes(origin.moduleSymbol.name) : undefined; } function getCompletionEntriesFromSymbols( @@ -529,7 +537,7 @@ namespace ts.Completions { } function getSymbolName(symbol: Symbol, origin: SymbolOriginInfo | undefined, target: ScriptTarget): string { - return origin && origin.type === "export" && origin.isDefaultExport && symbol.escapedName === InternalSymbolName.Default + return origin && originIsExport(origin) && origin.isDefaultExport && symbol.escapedName === InternalSymbolName.Default // Name of "export default foo;" is "foo". Name of "export default 0" is the filename converted to camelCase. ? firstDefined(symbol.declarations, d => isExportAssignment(d) && isIdentifier(d.expression) ? d.expression.text : undefined) || codefix.moduleSymbolToValidIdentifier(origin.moduleSymbol, target) @@ -648,7 +656,7 @@ namespace ts.Completions { preferences: UserPreferences, ): CodeActionsAndSourceDisplay { const symbolOriginInfo = symbolToOriginInfoMap[getSymbolId(symbol)]; - if (!symbolOriginInfo || symbolOriginInfo.type !== "export") { + if (!symbolOriginInfo || !originIsExport(symbolOriginInfo)) { return { codeActions: undefined, sourceDisplay: undefined }; } @@ -1124,7 +1132,9 @@ namespace ts.Completions { const firstAccessibleSymbol = nameSymbol && getFirstSymbolInChain(nameSymbol, contextToken, typeChecker); if (firstAccessibleSymbol && !symbolToOriginInfoMap[getSymbolId(firstAccessibleSymbol)]) { symbols.push(firstAccessibleSymbol); - symbolToOriginInfoMap[getSymbolId(firstAccessibleSymbol)] = { type: "symbol-member" }; + const moduleSymbol = firstAccessibleSymbol.parent; + symbolToOriginInfoMap[getSymbolId(firstAccessibleSymbol)] = + !moduleSymbol || !isExternalModuleSymbol(moduleSymbol) ? { kind: SymbolOriginInfoKind.SymbolMemberNoExport } : { kind: SymbolOriginInfoKind.SymbolMemberExport, moduleSymbol, isDefaultExport: false }; } } else { @@ -1222,7 +1232,7 @@ namespace ts.Completions { const thisType = typeChecker.tryGetThisTypeAt(scopeNode); if (thisType) { for (const symbol of getPropertiesForCompletion(thisType, typeChecker)) { - symbolToOriginInfoMap[getSymbolId(symbol)] = { type: "this-type" }; + symbolToOriginInfoMap[getSymbolId(symbol)] = { kind: SymbolOriginInfoKind.ThisType }; symbols.push(symbol); } } @@ -1374,7 +1384,7 @@ namespace ts.Completions { symbol = getLocalSymbolForExportDefault(symbol) || symbol; } - const origin: SymbolOriginInfo = { type: "export", moduleSymbol, isDefaultExport }; + const origin: SymbolOriginInfoExport = { kind: SymbolOriginInfoKind.Export, moduleSymbol, isDefaultExport }; if (detailsEntryId || stringContainsCharactersInOrder(getSymbolName(symbol, origin, target).toLowerCase(), tokenTextLowerCase)) { symbols.push(symbol); symbolToOriginInfoMap[getSymbolId(symbol)] = origin; diff --git a/src/services/findAllReferences.ts b/src/services/findAllReferences.ts index 1830be27053..35da81535fe 100644 --- a/src/services/findAllReferences.ts +++ b/src/services/findAllReferences.ts @@ -704,7 +704,7 @@ namespace ts.FindAllReferences.Core { - But if the parent has `export as namespace`, the symbol is globally visible through that namespace. */ const exposedByParent = parent && !(symbol.flags & SymbolFlags.TypeParameter); - if (exposedByParent && !((parent!.flags & SymbolFlags.Module) && isExternalModuleSymbol(parent!) && !parent!.globalExports)) { + if (exposedByParent && !(isExternalModuleSymbol(parent!) && !parent!.globalExports)) { return undefined; } diff --git a/src/services/utilities.ts b/src/services/utilities.ts index 11c745a3e25..42f6a3da60b 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -1184,8 +1184,7 @@ namespace ts { /** True if the symbol is for an external module, as opposed to a namespace. */ export function isExternalModuleSymbol(moduleSymbol: Symbol): boolean { - Debug.assert(!!(moduleSymbol.flags & SymbolFlags.Module)); - return moduleSymbol.name.charCodeAt(0) === CharacterCodes.doubleQuote; + return !!(moduleSymbol.flags & SymbolFlags.Module) && moduleSymbol.name.charCodeAt(0) === CharacterCodes.doubleQuote; } /** Returns `true` the first time it encounters a node and `false` afterwards. */ diff --git a/tests/cases/fourslash/completionsUniqueSymbol_import.ts b/tests/cases/fourslash/completionsUniqueSymbol_import.ts index f15bc59abff..df3034e56fe 100644 --- a/tests/cases/fourslash/completionsUniqueSymbol_import.ts +++ b/tests/cases/fourslash/completionsUniqueSymbol_import.ts @@ -1,12 +1,17 @@ /// -// @Filename: /a.ts +// @noLib: true + +// @Filename: /globals.d.ts ////declare const Symbol: () => symbol; + +// @Filename: /a.ts ////const privateSym = Symbol(); ////export const publicSym = Symbol(); ////export interface I { //// [privateSym]: number; //// [publicSym]: number; +//// [defaultPublicSym]: number; //// n: number; ////} ////export const i: I; @@ -17,10 +22,21 @@ verify.completions({ marker: "", - // TODO: GH#25095 Should include `publicSym` - exact: "n", + exact: [ + "n", + { name: "publicSym", insertText: "[publicSym]", replacementSpan: test.ranges()[0], hasAction: true }, + ], preferences: { includeInsertTextCompletions: true, includeCompletionsForModuleExports: true, }, }); + +verify.applyCodeActionFromCompletion("", { + name: "publicSym", + source: "/a", + description: `Add 'publicSym' to existing import declaration from "./a"`, + newFileContent: +`import { i, publicSym } from "./a"; +i.;` +});