Bug 1330147 - add eslint rule to reject newURI(uri, null, null) and removeObserver(notificationName, observer, false), r=jaws.
authorFlorian Quèze <florian@queze.net>
Wed, 11 Jan 2017 22:27:42 +0100
changeset 328981 ac0d90af0af192e2278393292e25df2b6710b4c8
parent 328980 b8257327be700ff391848d9568ba106d45cbf4a3
child 328982 8e0b7bc5fd7164d083261a709bdf3dc0266365ca
push id85601
push userflorian@queze.net
push dateWed, 11 Jan 2017 21:27:40 +0000
treeherdermozilla-inbound@ccf9b5082e8b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjaws
bugs1330147
milestone53.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 1330147 - add eslint rule to reject newURI(uri, null, null) and removeObserver(notificationName, observer, false), r=jaws.
.eslintrc.js
tools/lint/docs/linters/eslint-plugin-mozilla.rst
tools/lint/eslint/eslint-plugin-mozilla/lib/index.js
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
tools/lint/eslint/eslint-plugin-mozilla/tests/test-run-all.js
--- a/.eslintrc.js
+++ b/.eslintrc.js
@@ -3,16 +3,17 @@
 module.exports = {
   // When adding items to this file please check for effects on sub-directories.
   "plugins": [
     "mozilla"
   ],
   "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",
   },
   "env": {
     "es6": true
   },
   "parserOptions": {
--- a/tools/lint/docs/linters/eslint-plugin-mozilla.rst
+++ b/tools/lint/docs/linters/eslint-plugin-mozilla.rst
@@ -120,16 +120,23 @@ no-import-into-var-and-global
 Reject use of ``Cu.import`` (or ``Components.utils.import``) where it attempts to
 import into a var and into the global scope at the same time, e.g.
 
 ``var foo = Cu.import("path.jsm", this);``
 
 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
 -----------------------------
 
 Rejects calls to ``Cu.importGlobalProperties``.  Use of this function is
 undesirable in some parts of the tree.
 
 
 reject-some-requires
--- a/tools/lint/eslint/eslint-plugin-mozilla/lib/index.js
+++ b/tools/lint/eslint/eslint-plugin-mozilla/lib/index.js
@@ -21,27 +21,29 @@ module.exports = {
     "import-globals": require("../lib/rules/import-globals"),
     "import-headjs-globals": require("../lib/rules/import-headjs-globals"),
     "import-browserjs-globals": require("../lib/rules/import-browserjs-globals"),
     "mark-test-function-used": require("../lib/rules/mark-test-function-used"),
     "no-aArgs": require("../lib/rules/no-aArgs"),
     "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")
   },
   rulesConfig: {
     "balanced-listeners": 0,
     "import-globals": 0,
     "import-headjs-globals": 0,
     "import-browserjs-globals": 0,
     "mark-test-function-used": 0,
     "no-aArgs": 0,
     "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
   }
 };
new file mode 100644
--- /dev/null
+++ b/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.");
+        }
+      }
+    }
+  };
+};
--- 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.7",
+  "version": "0.2.8",
   "description": "A collection of rules that help enforce JavaScript coding standard in the Mozilla project.",
   "keywords": [
     "eslint",
     "eslintplugin",
     "eslint-plugin",
     "mozilla",
     "firefox"
   ],
new file mode 100644
--- /dev/null
+++ b/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.")
+      }
+    ]
+  });
+};
--- a/tools/lint/eslint/eslint-plugin-mozilla/tests/test-run-all.js
+++ b/tools/lint/eslint/eslint-plugin-mozilla/tests/test-run-all.js
@@ -5,14 +5,14 @@
 
 const RuleTester = require("eslint/lib/testers/rule-tester");
 const fs = require('fs');
 
 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);
     }
   });
 });