Fix find all references for salsa #6632
Conversation
|
Where are the tests for this change? |
|
looks good. please add some tests. |
… fixFindAllRefForSalsa
|
@DanielRosenwasser @mhegazy @RyanCavanaugh any further comments? |
… fixFindAllRefForSalsa
| return getSymbolOfNode(entityName.parent); | ||
| case SpecialPropertyAssignmentKind.ThisProperty: | ||
| case SpecialPropertyAssignmentKind.ModuleExports: | ||
| return getSymbolOfNode(entityName.parent.parent); |
There was a problem hiding this comment.
I assume in most cases it won't be a "specialPropertyAssignmentKind" and you will just fall through here? Can you make this clear in the code by explicitly adding a default: with just a comment to this effect, or wrapping the switch in an if(specialPropertyAssignmentKind...)?
|
Added a couple of comments. Looks 👍 to me after that. |
| ////var t = mod./**/[|area|](10); | ||
|
|
||
| goTo.marker(); | ||
| verify.renameLocations(false, false); |
There was a problem hiding this comment.
Add a leading comment for each false
|
There are no tests for find all refs/rename locs at declaration sites - is this because it doesn't work right now? If it does work, can you add tests? Either way, can you rename the tests and add a numbered suffix? |
Fix find all references for salsa
Issue:
currently "find all reference" would return no results if called on exported properties in JavaScript file. For example:
After the fix:
