Bug 1358484 - Add an autofix option to ESLint's no-useless-parameters. r=florian
authorMark Banner <standard8@mozilla.com>
Fri, 21 Apr 2017 14:34:37 +0100
changeset 402874 ba2e842d469aea679e4140ba5dde2e655d65e8aa
parent 402873 3c2c6b5eb8812c59a410ce51a82ea3aefbefb713
child 402875 66bfd5c79d988fc2eadef5bef8e97832a0f2e5ea
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersflorian
bugs1358484
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1358484 - Add an autofix option to ESLint's no-useless-parameters. r=florian MozReview-Commit-ID: B8mQteJ1CzF
tools/lint/docs/linters/eslint-plugin-mozilla.rst
tools/lint/eslint/eslint-plugin-mozilla/lib/rules/no-useless-parameters.js
tools/lint/eslint/eslint-plugin-mozilla/package.json
tools/lint/eslint/eslint-plugin-mozilla/tests/no-useless-parameters.js
--- a/tools/lint/docs/linters/eslint-plugin-mozilla.rst
+++ b/tools/lint/docs/linters/eslint-plugin-mozilla.rst
@@ -187,16 +187,18 @@ imported.
 
 no-useless-parameters
 ---------------------
 
 Reject common XPCOM methods called with useless optional parameters (eg.
 ``Services.io.newURI(url, null, null)``, or non-existent parameters (eg.
 ``Services.obs.removeObserver(name, observer, false)``).
 
+This option can be autofixed (``--fix``).
+
 no-useless-removeEventListener
 ------------------------------
 
 Reject calls to removeEventListener where {once: true} could be used instead.
 
 reject-importGlobalProperties
 -----------------------------
 
--- a/tools/lint/eslint/eslint-plugin-mozilla/lib/rules/no-useless-parameters.js
+++ b/tools/lint/eslint/eslint-plugin-mozilla/lib/rules/no-useless-parameters.js
@@ -9,16 +9,21 @@
 
 "use strict";
 
 // -----------------------------------------------------------------------------
 // Rule Definition
 // -----------------------------------------------------------------------------
 
 module.exports = function(context) {
+  function getRangeAfterArgToEnd(argNumber, args) {
+    let sourceCode = context.getSourceCode();
+    return [sourceCode.getTokenAfter(args[argNumber]).range[0],
+      args[args.length - 1].range[1]];
+  }
 
   // ---------------------------------------------------------------------------
   // Public
   //  --------------------------------------------------------------------------
 
   return {
     "CallExpression": function(node) {
       let callee = node.callee;
@@ -31,44 +36,86 @@ module.exports = function(context) {
       let isFalsy = arg => arg.type === "Literal" && !arg.value;
       let isBool = arg => arg.type === "Literal" && (arg.value === false ||
                                                      arg.value === true);
       let name = callee.property.name;
       let args = node.arguments;
 
       if (["addEventListener", "removeEventListener", "addObserver"]
           .includes(name) && args.length === 3 && isFalse(args[2])) {
-        context.report(node, name +
-                       "'s third parameter can be omitted when it's false.");
+        context.report({
+          node,
+          fix: fixer => {
+            return fixer.removeRange(getRangeAfterArgToEnd(1, args));
+          },
+          message: `${name}'s third parameter can be omitted when it's false.`
+        });
       }
 
       if (name === "clearUserPref" && args.length > 1) {
-        context.report(node, name + " takes only 1 parameter.");
+        context.report({
+          node,
+          fix: fixer => {
+            return fixer.removeRange(getRangeAfterArgToEnd(0, args));
+          },
+          message: `${name} takes only 1 parameter.`
+        });
       }
 
       if (name === "removeObserver" && args.length === 3 && isBool(args[2])) {
-        context.report(node, "removeObserver only takes 2 parameters.");
+        context.report({
+          node,
+          fix: fixer => {
+            return fixer.removeRange(getRangeAfterArgToEnd(1, args));
+          },
+          message: "removeObserver only takes 2 parameters."
+        });
       }
 
       if (name === "appendElement" && args.length === 2 && isFalse(args[1])) {
-        context.report(node, name +
-                       "'s second parameter can be omitted when it's false.");
+        context.report({
+          node,
+          fix: fixer => {
+            return fixer.removeRange(getRangeAfterArgToEnd(0, args));
+          },
+          message: `${name}'s second parameter can be omitted when it's false.`
+        });
       }
 
       if (name === "notifyObservers" && args.length === 3 &&
           isFalsy(args[2])) {
-        context.report(node, name +
-                       "'s third parameter can be omitted.");
+        context.report({
+          node,
+          fix: fixer => {
+            return fixer.removeRange(getRangeAfterArgToEnd(1, args));
+          },
+          message: `${name}'s third parameter can be omitted.`
+        });
       }
 
       if (name === "getComputedStyle" && args.length === 2 &&
           isFalsy(args[1])) {
-        context.report(node,
-                       "getComputedStyle's second parameter can be omitted.");
+        context.report({
+          node,
+          fix: fixer => {
+            return fixer.removeRange(getRangeAfterArgToEnd(0, args));
+          },
+          message: "getComputedStyle's second parameter can be omitted."
+        });
       }
 
       if (name === "newURI" && args.length > 1 &&
           isFalsy(args[args.length - 1])) {
-        context.report(node, "newURI's last parameters are optional.");
+        context.report({
+          node,
+          fix: fixer => {
+            if (args.length > 2 && isFalsy(args[args.length - 2])) {
+              return fixer.removeRange(getRangeAfterArgToEnd(0, args));
+            }
+
+            return fixer.removeRange(getRangeAfterArgToEnd(args.length - 2, args));
+          },
+          message: "newURI's last parameters are optional."
+        });
       }
     }
   };
 };
--- a/tools/lint/eslint/eslint-plugin-mozilla/package.json
+++ b/tools/lint/eslint/eslint-plugin-mozilla/package.json
@@ -1,11 +1,11 @@
 {
   "name": "eslint-plugin-mozilla",
-  "version": "0.2.43",
+  "version": "0.2.45",
   "description": "A collection of rules that help enforce JavaScript coding standard in the Mozilla project.",
   "keywords": [
     "eslint",
     "eslintplugin",
     "eslint-plugin",
     "mozilla",
     "firefox"
   ],
--- a/tools/lint/eslint/eslint-plugin-mozilla/tests/no-useless-parameters.js
+++ b/tools/lint/eslint/eslint-plugin-mozilla/tests/no-useless-parameters.js
@@ -39,87 +39,109 @@ ruleTester.run("no-useless-parameters", 
     "Services.obs.notifyObservers(obj, 'topic', 'data');",
     "Services.obs.notifyObservers(obj, 'topic');",
     "window.getComputedStyle(elt);",
     "window.getComputedStyle(elt, ':before');"
   ],
   invalid: [
     {
       code: "Services.prefs.clearUserPref('browser.search.custom', false);",
+      output: "Services.prefs.clearUserPref('browser.search.custom');",
+      errors: callError("clearUserPref takes only 1 parameter.")
+    },
+    {
+      code: "Services.prefs.clearUserPref('browser.search.custom',\n   false);",
+      output: "Services.prefs.clearUserPref('browser.search.custom');",
       errors: callError("clearUserPref takes only 1 parameter.")
     },
     {
       code: "Services.removeObserver('notification name', {}, false);",
+      output: "Services.removeObserver('notification name', {});",
       errors: callError("removeObserver only takes 2 parameters.")
     },
     {
       code: "Services.removeObserver('notification name', {}, true);",
+      output: "Services.removeObserver('notification name', {});",
       errors: callError("removeObserver only takes 2 parameters.")
     },
     {
       code: "Services.io.newURI('http://example.com', null, null);",
+      output: "Services.io.newURI('http://example.com');",
       errors: callError("newURI's last parameters are optional.")
     },
     {
       code: "Services.io.newURI('http://example.com', 'utf8', null);",
+      output: "Services.io.newURI('http://example.com', 'utf8');",
       errors: callError("newURI's last parameters are optional.")
     },
     {
       code: "Services.io.newURI('http://example.com', null);",
+      output: "Services.io.newURI('http://example.com');",
       errors: callError("newURI's last parameters are optional.")
     },
     {
       code: "Services.io.newURI('http://example.com', '', '');",
+      output: "Services.io.newURI('http://example.com');",
       errors: callError("newURI's last parameters are optional.")
     },
     {
       code: "Services.io.newURI('http://example.com', '');",
+      output: "Services.io.newURI('http://example.com');",
       errors: callError("newURI's last parameters are optional.")
     },
     {
       code: "elt.addEventListener('click', handler, false);",
+      output: "elt.addEventListener('click', handler);",
       errors: callError(
         "addEventListener's third parameter can be omitted when it's false.")
     },
     {
       code: "elt.removeEventListener('click', handler, false);",
+      output: "elt.removeEventListener('click', handler);",
       errors: callError(
         "removeEventListener's third parameter can be omitted when it's" +
         " false.")
     },
     {
       code: "Services.obs.addObserver(this, 'topic', false);",
+      output: "Services.obs.addObserver(this, 'topic');",
       errors: callError(
         "addObserver's third parameter can be omitted when it's" +
         " false.")
     },
     {
       code: "Services.prefs.addObserver('branch', this, false);",
+      output: "Services.prefs.addObserver('branch', this);",
       errors: callError(
         "addObserver's third parameter can be omitted when it's" +
         " false.")
     },
     {
       code: "array.appendElement(elt, false);",
+      output: "array.appendElement(elt);",
       errors: callError(
         "appendElement's second parameter can be omitted when it's" +
         " false.")
     },
     {
       code: "Services.obs.notifyObservers(obj, 'topic', null);",
+      output: "Services.obs.notifyObservers(obj, 'topic');",
       errors: callError(
         "notifyObservers's third parameter can be omitted.")
     },
     {
       code: "Services.obs.notifyObservers(obj, 'topic', '');",
+      output: "Services.obs.notifyObservers(obj, 'topic');",
       errors: callError(
         "notifyObservers's third parameter can be omitted.")
     },
     {
       code: "window.getComputedStyle(elt, null);",
+      output: "window.getComputedStyle(elt);",
       errors: callError("getComputedStyle's second parameter can be omitted.")
     },
     {
       code: "window.getComputedStyle(elt, '');",
+      output: "window.getComputedStyle(elt);",
       errors: callError("getComputedStyle's second parameter can be omitted.")
     }
   ]
 });