Skip to content

Commit 25de00f

Browse files
bmishbmish
committed
Breaking: Update fixer-return and prefer-replace-text rules to also apply to suggestion fixer functions
1 parent adcd420 commit 25de00f

File tree

6 files changed

+244
-27
lines changed

6 files changed

+244
-27
lines changed

‎lib/rules/fixer-return.js

+1-11
Original file line numberDiff line numberDiff line change
@@ -98,22 +98,12 @@ module.exports = {
9898

9999
// Stacks this function's information.
100100
onCodePathStart (codePath, node) {
101-
const parent = node.parent;
102-
103-
// Whether we are inside the fixer function we care about.
104-
const shouldCheck = ['FunctionExpression', 'ArrowFunctionExpression'].includes(node.type) &&
105-
parent.parent.type === 'ObjectExpression' &&
106-
parent.parent.parent.type === 'CallExpression' &&
107-
contextIdentifiers.has(parent.parent.parent.callee.object) &&
108-
parent.parent.parent.callee.property.name === 'report' &&
109-
utils.getReportInfo(parent.parent.parent.arguments).fix === node;
110-
111101
funcInfo = {
112102
upper: funcInfo,
113103
codePath,
114104
hasYieldWithFixer: false,
115105
hasReturnWithFixer: false,
116-
shouldCheck,
106+
shouldCheck: utils.isAutoFixerFunction(node, contextIdentifiers) || utils.isSuggestionFixerFunction(node, contextIdentifiers),
117107
node,
118108
};
119109
},

‎lib/rules/prefer-replace-text.js

+1-9
Original file line numberDiff line numberDiff line change
@@ -43,18 +43,10 @@ module.exports = {
4343

4444
// Stacks this function's information.
4545
onCodePathStart (codePath, node) {
46-
const parent = node.parent;
47-
const shouldCheck = (node.type === 'FunctionExpression' || node.type === 'ArrowFunctionExpression') &&
48-
parent.parent.type === 'ObjectExpression' &&
49-
parent.parent.parent.type === 'CallExpression' &&
50-
contextIdentifiers.has(parent.parent.parent.callee.object) &&
51-
parent.parent.parent.callee.property.name === 'report' &&
52-
utils.getReportInfo(parent.parent.parent.arguments, context).fix === node;
53-
5446
funcInfo = {
5547
upper: funcInfo,
5648
codePath,
57-
shouldCheck,
49+
shouldCheck: utils.isAutoFixerFunction(node, contextIdentifiers) || utils.isSuggestionFixerFunction(node, contextIdentifiers),
5850
node,
5951
};
6052
},

‎lib/utils.js

+40
Original file line numberDiff line numberDiff line change
@@ -424,4 +424,44 @@ module.exports = {
424424
),
425425
];
426426
},
427+
428+
/**
429+
* Whether the provided node represents an autofixer function.
430+
* @param {Node} node
431+
* @param {Node[]} contextIdentifiers
432+
* @returns {boolean}
433+
*/
434+
isAutoFixerFunction (node, contextIdentifiers) {
435+
const parent = node.parent;
436+
return ['FunctionExpression', 'ArrowFunctionExpression'].includes(node.type) &&
437+
parent.parent.type === 'ObjectExpression' &&
438+
parent.parent.parent.type === 'CallExpression' &&
439+
contextIdentifiers.has(parent.parent.parent.callee.object) &&
440+
parent.parent.parent.callee.property.name === 'report' &&
441+
this.getReportInfo(parent.parent.parent.arguments).fix === node;
442+
},
443+
444+
/**
445+
* Whether the provided node represents a suggestion fixer function.
446+
* @param {Node} node
447+
* @param {Node[]} contextIdentifiers
448+
* @returns {boolean}
449+
*/
450+
isSuggestionFixerFunction (node, contextIdentifiers) {
451+
const parent = node.parent;
452+
return (node.type === 'FunctionExpression' || node.type === 'ArrowFunctionExpression') &&
453+
parent.type === 'Property' &&
454+
parent.key.type === 'Identifier' &&
455+
parent.key.name === 'fix' &&
456+
parent.parent.type === 'ObjectExpression' &&
457+
parent.parent.parent.type === 'ArrayExpression' &&
458+
parent.parent.parent.parent.type === 'Property' &&
459+
parent.parent.parent.parent.key.type === 'Identifier' &&
460+
parent.parent.parent.parent.key.name === 'suggest' &&
461+
parent.parent.parent.parent.parent.type === 'ObjectExpression' &&
462+
parent.parent.parent.parent.parent.parent.type === 'CallExpression' &&
463+
contextIdentifiers.has(parent.parent.parent.parent.parent.parent.callee.object) &&
464+
parent.parent.parent.parent.parent.parent.callee.property.name === 'report' &&
465+
this.getReportInfo(parent.parent.parent.parent.parent.parent.arguments).suggest === parent.parent.parent;
466+
},
427467
};

