New: Add suggestions API (#12384)

* New: Initial suggestions support in the linter

* Docs: Update some docs with info about suggestions

* Chore: Add some tests for suggestions

* New: Add support for suggestion messageId-based descriptions

* New: WIP add suggestions for no-useless-escape

* Chore: Update no-useless-escape suggestions to use messageIds

* New: Support output option for rule-tester tests of suggestions

* Chore: Respond to PR feedback

* Chore: Update rule-tester suggestion testing capabilities and tests

* Fix: Bug in no-useless-escape suggestions

* Chore: Use messageId more for suggestion example

* Chore: Update rule-tester suggestion support

* Chore: Remove rule-tester support for testing fix object

docs/developer-guide/nodejs-api.md

@@ -108,7 +108,7 @@ The most important method on `Linter` is `verify()`, which initiates linting of
     * `preprocess` - (optional) A function that [Processors in Plugins](/docs/developer-guide/working-with-plugins.md#processors-in-plugins) documentation describes as the `preprocess` method.
     * `postprocess` - (optional) A function that [Processors in Plugins](/docs/developer-guide/working-with-plugins.md#processors-in-plugins) documentation describes as the `postprocess` method.
     * `filterCodeBlock` - (optional) A function that decides which code blocks the linter should adopt. The function receives two arguments. The first argument is the virtual filename of a code block. The second argument is the text of the code block. If the function returned `true` then the linter adopts the code block. If the function was omitted, the linter adopts only `*.js` code blocks. If you provided a `filterCodeBlock` function, it overrides this default behavior, so the linter doesn't adopt `*.js` code blocks automatically.
-    * `disableFixes` - (optional) when set to `true`, the linter doesn't make the `fix` property of the lint result.
+    * `disableFixes` - (optional) when set to `true`, the linter doesn't make either the `fix` or `suggestions` property of the lint result.
     * `allowInlineConfig` - (optional) set to `false` to disable inline comments from changing ESLint rules.
     * `reportUnusedDisableDirectives` - (optional) when set to `true`, adds reported errors for unused `eslint-disable` directives when no problems would be reported in the disabled area anyway.
 
@@ -170,6 +170,7 @@ The information available for each linting message is:
 * `endColumn` - the end column of the range on which the error occurred (this property is omitted if it's not range).
 * `endLine` - the end line of the range on which the error occurred (this property is omitted if it's not range).
 * `fix` - an object describing the fix for the problem (this property is omitted if no fix is available).
+* `suggestions` - an array of objects describing possible lint fixes for editors to programmatically enable (see details in the [Working with Rules docs](./working-with-rules.md#providing-suggestions)).
 
 Linting message objects have a deprecated `source` property. This property **will be removed** from linting messages in an upcoming breaking release. If you depend on this property, you should now use the `SourceCode` instance provided by the linter.
 
@@ -437,9 +438,23 @@ The return value is an object containing the results of the linting operation. H
                 column: 13,
                 nodeType: "ExpressionStatement",
                 fix: { range: [12, 12], text: ";" }
+            }, {
+                ruleId: "no-useless-escape",
+                severity: 1,
+                message: "disallow unnecessary escape characters",
+                line: 1,
+                column: 10,
+                nodeType: "ExpressionStatement",
+                suggestions: [{
+                    desc: "Remove unnecessary escape. This maintains the current functionality.",
+                    fix: { range: [9, 10], text: "" }
+                }, {
+                    desc: "Escape backslash to include it in the RegExp.",
+                    fix: { range: [9, 9], text: "\\" }
+                }]
             }],
             errorCount: 1,
-            warningCount: 0,
+            warningCount: 1,
             fixableErrorCount: 1,
             fixableWarningCount: 0,
             source: "\"use strict\"\n"
@@ -865,6 +880,7 @@ In addition to the properties above, invalid test cases can also have the follow
     * `column` (number): The 1-based column number of the reported location
     * `endLine` (number): The 1-based line number of the end of the reported location
     * `endColumn` (number): The 1-based column number of the end of the reported location
+    * `suggestions` (array): An array of objects with suggestion details to check. See [Testing Suggestions](#testing-suggestions) for details
 
     If a string is provided as an error instead of an object, the string is used to assert the `message` of the error.
 * `output` (string, optional): Asserts the output that will be produced when using this rule for a single pass of autofixing (e.g. with the `--fix` command line flag). If this is `null`, asserts that none of the reported problems suggest autofixes.
@@ -880,6 +896,31 @@ Any additional properties of a test case will be passed directly to the linter a
 
 If a valid test case only uses the `code` property, it can optionally be provided as a string containing the code, rather than an object with a `code` key.
 
+#### Testing Suggestions
+
+Suggestions can be tested by defining a `suggestions` key on an errors object. The options to check for the suggestions are the following (all are optional):
+    * `desc` (string): The suggestion `desc` value
+    * `messageId` (string): The suggestion `messageId` value for suggestions that use `messageId`s
+    * `output` (string): A code string representing the result of applying the suggestion fix to the input code
+
+Example:
+
+```js
+ruleTester.run("my-rule-for-no-foo", rule, {
+    valid: [],
+    invalid: [{
+        code: "var foo;",
+        errors: [{
+            suggestions: [{
+                desc: "Rename identifier 'foo' to 'bar'",
+                messageId: "renameFoo",
+                output: "var bar;"
+            }]
+        }]
+    }]
+})
+```
+
 ### Customizing RuleTester
 
 `RuleTester` depends on two functions to run tests: `describe` and `it`. These functions can come from various places:

docs/developer-guide/unit-tests.md

@@ -12,10 +12,10 @@ This automatically starts Mocha and runs all tests in the `tests` directory. You
 
 If you want to quickly run just one test, you can do so by running Mocha directly and passing in the filename. For example:
 
-    npm test:cli tests/lib/rules/no-wrap-func.js
+    npm run test:cli tests/lib/rules/no-wrap-func.js
 
 Running individual tests is useful when you're working on a specific bug and iterating on the solution. You should be sure to run `npm test` before submitting a pull request.
 
 ## More Control on Unit Testing
 
-`npm test:cli` is an alias of the Mocha cli in `./node_modules/.bin/mocha`. [Options](https://mochajs.org/#command-line-usage) are available to be provided to help to better control the test to run.
+`npm run test:cli` is an alias of the Mocha cli in `./node_modules/.bin/mocha`. [Options](https://mochajs.org/#command-line-usage) are available to be provided to help to better control the test to run.

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

@@ -62,6 +62,7 @@ The source file for a rule exports an object with the following properties.
     * `category` (string) specifies the heading under which the rule is listed in the [rules index](../rules/)
     * `recommended` (boolean) is whether the `"extends": "eslint:recommended"` property in a [configuration file](../user-guide/configuring.md#extending-configuration-files) enables the rule
     * `url` (string) specifies the URL at which the full documentation can be accessed
+    * `suggestion` (boolean) specifies whether rules can return suggestions (defaults to false if omitted)
 
     In a custom rule or plugin, you can omit `docs` or include any properties that you need in it.
 
@@ -344,6 +345,79 @@ Best practices for fixes:
 
     * This fixer can just select a quote type arbitrarily. If it guesses wrong, the resulting code will be automatically reported and fixed by the [`quotes`](/docs/rules/quotes.md) rule.
 
+### Providing Suggestions
+
+In some cases fixes aren't appropriate to be automatically applied, for example, if a fix potentially changes functionality or if there are multiple valid ways to fix a rule depending on the implementation intent (see the best practices for [applying fixes](#applying-fixes) listed above). In these cases, there is an alternative `suggest` option on `context.report()` that allows other tools, such as editors, to expose helpers for users to manually apply a suggestion.
+
+In order to provide suggestions, use the `suggest` key in the report argument with an array of suggestion objects. The suggestion objects represent individual suggestions that could be applied and require either a `desc` key string that describes what applying the suggestion would do or a `messageId` key (see [below](#suggestion-messageids)), and a `fix` key that is a function defining the suggestion result. This `fix` function follows the same API as regular fixes (described above in [applying fixes](#applying-fixes)).
+
+```js
+context.report({
+    node: node,
+    message: "Unnecessary escape character: \\{{character}}.",
+    data: { character },
+    suggest: [
+        {
+            desc: "Remove the `\\`. This maintains the current functionality.",
+            fix: function(fixer) {
+                return fixer.removeRange(range);
+            }
+        },
+        {
+            desc: "Replace the `\\` with `\\\\` to include the actual backslash character.",
+            fix: function(fixer) {
+                return fixer.insertTextBeforeRange(range, "\\");
+            }
+        }
+    ]
+});
+```
+
+Note: Suggestions will be applied as a stand-alone change, without triggering multipass fixes. Each suggestion should focus on a singular change in the code and should not try to conform to user defined styles. For example, if a suggestion is adding a new statement into the codebase, it should not try to match correct indentation, or confirm to user preferences on presence/absence of semicolumns. All of those things can be corrected by multipass autofix when the user triggers it.
+
+Best practices for suggestions:
+
+1. Don't try to do too much and suggest large refactors that could introduce a lot of breaking changes.
+1. As noted above, don't try to conform to user-defined styles.
+
+#### Suggestion `messageId`s
+
+Instead of using a `desc` key for suggestions a `messageId` can be used instead. This works the same way as `messageId`s for the overall error (see [messageIds](#messageIds)). Here is an example of how to use it in a rule:
+
+```js
+module.exports = {
+    meta: {
+        messages: {
+            unnecessaryEscape: "Unnecessary escape character: \\{{character}}.",
+            removeEscape: "Remove the `\\`. This maintains the current functionality.",
+            escapeBackslash: "Replace the `\\` with `\\\\` to include the actual backslash character."
+        }
+    },
+    create: function(context) {
+        // ...
+        context.report({
+            node: node,
+            messageId: 'unnecessaryEscape',
+            data: { character },
+            suggest: [
+                {
+                    messageId: "removeEscape",
+                    fix: function(fixer) {
+                        return fixer.removeRange(range);
+                    }
+                },
+                {
+                    messageId: "escapeBackslash",
+                    fix: function(fixer) {
+                        return fixer.insertTextBeforeRange(range, "\\");
+                    }
+                }
+            ]
+        });
+    }
+};
+```
+
 ### context.options
 
 Some rules require options in order to function correctly. These options appear in configuration (`.eslintrc`, command line, or in comments). For example:

lib/linter/report-translator.js

@@ -26,6 +26,7 @@ const interpolate = require("./interpolate");
  * @property {Object} [data] Optional data to use to fill in placeholders in the
  *      message.
  * @property {Function} [fix] The function to call that creates a fix command.
+ * @property {Array<{desc?: string, messageId?: string, fix: Function}>} suggest Suggestion descriptions and functions to create a the associated fixes.
  */
 
 /**
@@ -34,14 +35,15 @@ const interpolate = require("./interpolate");
  * @property {string} ruleId
  * @property {(0|1|2)} severity
  * @property {(string|undefined)} message
- * @property {(string|undefined)} messageId
+ * @property {(string|undefined)} [messageId]
  * @property {number} line
  * @property {number} column
- * @property {(number|undefined)} endLine
- * @property {(number|undefined)} endColumn
+ * @property {(number|undefined)} [endLine]
+ * @property {(number|undefined)} [endColumn]
  * @property {(string|null)} nodeType
  * @property {string} source
- * @property {({text: string, range: (number[]|null)}|null)} fix
+ * @property {({text: string, range: (number[]|null)}|null)} [fix]
+ * @property {Array<{text: string, range: (number[]|null)}|null>} [suggestions]
  */
 
 //------------------------------------------------------------------------------
@@ -182,6 +184,29 @@ function normalizeFixes(descriptor, sourceCode) {
     return fix;
 }
 
+/**
+ * Gets an array of suggestion objects from the given descriptor.
+ * @param {MessageDescriptor} descriptor The report descriptor.
+ * @param {SourceCode} sourceCode The source code object to get text between fixes.
+ * @param {Object} messages Object of meta messages for the rule.
+ * @returns {Array<SuggestionResult>} The suggestions for the descriptor
+ */
+function mapSuggestions(descriptor, sourceCode, messages) {
+    if (!descriptor.suggest || !Array.isArray(descriptor.suggest)) {
+        return [];
+    }
+
+    return descriptor.suggest.map(suggestInfo => {
+        const computedDesc = suggestInfo.desc || messages[suggestInfo.messageId];
+
+        return {
+            ...suggestInfo,
+            desc: interpolate(computedDesc, suggestInfo.data),
+            fix: normalizeFixes(suggestInfo, sourceCode)
+        };
+    });
+}
+
 /**
  * Creates information about the report from a descriptor
  * @param {Object} options Information about the problem
@@ -192,6 +217,7 @@ function normalizeFixes(descriptor, sourceCode) {
  * @param {string} [options.messageId] The error message ID.
  * @param {{start: SourceLocation, end: (SourceLocation|null)}} options.loc Start and end location
  * @param {{text: string, range: (number[]|null)}} options.fix The fix object
+ * @param {Array<{text: string, range: (number[]|null)}>} options.suggestions The array of suggestions objects
  * @returns {function(...args): ReportInfo} Function that returns information about the report
  */
 function createProblem(options) {
@@ -221,9 +247,47 @@ function createProblem(options) {
         problem.fix = options.fix;
     }
 
+    if (options.suggestions && options.suggestions.length > 0) {
+        problem.suggestions = options.suggestions;
+    }
+
     return problem;
 }
 
+/**
+ * Validates that suggestions are properly defined. Throws if an error is detected.
+ * @param {Array<{ desc?: string, messageId?: string }>} suggest The incoming suggest data.
+ * @param {Object} messages Object of meta messages for the rule.
+ * @returns {void}
+ */
+function validateSuggestions(suggest, messages) {
+    if (suggest && Array.isArray(suggest)) {
+        suggest.forEach(suggestion => {
+            if (suggestion.messageId) {
+                const { messageId } = suggestion;
+
+                if (!messages) {
+                    throw new TypeError(`context.report() called with a suggest option with a messageId '${messageId}', but no messages were present in the rule metadata.`);
+                }
+
+                if (!messages[messageId]) {
+                    throw new TypeError(`context.report() called with a suggest option with a messageId '${messageId}' which is not present in the 'messages' config: ${JSON.stringify(messages, null, 2)}`);
+                }
+
+                if (suggestion.desc) {
+                    throw new TypeError("context.report() called with a suggest option that defines both a 'messageId' and an 'desc'. Please only pass one.");
+                }
+            } else if (!suggestion.desc) {
+                throw new TypeError("context.report() called with a suggest option that doesn't have either a `desc` or `messageId`");
+            }
+
+            if (typeof suggestion.fix !== "function") {
+                throw new TypeError(`context.report() called with a suggest option without a fix function. See: ${suggestion}`);
+            }
+        });
+    }
+}
+
 /**
  * Returns a function that converts the arguments of a `context.report` call from a rule into a reported
  * problem for the Node.js API.
@@ -242,17 +306,17 @@ module.exports = function createReportTranslator(metadata) {
      */
     return (...args) => {
         const descriptor = normalizeMultiArgReportCall(...args);
+        const messages = metadata.messageIds;
 
         assertValidNodeInfo(descriptor);
 
         let computedMessage;
 
         if (descriptor.messageId) {
-            if (!metadata.messageIds) {
+            if (!messages) {
                 throw new TypeError("context.report() called with a messageId, but no messages were present in the rule metadata.");
             }
             const id = descriptor.messageId;
-            const messages = metadata.messageIds;
 
             if (descriptor.message) {
                 throw new TypeError("context.report() called with a message and a messageId. Please only pass one.");
@@ -267,6 +331,7 @@ module.exports = function createReportTranslator(metadata) {
             throw new TypeError("Missing `message` property in report() call; add a message that describes the linting problem.");
         }
 
+        validateSuggestions(descriptor.suggest, messages);
 
         return createProblem({
             ruleId: metadata.ruleId,
@@ -275,7 +340,8 @@ module.exports = function createReportTranslator(metadata) {
             message: interpolate(computedMessage, descriptor.data),
             messageId: descriptor.messageId,
             loc: normalizeReportLoc(descriptor),
-            fix: metadata.disableFixes ? null : normalizeFixes(descriptor, metadata.sourceCode)
+            fix: metadata.disableFixes ? null : normalizeFixes(descriptor, metadata.sourceCode),
+            suggestions: metadata.disableFixes ? [] : mapSuggestions(descriptor, metadata.sourceCode, messages)
         });
     };
 };