Skip to content

findAllReferences: Share code between populateSearchSymbolSet and getRelatedSymbol#23028

Merged
2 commits merged into
masterfrom
forEachRelatedSymbol
Apr 5, 2018
Merged

findAllReferences: Share code between populateSearchSymbolSet and getRelatedSymbol#23028
2 commits merged into
masterfrom
forEachRelatedSymbol

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Mar 30, 2018

These functions iterate over nearly the same set of symbols; the difference is that populateSearchSymbolSet adds them to an array, while getRelatedSymbol searches for one contained in that array. There are also a few differences related to handling of parameter properties and root symbols, but those can be solved by giving those their own callbacks. There's also the includeShorthandDestructuring parameter which I'm not sure should be different in each case, but it would change a bunch of baselines to make it consistent, so that's something for another day.

@ghost ghost requested review from armanio123 and sheetalkamat March 30, 2018 17:39
@ghost ghost force-pushed the forEachRelatedSymbol branch from 34c5fac to a7cfd8e Compare March 30, 2018 20:34
@ghost ghost force-pushed the forEachRelatedSymbol branch from a7cfd8e to ad533fe Compare March 30, 2018 20:35
@ghost
Copy link
Copy Markdown
Author

ghost commented Apr 5, 2018

@sheetalkamat @armanio123 Could someone review?

function addRootSymbols(sym: Symbol): void {
// If this is a union property, add all the symbols from all its source symbols in all unioned types.
// If the symbol is an instantiation from a another symbol (e.g. widened symbol) , add the root the list
for (const rootSymbol of checker.getRootSymbols(sym)) {
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 please move this comment may be at fromRoot(symbol) so we know we are iterating over symbols for unioned types? Thanks

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Moved comment to fromRoot and elaborated (since behavior is different in the two cases)

@ghost ghost force-pushed the forEachRelatedSymbol branch from 89463cc to ed3f110 Compare April 5, 2018 21:41
@ghost ghost force-pushed the forEachRelatedSymbol branch from ed3f110 to 48c846d Compare April 5, 2018 21:42
@ghost ghost merged commit aa8631d into master Apr 5, 2018
@ghost ghost deleted the forEachRelatedSymbol branch April 5, 2018 22:00
@microsoft microsoft locked and limited conversation to collaborators Jul 25, 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.

1 participant