‎tests/lib/rules/fixer-return.js

+87
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,55 @@ ruleTester.run('fixer-return', rule, {
221221
}
222222
};
223223
`,
224+
225+
// Suggestion
226+
`
227+
module.exports = {
228+
create: function(context) {
229+
context.report( {
230+
suggest: [
231+
{
232+
fix: function(fixer) {
233+
return fixer.foo();
234+
}
235+
}
236+
]
237+
});
238+
}
239+
};
240+
`,
241+
// Suggestion but wrong `suggest` key
242+
`
243+
module.exports = {
244+
create: function(context) {
245+
context.report( {
246+
notSuggest: [
247+
{
248+
fix: function(fixer) {
249+
fixer.foo();
250+
}
251+
}
252+
]
253+
});
254+
}
255+
};
256+
`,
257+
// Suggestion but wrong `fix` key
258+
`
259+
module.exports = {
260+
create: function(context) {
261+
context.report( {
262+
suggest: [
263+
{
264+
notFix: function(fixer) {
265+
fixer.foo();
266+
}
267+
}
268+
]
269+
});
270+
}
271+
};
272+
`,
224273
],
225274

226275
invalid: [
@@ -239,6 +288,25 @@ ruleTester.run('fixer-return', rule, {
239288
`,
240289
errors: [{ messageId: 'missingFix', type: 'FunctionExpression', line: 5, column: 24 }],
241290
},
291+
{
292+
// Fix but missing return (suggestion)
293+
code: `
294+
module.exports = {
295+
create: function(context) {
296+
context.report({
297+
suggest: [
298+
{
299+
fix(fixer) {
300+
fixer.foo();
301+
}
302+
}
303+
]
304+
});
305+
}
306+
};
307+
`,
308+
errors: [{ messageId: 'missingFix', type: 'FunctionExpression', line: 7, column: 36 }],
309+
},
242310
{
243311
// Fix but missing return (arrow function, report on arrow)
244312
code: `
@@ -254,6 +322,25 @@ ruleTester.run('fixer-return', rule, {
254322
`,
255323
errors: [{ messageId: 'missingFix', type: 'ArrowFunctionExpression', line: 5, endLine: 5, column: 34, endColumn: 36 }],
256324
},
325+
{
326+
// Fix but missing return (arrow function, report on arrow, suggestion)
327+
code: `
328+
module.exports = {
329+
create: function(context) {
330+
context.report({
331+
suggest: [
332+
{
333+
fix: (fixer) => {
334+
fixer.foo();
335+
}
336+
}
337+
]
338+
});
339+
}
340+
};
341+
`,
342+
errors: [{ messageId: 'missingFix', type: 'ArrowFunctionExpression', line: 7, endLine: 7, column: 46, endColumn: 48 }],
343+
},
257344
{
258345
// With no autofix (arrow function, explicit return, report on arrow)
259346
code: `

‎tests/lib/rules/prefer-replace-text.js

+42-7
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@ const RuleTester = require('eslint').RuleTester;
1717
// ------------------------------------------------------------------------------
1818

1919
const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 6 } });
20-
const ERROR = { messageId: 'useReplaceText', type: 'CallExpression' };
21-
2220

2321
ruleTester.run('prefer-placeholders', rule, {
2422
valid: [
@@ -52,6 +50,23 @@ ruleTester.run('prefer-placeholders', rule, {
5250
`
5351
fixer.replaceTextRange([node.range[0], node.range[1]], '');
5452
`,
53+
54+
// Suggestion
55+
`
56+
module.exports = {
57+
create(context) {
58+
context.report({
59+
suggest: [
60+
{
61+
fix(fixer) {
62+
return fixer.replaceTextRange([start, end], '');
63+
}
64+
}
65+
]
66+
});
67+
}
68+
};
69+
`,
5570
],
5671

5772
invalid: [
@@ -67,7 +82,7 @@ ruleTester.run('prefer-placeholders', rule, {
6782
}
6883
};
6984
`,
70-
errors: [ERROR],
85+
errors: [{ messageId: 'useReplaceText', type: 'CallExpression' }],
7186
},
7287
{
7388
code: `
@@ -81,7 +96,7 @@ ruleTester.run('prefer-placeholders', rule, {
8196
}
8297
};
8398
`,
84-
errors: [ERROR],
99+
errors: [{ messageId: 'useReplaceText', type: 'CallExpression' }],
85100
},
86101
{
87102
code: `
@@ -95,7 +110,7 @@ ruleTester.run('prefer-placeholders', rule, {
95110
}
96111
};
97112
`,
98-
errors: [ERROR],
113+
errors: [{ messageId: 'useReplaceText', type: 'CallExpression' }],
99114
},
100115
{
101116
code: `
@@ -107,7 +122,7 @@ ruleTester.run('prefer-placeholders', rule, {
107122
}
108123
};
109124
`,
110-
errors: [ERROR],
125+
errors: [{ messageId: 'useReplaceText', type: 'CallExpression' }],
111126
},
112127
{
113128
code: `
@@ -121,7 +136,27 @@ ruleTester.run('prefer-placeholders', rule, {
121136
}
122137
};
123138
`,
124-
errors: [ERROR],
139+
errors: [{ messageId: 'useReplaceText', type: 'CallExpression' }],
140+
},
141+
142+
{
143+
// Suggestion fixer
144+
code: `
145+
module.exports = {
146+
create(context) {
147+
context.report({
148+
suggest: [
149+
{
150+
fix(fixer) {
151+
return fixer.replaceTextRange([node.range[0], node.range[1]], '');
152+
}
153+
}
154+
]
155+
});
156+
}
157+
};
158+
`,
159+
errors: [{ messageId: 'useReplaceText', type: 'CallExpression' }],
125160
},
126161
],
127162
});

