Bug 1330147 - add eslint rule to reject newURI(uri, null, null) and r…

…emoveObserver(notificationName, observer, false), r=jaws.

.eslintrc.js

@@ -8,6 +8,7 @@ module.exports = {
   "rules": {
     "mozilla/import-globals": "warn",
     "mozilla/no-import-into-var-and-global": "error",
+    "mozilla/no-useless-parameters": "error",
 
     // No (!foo in bar) or (!object instanceof Class)
     "no-unsafe-negation": "error",

tools/lint/docs/linters/eslint-plugin-mozilla.rst

@@ -125,6 +125,13 @@ import into a var and into the global scope at the same time, e.g.
 This is considered bad practice as it is confusing as to what is actually being
 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)``).
+
 reject-importGlobalProperties
 -----------------------------
 

tools/lint/eslint/eslint-plugin-mozilla/lib/index.js

@@ -26,6 +26,7 @@ module.exports = {
     "no-cpows-in-tests": require("../lib/rules/no-cpows-in-tests"),
     "no-single-arg-cu-import": require("../lib/rules/no-single-arg-cu-import"),
     "no-import-into-var-and-global": require("../lib/rules/no-import-into-var-and-global.js"),
+    "no-useless-parameters": require("../lib/rules/no-useless-parameters"),
     "reject-importGlobalProperties": require("../lib/rules/reject-importGlobalProperties"),
     "reject-some-requires": require("../lib/rules/reject-some-requires"),
     "var-only-at-top-level": require("../lib/rules/var-only-at-top-level")
@@ -40,6 +41,7 @@ module.exports = {
     "no-cpows-in-tests": 0,
     "no-single-arg-cu-import": 0,
     "no-import-into-var-and-global": 0,
+    "no-useless-parameters": 0,
     "reject-importGlobalProperties": 0,
     "reject-some-requires": 0,
     "var-only-at-top-level": 0

tools/lint/eslint/eslint-plugin-mozilla/lib/rules/no-useless-parameters.js

@@ -0,0 +1,50 @@
+/**
+ * @fileoverview Reject common XPCOM methods called with useless optional
+ *               parameters, or non-existent parameters.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+"use strict";
+
+// -----------------------------------------------------------------------------
+// Rule Definition
+// -----------------------------------------------------------------------------
+
+var helpers = require("../helpers");
+
+module.exports = function(context) {
+
+  // ---------------------------------------------------------------------------
+  // Public
+  //  --------------------------------------------------------------------------
+
+  return {
+    "CallExpression": function(node) {
+      let callee = node.callee;
+      if (callee.type !== "MemberExpression" ||
+          callee.property.type !== "Identifier") {
+        return;
+      }
+
+      if (callee.property.name === "removeObserver" &&
+          node.arguments.length === 3) {
+        let arg = node.arguments[2];
+        if (arg.type === "Literal" && (arg.value === false ||
+                                       arg.value === true)) {
+          context.report(node, "removeObserver only takes 2 parameters.");
+        }
+      }
+
+      if (callee.property.name === "newURI" &&
+          node.arguments.length === 3) {
+        let arg = node.arguments[2];
+        if (arg.type === "Literal" && arg.value === null) {
+          context.report(node, "newURI's optional parameters passed as null.");
+        }
+      }
+    }
+  };
+};

tools/lint/eslint/eslint-plugin-mozilla/package.json

@@ -1,6 +1,6 @@
 {
   "name": "eslint-plugin-mozilla",
-  "version": "0.2.7",
+  "version": "0.2.8",
   "description": "A collection of rules that help enforce JavaScript coding standard in the Mozilla project.",
   "keywords": [
     "eslint",

tools/lint/eslint/eslint-plugin-mozilla/tests/no-useless-parameters.js

@@ -0,0 +1,46 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+//------------------------------------------------------------------------------
+// Requirements
+//------------------------------------------------------------------------------
+
+var rule = require("../lib/rules/no-useless-parameters");
+
+//------------------------------------------------------------------------------
+// Tests
+//------------------------------------------------------------------------------
+
+function callError(message) {
+  return [{message: message, type: "CallExpression"}];
+}
+
+exports.runTest = function(ruleTester) {
+  ruleTester.run("no-useless-parameters", rule, {
+    valid: [
+      "Services.removeObserver('notification name', {});",
+      "Services.io.newURI('http://example.com');",
+      "Services.io.newURI('http://example.com', 'utf8');",
+    ],
+    invalid: [
+      {
+        code: "Services.removeObserver('notification name', {}, false);",
+        errors: callError("removeObserver only takes 2 parameters.")
+      },
+      {
+        code: "Services.removeObserver('notification name', {}, true);",
+        errors: callError("removeObserver only takes 2 parameters.")
+      },
+      {
+        code: "Services.io.newURI('http://example.com', null, null);",
+        errors: callError("newURI's optional parameters passed as null.")
+      },
+      {
+        code: "Services.io.newURI('http://example.com', 'utf8', null);",
+        errors: callError("newURI's optional parameters passed as null.")
+      }
+    ]
+  });
+};

tools/lint/eslint/eslint-plugin-mozilla/tests/test-run-all.js

@@ -10,7 +10,7 @@ var ruleTester = new RuleTester();
 
 fs.readdir(__dirname, (err, files) => {
   files.forEach(file => {
-    if (file != "test-run-all.js") {
+    if (file != "test-run-all.js" && !file.endsWith("~")) {
       console.log(`Running ${file}`);
       require("./" + file).runTest(ruleTester);
     }