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
fix: Make no-unused-var function the same way in for..of loops as for..in loops #15868
Conversation
✅ Deploy Preview for docs-eslint canceled.
|
Hi @alexbassy!, thanks for the Pull Request The first commit message isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.
To Fix: You can fix this problem by running Read more about contributing to ESLint here |
|
Hi @alexbassy, thanks for the PR!
According to the discussion in the original issue #2342 to which this behavior relates, it seems that this pattern should have been allowed for On the other hand, a drawback of allowing more patterns is always that some cases where the variable was intended to be used could be missed, and this pattern with const [name] = iterable
if (typeof name !== "undefined") return true Granted, this isn't fully equivalent (in particular, it doesn't account for the possibility that the first element of the sequence is |
Thank you @mdjermanovic 🎈 for some reason, the Happy to go with your suggestion and wait for some more feedback 🤝 |
I've just tried with (maybe I didn't understand, did you mean that varsIgnorePattern doesn't allow these cases?) |
I think it makes sense to match the behavior of for-of to that of for-in. 👍 |
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.
LGTM, thanks for contributing!
Some checks didn't run. I'll close-reopen to see if that will trigger the missing checks. |
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [eslint](https://eslint.org) ([source](https://github.com/eslint/eslint)) | devDependencies | minor | [`8.17.0` -> `8.18.0`](https://renovatebot.com/diffs/npm/eslint/8.17.0/8.18.0) | --- ### Release Notes <details> <summary>eslint/eslint</summary> ### [`v8.18.0`](https://github.com/eslint/eslint/releases/tag/v8.18.0) [Compare Source](eslint/eslint@v8.17.0...v8.18.0) #### Features - [`a6273b8`](eslint/eslint@a6273b8) feat: account for rule creation time in performance reports ([#​15982](eslint/eslint#15982)) (Nitin Kumar) #### Bug Fixes - [`f364d47`](eslint/eslint@f364d47) fix: Make no-unused-vars treat for..of loops same as for..in loops ([#​15868](eslint/eslint#15868)) (Alex Bass) #### Documentation - [`4871047`](eslint/eslint@4871047) docs: Update analytics, canonical URL, ads ([#​15996](eslint/eslint#15996)) (Nicholas C. Zakas) - [`cddad14`](eslint/eslint@cddad14) docs: Add correct/incorrect containers ([#​15998](eslint/eslint#15998)) (Nicholas C. Zakas) - [`b04bc6f`](eslint/eslint@b04bc6f) docs: Add rules meta info to rule pages ([#​15902](eslint/eslint#15902)) (Nicholas C. Zakas) - [`1324f10`](eslint/eslint@1324f10) docs: unify the wording referring to optional exception ([#​15893](eslint/eslint#15893)) (Abdelrahman Elkady) - [`ad54d02`](eslint/eslint@ad54d02) docs: add missing trailing slash to some internal links ([#​15991](eslint/eslint#15991)) (Milos Djermanovic) - [`df7768e`](eslint/eslint@df7768e) docs: Switch to version-relative URLs ([#​15978](eslint/eslint#15978)) (Nicholas C. Zakas) - [`21d6479`](eslint/eslint@21d6479) docs: change some absolute links to relative ([#​15970](eslint/eslint#15970)) (Milos Djermanovic) - [`f31216a`](eslint/eslint@f31216a) docs: Update README team and sponsors (ESLint Jenkins) #### Build Related - [`ed49f15`](eslint/eslint@ed49f15) build: remove unwanted parallel and image-min for dev server ([#​15986](eslint/eslint#15986)) (Strek) #### Chores - [`f6e2e63`](eslint/eslint@f6e2e63) chore: fix 'replaced by' rule list ([#​16007](eslint/eslint#16007)) (Milos Djermanovic) - [`d94dc84`](eslint/eslint@d94dc84) chore: remove unused deprecation warnings ([#​15994](eslint/eslint#15994)) (Francesco Trotta) - [`cdcf11e`](eslint/eslint@cdcf11e) chore: fix versions link ([#​15995](eslint/eslint#15995)) (Milos Djermanovic) - [`d2a8715`](eslint/eslint@d2a8715) chore: add trailing slash to `pathPrefix` ([#​15993](eslint/eslint#15993)) (Milos Djermanovic) - [`58a1bf0`](eslint/eslint@58a1bf0) chore: tweak URL rewriting for local previews ([#​15992](eslint/eslint#15992)) (Milos Djermanovic) - [`80404d2`](eslint/eslint@80404d2) chore: remove docs deploy workflow ([#​15984](eslint/eslint#15984)) (Nicholas C. Zakas) - [`71bc750`](eslint/eslint@71bc750) chore: Set permissions for GitHub actions ([#​15971](eslint/eslint#15971)) (Naveen) - [`90ff647`](eslint/eslint@90ff647) chore: avoid generating subdirectories for each page on new docs site ([#​15967](eslint/eslint#15967)) (Milos Djermanovic) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox. --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). Co-authored-by: cabr2-bot <cabr2.help@gmail.com> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1427 Reviewed-by: Epsilon_02 <epsilon_02@noreply.codeberg.org> Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org> Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
@alexbassy we'd like to pay you for this contribution, but I couldn't find an email address for you. Can you please contact us at contact (at) eslint (dot) org? Thanks. |
Prerequisites checklist
What is the purpose of this pull request? (put an "X" next to an item)
[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[x] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:
What changes did you make? (Give an overview)
Tweaked the
no-unused-vars
rule to treatfor..of
loops andfor..in
loops the sameSorry for the labels 🙈 not sure if this is a bug or a feature
What rule do you want to change?
no-unused-vars`
Does this change cause the rule to produce more or fewer warnings?
Fewer
How will the change be implemented? (New option, new default behavior, etc.)?
for..of
loops will be treated the same way asfor..in
loopsPlease provide some example code that this change will affect:
What does the rule currently do for this code?
It will cause show a
no-unused-vars
error on the variable declaration in the for..of loopWhat will the rule do after it's changed?
It will not show an error
Is there anything you'd like reviewers to focus on?