Bug 1605995 - Add linting rule against relative paths in DevTools require r=Standard8
authorJulian Descottes <jdescottes@mozilla.com>
Mon, 06 Jan 2020 12:54:39 +0000
changeset 508903 82bca3b583c8a6bb443b78cf6d9ec654768c7bec
parent 508902 9664ea28364dff5316d809ec69aec0d3d1609147
child 508904 564b928b0203ccbf5f9566f45bbb001f93e5dc7a
push id104289
push userjdescottes@mozilla.com
push dateMon, 06 Jan 2020 12:55:19 +0000
treeherderautoland@82bca3b583c8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersStandard8
bugs1605995
milestone73.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 1605995 - Add linting rule against relative paths in DevTools require r=Standard8 Depends on D58243 Differential Revision: https://phabricator.services.mozilla.com/D58253
devtools/.eslintrc.js
tools/lint/eslint/eslint-plugin-mozilla/lib/helpers.js
tools/lint/eslint/eslint-plugin-mozilla/lib/index.js
tools/lint/eslint/eslint-plugin-mozilla/lib/rules/reject-relative-requires.js
tools/lint/eslint/eslint-plugin-mozilla/lib/rules/reject-some-requires.js
tools/lint/eslint/eslint-plugin-mozilla/tests/reject-relative-requires.js
tools/lint/eslint/eslint-plugin-mozilla/tests/reject-some-requires.js
--- a/devtools/.eslintrc.js
+++ b/devtools/.eslintrc.js
@@ -128,17 +128,33 @@ module.exports = {
       "startup/aboutdebugging-registration.js",
       "startup/aboutdevtools/aboutdevtools-registration.js",
       "startup/aboutdevtoolstoolbox-registration.js",
       "startup/devtools-startup.js",
     ],
     "rules": {
       "mozilla/no-define-cc-etc": "off",
     }
-  }, ],
+  }, {
+    // All DevTools files should avoid relative paths.
+    "files": [
+      "**"
+    ],
+    "excludedFiles": [
+      // Debugger modules have a custom bundling logic which relies on relative
+      // paths.
+      "client/debugger/**",
+      // `client/shared/build` contains node helpers to build the debugger and
+      // not devtools modules.
+      "client/shared/build/**",
+    ],
+    "rules": {
+      "mozilla/reject-relative-requires": "error",
+    }
+  }],
   "rules": {
     // These are the rules that have been configured so far to match the
     // devtools coding style.
 
     // Rules from the mozilla plugin
     "mozilla/no-aArgs": "error",
     // See bug 1224289.
     "mozilla/reject-importGlobalProperties": ["error", "everything"],
--- a/tools/lint/eslint/eslint-plugin-mozilla/lib/helpers.js
+++ b/tools/lint/eslint/eslint-plugin-mozilla/lib/helpers.js
@@ -805,9 +805,33 @@ module.exports = {
     return require("./environments/saved-globals.json").environments[
       environment
     ];
   },
 
   getSavedRuleData(rule) {
     return require("./rules/saved-rules-data.json").rulesData[rule];
   },
+
+  /**
+   * Extract the path of require (and require-like) helpers used in DevTools.
+   */
+  getDevToolsRequirePath(node) {
+    if (
+      node.callee.type == "Identifier" &&
+      node.callee.name == "require" &&
+      node.arguments.length == 1 &&
+      node.arguments[0].type == "Literal"
+    ) {
+      return node.arguments[0].value;
+    } else if (
+      node.callee.type == "MemberExpression" &&
+      node.callee.property.type == "Identifier" &&
+      (node.callee.property.name == "lazyRequireGetter" ||
+        node.callee.property.name == "lazyImporter") &&
+      node.arguments.length >= 3 &&
+      node.arguments[2].type == "Literal"
+    ) {
+      return node.arguments[2].value;
+    }
+    return null;
+  },
 };
--- a/tools/lint/eslint/eslint-plugin-mozilla/lib/index.js
+++ b/tools/lint/eslint/eslint-plugin-mozilla/lib/index.js
@@ -46,16 +46,17 @@ 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-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"),
     "prefer-boolean-length-check": require("../lib/rules/prefer-boolean-length-check"),
     "reject-importGlobalProperties": require("../lib/rules/reject-importGlobalProperties"),
+    "reject-relative-requires": require("../lib/rules/reject-relative-requires"),
     "reject-some-requires": require("../lib/rules/reject-some-requires"),
     "rejects-requires-await": require("../lib/rules/rejects-requires-await"),
     "use-cc-etc": require("../lib/rules/use-cc-etc"),
     "use-chromeutils-generateqi": require("../lib/rules/use-chromeutils-generateqi"),
     "use-chromeutils-import": require("../lib/rules/use-chromeutils-import"),
     "use-default-preference-values": require("../lib/rules/use-default-preference-values"),
     "use-ownerGlobal": require("../lib/rules/use-ownerGlobal"),
     "use-includes-instead-of-indexOf": require("../lib/rules/use-includes-instead-of-indexOf"),
