diff --git a/docs/developer-guide/nodejs-api.md b/docs/developer-guide/nodejs-api.md index ec25f675a67..ee1a7a44970 100644 --- a/docs/developer-guide/nodejs-api.md +++ b/docs/developer-guide/nodejs-api.md @@ -886,7 +886,7 @@ In addition to the properties above, invalid test cases can also have the follow * `suggestions` (array): An array of objects with suggestion details to check. See [Testing Suggestions](#testing-suggestions) for details If a string is provided as an error instead of an object, the string is used to assert the `message` of the error. -* `output` (string, optional): Asserts the output that will be produced when using this rule for a single pass of autofixing (e.g. with the `--fix` command line flag). If this is `null`, asserts that none of the reported problems suggest autofixes. +* `output` (string, required if the rule fixes code): Asserts the output that will be produced when using this rule for a single pass of autofixing (e.g. with the `--fix` command line flag). If this is `null`, asserts that none of the reported problems suggest autofixes. Any additional properties of a test case will be passed directly to the linter as config options. For example, a test case can have a `parserOptions` property to configure parser behavior: diff --git a/lib/rule-tester/rule-tester.js b/lib/rule-tester/rule-tester.js index acfaf18325f..1c1737152c1 100644 --- a/lib/rule-tester/rule-tester.js +++ b/lib/rule-tester/rule-tester.js @@ -45,16 +45,20 @@ const path = require("path"), util = require("util"), lodash = require("lodash"), + Traverser = require("../../lib/shared/traverser"), { getRuleOptionsSchema, validate } = require("../shared/config-validator"), { Linter, SourceCodeFixer, interpolate } = require("../linter"); const ajv = require("../shared/ajv")({ strictDefaults: true }); +const espreePath = require.resolve("espree"); //------------------------------------------------------------------------------ // Typedefs //------------------------------------------------------------------------------ +/** @typedef {import("../shared/types").Parser} Parser */ + /** * A test case that is expected to pass lint. * @typedef {Object} ValidTestCase @@ -206,6 +210,70 @@ function sanitize(text) { ); } +/** + * Define `start`/`end` properties as throwing error. + * @param {string} objName Object name used for error messages. + * @param {ASTNode} node The node to define. + * @returns {void} + */ +function defineStartEndAsError(objName, node) { + Object.defineProperties(node, { + start: { + get() { + throw new Error(`Use ${objName}.range[0] instead of ${objName}.start`); + }, + configurable: true, + enumerable: false + }, + end: { + get() { + throw new Error(`Use ${objName}.range[1] instead of ${objName}.end`); + }, + configurable: true, + enumerable: false + } + }); +} + +/** + * Define `start`/`end` properties of all nodes of the given AST as throwing error. + * @param {ASTNode} ast The root node to errorize `start`/`end` properties. + * @param {Object} [visitorKeys] Visitor keys to be used for traversing the given ast. + * @returns {void} + */ +function defineStartEndAsErrorInTree(ast, visitorKeys) { + Traverser.traverse(ast, { visitorKeys, enter: defineStartEndAsError.bind(null, "node") }); + ast.tokens.forEach(defineStartEndAsError.bind(null, "token")); + ast.comments.forEach(defineStartEndAsError.bind(null, "token")); +} + +/** + * Wraps the given parser in order to intercept and modify return values from the `parse` and `parseForESLint` methods, for test purposes. + * In particular, to modify ast nodes, tokens and comments to throw on access to their `start` and `end` properties. + * @param {Parser} parser Parser object. + * @returns {Parser} Wrapped parser object. + */ +function wrapParser(parser) { + if (typeof parser.parseForESLint === "function") { + return { + parseForESLint(...args) { + const ret = parser.parseForESLint(...args); + + defineStartEndAsErrorInTree(ret.ast, ret.visitorKeys); + return ret; + } + }; + } + return { + parse(...args) { + const ast = parser.parse(...args); + + defineStartEndAsErrorInTree(ast); + return ast; + } + }; +} + //------------------------------------------------------------------------------ // Public Interface //------------------------------------------------------------------------------ @@ -450,9 +518,12 @@ class RuleTester { if (typeof config.parser === "string") { assert(path.isAbsolute(config.parser), "Parsers provided as strings to RuleTester must be absolute paths"); - linter.defineParser(config.parser, require(config.parser)); + } else { + config.parser = espreePath; } + linter.defineParser(config.parser, wrapParser(require(config.parser))); + if (schema) { ajv.validateSchema(schema); @@ -483,13 +554,9 @@ class RuleTester { // Verify the code. const messages = linter.verify(code, config, filename); + const fatalErrorMessage = messages.find(m => m.fatal); - // Ignore syntax errors for backward compatibility if `errors` is a number. - if (typeof item.errors !== "number") { - const errorMessage = messages.find(m => m.fatal); - - assert(!errorMessage, `A fatal parsing error occurred: ${errorMessage && errorMessage.message}`); - } + assert(!fatalErrorMessage, `A fatal parsing error occurred: ${fatalErrorMessage && fatalErrorMessage.message}`); // Verify if autofix makes a syntax error or not. if (messages.some(m => m.fix)) { @@ -772,6 +839,12 @@ class RuleTester { } else { assert.strictEqual(result.output, item.output, "Output is incorrect."); } + } else { + assert.strictEqual( + result.output, + item.code, + "The rule fixed the code. Please add 'output' property." + ); } assertASTDidntChange(result.beforeAST, result.afterAST); diff --git a/tests/lib/rule-tester/rule-tester.js b/tests/lib/rule-tester/rule-tester.js index 88d3e3dd6ea..65a06da85d9 100644 --- a/tests/lib/rule-tester/rule-tester.js +++ b/tests/lib/rule-tester/rule-tester.js @@ -11,7 +11,8 @@ const sinon = require("sinon"), EventEmitter = require("events"), { RuleTester } = require("../../../lib/rule-tester"), assert = require("chai").assert, - nodeAssert = require("assert"); + nodeAssert = require("assert"), + { noop } = require("lodash"); const NODE_ASSERT_STRICT_EQUAL_OPERATOR = (() => { try { @@ -233,7 +234,7 @@ describe("RuleTester", () => { "bar = baz;" ], invalid: [ - { code: "var foo = bar;", errors: ["Bad var."] } + { code: "var foo = bar;", output: " foo = bar;", errors: ["Bad var."] } ] }); }); @@ -242,7 +243,7 @@ describe("RuleTester", () => { ruleTester.run("no-var", require("../../fixtures/testers/rule-tester/no-var"), { valid: [], invalid: [ - { code: "var foo = bar;", errors: [/^Bad var/u] } + { code: "var foo = bar;", output: " foo = bar;", errors: [/^Bad var/u] } ] }); }); @@ -283,7 +284,7 @@ describe("RuleTester", () => { ruleTester.run("no-var", require("../../fixtures/testers/rule-tester/no-var"), { valid: [], invalid: [ - { code: "var foo = bar;", errors: [{ message: /^Bad var/u }] } + { code: "var foo = bar;", output: " foo = bar;", errors: [{ message: /^Bad var/u }] } ] }); }); @@ -392,6 +393,19 @@ describe("RuleTester", () => { }, /Expected no autofixes to be suggested/u); }); + it("should throw an error when the expected output isn't specified and problems produce output", () => { + assert.throws(() => { + ruleTester.run("no-var", require("../../fixtures/testers/rule-tester/no-var"), { + valid: [ + "bar = baz;" + ], + invalid: [ + { code: "var foo = bar;", errors: 1 } + ] + }); + }, "The rule fixed the code. Please add 'output' property."); + }); + it("should throw an error if invalid code specifies wrong type", () => { assert.throws(() => { ruleTester.run("no-eval", require("../../fixtures/testers/rule-tester/no-eval"), { @@ -532,6 +546,45 @@ describe("RuleTester", () => { }, /Should have 2 errors but had 1/u); }); + it("should throw an error if there's a parsing error in a valid test", () => { + assert.throws(() => { + ruleTester.run("no-eval", require("../../fixtures/testers/rule-tester/no-eval"), { + valid: [ + "1eval('foo')" + ], + invalid: [ + { code: "eval('foo')", errors: [{}] } + ] + }); + }, /fatal parsing error/iu); + }); + + it("should throw an error if there's a parsing error in an invalid test", () => { + assert.throws(() => { + ruleTester.run("no-eval", require("../../fixtures/testers/rule-tester/no-eval"), { + valid: [ + "noeval('foo')" + ], + invalid: [ + { code: "1eval('foo')", errors: [{}] } + ] + }); + }, /fatal parsing error/iu); + }); + + it("should throw an error if there's a parsing error in an invalid test and errors is just a number", () => { + assert.throws(() => { + ruleTester.run("no-eval", require("../../fixtures/testers/rule-tester/no-eval"), { + valid: [ + "noeval('foo')" + ], + invalid: [ + { code: "1eval('foo')", errors: 1 } + ] + }); + }, /fatal parsing error/iu); + }); + // https://github.com/eslint/eslint/issues/4779 it("should throw an error if there's a parsing error and output doesn't match", () => { assert.throws(() => { @@ -686,6 +739,37 @@ describe("RuleTester", () => { assert.strictEqual(spy.args[1][1].parser, require.resolve("esprima")); }); + it("should pass-through services from parseForESLint to the rule", () => { + const enhancedParserPath = require.resolve("../../fixtures/parsers/enhanced-parser"); + const disallowHiRule = { + create: context => ({ + Literal(node) { + const disallowed = context.parserServices.test.getMessage(); // returns "Hi!" + + if (node.value === disallowed) { + context.report({ node, message: `Don't use '${disallowed}'` }); + } + } + }) + }; + + ruleTester.run("no-hi", disallowHiRule, { + valid: [ + { + code: "'Hello!'", + parser: enhancedParserPath + } + ], + invalid: [ + { + code: "'Hi!'", + parser: enhancedParserPath, + errors: [{ message: "Don't use 'Hi!'" }] + } + ] + }); + }); + it("should prevent invalid options schemas", () => { assert.throws(() => { ruleTester.run("no-invalid-schema", require("../../fixtures/testers/rule-tester/no-invalid-schema"), { @@ -904,6 +988,122 @@ describe("RuleTester", () => { }, "Rule should not modify AST."); }); + it("should throw an error if rule uses start and end properties on nodes, tokens or comments", () => { + const usesStartEndRule = { + create(context) { + + const sourceCode = context.getSourceCode(); + + return { + CallExpression(node) { + noop(node.arguments[1].start); + }, + "BinaryExpression[operator='+']"(node) { + noop(node.end); + }, + "UnaryExpression[operator='-']"(node) { + noop(sourceCode.getFirstToken(node).start); + }, + ConditionalExpression(node) { + noop(sourceCode.getFirstToken(node).end); + }, + BlockStatement(node) { + noop(sourceCode.getCommentsInside(node)[0].start); + }, + ObjectExpression(node) { + noop(sourceCode.getCommentsInside(node)[0].end); + }, + Decorator(node) { + noop(node.start); + } + }; + } + }; + + assert.throws(() => { + ruleTester.run("uses-start-end", usesStartEndRule, { + valid: ["foo(a, b)"], + invalid: [] + }); + }, "Use node.range[0] instead of node.start"); + assert.throws(() => { + ruleTester.run("uses-start-end", usesStartEndRule, { + valid: [], + invalid: [{ code: "var a = b * (c + d) / e;", errors: 1 }] + }); + }, "Use node.range[1] instead of node.end"); + assert.throws(() => { + ruleTester.run("uses-start-end", usesStartEndRule, { + valid: [], + invalid: [{ code: "var a = -b * c;", errors: 1 }] + }); + }, "Use token.range[0] instead of token.start"); + assert.throws(() => { + ruleTester.run("uses-start-end", usesStartEndRule, { + valid: ["var a = b ? c : d;"], + invalid: [] + }); + }, "Use token.range[1] instead of token.end"); + assert.throws(() => { + ruleTester.run("uses-start-end", usesStartEndRule, { + valid: ["function f() { /* comment */ }"], + invalid: [] + }); + }, "Use token.range[0] instead of token.start"); + assert.throws(() => { + ruleTester.run("uses-start-end", usesStartEndRule, { + valid: [], + invalid: [{ code: "var x = //\n {\n //comment\n //\n}", errors: 1 }] + }); + }, "Use token.range[1] instead of token.end"); + + const enhancedParserPath = require.resolve("../../fixtures/parsers/enhanced-parser"); + + assert.throws(() => { + ruleTester.run("uses-start-end", usesStartEndRule, { + valid: [{ code: "foo(a, b)", parser: enhancedParserPath }], + invalid: [] + }); + }, "Use node.range[0] instead of node.start"); + assert.throws(() => { + ruleTester.run("uses-start-end", usesStartEndRule, { + valid: [], + invalid: [{ code: "var a = b * (c + d) / e;", parser: enhancedParserPath, errors: 1 }] + }); + }, "Use node.range[1] instead of node.end"); + assert.throws(() => { + ruleTester.run("uses-start-end", usesStartEndRule, { + valid: [], + invalid: [{ code: "var a = -b * c;", parser: enhancedParserPath, errors: 1 }] + }); + }, "Use token.range[0] instead of token.start"); + assert.throws(() => { + ruleTester.run("uses-start-end", usesStartEndRule, { + valid: [{ code: "var a = b ? c : d;", parser: enhancedParserPath }], + invalid: [] + }); + }, "Use token.range[1] instead of token.end"); + assert.throws(() => { + ruleTester.run("uses-start-end", usesStartEndRule, { + valid: [{ code: "function f() { /* comment */ }", parser: enhancedParserPath }], + invalid: [] + }); + }, "Use token.range[0] instead of token.start"); + assert.throws(() => { + ruleTester.run("uses-start-end", usesStartEndRule, { + valid: [], + invalid: [{ code: "var x = //\n {\n //comment\n //\n}", parser: enhancedParserPath, errors: 1 }] + }); + }, "Use token.range[1] instead of token.end"); + + assert.throws(() => { + ruleTester.run("uses-start-end", usesStartEndRule, { + valid: [{ code: "@foo class A {}", parser: require.resolve("../../fixtures/parsers/enhanced-parser2") }], + invalid: [] + }); + }, "Use node.range[0] instead of node.start"); + }); + it("should throw an error if no test scenarios given", () => { assert.throws(() => { ruleTester.run("foo", require("../../fixtures/testers/rule-tester/modify-ast-at-last")); @@ -1582,6 +1782,7 @@ describe("RuleTester", () => { invalid: [ { code: "var x = invalid(code);", + output: " x = invalid(code);", errors: 1 } ] diff --git a/tests/lib/rules/_set-default-parser.js b/tests/lib/rules/_set-default-parser.js deleted file mode 100644 index 7774313855e..00000000000 --- a/tests/lib/rules/_set-default-parser.js +++ /dev/null @@ -1,20 +0,0 @@ -/** - * @author Toru Nagashima - * - * This file must be loaded before rule files. - * - * This file configures the default config of RuleTester to use TestParser - * instead of espree. The TestParser parses the given source code by espree, - * then remove the `start` and `end` properties of nodes, tokens, and comments. - * - * We have not endorsed that the properties exist on the AST of custom parsers, - * so we should check that core rules don't use the properties. - */ -"use strict"; - -const path = require("path"); -const { RuleTester } = require("../../../lib/rule-tester"); - -RuleTester.setDefaultConfig({ - parser: path.resolve(__dirname, "../../../tools/internal-testers/test-parser") -}); diff --git a/tests/lib/rules/spaced-comment.js b/tests/lib/rules/spaced-comment.js index 68258214c19..5d3d68fc06b 100644 --- a/tests/lib/rules/spaced-comment.js +++ b/tests/lib/rules/spaced-comment.js @@ -8,8 +8,7 @@ const rule = require("../../../lib/rules/spaced-comment"), { RuleTester } = require("../../../lib/rule-tester"); const ruleTester = new RuleTester(), - validShebangProgram = "#!/path/to/node\nvar a = 3;", - invalidShebangProgram = "#!/path/to/node\n#!/second/shebang\nvar a = 3;"; + validShebangProgram = "#!/path/to/node\nvar a = 3;"; ruleTester.run("spaced-comment", rule, { @@ -637,20 +636,6 @@ ruleTester.run("spaced-comment", rule, { }] }, - // Parser errors - { - code: invalidShebangProgram, - output: null, - options: ["always"], - errors: 1 - }, - { - code: invalidShebangProgram, - output: null, - options: ["never"], - errors: 1 - }, - // not a marker-only comment, regression tests for https://github.com/eslint/eslint/issues/12036 { code: "//#endregionfoo",