Allow more control over fixes in CLIEngine

[IanVS]

I'll talk a little about the use case I'm attempting to solve, and then my proposed solution:

Problem

Currently, it is not a simple matter to tell ESLint to apply fixes from only one rule, or to disable particular rules from fixes. Per-rule autofixing is being discussed in #8018, and @ljharb mentioned his use case of fixing one single rule at a time. In a subsequent chat discussion, I learned that Airbnb also uses inline config to change some rules to warn severity, and he would like to avoid applying fixes to those lines. Currently, there is no built-in way that I know of for ESLint to support this use case.

Potential Solution

I would like to write a CLI tool (possibly an enhancement to eslint-nibble which would allow users to autofix one single rule at a time, while still respecting the rest of the eslint configuration in their .eslintrc file. Furthermore, I would like to allow an option to not fix messages with a severity of 1.

Unfortunately, this is not currently possible. To apply fixes with CLIEngine, it's necessary to configure it with fix: true. This adds an output field to the results, which CLIEngine.outputFixes() applies to the filesystem. However, the creation of the error messages and output occur together, when calling CLIEngine.executeOnFiles or CLIEngine.executeOnText.

Proposal

My proposal is to allow users to modify the messages within report.results (for example, to strip out any messages with severity: 1 or all except a particular rule), before fixes are applied.

I have experimented with adding an applyFixesToReport method to CLIEngine, which would accept a report object, do a little bit of work to get sourceFile objects using our existing utilities, and then call SourceCodeFixer.applyFixes(), returning a new report including an output string if fixes were applied from the messages in the originally supplied report.

The changes to ESLint are pretty minimal using this approach, and would allow much more flexibility in the fixes that ESLint performs. I'd be happy to submit a PR if this suggestion is approved.

[ilyavolodin]

While I'm not against this proposal, in fact, I proposed exactly the same thing here: #8018 (comment) if you are writing a stand-alone tool for this, you can do this by not using CLIEngine, but instead using child_process and pure CLI + --no-eslintrc and --rule flags.

[IanVS]

@ilyavolodin unfortunately it's not quite that easy. .eslintrc contains more than just rules, including parser configurations, and --no-eslintrc will wipe all of that out. Also, you wouldn't be able to avoid fixing warnings.

[nzakas]

What if fix could be a function that returns true if a passed-in message should be fixed?

[IanVS]

I like that idea, @nzakas. It would also prevent one of the issues I was thinking about, which is that my approach would not be terribly efficient.

[IanVS]

Can I move forward with a PR on this issue? Are there any objections to allowing the fix option to be either a boolean or a function, as @nzakas suggested?

[ilyavolodin]

Sorry, I might be slightly slow, but I'm not 100% sure I understand the proposed change for fix. @IanVS would you be able to provide a very simple sample? Or maybe even point me to the variable in the current source code that is proposed to be changed into a function?

[IanVS]

I'm talking about expanding what can be provided as the fix property when creating a CLIEngine (http://eslint.org/docs/developer-guide/nodejs-api#cliengine). Right now, it can only be true if you want ESLint to calculate fixes and create an output string. The proposal is to allow submitting a function which would be given each message in turn, and return true/false depending on if that particular message should be attempted to be fixed.

[IanVS]

Here's one of the places this is used: https://github.com/eslint/eslint/blob/master/lib/cli-engine.js#L201.

[ilyavolodin]

I see. In that case, I would support this enhancement. We should also make this backwards compatible, not to break existing integrations.

[platinumazure]

I'm interested in implementing this, since this could be leveraged to fix #8675. If anyone has any objections or concerns, let me know here, otherwise I'll probably work on this in the next few days.

[IanVS]

@platinumazure if you haven't started yet, I have a branch locally with a good start at this, I just never took the time to finish it up. I'm not sure if I can take it across the finish line in the next few days, but I can at least push up the branch today or tomorrow if you want to check it out.

[platinumazure]

@IanVS Thanks, I'd love to see your branch.

[IanVS]

@platinumazure I've opened a quick PR, as it seems closer than I remembered.