Better completion for property access of computed properties #56220
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -240,6 +240,7 @@ import { | |
| isStringLiteralOrTemplate, | ||
| isStringTextContainingNode, | ||
| isSyntaxList, | ||
| isTransientSymbol, | ||
| isTypeKeyword, | ||
| isTypeKeywordTokenOrIdentifier, | ||
| isTypeLiteralNode, | ||
|
|
@@ -3124,6 +3125,15 @@ function getContextualType(previousToken: Node, position: number, sourceFile: So | |
| } | ||
| } | ||
|
|
||
| /** | ||
| * An accessible symbol is a local in the same block or any enclosing blocks, up to and including the global scope (note imports are global). | ||
| * An accessible symbol chain is a list of symbols [s_1, s_2, ..., s_n] where import s_i is an alias for symbol s_{i+1} exported from another module. | ||
| * Note the chain can have just one element as well. | ||
| * This function follows the given symbol up its parents until it finds a symbol S which has an accessible symbol chain with the first link being | ||
| * the accessible symbol from the enclosing declaration and the last link being S. If such a symbol is found, the first link in the accessible chain is returned. | ||
| * Otherwise, there are two cases. Either its highest parent is a module symbol, in which case that parent's child is an export and it is returned. Or | ||
| * the highest parent is just a global unexported symbol in which case undefined is returned. | ||
| */ | ||
| function getFirstSymbolInChain(symbol: Symbol, enclosingDeclaration: Node, checker: TypeChecker): Symbol | undefined { | ||
| const chain = checker.getAccessibleSymbolChain(symbol, enclosingDeclaration, /*meaning*/ SymbolFlags.All, /*useOnlyExternalAliasing*/ false); | ||
| if (chain) return first(chain); | ||
|
|
@@ -3663,71 +3673,130 @@ function getCompletionData( | |
| } | ||
|
|
||
| function addPropertySymbol(symbol: Symbol, insertAwait: boolean, insertQuestionDot: boolean) { | ||
| // For a computed property `x.y` in an exported namespace `n` that is imported | ||
| // in another file as `m`, we can access `y` as `m.n.x.y`. To form this access chain, first | ||
| // we follow `x` up its symbol parents until we find a symbol that is accessible from the completion | ||
| // location. This gives us a property access chain (`m.n.x`) which we can then combine with the original | ||
| // computed property expression (`x.y`) by substitution of `m.n.x` for `x` in `x.y` to get `m.n.x.y`. | ||
| // If this fails, we will fall back to the literal value of `y`. | ||
|
|
||
| const computedPropertyName = firstDefined(symbol.declarations, decl => tryCast(getNameOfDeclaration(decl), isComputedPropertyName)); | ||
| if (!computedPropertyName) { | ||
| addSymbol(); | ||
| return; | ||
| } | ||
|
|
||
| const computedPropertyNameExpression = computedPropertyName.expression; | ||
| const name = isEntityName(computedPropertyNameExpression) ? computedPropertyNameExpression : | ||
| isPropertyAccessExpression(computedPropertyNameExpression) ? computedPropertyNameExpression.name : undefined; | ||
| const nameSymbol = name && typeChecker.getSymbolAtLocation(name); | ||
| const nameSymbolId = nameSymbol && getSymbolId(nameSymbol); | ||
| if (!nameSymbolId) { // Not a property access or entity name | ||
| addSymbol(); | ||
| return; | ||
| } | ||
|
|
||
| if (addToSeen(seenPropertySymbols, nameSymbolId)) { | ||
| const leftMostName = getLeftMostName(computedPropertyNameExpression); | ||
| const leftMostNameSymbol = leftMostName && typeChecker.getSymbolAtLocation(leftMostName); | ||
| const firstAccessibleSymbol = leftMostNameSymbol && getFirstSymbolInChain(leftMostNameSymbol, contextToken, typeChecker); | ||
| if (!firstAccessibleSymbol) { // Symbol is not accessible from completion location | ||
| addSymbol(); | ||
| return; | ||
| } | ||
|
|
||
| Debug.assert(isOnlyPropertyAccess(computedPropertyNameExpression)); | ||
|
|
||
| const moduleSymbol = firstAccessibleSymbol.parent; | ||
| if ( | ||
| !moduleSymbol || | ||
| !isExternalModuleSymbol(moduleSymbol) || | ||
| typeChecker.tryGetMemberInModuleExportsAndProperties(firstAccessibleSymbol.name, moduleSymbol) !== firstAccessibleSymbol | ||
| ) { | ||
| // If preferences allow insert text, add completion for [<QualifiedSymbolName>] | ||
| const node = preferences.includeCompletionsWithInsertText | ||
| ? createComputedPropertyAccess(nameSymbol, leftMostNameSymbol, computedPropertyNameExpression) | ||
| : undefined; | ||
|
|
||
| if (!node) { | ||
| addSymbol(); | ||
| return; | ||
| } | ||
|
|
||
| const index = symbols.length; | ||
| symbols.push(nameSymbol); | ||
| const printer = createPrinter({ | ||
| removeComments: true, | ||
| module: compilerOptions.module, | ||
| target: compilerOptions.target, | ||
| omitTrailingSemicolon: true, | ||
| }); | ||
| const origin: SymbolOriginInfoComputedPropertyName = { | ||
| kind: getNullableSymbolOriginInfoKind(SymbolOriginInfoKind.SymbolMemberNoExport) | SymbolOriginInfoKind.ComputedPropertyName, | ||
| symbolName: printer.printNode(EmitHint.Unspecified, node, contextToken.getSourceFile()), | ||
| }; | ||
| symbolToOriginInfoMap[index] = origin; | ||
| } | ||
| else { | ||
| const index = symbols.length; | ||
| symbols.push(nameSymbol); | ||
| const fileName = isExternalModuleNameRelative(stripQuotes(moduleSymbol.name)) ? getSourceFileOfModule(moduleSymbol)?.fileName : undefined; | ||
| const { moduleSpecifier } = (importSpecifierResolver ||= codefix.createImportSpecifierResolver(sourceFile, program, host, preferences)).getModuleSpecifierForBestExportInfo( | ||
| [{ | ||
| exportKind: ExportKind.Named, | ||
| moduleFileName: fileName, | ||
| isFromPackageJson: false, | ||
| moduleSymbol, | ||
| symbol: firstAccessibleSymbol, | ||
| targetFlags: skipAlias(firstAccessibleSymbol, typeChecker).flags, | ||
| }], | ||
| position, | ||
| isValidTypeOnlyAliasUseSite(location), | ||
| ) || {}; | ||
|
|
||
| if (moduleSpecifier) { | ||
| const origin: SymbolOriginInfoResolvedExport = { | ||
| kind: getNullableSymbolOriginInfoKind(SymbolOriginInfoKind.SymbolMemberExport), | ||
| moduleSymbol, | ||
| isDefaultExport: false, | ||
| symbolName: firstAccessibleSymbol.name, | ||
| exportName: firstAccessibleSymbol.name, | ||
| fileName, | ||
| moduleSpecifier, | ||
| }; | ||
| symbolToOriginInfoMap[index] = origin; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| function addSymbol() { | ||
| addSymbolOriginInfo(symbol); | ||
| addSymbolSortInfo(symbol); | ||
| symbols.push(symbol); | ||
| } | ||
|
|
||
| /** | ||
| * For a computed property [x.y.z], we get an expression for the left-most node (x) from the completion location. For example, if x is in | ||
| * a namespace n which is imported into the completion location's scope as m, then the expression would be m.n.x. Then we combine this expression | ||
| * with the property access expression at the declaration site (x.y.z) by substituting the left-most expression's symbol (m.n.x) for the left-most | ||
| * identifier in the property access expression (x). This gives us m.n.x.y.z. | ||
| */ | ||
| function createComputedPropertyAccess(nameSymbol: Symbol, leftMostNameSymbol: Symbol, computedPropertyNameExpression: OnlyPropertyAccess) { | ||
| let node: Node | undefined; | ||
| if (!isTransientSymbol(nameSymbol)) { | ||
| node = typeChecker.symbolToEntityName(nameSymbol, SymbolFlags.All, contextToken, NodeBuilderFlags.UseAliasDefinedOutsideCurrentScope); | ||
| } | ||
| else { | ||
| // Object literals assigned as const | ||
|
There was a problem hiding this comment. What's this case for, exactly? I think I don't understand the comment here and how we know this from knowing that There was a problem hiding this comment. This scenario goes into this branch: const x = { a: "foo" } as const;
const y = { [x.a]: 0 };
y.|It's because the symbol That being said, I don't know if There was a problem hiding this comment. I'm not sure if all transient symbols run into this problem with |
||
| const leftMostNodeAccessExpression = typeChecker.symbolToExpression(leftMostNameSymbol, SymbolFlags.All, contextToken, NodeBuilderFlags.UseAliasDefinedOutsideCurrentScope); | ||
| node = leftMostNodeAccessExpression && createPropertyAccess(leftMostNodeAccessExpression, computedPropertyNameExpression); | ||
| function createPropertyAccess(leftSide: Expression, rightSide: OnlyPropertyAccess): Expression { | ||
| return isIdentifier(rightSide) ? leftSide : factory.createPropertyAccessExpression(createPropertyAccess(leftSide, rightSide.expression), rightSide.name); | ||
| } | ||
| } | ||
| return node; | ||
| } | ||
|
|
||
| function addSymbolSortInfo(symbol: Symbol) { | ||
| if (isStaticProperty(symbol)) { | ||
| symbolToSortTextMap[getSymbolId(symbol)] = SortText.LocalDeclarationPriority; | ||
|
|
@@ -3755,6 +3824,11 @@ function getCompletionData( | |
| return isIdentifier(e) ? e : isPropertyAccessExpression(e) ? getLeftMostName(e.expression) : undefined; | ||
| } | ||
|
|
||
| type OnlyPropertyAccess = Identifier | (PropertyAccessExpression & { expression: OnlyPropertyAccess; }); | ||
| function isOnlyPropertyAccess(e: Expression | Identifier): e is OnlyPropertyAccess { | ||
| return isIdentifier(e) ? true : isPropertyAccessExpression(e) ? isOnlyPropertyAccess(e.expression) : false; | ||
| } | ||
|
|
||
| function tryGetGlobalSymbols(): boolean { | ||
| const result: GlobalsSearch = tryGetObjectTypeLiteralInTypeArgumentCompletionSymbols() | ||
| || tryGetObjectLikeCompletionSymbols() | ||
|
|
@@ -5175,7 +5249,7 @@ function getCompletionEntryDisplayNameForSymbol( | |
| case CompletionKind.PropertyAccess: | ||
| case CompletionKind.Global: // For a 'this.' completion it will be in a global context, but may have a non-identifier name. | ||
| // Don't add a completion for a name starting with a space. See https://github.com/Microsoft/TypeScript/pull/20547 | ||
| return name.charCodeAt(0) === CharacterCodes.space ? undefined : { name, needsConvertPropertyAccess: !originIsComputedPropertyName(origin) }; | ||
| case CompletionKind.None: | ||
| case CompletionKind.String: | ||
| return validNameResult; | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if this comment is 100% accurate, but this is my understanding of what this method does. Open to change this if it's wrong.