Bug 1536556 - Add custom no-throw-cr-literal ESLint rule, and enable it by default. r=Standard8

This rule is based on the ESLint built-in no-throw-literal. Cr.ERRORs are also
literals since they are just integers and so have all the same disadvantages of
no stack info.

TestInterfaceJS.js is explicitly testing handling of throwing raw Cr.ERRORs and
thus needs to stay.

Differential Revision: https://phabricator.services.mozilla.com/D28072

docs/code-quality/lint/linters/eslint-plugin-mozilla.rst

@@ -183,6 +183,25 @@ This disallows statements such as:
 These used to be necessary but have now been defined globally for all chrome
 contexts.
 
+no-throw-cr-literal
+-------------------
+
+This is similar to the ESLint built-in rule no-throw-literal.
+It disallows statements such as:
+
+.. code-block:: js
+
+   throw Cr.NS_ERROR_UNEXPECTED;
+   throw Components.results.NS_ERROR_ABORT;
+
+Throwing bare literals is inferior to throwing Exception objects, which provide
+stack information.  Cr.ERRORs should be be passed as the second argument to
+``Components.Exception()`` to create an Exception object with stack info, and
+the correct result property corresponding to the NS_ERROR that other code
+expects.
+
+This option can be autofixed (``--fix``).
+
 no-useless-parameters
 ---------------------
 

dom/bindings/test/TestInterfaceJS.js

@@ -125,6 +125,8 @@ TestInterfaceJS.prototype = {
   },
 
   testThrowNsresult() {
+    // This is explicitly testing preservation of raw thrown Crs in XPCJS
+    // eslint-disable-next-line mozilla/no-throw-cr-literal
     throw Cr.NS_BINDING_ABORTED;
   },
 

tools/lint/eslint/eslint-plugin-mozilla/lib/configs/recommended.js

@@ -124,6 +124,7 @@ module.exports = {
     "mozilla/import-globals": "error",
     "mozilla/no-compare-against-boolean-literals": "error",
     "mozilla/no-define-cc-etc": "error",
+    "mozilla/no-throw-cr-literal": "error",
     "mozilla/no-useless-parameters": "error",
     "mozilla/no-useless-removeEventListener": "error",
     "mozilla/prefer-boolean-length-check": "error",

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

@@ -48,6 +48,7 @@ module.exports = {
     "no-compare-against-boolean-literals": require("../lib/rules/no-compare-against-boolean-literals"),
     "no-define-cc-etc": require("../lib/rules/no-define-cc-etc"),
     "no-task": require("../lib/rules/no-task"),
+    "no-throw-cr-literal": require("../lib/rules/no-throw-cr-literal"),
     "no-useless-parameters": require("../lib/rules/no-useless-parameters"),
     "no-useless-removeEventListener": require("../lib/rules/no-useless-removeEventListener"),
     "no-useless-run-test": require("../lib/rules/no-useless-run-test"),

tools/lint/eslint/eslint-plugin-mozilla/lib/rules/no-throw-cr-literal.js

@@ -0,0 +1,62 @@
+/**
+ * @fileoverview Rule to prevent throwing bare Cr.ERRORs.
+ *
+ * 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
+// -----------------------------------------------------------------------------
+
+module.exports = {
+  meta: {
+    fixable: "code",
+    messages: {
+      bareCR: "Do not throw bare Cr.ERRORs, use Components.Exception instead",
+      bareComponentsResults:
+        "Do not throw bare Components.results.ERRORs, use Components.Exception instead",
+    },
+  },
+
+  create(context) {
+    return {
+      ThrowStatement(node) {
+        if (node.argument.type === "MemberExpression") {
+          function fix(fixer) {
+            const sourceCode = context.getSourceCode();
+            const source = sourceCode.getText(node.argument);
+            return fixer.replaceText(
+              node.argument,
+              `Components.Exception("", ${source})`
+            );
+          }
+
+          const obj = node.argument.object;
+          if (obj.type === "Identifier" && obj.name === "Cr") {
+            context.report({
+              node,
+              messageId: "bareCR",
+              fix,
+            });
+          } else if (
+            obj.type === "MemberExpression" &&
+            obj.object.type === "Identifier" &&
+            obj.object.name === "Components" &&
+            obj.property.type === "Identifier" &&
+            obj.property.name === "results"
+          ) {
+            context.report({
+              node,
+              messageId: "bareComponentsResults",
+              fix,
+            });
+          }
+        }
+      },
+    };
+  },
+};

tools/lint/eslint/eslint-plugin-mozilla/tests/no-throw-cr-literal.js

@@ -0,0 +1,52 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// ------------------------------------------------------------------------------
+// Requirements
+// ------------------------------------------------------------------------------
+
+var rule = require("../lib/rules/no-throw-cr-literal");
+var RuleTester = require("eslint").RuleTester;
+
+const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 6 } });
+
+// ------------------------------------------------------------------------------
+// Tests
+// ------------------------------------------------------------------------------
+
+function invalidCode(code, output, messageId) {
+  return {
+    code,
+    output,
+    errors: [{ messageId, type: "ThrowStatement" }],
+  };
+}
+
+ruleTester.run("no-throw-cr-literal", rule, {
+  valid: [
+    'throw Components.Exception("", Cr.NS_ERROR_NOT_IMPLEMENTED);',
+    'throw Components.Exception("", Components.results.NS_ERROR_UNEXPECTED);',
+    'function t() { throw Components.Exception("", Cr.NS_ERROR_NO_CONTENT); }',
+    // We don't handle combined values, regular no-throw-literal catches them
+    'throw Components.results.NS_ERROR_UNEXPECTED + "whoops";',
+  ],
+  invalid: [
+    invalidCode(
+      "throw Cr.NS_ERROR_NO_INTERFACE;",
+      'throw Components.Exception("", Cr.NS_ERROR_NO_INTERFACE);',
+      "bareCR"
+    ),
+    invalidCode(
+      "throw Components.results.NS_ERROR_ABORT;",
+      'throw Components.Exception("", Components.results.NS_ERROR_ABORT);',
+      "bareComponentsResults"
+    ),
+    invalidCode(
+      "function t() { throw Cr.NS_ERROR_NULL_POINTER; }",
+      'function t() { throw Components.Exception("", Cr.NS_ERROR_NULL_POINTER); }',
+      "bareCR"
+    ),
+  ],
+});