Skip to content

Handle indexed access types in getSymbolAtLocation and findAllReferences #18149

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
2 commits merged into from
Aug 30, 2017
Merged

Handle indexed access types in getSymbolAtLocation and findAllReferences #18149

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
524c223
Handle indexed access types in getSymbolAtLocation and findAllReferences
Aug 30, 2017
f4ab101
Update baselines, simplify `const objectType`
Aug 30, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 14 additions & 7 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22929,7 +22929,7 @@ namespace ts {
return undefined;
}

function getSymbolAtLocation(node: Node) {
function getSymbolAtLocation(node: Node): Symbol | undefined {
if (node.kind === SyntaxKind.SourceFile) {
return isExternalModule(<SourceFile>node) ? getMergedSymbol(node.symbol) : undefined;
}
Expand Down Expand Up @@ -23007,13 +23007,20 @@ namespace ts {

case SyntaxKind.NumericLiteral:
// index access
if (node.parent.kind === SyntaxKind.ElementAccessExpression && (<ElementAccessExpression>node.parent).argumentExpression === node) {
const objectType = getTypeOfExpression((<ElementAccessExpression>node.parent).expression);
return getPropertyOfType(objectType, (<NumericLiteral>node).text as __String);
}
break;
const objectType = (() => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the IIFE seems like overkill for what could be let objectType; switch ... with some assignments to objectType inside. Or does it introduce some names that conflict with the outside scope?

switch (node.parent.kind) {
case SyntaxKind.ElementAccessExpression:
const { argumentExpression, expression } = node.parent as ElementAccessExpression;
return argumentExpression === node ? getTypeOfExpression(expression) : undefined;
case SyntaxKind.LiteralType:
return isIndexedAccessTypeNode(node.parent.parent) ? getTypeFromTypeNode(node.parent.parent.objectType) : undefined;
}
})();
return objectType && getPropertyOfType(objectType, escapeLeadingUnderscores((node as StringLiteral | NumericLiteral).text));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you construct a test case where the lack of escapeLeadingUnderscores makes a string literal element access fail? It would be something like x["__foo"], right?

Copy link
Author

@ghost ghost Aug 30, 2017 •
edited by ghost
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Turns out I needed to update a few baselines related to this anyway.


default:
return undefined;
}
return undefined;
}

function getShorthandAssignmentValueSymbol(location: Node): Symbol {
Expand Down
4 changes: 2 additions & 2 deletions src/services/findAllReferences.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -748,11 +748,11 @@ namespace ts.FindAllReferences.Core {
return (node as Identifier).text.length === searchSymbolName.length;

case SyntaxKind.StringLiteral:
return (isLiteralNameOfPropertyDeclarationOrIndexAccess(node) || isNameOfExternalModuleImportOrDeclaration(node)) &&
return (isLiteralNameOfPropertyDeclarationOrIndexAccess(node as StringLiteral) || isNameOfExternalModuleImportOrDeclaration(node)) &&
(node as StringLiteral).text.length === searchSymbolName.length;

case SyntaxKind.NumericLiteral:
return isLiteralNameOfPropertyDeclarationOrIndexAccess(node) && (node as NumericLiteral).text.length === searchSymbolName.length;
return isLiteralNameOfPropertyDeclarationOrIndexAccess(node as NumericLiteral) && (node as NumericLiteral).text.length === searchSymbolName.length;

default:
return false;
Expand Down
14 changes: 10 additions & 4 deletions src/services/rename.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,15 @@ namespace ts.Rename {
}

function nodeIsEligibleForRename(node: Node): boolean {
return node.kind === ts.SyntaxKind.Identifier ||
node.kind === SyntaxKind.StringLiteral ||
isLiteralNameOfPropertyDeclarationOrIndexAccess(node) ||
isThis(node);
switch (node.kind) {
case SyntaxKind.Identifier:
case SyntaxKind.StringLiteral:
case SyntaxKind.ThisKeyword:
return true;
case SyntaxKind.NumericLiteral:
return isLiteralNameOfPropertyDeclarationOrIndexAccess(node as NumericLiteral);
default:
return false;
}
}
}
38 changes: 18 additions & 20 deletions src/services/utilities.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,27 +244,25 @@ namespace ts {
isFunctionLike(node.parent) && (<FunctionLikeDeclaration>node.parent).name === node;
}

export function isLiteralNameOfPropertyDeclarationOrIndexAccess(node: Node): boolean {
if (node.kind === SyntaxKind.StringLiteral || node.kind === SyntaxKind.NumericLiteral) {
switch (node.parent.kind) {
case SyntaxKind.PropertyDeclaration:
case SyntaxKind.PropertySignature:
case SyntaxKind.PropertyAssignment:
case SyntaxKind.EnumMember:
case SyntaxKind.MethodDeclaration:
case SyntaxKind.MethodSignature:
case SyntaxKind.GetAccessor:
case SyntaxKind.SetAccessor:
case SyntaxKind.ModuleDeclaration:
return getNameOfDeclaration(<Declaration>node.parent) === node;
case SyntaxKind.ElementAccessExpression:
return (<ElementAccessExpression>node.parent).argumentExpression === node;
case SyntaxKind.ComputedPropertyName:
return true;
}
export function isLiteralNameOfPropertyDeclarationOrIndexAccess(node: StringLiteral | NumericLiteral): boolean {
switch (node.parent.kind) {
case SyntaxKind.PropertyDeclaration:
case SyntaxKind.PropertySignature:
case SyntaxKind.PropertyAssignment:
case SyntaxKind.EnumMember:
case SyntaxKind.MethodDeclaration:
case SyntaxKind.MethodSignature:
case SyntaxKind.GetAccessor:
case SyntaxKind.SetAccessor:
case SyntaxKind.ModuleDeclaration:
return getNameOfDeclaration(<Declaration>node.parent) === node;
case SyntaxKind.ElementAccessExpression:
return (<ElementAccessExpression>node.parent).argumentExpression === node;
case SyntaxKind.ComputedPropertyName:
return true;
case SyntaxKind.LiteralType:
return node.parent.parent.kind === SyntaxKind.IndexedAccessType;
}

return false;
}

export function isExpressionOfExternalModuleImportEqualsDeclaration(node: Node) {
Expand Down
14 changes: 14 additions & 0 deletions tests/cases/fourslash/findAllRefsIndexedAccessTypes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/// <reference path='fourslash.ts' />

////interface I {
//// [|{| "isDefinition": true, "isWriteAccess": true |}0|]: number;
//// [|{| "isDefinition": true, "isWriteAccess": true |}s|]: string;
////}
////interface J {
//// a: I[[|0|]],
//// b: I["[|s|]"],
////}

const [n0, s0, n1, s1] = test.ranges();
verify.singleReferenceGroup("(property) I[0]: number", [n0, n1]);
verify.singleReferenceGroup("(property) I.s: string", [s0, s1]);