Breaking: require meta for fixable rules (fixes #13349) (#14634)

mdjermanovic · web-flow · commit 4a7aab7d4323 · 2021-08-05T13:04:15.000-07:00
* Breaking: require `meta` for fixable rules (fixes #13349) * Change error message
diff --git a/lib/linter/linter.js b/lib/linter/linter.js
@@ -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") {
diff --git a/lib/rule-tester/rule-tester.js b/lib/rule-tester/rule-tester.js
@@ -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);
         }
 
diff --git 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
@@ -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
@@ -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,
@@ -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" } });
 
@@ -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) {
@@ -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);
         });
     });
 
diff --git a/tests/lib/rule-tester/rule-tester.js b/tests/lib/rule-tester/rule-tester.js
@@ -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", () => {
 
@@ -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", () => {
@@ -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: []

Original file line number	Diff line number	Diff line change
`@@ -925,8 +925,8 @@ function runRules(sourceCode, configuredRules, ruleMapper, parserOptions, parser`
`925`	`925`	`}`
`926`	`926`	`const problem = reportTranslator(...args);`
`927`	`927`
`928`		`- if (problem.fix && rule.meta && !rule.meta.fixable) {`
`929`		- throw new Error("Fixable rules should export a `meta.fixable` property.");
	`928`	`+ if (problem.fix && !(rule.meta && rule.meta.fixable)) {`
	`929`	+ throw new Error("Fixable rules must set the `meta.fixable` property to \"code\" or \"whitespace\".");
`930`	`930`	`}`
`931`	`931`	`if (problem.suggestions && !(rule.meta && rule.meta.hasSuggestions === true)) {`
`932`	`932`	`if (rule.meta && rule.meta.docs && typeof rule.meta.docs.suggestion !== "undefined") {`