From a647c7c10aea2d1e42bfcbc4d2c5a072ab148cac Mon Sep 17 00:00:00 2001 From: Milos Djermanovic Date: Wed, 26 May 2021 14:14:30 +0200 Subject: [PATCH 1/2] Breaking: require `meta` for fixable rules (fixes #13349) --- lib/linter/linter.js | 2 +- lib/rule-tester/rule-tester.js | 10 --- .../fixtures/rules/make-syntax-error-rule.js | 31 +++++---- .../testers/rule-tester/fixes-one-problem.js | 29 ++++---- tests/lib/linter/linter.js | 69 +++++++++++++------ tests/lib/rule-tester/rule-tester.js | 25 ++++--- 6 files changed, 101 insertions(+), 65 deletions(-) diff --git a/lib/linter/linter.js b/lib/linter/linter.js index bdc6c1b1d01..f4f8c31d831 100644 --- a/lib/linter/linter.js +++ b/lib/linter/linter.js @@ -919,7 +919,7 @@ function runRules(sourceCode, configuredRules, ruleMapper, parserOptions, parser } const problem = reportTranslator(...args); - if (problem.fix && rule.meta && !rule.meta.fixable) { + if (problem.fix && !(rule.meta && rule.meta.fixable)) { throw new Error("Fixable rules should export a `meta.fixable` property."); } lintingProblems.push(problem); diff --git a/lib/rule-tester/rule-tester.js b/lib/rule-tester/rule-tester.js index b08303c62b2..3627a96eb0b 100644 --- a/lib/rule-tester/rule-tester.js +++ b/lib/rule-tester/rule-tester.js @@ -865,16 +865,6 @@ class RuleTester { ); } - // 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 a rule that produced a fix has `meta` but doesn't have `meta.fixable`. - } - assertASTDidntChange(result.beforeAST, result.afterAST); } diff --git a/tests/fixtures/rules/make-syntax-error-rule.js b/tests/fixtures/rules/make-syntax-error-rule.js index 528b4b0f4de..fce60a56c4c 100644 --- a/tests/fixtures/rules/make-syntax-error-rule.js +++ b/tests/fixtures/rules/make-syntax-error-rule.js @@ -1,14 +1,19 @@ -module.exports = function(context) { - return { - Program: function(node) { - context.report({ - node: node, - message: "ERROR", - fix: function(fixer) { - return fixer.insertTextAfter(node, "this is a syntax error."); - } - }); - } - }; +module.exports = { + meta: { + schema: [], + fixable: "code" + }, + create(context) { + return { + Program: function(node) { + context.report({ + node: node, + message: "ERROR", + fix: function(fixer) { + return fixer.insertTextAfter(node, "this is a syntax error."); + } + }); + } + }; + } }; -module.exports.schema = []; diff --git a/tests/fixtures/testers/rule-tester/fixes-one-problem.js b/tests/fixtures/testers/rule-tester/fixes-one-problem.js index 9ed9ca71cbd..e692cae3386 100644 --- a/tests/fixtures/testers/rule-tester/fixes-one-problem.js +++ b/tests/fixtures/testers/rule-tester/fixes-one-problem.js @@ -5,19 +5,24 @@ "use strict"; -module.exports = context => { - return { - Program(node) { - context.report({ - node, - message: "No programs allowed." - }); +module.exports = { + meta: { + fixable: "code" + }, + create(context) { + return { + Program(node) { + context.report({ + node, + message: "No programs allowed." + }); - context.report({ - node, - message: "Seriously, no programs allowed.", - fix: fixer => fixer.remove(node) - }); + context.report({ + node, + message: "Seriously, no programs allowed.", + fix: fixer => fixer.remove(node) + }); + } } } }; diff --git a/tests/lib/linter/linter.js b/tests/lib/linter/linter.js index 976bd765755..01f4f9b1c15 100644 --- a/tests/lib/linter/linter.js +++ b/tests/lib/linter/linter.js @@ -4934,17 +4934,24 @@ var a = "test2"; it("should use postprocessed problem ranges when applying autofixes", () => { const code = "foo bar baz"; - linter.defineRule("capitalize-identifiers", context => ({ - Identifier(node) { - if (node.name !== node.name.toUpperCase()) { - context.report({ - node, - message: "Capitalize this identifier", - fix: fixer => fixer.replaceText(node, node.name.toUpperCase()) - }); - } + linter.defineRule("capitalize-identifiers", { + meta: { + fixable: "code" + }, + create(context) { + return { + Identifier(node) { + if (node.name !== node.name.toUpperCase()) { + context.report({ + node, + message: "Capitalize this identifier", + fix: fixer => fixer.replaceText(node, node.name.toUpperCase()) + }); + } + } + }; } - })); + }); const fixResult = linter.verifyAndFix( code, @@ -5033,15 +5040,23 @@ var a = "test2"; }); it("stops fixing after 10 passes", () => { - linter.defineRule("add-spaces", context => ({ - Program(node) { - context.report({ - node, - message: "Add a space before this node.", - fix: fixer => fixer.insertTextBefore(node, " ") - }); + + linter.defineRule("add-spaces", { + meta: { + fixable: "whitespace" + }, + create(context) { + return { + Program(node) { + context.report({ + node, + message: "Add a space before this node.", + fix: fixer => fixer.insertTextBefore(node, " ") + }); + } + }; } - })); + }); const fixResult = linter.verifyAndFix("a", { rules: { "add-spaces": "error" } }); @@ -5068,7 +5083,7 @@ var a = "test2"; }, /Fixable rules should export a `meta\.fixable` property.\nOccurred while linting :1$/u); }); - it("should not throw an error if fix is passed and there is no metadata", () => { + it("should throw an error if fix is passed and there is no metadata", () => { linter.defineRule("test-rule", { create: context => ({ Program(node) { @@ -5077,7 +5092,21 @@ var a = "test2"; }) }); - linter.verify("0", { rules: { "test-rule": "error" } }); + assert.throws(() => { + linter.verify("0", { rules: { "test-rule": "error" } }); + }, /Fixable rules should export a `meta\.fixable` property./u); + }); + + it("should throw an error if fix is passed from a legacy-format rule", () => { + linter.defineRule("test-rule", context => ({ + Program(node) { + context.report(node, "hello world", {}, () => ({ range: [1, 1], text: "" })); + } + })); + + assert.throws(() => { + linter.verify("0", { rules: { "test-rule": "error" } }); + }, /Fixable rules should export a `meta\.fixable` property./u); }); }); diff --git a/tests/lib/rule-tester/rule-tester.js b/tests/lib/rule-tester/rule-tester.js index 3aaf6f91f3f..1049e5bbd6f 100644 --- a/tests/lib/rule-tester/rule-tester.js +++ b/tests/lib/rule-tester/rule-tester.js @@ -1923,17 +1923,24 @@ describe("RuleTester", () => { assert.throw(() => { ruleTester.run( "foo", - context => ({ - Identifier(node) { - context.report({ - node, - message: "make a syntax error", - fix(fixer) { - return fixer.replaceText(node, "one two"); + { + meta: { + fixable: "code" + }, + create(context) { + return { + Identifier(node) { + context.report({ + node, + message: "make a syntax error", + fix(fixer) { + return fixer.replaceText(node, "one two"); + } + }); } - }); + }; } - }), + }, { valid: ["one()"], invalid: [] From 7a983f9a1d309b22ac657ad0a6f9aba741d77516 Mon Sep 17 00:00:00 2001 From: Milos Djermanovic Date: Thu, 10 Jun 2021 17:43:44 +0200 Subject: [PATCH 2/2] Change error message --- lib/linter/linter.js | 2 +- tests/lib/linter/linter.js | 6 +++--- tests/lib/rule-tester/rule-tester.js | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/linter/linter.js b/lib/linter/linter.js index f4f8c31d831..f6cfdb4963f 100644 --- a/lib/linter/linter.js +++ b/lib/linter/linter.js @@ -920,7 +920,7 @@ function runRules(sourceCode, configuredRules, ruleMapper, parserOptions, parser const problem = reportTranslator(...args); if (problem.fix && !(rule.meta && rule.meta.fixable)) { - throw new Error("Fixable rules should export a `meta.fixable` property."); + throw new Error("Fixable rules must set the `meta.fixable` property to \"code\" or \"whitespace\"."); } lintingProblems.push(problem); } diff --git a/tests/lib/linter/linter.js b/tests/lib/linter/linter.js index 01f4f9b1c15..0eb0970216f 100644 --- a/tests/lib/linter/linter.js +++ b/tests/lib/linter/linter.js @@ -5080,7 +5080,7 @@ var a = "test2"; assert.throws(() => { linter.verify("0", { rules: { "test-rule": "error" } }); - }, /Fixable rules should export a `meta\.fixable` property.\nOccurred while linting :1$/u); + }, /Fixable rules must set the `meta\.fixable` property to "code" or "whitespace".\nOccurred while linting :1$/u); }); it("should throw an error if fix is passed and there is no metadata", () => { @@ -5094,7 +5094,7 @@ var a = "test2"; assert.throws(() => { linter.verify("0", { rules: { "test-rule": "error" } }); - }, /Fixable rules should export a `meta\.fixable` property./u); + }, /Fixable rules must set the `meta\.fixable` property/u); }); it("should throw an error if fix is passed from a legacy-format rule", () => { @@ -5106,7 +5106,7 @@ var a = "test2"; assert.throws(() => { linter.verify("0", { rules: { "test-rule": "error" } }); - }, /Fixable rules should export a `meta\.fixable` property./u); + }, /Fixable rules must set the `meta\.fixable` property/u); }); }); diff --git a/tests/lib/rule-tester/rule-tester.js b/tests/lib/rule-tester/rule-tester.js index 1049e5bbd6f..9c32e0d31cf 100644 --- a/tests/lib/rule-tester/rule-tester.js +++ b/tests/lib/rule-tester/rule-tester.js @@ -1289,7 +1289,7 @@ describe("RuleTester", () => { { code: "var foo = bar;", output: "5", errors: 1 } ] }); - }, "Fixable rules should export a `meta.fixable` property."); + }, /Fixable rules must set the `meta\.fixable` property/u); }); it("should throw an error if a legacy-format rule produces fixes", () => { @@ -1313,7 +1313,7 @@ describe("RuleTester", () => { { code: "var foo = bar;", output: "5", errors: 1 } ] }); - }, "Fixable rules should export a `meta.fixable` property."); + }, /Fixable rules must set the `meta\.fixable` property/u); }); describe("suggestions", () => {