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.

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

Conversation

ghost
Copy link

@ghost ghost commented Aug 30, 2017

Redo of #17787 which I accidentally merged.

@ghost ghost requested review from sandersn and sheetalkamat August 30, 2017 15:29
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

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.

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?

Copy link
Member

@sandersn sandersn left a comment

Choose a reason for hiding this comment

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

It's much easier to read with the new commit.

@ghost ghost merged commit 601c113 into master Aug 30, 2017
@ghost ghost deleted the index2 branch August 30, 2017 21:19
@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants