-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
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: add destructuredArrayIgnorePattern
option in no-unused-vars
#15649
Conversation
Hi @snitin315!, thanks for the Pull Request The pull request title 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 clicking 'Edit' next to the pull request title at the top of this page. Read more about contributing to ESLint here |
destructuredArrayIgnorePattern
option in no-unused-vars
ruledestructuredArrayIgnorePattern
option in no-unused-vars
Should the option also allow this: /*eslint no-unused-vars: ["error", { "destructuredArrayIgnorePattern": "^_" }]*/
let _a, b;
foo.forEach(item => {
[_a, b] = item;
doSomething(b);
}); That might be consistent with the change we recently accepted (#14163) for option /*eslint no-unused-vars: ["error", { "ignoreRestSiblings": true }]*/
let a, b; // 'a' is allowed because it is used as a rest sibling
foo.forEach(item => {
({ a, ...b } = item);
doSomething(b);
}); |
Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
I agree, it makes sense to allow this. |
@mdjermanovic It seems /*eslint no-unused-vars: ["error", { "ignoreRestSiblings": true }]*/
let a, b; // reports a
foo.forEach(item => {
([ a, ...b ] = item);
console.log(b);
});
const [c, ...d] = []; // reports c
console.log(d)
const {e, ...f} = {}; // ok, doesn't report e
console.log(f);
let x, y; // ok, doesn't report x
foo.forEach(item => {
({x, ...y} = item);
console.log(y);
}); |
I think it works as intended because documentation for |
Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
const [a, _b, c] = ["a", "b", "c"]; | ||
console.log(a+c); | ||
|
||
const { x: [_a, foo] } = bar; | ||
console.log(foo); | ||
|
||
function baz([_c, x]) { | ||
x; | ||
} | ||
baz(); | ||
|
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'm confused. in an array pattern, you never need an unused variable, you just omit the binding - ie, [, x]
etc. Why would you ever want an exception here?
It's only in object destructuring that omitting the binding has a use case.
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.
The rationale for this option is in Additional Comments of the original post in #15611.
This is optional behavior, unused variables in array patterns are not ignored by default. Users who prefer elisions can simply not enable this option.
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.
So, this is a style option, added after the prohibition on such things?
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!
I have error with this rule on Eslint 8.11.0 My config:
Error:
|
Can you once try the following config? Maybe
|
I have installed: With this configuration, I have an error
But there is one strange thing.
In VSC and Eslint plugin - everything is correct, but I have problem only in CLI with eslint |
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [eslint](https://eslint.org) ([source](https://github.com/eslint/eslint)) | devDependencies | minor | [`8.10.0` -> `8.11.0`](https://renovatebot.com/diffs/npm/eslint/8.10.0/8.11.0) | --- ### Release Notes <details> <summary>eslint/eslint</summary> ### [`v8.11.0`](https://github.com/eslint/eslint/releases/v8.11.0) [Compare Source](eslint/eslint@v8.10.0...v8.11.0) #### Features - [`800bd25`](eslint/eslint@800bd25) feat: add `destructuredArrayIgnorePattern` option in `no-unused-vars` ([#​15649](eslint/eslint#15649)) (Nitin Kumar) - [`8933fe7`](eslint/eslint@8933fe7) feat: Catch `undefined` and `Boolean()` in no-constant-condition ([#​15613](eslint/eslint#15613)) (Jordan Eldredge) - [`f90fd9d`](eslint/eslint@f90fd9d) feat: Add ESLint favicon to the HTML report document ([#​15671](eslint/eslint#15671)) (Mahdi Hosseinzadeh) - [`57b8a57`](eslint/eslint@57b8a57) feat: `valid-typeof` always ban `undefined` ([#​15635](eslint/eslint#15635)) (Zzzen) #### Bug Fixes - [`6814922`](eslint/eslint@6814922) fix: escaping for square brackets in ignore patterns ([#​15666](eslint/eslint#15666)) (Milos Djermanovic) - [`c178ce7`](eslint/eslint@c178ce7) fix: extend the autofix range in comma-dangle to ensure the last element ([#​15669](eslint/eslint#15669)) (Milos Djermanovic) #### Documentation - [`c481cec`](eslint/eslint@c481cec) docs: add fast-eslint-8 to atom integrations (userguide) ([#​15695](eslint/eslint#15695)) (db developer) - [`d2255db`](eslint/eslint@d2255db) docs: Add clarification about `eslint-enable` ([#​15680](eslint/eslint#15680)) (dosisod) - [`8b9433c`](eslint/eslint@8b9433c) docs: add object pattern to first section of computed-property-spacing ([#​15679](eslint/eslint#15679)) (Milos Djermanovic) - [`de800c3`](eslint/eslint@de800c3) docs: link to minimatch docs added. ([#​15688](eslint/eslint#15688)) (Gaurav Tewari) - [`8f675b1`](eslint/eslint@8f675b1) docs: sort-imports add single named import example ([#​15675](eslint/eslint#15675)) (Arye Eidelman) #### Chores - [`385c9ad`](eslint/eslint@385c9ad) chore: rm trailing space in docs ([#​15689](eslint/eslint#15689)) (唯然) </details> --- ### Configuration 📅 **Schedule**: 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/1217 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>
I have the same issue as @pstachula-dev after this PR change |
…eslint#15649) * feat: add `destructuredArrayIgnorePattern` option in `no-unused-vars` Fixes eslint#15611 * docs: add `destructuredArrayIgnorePattern` option in `no-unused-vars` * fix: remove false positives and false negatives * docs: update * test: add more cases * docs: update * fix: remove false positives * docs: update Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com> * feat: improve error message * feat: cover more cases * docs: add more example * refactor: code * fix: check for all references * docs: add more examples * chore: apply suggestions from code review Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com> Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
…ed-vars` (eslint#15649)" This reverts commit 8bfe607.
Prerequisites checklist
What is the purpose of this pull request? (put an "X" next to an item)
[x] 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)
Fix #15611
Added a new option
destructuredArrayIgnorePattern
to theno-unused-vars
rule.Is there anything you'd like reviewers to focus on?
No