Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Breaking: require meta for fixable rules (fixes #13349) (#14634)
* Breaking: require `meta` for fixable rules (fixes #13349)

* Change error message
  • Loading branch information
mdjermanovic committed Aug 5, 2021
1 parent d6a761f commit 4a7aab7
Show file tree
Hide file tree
Showing 6 changed files with 105 additions and 69 deletions.
4 changes: 2 additions & 2 deletions lib/linter/linter.js
Expand Up @@ -925,8 +925,8 @@ 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.");
if (problem.fix && !(rule.meta && rule.meta.fixable)) {
throw new Error("Fixable rules must set the `meta.fixable` property to \"code\" or \"whitespace\".");
}
if (problem.suggestions && !(rule.meta && rule.meta.hasSuggestions === true)) {
if (rule.meta && rule.meta.docs && typeof rule.meta.docs.suggestion !== "undefined") {
Expand Down
10 changes: 0 additions & 10 deletions lib/rule-tester/rule-tester.js
Expand Up @@ -921,16 +921,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);
}

Expand Down
31 changes: 18 additions & 13 deletions 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 = [];
29 changes: 17 additions & 12 deletions tests/fixtures/testers/rule-tester/fixes-one-problem.js
Expand Up @@ -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)
});
}
}
}
};
71 changes: 50 additions & 21 deletions tests/lib/linter/linter.js
Expand Up @@ -5106,17 +5106,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,
Expand Down Expand Up @@ -5205,15 +5212,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" } });

Expand All @@ -5237,10 +5252,10 @@ var a = "test2";

assert.throws(() => {
linter.verify("0", { rules: { "test-rule": "error" } });
}, /Fixable rules should export a `meta\.fixable` property.\nOccurred while linting <input>:1$/u);
}, /Fixable rules must set the `meta\.fixable` property to "code" or "whitespace".\nOccurred while linting <input>: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) {
Expand All @@ -5249,7 +5264,21 @@ var a = "test2";
})
});

linter.verify("0", { rules: { "test-rule": "error" } });
assert.throws(() => {
linter.verify("0", { rules: { "test-rule": "error" } });
}, /Fixable rules must set the `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 must set the `meta\.fixable` property/u);
});
});

Expand Down
29 changes: 18 additions & 11 deletions tests/lib/rule-tester/rule-tester.js
Expand Up @@ -1747,7 +1747,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", () => {

Expand All @@ -1771,7 +1771,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", () => {
Expand Down Expand Up @@ -2392,17 +2392,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: []
Expand Down

0 comments on commit 4a7aab7

Please sign in to comment.