Update: allow autofixing when using processors (fixes #7510) (#9090)

This implements the proposal from #7510 (comment) to allow autofixes to be applied when using processors.

The processor API basically remains the same. The only changes are:

* Processors can now export a `supportsAutofix: true` property, which opts a processor into autofixing.
* If a processor supports autofixing, the `postprocess` method is expected to also transform autofix ranges as necessary when it transforms the locations of reported problems. Afterwards, the transformed fixes are applied to the original, unprocessed text.

Multipass autofixing works by calling `preprocess` and `postprocess` for each pass.

docs/developer-guide/nodejs-api.md

@@ -67,6 +67,8 @@ The most important method on `Linter` is `verify()`, which initiates linting of
     * **Note**: If you want to lint text and have your configuration be read and processed, use CLIEngine's [`executeOnFiles`](#executeonfiles) or [`executeOnText`](#executeontext) instead.
 * `options` - (optional) Additional options for this run.
     * `filename` - (optional) the filename to associate with the source code.
+    * `preprocess` - (optional) A function that accepts a string containing source text, and returns an array of strings containing blocks of code to lint. Also see: [Processors in Plugins](/docs/developer-guide/working-with-plugins#processors-in-plugins)
+    * `postprocess` - (optional) A function that accepts an array of problem lists (one list of problems for each block of code from `preprocess`), and returns a one-dimensional array of problems containing problems for the original, unprocessed text. Also see: [Processors in Plugins](/docs/developer-guide/working-with-plugins#processors-in-plugins)
     * `allowInlineConfig` - (optional) set to `false` to disable inline comments from changing eslint rules.
 
 If the third argument is a string, it is interpreted as the `filename`.

docs/developer-guide/working-with-plugins.md

@@ -72,15 +72,45 @@ processors: {
 
             // you need to return a one-dimensional array of the messages you want to keep
             return [Message];
-        }
+        },
+
+        supportsAutofix: true // (optional, defaults to false)
     }
 }
 ```
 
 The `preprocess` method takes the file contents and filename as arguments, and returns an array of strings to lint. The strings will be linted separately but still be registered to the filename. It's up to the plugin to decide if it needs to return just one part, or multiple pieces. For example in the case of processing `.html` files, you might want to return just one item in the array by combining all scripts, but for `.md` file where each JavaScript block might be independent, you can return multiple items.
 
-The `postprocess` method takes a two-dimensional array of arrays of lint messages and the filename. Each item in the input
-array corresponds to the part that was returned from the `preprocess` method. The `postprocess` method must adjust the location of all errors and aggregate them into a single flat array and return it.
+The `postprocess` method takes a two-dimensional array of arrays of lint messages and the filename. Each item in the input array corresponds to the part that was returned from the `preprocess` method. The `postprocess` method must adjust the locations of all errors to correspond to locations in the original, unprocessed code, and aggregate them into a single flat array and return it.
+
+Reported problems have the following location information:
+
+```typescript
+{
+    line: number,
+    column: number,
+
+    endLine?: number,
+    endColumn?: number
+}
+```
+
+By default, ESLint will not perform autofixes when a processor is used, even when the `--fix` flag is enabled on the command line. To allow ESLint to autofix code when using your processor, you should take the following additional steps:
+
+1. Update the `postprocess` method to additionally transform the `fix` property of reported problems. All autofixable problems will have a `fix` property, which is an object with the following schema:
+
+    ```js
+    {
+        range: [number, number],
+        text: string
+    }
+    ```
+
+    The `range` property contains two indexes in the code, referring to the start and end location of a contiguous section of text that will be replaced. The `text` property refers to the text that will replace the given range.
+
+    In the initial list of problems, the `fix` property will refer refer to a fix in the processed JavaScript. The `postprocess` method should transform the object to refer to a fix in the original, unprocessed file.
+
+2. Add a `supportsAutofix: true` property to the processor.
 
 You can have both rules and processors in a single plugin. You can also have multiple processors in one plugin.
 To support multiple extensions, add each one to the `processors` element and point them to the same object.

docs/user-guide/command-line-interface.md

@@ -359,7 +359,7 @@ The resulting configuration file will be created in the current directory.
 This option instructs ESLint to try to fix as many issues as possible. The fixes are made to the actual files themselves and only the remaining unfixed issues are output. Not all problems are fixable using this option, and the option does not work in these situations:
 
 1. This option throws an error when code is piped to ESLint.
-1. This option has no effect on code that uses processors.
+1. This option has no effect on code that uses a processor, unless the processor opts into allowing autofixes.
 
 #### `--debug`
 

lib/cli-engine.js

@@ -143,10 +143,8 @@ function calculateStatsPerRun(results) {
  */
 function processText(text, configHelper, filename, fix, allowInlineConfig, linter) {
     let filePath,
-        messages,
         fileExtension,
-        processor,
-        fixedResult;
+        processor;
 
     if (filename) {
         filePath = path.resolve(filename);
@@ -170,51 +168,28 @@ function processText(text, configHelper, filename, fix, allowInlineConfig, linte
         }
     }
 
-    if (processor) {
-        debug("Using processor");
-        const parsedBlocks = processor.preprocess(text, filename);
-        const unprocessedMessages = [];
+    const autofixingEnabled = typeof fix !== "undefined" && (!processor || processor.supportsAutofix);
 
-        parsedBlocks.forEach(block => {
-            unprocessedMessages.push(linter.verify(block, config, {
-                filename,
-                allowInlineConfig
-            }));
-        });
-
-        // TODO(nzakas): Figure out how fixes might work for processors
-
-        messages = processor.postprocess(unprocessedMessages, filename);
-
-    } else {
-
-        if (fix) {
-            fixedResult = linter.verifyAndFix(text, config, {
-                filename,
-                allowInlineConfig,
-                fix
-            });
-            messages = fixedResult.messages;
-        } else {
-            messages = linter.verify(text, config, {
-                filename,
-                allowInlineConfig
-            });
-        }
-    }
+    const fixedResult = linter.verifyAndFix(text, config, {
+        filename,
+        allowInlineConfig,
+        fix: !!autofixingEnabled && fix,
+        preprocess: processor && (rawText => processor.preprocess(rawText, filename)),
+        postprocess: processor && (problemLists => processor.postprocess(problemLists, filename))
+    });
 
-    const stats = calculateStatsPerFile(messages);
+    const stats = calculateStatsPerFile(fixedResult.messages);
 
     const result = {
         filePath: filename,
-        messages,
+        messages: fixedResult.messages,
         errorCount: stats.errorCount,
         warningCount: stats.warningCount,
         fixableErrorCount: stats.fixableErrorCount,
         fixableWarningCount: stats.fixableWarningCount
     };
 
-    if (fixedResult && fixedResult.fixed) {
+    if (fixedResult.fixed) {
         result.output = fixedResult.output;
     }
 

lib/linter.js

@@ -12,6 +12,7 @@
 const EventEmitter = require("events").EventEmitter,
     eslintScope = require("eslint-scope"),
     levn = require("levn"),
+    lodash = require("lodash"),
     blankScriptAST = require("../conf/blank-script.json"),
     defaultConfig = require("../conf/default-config-options.js"),
     CodePathAnalyzer = require("./code-path-analysis/code-path-analyzer"),
@@ -721,7 +722,7 @@ module.exports = class Linter {
      */
 
     /**
-     * Verifies the text against the rules specified by the second argument.
+     * Same as linter.verify, except without support for processors.
      * @param {string|SourceCode} textOrSourceCode The text to parse or a SourceCode object.
      * @param {ESLintConfig} config An ESLintConfig instance to configure everything.
      * @param {(string|Object)} [filenameOrOptions] The optional filename of the file being checked.
@@ -731,7 +732,7 @@ module.exports = class Linter {
      *      Useful if you want to validate JS without comments overriding rules.
      * @returns {Object[]} The results as an array of messages or null if no messages.
      */
-    verify(textOrSourceCode, config, filenameOrOptions) {
+    _verifyWithoutProcessors(textOrSourceCode, config, filenameOrOptions) {
         let text,
             parserServices,
             allowInlineConfig,
@@ -965,6 +966,35 @@ module.exports = class Linter {
         });
     }
 
+    /**
+     * Verifies the text against the rules specified by the second argument.
+     * @param {string|SourceCode} textOrSourceCode The text to parse or a SourceCode object.
+     * @param {ESLintConfig} config An ESLintConfig instance to configure everything.
+     * @param {(string|Object)} [filenameOrOptions] The optional filename of the file being checked.
+     *      If this is not set, the filename will default to '<input>' in the rule context. If
+     *      an object, then it has "filename", "saveState", and "allowInlineConfig" properties.
+     * @param {boolean} [saveState] Indicates if the state from the last run should be saved.
+     *      Mostly useful for testing purposes.
+     * @param {boolean} [filenameOrOptions.allowInlineConfig] Allow/disallow inline comments' ability to change config once it is set. Defaults to true if not supplied.
+     *      Useful if you want to validate JS without comments overriding rules.
+     * @param {function(string): string[]} [filenameOrOptions.preprocess] preprocessor for source text. If provided,
+     *      this should accept a string of source text, and return an array of code blocks to lint.
+     * @param {function(Array<Object[]>): Object[]} [filenameOrOptions.postprocess] postprocessor for report messages. If provided,
+     *      this should accept an array of the message lists for each code block returned from the preprocessor,
+     *      apply a mapping to the messages as appropriate, and return a one-dimensional array of messages
+     * @returns {Object[]} The results as an array of messages or null if no messages.
+     */
+    verify(textOrSourceCode, config, filenameOrOptions) {
+        const preprocess = filenameOrOptions && filenameOrOptions.preprocess || (rawText => [rawText]);
+        const postprocess = filenameOrOptions && filenameOrOptions.postprocess || lodash.flatten;
+
+        return postprocess(
+            preprocess(textOrSourceCode).map(
+                textBlock => this._verifyWithoutProcessors(textBlock, config, filenameOrOptions)
+            )
+        );
+    }
+
     /**
      * Gets the SourceCode object representing the parsed source.
      * @returns {SourceCode} The SourceCode object.
@@ -1012,6 +1042,11 @@ module.exports = class Linter {
      * @param {boolean} options.allowInlineConfig Flag indicating if inline comments
      *      should be allowed.
      * @param {boolean|Function} options.fix Determines whether fixes should be applied
+     * @param {Function} options.preprocess preprocessor for source text. If provided, this should
+     *      accept a string of source text, and return an array of code blocks to lint.
+     * @param {Function} options.postprocess postprocessor for report messages. If provided,
+     *      this should accept an array of the message lists for each code block returned from the preprocessor,
+     *      apply a mapping to the messages as appropriate, and return a one-dimensional array of messages
      * @returns {Object} The result of the fix operation as returned from the
      *      SourceCodeFixer.
      */

tests/lib/cli-engine.js

@@ -2478,6 +2478,94 @@ describe("CLIEngine", () => {
                 assert.equal(report.results[0].messages[0].message, "'b' is defined but never used.");
                 assert.equal(report.results[0].messages[0].ruleId, "post-processed");
             });
+
+            describe("autofixing with processors", () => {
+                const HTML_PROCESSOR = Object.freeze({
+                    preprocess(text) {
+                        return [text.replace(/^<script>/, "").replace(/<\/script>$/, "")];
+                    },
+                    postprocess(problemLists) {
+                        return problemLists[0].map(problem => {
+                            if (problem.fix) {
+                                const updatedFix = Object.assign({}, problem.fix, {
+                                    range: problem.fix.range.map(index => index + "<script>".length)
+                                });
+
+                                return Object.assign({}, problem, { fix: updatedFix });
+                            }
+                            return problem;
+                        });
+                    }
+                });
+
+
+                it("should run in autofix mode when using a processor that supports autofixing", () => {
+                    engine = new CLIEngine({
+                        useEslintrc: false,
+                        plugins: ["test-processor"],
+                        rules: {
+                            semi: 2
+                        },
+                        extensions: ["js", "txt"],
+                        ignore: false,
+                        fix: true
+                    });
+
+                    engine.addPlugin("test-processor", {
+                        processors: {
+                            ".html": Object.assign({ supportsAutofix: true }, HTML_PROCESSOR)
+                        }
+                    });
+
+                    const report = engine.executeOnText("<script>foo</script>", "foo.html");
+
+                    assert.strictEqual(report.results[0].messages.length, 0);
+                    assert.strictEqual(report.results[0].output, "<script>foo;</script>");
+                });
+
+                it("should not run in autofix mode when using a processor that does not support autofixing", () => {
+                    engine = new CLIEngine({
+                        useEslintrc: false,
+                        plugins: ["test-processor"],
+                        rules: {
+                            semi: 2
+                        },
+                        extensions: ["js", "txt"],
+                        ignore: false,
+                        fix: true
+                    });
+
+                    engine.addPlugin("test-processor", { processors: { ".html": HTML_PROCESSOR } });
+
+                    const report = engine.executeOnText("<script>foo</script>", "foo.html");
+
+                    assert.strictEqual(report.results[0].messages.length, 1);
+                    assert.isFalse(Object.prototype.hasOwnProperty.call(report.results[0], "output"));
+                });
+
+                it("should not run in autofix mode when `fix: true` is not provided, even if the processor supports autofixing", () => {
+                    engine = new CLIEngine({
+                        useEslintrc: false,
+                        plugins: ["test-processor"],
+                        rules: {
+                            semi: 2
+                        },
+                        extensions: ["js", "txt"],
+                        ignore: false
+                    });
+
+                    engine.addPlugin("test-processor", {
+                        processors: {
+                            ".html": Object.assign({ supportsAutofix: true }, HTML_PROCESSOR)
+                        }
+                    });
+
+                    const report = engine.executeOnText("<script>foo</script>", "foo.html");
+
+                    assert.strictEqual(report.results[0].messages.length, 1);
+                    assert.isFalse(Object.prototype.hasOwnProperty.call(report.results[0], "output"));
+                });
+            });
         });
     });