Skip to content

Commit dc29b03

Browse files
bmishbmish
authored
Update: Fix false positives/negatives in require-meta-fixable rule (#158)
False positives fixed: * When `meta.fixable` uses a variable False negatives fixed: * When there's a fixer but `meta.fixable` = null * When there's a fixer but `meta.fixable` = undefined Also improves reporting location.
1 parent 88cb2bf commit dc29b03

File tree

3 files changed

+76
-41
lines changed

3 files changed

+76
-41
lines changed

‎docs/rules/require-meta-fixable.md

Lines changed: 4 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,19 @@
22

33
✔️ The `"extends": "plugin:eslint-plugin/recommended"` property in a configuration file enables this rule.
44

5-
A fixable ESLint rule must have a valid `meta.fixable` property. A rule reports a problem with a `fix()` function but does not export a `meta.fixable` property is likely to cause an unexpected error.
5+
ESLint requires fixable rules to specify a valid `meta.fixable` property (with value `code` or `whitespace`).
66

77
## Rule Details
88

9-
This rule aims to require ESLint rules to have a `meta.fixable` property if necessary.
9+
This rule aims to require fixable ESLint rules to have a valid `meta.fixable` property.
1010

1111
Examples of **incorrect** code for this rule:
1212

1313
```js
1414
/* eslint eslint-plugin/require-meta-fixable: "error" */
1515

1616
module.exports = {
17-
meta: {},
17+
meta: {}, // missing `fixable` property
1818
create (context) {
1919
context.report({
2020
node,
@@ -44,20 +44,6 @@ module.exports = {
4444
};
4545
```
4646

47-
```js
48-
/* eslint eslint-plugin/require-meta-fixable: "error" */
49-
50-
module.exports = { create (context) {
51-
context.report({
52-
node,
53-
message: 'foo',
54-
fix (fixer) {
55-
return fixer.remove(node);
56-
},
57-
});
58-
} };
59-
```
60-
6147
Examples of **correct** code for this rule:
6248

6349
```js
@@ -91,10 +77,7 @@ module.exports = {
9177
};
9278
```
9379

94-
## When Not To Use It
95-
96-
If you do not plan to implement autofixable rules, you can turn off this rule.
97-
9880
## Further Reading
9981

10082
* [ESLint's autofix API](http://eslint.org/docs/developer-guide/working-with-rules#applying-fixes)
83+
* [ESLint's rule basics mentioning `meta.fixable`](https://eslint.org/docs/developer-guide/working-with-rules#rule-basics)

‎lib/rules/require-meta-fixable.js

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
'use strict';
77

8+
const { getStaticValue } = require('eslint-utils');
89
const utils = require('../utils');
910

1011
// ------------------------------------------------------------------------------
@@ -20,6 +21,10 @@ module.exports = {
2021
recommended: true,
2122
},
2223
schema: [],
24+
messages: {
25+
invalid: '`meta.fixable` must be either `code`, `whitespace` or `null`.',
26+
missing: 'Fixable rules must export a `meta.fixable` property.',
27+
},
2328
},
2429

2530
create (context) {
@@ -62,19 +67,25 @@ module.exports = {
6267
ruleInfo.meta.properties.find(prop => utils.getKeyName(prop) === 'fixable');
6368

6469
if (metaFixableProp) {
65-
const VALID_VALUES = new Set(['code', 'whitespace', null, undefined]);
66-
const valueIsValid = metaFixableProp.value.type === 'Literal'
67-
? VALID_VALUES.has(metaFixableProp.value.value)
68-
// eslint-disable-next-line unicorn/no-nested-ternary
69-
: metaFixableProp.value.type === 'TemplateLiteral' && metaFixableProp.value.quasis.length === 1
70-
? VALID_VALUES.has(metaFixableProp.value.quasis[0].value.cooked)
71-
: metaFixableProp.value.type === 'Identifier' && metaFixableProp.value.name === 'undefined';
70+
const staticValue = getStaticValue(metaFixableProp.value, context.getScope());
71+
if (!staticValue) {
72+
// Ignore non-static values since we can't determine what they look like.
73+
return;
74+
}
75+
76+
if (!['code', 'whitespace', null, undefined].includes(staticValue.value)) {
77+
// `fixable` property has an invalid value.
78+
context.report({ node: metaFixableProp.value, messageId: 'invalid' });
79+
return;
80+
}
7281

73-
if (!valueIsValid) {
74-
context.report({ node: metaFixableProp, message: '`meta.fixable` must be either `code`, `whitespace` or `null`.' });
82+
if (usesFixFunctions && !['code', 'whitespace'].includes(staticValue.value)) {
83+
// Rule is fixable but `fixable` property does not have a fixable value.
84+
context.report({ node: metaFixableProp.value, messageId: 'missing' });
7585
}
76-
} else if (usesFixFunctions) {
77-
context.report({ node: ruleInfo.create, message: 'Fixable rules must export a `meta.fixable` property.' });
86+
} else if (!metaFixableProp && usesFixFunctions) {
87+
// Rule is fixable but is missing the `fixable` property.
88+
context.report({ node: ruleInfo.meta || ruleInfo.create, messageId: 'missing' });
7889
}
7990
},
8091
};

‎tests/lib/rules/require-meta-fixable.js

Lines changed: 50 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,6 @@
1212
const rule = require('../../../lib/rules/require-meta-fixable');
1313
const RuleTester = require('eslint').RuleTester;
1414

15-
const MISSING_ERROR = { message: 'Fixable rules must export a `meta.fixable` property.', type: 'FunctionExpression' };
16-
const INVALID_ERROR = { message: '`meta.fixable` must be either `code`, `whitespace` or `null`.', type: 'Property' };
17-
1815
// ------------------------------------------------------------------------------
1916
// Tests
2017
// ------------------------------------------------------------------------------
@@ -37,6 +34,16 @@ ruleTester.run('require-meta-fixable', rule, {
3734
}
3835
};
3936
`,
37+
// Value in variable.
38+
`
39+
const fixable = 'code';
40+
module.exports = {
41+
meta: { fixable },
42+
create(context) {
43+
context.report({node, message, fix: foo});
44+
}
45+
};
46+
`,
4047
`
4148
module.exports = {
4249
meta: { fixable: 'whitespace' },
@@ -93,6 +100,13 @@ ruleTester.run('require-meta-fixable', rule, {
93100
}
94101
};
95102
`,
103+
// `fixable` uses variable but no static value available.
104+
`
105+
module.exports = {
106+
meta: { fixable: foo },
107+
create(context) { context.report({node, message, fix: foo}); }
108+
};
109+
`,
96110
`
97111
module.exports = {
98112
meta: {},
@@ -128,7 +142,15 @@ ruleTester.run('require-meta-fixable', rule, {
128142
create(context) { context.report({node, message, fix: foo}); }
129143
};
130144
`,
131-
errors: [MISSING_ERROR],
145+
errors: [{ messageId: 'missing', type: 'ObjectExpression' }],
146+
},
147+
{
148+
code: `
149+
module.exports = {
150+
create(context) { context.report({node, message, fix: foo}); }
151+
};
152+
`,
153+
errors: [{ messageId: 'missing', type: 'FunctionExpression' }],
132154
},
133155
{
134156
code: `
@@ -137,7 +159,7 @@ ruleTester.run('require-meta-fixable', rule, {
137159
create(context) { context.report(node, loc, message, data, fix); }
138160
};
139161
`,
140-
errors: [MISSING_ERROR],
162+
errors: [{ messageId: 'missing', type: 'ObjectExpression' }],
141163
},
142164
{
143165
code: `
@@ -146,7 +168,7 @@ ruleTester.run('require-meta-fixable', rule, {
146168
create(context) { context.report({node, message}); }
147169
};
148170
`,
149-
errors: [INVALID_ERROR],
171+
errors: [{ messageId: 'invalid', type: 'Literal' }],
150172
},
151173
{
152174
code: `
@@ -155,16 +177,35 @@ ruleTester.run('require-meta-fixable', rule, {
155177
create(context) { context.report({node, message, fix: foo}); }
156178
};
157179
`,
158-
errors: [INVALID_ERROR],
180+
errors: [{ messageId: 'invalid', type: 'Literal' }],
181+
},
182+
{
183+
code: `
184+
const fixable = 'invalid';
185+
module.exports = {
186+
meta: { fixable },
187+
create(context) { context.report({node, message, fix: foo}); }
188+
};
189+
`,
190+
errors: [{ messageId: 'invalid', type: 'Identifier' }],
191+
},
192+
{
193+
code: `
194+
module.exports = {
195+
meta: { fixable: null },
196+
create(context) { context.report({node, message, fix: foo}); }
197+
};
198+
`,
199+
errors: [{ messageId: 'missing', type: 'Literal' }],
159200
},
160201
{
161202
code: `
162203
module.exports = {
163-
meta: { fixable: foo },
204+
meta: { fixable: undefined },
164205
create(context) { context.report({node, message, fix: foo}); }
165206
};
166207
`,
167-
errors: [INVALID_ERROR],
208+
errors: [{ messageId: 'missing', type: 'Identifier' }],
168209
},
169210
],
170211
});

0 commit comments

Comments
 (0)