Skip to content

Commit

Permalink
Update: no-unused-vars to account for rest property omissions
Browse files Browse the repository at this point in the history
Update docs

Update: use RestProperty and only check last property

Temp

temp
  • Loading branch information
Zack Argyle committed Jan 24, 2017
1 parent 72d41f0 commit c4efcec
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 2 deletions.
12 changes: 11 additions & 1 deletion docs/rules/no-unused-vars.md
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ By default this rule is enabled with `all` option for variables and `after-used`
```json
{
"rules": {
"no-unused-vars": ["error", { "vars": "all", "args": "after-used" }]
"no-unused-vars": ["error", { "vars": "all", "args": "after-used", "ignoreRestSiblings": false }]
}
}
```
Expand Down Expand Up @@ -195,6 +195,16 @@ Examples of **correct** code for the `{ "args": "none" }` option:
})();
```

### ignoreRestSiblings

The `ignoreRestSiblings` option is a boolean (default: `false`). With modern JavaScript it is possible to "omit" properties from an object by using a rest parameter, but by default the sibling properties are marked as "unused". With this option enabled the omitted properties are considered "used", as their existence has a side-effect. In the example, `type` is considered "used".

```js
/*eslint no-unused-vars: ["error", { "ignoreRestSiblings": true }]*/
// 'type' is considered "used" here, as its existence affects "coords".
var { type, ...coords } = data;
```

### argsIgnorePattern

The `argsIgnorePattern` option specifies exceptions not to check for usage: arguments whose names match a regexp pattern. For example, variables whose names begin with an underscore.
Expand Down
30 changes: 29 additions & 1 deletion lib/rules/no-unused-vars.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ module.exports = {
args: {
enum: ["all", "after-used", "none"]
},
ignoreRestSiblings: {
type: "boolean"
},
argsIgnorePattern: {
type: "string"
},
Expand All @@ -66,6 +69,7 @@ module.exports = {
const config = {
vars: "all",
args: "after-used",
ignoreRestSiblings: false,
caughtErrors: "none"
};

Expand All @@ -77,6 +81,7 @@ module.exports = {
} else {
config.vars = firstOption.vars || config.vars;
config.args = firstOption.args || config.args;
config.ignoreRestSiblings = firstOption.ignoreRestSiblings || config.ignoreRestSiblings;
config.caughtErrors = firstOption.caughtErrors || config.caughtErrors;

if (firstOption.varsIgnorePattern) {
Expand Down Expand Up @@ -125,6 +130,29 @@ module.exports = {
}
}

/**
* Determines if a variable has a sibling rest property
* @param {Variable} variable - EScope variable object.
* @returns {boolean} True if the variable is exported, false if not.
* @private
*/
function hasRestSpreadSibling(variable) {
if (config.ignoreRestSiblings === true) {
const restProperties = new Set(["ExperimentalRestProperty", "RestProperty"]);

return variable.defs
.filter(def => def.name.type === "Identifier")
.some(def => (
def.node.id &&
def.node.id.type === "ObjectPattern" &&
def.node.id.properties.length &&
restProperties.has(def.node.id.properties[def.node.id.properties.length - 1].type)
));
} else {
return false;
}
}

/**
* Determines if a reference is a read operation.
* @param {Reference} ref - An escope Reference
Expand Down Expand Up @@ -495,7 +523,7 @@ module.exports = {
}
}

if (!isUsedVariable(variable) && !isExported(variable)) {
if (!isUsedVariable(variable) && !isExported(variable) && !hasRestSpreadSibling(variable)) {
unusedVars.push(variable);
}
}
Expand Down
27 changes: 27 additions & 0 deletions tests/lib/rules/no-unused-vars.js
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,18 @@ ruleTester.run("no-unused-vars", rule, {
options: [{ vars: "all", args: "all" }]
},

// Using object rest for variable omission
{
code: "const data = { type: 'coords', x: 1, y: 2 };\nconst { type, ...coords } = data;\n console.log(coords);",
options: [{ ignoreRestSiblings: true }],
parserOptions: {
ecmaVersion: 8,
ecmaFeatures: {
experimentalObjectRestSpread: true
}
}
},

// https://github.com/eslint/eslint/issues/6348
{ code: "var a = 0, b; b = a = a + 1; foo(b);" },
{ code: "var a = 0, b; b = a += a + 1; foo(b);" },
Expand Down Expand Up @@ -333,6 +345,21 @@ ruleTester.run("no-unused-vars", rule, {
]
},

// Destructured var without object rest sibling.
{
code: "const data = { type: 'coords', x: 1, y: 2 };\nconst { type } = data;",
options: [{ ignoreRestSiblings: true }],
parserOptions: {
ecmaVersion: 8,
ecmaFeatures: {
experimentalObjectRestSpread: true
}
},
errors: [
{ line: 2, column: 9, message: "'type' is assigned a value but never used." }
]
},

// https://github.com/eslint/eslint/issues/3714
{
code: "/* global a$fooz,$foo */\na$fooz;",
Expand Down

0 comments on commit c4efcec

Please sign in to comment.