Update: add ignoreRestSiblings option to no-unused-vars (#7968)

Update: no-unused-vars to account for rest property omissions

Update docs

Update: use RestProperty and only check last property

Temp

temp

temp

Use parent type

Author: zackargyle

docs/rules/no-unused-vars.md

@@ -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 }]
     }
 }
 ```
@@ -195,6 +195,18 @@ Examples of **correct** code for the `{ "args": "none" }` option:
 })();
 ```
 
+### ignoreRestSiblings
+
+The `ignoreRestSiblings` option is a boolean (default: `false`). Using a [Rest Property](https://github.com/sebmarkbage/ecmascript-rest-spread) it is possible to "omit" properties from an object, but by default the sibling properties are marked as "unused". With this option enabled the rest property's siblings are ignored.
+
+Examples of **correct** code for the `{ "ignoreRestSiblings": true }` option:
+
+```js
+/*eslint no-unused-vars: ["error", { "ignoreRestSiblings": true }]*/
+// 'type' is ignored because it has a rest property sibling.
+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.

lib/rules/no-unused-vars.js

@@ -42,6 +42,9 @@ module.exports = {
                             args: {
                                 enum: ["all", "after-used", "none"]
                             },
+                            ignoreRestSiblings: {
+                                type: "boolean"
+                            },
                             argsIgnorePattern: {
                                 type: "string"
                             },
@@ -66,6 +69,7 @@ module.exports = {
         const config = {
             vars: "all",
             args: "after-used",
+            ignoreRestSiblings: false,
             caughtErrors: "none"
         };
 
@@ -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) {
@@ -125,6 +130,30 @@ 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) {
+                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) &&  // last property is a rest property
+                        !restProperties.has(def.name.parent.type)  // variable is sibling of the rest property
+                    ));
+            } else {
+                return false;
+            }
+        }
+
         /**
          * Determines if a reference is a read operation.
          * @param {Reference} ref - An escope Reference
@@ -495,7 +524,7 @@ module.exports = {
                         }
                     }
 
-                    if (!isUsedVariable(variable) && !isExported(variable)) {
+                    if (!isUsedVariable(variable) && !isExported(variable) && !hasRestSpreadSibling(variable)) {
                         unusedVars.push(variable);
                     }
                 }

tests/lib/rules/no-unused-vars.js

@@ -34,6 +34,22 @@ ruleTester.defineRule("use-every-a", context => {
     };
 });
 
+/**
+* Returns an extended test that includes es2017 parser options.
+* @param {Object} test The test to extend
+* @returns {Object} A parser-extended test case
+*/
+function includeRestPropertyParser(test) {
+    return Object.assign({
+        parserOptions: {
+            ecmaVersion: 2017,
+            ecmaFeatures: {
+                experimentalObjectRestSpread: true
+            }
+        }
+    }, test);
+}
+
 /**
 * Returns an expected error for defined-but-not-used variables.
 * @param {string} varName The name of the variable
@@ -179,6 +195,12 @@ ruleTester.run("no-unused-vars", rule, {
             options: [{ vars: "all", args: "all" }]
         },
 
+        // Using object rest for variable omission
+        includeRestPropertyParser({
+            code: "const data = { type: 'coords', x: 1, y: 2 };\nconst { type, ...coords } = data;\n console.log(coords);",
+            options: [{ ignoreRestSiblings: 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);" },
@@ -333,6 +355,47 @@ ruleTester.run("no-unused-vars", rule, {
             ]
         },
 
+        // Rest property sibling without ignoreRestSiblings
+        includeRestPropertyParser({
+            code: "const data = { type: 'coords', x: 1, y: 2 };\nconst { type, ...coords } = data;\n console.log(coords);",
+            errors: [
+                { line: 2, column: 9, message: "'type' is assigned a value but never used." }
+            ]
+        }),
+
+        // Unused rest property with ignoreRestSiblings
+        includeRestPropertyParser({
+            code: "const data = { type: 'coords', x: 1, y: 2 };\nconst { type, ...coords } = data;\n console.log(type)",
+            options: [{ ignoreRestSiblings: true }],
+            errors: [
+                { line: 2, column: 18, message: "'coords' is assigned a value but never used." }
+            ]
+        }),
+
+        // Unused rest property without ignoreRestSiblings
+        includeRestPropertyParser({
+            code: "const data = { type: 'coords', x: 1, y: 2 };\nconst { type, ...coords } = data;\n console.log(type)",
+            errors: [
+                { line: 2, column: 18, message: "'coords' is assigned a value but never used." }
+            ]
+        }),
+
+        // Nested array destructuring with rest property
+        includeRestPropertyParser({
+            code: "const data = { vars: ['x','y'], x: 1, y: 2 };\nconst { vars: [x], ...coords } = data;\n console.log(coords)",
+            errors: [
+                { line: 2, column: 16, message: "'x' is assigned a value but never used." }
+            ]
+        }),
+
+        // Nested object destructuring with rest property
+        includeRestPropertyParser({
+            code: "const data = { defaults: { x: 0 }, x: 1, y: 2 };\nconst { defaults: { x }, ...coords } = data;\n console.log(coords)",
+            errors: [
+                { line: 2, column: 21, message: "'x' is assigned a value but never used." }
+            ]
+        }),
+
         // https://github.com/eslint/eslint/issues/3714
         {
             code: "/* global a$fooz,$foo */\na$fooz;",