Conversation
rbuckton
left a comment
There was a problem hiding this comment.
I don't think we want to make exportSymbol public as we generally pass it through getMergedSymbol before using it ourselves (see getExportSymbolOfValueSymbolIfExported). I'd prefer we added a checker.getExportSymbolOfSymbol that is just a pointer to getExportSymbolOfValueSymbolIfExported.
src/compiler/checker.ts
Outdated
| node = getParseTreeNode(node, isExportSpecifier); | ||
| return node ? getExportSpecifierLocalTargetSymbol(node) : undefined; | ||
| }, | ||
| getExportSymbolOfSymbol: getExportSymbolOfValueSymbolIfExported, |
There was a problem hiding this comment.
My apologies. getExportSymbolOfValueSymbolIfExported will only get the export symbol if the symbol is a Value symbol, not a Type symbol.
Perhaps this might be more reliable:
function getExportSymbolOfSymbol(symbol: Symbol) {
return getMergedSymbol(symbol.exportSymbol || symbol);
}Otherwise, the example in your comment in types.ts isn't strictly valid.
rbuckton
left a comment
There was a problem hiding this comment.
It would be nice to have unit tests for this, though we don't currently have much in the way of unit tests for checker's public API surface.
#16742 changed the behavior of
getSymbolsInScopeto return local symbols and not exported symbols.This means that in:
The
getSymbolAtLocationatTinN.Twill return the exported symbol forT.However,
getSymbolsInScopeat the same location returns the local symbol forT.Tslint used to compare
getSymbolAtLocationwithgetSymbolsInScopeto determine when a namespace qualifier was unnecessary; this was broken whengetSymbolAtLocationstarted returning local symbols. This was "fixed" by palantir/tslint#3052 which checks every declaration for equality, but a better solution would be to test for.exportSymbolof the local symbol.We could instead add a
checker.getExportedSymbolOfLocalSymbolmethod if you think that would make for a better API.