Bug 1392098 - Add an ESLint rule to avoid unnecessary run_test() functions. r?Mossop
MozReview-Commit-ID: pbXjamX4bk
--- a/tools/lint/docs/linters/eslint-plugin-mozilla.rst
+++ b/tools/lint/docs/linters/eslint-plugin-mozilla.rst
@@ -208,16 +208,23 @@ Reject common XPCOM methods called with
This option can be autofixed (``--fix``).
no-useless-removeEventListener
------------------------------
Reject calls to removeEventListener where {once: true} could be used instead.
+no-useless-run-test
+-------------------
+
+Designed for xpcshell-tests. Rejects definitions of ``run_test()`` where the
+function only contains a single call to ``run_next_test()``. xpcshell's head.js
+already defines a utility function so there is no need for duplication.
+
reject-importGlobalProperties
-----------------------------
Rejects calls to ``Cu.importGlobalProperties``. Use of this function is
undesirable in some parts of the tree.
reject-some-requires
--- a/tools/lint/eslint/eslint-plugin-mozilla/lib/index.js
+++ b/tools/lint/eslint/eslint-plugin-mozilla/lib/index.js
@@ -47,16 +47,18 @@ module.exports = {
"no-cpows-in-tests": require("../lib/rules/no-cpows-in-tests"),
"no-single-arg-cu-import": require("../lib/rules/no-single-arg-cu-import"),
"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"),
"reject-importGlobalProperties":
require("../lib/rules/reject-importGlobalProperties"),
"reject-some-requires": require("../lib/rules/reject-some-requires"),
"use-default-preference-values":
require("../lib/rules/use-default-preference-values"),
"use-ownerGlobal": require("../lib/rules/use-ownerGlobal"),
"var-only-at-top-level": require("../lib/rules/var-only-at-top-level")
},
@@ -72,16 +74,17 @@ module.exports = {
"mark-test-function-used": "off",
"no-aArgs": "off",
"no-arbitrary-setTimeout": "off",
"no-cpows-in-tests": "off",
"no-single-arg-cu-import": "off",
"no-import-into-var-and-global": "off",
"no-task": "off",
"no-useless-parameters": "off",
+ "no-useless-run-test": "off",
"no-useless-removeEventListener": "off",
"reject-importGlobalProperties": "off",
"reject-some-requires": "off",
"use-default-preference-values": "off",
"use-ownerGlobal": "off",
"var-only-at-top-level": "off"
}
};
new file mode 100644
--- /dev/null
+++ b/tools/lint/eslint/eslint-plugin-mozilla/lib/rules/no-useless-run-test.js
@@ -0,0 +1,68 @@
+/**
+ * @fileoverview Reject run_test() definitions where they aren't necessary.
+ *
+ * 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 = function(context) {
+
+ // ---------------------------------------------------------------------------
+ // Public
+ // --------------------------------------------------------------------------
+
+ return {
+ "Program > FunctionDeclaration": function(node) {
+ if (node.id.name === "run_test" &&
+ node.body.type === "BlockStatement" &&
+ node.body.body.length === 1 &&
+ node.body.body[0].type === "ExpressionStatement" &&
+ node.body.body[0].expression.type === "CallExpression" &&
+ node.body.body[0].expression.callee.name === "run_next_test") {
+ context.report({
+ node,
+ fix: fixer => {
+ let sourceCode = context.getSourceCode();
+ let startNode;
+ if (sourceCode.getCommentsBefore) {
+ // ESLint 4 has getCommentsBefore.
+ startNode = sourceCode.getCommentsBefore(node);
+ } else if (node && node.body && node.leadingComments) {
+ // This is for ESLint 3.
+ startNode = node.leadingComments;
+ }
+
+ // If we have comments, we want the start node to be the comments,
+ // rather than the token before the comments, so that we don't
+ // remove the comments - for run_test, these are likely to be useful
+ // information about the test.
+ if (startNode && startNode.length) {
+ startNode = startNode[startNode.length - 1];
+ } else {
+ startNode = sourceCode.getTokenBefore(node);
+ }
+
+ return fixer.removeRange([
+ // If there's no startNode, we fall back to zero, i.e. start of
+ // file.
+ startNode ? (startNode.end + 1) : 0,
+ // We know the function is a block and it'll end with }. Normally
+ // there's a new line after that, so just advance past it. This
+ // may be slightly not dodgy in some cases, but covers the existing
+ // cases.
+ node.end + 1
+ ]);
+ },
+ message: "Useless run_test function - only contains run_next_test; whole function can be removed"
+ });
+ }
+ }
+ };
+};
--- a/tools/lint/eslint/eslint-plugin-mozilla/package-lock.json
+++ b/tools/lint/eslint/eslint-plugin-mozilla/package-lock.json
@@ -1,11 +1,11 @@
{
"name": "eslint-plugin-mozilla",
- "version": "0.4.1",
+ "version": "0.4.3",
"lockfileVersion": 1,
"requires": true,
"dependencies": {
"acorn": {
"version": "5.1.1",
"resolved": "https://registry.npmjs.org/acorn/-/acorn-5.1.1.tgz",
"integrity": "sha512-vOk6uEMctu0vQrvuSqFdJyqj1Q0S5VTDL79qtjo+DhRr+1mmaD+tluFSCZqhvi/JUhXSzoZN2BhtstaPEeE8cw=="
},
--- a/tools/lint/eslint/eslint-plugin-mozilla/package.json
+++ b/tools/lint/eslint/eslint-plugin-mozilla/package.json
@@ -1,11 +1,11 @@
{
"name": "eslint-plugin-mozilla",
- "version": "0.4.2",
+ "version": "0.4.3",
"description": "A collection of rules that help enforce JavaScript coding standard in the Mozilla project.",
"keywords": [
"eslint",
"eslintplugin",
"eslint-plugin",
"mozilla",
"firefox"
],
new file mode 100644
--- /dev/null
+++ b/tools/lint/eslint/eslint-plugin-mozilla/tests/no-useless-run-test.js
@@ -0,0 +1,114 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// ------------------------------------------------------------------------------
+// Requirements
+// ------------------------------------------------------------------------------
+
+var rule = require("../lib/rules/no-useless-run-test");
+var RuleTester = require("eslint/lib/testers/rule-tester");
+
+const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 6 } });
+
+// ------------------------------------------------------------------------------
+// Tests
+// ------------------------------------------------------------------------------
+
+function invalidCode(code, output) {
+ let message =
+ "Useless run_test function - only contains run_next_test; whole function can be removed";
+ return {code, output, errors: [{message, type: "FunctionDeclaration"}]};
+}
+
+ruleTester.run("no-useless-run-test", rule, {
+ valid: [
+ "function run_test() {}",
+ "function run_test() {let args = 1; run_next_test();}",
+ "function run_test() {fakeCall(); run_next_test();}"
+ ],
+ invalid: [
+ // Single-line case.
+ invalidCode(
+ "function run_test() { run_next_test(); }",
+ ""),
+ // Multiple-line cases
+ invalidCode(`
+function run_test() {
+ run_next_test();
+}`,
+ ``),
+ invalidCode(`
+foo();
+function run_test() {
+ run_next_test();
+}
+`,
+ `
+foo();
+`),
+ invalidCode(`
+foo();
+function run_test() {
+ run_next_test();
+}
+bar();
+`,
+ `
+foo();
+bar();
+`),
+ invalidCode(`
+foo();
+
+function run_test() {
+ run_next_test();
+}
+
+bar();`,
+ `
+foo();
+
+bar();`),
+ invalidCode(`
+foo();
+
+function run_test() {
+ run_next_test();
+}
+
+// A comment
+bar();
+`,
+ `
+foo();
+
+// A comment
+bar();
+`),
+ invalidCode(`
+foo();
+
+/**
+ * A useful comment.
+ */
+function run_test() {
+ run_next_test();
+}
+
+// A comment
+bar();
+`,
+ `
+foo();
+
+/**
+ * A useful comment.
+ */
+
+// A comment
+bar();
+`)
+ ]
+});