Find references in importing modules for members of export =ed symbols
#62428
Conversation
There was a problem hiding this comment.
tests/cases/fourslash/findAllRefsEqualExportedObjectMember1.ts
tests/cases/fourslash/findAllRefsEqualExportedObjectMember2.ts
tests/cases/fourslash/findAllRefsEqualExportedClassInstanceMember1.ts
The 3 above can't find the 1st marker at 2nd and 3rd markers. That's the opposite problem to the raised issue and I think it can be treated as such (once this one would get merged I could open a new issue about it). I think it's still worth keeping the tests here though.
| const sourceFile = getSourceFileOfNode(node); | ||
| if (sourceFile.symbol?.exports?.has(InternalSymbolName.ExportEquals)) { | ||
| const moduleSymbol = checker.resolveExternalModuleSymbol(sourceFile.symbol); | ||
| if (moduleSymbol && (moduleSymbol === symbol.parent || some(checker.getPropertiesOfType(checker.getTypeOfSymbol(moduleSymbol)), s => getSymbolTarget(s, checker) === symbol))) { |
There was a problem hiding this comment.
I'm open to suggestions how to improve some(getPropertiesOfType(getTypeOfSymbol(...)), ...) bit. I couldn't find an easier way to do this for the presented object literal (and class instance!) cases.
| escapedText, | ||
| parents, | ||
| allSearchSymbols, | ||
| includes: sym => contains(allSearchSymbols, sym, (s1, s2) => this.checker.getMergedSymbol(s1) === this.checker.getMergedSymbol(s2)), |
There was a problem hiding this comment.
It turns out that the added checker.resolveExternalModuleSymbol call triggered a symbol merge. And that broke this test case (tests/cases/fourslash/renameExportCrash.ts):
///<reference path="fourslash.ts" />
// @allowNonTsExtensions: true
// @Filename: Foo.js
//// let a;
//// module.exports = /**/a;
//// exports["foo"] = a;
verify.baselineRename("", { });- The search state had the original symbol in its
allSearchSymbols(the state was created first) - then the symbol was merged by the added code (through
resolveExternalModuleSymbol -> getCommonJsExportEquals -> cloneSymbol -> recordMergedSymbol) - then
state.checker.getSymbolAtLocation(referenceLocation)called bygetReferencesAtLocationfound the merged symbol throughresolveEntityName -> resolveNameHelper -> result = lookup(location.locals, name, meaning) -> getSymbol -> getMergedSymbol - and at the ned this
includeswas called with that merged symbol
So it seems this was always prone to issues with lazily-merged symbols and comparing merged symbols of both looks like a reasonable solution to me.
fixes #62348