Breaking: rewrite indent (fixes #1801, #3737, #3845, #6007, ...16 mor…

…e) (#7618) * Update: rewrite `indent` (fixes #1801, #3737, #3845, #6007, ...16 more) Fixes #1801, fixes #3737, fixes #3845, fixes #6007, fixes #6571, fixes #6670, fixes #6813, fixes #7242, fixes #7274, fixes #7320, fixes #7420, fixes #7522, fixes #7616, fixes #7641, fixes #7662, fixes #7771, fixes #7892, fixes #8011, fixes #8038, fixes #8144 The existing implementation of `indent` had a lot of bugs (see above list). It worked by detecting a node type (e.g. `ObjectExpression`), and then ensuring that the indentation around the object satisfies certain constraints (e.g. the properties of the `ObjectExpression` are offset by 4 spaces from the opening bracket). This approach had a number of disadvantages: - Since it only checked indentation according to an explicit list of patterns, there were a lot of cases where it accidentally didn't check the indentation at all. For example, there was no check for the indentation of a closing `)` in a `CallExpression`, so the rule just silently ignored incorrect indentation in these cases. (#7522) - there were a lot of nodes where indentation wasn't checked at all. For example, it didn't check indentation for ternary expressions (#7420) or destructuring assignments (#6813). - Since it could only check indent patterns on nodes, it couldn't check the indentation of comments (#3845, #6571) or optional tokens such as parentheses around an expression (#7522) This commit rewrites the `indent` rule. The new strategy is based on tokens rather than nodes: 1. Create a hashmap (`OffsetStorage#desiredOffsets`). The keys are all the tokens and comments in the file, and the values are objects containing information for a specific offset, measured in indent levels, from a either a specific token or the first column. For example, an element in an array will have `{offset: 1, from: openingCurly}` to indicate that it is offset by one indentation level from the opening curly brace. All the offsets are initialized to 0 at the start. 1. As the AST is traversed, modify the offsets of tokens accordingly. For example, when entering a `BlockStatement`, offset all of the tokens in the `BlockStatement` by 1 from the opening curly brace of the `BlockStatement`. 1. After traversing the AST, calculate the expected indentation levels of every token in the file (according to the `desiredOffsets` map). 1. For each token, compare the expected indentation to the actual indentation in the file, and report the token if the two values are not equal. This has the following advantages: - It is guaranteed to check the indentation of every single token in the file, with the exception of some tokens that are explicitly ignored*. This ensures that no tokens end up unexpectedly being ignored. - Since tokens/comments are used instead of nodes, there are no unchecked "stray tokens". - All nodes are evaluated in a context-free manner. In other words, each node only has to set an offset for its own children, without worrying about what how much indentation the node itself has or what the node's parents are. - The rule ends up with an expected indentation map for the entire file at once, and so it can fix the entire file in one pass. (The previous implementation often required multiple passes. For example, if a node was misaligned with its parent in the previous implementation, the node would get fixed, even if the node's position was actually correct and the parent was off.) *There are a few cases where the new implementation explicitly ignores lines. I decided to do this because there is a huge amount of inconsistency in what people seem to prefer for these cases. In the future, we might want to stop ignoring these cases so that the indentation of all lines is checked. One such case is: ```js ({ foo: bar }); // versus ({ foo: bar }); ``` Comments are treated a bit differently from tokens in that they can have several different indentations. This is because it can be difficult to tell what the comment is referring to. For example: ```js if (foo) { doSomething(); // comment about the doSomething() call } else if (bar.baz()) { doSomethingElse(); } // versus if (foo) { doSomething(); // comment about the bar.baz() call } else if (bar.baz()) { doSomethingElse(); } ``` Specifically, a comment is allowed to have one of three indentations: 1. The same indentation as the token right before it 1. The same indentation as the token right after it 1. The computed indentation for the comment itself * Ensure reported range has endLine and endColumn * Use objects instead of WeakMaps to improve performance * Update the big explanation comment at the top of the file * Fix variable capitalization * Remove unneeded IfStatement logic * Remove unused equality check * Add test for else without block * Fix single-line statements with semicolon-first style
eslint · Apr 7, 2017 · cc53481 · cc53481
1 parent 867dd2e
commit cc53481
Show file tree

Hide file tree

Showing 19 changed files with 6,100 additions and 3,792 deletions.
diff --git a/docs/rules/indent.md b/docs/rules/indent.md
@@ -71,17 +71,18 @@ This rule has an object option:
 * `"SwitchCase"` (default: 0) enforces indentation level for `case` clauses in `switch` statements
 * `"VariableDeclarator"` (default: 1) enforces indentation level for `var` declarators; can also take an object to define separate rules for `var`, `let` and `const` declarations.
 * `"outerIIFEBody"` (default: 1) enforces indentation level for file-level IIFEs.
-* `"MemberExpression"` (off by default) enforces indentation level for multi-line property chains (except in variable declarations and assignments)
+* `"MemberExpression"` (default: 1) enforces indentation level for multi-line property chains (except in variable declarations and assignments). This can also be set to `"off"` to disable checking for MemberExpression indentation.
 * `"FunctionDeclaration"` takes an object to define rules for function declarations.
-    * `parameters` (off by default) enforces indentation level for parameters in a function declaration. This can either be a number indicating indentation level, or the string `"first"` indicating that all parameters of the declaration must be aligned with the first parameter.
+    * `parameters` (default: 1) enforces indentation level for parameters in a function declaration. This can either be a number indicating indentation level, or the string `"first"` indicating that all parameters of the declaration must be aligned with the first parameter. This can also be set to `"off"` to disable checking for FunctionDeclaration parameters.
     * `body` (default: 1) enforces indentation level for the body of a function declaration.
 * `"FunctionExpression"` takes an object to define rules for function expressions.
-    * `parameters` (off by default) enforces indentation level for parameters in a function expression. This can either be a number indicating indentation level, or the string `"first"` indicating that all parameters of the expression must be aligned with the first parameter.
+    * `parameters` (default: 1) enforces indentation level for parameters in a function expression. This can either be a number indicating indentation level, or the string `"first"` indicating that all parameters of the expression must be aligned with the first parameter. This can also be set to `"off"` to disable checking for FunctionExpression parameters.
     * `body` (default: 1) enforces indentation level for the body of a function expression.
 * `"CallExpression"` takes an object to define rules for function call expressions.
-    * `arguments` (off by default) enforces indentation level for arguments in a call expression. This can either be a number indicating indentation level, or the string `"first"` indicating that all arguments of the expression must be aligned with the first argument.
-* `"ArrayExpression"` (default: 1) enforces indentation level for elements in arrays. It can also be set to the string `"first"`, indicating that all the elements in the array should be aligned with the first element.
-* `"ObjectExpression"` (default: 1) enforces indentation level for properties in objects. It can be set to the string `"first"`, indicating that all properties in the object should be aligned with the first property.
+    * `arguments` (default: 1) enforces indentation level for arguments in a call expression. This can either be a number indicating indentation level, or the string `"first"` indicating that all arguments of the expression must be aligned with the first argument. This can also be set to `"off"` to disable checking for CallExpression arguments.
+* `"ArrayExpression"` (default: 1) enforces indentation level for elements in arrays. It can also be set to the string `"first"`, indicating that all the elements in the array should be aligned with the first element. This can also be set to `"off"` to disable checking for array elements.
+* `"ObjectExpression"` (default: 1) enforces indentation level for properties in objects. It can be set to the string `"first"`, indicating that all properties in the object should be aligned with the first property. This can also be set to `"off"` to disable checking for object properties.
+* `"flatTernaryExpressions": true` (`false` by default) requires no indentation for ternary expressions which are nested in other ternary expressions.
 
 Level of indentation denotes the multiple of the indent specified. Example:
 
@@ -522,6 +523,56 @@ var foo = { bar: 1,
             baz: 2 };
 ```
 
+### flatTernaryExpressions
+
+Examples of **incorrect** code for this rule with the default `4, { "flatTernaryExpressions": false }` option:
+
+```js
+/*eslint indent: ["error", 4, { "flatTernaryExpressions": false }]*/
+
+foo
+    ? bar
+    : baz
+    ? qux
+    : boop;
+```
+
+Examples of **correct** code for this rule with the default `4, { "flatTernaryExpressions": false }` option:
+
+```js
+/*eslint indent: ["error", 4, { "flatTernaryExpressions": false }]*/
+
+foo
+    ? bar
+    : baz
+        ? qux
+        : boop;
+```
+
+Examples of **incorrect** code for this rule with the `4, { "flatTernaryExpressions": true }` option:
+
+```js
+/*eslint indent: ["error", 4, { "flatTernaryExpressions": true }]*/
+
+foo
+    ? bar
+    : baz
+        ? qux
+        : boop;
+```
+
+Examples of **correct** code for this rule with the `4, { "flatTernaryExpressions": true }` option:
+
+```js
+/*eslint indent: ["error", 4, { "flatTernaryExpressions": true }]*/
+
+foo
+    ? bar
+    : baz
+    ? qux
+    : boop;
+```
+
 
 ## Compatibility
 

diff --git a/lib/config/autoconfig.js b/lib/config/autoconfig.js
@@ -37,11 +37,11 @@ const MAX_CONFIG_COMBINATIONS = 17, // 16 combinations + 1 for severity only
  * @param   {number}     errorCount    The number of errors encountered when linting with the config
  */
 
- /**
-  * This callback is used to measure execution status in a progress bar
-  * @callback progressCallback
-  * @param {number} The total number of times the callback will be called.
-  */
+/**
+ * This callback is used to measure execution status in a progress bar
+ * @callback progressCallback
+ * @param {number} The total number of times the callback will be called.
+ */
 
 /**
  * Create registryItems for rules

diff --git a/lib/config/config-file.js b/lib/config/config-file.js
@@ -368,18 +368,18 @@ function getLookupPath(configFilePath) {
 function getEslintCoreConfigPath(name) {
     if (name === "eslint:recommended") {
 
-       /*
-        * Add an explicit substitution for eslint:recommended to
-        * conf/eslint-recommended.js.
-        */
+        /*
+         * Add an explicit substitution for eslint:recommended to
+         * conf/eslint-recommended.js.
+         */
         return path.resolve(__dirname, "../../conf/eslint-recommended.js");
     }
 
     if (name === "eslint:all") {
 
-       /*
-        * Add an explicit substitution for eslint:all to conf/eslint-all.js
-        */
+        /*
+         * Add an explicit substitution for eslint:all to conf/eslint-all.js
+         */
         return path.resolve(__dirname, "../../conf/eslint-all.js");
     }
 

diff --git a/lib/config/config-rule.js b/lib/config/config-rule.js
@@ -168,16 +168,16 @@ function combinePropertyObjects(objArr1, objArr2) {
     return res;
 }
 
- /**
-  * Creates a new instance of a rule configuration set
-  *
-  * A rule configuration set is an array of configurations that are valid for a
-  * given rule.  For example, the configuration set for the "semi" rule could be:
-  *
-  * ruleConfigSet.ruleConfigs // -> [[2], [2, "always"], [2, "never"]]
-  *
-  * Rule configuration set class
-  */
+/**
+ * Creates a new instance of a rule configuration set
+ *
+ * A rule configuration set is an array of configurations that are valid for a
+ * given rule.  For example, the configuration set for the "semi" rule could be:
+ *
+ * ruleConfigSet.ruleConfigs // -> [[2], [2, "always"], [2, "never"]]
+ *
+ * Rule configuration set class
+ */
 class RuleConfigSet {
 
     /**

diff --git a/lib/rules/comma-spacing.js b/lib/rules/comma-spacing.js
@@ -87,8 +87,8 @@ module.exports = {
 
                 },
                 message: options[dir]
-                  ? "A space is required {{dir}} ','."
-                  : "There should be no space {{dir}} ','.",
+                    ? "A space is required {{dir}} ','."
+                    : "There should be no space {{dir}} ','.",
                 data: {
                     dir
                 }

diff --git a/lib/rules/comma-style.js b/lib/rules/comma-style.js
@@ -202,7 +202,7 @@ module.exports = {
                      */
                     if (astUtils.isCommaToken(commaToken)) {
                         validateCommaItemSpacing(previousItemToken, commaToken,
-                                currentItemToken, reportItem);
+                            currentItemToken, reportItem);
                     }
 
                     if (item) {