Closed antongolub closed 1 year ago
Do you have a test case for reproduction? Or could you give a small example? I'm not sure which test it failed on in the zx
repo.
https://github.com/google/zx/commit/2dbf8e8e7612e702557d401dbb8bef8e30d4c5c5 https://github.com/google/zx/tree/dev/test-d
Seems it fails on any attempt to load a test. Any test. So it's hard to say what exaclty is broken.
Weird. And does this fixing maybeAlias
work? The CLI also reads a flags
property:
Though maybeAlias.flags
being undefined is more likely.
I've checked this point too. Then found this:
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const maybeAlias = checker.getSymbolAtLocation(expression);
console.log('!!!', maybeAlias)
members: Map(4) {
'__new' => [SymbolObject],
'__call' => [SymbolObject],
'isArray' => [Circular *1],
'prototype' => [SymbolObject]
},
parent: undefined,
mergeId: 7
},
id: 48790
}
!!! undefined
Cannot read properties of undefined (reading 'flags')
Right then, guess it is that. @sindresorhus I assumed here that since we have a valid node
, checker.getSymbolAtLocation(expression)
would always be defined:
I'm not sure how to reproduce when the Symbol
is undefined, but seems that right now we should not assume it is. I think a simple if
is clearer than null chaining:
const maybeAlias = checker.getSymbolAtLocation(expression)
if (maybeAlias) {
// ...
}
My first thought was to add fail-fast. But it's not clear to me: should it be just return
or forEachChild(node, walkNodes)
is needed anyway.
if (!maybeAlias) {
// ...
}
I suppose a return
would be fine since that means the current node
doesn't have a Symbol
, but I'm not keen on tempting fate again lol. Simplest to just move the logic into the if
and change the name of maybeAlias
to be more descriptive:
const maybeSymbol = checker.getSymbolAtLocation(expression);
if (maybeSymbol) {
const symbol = maybeSymbol.flags & SymbolFlags.Alias ?
checker.getAliasedSymbol(maybeSymbol) :
maybeSymbol;
// ...
}
Looks good to me. Leaving a suggestion here for future refactoring:
function handleNode(node: CallExpression) {
// ...
const maybeSymbol = checker.getSymbolAtLocation(expression);
if (!maybeSymbol) {
return;
}
// ...
}
function walkNodes(node: Node) {
if (isCallExpression(node)) {
handleNode(node);
}
forEachChild(node, walkNodes);
}
closes #160