new file mode 100644
--- /dev/null
+++ b/tools/lint/eslint/eslint-plugin-mozilla/lib/rules/reject-relative-requires.js
@@ -0,0 +1,34 @@
+/**
+ * @fileoverview Reject some uses of require.
+ *
+ * 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
+  //  --------------------------------------------------------------------------
+
+  const isRelativePath = function(path) {
+    return path.startsWith("./") || path.startsWith("../");
+  };
+
+  return {
+    CallExpression(node) {
+      const path = helpers.getDevToolsRequirePath(node);
+      if (path && isRelativePath(path)) {
+        context.report(node, "relative paths are not allowed with require()");
+      }
+    },
+  };
+};
--- a/tools/lint/eslint/eslint-plugin-mozilla/lib/rules/reject-some-requires.js
+++ b/tools/lint/eslint/eslint-plugin-mozilla/lib/rules/reject-some-requires.js
@@ -7,46 +7,29 @@
  */
 
 "use strict";
 
 // -----------------------------------------------------------------------------
 // Rule Definition
 // -----------------------------------------------------------------------------
 
+var helpers = require("../helpers");
+
 module.exports = function(context) {
   // ---------------------------------------------------------------------------
   // Public
   //  --------------------------------------------------------------------------
 
   if (typeof context.options[0] !== "string") {
     throw new Error("reject-some-requires expects a regexp");
   }
   const RX = new RegExp(context.options[0]);
 
-  const checkPath = function(node, path) {
-    if (RX.test(path)) {
-      context.report(node, `require(${path}) is not allowed`);
-    }
-  };
-
   return {
     CallExpression(node) {
-      if (
-        node.callee.type == "Identifier" &&
-        node.callee.name == "require" &&
-        node.arguments.length == 1 &&
-        node.arguments[0].type == "Literal"
-      ) {
-        checkPath(node, node.arguments[0].value);
-      } else if (
-        node.callee.type == "MemberExpression" &&
-        node.callee.property.type == "Identifier" &&
-        (node.callee.property.name == "lazyRequireGetter" ||
-          node.callee.property.name == "lazyImporter") &&
-        node.arguments.length >= 3 &&
-        node.arguments[2].type == "Literal"
-      ) {
-        checkPath(node, node.arguments[2].value);
+      const path = helpers.getDevToolsRequirePath(node);
+      if (path && RX.test(path)) {
+        context.report(node, `require(${path}) is not allowed`);
       }
     },
   };
 };
new file mode 100644
--- /dev/null
+++ b/tools/lint/eslint/eslint-plugin-mozilla/tests/reject-relative-requires.js
@@ -0,0 +1,58 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// ------------------------------------------------------------------------------
+// Requirements
+// ------------------------------------------------------------------------------
+
+var rule = require("../lib/rules/reject-relative-requires");
+var RuleTester = require("eslint").RuleTester;
+
+const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 8 } });
+
+// ------------------------------------------------------------------------------
+// Tests
+// ------------------------------------------------------------------------------
+
+function invalidError() {
+  let message = "relative paths are not allowed with require()";
+  return [{ message, type: "CallExpression" }];
+}
+
+ruleTester.run("reject-relative-requires", rule, {
+  valid: [
+    'require("devtools/absolute/path")',
+    'require("resource://gre/modules/SomeModule.jsm")',
+    'loader.lazyRequireGetter(this, "path", "devtools/absolute/path", true)',
+    'loader.lazyRequireGetter(this, "Path", "devtools/absolute/path")',
+  ],
+  invalid: [
+    {
+      code: 'require("./relative/path")',
+      errors: invalidError(),
+    },
+    {
+      code: 'require("../parent/folder/path")',
+      errors: invalidError(),
+    },
+    {
+      code: 'loader.lazyRequireGetter(this, "path", "./relative/path", true)',
+      errors: invalidError(),
+    },
+    {
+      code:
+        'loader.lazyRequireGetter(this, "path", "../parent/folder/path", true)',
+      errors: invalidError(),
+    },
+    {
+      code: 'loader.lazyRequireGetter(this, "path", "./relative/path")',
+      errors: invalidError(),
+    },
+    {
+      code: 'loader.lazyRequireGetter(this, "path", "../parent/folder/path")',
+      errors: invalidError(),
+    },
+  ],
+});
new file mode 100644
--- /dev/null
+++ b/tools/lint/eslint/eslint-plugin-mozilla/tests/reject-some-requires.js
@@ -0,0 +1,49 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// ------------------------------------------------------------------------------
+// Requirements
+// ------------------------------------------------------------------------------
+
+var rule = require("../lib/rules/reject-some-requires");
+var RuleTester = require("eslint").RuleTester;
+
+const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 8 } });
+
+// ------------------------------------------------------------------------------
+// Tests
+// ------------------------------------------------------------------------------
+
+function requirePathError(path) {
+  const message = `require(${path}) is not allowed`;
+  return [{ message, type: "CallExpression" }];
+}
+
+const DEVTOOLS_FORBIDDEN_PATH = "^(resource://)?devtools/forbidden";
+
+ruleTester.run("reject-some-requires", rule, {
+  valid: [
+    {
+      code: 'require("devtools/not-forbidden/path")',
+      options: [DEVTOOLS_FORBIDDEN_PATH],
+    },
+    {
+      code: 'require("resource://devtools/not-forbidden/path")',
+      options: [DEVTOOLS_FORBIDDEN_PATH],
+    },
+  ],
+  invalid: [
+    {
+      code: 'require("devtools/forbidden/path")',
+      errors: requirePathError("devtools/forbidden/path"),
+      options: [DEVTOOLS_FORBIDDEN_PATH],
+    },
+    {
+      code: 'require("resource://devtools/forbidden/path")',
+      errors: requirePathError("resource://devtools/forbidden/path"),
+      options: [DEVTOOLS_FORBIDDEN_PATH],
+    },
+  ],
+});