Chore: enable rest/spread rules on ESLint codebase (#10211)

not-an-aardvark · web-flow · commit 81629d2d92bf · 2018-04-12T23:57:10.000-04:00
This enables the `prefer-rest-params`, `prefer-spread`, and `rest-spread-spacing` rules on the ESLint codebase.
diff --git a/Makefile.js b/Makefile.js
@@ -979,10 +979,7 @@ function createConfigForPerformanceTest() {
         "rules:"
     ];
 
-    content.push.apply(
-        content,
-        ls("lib/rules").map(fileName => `    ${path.basename(fileName, ".js")}: 1`)
-    );
+    content.push(...ls("lib/rules").map(fileName => `    ${path.basename(fileName, ".js")}: 1`));
 
     content.join("\n").to(PERF_ESLINTRC);
 }
diff --git a/lib/file-finder.js b/lib/file-finder.js
@@ -134,7 +134,7 @@ class FileFinder {
 
         // Add what has been cached previously to the cache of each directory searched.
         for (let i = 0; i < searched; i++) {
-            [].push.apply(cache[dirs[i]], cache[directory]);
+            cache[dirs[i]].push(...cache[directory]);
         }
 
         yield* cache[dirs[0]];
diff --git a/lib/linter.js b/lib/linter.js
@@ -284,7 +284,7 @@ function getDirectiveComments(filename, ast, ruleMapper) {
         if (/^eslint-disable-(next-)?line$/.test(match[1]) && comment.loc.start.line === comment.loc.end.line) {
             const directiveType = match[1].slice("eslint-".length);
 
-            [].push.apply(disableDirectives, createDisableDirectives(directiveType, comment.loc.start, directiveValue));
+            disableDirectives.push(...createDisableDirectives(directiveType, comment.loc.start, directiveValue));
         } else if (comment.type === "Block") {
             switch (match[1]) {
                 case "exported":
@@ -297,11 +297,11 @@ function getDirectiveComments(filename, ast, ruleMapper) {
                     break;
 
                 case "eslint-disable":
-                    [].push.apply(disableDirectives, createDisableDirectives("disable", comment.loc.start, directiveValue));
+                    disableDirectives.push(...createDisableDirectives("disable", comment.loc.start, directiveValue));
                     break;
 
                 case "eslint-enable":
-                    [].push.apply(disableDirectives, createDisableDirectives("enable", comment.loc.start, directiveValue));
+                    disableDirectives.push(...createDisableDirectives("enable", comment.loc.start, directiveValue));
                     break;
 
                 case "eslint": {
@@ -724,10 +724,8 @@ const BASE_TRAVERSAL_CONTEXT = Object.freeze(
     Object.keys(DEPRECATED_SOURCECODE_PASSTHROUGHS).reduce(
         (contextInfo, methodName) =>
             Object.assign(contextInfo, {
-                [methodName]() {
-                    const sourceCode = this.getSourceCode();
-
-                    return sourceCode[DEPRECATED_SOURCECODE_PASSTHROUGHS[methodName]].apply(sourceCode, arguments);
+                [methodName](...args) {
+                    return this.getSourceCode()[DEPRECATED_SOURCECODE_PASSTHROUGHS[methodName]](...args);
                 }
             }),
         {}
@@ -816,7 +814,7 @@ function runRules(sourceCode, configuredRules, ruleMapper, parserOptions, parser
                 {
                     id: ruleId,
                     options: getRuleOptions(configuredRules[ruleId]),
-                    report() {
+                    report(...args) {
 
                         /*
                          * Create a report translator lazily.
@@ -831,7 +829,7 @@ function runRules(sourceCode, configuredRules, ruleMapper, parserOptions, parser
                         if (reportTranslator === null) {
                             reportTranslator = createReportTranslator({ ruleId, severity, sourceCode, messageIds });
                         }
-                        const problem = reportTranslator.apply(null, arguments);
+                        const problem = reportTranslator(...args);
 
                         if (problem.fix && rule.meta && !rule.meta.fixable) {
                             throw new Error("Fixable rules should export a `meta.fixable` property.");
diff --git a/lib/logging.js b/lib/logging.js
@@ -14,15 +14,15 @@ module.exports = {
      * Cover for console.log
      * @returns {void}
      */
-    info() {
-        console.log.apply(console, arguments);
+    info(...args) {
+        console.log(...args);
     },
 
     /**
      * Cover for console.error
      * @returns {void}
      */
-    error() {
-        console.error.apply(console, arguments);
+    error(...args) {
+        console.error(...args);
     }
 };
diff --git a/lib/report-translator.js b/lib/report-translator.js
@@ -51,35 +51,35 @@ const interpolate = require("./util/interpolate");
 
 /**
  * Translates a multi-argument context.report() call into a single object argument call
- * @param {...*} arguments A list of arguments passed to `context.report`
+ * @param {...*} args A list of arguments passed to `context.report`
  * @returns {MessageDescriptor} A normalized object containing report information
  */
-function normalizeMultiArgReportCall() {
+function normalizeMultiArgReportCall(...args) {
 
     // If there is one argument, it is considered to be a new-style call already.
-    if (arguments.length === 1) {
+    if (args.length === 1) {
 
         // Shallow clone the object to avoid surprises if reusing the descriptor
-        return Object.assign({}, arguments[0]);
+        return Object.assign({}, args[0]);
     }
 
     // If the second argument is a string, the arguments are interpreted as [node, message, data, fix].
-    if (typeof arguments[1] === "string") {
+    if (typeof args[1] === "string") {
         return {
-            node: arguments[0],
-            message: arguments[1],
-            data: arguments[2],
-            fix: arguments[3]
+            node: args[0],
+            message: args[1],
+            data: args[2],
+            fix: args[3]
         };
     }
 
     // Otherwise, the arguments are interpreted as [node, loc, message, data, fix].
     return {
-        node: arguments[0],
-        loc: arguments[1],
-        message: arguments[2],
-        data: arguments[3],
-        fix: arguments[4]
+        node: args[0],
+        loc: args[1],
+        message: args[2],
+        data: args[3],
+        fix: args[4]
     };
 }
 
@@ -240,8 +240,8 @@ module.exports = function createReportTranslator(metadata) {
      * called every time a rule reports a problem, which happens much less frequently (usually, the vast
      * majority of rules don't report any problems for a given file).
      */
-    return function() {
-        const descriptor = normalizeMultiArgReportCall.apply(null, arguments);
+    return (...args) => {
+        const descriptor = normalizeMultiArgReportCall(...args);
 
         assertValidNodeInfo(descriptor);
 
diff --git a/lib/rules/no-irregular-whitespace.js b/lib/rules/no-irregular-whitespace.js
@@ -225,7 +225,7 @@ module.exports = {
 
                 // If we have any errors remaining report on them
                 errors.forEach(error => {
-                    context.report.apply(context, error);
+                    context.report(...error);
                 });
             };
         } else {
diff --git a/lib/rules/no-shadow.js b/lib/rules/no-shadow.js
@@ -179,7 +179,7 @@ module.exports = {
                 while (stack.length) {
                     const scope = stack.pop();
 
-                    stack.push.apply(stack, scope.childScopes);
+                    stack.push(...scope.childScopes);
                     checkForShadows(scope);
                 }
             }
diff --git a/lib/rules/no-undefined.js b/lib/rules/no-undefined.js
@@ -68,7 +68,7 @@ module.exports = {
                 while (stack.length) {
                     const scope = stack.pop();
 
-                    stack.push.apply(stack, scope.childScopes);
+                    stack.push(...scope.childScopes);
                     checkScope(scope);
                 }
             }
diff --git a/lib/timing.js b/lib/timing.js
@@ -116,10 +116,10 @@ module.exports = (function() {
             data[key] = 0;
         }
 
-        return function() {
+        return function(...args) {
             let t = process.hrtime();
 
-            fn.apply(null, Array.prototype.slice.call(arguments));
+            fn(...args);
             t = process.hrtime(t);
             data[key] += t[0] * 1e3 + t[1] / 1e6;
         };
diff --git a/lib/util/safe-emitter.js b/lib/util/safe-emitter.js
@@ -27,8 +27,6 @@
  * another module throws an error or registers a listener.
  * 2. It calls listener functions without any `this` value. (`EventEmitter` calls listeners with a
  * `this` value of the emitter instance, which would give listeners access to other listeners.)
- * 3. Events can be emitted with at most 3 arguments. (For example: when using `emitter.emit('foo', a, b, c)`,
- * the arguments `a`, `b`, and `c` will be passed to the listener functions.)
  * @returns {SafeEmitter} An emitter
  */
 module.exports = () => {
@@ -42,9 +40,9 @@ module.exports = () => {
                 listeners[eventName] = [listener];
             }
         },
-        emit(eventName, a, b, c) {
+        emit(eventName, ...args) {
             if (eventName in listeners) {
-                listeners[eventName].forEach(listener => listener(a, b, c));
+                listeners[eventName].forEach(listener => listener(...args));
             }
         },
         eventNames() {
diff --git a/packages/eslint-config-eslint/default.yml b/packages/eslint-config-eslint/default.yml
@@ -152,11 +152,14 @@ rules:
     prefer-const: "error"
     prefer-numeric-literals: "error"
     prefer-promise-reject-errors: "error"
+    prefer-rest-params: "error"
+    prefer-spread: "error"
     prefer-template: "error"
     quotes: ["error", "double"]
     quote-props: ["error", "as-needed"]
     radix: "error"
     require-jsdoc: "error"
+    rest-spread-spacing: "error"
     semi: "error"
     semi-spacing: ["error", {before: false, after: true}]
     semi-style: "error"
diff --git a/tests/lib/ast-utils.js b/tests/lib/ast-utils.js
@@ -42,10 +42,10 @@ describe("ast-utils", () => {
      */
     function mustCall(func) {
         callCounts.set(func, 0);
-        return function Wrapper() {
+        return function Wrapper(...args) {
             callCounts.set(func, callCounts.get(func) + 1);
 
-            return func.apply(this, arguments);
+            return func.call(this, ...args);
         };
     }
 
diff --git a/tests/lib/cli-engine.js b/tests/lib/cli-engine.js
@@ -46,15 +46,11 @@ describe("CLIEngine", () => {
      * @returns {string} The path inside the fixture directory.
      * @private
      */
-    function getFixturePath() {
-        const args = Array.prototype.slice.call(arguments);
-
-        args.unshift(fixtureDir);
-        let filepath = path.join.apply(path, args);
+    function getFixturePath(...args) {
+        const filepath = path.join(fixtureDir, ...args);
 
         try {
-            filepath = fs.realpathSync(filepath);
-            return filepath;
+            return fs.realpathSync(filepath);
         } catch (e) {
             return filepath;
         }
@@ -565,11 +561,11 @@ describe("CLIEngine", () => {
             /* eslint-disable no-underscore-dangle */
             before(() => {
                 originalFindPath = Module._findPath;
-                Module._findPath = function(id) {
+                Module._findPath = function(id, ...otherArgs) {
                     if (id === "@scope/eslint-plugin") {
                         return path.resolve(__dirname, "../fixtures/plugin-shorthand/basic/node_modules/@scope/eslint-plugin/index.js");
                     }
-                    return originalFindPath.apply(this, arguments);
+                    return originalFindPath.call(this, id, ...otherArgs);
                 };
             });
             after(() => {
diff --git a/tests/lib/cli.js b/tests/lib/cli.js
@@ -72,11 +72,8 @@ describe("cli", () => {
      * @returns {string} The path inside the fixture directory.
      * @private
      */
-    function getFixturePath() {
-        const args = Array.prototype.slice.call(arguments);
-
-        args.unshift(fixtureDir);
-        return path.join.apply(path, args);
+    function getFixturePath(...args) {
+        return path.join(fixtureDir, ...args);
     }
 
     // copy into clean area so as not to get "infected" by this project's .eslintrc files
diff --git a/tests/lib/config.js b/tests/lib/config.js
@@ -97,12 +97,8 @@ describe("Config", () => {
      * @returns {string} The path inside the fixture directory.
      * @private
      */
-    function getFixturePath() {
-        const args = Array.prototype.slice.call(arguments);
-
-        args.unshift("config-hierarchy");
-        args.unshift(fixtureDir);
-        return path.join.apply(path, args);
+    function getFixturePath(...args) {
+        return path.join(fixtureDir, "config-hierarchy", ...args);
     }
 
     /**
@@ -184,14 +180,8 @@ describe("Config", () => {
          * @returns {string} The path inside the fixture directory.
          * @private
          */
-        function getFakeFixturePath() {
-            const args = Array.prototype.slice.call(arguments);
-
-            args.unshift("config-hierarchy");
-            args.unshift("fixtures");
-            args.unshift("eslint");
-            args.unshift(process.cwd());
-            return path.join.apply(path, args);
+        function getFakeFixturePath(...args) {
+            return path.join(process.cwd(), "eslint", "fixtures", "config-hierarchy", ...args);
         }
 
         before(() => {
@@ -834,14 +824,8 @@ describe("Config", () => {
              * @returns {string} The path inside the fixture directory.
              * @private
              */
-            function getFakeFixturePath() {
-                const args = Array.prototype.slice.call(arguments);
-
-                args.unshift("config-hierarchy");
-                args.unshift("fixtures");
-                args.unshift("eslint");
-                args.unshift(process.cwd());
-                return path.join.apply(path, args);
+            function getFakeFixturePath(...args) {
+                return path.join(process.cwd(), "eslint", "fixtures", "config-hierarchy", ...args);
             }
 
             /**
@@ -979,14 +963,8 @@ describe("Config", () => {
              * @returns {string} The path inside the fixture directory.
              * @private
              */
-            function getFakeFixturePath() {
-                const args = Array.prototype.slice.call(arguments);
-
-                args.unshift("config-hierarchy");
-                args.unshift("fixtures");
-                args.unshift("eslint");
-                args.unshift(process.cwd());
-                return path.join.apply(path, args);
+            function getFakeFixturePath(...args) {
+                return path.join(process.cwd(), "eslint", "fixtures", "config-hierarchy", ...args);
             }
 
             /**
@@ -1117,15 +1095,8 @@ describe("Config", () => {
              * @returns {string} The path inside the fixture directory.
              * @private
              */
-            function getFakeFixturePath() {
-                const pathSegments = Array.from(arguments);
-
-                pathSegments.unshift("config-hierarchy");
-                pathSegments.unshift("fixtures");
-                pathSegments.unshift("eslint");
-                pathSegments.unshift(process.cwd());
-
-                return path.join.apply(path, pathSegments);
+            function getFakeFixturePath(...pathSegments) {
+                return path.join(process.cwd(), "eslint", "fixtures", "config-hierarchy", ...pathSegments);
             }
 
             before(() => {
diff --git a/tests/lib/config/config-initializer.js b/tests/lib/config/config-initializer.js
@@ -64,15 +64,11 @@ describe("configInitializer", () => {
      * @returns {string} The path inside the fixture directory.
      * @private
      */
-    function getFixturePath() {
-        const args = Array.prototype.slice.call(arguments);
-
-        args.unshift(fixtureDir);
-        let filepath = path.join.apply(path, args);
+    function getFixturePath(...args) {
+        const filepath = path.join(fixtureDir, ...args);
 
         try {
-            filepath = fs.realpathSync(filepath);
-            return filepath;
+            return fs.realpathSync(filepath);
         } catch (e) {
             return filepath;
         }
diff --git a/tests/lib/ignored-paths.js b/tests/lib/ignored-paths.js
diff --git a/tests/lib/util/glob-util.js b/tests/lib/util/glob-util.js
diff --git a/tests/lib/util/source-code-util.js b/tests/lib/util/source-code-util.js

Original file line number	Diff line number	Diff line change
`@@ -134,7 +134,7 @@ class FileFinder {`
`134`	`134`
`135`	`135`	`// Add what has been cached previously to the cache of each directory searched.`
`136`	`136`	`for (let i = 0; i < searched; i++) {`
`137`		`- [].push.apply(cache[dirs[i]], cache[directory]);`
	`137`	`+ cache[dirs[i]].push(...cache[directory]);`
`138`	`138`	`}`
`139`	`139`
`140`	`140`	`yield* cache[dirs[0]];`
Original file line number	Diff line number	Diff line change
`@@ -179,7 +179,7 @@ module.exports = {`
`179`	`179`	`while (stack.length) {`
`180`	`180`	`const scope = stack.pop();`
`181`	`181`
`182`		`- stack.push.apply(stack, scope.childScopes);`
	`182`	`+ stack.push(...scope.childScopes);`
`183`	`183`	`checkForShadows(scope);`
`184`	`184`	`}`
`185`	`185`	`}`
Original file line number	Diff line number	Diff line change
`@@ -68,7 +68,7 @@ module.exports = {`
`68`	`68`	`while (stack.length) {`
`69`	`69`	`const scope = stack.pop();`
`70`	`70`
`71`		`- stack.push.apply(stack, scope.childScopes);`
	`71`	`+ stack.push(...scope.childScopes);`
`72`	`72`	`checkScope(scope);`
`73`	`73`	`}`
`74`	`74`	`}`