Autofix rules

[sindresorhus]

Would be useful to support autofixing for some of the rules. Not all are possible.

http://eslint.org/docs/developer-guide/working-with-rules#contextreport

• no-skip-test (just remove the skip modifier)
• no-only-test (just remove the only modifier)
• no-identical-title (append Discussion #1, no-cb-test rule #2, etc, to the title)
• no-skip-assert (just remove the skip modifier)

Can't think of any way to reliably fix the other rules, but input welcome!

[jfmengels]

👍 Great idea.

Not sure about no-identical-title though, as they won't really make things more readable in the source code (only when matching AVA's output to a test). Don't forget that the rule can also be triggered because there is no title.

[sindresorhus]

Not sure about no-identical-title though, as they won't really make things more readable in the source code (only when matching AVA's output to a test).

Appending a #n does make it clear it's testing the same thing just split up for readability, but I don't feel strongly about it, and is a pretty edge-case anyways.

[jamestalmage]

W/regard to the no-skip-* rules, I think auto fixing those is problematic. It will likely make the tests fail. If I am running xo --fix to clear up some whitespace / semicolon mistakes - I probably don't want you messing with my tests/assertions.

I don't think we should auto-fix anything that modifies code behavior (unless maybe the user sets some other CLI option or environment variable? FIX_AVA=true xo --fix.

One exception to the above. I think removing only is fine. Users should never check in .only tests, but .skip is acceptable in a number of situations (i.e. somebody has submitted a PR with a failing test but not the fix itself).

[sindresorhus]

But without it will make XO fail. So it will still fail.

[jamestalmage]

Does --fix only fix errors, or warnings too?

[sindresorhus]

Warnings too.

[florianb]

@sindresorhus: i would like to give it a try if it is still relevant.

[sindresorhus]

@florianb Sure. Go ahead. Seems like we couldn't agree on all of these yet, but you could start with autofixing for no-only-test :) There might also be other rules not mentioned here that could potentially have autofixing. Feel free to look into that too.

[florianb]

Great - i am sure a PR will stimulate discussions.. 😄

[florianb]

Hey everybody, i am stuck testing the fixResult.output since error.actual does not get printed. If i am not wrong this needs to be fixed in https://github.com/jfmengels/eslint-ava-rule-tester - i would be very happy to fix that if a contributor could check my assumption and approve that a PR for that would be welcome.

Thank you very much! 💝

[sindresorhus]

@florianb Just apply the fix mention in jfmengels/eslint-ava-rule-tester#5 locally to your node_modules and you should be able to debug it for now.

[florianb]

@sindresorhus i guess this fix is not applicable since eslint forbids the modification of the AST. While it it would (probably) be possible to rename .only into something different, i can't remove it without causing the following exception:

Rule should not modify AST.

Actual:
[object Object]

Expected:
[object Object]

assertASTDidntChange (node_modules/eslint/lib/testers/rule-tester.js:406:24)
...

I opened a corresponding SO-question without helpful responses. I guess i could start a bounty to raise attention but i can't think of any solution overcoming this barrier.

I am really sorry that i just got stuck again. 😞