Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Improve @template lookup and resilience#42851

Merged
sandersn merged 3 commits into
mastermicrosoft/TypeScript:masterfrom
improve-template-tag-lookupmicrosoft/TypeScript:improve-template-tag-lookupCopy head branch name to clipboard
Feb 18, 2021
Merged

Improve @template lookup and resilience#42851
sandersn merged 3 commits into
mastermicrosoft/TypeScript:masterfrom
improve-template-tag-lookupmicrosoft/TypeScript:improve-template-tag-lookupCopy head branch name to clipboard

Conversation

@sandersn

@sandersn sandersn commented Feb 18, 2021

Copy link
Copy Markdown
Member
  1. @template parsing may produce a template tag with a type parameter whose name is the missing identifier. These tags should be skipped in the checker because they receive an error in the parser.
  2. The fix in Look for outer type parameters on VariableStatements #37819 was incorrect; there's no such thing as a type parameter declared on a variable declaration. Instead, there needs to be a type
    parameter declared on a jsdoc comment, because that's the scope for tags like @return and @typedef.

There are 3 tests because either fix (1) and (2) fix the first test's failure, but both are required to fix the last two tests' failures.

Fixes #42812

Edit: The fix in (1) isn't as good as the one in #42017, so I'm going to take that and leave the test cases in place.

1. @template parsing may produce a template tag with a type parameter
whose name is the missing identifier. These tags should be skipped
in the checker because they receive an error in the parser.
2. The fix in #37819 was incorrect; there's no such thing as a type
parameter declared on a variable declaration. Instead, there needs to be a type
parameter declared on a jsdoc comment, because that's the scope for tags
like `@return` and `@typedef`.

There are 3 tests because either fix (1) and (2) fix the first test's
failure, but both are required to fix the last two tests' failures.
Comment thread src/compiler/utilities.ts Outdated

export function getJSDocTypeParameterDeclarations(node: DeclarationWithTypeParameters): readonly TypeParameterDeclaration[] {
return flatMap(getJSDocTags(node), tag => isNonTypeAliasTemplate(tag) ? tag.typeParameters : undefined);
return flatMap(getJSDocTags(node), tag => isNonTypeAliasTemplate(tag) ? tag.typeParameters.filter(p => !containsParseError(p)) : undefined);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I heard u like Haskell

Suggested change
return flatMap(getJSDocTags(node), tag => isNonTypeAliasTemplate(tag) ? tag.typeParameters.filter(p => !containsParseError(p)) : undefined);
return flatMap(getJSDocTags(node), tag => isNonTypeAliasTemplate(tag) ? tag.typeParameters.filter(not(containsParseError)) : undefined);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Um....oops. This isn't needed since I found that the Pursuit fellows made a better fix in December.

@sandersn sandersn merged commit 3d7ec8a into master Feb 18, 2021
@sandersn sandersn deleted the improve-template-tag-lookup branch February 18, 2021 01:21
@microsoft microsoft locked as resolved and limited conversation to collaborators Oct 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Author: Team For Milestone Bug PRs that fix a bug with a specific milestone

Projects

None yet

Development

Successfully merging this pull request may close these issues.

JSDoc @return, @template tags crash tsc even with --checkJs=false

3 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.