Bug 1461997 - Create a rule to prevent usages of Assert.throws/rejects without an 'expected' argument. r?Mossop
MozReview-Commit-ID: 979uJQUjybl
--- a/.eslintrc.js
+++ b/.eslintrc.js
@@ -43,10 +43,29 @@ module.exports = {
// XXX Bug 1436303. These directories are still being fixed, so turn off
// mozilla/no-cc-etc for now.
"files": [
"devtools/**"
],
"rules": {
"mozilla/no-define-cc-etc": "off",
}
+ }, {
+ // XXX Bug 1452706. These directories are still being fixed, so turn off
+ // mozilla/require-expected-throws-or-rejects for now.
+ "files": [
+ "browser/components/**",
+ "browser/extensions/formautofill/test/unit/test_storage_tombstones.js",
+ "browser/modules/test/browser/**",
+ "browser/tools/mozscreenshots/browser_boundingbox.js",
+ "devtools/client/inspector/extensions/test/head_devtools_inspector_sidebar.js",
+ "services/**",
+ "storage/test/unit/**",
+ "testing/marionette/test/unit/**",
+ "toolkit/components/**",
+ "toolkit/modules/tests/xpcshell/**",
+ "toolkit/mozapps/extensions/test/xpcshell/**"
+ ],
+ "rules": {
+ "mozilla/require-expected-throws-or-rejects": "off",
+ }
}]
};
--- a/tools/lint/eslint/eslint-plugin-mozilla/lib/configs/recommended.js
+++ b/tools/lint/eslint/eslint-plugin-mozilla/lib/configs/recommended.js
@@ -171,16 +171,17 @@ module.exports = {
"mozilla/avoid-removeChild": "error",
"mozilla/import-browser-window-globals": "error",
"mozilla/import-globals": "error",
"mozilla/no-compare-against-boolean-literals": "error",
"mozilla/no-define-cc-etc": "error",
"mozilla/no-import-into-var-and-global": "error",
"mozilla/no-useless-parameters": "error",
"mozilla/no-useless-removeEventListener": "error",
+ "mozilla/require-expected-throws-or-rejects": "error",
"mozilla/use-cc-etc": "error",
"mozilla/use-chromeutils-generateqi": "error",
"mozilla/use-chromeutils-import": "error",
"mozilla/use-default-preference-values": "error",
"mozilla/use-includes-instead-of-indexOf": "error",
"mozilla/use-ownerGlobal": "error",
"mozilla/use-services": "error",
--- a/tools/lint/eslint/eslint-plugin-mozilla/lib/index.js
+++ b/tools/lint/eslint/eslint-plugin-mozilla/lib/index.js
@@ -49,16 +49,18 @@ module.exports = {
"no-import-into-var-and-global":
require("../lib/rules/no-import-into-var-and-global.js"),
"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"),
+ "require-expected-throws-or-rejects":
+ require("../lib/rules/require-expected-throws-or-rejects"),
"reject-importGlobalProperties":
require("../lib/rules/reject-importGlobalProperties"),
"reject-some-requires": require("../lib/rules/reject-some-requires"),
"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"),
new file mode 100644
--- /dev/null
+++ b/tools/lint/eslint/eslint-plugin-mozilla/lib/rules/require-expected-throws-or-rejects.js
@@ -0,0 +1,70 @@
+/**
+ * @fileoverview Reject use of Cu.importGlobalProperties
+ *
+ * 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 checkArgs(context, node, name) {
+ if (node.arguments.length < 2) {
+ context.report({
+ node,
+ messageId: "needsTwoArguments",
+ data: {
+ name
+ }
+ });
+ return;
+ }
+
+ if (!("regex" in node.arguments[1]) &&
+ node.arguments[1].type !== "FunctionExpression" &&
+ node.arguments[1].type !== "ArrowFunctionExpression" &&
+ (node.arguments[1].type !== "Identifier" ||
+ node.arguments[1].name === "undefined")) {
+ context.report({
+ node,
+ messageId: "requireExpected",
+ data: {
+ name
+ }
+ });
+ }
+}
+
+module.exports = {
+ meta: {
+ messages: {
+ needsTwoArguments: "Assert.{{name}} should have at least two arguments (assert, expected).",
+ requireExpected: "Second argument to Assert.{{name}} should be a RegExp, function or object to compare the exception to."
+ }
+ },
+
+ create(context) {
+ return {
+ "CallExpression": function(node) {
+ if (node.callee.type === "MemberExpression") {
+ let memexp = node.callee;
+ if (memexp.object.type === "Identifier" &&
+ memexp.object.name === "Assert" &&
+ memexp.property.type === "Identifier" &&
+ memexp.property.name === "rejects") {
+ // We have ourselves an Assert.rejects.
+ checkArgs(context, node, "rejects");
+ }
+
+ if (memexp.object.type === "Identifier" &&
+ memexp.object.name === "Assert" &&
+ memexp.property.type === "Identifier" &&
+ memexp.property.name === "throws") {
+ // We have ourselves an Assert.throws.
+ checkArgs(context, node, "throws");
+ }
+ }
+ }
+ };
+ }
+};
new file mode 100644
--- /dev/null
+++ b/tools/lint/eslint/eslint-plugin-mozilla/tests/require-expected-throws-or-rejects.js
@@ -0,0 +1,62 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// ------------------------------------------------------------------------------
+// Requirements
+// ------------------------------------------------------------------------------
+
+var rule = require("../lib/rules/require-expected-throws-or-rejects");
+var RuleTester = require("eslint/lib/testers/rule-tester");
+
+const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 6 } });
+
+// ------------------------------------------------------------------------------
+// Tests
+// ------------------------------------------------------------------------------
+
+function invalidCode(code, messageId, name) {
+ return {code, errors: [{messageId, data: {name}}]};
+}
+
+ruleTester.run("no-useless-run-test", rule, {
+ valid: [
+ "Assert.throws(() => foo(), /assertion/);",
+ "Assert.throws(() => foo(), /assertion/, 'message');",
+ "Assert.throws(() => foo(), ex => {}, 'message');",
+ "Assert.throws(() => foo(), foo, 'message');",
+ "Assert.rejects(foo, /assertion/)",
+ "Assert.rejects(foo, /assertion/, 'msg')",
+ "Assert.rejects(foo, ex => {}, 'msg')",
+ "Assert.rejects(foo, foo, 'msg')"
+ ],
+ invalid: [
+ invalidCode("Assert.throws(() => foo());",
+ "needsTwoArguments", "throws"),
+
+ invalidCode("Assert.throws(() => foo(), 'message');",
+ "requireExpected", "throws"),
+
+ invalidCode("Assert.throws(() => foo(), 'invalid', 'message');",
+ "requireExpected", "throws"),
+
+ invalidCode("Assert.throws(() => foo(), null, 'message');",
+ "requireExpected", "throws"),
+
+ invalidCode("Assert.throws(() => foo(), undefined, 'message');",
+ "requireExpected", "throws"),
+
+ invalidCode("Assert.rejects(foo)",
+ "needsTwoArguments", "rejects"),
+
+ invalidCode("Assert.rejects(foo, 'msg')",
+ "requireExpected", "rejects"),
+
+ invalidCode("Assert.rejects(foo, 'invalid', 'msg')",
+ "requireExpected", "rejects"),
+
+ invalidCode("Assert.rejects(foo, undefined, 'msg')",
+ "requireExpected", "rejects")
+ ]
+});