From 27dd761bf0036331f4ec0f8bbd3843ed02db1dfc Mon Sep 17 00:00:00 2001 From: "Pranav Senthilnathan (from Dev Box)" Date: Wed, 1 Nov 2023 14:43:37 -0700 Subject: [PATCH] pr feedback --- src/services/completions.ts | 88 +++++++++++++++++++++---------------- 1 file changed, 51 insertions(+), 37 deletions(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index 3084a4f6dc7..546a73edd86 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -3125,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); @@ -3666,14 +3675,14 @@ 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 it symbol parents until we find a symbol that is accessible from the completion + // 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) { - addLiteralSymbol(); + addSymbol(); return; } @@ -3683,7 +3692,7 @@ function getCompletionData( const nameSymbol = name && typeChecker.getSymbolAtLocation(name); const nameSymbolId = nameSymbol && getSymbolId(nameSymbol); if (!nameSymbolId) { // Not a property access or entity name - addLiteralSymbol(); + addSymbol(); return; } @@ -3692,12 +3701,12 @@ function getCompletionData( const leftMostNameSymbol = leftMostName && typeChecker.getSymbolAtLocation(leftMostName); const firstAccessibleSymbol = leftMostNameSymbol && getFirstSymbolInChain(leftMostNameSymbol, contextToken, typeChecker); if (!firstAccessibleSymbol) { // Symbol is not accessible from completion location - addLiteralSymbol(); + addSymbol(); return; } - const index = symbols.length; - symbols.push(nameSymbol); + Debug.assert(isOnlyPropertyAccess(computedPropertyNameExpression)); + const moduleSymbol = firstAccessibleSymbol.parent; if ( !moduleSymbol || @@ -3710,24 +3719,27 @@ function getCompletionData( : undefined; if (!node) { - // Switch to literal symbol if user doesn't want insert text - symbols[index] = symbol; - } - else { - 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; + 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( [{ @@ -3757,32 +3769,29 @@ function getCompletionData( } } - function addLiteralSymbol() { + function addSymbol() { addSymbolOriginInfo(symbol); addSymbolSortInfo(symbol); symbols.push(symbol); } - /** Combines a property access expression (from the completion location to the left-most name in the computed property expression) - * and the computed property expression itself to return an expression to access the computed property from the completion location. - * Assumes that the left-most symbol is accessible so there should always be a way to access the computed property symbolically. */ - function createComputedPropertyAccess(nameSymbol: Symbol, leftMostNameSymbol: Symbol, computedPropertyNameExpression: Expression) { + /** + * 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, /*meaning*/ undefined!, contextToken, NodeBuilderFlags.UseAliasDefinedOutsideCurrentScope); + node = typeChecker.symbolToEntityName(nameSymbol, SymbolFlags.All, contextToken, NodeBuilderFlags.UseAliasDefinedOutsideCurrentScope); } else { // Object literals assigned as const - const leftMostNodeAccessExpression = typeChecker.symbolToNode(leftMostNameSymbol, /*meaning*/ undefined!, contextToken, NodeBuilderFlags.UseAliasDefinedOutsideCurrentScope); - type OnlyPropertyAccess = Identifier | (PropertyAccessExpression & { expression: OnlyPropertyAccess; }); - node = createPropertyAccess(computedPropertyNameExpression as OnlyPropertyAccess); - function createPropertyAccess(n: OnlyPropertyAccess): Expression { - if (isIdentifier(n)) { - Debug.assertNode(leftMostNodeAccessExpression, isExpression); - return leftMostNodeAccessExpression; - } - - return factory.createPropertyAccessExpression(createPropertyAccess(n.expression), n.name); + 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; @@ -3815,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()