Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve @implements error #58904

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

sandersn
Copy link
Member

@sandersn sandersn commented Jun 17, 2024

Make @implements/@extends parse its expression with parseLeftHandSideExpressionOrHigher instead of parsePropertyAccessEntityNameExpression. This turns many more parse errors into grammar errors, just like in TS. I kept parseExpressionWithTypeArgumentsForJSDoc as a separate function in order to keep parsing asterisks -- and @implements still only supports a single interface, unlike TS which supports a list of them.

This changes the public API, making the expression property a supertype of what it used to be. The one change it required in our codebase is in checker.ts. I think it's fine because the code to handle jsdoc @implements should really be the same as for TS, which already uses ExpressionWithTypeArguments.

Fixes #58542

Make @implements/@extends parse its expression with
`parseLeftHandSideExpressionOrHigher` instead of
`parsePropertyAccessEntityNameExpression`. This turns many more parse
errors into grammar errors, just like in TS. I kept
`parseExpressionWithTypeArgumentsForJSDoc` as a separate function in
order to keep parsing asterisks -- and `@implements` still only supports
a single interface, unlike TS which supports a list of them.

Fixes microsoft#58542
@typescript-bot typescript-bot added Author: Team For Milestone Bug PRs that fix a bug with a specific milestone labels Jun 17, 2024
@typescript-bot
Copy link
Collaborator

Looks like you're introducing a change to the public API surface area. If this includes breaking changes, please document them on our wiki's API Breaking Changes page.

Also, please make sure @DanielRosenwasser and @RyanCavanaugh are aware of the changes, just as a heads up.

readonly class: ExpressionWithTypeArguments & { readonly expression: Identifier | PropertyAccessEntityNameExpression; };
readonly class: ExpressionWithTypeArguments;
Copy link
Member

Choose a reason for hiding this comment

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

Are these types wrong and that's why they're changing?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Milestone Bug PRs that fix a bug with a specific milestone
Projects
Status: Waiting on reviewers
Development

Successfully merging this pull request may close these issues.

Bad error when using import() type within JSDoc tag @implements
3 participants