From 2a9ce4e41dad040b90cec3b2196dff938d4c1299 Mon Sep 17 00:00:00 2001 From: Milos Djermanovic Date: Mon, 13 Jul 2020 23:37:34 +0200 Subject: [PATCH 1/3] Update: require `meta` for fixable rules in RuleTester (refs #13349) --- lib/rule-tester/rule-tester.js | 10 ++++ tests/lib/rule-tester/rule-tester.js | 69 ++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+) diff --git a/lib/rule-tester/rule-tester.js b/lib/rule-tester/rule-tester.js index 3e391576716..8b40e878c14 100644 --- a/lib/rule-tester/rule-tester.js +++ b/lib/rule-tester/rule-tester.js @@ -861,6 +861,16 @@ class RuleTester { ); } + // new-format (object format) rules must have `meta.fixable` property if they make fixes. + if (result.output !== item.code && typeof rule !== "function") { + assert.ok( + hasOwnProperty(rule, "meta"), + "Fixable rules should export a `meta.fixable` property." + ); + + // Linter throws if rule has `meta` but doesn't have `meta.fixable`. + } + assertASTDidntChange(result.beforeAST, result.afterAST); } diff --git a/tests/lib/rule-tester/rule-tester.js b/tests/lib/rule-tester/rule-tester.js index 7e26e1a826c..c168ee608dd 100644 --- a/tests/lib/rule-tester/rule-tester.js +++ b/tests/lib/rule-tester/rule-tester.js @@ -304,6 +304,10 @@ describe("RuleTester", () => { it("should use strict equality to compare output", () => { const replaceProgramWith5Rule = { + meta: { + fixable: "code" + }, + create: context => ({ Program(node) { context.report({ node, message: "bad", fix: fixer => fixer.replaceText(node, "5") }); @@ -1237,6 +1241,71 @@ describe("RuleTester", () => { }, "Error must specify 'messageId' if 'data' is used."); }); + // fixable rules with or without `meta` property + it("should not throw an error if a legacy-format rule makes fixes", () => { + + /** + * Legacy-format rule (a function instead of an object with `create` method). + * @param {RuleContext} context The ESLint rule context object. + * @returns {void} No return value + */ + function replaceProgramWith5Rule(context) { + return { + Program(node) { + context.report({ node, message: "bad", fix: fixer => fixer.replaceText(node, "5") }); + } + }; + } + + ruleTester.run("replaceProgramWith5", replaceProgramWith5Rule, { + valid: [], + invalid: [ + { code: "var foo = bar;", output: "5", errors: 1 } + ] + }); + }); + it("should not throw an error if a new-format rule that has `meta.fixable` makes fixes", () => { + const replaceProgramWith5Rule = { + meta: { + fixable: "code" + }, + create(context) { + return { + Program(node) { + context.report({ node, message: "bad", fix: fixer => fixer.replaceText(node, "5") }); + } + }; + } + }; + + ruleTester.run("replaceProgramWith5", replaceProgramWith5Rule, { + valid: [], + invalid: [ + { code: "var foo = bar;", output: "5", errors: 1 } + ] + }); + }); + it("should throw an error if a new-format rule that doesn't have `meta` makes fixes", () => { + const replaceProgramWith5Rule = { + create(context) { + return { + Program(node) { + context.report({ node, message: "bad", fix: fixer => fixer.replaceText(node, "5") }); + } + }; + } + }; + + assert.throws(() => { + ruleTester.run("replaceProgramWith5", replaceProgramWith5Rule, { + valid: [], + invalid: [ + { code: "var foo = bar;", output: "5", errors: 1 } + ] + }); + }, "Fixable rules should export a `meta.fixable` property."); + }); + describe("suggestions", () => { it("should pass with valid suggestions (tested using desc)", () => { ruleTester.run("suggestions-basic", require("../../fixtures/testers/rule-tester/suggestions").basic, { From af96bc5549b07975cf8b8a80bb6a2ccba1eb608e Mon Sep 17 00:00:00 2001 From: Milos Djermanovic Date: Wed, 15 Jul 2020 22:21:07 +0200 Subject: [PATCH 2/3] Fix JSDoc --- tests/lib/rule-tester/rule-tester.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/lib/rule-tester/rule-tester.js b/tests/lib/rule-tester/rule-tester.js index c168ee608dd..66f89874d8c 100644 --- a/tests/lib/rule-tester/rule-tester.js +++ b/tests/lib/rule-tester/rule-tester.js @@ -1247,7 +1247,7 @@ describe("RuleTester", () => { /** * Legacy-format rule (a function instead of an object with `create` method). * @param {RuleContext} context The ESLint rule context object. - * @returns {void} No return value + * @returns {Object} Listeners. */ function replaceProgramWith5Rule(context) { return { From 49837b54b1c08aad209d04cc43548f3c2105fac4 Mon Sep 17 00:00:00 2001 From: Milos Djermanovic Date: Thu, 23 Jul 2020 17:15:10 +0200 Subject: [PATCH 3/3] Throw for legacy-format fixable rules --- lib/rule-tester/rule-tester.js | 6 +-- tests/fixtures/testers/rule-tester/no-var.js | 48 ++++++++++--------- tests/lib/rule-tester/rule-tester.js | 50 ++++++++++---------- 3 files changed, 55 insertions(+), 49 deletions(-) diff --git a/lib/rule-tester/rule-tester.js b/lib/rule-tester/rule-tester.js index 8b40e878c14..905f3418121 100644 --- a/lib/rule-tester/rule-tester.js +++ b/lib/rule-tester/rule-tester.js @@ -861,14 +861,14 @@ class RuleTester { ); } - // new-format (object format) rules must have `meta.fixable` property if they make fixes. - if (result.output !== item.code && typeof rule !== "function") { + // Rules that produce fixes must have `meta.fixable` property. + if (result.output !== item.code) { assert.ok( hasOwnProperty(rule, "meta"), "Fixable rules should export a `meta.fixable` property." ); - // Linter throws if rule has `meta` but doesn't have `meta.fixable`. + // Linter throws if a rule that produced a fix has `meta` but doesn't have `meta.fixable`. } assertASTDidntChange(result.beforeAST, result.afterAST); diff --git a/tests/fixtures/testers/rule-tester/no-var.js b/tests/fixtures/testers/rule-tester/no-var.js index 382885613d2..5841f15bfa1 100644 --- a/tests/fixtures/testers/rule-tester/no-var.js +++ b/tests/fixtures/testers/rule-tester/no-var.js @@ -7,27 +7,31 @@ // Rule Definition //------------------------------------------------------------------------------ -module.exports = function(context) { - - "use strict"; - - - var sourceCode = context.getSourceCode(); - - return { - - "VariableDeclaration": function(node) { - if (node.kind === "var") { - context.report({ - node: node, - loc: sourceCode.getFirstToken(node).loc, - message: "Bad var.", - fix: function(fixer) { - return fixer.remove(sourceCode.getFirstToken(node)); - } - }) +"use strict"; + +module.exports = { + + meta: { + fixable: "code" + }, + + create(context) { + + var sourceCode = context.getSourceCode(); + + return { + "VariableDeclaration": function(node) { + if (node.kind === "var") { + context.report({ + node: node, + loc: sourceCode.getFirstToken(node).loc, + message: "Bad var.", + fix: function(fixer) { + return fixer.remove(sourceCode.getFirstToken(node)); + } + }) + } } - } - }; - + }; + } }; diff --git a/tests/lib/rule-tester/rule-tester.js b/tests/lib/rule-tester/rule-tester.js index 66f89874d8c..3f2393621b6 100644 --- a/tests/lib/rule-tester/rule-tester.js +++ b/tests/lib/rule-tester/rule-tester.js @@ -1242,29 +1242,7 @@ describe("RuleTester", () => { }); // fixable rules with or without `meta` property - it("should not throw an error if a legacy-format rule makes fixes", () => { - - /** - * Legacy-format rule (a function instead of an object with `create` method). - * @param {RuleContext} context The ESLint rule context object. - * @returns {Object} Listeners. - */ - function replaceProgramWith5Rule(context) { - return { - Program(node) { - context.report({ node, message: "bad", fix: fixer => fixer.replaceText(node, "5") }); - } - }; - } - - ruleTester.run("replaceProgramWith5", replaceProgramWith5Rule, { - valid: [], - invalid: [ - { code: "var foo = bar;", output: "5", errors: 1 } - ] - }); - }); - it("should not throw an error if a new-format rule that has `meta.fixable` makes fixes", () => { + it("should not throw an error if a rule that has `meta.fixable` produces fixes", () => { const replaceProgramWith5Rule = { meta: { fixable: "code" @@ -1285,7 +1263,7 @@ describe("RuleTester", () => { ] }); }); - it("should throw an error if a new-format rule that doesn't have `meta` makes fixes", () => { + it("should throw an error if a new-format rule that doesn't have `meta` produces fixes", () => { const replaceProgramWith5Rule = { create(context) { return { @@ -1305,6 +1283,30 @@ describe("RuleTester", () => { }); }, "Fixable rules should export a `meta.fixable` property."); }); + it("should throw an error if a legacy-format rule produces fixes", () => { + + /** + * Legacy-format rule (a function instead of an object with `create` method). + * @param {RuleContext} context The ESLint rule context object. + * @returns {Object} Listeners. + */ + function replaceProgramWith5Rule(context) { + return { + Program(node) { + context.report({ node, message: "bad", fix: fixer => fixer.replaceText(node, "5") }); + } + }; + } + + assert.throws(() => { + ruleTester.run("replaceProgramWith5", replaceProgramWith5Rule, { + valid: [], + invalid: [ + { code: "var foo = bar;", output: "5", errors: 1 } + ] + }); + }, "Fixable rules should export a `meta.fixable` property."); + }); describe("suggestions", () => { it("should pass with valid suggestions (tested using desc)", () => {