New: Add --fix-type option to CLI (fixes #10855) (#10912)

* New: Add --fix-type option to CLI (fixes #10855)

* Chore: Update rule types

* Docs: Update Nodejs API docs with new CLIEngine option

* Chore: Fix lint errors

* Chore: Update options message for --fix-type

* CLIEngine validates fixType values

* Check for rule.meta when filtering fixes

* Call getRules() just once when filtering fixes

* Set no-multiline-str to suggestion

* Add check for missing rule info in rule-types.json

* Account for late definition of rules in CLIEngine

* Add ruleType data to doc page for rules

* Remove duplicate test

* Update spacing rules to be of type style

* Fix lint issues in update-rule-types.js

* Change ruleType to rule_type in doc page front matter

* Update rule type mappings based on feedback

* Update rules to use 'layout' instead of 'style'

* Fix update-rule-types script and prefer-arrow-callback

* Fix lint error

* Update rule types based on latest feedback

* Fix bug CLIEngine

* Update new rule issue template with category info

* Final update of rule types

* Refactor CLIEngine piece based on feedback

.github/ISSUE_TEMPLATE/NEW_RULE.md

@@ -21,9 +21,9 @@ about: Propose a new rule to be added to ESLint
 
 **What category of rule is this? (place an "X" next to just one item)**
 
-[ ] Enforces code style
-[ ] Warns about a potential error
-[ ] Suggests an alternate way of doing something
+[ ] Warns about a potential error (problem)
+[ ] Suggests an alternate way of doing something (suggestion)
+[ ] Enforces code style (layout)
 [ ] Other (please specify:)
 
 **Provide 2-3 code examples that this rule will warn about:**

Makefile.js

@@ -708,6 +708,7 @@ target.gensite = function(prereleaseVersion) {
                 ruleName = path.basename(filename, ".md"),
                 filePath = path.join("docs", path.relative("tmp", filename));
             let text = cat(filename),
+                ruleType = "",
                 title;
 
             process.stdout.write(`> Updating files (Steps 4-9): ${i}/${length} - ${filePath + " ".repeat(30)}\r`);
@@ -726,8 +727,11 @@ target.gensite = function(prereleaseVersion) {
                 const ruleDocsContent = textSplit.slice(1).join("\n");
 
                 text = `${ruleHeading}${isRecommended ? RECOMMENDED_TEXT : ""}${isFixable ? FIXABLE_TEXT : ""}\n${ruleDocsContent}`;
-
                 title = `${ruleName} - Rules`;
+
+                if (rule && rule.meta) {
+                    ruleType = `rule_type: ${rule.meta.type}`;
+                }
             } else {
 
                 // extract the title from the file itself
@@ -744,6 +748,7 @@ target.gensite = function(prereleaseVersion) {
                 `title: ${title}`,
                 "layout: doc",
                 `edit_link: https://github.com/eslint/eslint/edit/master/${filePath}`,
+                ruleType,
                 "---",
                 "<!-- Note: No pull requests accepted for this file. See README.md in the root directory for details. -->",
                 "",
@@ -863,7 +868,7 @@ target.checkRuleFiles = function() {
     echo("Validating rules");
 
     const eslintRecommended = require("./conf/eslint-recommended").rules;
-
+    const ruleTypes = require("./tools/rule-types.json");
     const ruleFiles = find("lib/rules/").filter(fileType("js"));
     let errors = 0;
 
@@ -880,6 +885,15 @@ target.checkRuleFiles = function() {
             return Object.prototype.hasOwnProperty.call(eslintRecommended, basename);
         }
 
+        /**
+         * Check if basename is present in rule-types.json file.
+         * @returns {boolean} true if present
+         * @private
+         */
+        function isInRuleTypes() {
+            return Object.prototype.hasOwnProperty.call(ruleTypes, basename);
+        }
+
         /**
          * Check if id is present in title
          * @param {string} id id to check for
@@ -918,6 +932,12 @@ target.checkRuleFiles = function() {
             errors++;
         }
 
+        // check for recommended configuration
+        if (!isInRuleTypes()) {
+            console.error("Missing setting for %s in tools/rule-types.json", basename);
+            errors++;
+        }
+
         // check for tests
         if (!test("-f", `tests/lib/rules/${basename}.js`)) {
             console.error("Missing tests for rule %s", basename);

docs/developer-guide/nodejs-api.md

@@ -347,7 +347,8 @@ The `CLIEngine` is a constructor, and you can create a new instance by passing i
 * `cwd` - Path to a directory that should be considered as the current working directory.
 * `envs` - An array of environments to load (default: empty array). Corresponds to `--env`.
 * `extensions` - An array of filename extensions that should be checked for code. The default is an array containing just `".js"`. Corresponds to `--ext`. It is only used in conjunction with directories, not with filenames or glob patterns.
-* `fix` - This can be a boolean or a function which will be provided each linting message and should return a boolean. True indicates that fixes should be included with the output report, and that errors and warnings should not be listed if they can be fixed. However, the files on disk will not be changed. To persist changes to disk, call [`outputFixes()`](#cliengineoutputfixes).
+* `fix` - A boolean or a function (default: `false`). If a function, it will be passed each linting message and should return a boolean indicating whether the fix should be included with the output report (errors and warnings will not be listed if fixed). Files on disk are never changed regardless of the value of `fix`. To persist changes to disk, call [`outputFixes()`](#cliengineoutputfixes).
+* `fixTypes` - An array of rule types for which fixes should be applied (default: `null`). This array acts like a filter, only allowing rules of the given types to apply fixes. Possible array values are `"problem"`, `"suggestion"`, and `"layout"`.
 * `globals` - An array of global variables to declare (default: empty array). Corresponds to `--global`.
 * `ignore` - False disables use of `.eslintignore`, `ignorePath` and `ignorePattern` (default: true). Corresponds to `--no-ignore`.
 * `ignorePath` - The ignore file to use instead of `.eslintignore` (default: null). Corresponds to `--ignore-path`.

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

@@ -26,6 +26,8 @@ Here is the basic format of the source file for a rule:
 
 module.exports = {
     meta: {
+        type: "suggestion",
+
         docs: {
             description: "disallow unnecessary semicolons",
             category: "Possible Errors",
@@ -49,6 +51,11 @@ The source file for a rule exports an object with the following properties.
 
 `meta` (object) contains metadata for the rule:
 
+* `type` (string) indicates the type of rule, which is one of `"problem"`, `"suggestion"`, or `"layout"`:
+    * `"problem"` means the rule is identifying code that either will cause an error or may cause a confusing behavior. Developers should consider this a high priority to resolve.
+    * `"suggestion"` means the rule is identifying something that could be done in a better way but no errors will occur if the code isn't changed.
+    * `"layout"` means the rule cares primarily about whitespace, semicolons, commas, and parentheses, all the parts of the program that determine how the code looks rather than how it executes. These rules work on parts of the code that aren't specified in the AST.
+
 * `docs` (object) is required for core rules of ESLint:
 
     * `description` (string) provides the short description of the rule in the [rules index](../rules/)

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

@@ -46,6 +46,7 @@ Specifying rules and plugins:
 Fixing problems:
   --fix                          Automatically fix problems
   --fix-dry-run                  Automatically fix problems without saving the changes to the file system
+  --fix-type Array               Specify the types of fixes to apply (problem, suggestion, layout)
 
 Ignoring files:
   --ignore-path path::String     Specify path of ignore file
@@ -233,6 +234,24 @@ getSomeText | eslint --stdin --fix-dry-run --format=json
 
 This flag can be useful for integrations (e.g. editor plugins) which need to autofix text from the command line without saving it to the filesystem.
 
+#### `--fix-type`
+
+This option allows you to specify the type of fixes to apply when using either `--fix` or `--fix-dry-run`. The three types of fixes are:
+
+1. `problem` - fix potential errors in the code
+1. `suggestion` - apply fixes to the code that improve it
+1. `layout` - apply fixes that do not change the program structure (AST)
+
+You can specify one or more fix type on the command line. Here are some examples:
+
+```
+eslint --fix --fix-type suggestion .
+eslint --fix --fix-type suggestion --fix-type problem .
+eslint --fix --fix-type suggestion,layout .
+```
+
+This option is helpful if you are using another program to format your code but you would still like ESLint to apply other types of fixes.
+
 ### Ignoring files
 
 #### `--ignore-path`

lib/cli-engine.js

@@ -33,6 +33,7 @@ const fs = require("fs"),
 
 const debug = require("debug")("eslint:cli-engine");
 const resolver = new ModuleResolver();
+const validFixTypes = new Set(["problem", "suggestion", "layout"]);
 
 //------------------------------------------------------------------------------
 // Typedefs
@@ -50,6 +51,7 @@ const resolver = new ModuleResolver();
  * @property {string[]} envs An array of environments to load.
  * @property {string[]} extensions An array of file extensions to check.
  * @property {boolean|Function} fix Execute in autofix mode. If a function, should return a boolean.
+ * @property {string[]} fixTypes Array of rule types to apply fixes for.
  * @property {string[]} globals An array of global variables to declare.
  * @property {boolean} ignore False disables use of .eslintignore.
  * @property {string} ignorePath The ignore file to use instead of .eslintignore.
@@ -86,6 +88,21 @@ const resolver = new ModuleResolver();
 // Helpers
 //------------------------------------------------------------------------------
 
+/**
+ * Determines if each fix type in an array is supported by ESLint and throws
+ * an error if not.
+ * @param {string[]} fixTypes An array of fix types to check.
+ * @returns {void}
+ * @throws {Error} If an invalid fix type is found.
+ */
+function validateFixTypes(fixTypes) {
+    for (const fixType of fixTypes) {
+        if (!validFixTypes.has(fixType)) {
+            throw new Error(`Invalid fix type "${fixType}" found.`);
+        }
+    }
+}
+
 /**
  * It will calculate the error and warning count for collection of messages per file
  * @param {Object[]} messages - Collection of messages
@@ -176,7 +193,6 @@ function processText(text, configHelper, filename, fix, allowInlineConfig, repor
     }
 
     const autofixingEnabled = typeof fix !== "undefined" && (!processor || processor.supportsAutofix);
-
     const fixedResult = linter.verifyAndFix(text, config, {
         filename: effectiveFilename,
         allowInlineConfig,
@@ -185,7 +201,6 @@ function processText(text, configHelper, filename, fix, allowInlineConfig, repor
         preprocess: processor && (rawText => processor.preprocess(rawText, effectiveFilename)),
         postprocess: processor && (problemLists => processor.postprocess(problemLists, effectiveFilename))
     });
-
     const stats = calculateStatsPerFile(fixedResult.messages);
 
     const result = {
@@ -456,6 +471,33 @@ class CLIEngine {
              */
             this._lintResultCache = new LintResultCache(cacheFile, this.config);
         }
+
+        // setup special filter for fixes
+        if (this.options.fix && this.options.fixTypes && this.options.fixTypes.length > 0) {
+
+            debug(`Using fix types ${this.options.fixTypes}`);
+
+            // throw an error if any invalid fix types are found
+            validateFixTypes(this.options.fixTypes);
+
+            // convert to Set for faster lookup
+            const fixTypes = new Set(this.options.fixTypes);
+
+            // save original value of options.fix in case it's a function
+            const originalFix = (typeof this.options.fix === "function")
+                ? this.options.fix : () => this.options.fix;
+
+            // create a cache of rules (but don't populate until needed)
+            this._rulesCache = null;
+
+            this.options.fix = lintResult => {
+                const rule = this._rulesCache.get(lintResult.ruleId);
+                const matches = rule.meta && fixTypes.has(rule.meta.type);
+
+                return matches && originalFix(lintResult);
+            };
+        }
+
     }
 
     getRules() {
@@ -560,6 +602,11 @@ class CLIEngine {
                 }
             }
 
+            // if there's a cache, populate it
+            if ("_rulesCache" in this) {
+                this._rulesCache = this.getRules();
+            }
+
             debug(`Processing ${fileInfo.filename}`);
 
             const { result, config } = processFile(fileInfo.filename, configHelper, options, this.linter);
@@ -630,6 +677,12 @@ class CLIEngine {
             }
             usedDeprecatedRules = [];
         } else {
+
+            // if there's a cache, populate it
+            if ("_rulesCache" in this) {
+                this._rulesCache = this.getRules();
+            }
+
             const { result, config } = processText(
                 text,
                 configHelper,

lib/cli.js

@@ -64,6 +64,7 @@ function translateOptions(cliOptions) {
         cacheFile: cliOptions.cacheFile,
         cacheLocation: cliOptions.cacheLocation,
         fix: (cliOptions.fix || cliOptions.fixDryRun) && (cliOptions.quiet ? quietFixPredicate : true),
+        fixTypes: cliOptions.fixType,
         allowInlineConfig: cliOptions.inlineConfig,
         reportUnusedDisableDirectives: cliOptions.reportUnusedDisableDirectives
     };
@@ -187,8 +188,12 @@ const cli = {
                 return 2;
             }
 
-            const engine = new CLIEngine(translateOptions(currentOptions));
+            if (currentOptions.fixType && !currentOptions.fix && !currentOptions.fixDryRun) {
+                log.error("The --fix-type option requires either --fix or --fix-dry-run.");
+                return 2;
+            }
 
+            const engine = new CLIEngine(translateOptions(currentOptions));
             const report = useStdin ? engine.executeOnText(text, currentOptions.stdinFilename, true) : engine.executeOnFiles(files);
 
             if (currentOptions.fix) {

lib/options.js

@@ -97,6 +97,11 @@ module.exports = optionator({
             default: false,
             description: "Automatically fix problems without saving the changes to the file system"
         },
+        {
+            option: "fix-type",
+            type: "Array",
+            description: "Specify the types of fixes to apply (problem, suggestion, layout)"
+        },
         {
             heading: "Ignoring files"
         },

lib/rules/accessor-pairs.js

@@ -72,12 +72,15 @@ function isPropertyDescriptor(node) {
 
 module.exports = {
     meta: {
+        type: "suggestion",
+
         docs: {
             description: "enforce getter and setter pairs in objects",
             category: "Best Practices",
             recommended: false,
             url: "https://eslint.org/docs/rules/accessor-pairs"
         },
+
         schema: [{
             type: "object",
             properties: {
@@ -90,6 +93,7 @@ module.exports = {
             },
             additionalProperties: false
         }],
+
         messages: {
             getter: "Getter is not present.",
             setter: "Setter is not present."

lib/rules/array-bracket-newline.js

@@ -13,13 +13,17 @@ const astUtils = require("../util/ast-utils");
 
 module.exports = {
     meta: {
+        type: "layout",
+
         docs: {
             description: "enforce linebreaks after opening and before closing array brackets",
             category: "Stylistic Issues",
             recommended: false,
             url: "https://eslint.org/docs/rules/array-bracket-newline"
         },
+
         fixable: "whitespace",
+
         schema: [
             {
                 oneOf: [
@@ -42,6 +46,7 @@ module.exports = {
                 ]
             }
         ],
+
         messages: {
             unexpectedOpeningLinebreak: "There should be no linebreak after '['.",
             unexpectedClosingLinebreak: "There should be no linebreak before ']'.",

lib/rules/array-bracket-spacing.js

@@ -12,13 +12,17 @@ const astUtils = require("../util/ast-utils");
 
 module.exports = {
     meta: {
+        type: "layout",
+
         docs: {
             description: "enforce consistent spacing inside array brackets",
             category: "Stylistic Issues",
             recommended: false,
             url: "https://eslint.org/docs/rules/array-bracket-spacing"
         },
+
         fixable: "whitespace",
+
         schema: [
             {
                 enum: ["always", "never"]
@@ -39,6 +43,7 @@ module.exports = {
                 additionalProperties: false
             }
         ],
+
         messages: {
             unexpectedSpaceAfter: "There should be no space after '{{tokenValue}}'.",
             unexpectedSpaceBefore: "There should be no space before '{{tokenValue}}'.",

lib/rules/array-callback-return.js

@@ -141,6 +141,8 @@ function isCallbackOfArrayMethod(node) {
 
 module.exports = {
     meta: {
+        type: "problem",
+
         docs: {
             description: "enforce `return` statements in callbacks of array methods",
             category: "Best Practices",

lib/rules/array-element-newline.js

@@ -13,13 +13,17 @@ const astUtils = require("../util/ast-utils");
 
 module.exports = {
     meta: {
+        type: "layout",
+
         docs: {
             description: "enforce line breaks after each array element",
             category: "Stylistic Issues",
             recommended: false,
             url: "https://eslint.org/docs/rules/array-element-newline"
         },
+
         fixable: "whitespace",
+
         schema: [
             {
                 oneOf: [