Fix: ESLint cache no longer stops autofix (fixes #10679)

[platinumazure]

What is the purpose of this pull request? (put an "X" next to item)

[ ] Documentation update
[x] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofixing to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:

See issue #10679.

What changes did you make? (Give an overview)

Changed CLIEngine so that it will not always used cached lint results. Specifically, if the cached lint result had errors and autofix has been turned on for the current CLIEngine run, CLIEngine will ignore the cached results and process the file normally.

This fixes the following scenario:

1. Run ESLint with --cache flag and without --fix flag
2. Then run ESLint with --cache flag and --fix flag

Before this change, the cached, fixless results would be returned. With this change, the cache results are ignored and the file is processed and autofixed.

Is there anything you'd like reviewers to focus on?

I'm realizing as I type this that maybe this could be made smarter if we look at fixableErrorCount and fixableWarningCount: if those are both 0 and the cache is still valid, then no autofix would need to be applied and we could use the cache results. Thoughts?

Other than that, are there any other test cases I should add?

[nstepien]

@platinumazure
I tested this locally just now and it works great!
One suggestion if I may: once ESLint is done autofixing all the files, the cache should be updated as well, so the next run won't have to work on the freshly fixed files again.

[platinumazure]

Thanks @MayhemYDG for testing, and I'm glad it worked!

One suggestion if I may: once ESLint is done autofixing all the files, the cache should be updated as well, so the next run won't have to work on the freshly fixed files again.

I think that's a great idea, but I don't think I want to include it in this pull request for a couple of reasons:

• Philosophically, this PR is about fixing a regression from a new feature. The feature in question wasn't intended to cache results of fixed files (although I agree it would be a worthwhile enhancement). So I don't think it belongs here.
• Architecturally, some changes would be required as well.

The architectural changes I'm talking about have to do with the fact that CLIEngine has separate APIs for linting (and calculating fix information for) text/files, vs applying the fix information to files on disk. At the moment, the latter API is a static method: when it was originally designed, there was no need for state information held by CLIEngine instances. So in order to implement the enhancement, one approach might be to add an instance method to CLIEngine which would apply fixes and update the cache associated within that instance, and have our CLI use that (and the static method could be deprecated, or left around as an "update without caching" option). But at this point, we would need to have a discussion among the team members for what makes the most sense architecturally.

Apologies for the long-winded answer, but I wanted to give a good idea the consideration of a proper explanation. I would love if you or I opened an enhancement request in a new issue to discuss further.

[nstepien]

Sounds fair to me. Getting fixes in early, and refinements later is never a bad idea.

[not-an-aardvark]

LGTM, thanks!