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.

Sign up for GitHub

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

Jump to bottom

feat: add destructuredArrayIgnorePattern option in no-unused-vars #15649

Merged
merged 15 commits into from Mar 11, 2022
Merged

feat: add destructuredArrayIgnorePattern option in no-unused-vars #15649

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
2babca8
feat: add `destructuredArrayIgnorePattern` option in `no-unused-vars`
snitin315 Feb 26, 2022
2a8de61
docs: add `destructuredArrayIgnorePattern` option in `no-unused-vars`
snitin315 Feb 26, 2022
74ffd7f
fix: remove false positives and false negatives
snitin315 Mar 1, 2022
a84c10a
docs: update
snitin315 Mar 1, 2022
0c8a455
test: add more cases
snitin315 Mar 1, 2022
349a728
docs: update
snitin315 Mar 1, 2022
c2a8829
fix: remove false positives
snitin315 Mar 1, 2022
3291527
docs: update
snitin315 Mar 2, 2022
84612e5
feat: improve error message
snitin315 Mar 5, 2022
41102f8
feat: cover more cases
snitin315 Mar 5, 2022
6e6e1a6
docs: add more example
snitin315 Mar 5, 2022
1cf3d93
refactor: code
snitin315 Mar 5, 2022
3a1a4b2
fix: check for all references
snitin315 Mar 9, 2022
e755d3f
docs: add more examples
snitin315 Mar 9, 2022
bcebe25
chore: apply suggestions from code review
snitin315 Mar 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
26 changes: 26 additions & 0 deletions docs/rules/no-unused-vars.md
View file
Open in desktop
Expand Up @@ -238,6 +238,32 @@ function foo(x, _y) {
foo();
```

### destructuredArrayIgnorePattern

The `destructuredArrayIgnorePattern` option specifies exceptions not to check for usage: destructuring assignments from an array whose names match a regexp pattern. For example, variables whose names begin with an underscore.
snitin315 marked this conversation as resolved.
Show resolved Hide resolved

Examples of **correct** code for the `{ "destructuredArrayIgnorePattern": "^_" }` option:

```js
/*eslint no-unused-vars: ["error", { "destructuredArrayIgnorePattern": "^_" }]*/

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();

Comment on lines +250 to +260
Copy link
Sponsor Contributor

@ljharb ljharb Mar 10, 2022 •
edited

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.

Copy link
Member

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.

Copy link
Sponsor Contributor

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?

function test({p: [_q, r]}) {
r;
}
test();
```

### caughtErrors

The `caughtErrors` option is used for `catch` block arguments validation.
Expand Down
23 changes: 22 additions & 1 deletion lib/rules/no-unused-vars.js
View file
Open in desktop
Expand Up @@ -67,6 +67,9 @@ module.exports = {
},
caughtErrorsIgnorePattern: {
type: "string"
},
destructuredArrayIgnorePattern: {
type: "string"
}
},
additionalProperties: false
Expand Down Expand Up @@ -114,6 +117,10 @@ module.exports = {
if (firstOption.caughtErrorsIgnorePattern) {
config.caughtErrorsIgnorePattern = new RegExp(firstOption.caughtErrorsIgnorePattern, "u");
}

if (firstOption.destructuredArrayIgnorePattern) {
config.destructuredArrayIgnorePattern = new RegExp(firstOption.destructuredArrayIgnorePattern, "u");
}
}
}

Expand Down Expand Up @@ -155,7 +162,16 @@ module.exports = {
* @returns {UnusedVarMessageData} The message data to be used with this unused variable.
*/
function getAssignedMessageData(unusedVar) {
const additional = config.varsIgnorePattern ? `. Allowed unused vars must match ${config.varsIgnorePattern.toString()}` : "";
const def = unusedVar.defs && unusedVar.defs[0];
snitin315 marked this conversation as resolved.
Show resolved Hide resolved
let patternToMatch;

if (config.destructuredArrayIgnorePattern && def.name.parent.type === "ArrayPattern") {
snitin315 marked this conversation as resolved.
Show resolved Hide resolved
patternToMatch = config.destructuredArrayIgnorePattern;
} else if (config.varsIgnorePattern) {
patternToMatch = config.varsIgnorePattern;
}

const additional = patternToMatch ? `. Allowed unused vars must match ${patternToMatch.toString()}` : "";
snitin315 marked this conversation as resolved.
Show resolved Hide resolved

return {
varName: unusedVar.name,
Expand Down Expand Up @@ -585,6 +601,11 @@ module.exports = {
if (def) {
const type = def.type;

// skip array patterns
if (def.name.parent.type === "ArrayPattern" && config.destructuredArrayIgnorePattern && config.destructuredArrayIgnorePattern.test(def.name.name)) {
continue;
}

// skip catch variables
if (type === "CatchClause") {
if (config.caughtErrors === "none") {
Expand Down
108 changes: 108 additions & 0 deletions tests/lib/rules/no-unused-vars.js
View file
Open in desktop
Expand Up @@ -162,6 +162,12 @@ ruleTester.run("no-unused-vars", rule, {
{ code: "function foo(_a) { } foo();", options: [{ args: "all", argsIgnorePattern: "^_" }] },
{ code: "function foo(a, _b) { return a; } foo();", options: [{ args: "after-used", argsIgnorePattern: "^_" }] },
{ code: "var [ firstItemIgnored, secondItem ] = items;\nconsole.log(secondItem);", options: [{ vars: "all", varsIgnorePattern: "[iI]gnored" }], parserOptions: { ecmaVersion: 6 } },
{ code: "const [ a, _b, c ] = items;\nconsole.log(a+c);", options: [{ destructuredArrayIgnorePattern: "^_" }], parserOptions: { ecmaVersion: 6 } },
{ code: "const [ [a, _b, c] ] = items;\nconsole.log(a+c);", options: [{ destructuredArrayIgnorePattern: "^_" }], parserOptions: { ecmaVersion: 6 } },
{ code: "const { x: [_a, foo] } = bar;\nconsole.log(foo);", options: [{ destructuredArrayIgnorePattern: "^_" }], parserOptions: { ecmaVersion: 6 } },
{ code: "function baz([_b, foo]) { foo; };\nbaz()", options: [{ destructuredArrayIgnorePattern: "^_" }], parserOptions: { ecmaVersion: 6 } },
{ code: "function baz({x: [_b, foo]}) {foo};\nbaz()", options: [{ destructuredArrayIgnorePattern: "^_" }], parserOptions: { ecmaVersion: 6 } },
{ code: "function baz([{x: [_b, foo]}]) {foo};\nbaz()", options: [{ destructuredArrayIgnorePattern: "^_" }], parserOptions: { ecmaVersion: 6 } },

// for-in loops (see #2342)
"(function(obj) { var name; for ( name in obj ) return; })({});",
Expand Down Expand Up @@ -463,6 +469,108 @@ ruleTester.run("no-unused-vars", rule, {
}]
},

// https://github.com/eslint/eslint/issues/15611
{
code: `
const array = ['a', 'b', 'c'];
const [a, _b, c] = array;
const newArray = [a, c];
`,
options: [{ destructuredArrayIgnorePattern: "^_" }],
parserOptions: { ecmaVersion: 2020 },
errors: [

// should report only `newArray`
{ ...assignedError("newArray"), line: 4, column: 19 }
]
},
{
code: `
const array = ['a', 'b', 'c', 'd', 'e'];
const [a, _b, c] = array;
`,
options: [{ destructuredArrayIgnorePattern: "^_" }],
parserOptions: { ecmaVersion: 2020 },
errors: [
{
...assignedError("a", ". Allowed unused vars must match /^_/u"),
line: 3,
column: 20
},
{
...assignedError("c", ". Allowed unused vars must match /^_/u"),
line: 3,
column: 27
}
]
},
{
code: `
const array = ['a', 'b', 'c'];
const [a, _b, c] = array;
const fooArray = ['foo'];
const barArray = ['bar'];
const ignoreArray = ['ignore'];
`,
options: [{ destructuredArrayIgnorePattern: "^_", varsIgnorePattern: "ignore" }],
parserOptions: { ecmaVersion: 2020 },
errors: [
{
...assignedError("a", ". Allowed unused vars must match /^_/u"),
line: 3,
column: 20
},
{
...assignedError("c", ". Allowed unused vars must match /^_/u"),
line: 3,
column: 27
},
{
...assignedError("fooArray", ". Allowed unused vars must match /ignore/u"),
line: 4,
column: 19
},
{
...assignedError("barArray", ". Allowed unused vars must match /ignore/u"),
line: 5,
column: 19
}
]
},
{
code: `
const array = [obj];
const [{_a, foo}] = array;
console.log(foo);
`,
options: [{ destructuredArrayIgnorePattern: "^_" }],
parserOptions: { ecmaVersion: 2020 },
errors: [
{
...assignedError("_a"),
line: 3,
column: 21
}
]
},
{
code: `
function foo([{_a, bar}]) {
bar;
}
foo();
`,
options: [{ destructuredArrayIgnorePattern: "^_" }],
parserOptions: { ecmaVersion: 2020 },
errors: [
{
...definedError("_a"),
line: 2,
column: 28
}
]
},

// for-in loops (see #2342)
{
code: "(function(obj) { var name; for ( name in obj ) { i(); return; } })({});",
Expand Down