Repository: eslint/eslint
Pull request #10935 (2018-10-07 21:49:18)

Chore: Simplify space-infix-ops
**What is the purpose of this pull request? (put an "X" next to item)**

[ ] Documentation update
[ ] Bug fix ([template](https://raw.githubusercontent.com/eslint/eslint/master/templates/bug-report.md))
[ ] New rule ([template](https://raw.githubusercontent.com/eslint/eslint/master/templates/rule-proposal.md))
[ ] Changes an existing rule ([template](https://raw.githubusercontent.com/eslint/eslint/master/templates/rule-change-proposal.md))
[ ] Add autofixing to a rule
[ ] Add a CLI option
[ ] Add something to the core
[x] Other, please explain:

**What changes did you make? (Give an overview)**

`space-infix-ops` is a bit overcomplicated. there can be a lot of tokens between `left` and `right`, but only one of them can be an infix operator, the others must be opening and closing parens. if i'm mistaken, i'd happy to close this PR and open another one that adds those test cases :smile_cat: 

**Is there anything you'd like reviewers to focus on?**

afaik javascript engines are pretty good at caching loop invariants (so there's no need to save `tokens.length`), so i made the loop a bit more readable. i wasn't able to do performance testing (as `space-infix-ops` is not part of `eslint:recommended`, so `npm run perf` is no good), but i'd happy to tweak the implementation if someone can give me some pointer about how to do it.


Patch:
diff --git a/lib/rules/one-var.js b/lib/rules/one-var.js
index 15b664d77..3efd0f27f 100644
--- a/lib/rules/one-var.js
+++ b/lib/rules/one-var.js
@@ -314,6 +314,11 @@ module.exports = {
         function splitDeclarations(declaration) {
             return fixer => declaration.declarations.map(declarator => {
                 const tokenAfterDeclarator = sourceCode.getTokenAfter(declarator);
+
+                if (tokenAfterDeclarator === null) {
+                    return null;
+                }
+
                 const afterComma = sourceCode.getTokenAfter(tokenAfterDeclarator, { includeComments: true });
 
                 if (tokenAfterDeclarator.value !== ",") {
diff --git a/lib/rules/space-infix-ops.js b/lib/rules/space-infix-ops.js
index 49b64658d..17b49cc11 100644
--- a/lib/rules/space-infix-ops.js
+++ b/lib/rules/space-infix-ops.js
@@ -34,37 +34,25 @@ module.exports = {
 
     create(context) {
         const int32Hint = context.options[0] ? context.options[0].int32Hint === true : false;
-
-        const OPERATORS = [
-            "*", "/", "%", "+", "-", "<<", ">>", ">>>", "<", "<=", ">", ">=", "in",
-            "instanceof", "==", "!=", "===", "!==", "&", "^", "|", "&&", "||", "=",
-            "+=", "-=", "*=", "/=", "%=", "<<=", ">>=", ">>>=", "&=", "^=", "|=",
-            "?", ":", ",", "**"
-        ];
-
         const sourceCode = context.getSourceCode();
 
         /**
          * Returns the first token which violates the rule
          * @param {ASTNode} left - The left node of the main node
          * @param {ASTNode} right - The right node of the main node
+         * @param {string} op - The operator of the main node
          * @returns {Object} The violator token or null
          * @private
          */
-        function getFirstNonSpacedToken(left, right) {
-            const tokens = sourceCode.getTokensBetween(left, right, 1);
-
-            for (let i = 1, l = tokens.length - 1; i < l; ++i) {
-                const op = tokens[i];
-
-                if (
-                    (op.type === "Punctuator" || op.type === "Keyword") &&
-                    OPERATORS.indexOf(op.value) >= 0 &&
-                    (tokens[i - 1].range[1] >= op.range[0] || op.range[1] >= tokens[i + 1].range[0])
-                ) {
-                    return op;
-                }
+        function getFirstNonSpacedToken(left, right, op) {
+            const operator = sourceCode.getFirstTokenBetween(left, right, token => token.value === op);
+            const prev = sourceCode.getTokenBefore(operator);
+            const next = sourceCode.getTokenAfter(operator);
+
+            if (!sourceCode.isSpaceBetweenTokens(prev, operator) || !sourceCode.isSpaceBetweenTokens(operator, next)) {
+                return operator;
             }
+
             return null;
         }
 
@@ -110,7 +98,10 @@ module.exports = {
             const leftNode = (node.left.typeAnnotation) ? node.left.typeAnnotation : node.left;
             const rightNode = node.right;
 
-            const nonSpacedNode = getFirstNonSpacedToken(leftNode, rightNode);
+            // search for = in AssignmentPattern nodes
+            const operator = node.operator || "=";
+
+            const nonSpacedNode = getFirstNonSpacedToken(leftNode, rightNode, operator);
 
             if (nonSpacedNode) {
                 if (!(int32Hint && sourceCode.getText(node).endsWith("|0"))) {
@@ -126,8 +117,8 @@ module.exports = {
          * @private
          */
         function checkConditional(node) {
-            const nonSpacedConsequesntNode = getFirstNonSpacedToken(node.test, node.consequent);
-            const nonSpacedAlternateNode = getFirstNonSpacedToken(node.consequent, node.alternate);
+            const nonSpacedConsequesntNode = getFirstNonSpacedToken(node.test, node.consequent, "?");
+            const nonSpacedAlternateNode = getFirstNonSpacedToken(node.consequent, node.alternate, ":");
 
             if (nonSpacedConsequesntNode) {
                 report(node, nonSpacedConsequesntNode);
@@ -147,7 +138,7 @@ module.exports = {
             const rightNode = node.init;
 
             if (rightNode) {
-                const nonSpacedNode = getFirstNonSpacedToken(leftNode, rightNode);
+                const nonSpacedNode = getFirstNonSpacedToken(leftNode, rightNode, "=");
 
                 if (nonSpacedNode) {
                     report(node, nonSpacedNode);


Test patch:
diff --git a/tests/lib/rules/one-var.js b/tests/lib/rules/one-var.js
index b628f512a..e5470d2e9 100644
--- a/tests/lib/rules/one-var.js
+++ b/tests/lib/rules/one-var.js
@@ -949,6 +949,17 @@ ruleTester.run("one-var", rule, {
                 column: 1
             }]
         },
+        {
+            code: "var a = 1, b = 2",
+            output: "var a = 1; var b = 2",
+            options: ["never"],
+            errors: [{
+                message: "Split 'var' declarations into multiple statements.",
+                type: "VariableDeclaration",
+                line: 1,
+                column: 1
+            }]
+        },
         {
             code: "var foo = require('foo'), bar;",
             output: null,