‎tests/lib/utils.js

+73
Original file line numberDiff line numberDiff line change
@@ -471,4 +471,77 @@ describe('utils', () => {
471471
}
472472
});
473473
});
474+
475+
describe('isAutoFixerFunction', () => {
476+
const CASES = {
477+
'context.report({ fix(fixer) {} });' (ast) {
478+
return { expected: true, node: ast.body[0].expression.arguments[0].properties[0].value, context: ast.body[0].expression.callee.object };
479+
},
480+
'context.notReport({ fix(fixer) {} });' (ast) {
481+
return { expected: false, node: ast.body[0].expression.arguments[0].properties[0].value, context: ast.body[0].expression.callee.object };
482+
},
483+
'context.report({ notFix(fixer) {} });' (ast) {
484+
return { expected: false, node: ast.body[0].expression.arguments[0].properties[0].value, context: ast.body[0].expression.callee.object };
485+
},
486+
'notContext.report({ notFix(fixer) {} });' (ast) {
487+
return { expected: false, node: ast.body[0].expression.arguments[0].properties[0].value, context: undefined };
488+
},
489+
};
490+
491+
Object.keys(CASES).forEach(ruleSource => {
492+
it(ruleSource, () => {
493+
const ast = espree.parse(ruleSource, { ecmaVersion: 6, range: true });
494+
495+
// Add parent to each node.
496+
estraverse.traverse(ast, {
497+
enter (node, parent) {
498+
node.parent = parent;
499+
},
500+
});
501+
502+
const testCase = CASES[ruleSource](ast);
503+
const contextIdentifiers = new Set([testCase.context]);
504+
const result = utils.isAutoFixerFunction(testCase.node, contextIdentifiers);
505+
assert.strictEqual(result, testCase.expected);
506+
});
507+
});
508+
});
509+
510+
describe('isSuggestionFixerFunction', () => {
511+
const CASES = {
512+
'context.report({ suggest: [{ fix(fixer) {} }] });' (ast) {
513+
return { expected: true, node: ast.body[0].expression.arguments[0].properties[0].value.elements[0].properties[0].value, context: ast.body[0].expression.callee.object };
514+
},
515+
'context.notReport({ suggest: [{ fix(fixer) {} }] });' (ast) {
516+
return { expected: false, node: ast.body[0].expression.arguments[0].properties[0].value.elements[0].properties[0].value, context: ast.body[0].expression.callee.object };
517+
},
518+
'context.report({ notSuggest: [{ fix(fixer) {} }] });' (ast) {
519+
return { expected: false, node: ast.body[0].expression.arguments[0].properties[0].value.elements[0].properties[0].value, context: ast.body[0].expression.callee.object };
520+
},
521+
'context.report({ suggest: [{ notFix(fixer) {} }] });' (ast) {
522+
return { expected: false, node: ast.body[0].expression.arguments[0].properties[0].value.elements[0].properties[0].value, context: ast.body[0].expression.callee.object };
523+
},
524+
'notContext.report({ suggest: [{ fix(fixer) {} }] });' (ast) {
525+
return { expected: false, node: ast.body[0].expression.arguments[0].properties[0].value, context: undefined };
526+
},
527+
};
528+
529+
Object.keys(CASES).forEach(ruleSource => {
530+
it(ruleSource, () => {
531+
const ast = espree.parse(ruleSource, { ecmaVersion: 6, range: true });
532+
533+
// Add parent to each node.
534+
estraverse.traverse(ast, {
535+
enter (node, parent) {
536+
node.parent = parent;
537+
},
538+
});
539+
540+
const testCase = CASES[ruleSource](ast);
541+
const contextIdentifiers = new Set([testCase.context]);
542+
const result = utils.isSuggestionFixerFunction(testCase.node, contextIdentifiers);
543+
assert.strictEqual(result, testCase.expected);
544+
});
545+
});
546+
});
474547
});

0 commit comments

Comments
 (0)