Add typedef declaration space, unify typedef name gathering#18172
Conversation
|
|
||
| export function assertNever(_member: never, message?: string, stackCrawlMark?: Function): never { | ||
| fail(message, stackCrawlMark || assertNever); | ||
| throw "Unreachable"; |
There was a problem hiding this comment.
You could make fail return never and write return fail(message, stackCrawlMark || assertNever)
There was a problem hiding this comment.
It's nice to check that the type of the member is never though. Or did you just mean to change the implementation of assertNever instead of to replace it?
There was a problem hiding this comment.
I think you just mean that throw "Unreachable"; is not required if fail just returns never - which seems fine to me; so I've done that.
|
|
||
| export interface JSDoc extends Node { | ||
| kind: SyntaxKind.JSDocComment; | ||
| parent?: HasJSDoc; |
There was a problem hiding this comment.
All members of HasJSDoc are already nodes. The interface HasJSDocNodes would require that. Is that confusing? Should I rename the second to something like JSDocContainer?
There was a problem hiding this comment.
JSDocContainer seems like a better name
…en errorUnusedLocal
…row to satisfy checker
weswigham
force-pushed
the
fix-typedef-crash
branch
from
01acb14 to
ca8d298
Compare
4 participants
And strengthen
errorUnusedLocalto no longer error when a declaration name cannot be found.Fixes #18097
There were actually two separate but related crashes in that issue - one because
getDeclarationSpacesdid not contain a clause for jsdoc typedefs, and one becausegetNameOfDeclarationdid not know how to find the name for a jsdoc typedef when its own.namefield was not provided - I've unified the codepath used by the binder to get the name with the path used ingetNameOfDeclaration, so minimally if the symbol is bound, then it should be nameable now. In doing so I found that we had some test cases which were unnameable but bound or nameable but unbound, and added clauses to the newgetNameOfJSDocTypedefbased on where I saw either naming or binding occur prior to this change, so both should always be possible in both cases now. To do this, I wanted to ensure I was being exhaustive with the nodes checked; so I've also separated the jsdoc node properties from theNodeinterface and created aHasJSDocunion which represents all nodes which the parser applies JSDoc to. In doing so, I actually found a few types where we could have been applying jsdoc, but were not - which we now are.