Don't bind JSDoc type parameter in a TS file #16413
Conversation
| @@ -5274,6 +5280,10 @@ namespace ts { | |||
|
|
|||
| /* @internal */ | |||
| export function isDeclaration(node: Node): node is NamedDeclaration { | |||
| if (node.kind === SyntaxKind.TypeParameter) { | |||
There was a problem hiding this comment.
In a TypeScript file, a TypeParameter in JSDoc is just a comment, albeit one that is parsed.
In a JavaScript file, it's actually a declaration.
|
@rbuckton can you please review this change. |
src/compiler/checker.ts
Outdated
| return parameter && parameter.symbol; | ||
| } | ||
|
|
||
| if (entityName.parent!.kind === SyntaxKind.TypeParameter && entityName.parent!.parent!.kind === SyntaxKind.JSDocTemplateTag) { |
There was a problem hiding this comment.
We don't compile with strictNullChecks (yet), and line 22553 is the only place in the whole file where we have a non-null expression. I'd rather we not use it here and remove the one above, as when the time comes and we do enable strictNullChecks, we could unintentionally miss a case because we added these too early.
| const fullStart = state.options.findInComments || container.jsDoc !== undefined || forEach(search.symbol.declarations, d => d.kind === ts.SyntaxKind.JSDocTypedefTag); | ||
| for (const position of getPossibleSymbolReferencePositions(sourceFile, search.text, container, fullStart)) { | ||
| // Need to search in the full start of the node in case there is a reference inside JSDoc. | ||
| for (const position of getPossibleSymbolReferencePositions(sourceFile, search.text, container, /*fullStart*/ true)) { |
There was a problem hiding this comment.
Wouldn't this have been true if container.jsDoc !== undefined, or are we opening this up to all comments of the container because we need to find it on a node where we don't currently parse JSDoc comments in the parser?
There was a problem hiding this comment.
That was a bad test in the first place, because if you container is the SourceFile, container.jsDoc will always be undefined, but calling .getStart() on the SourceFile could still skip past jsdoc comments of its children.
We should probably never even bother with calling .getStart() in getPossibleSymbolReferencePositions anyway, because scanning to the start of a node is probably more expensive than just doing text search on the full range.
ghost
mentioned this pull request
ghost
merged commit 
Fixes #16358
Now that we no longer bind
@templatetags, had to add some code togetSymbolAtLocationto get find-all-references working again.Also discovered #16411.