Skip to content

Commit

Permalink
Breaking: require meta for fixable rules (fixes #13349)
Browse files Browse the repository at this point in the history
  • Loading branch information
mdjermanovic committed May 26, 2021
1 parent ec28b5a commit a647c7c
Show file tree
Hide file tree
Showing 6 changed files with 101 additions and 65 deletions.
2 changes: 1 addition & 1 deletion lib/linter/linter.js
Expand Up @@ -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);
Expand Down
10 changes: 0 additions & 10 deletions lib/rule-tester/rule-tester.js
Expand Up @@ -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);
}

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)
});
}
}
}
};
69 changes: 49 additions & 20 deletions tests/lib/linter/linter.js
Expand Up @@ -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,
Expand Down Expand Up @@ -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" } });

Expand All @@ -5068,7 +5083,7 @@ var a = "test2";
}, /Fixable rules should export a `meta\.fixable` property.\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 @@ -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);
});
});

Expand Down
25 changes: 16 additions & 9 deletions tests/lib/rule-tester/rule-tester.js
Expand Up @@ -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: []
Expand Down

0 comments on commit a647c7c

Please sign in to comment.