Skip to content

Commit 2c08674

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

File tree

6 files changed

+243
-24
lines changed

6 files changed

+243
-24
lines changed

‎lib/rules/fixer-return.js

Lines changed: 1 addition & 11 deletions
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

Lines changed: 1 addition & 9 deletions
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

Lines changed: 44 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -398,10 +398,10 @@ module.exports = {
398398
},
399399

400400
/**
401-
* Collect all context.report({...}) violation/suggestion-related nodes into a standardized array for convenience.
402-
* @param {Object} reportInfo - Result of getReportInfo().
403-
* @returns {messageId?: String, message?: String, data?: Object, fix?: Function}[]
404-
*/
401+
* Collect all context.report({...}) violation/suggestion-related nodes into a standardized array for convenience.
402+
* @param {Object} reportInfo - Result of getReportInfo().
403+
* @returns {messageId?: String, message?: String, data?: Object, fix?: Function}[]
404+
*/
405405
collectReportViolationAndSuggestionData (reportInfo) {
406406
return [
407407
// Violation message
@@ -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

Lines changed: 87 additions & 0 deletions
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

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,23 @@ ruleTester.run('prefer-placeholders', rule, {
5252
`
5353
fixer.replaceTextRange([node.range[0], node.range[1]], '');
5454
`,
55+
56+
// Suggestion
57+
`
58+
module.exports = {
59+
create(context) {
60+
context.report({
61+
suggest: [
62+
{
63+
fix(fixer) {
64+
return fixer.replaceTextRange([start, end], '');
65+
}
66+
}
67+
]
68+
});
69+
}
70+
};
71+
`,
5572
],
5673

5774
invalid: [
@@ -123,5 +140,25 @@ ruleTester.run('prefer-placeholders', rule, {
123140
`,
124141
errors: [ERROR],
125142
},
143+
144+
{
145+
// Suggestion fixer
146+
code: `
147+
module.exports = {
148+
create(context) {
149+
context.report({
150+
suggest: [
151+
{
152+
fix(fixer) {
153+
return fixer.replaceTextRange([node.range[0], node.range[1]], '');
154+
}
155+
}
156+
]
157+
});
158+
}
159+
};
160+
`,
161+
errors: [ERROR],
162+
},
126163
],
127164
});

‎tests/lib/utils.js

Lines changed: 73 additions & 0 deletions
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)