From 3b345a689e6a720f0a5dd5057863278ab2020690 Mon Sep 17 00:00:00 2001 From: Julian Descottes Date: Mon, 6 Jan 2020 12:54:39 +0000 Subject: [PATCH] Bug 1605995 - Add linting rule against relative paths in DevTools require r=Standard8 Depends on D58243 Differential Revision: https://phabricator.services.mozilla.com/D58253 --HG-- extra : moz-landing-system : lando --- devtools/.eslintrc.js | 18 +++++- .../eslint-plugin-mozilla/lib/helpers.js | 24 ++++++++ .../eslint/eslint-plugin-mozilla/lib/index.js | 1 + .../lib/rules/reject-relative-requires.js | 34 +++++++++++ .../lib/rules/reject-some-requires.js | 27 ++------- .../tests/reject-relative-requires.js | 58 +++++++++++++++++++ .../tests/reject-some-requires.js | 49 ++++++++++++++++ 7 files changed, 188 insertions(+), 23 deletions(-) create mode 100644 tools/lint/eslint/eslint-plugin-mozilla/lib/rules/reject-relative-requires.js create mode 100644 tools/lint/eslint/eslint-plugin-mozilla/tests/reject-relative-requires.js create mode 100644 tools/lint/eslint/eslint-plugin-mozilla/tests/reject-some-requires.js diff --git a/devtools/.eslintrc.js b/devtools/.eslintrc.js index 9c89af1ea5bc..f565fd09b89b 100644 --- a/devtools/.eslintrc.js +++ b/devtools/.eslintrc.js @@ -133,7 +133,23 @@ module.exports = { "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. diff --git a/tools/lint/eslint/eslint-plugin-mozilla/lib/helpers.js b/tools/lint/eslint/eslint-plugin-mozilla/lib/helpers.js index a656aa3f8963..281d86e81c63 100644 --- a/tools/lint/eslint/eslint-plugin-mozilla/lib/helpers.js +++ b/tools/lint/eslint/eslint-plugin-mozilla/lib/helpers.js @@ -810,4 +810,28 @@ module.exports = { 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; + }, }; diff --git a/tools/lint/eslint/eslint-plugin-mozilla/lib/index.js b/tools/lint/eslint/eslint-plugin-mozilla/lib/index.js index 9d297db9b693..326ce56c9d66 100644 --- a/tools/lint/eslint/eslint-plugin-mozilla/lib/index.js +++ b/tools/lint/eslint/eslint-plugin-mozilla/lib/index.js @@ -51,6 +51,7 @@ module.exports = { "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"), diff --git a/tools/lint/eslint/eslint-plugin-mozilla/lib/rules/reject-relative-requires.js b/tools/lint/eslint/eslint-plugin-mozilla/lib/rules/reject-relative-requires.js new file mode 100644 index 000000000000..eb100b13fa9d --- /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()"); + } + }, + }; +}; diff --git 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 index 5dff5da2d5d1..ab310692930c 100644 --- 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 @@ -12,6 +12,8 @@ // Rule Definition // ----------------------------------------------------------------------------- +var helpers = require("../helpers"); + module.exports = function(context) { // --------------------------------------------------------------------------- // Public @@ -22,30 +24,11 @@ module.exports = function(context) { } 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`); } }, }; diff --git a/tools/lint/eslint/eslint-plugin-mozilla/tests/reject-relative-requires.js b/tools/lint/eslint/eslint-plugin-mozilla/tests/reject-relative-requires.js new file mode 100644 index 000000000000..19f30559eca7 --- /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(), + }, + ], +}); diff --git a/tools/lint/eslint/eslint-plugin-mozilla/tests/reject-some-requires.js b/tools/lint/eslint/eslint-plugin-mozilla/tests/reject-some-requires.js new file mode 100644 index 000000000000..b56dd2cf965e --- /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], + }, + ], +});