-
Notifications
You must be signed in to change notification settings - Fork 289
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
[LANGUAGE] Warning unused var #4881
Conversation
…into warning-unused-var
for more information, see https://pre-commit.ci
…into warning-unused-var
for more information, see https://pre-commit.ci
…into warning-unused-var
for more information, see https://pre-commit.ci
…into warning-unused-var
for more information, see https://pre-commit.ci
…into warning-unused-var
for more information, see https://pre-commit.ci
…into undefined-exception
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested it and this works great! I wonder if in the future we'd want to show the user several warnings or errors at the same time? Or do you think this would overwhelm them?
Thank you for contributing! Your pull request is now going on the merge train (choo choo! Do not click update from main anymore, and be sure to allow changes to be pushed to your fork). |
Ow great question! I have to think about that a bit more! |
Thank you for contributing! Your pull request is now going on the merge train (choo choo! Do not click update from main anymore, and be sure to allow changes to be pushed to your fork). |
Fixes #2394 (finally!!) by checking if declared variables are actually used, also fixes #4884 that was introduced here.
Depends-On: #4880
How to test
I have added a test for the exception. To see it in action on the front-end, run code that defines but not uses a variable, and observe you get a warning but the code still runs:
Note that this PR needs #4880 to show the warning in the front-end!