Bug 1801092 - Add an ESLint rule to disllow use of Cu.reportError with a single argument. r=Gijs,cmkm

This enforces the bulk of the cases for Cu.reportError. A follow-up will extend this to the
two argument form.

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

.eslintrc.js

@@ -204,6 +204,48 @@ module.exports = {
         "no-shadow": "warn",
       },
     },
+    {
+      // Bug 877389 - Gradually migrate from Cu.reportError to console.error.
+      // Report as warnings where it is not yet passing.
+      files: [
+        "browser/actors/**",
+        "browser/base/content/**",
+        "browser/components/Browser*.*",
+        "browser/components/**",
+        "browser/extensions/formautofill/**",
+        "browser/extensions/report-site-issue/**",
+        "browser/extensions/search-detection/**",
+        "browser/modules/**",
+        "browser/themes/BuiltInThemes.sys.mjs",
+        "devtools/**",
+        "dom/base/DOMRequestHelper.*",
+        "dom/console/ConsoleAPIStorage.*",
+        "dom/push/PushComponents.sys.mjs",
+        "dom/push/PushServiceHttp2.sys.mjs",
+        "dom/push/test/mockpushserviceparent.js",
+        "dom/push/test/xpcshell/head.js",
+        "dom/serviceworkers/test/browser_download.js",
+        "dom/system/NetworkGeolocationProvider.*",
+        "dom/tests/browser/browser_hasbeforeunload.js",
+        "editor/AsyncSpellCheckTestHelper.*",
+        "intl/locale/LangPackMatcher.sys.mjs",
+        "js/xpconnect/loader/XPCOMUtils.sys.mjs",
+        "layout/base/tests/chrome/**",
+        "mobile/android/**",
+        "netwerk/dns/PublicSuffixList.*",
+        "netwerk/url-classifier/UrlClassifierExceptionListService.*",
+        "remote/cdp/Error.sys.mjs",
+        "remote/cdp/domains/Domain.sys.mjs",
+        "remote/shared/webdriver/Errors.sys.mjs",
+        "security/manager/ssl/tests/mochitest/mixedcontent/test_bug383369.html",
+        "testing/**",
+        "toolkit/**",
+        "uriloader/exthandler/**",
+      ],
+      rules: {
+        "mozilla/no-cu-reportError": "off",
+      },
+    },
     {
       files: ["layout/**"],
       rules: {

browser/components/newtab/.eslintrc.js

@@ -126,6 +126,10 @@ module.exports = {
     "react/no-unknown-property": 2,
     "react/require-render-return": 2,
 
+    // Bug 877389 - Gradually migrate from Cu.reportError to console.error.
+    // Report as warnings where it is not yet passing.
+    "mozilla/no-cu-reportError": "off",
+
     "accessor-pairs": [2, { setWithoutGet: true, getWithoutSet: false }],
     "array-callback-return": 2,
     "block-scoped-var": 2,

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

@@ -40,6 +40,7 @@ The plugin implements the following rules:
    eslint-plugin-mozilla/no-addtask-setup
    eslint-plugin-mozilla/no-arbitrary-setTimeout
    eslint-plugin-mozilla/no-compare-against-boolean-literals
+   eslint-plugin-mozilla/no-cu-reportError
    eslint-plugin-mozilla/no-define-cc-etc
    eslint-plugin-mozilla/no-throw-cr-literal
    eslint-plugin-mozilla/no-useless-parameters

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

@@ -0,0 +1,23 @@
+no-cu-reportError
+=================
+
+Disallows Cu.reportError. This has been deprecated and should be replaced by
+console.error.
+
+Examples of incorrect code for this rule:
+-----------------------------------------
+
+.. code-block:: js
+
+    Cu.reportError("message");
+    Cu.reportError("message", stack);
+
+Examples of correct code for this rule:
+---------------------------------------
+
+.. code-block:: js
+
+    console.error("message");
+    let error = new Error("message");
+    error.stack = stack;
+    console.error(error);

toolkit/modules/Log.sys.mjs

@@ -11,6 +11,8 @@ const INTERNAL_FIELDS = new Set(["_level", "_message", "_time", "_namespace"]);
  */
 function dumpError(text) {
   dump(text + "\n");
+  // TODO: Bug 1801091 - Figure out how to replace this.
+  // eslint-disable-next-line mozilla/no-cu-reportError
   Cu.reportError(text);
 }
 
@@ -716,6 +718,8 @@ class ConsoleAppender extends Appender {
     if (message) {
       let m = this._formatter.format(message);
       if (message.level > Log.Level.Warn) {
+        // TODO: Bug 1801091 - Figure out how to replace this.
+        // eslint-disable-next-line mozilla/no-cu-reportError
         Cu.reportError(m);
         return;
       }

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

@@ -152,6 +152,7 @@ module.exports = {
     "mozilla/import-browser-window-globals": "error",
     "mozilla/import-globals": "error",
     "mozilla/no-compare-against-boolean-literals": "error",
+    "mozilla/no-cu-reportError": "error",
     "mozilla/no-define-cc-etc": "error",
     "mozilla/no-throw-cr-literal": "error",
     "mozilla/no-useless-parameters": "error",

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

@@ -53,6 +53,7 @@ module.exports = {
     "no-addtask-setup": require("../lib/rules/no-addtask-setup"),
     "no-arbitrary-setTimeout": require("../lib/rules/no-arbitrary-setTimeout"),
     "no-compare-against-boolean-literals": require("../lib/rules/no-compare-against-boolean-literals"),
+    "no-cu-reportError": require("../lib/rules/no-cu-reportError"),
     "no-define-cc-etc": require("../lib/rules/no-define-cc-etc"),
     "no-throw-cr-literal": require("../lib/rules/no-throw-cr-literal"),
     "no-useless-parameters": require("../lib/rules/no-useless-parameters"),

tools/lint/eslint/eslint-plugin-mozilla/lib/rules/no-cu-reportError.js

@@ -0,0 +1,73 @@
+/**
+ * @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";
+
+function isCuReportError(node) {
+  return (
+    node.type == "MemberExpression" &&
+    node.object.type == "Identifier" &&
+    node.object.name == "Cu" &&
+    node.property.type == "Identifier" &&
+    node.property.name == "reportError"
+  );
+}
+
+module.exports = {
+  meta: {
+    docs: {
+      url:
+        "https://firefox-source-docs.mozilla.org/code-quality/lint/linters/eslint-plugin-mozilla/no-cu-reportError.html",
+    },
+    fixable: "code",
+    messages: {
+      useConsoleError: "Please use console.error instead of Cu.reportError",
+    },
+    type: "suggestion",
+  },
+
+  create(context) {
+    return {
+      CallExpression(node) {
+        let checkNode;
+        if (
+          node.arguments.length >= 1 &&
+          node.arguments[0].type == "MemberExpression"
+        ) {
+          // Handles cases of `.foo(Cu.reportError)`.
+          checkNode = node.arguments[0];
+        } else {
+          // Handles cases of `Cu.reportError()`.
+          checkNode = node.callee;
+        }
+        if (!isCuReportError(checkNode)) {
+          return;
+        }
+
+        if (checkNode == node.callee && node.arguments.length > 1) {
+          // TODO: Bug 1802347 For initial landing, we allow the two
+          // argument form of Cu.reportError as the second argument is a stack
+          // argument which is more complicated to deal with.
+          return;
+        }
+
+        context.report({
+          node,
+          fix: fixer => {
+            return [
+              fixer.replaceText(checkNode.object, "console"),
+              fixer.replaceText(checkNode.property, "error"),
+            ];
+          },
+          messageId: "useConsoleError",
+        });
+      },
+    };
+  },
+};

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

@@ -1,6 +1,6 @@
 {
   "name": "eslint-plugin-mozilla",
-  "version": "3.0.0",
+  "version": "3.0.1",
   "lockfileVersion": 2,
   "requires": true,
   "packages": {

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

@@ -1,6 +1,6 @@
 {
   "name": "eslint-plugin-mozilla",
-  "version": "3.0.0",
+  "version": "3.0.1",
   "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-cu-reportError.js

@@ -0,0 +1,48 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// ------------------------------------------------------------------------------
+// Requirements
+// ------------------------------------------------------------------------------
+
+var rule = require("../lib/rules/no-cu-reportError");
+var RuleTester = require("eslint").RuleTester;
+
+const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 9 } });
+
+// ------------------------------------------------------------------------------
+// Tests
+// ------------------------------------------------------------------------------
+
+function callError() {
+  return [{ messageId: "useConsoleError", type: "CallExpression" }];
+}
+
+ruleTester.run("no-cu-reportError", rule, {
+  valid: [
+    "console.error('foo')",
+    "Cu.cloneInto({}, {})",
+    "foo().catch(console.error)",
+    // TODO: Bug 1802347 - this should be treated as an error as well.
+    "Cu.reportError('foo', stack)",
+  ],
+  invalid: [
+    {
+      code: "Cu.reportError('foo')",
+      output: "console.error('foo')",
+      errors: callError(),
+    },
+    {
+      code: "Cu.reportError(bar)",
+      output: "console.error(bar)",
+      errors: callError(),
+    },
+    {
+      code: "foo().catch(Cu.reportError)",
+      output: "foo().catch(console.error)",
+      errors: callError(),
+    },
+  ],
+});