Skip to content

Handle indexed access types in getSymbolAtLocation and findAllReferences#18149

Merged
2 commits merged into
masterfrom
index2
Aug 30, 2017
Merged

Handle indexed access types in getSymbolAtLocation and findAllReferences#18149
2 commits merged into
masterfrom
index2

Conversation

@ghost
Copy link
Copy Markdown

@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
Comment thread src/compiler/checker.ts
return isIndexedAccessTypeNode(node.parent.parent) ? getTypeFromTypeNode(node.parent.parent.objectType) : undefined;
}
})();
return objectType && getPropertyOfType(objectType, escapeLeadingUnderscores((node as StringLiteral | NumericLiteral).text));
Copy link
Copy Markdown
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
Copy Markdown
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.

Comment thread src/compiler/checker.ts Outdated
return getPropertyOfType(objectType, (<NumericLiteral>node).text as __String);
}
break;
const objectType = (() => {
Copy link
Copy Markdown
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
Copy Markdown
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