Add support for suggestion messageId-based descriptions

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

@@ -349,17 +349,24 @@ Best practices for fixes:
 
 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 a `desc` key string that describes what applying the suggestion would do, 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)).
+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: "Missing semicolon",
+    message: "Unnecessary escape character: \\{{character}}.",
+    data: { character },
     suggest: [
         {
-            desc: "Insert the missing semicolon",
+            desc: "Remove unnecessary escape.",
             fix: function(fixer) {
-                return fixer.insertTextAfter(node, ";");
+                return fixer.removeRange(range);
+            }
+        },
+        {
+            desc: "Escape backslash to include it in the RegExp.",
+            fix: function(fixer) {
+                return fixer.insertTextBeforeRange(range, "\\");
             }
         }
     ]
@@ -368,6 +375,43 @@ context.report({
 
 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 confirm 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.
 
+#### 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: {
+            removeEscape: "Remove unnecessary escape.",
+            escapeBackslash: "Escape backslash to include it in the RegExp."
+        }
+    },
+    create: function(context) {
+        // ...
+        context.report({
+            node: node,
+            message: "Unnecessary escape character: \\{{character}}.",
+            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,7 +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, fix: Function}>} suggest Suggestion descriptions and functions to create a the associated fixes.
+ * @property {Array<{desc?: string, messageId?: string, fix: Function}>} suggest Suggestion descriptions and functions to create a the associated fixes.
  */
 
 /**
@@ -188,17 +188,23 @@ function normalizeFixes(descriptor, sourceCode) {
  * 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.
- * @returns {Array<{text: string, range: number[]}>} The suggestions for the descriptor
+ * @param {Object} messages Object of meta messages for the rule.
+ * @returns {Array<SuggestionResult>} The suggestions for the descriptor
  */
-function mapSuggestions(descriptor, sourceCode) {
+function mapSuggestions(descriptor, sourceCode, messages) {
     if (!descriptor.suggest || !Array.isArray(descriptor.suggest)) {
         return [];
     }
 
-    return descriptor.suggest.map(suggestInfo => ({
-        desc: suggestInfo.desc,
-        fix: normalizeFixes(suggestInfo, sourceCode)
-    }));
+    return descriptor.suggest.map(suggestInfo => {
+        const computedDesc = suggestInfo.desc || messages[suggestInfo.messageId];
+
+        return {
+            ...suggestInfo,
+            desc: interpolate(computedDesc, suggestInfo.data),
+            fix: normalizeFixes(suggestInfo, sourceCode)
+        };
+    });
 }
 
 /**
@@ -248,6 +254,40 @@ function createProblem(options) {
     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.
@@ -266,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.");
@@ -291,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,
@@ -300,7 +341,7 @@ module.exports = function createReportTranslator(metadata) {
             messageId: descriptor.messageId,
             loc: normalizeReportLoc(descriptor),
             fix: metadata.disableFixes ? null : normalizeFixes(descriptor, metadata.sourceCode),
-            suggestions: metadata.disableFixes ? [] : mapSuggestions(descriptor, metadata.sourceCode)
+            suggestions: metadata.disableFixes ? [] : mapSuggestions(descriptor, metadata.sourceCode, messages)
         });
     };
 };

lib/shared/types.js

@@ -91,7 +91,14 @@ module.exports = {};
  * @property {string} message The error message.
  * @property {string|null} ruleId The ID of the rule which makes this message.
  * @property {0|1|2} severity The severity of this message.
- * @property {Array<{desc: string, fix: {range: [number, number], text: string}}>} [suggestions] Information for suggestions.
+ * @property {Array<{desc?: string, messageId?: string, fix: {range: [number, number], text: string}}>} [suggestions] Information for suggestions.
+ */
+
+/**
+ * @typedef {Object} SuggestionResult
+ * @property {string} [desc] A short description. Required unless `messageId` is defined.
+ * @property {string} [messageId] Id referencing a message for the description.
+ * @property {{ text: string, range: number[] }} fix fix result info
  */
 
 /**

tests/lib/linter/linter.js

@@ -4385,6 +4385,50 @@ describe("Linter", () => {
                 }
             }]);
         });
+
+        it("supports messageIds for suggestions", () => {
+            linter.defineRule("rule-with-suggestions", {
+                meta: {
+                    messages: {
+                        suggestion1: "Insert space at the beginning",
+                        suggestion2: "Insert space at the end"
+                    }
+                },
+                create: context => ({
+                    Program(node) {
+                        context.report({
+                            node,
+                            message: "Incorrect spacing",
+                            suggest: [{
+                                messageId: "suggestion1",
+                                fix: fixer => fixer.insertTextBefore(node, " ")
+                            }, {
+                                messageId: "suggestion2",
+                                fix: fixer => fixer.insertTextAfter(node, " ")
+                            }]
+                        });
+                    }
+                })
+            });
+
+            const messages = linter.verify("var a = 1;", { rules: { "rule-with-suggestions": "error" } });
+
+            assert.deepStrictEqual(messages[0].suggestions, [{
+                messageId: "suggestion1",
+                desc: "Insert space at the beginning",
+                fix: {
+                    range: [0, 0],
+                    text: " "
+                }
+            }, {
+                messageId: "suggestion2",
+                desc: "Insert space at the end",
+                fix: {
+                    range: [10, 10],
+                    text: " "
+                }
+            }]);
+        });
     });
 
     describe("mutability", () => {