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

feat(fe): add warning for 'let({x} = y);' #1211

Closed
wants to merge 3 commits into from

Conversation

msharipov
Copy link
Contributor

Added a new diagnostic E0720 that closes #1203. Issues a warning when a function let() is called on an object assignment expression.

#1203

Added a new diagnostic E0720 that closes quick-lint#1203. Issues a warning when a
function let() is called on an object assignment expression.

quick-lint#1203
Copy link
Collaborator

@strager strager left a comment

Choose a reason for hiding this comment

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

Looks good overall, but there's a bug which must be fixed ('must fix' comment), and the docs need some improvement ('must fix' comment).

src/quick-lint-js/diag/diagnostic-types-2.h Outdated Show resolved Hide resolved
src/quick-lint-js/fe/parse-statement.cpp Show resolved Hide resolved
docs/errors/E0720.md Outdated Show resolved Hide resolved
docs/errors/E0720.md Outdated Show resolved Hide resolved
docs/errors/E0720.md Outdated Show resolved Hide resolved
@msharipov msharipov marked this pull request as draft March 13, 2024 05:01
E0720 diagnostic did not properly check whether the callee is the 'let'
function. The naming of the diagnostic was also changed to use 'confusing'
instead of 'ambiguous' to more accurately reflect the underlying issue.
The documentation for E0720 did not follow the usual format and did not
properly explain what the issue was and how to fix it.
@msharipov msharipov marked this pull request as ready for review March 13, 2024 21:25
@msharipov
Copy link
Contributor Author

@strager Thank you for the feedback! I have made the necessary corrections to the pull request.

@msharipov msharipov requested a review from strager March 13, 2024 21:39
@strager
Copy link
Collaborator

strager commented Mar 16, 2024

Landed your patch as Git commit c8b3258.

@strager strager closed this Mar 16, 2024
@msharipov msharipov deleted the dev branch March 16, 2024 02:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10$: Warn on let ({x} = y);
2 participants