diff --git a/README.md b/README.md index 5b31a4c7..bc016031 100644 --- a/README.md +++ b/README.md @@ -53,6 +53,7 @@ For more details on how to extend your configuration from a plugin configuration | [no-hooks-from-ancestor-modules](docs/rules/no-hooks-from-ancestor-modules.md) | disallow the use of hooks from ancestor modules | ✅ | | | | [no-identical-names](docs/rules/no-identical-names.md) | disallow identical test and module names | ✅ | | | | [no-init](docs/rules/no-init.md) | disallow use of QUnit.init | ✅ | | | +| [no-invalid-names](docs/rules/no-invalid-names.md) | disallow invalid and missing test names | | 🔧 | | | [no-jsdump](docs/rules/no-jsdump.md) | disallow use of QUnit.jsDump | ✅ | | | | [no-loose-assertions](docs/rules/no-loose-assertions.md) | disallow the use of assert.equal/assert.ok/assert.notEqual/assert.notOk | | | | | [no-negated-ok](docs/rules/no-negated-ok.md) | disallow negation in assert.ok/assert.notOk | ✅ | 🔧 | | diff --git a/docs/rules/no-invalid-names.md b/docs/rules/no-invalid-names.md new file mode 100644 index 00000000..273484a0 --- /dev/null +++ b/docs/rules/no-invalid-names.md @@ -0,0 +1,65 @@ +# Disallow invalid and missing test names (`qunit/no-invalid-names`) + +🔧 This rule is automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix). + + + +QUnit tests can be difficult to debug without useful module and test names. The purpose +of this rule is to ensure that module and test names are present and valid. + +## Rule Details + +The following patterns are considered warnings: + +```js +// Missing names +module(function () {}); +test(function () {}); + +// Empty or space-only names +module("", function () {}); +test("", function () {}); +module(" ", function () {}); +test(" ", function () {}); + +// Leading and trailing spaces +module(' Foo Bar unit ', function () {}); +test(' it does foo ', function () {}); + +// Non-string names +module(["foo"], function () {}); +test(["foo"], function () {}); +module(1, function () {}); +test(1, function () {}); + +// Names starting or ending with QUnit delimiters (>, :) +module('>Foo Bar unit', function () {}); +test('>it does foo', function () {}); +module('Foo Bar unit>', function () {}); +test('it does foo>', function () {}); +module(':Foo Bar unit', function () {}); +test(':it does foo', function () {}); +module('Foo Bar unit:', function () {}); +test('it does foo:', function () {}); +``` + +The following patterns are not considered warnings: + +```js +// Valid strings +module("Foo Bar", function () {}); +test("Foo Bar", function () {}); + +// Templates are okay since those are strings +module(`Foo Bar ${foo}`, function () {}); +test(`Foo Bar ${foo}`, function () {}); + +// Can't check variables +module(foo, function () {}); +test(foo, function () {}); +``` + +## When Not to Use It + +This rule is mostly stylistic, but can cause problems in the case of QUnit delimiters +at the start and end of test names. diff --git a/lib/rules/no-invalid-names.js b/lib/rules/no-invalid-names.js new file mode 100644 index 00000000..efc98fa6 --- /dev/null +++ b/lib/rules/no-invalid-names.js @@ -0,0 +1,142 @@ +/** + * @fileoverview Disallow invalid and missing test names. + * @author Kevin Partington + */ +"use strict"; + +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +const utils = require("../utils"); + +//------------------------------------------------------------------------------ +// Rule Definition +//------------------------------------------------------------------------------ + +/** @type {import('eslint').Rule.RuleModule} */ +module.exports = { + meta: { + type: "suggestion", + docs: { + description: "disallow invalid and missing test names", + category: "Best Practices", + url: "https://github.com/platinumazure/eslint-plugin-qunit/blob/master/docs/rules/no-invalid-names.md" + }, + fixable: "code", + messages: { + moduleNameEmpty: "Module name is empty.", + moduleNameInvalidType: "Module name \"{{ name }}\" is invalid type: {{ type }}.", + moduleNameMissing: "Module name is missing.", + moduleNameOuterQUnitDelimiters: "Module name \"{{ name }}\" has leading and/or trailing QUnit delimiter: (> or :).", + moduleNameOuterSpaces: "Module name has leading and/or trailing spaces.", + testNameEmpty: "Test name is empty.", + testNameInvalidType: "Test name \"{{ name }}\" is invalid type: {{ type }}.", + testNameMissing: "Test name is missing.", + testNameOuterQUnitDelimiters: "Test name \"{{ name }}\" has leading and/or trailing QUnit delimiter (> or :).", + testNameOuterSpaces: "Test name has leading and/or trailing spaces." + }, + schema: [] + }, + + create: function (context) { + const sourceCode = context.getSourceCode(); + + const FUNCTION_TYPES = new Set(["FunctionExpression", "ArrowFunctionExpression"]); + const INVALID_NAME_AST_TYPES = new Set([ + "ArrayExpression", + "ObjectExpression", + "ThisExpression", + "UnaryExpression", + "UpdateExpression", + "BinaryExpression", + "AssignmentExpression", + "LogicalExpression" + ]); + const QUNIT_NAME_DELIMITERS = [">", ":"]; + + /** + * Check name for starting or ending with QUnit delimiters. + * @param {string} name The test or module name to check. + * @returns {boolean} True if the name starts or ends with a QUnit name delimiter, false otherwise. + */ + function nameHasOuterQUnitDelimiters(name) { + return QUNIT_NAME_DELIMITERS.some(delimiter => + name.startsWith(delimiter) || name.endsWith(delimiter) + ); + } + + /** + * Check the name argument of a module or test CallExpression. + * @param {ASTNode} firstArg The first argument of the test/module call. + * @param {"test"|"module"} objectType Whether this is a test or module call. + * @param {ASTNode} calleeForMissingName The callee, used as report location if the test/module name is missing. + * @returns {void} + */ + function checkNameArgument(firstArg, objectType, calleeForMissingName) { + if (!firstArg || FUNCTION_TYPES.has(firstArg.type)) { + context.report({ + node: calleeForMissingName, + messageId: `${objectType}NameMissing` + }); + } else if (INVALID_NAME_AST_TYPES.has(firstArg.type)) { + context.report({ + node: firstArg, + messageId: `${objectType}NameInvalidType`, + data: { + type: firstArg.type, + name: sourceCode.getText(firstArg) + } + }); + } else if (firstArg.type === "Literal") { + if (typeof firstArg.value !== "string") { + context.report({ + node: firstArg, + messageId: `${objectType}NameInvalidType`, + data: { + type: typeof firstArg.value, + name: sourceCode.getText(firstArg) + } + }); + } else if (firstArg.value.trim().length === 0) { + context.report({ + node: firstArg, + messageId: `${objectType}NameEmpty` + }); + } else if (firstArg.value.trim() !== firstArg.value) { + const trimmedValue = firstArg.value.trim(); + + const raw = firstArg.raw; + const startDelimiter = raw[0]; + const endDelimiter = raw[raw.length - 1]; + + context.report({ + node: firstArg, + messageId: `${objectType}NameOuterSpaces`, + fix: fixer => fixer.replaceText( + firstArg, + `${startDelimiter}${trimmedValue}${endDelimiter}` + ) + }); + } else if (nameHasOuterQUnitDelimiters(firstArg.value)) { + context.report({ + node: firstArg, + messageId: `${objectType}NameOuterQUnitDelimiters`, + data: { name: firstArg.value } + }); + } + } + } + + return { + "CallExpression": function (node) { + /* istanbul ignore else: Correctly does nothing */ + if (utils.isTest(node.callee)) { + checkNameArgument(node.arguments[0], "test", node.callee); + } else if (utils.isModule(node.callee)) { + checkNameArgument(node.arguments[0], "module", node.callee); + } + } + }; + } +}; diff --git a/tests/lib/rules/no-invalid-names.js b/tests/lib/rules/no-invalid-names.js new file mode 100644 index 00000000..4de125c6 --- /dev/null +++ b/tests/lib/rules/no-invalid-names.js @@ -0,0 +1,232 @@ +/** + * @fileoverview Disallow missing and invalid test/module names. + * @author Kevin Partington + */ +"use strict"; + +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +const rule = require("../../../lib/rules/no-invalid-names"), + RuleTester = require("eslint").RuleTester; + +//------------------------------------------------------------------------------ +// Helpers +//------------------------------------------------------------------------------ + +const TEST_FUNCTIONS = [ + "test", + "asyncTest", + "QUnit.test", + "QUnit.only" +]; + +const MODULE_FUNCTIONS = [ + "module", + "QUnit.module" +]; + +//------------------------------------------------------------------------------ +// Tests +//------------------------------------------------------------------------------ + +const ruleTester = new RuleTester(); + +ruleTester.run("no-invalid-names", rule, { + valid: [...TEST_FUNCTIONS, ...MODULE_FUNCTIONS].flatMap(callee => [ + `${callee}("simple valid name");`, + `${callee}("simple valid name", function () {});`, + + // Cannot check variables + `${callee}(name, function () {});` + ]), + + invalid: [[TEST_FUNCTIONS, "test"], [MODULE_FUNCTIONS, "module"]].flatMap( + ([callees, objType]) => callees.flatMap(callee => [ + { + code: `${callee}(function () {});`, + output: null, + errors: [{ messageId: `${objType}NameMissing` }] + }, + { + code: `${callee}(1, function () {});`, + output: null, + errors: [{ + messageId: `${objType}NameInvalidType`, + data: { + type: "number", + name: "1" + } + }] + }, + { + code: `${callee}(true, function () {});`, + output: null, + errors: [{ + messageId: `${objType}NameInvalidType`, + data: { + type: "boolean", + name: "true" + } + }] + }, + { + code: `${callee}(null, function () {});`, + output: null, + errors: [{ + messageId: `${objType}NameInvalidType`, + data: { + name: "null", + type: "object" + } + }] + }, + { + code: `${callee}(/regex/, function () {});`, + output: null, + errors: [{ + messageId: `${objType}NameInvalidType`, + data: { + type: "object", + name: "/regex/" + } + }] + }, + { + code: `${callee}([], function () {});`, + output: null, + errors: [{ + messageId: `${objType}NameInvalidType`, + data: { + type: "ArrayExpression", + name: "[]" + } + }] + }, + { + code: `${callee}({}, function () {});`, + output: null, + errors: [{ + messageId: `${objType}NameInvalidType`, + data: { + type: "ObjectExpression", + name: "{}" + } + }] + }, + { + code: `${callee}(this, function () {});`, + output: null, + errors: [{ + messageId: `${objType}NameInvalidType`, + data: { + type: "ThisExpression", + name: "this" + } + }] + }, + { + code: `${callee}(typeof foo, function () {});`, + output: null, + errors: [{ + messageId: `${objType}NameInvalidType`, + data: { + type: "UnaryExpression", + name: "typeof foo" + } + }] + }, + { + code: `${callee}(void foo, function () {});`, + output: null, + errors: [{ + messageId: `${objType}NameInvalidType`, + data: { + type: "UnaryExpression", + name: "void foo" + } + }] + }, + { + code: `${callee}(++foo, function () {});`, + output: null, + errors: [{ + messageId: `${objType}NameInvalidType`, + data: { + type: "UpdateExpression", + name: "++foo" + } + }] + }, + { + code: `${callee}(foo + bar, function () {});`, + output: null, + errors: [{ + messageId: `${objType}NameInvalidType`, + data: { + type: "BinaryExpression", + name: "foo + bar" + } + }] + }, + { + code: `${callee}(foo = "name", function () {});`, + output: null, + errors: [{ + messageId: `${objType}NameInvalidType`, + data: { + type: "AssignmentExpression", + name: "foo = \"name\"" + } + }] + }, + { + code: `${callee}(foo || "name", function () {});`, + output: null, + errors: [{ + messageId: `${objType}NameInvalidType`, + data: { + type: "LogicalExpression", + name: "foo || \"name\"" + } + }] + }, + { + code: `${callee}("", function () {});`, + output: null, + errors: [{ messageId: `${objType}NameEmpty` }] + }, + { + code: `${callee}(" \\t\\n ", function () {});`, + output: null, + errors: [{ messageId: `${objType}NameEmpty` }] + }, + { + code: `${callee}("\\t Leading and trailing space ", function () {});`, + output: `${callee}("Leading and trailing space", function () {});`, + errors: [{ messageId: `${objType}NameOuterSpaces` }] + }, + { + code: `${callee}("> Leading QUnit delimiter", function () {});`, + output: null, + errors: [{ messageId: `${objType}NameOuterQUnitDelimiters` }] + }, + { + code: `${callee}(": Leading QUnit delimiter", function () {});`, + output: null, + errors: [{ messageId: `${objType}NameOuterQUnitDelimiters` }] + }, + { + code: `${callee}("Trailing QUnit delimiter >", function () {});`, + output: null, + errors: [{ messageId: `${objType}NameOuterQUnitDelimiters` }] + }, + { + code: `${callee}("Trailing QUnit delimiter :", function () {});`, + output: null, + errors: [{ messageId: `${objType}NameOuterQUnitDelimiters` }] + } + ]) + ) +});