New: fixer-return #15
Conversation
aladdin-add
force-pushed
the
issue6
branch
2 times, most recently
from
74ec9a1 to
3c96706
Compare
.eslintrc.yml
Outdated
| @@ -11,3 +11,6 @@ rules: | |||
| self/report-message-format: | |||
| - error | |||
| - '^[^a-z].*\.$' | |||
| node/no-unpublished-require: | |||
| - error | |||
| - allowModules: ['espree', 'esutils'] | |||
There was a problem hiding this comment.
If we depend on espree and esutils, then I think we should explicitly list them as dependencies, in case eslint stops depending on them at some point.
lib/ast-utils.js
Outdated
| @@ -0,0 +1,1307 @@ | |||
| /** | |||
There was a problem hiding this comment.
I don't think we should copy all of ast-utils.js. That seems like it will be very hard to maintain.
Can utils.getKeyName be used instead of astUtils.getStaticPropertyName?
lib/rules/fixer-return.js
Outdated
| codePath, | ||
| hasReturn: false, | ||
| shouldCheck: | ||
| node.type === 'FunctionExpression' && |
There was a problem hiding this comment.
I don't think this should check all functions that are named fix, because some of them might not be related to eslint. It would be better to use getContextIdentifiers and getReportInfo to only check fix functions in context.report calls.
There was a problem hiding this comment.
we need code path analysis here. so I'm using an inelegant way, rather than using getReportInfo.
am I missing something?
|
@not-an-aardvark sorry about that I'm on a trip, so maybe need a few days to fix this. |
|
No problem! Enjoy your trip. |
lib/rules/fixer-return.js
Outdated
| node.body.type === 'BlockStatement' && | ||
| // check if it is context.report({fix}) | ||
| utils.getKeyName(parent) === 'fix' && | ||
| parent.parent.type === 'ObjectExpression' && |
There was a problem hiding this comment.
This will fail for the old-style context.report calls:
context.report(node, 'foo', {}, fixer => {});You can use utils.getReportInfo to get info about a report.
I think the best way to do it would be to use something like this:
const shouldCheck = node.type === 'FunctionExpression' &&
- node.body.type === 'BlockStatement' &&
- // check if it is context.report({fix})
- utils.getKeyName(parent) === 'fix' &&
parent.parent.type === 'ObjectExpression' &&
parent.parent.parent.type === 'CallExpression' &&
contextIdentifiers.has(parent.parent.parent.callee.object) &&
- parent.parent.parent.callee.property.name === 'report';
+ parent.parent.parent.callee.property.name === 'report' &&
+ utils.getReportInfo(parent.parent.parent.arguments).fix === node;
docs/rules/fixer-return.md
Outdated
|
|
||
| ## Rule Details | ||
|
|
||
| This rule enforces always return from a fixer function. |
There was a problem hiding this comment.
Nitpick: I think it would be better to word this as "This rule enforces that fixer functions always return a value."
fixes #3