From fe68f907d7b07c91be6cecb27598d5ac21f51a8d Mon Sep 17 00:00:00 2001 From: Ihor Diachenko Date: Sun, 4 Aug 2019 15:51:14 +0300 Subject: [PATCH 1/8] Autofix assignment --- lib/rules/operator-whitespace.js | 27 +++++++----- .../operator-whitespace.js | 44 ++++++++++++++++++- 2 files changed, 60 insertions(+), 11 deletions(-) diff --git a/lib/rules/operator-whitespace.js b/lib/rules/operator-whitespace.js index c89a384..0d2e8d5 100644 --- a/lib/rules/operator-whitespace.js +++ b/lib/rules/operator-whitespace.js @@ -15,7 +15,9 @@ module.exports = { description: "Ensure that operators are surrounded by a single space on either side" }, - schema: [] + schema: [], + + fixable: "whitespace" }, @@ -30,20 +32,25 @@ module.exports = { * behave as wildcard characters. So to make sure they're treated as simple strings, we add '\' before them. * As of today, these chars include: * / + | ^ */ - let node = emitted.node, - op = node.operator.replace(/([\+\*\/\|\^])/g, "\\$1"), opLength = node.operator.length; + const node = emitted.node; + const op = node.operator.replace(/([\+\*\/\|\^])/g, "\\$1"); + const opLength = node.operator.length; if (emitted.exit) { return; } // If expression is 'abc *= def;', then charsAfterLeftNode will contain ' *= d'. - let charsAfterLeftNode = sourceCode.getNextChars(node.left, 3 + opLength), - validationRegexp = new RegExp("^ " + op + " [^\\s]$"); + const charsAfterLeftNode = sourceCode.getNextChars(node.left, 3 + opLength); + const validationRegexp = new RegExp("^ " + op + " [^\\s]$"); (!validationRegexp.test(charsAfterLeftNode)) && context.report({ node: node.left, - message: "Assignment operator must have exactly single space on both sides of it." + message: "Assignment operator must have exactly single space on both sides of it.", + fix(fixer) { + const fixed = sourceCode.getText(node).replace(/\s+/g, " "); + return fixer.replaceText(node, fixed); + } }); } @@ -60,12 +67,12 @@ module.exports = { if (code [i] === "=") { (!/^[^\/\s] $/.test(code.slice(i-2, i))) && context.report({ node: node, - message: "There should be only a single space between assignment operator '=' and its left side." + message: "There should be only a single space between assignment operator '=' and its left side." }); (!/^ [^\/\s]$/.test(code.slice(i+1, i+3))) && context.report({ node: node, - message: "There should be only a single space between assignment operator '=' and its right side." + message: "There should be only a single space between assignment operator '=' and its right side." }); } } @@ -111,7 +118,7 @@ module.exports = { location: { column: sourceCode.getEndingColumn(node.left) }, - message: "Operator \"" + node.operator + "\" should be on the line where left side of the Binary expression ends." + message: "Operator \"" + node.operator + "\" should be on the line where left side of the Binary expression ends." }); } @@ -187,7 +194,7 @@ module.exports = { VariableDeclaration: inspectVariableDeclaration, AssignmentExpression: inspectAssignmentExpression }; - + } }; diff --git a/test/lib/rules/operator-whitespace/operator-whitespace.js b/test/lib/rules/operator-whitespace/operator-whitespace.js index c57df38..216dfe7 100644 --- a/test/lib/rules/operator-whitespace/operator-whitespace.js +++ b/test/lib/rules/operator-whitespace/operator-whitespace.js @@ -175,7 +175,7 @@ describe("[RULE] operator-whitespace: Rejections", function() { let errors; code = code.map(function(item){return toFunction(item);}); - + errors = Solium.lint(code [0], userConfig); errors.constructor.name.should.equal("Array"); errors.length.should.equal(1); @@ -339,3 +339,45 @@ describe("[RULE] operator-whitespace: Rejections", function() { }); }); + +describe("[RULE] operator-whitespace: Fixes", function() { + + it("should fix whitespace around assignment operator when fix is enabled", function(done) { + const testCases = [ + { + input: "foo \t= bar;\n", + output: "foo = bar;\n" + }, + { + input: "foo = \t\tbar;\n", + output: "foo = bar;\n" + }, + { + input: "foo = bar;\n", + output: "foo = bar;\n" + }, + { + input: "foo += bar;", + output: "foo += bar;" + } + ]; + + testCases.forEach(testCase => { + let fixed = Solium.lintAndFix(toFunction(testCase.input), userConfig); + + fixed.should.be.type("object"); + fixed.should.have.ownProperty("fixedSourceCode"); + fixed.should.have.ownProperty("errorMessages"); + fixed.should.have.ownProperty("fixesApplied"); + + fixed.fixedSourceCode.should.equal(toFunction(testCase.output)); + fixed.errorMessages.should.be.Array(); + fixed.errorMessages.length.should.equal(0); + fixed.fixesApplied.should.be.Array(); + fixed.fixesApplied.length.should.equal(1); + }); + + Solium.reset(); + done(); + }); +}); \ No newline at end of file From 1c0f0667ed9b7d28cc3cd0ef79b5b0cae3ea5c54 Mon Sep 17 00:00:00 2001 From: Ihor Diachenko Date: Mon, 5 Aug 2019 01:49:39 +0300 Subject: [PATCH 2/8] Handle variable assignments --- lib/rules/operator-whitespace.js | 53 +++++++++++++++---- lib/utils/string-utils.js | 0 .../operator-whitespace.js | 30 +++++++++-- test/lib/rules/whitespace/whitespace.js | 10 ++-- 4 files changed, 75 insertions(+), 18 deletions(-) create mode 100644 lib/utils/string-utils.js diff --git a/lib/rules/operator-whitespace.js b/lib/rules/operator-whitespace.js index 0d2e8d5..0997831 100644 --- a/lib/rules/operator-whitespace.js +++ b/lib/rules/operator-whitespace.js @@ -49,7 +49,7 @@ module.exports = { message: "Assignment operator must have exactly single space on both sides of it.", fix(fixer) { const fixed = sourceCode.getText(node).replace(/\s+/g, " "); - return fixer.replaceText(node, fixed); + return [fixer.replaceText(node, fixed)]; } }); } @@ -63,17 +63,50 @@ module.exports = { } //if a particular character is '=', check its left and right for single space + const oneSpaceOnLeft = /^[^\/\s] $/; + const whitespaceOnLeft = /(\s+)$/; + const oneSpaceOnRight = /^ [^\/\s]$/; + const whitespaceOnRight = /^(\s+)/; + for (let i = 2; i < code.length; i++) { if (code [i] === "=") { - (!/^[^\/\s] $/.test(code.slice(i-2, i))) && context.report({ - node: node, - message: "There should be only a single space between assignment operator '=' and its left side." - }); - - (!/^ [^\/\s]$/.test(code.slice(i+1, i+3))) && context.report({ - node: node, - message: "There should be only a single space between assignment operator '=' and its right side." - }); + if (!oneSpaceOnLeft.test(code.slice(i - 2, i))) { + context.report({ + node: node, + message: "There should be only a single space between assignment operator '=' and its left side.", + fix(fixer) { + const match = code.slice(0, i).match(whitespaceOnLeft); + if (!match) { + return fixer.insertTextAt(node.start + i, " "); + } + + const whitespace = match[1]; + const start = node.start + i - whitespace.length; + const end = node.start + i; + + return fixer.replaceTextRange([start, end], " "); + } + }); + } + + if (!oneSpaceOnRight.test(code.slice(i + 1, i + 3))) { + context.report({ + node: node, + message: "There should be only a single space between assignment operator '=' and its right side.", + fix(fixer) { + const match = code.slice(i + 1, code.length - 1).match(whitespaceOnRight); + if (!match) { + return fixer.insertTextAt(node.start + i + 1, " "); + } + + const whitespace = match[1]; + const start = node.start + i + 1; + const end = start + whitespace.length; + + return fixer.replaceTextRange([start, end], " "); + } + }); + } } } } diff --git a/lib/utils/string-utils.js b/lib/utils/string-utils.js new file mode 100644 index 0000000..e69de29 diff --git a/test/lib/rules/operator-whitespace/operator-whitespace.js b/test/lib/rules/operator-whitespace/operator-whitespace.js index 216dfe7..69ee7c1 100644 --- a/test/lib/rules/operator-whitespace/operator-whitespace.js +++ b/test/lib/rules/operator-whitespace/operator-whitespace.js @@ -342,8 +342,9 @@ describe("[RULE] operator-whitespace: Rejections", function() { describe("[RULE] operator-whitespace: Fixes", function() { - it("should fix whitespace around assignment operator when fix is enabled", function(done) { + it("should fix whitespace around operators when fix is enabled", function(done) { const testCases = [ + // Assignments { input: "foo \t= bar;\n", output: "foo = bar;\n" @@ -357,9 +358,31 @@ describe("[RULE] operator-whitespace: Fixes", function() { output: "foo = bar;\n" }, { - input: "foo += bar;", + input: "foo += bar;", output: "foo += bar;" + }, + + // // Variable declaration + { + input: "var a = \t\t \"hello\";", + output: "var a = \"hello\";" + }, + { + input: "var a\t \t \t= \"hello\";", + output: "var a = \"hello\";" + + }, + { + input: "var a \t\t = \t\t \"hello\";", + output: "var a = \"hello\";" + }, + { + input: "var a= \"hello\";", + output: "var a = \"hello\";" } + + // Binary expressions + ]; testCases.forEach(testCase => { @@ -373,8 +396,9 @@ describe("[RULE] operator-whitespace: Fixes", function() { fixed.fixedSourceCode.should.equal(toFunction(testCase.output)); fixed.errorMessages.should.be.Array(); fixed.errorMessages.length.should.equal(0); + fixed.fixesApplied.should.be.Array(); - fixed.fixesApplied.length.should.equal(1); + fixed.fixesApplied.length.should.be.aboveOrEqual(1); }); Solium.reset(); diff --git a/test/lib/rules/whitespace/whitespace.js b/test/lib/rules/whitespace/whitespace.js index 226e7ca..8e7439a 100644 --- a/test/lib/rules/whitespace/whitespace.js +++ b/test/lib/rules/whitespace/whitespace.js @@ -543,31 +543,31 @@ describe("[RULE] whitespace: Rejections", function() { code = "call (10\t, 20, 30 ,40,50);", errors = Solium.lint(toFunction(code), userConfig); - + errors.constructor.name.should.equal("Array"); errors.length.should.equal(2); code = "call ({name: \"foo\"\n, age: 20,id: 1 ,dept: \"math\"});", errors = Solium.lint(toFunction(code), userConfig); - + errors.constructor.name.should.equal("Array"); errors.length.should.equal(2); code = "(1 ,2\t,3\n,4);", errors = Solium.lint(toFunction(code), userConfig); - + errors.constructor.name.should.equal("Array"); errors.length.should.equal(3); code = "var (x , y,z\t, foo) = (1,2,3,4);", errors = Solium.lint(toFunction(code), userConfig); - + errors.constructor.name.should.equal("Array"); errors.length.should.equal(2); code = "var (x, y, z, foo) = (1 ,2,3\t,4);", errors = Solium.lint(toFunction(code), userConfig); - + errors.constructor.name.should.equal("Array"); errors.length.should.equal(2); From 55a54a2164648c45dd67deabc69ee99d98416446 Mon Sep 17 00:00:00 2001 From: Ihor Diachenko Date: Mon, 5 Aug 2019 23:41:35 +0300 Subject: [PATCH 3/8] Handle multiline binary expressions --- lib/rules/operator-whitespace.js | 170 +++++++++++++----- .../operator-whitespace.js | 36 +++- 2 files changed, 153 insertions(+), 53 deletions(-) diff --git a/lib/rules/operator-whitespace.js b/lib/rules/operator-whitespace.js index 0997831..19552a2 100644 --- a/lib/rules/operator-whitespace.js +++ b/lib/rules/operator-whitespace.js @@ -134,24 +134,64 @@ module.exports = { let validationRegexOpOnSameLineAsLeftNodeEnd = new RegExp("^[^\\n]*" + opRegExp), strBetweenLeftAndRightNode = sourceCode.getStringBetweenNodes(node.left, node.right); - if (rightNodeStartingLine !== leftNodeEndingLine + 1) { + if (!validationRegexOpOnSameLineAsLeftNodeEnd.test(strBetweenLeftAndRightNode)) { context.report({ node: node, location: { - column: sourceCode.getColumn(node.right), - line: sourceCode.getLine(node.right) + column: sourceCode.getEndingColumn(node.left) }, - message: "In Binary Expressions that span over multiple lines, expression on the right side of the operator (" + node.operator + ") must be exactly 1 line below the line on which the left expression ends." + message: "Operator \"" + node.operator + "\" should be on the line where left side of the Binary expression ends.", + fix(fixer) { + // Reduce whitespace to the left of operator to 1 space + // Insert a linebreak if there are no linebreaks to the right + + const operator = sourceCode.getStringBetweenNodes(node.left, node.right); + const match = operator.match(/^(\s*)/); + if (!match) { + return null; + } + const whitespace = match[1]; + + const start = node.left.end; + const end = node.left.end + whitespace.length; + + const fixes = [fixer.replaceTextRange([start, end], " ")]; + + const rightWhitespace = operator.slice( + whitespace.length + node.operator.length, + operator.length + ); + if (!rightWhitespace.includes("\n")) { + const operatorEnd = end + node.operator.length; + fixes.push(fixer.insertTextAt(operatorEnd, "\n")); + } + + return fixes; + } }); } - if (!validationRegexOpOnSameLineAsLeftNodeEnd.test(strBetweenLeftAndRightNode)) { + if (rightNodeStartingLine !== leftNodeEndingLine + 1) { context.report({ node: node, location: { - column: sourceCode.getEndingColumn(node.left) + column: sourceCode.getColumn(node.right), + line: sourceCode.getLine(node.right) }, - message: "Operator \"" + node.operator + "\" should be on the line where left side of the Binary expression ends." + message: "In Binary Expressions that span over multiple lines, expression on the right side of the operator (" + node.operator + ") must be exactly 1 line below the line on which the left expression ends.", + fix(fixer) { + const operator = sourceCode.getStringBetweenNodes(node.left, node.right); + const match = operator.match(/(\s*)$/); + if (!match) { + return null; + } + const whitespace = match[1]; + + const start = node.right.start - whitespace.length; + const end = node.right.start; + + return fixer.replaceTextRange([start, end], "\n"); + } }); } @@ -165,61 +205,89 @@ module.exports = { leftNode = node.left; } - let strBetweenLeftAndRight = sourceCode.getStringBetweenNodes(leftNode, node.right).split(node.operator), - onlyCharsRegExp = /^[^\s\/]$/; + const [leftOfOperator, rightOfOperator] = sourceCode + .getStringBetweenNodes(leftNode, node.right) + .split(node.operator); + const firstCharOnLeft = leftOfOperator.slice(-1); + const firstCharOnRight = rightOfOperator[0]; + + const onlyCharsRegExp = /^[^\s\/]$/; - if (strBetweenLeftAndRight [0].slice(-1) === " " || strBetweenLeftAndRight [1] [0] === " ") { - if (strBetweenLeftAndRight [0].slice(-1) !== strBetweenLeftAndRight [1] [0]) { + + if (hasWhitespace(firstCharOnLeft) || hasWhitespace(firstCharOnRight)) { + let trimWhitespace = false; + + // Check if either of sides has no whitespace + // and force spacing based on the left side of operator + if (Boolean(firstCharOnLeft) !== Boolean(firstCharOnRight)) { context.report({ node: node, location: { column: sourceCode.getEndingColumn(node.left) + 1 }, - message: "Single space should be either on both sides of '" + node.operator + "' or not at all." - }); - } else { - let secondLastCharOnLeft = strBetweenLeftAndRight [0].slice(-2, -1), - secondCharOnRight = strBetweenLeftAndRight [1] [1]; - - secondLastCharOnLeft && (!onlyCharsRegExp.test(secondLastCharOnLeft)) && context.report({ - node: node, - location: { - column: sourceCode.getEndingColumn(node.left) - }, - message: "There should be a maximum of single space and no comments between left side and '" + node.operator + "'." - }); + message: "Single space should be either on both sides of '" + node.operator + "' or not at all.", + fix(fixer) { + const left = leftOfOperator.slice(-1); + const endOfOperator = node.left.end + node.operator.length; + + if (!left) { + trimWhitespace = true; + return fixer.removeRange([endOfOperator, endOfOperator+1]); + } - secondCharOnRight && (!onlyCharsRegExp.test(secondCharOnRight)) && context.report({ - node: node, - location: { - column: sourceCode.getColumn(node.right) - }, - message: "There should be a maximum of single space and no comments between right side and '" + node.operator + "'." + return fixer.insertTextAt(endOfOperator + 1, " "); + } }); } - return; - } + if (!trimWhitespace) { + const secondLastCharOnLeft = leftOfOperator.slice(-2, -1); + const secondCharOnRight = rightOfOperator[1]; - let firstCharOnLeft = strBetweenLeftAndRight [0].slice(-1), - firstCharOnRight = strBetweenLeftAndRight [1] [0]; + const whitespaceOnLeft = secondLastCharOnLeft && !onlyCharsRegExp.test(secondLastCharOnLeft); + if (leftOfOperator === "\t" || whitespaceOnLeft) { + context.report({ + node: node, + location: { + column: sourceCode.getEndingColumn(node.left) + }, + message: "There should be a maximum of single space and no comments between left side and '" + node.operator + "'." + }); + } - firstCharOnLeft && (!onlyCharsRegExp.test(firstCharOnLeft)) && context.report({ - node: node, - location: { - column: sourceCode.getEndingColumn(node.left) - }, - message: "There should be no comments between left side and '" + node.operator + "'." - }); + const whitespaceOnRight = secondCharOnRight && !onlyCharsRegExp.test(secondCharOnRight); + if (rightOfOperator === "\t" || whitespaceOnRight) { + context.report({ + node: node, + location: { + column: sourceCode.getColumn(node.right) + }, + message: "There should be a maximum of single space and no comments between right side and '" + node.operator + "'." + }); + } + } + } - firstCharOnRight && (!onlyCharsRegExp.test(firstCharOnRight)) && context.report({ - node: node, - location: { - column: sourceCode.getColumn(node.right) - }, - message: "There should be no comments between right side and '" + node.operator + "'." - }); + // Handle comments w/o whitespace + if (firstCharOnLeft === "/") { + context.report({ + node: node, + location: { + column: sourceCode.getEndingColumn(node.left) + }, + message: "There should be no comments between left side and '" + node.operator + "'." + }); + } + if (firstCharOnRight === "/") { + context.report({ + node: node, + location: { + column: sourceCode.getColumn(node.right) + }, + message: "There should be no comments between right side and '" + node.operator + "'." + }); + } } return { @@ -231,3 +299,9 @@ module.exports = { } }; + +const whitespaceRegexp = /\s/; + +function hasWhitespace(str) { + return whitespaceRegexp.test(str); +} diff --git a/test/lib/rules/operator-whitespace/operator-whitespace.js b/test/lib/rules/operator-whitespace/operator-whitespace.js index 69ee7c1..e5a4223 100644 --- a/test/lib/rules/operator-whitespace/operator-whitespace.js +++ b/test/lib/rules/operator-whitespace/operator-whitespace.js @@ -178,7 +178,7 @@ describe("[RULE] operator-whitespace: Rejections", function() { errors = Solium.lint(code [0], userConfig); errors.constructor.name.should.equal("Array"); - errors.length.should.equal(1); + errors.length.should.equal(2); errors = Solium.lint(code [1], userConfig); errors.constructor.name.should.equal("Array"); @@ -230,7 +230,7 @@ describe("[RULE] operator-whitespace: Rejections", function() { errors = Solium.lint(code [13], userConfig); errors.constructor.name.should.equal("Array"); - errors.length.should.equal(1); + errors.length.should.equal(2); errors = Solium.lint(code [14], userConfig); errors.constructor.name.should.equal("Array"); @@ -362,7 +362,7 @@ describe("[RULE] operator-whitespace: Fixes", function() { output: "foo += bar;" }, - // // Variable declaration + // Variable declaration { input: "var a = \t\t \"hello\";", output: "var a = \"hello\";" @@ -379,10 +379,36 @@ describe("[RULE] operator-whitespace: Fixes", function() { { input: "var a= \"hello\";", output: "var a = \"hello\";" - } + }, - // Binary expressions + // Multiline binary expressions + { + input: "if (foobarMotherfuckers (price, 100)&&\n\n\t++crazyCounter) {\n}", + output: "if (foobarMotherfuckers (price, 100)&&\n++crazyCounter) {\n}" + }, + { + input: "if (foobarMotherfuckers (price, 100)\t\n&&++crazyCounter) {\n}", + output: "if (foobarMotherfuckers (price, 100) &&\n++crazyCounter) {\n}" + }, + { + input: "if (foobarMotherfuckers (price, 100)\t\n&&\n\n\n++crazyCounter) {\n}", + output: "if (foobarMotherfuckers (price, 100) &&\n++crazyCounter) {\n}" + }, + + // Binary operators + { + input: "if (foobarMotherfuckers (price, 100) &&crazyCounter) {\n}", + output: "if (foobarMotherfuckers (price, 100) && crazyCounter) {\n}" + }, + { + input: "if (foobarMotherfuckers (price, 100)&& crazyCounter) {\n}", + output: "if (foobarMotherfuckers (price, 100)&&crazyCounter) {\n}" + } + // { + // input: "if (foobarMotherfuckers (price, 100)\t\t\t &&crazyCounter) {\n}", + // output: "if (foobarMotherfuckers (price, 100) && ++crazyCounter) {\n}" + // } ]; testCases.forEach(testCase => { From c3e072f1115f43173f3dbe9d5559d7ffc0b3d7d8 Mon Sep 17 00:00:00 2001 From: Ihor Diachenko Date: Tue, 6 Aug 2019 23:58:58 +0300 Subject: [PATCH 4/8] Some refactoring --- lib/rules/operator-whitespace.js | 150 ++++++++---------- .../operator-whitespace.js | 37 +++-- 2 files changed, 88 insertions(+), 99 deletions(-) diff --git a/lib/rules/operator-whitespace.js b/lib/rules/operator-whitespace.js index 19552a2..da943d2 100644 --- a/lib/rules/operator-whitespace.js +++ b/lib/rules/operator-whitespace.js @@ -112,8 +112,7 @@ module.exports = { } function inspectBinaryExpression(emitted) { - let leftNode, - node = emitted.node; + let node = emitted.node; if (emitted.exit) { return; @@ -126,13 +125,13 @@ module.exports = { // 1. take line no. of both left & right expr. Line no (right) = line (left) + 1 // Take string btw them, should be NO \n before operator. that's it - let rightNodeStartingLine = sourceCode.getLine(node.right), - leftNodeEndingLine = sourceCode.getEndingLine(node.left), - opRegExp = node.operator.replace(/([\+\*\/\|\^])/g, "\\$1"); + const rightNodeStartingLine = sourceCode.getLine(node.right); + const leftNodeEndingLine = sourceCode.getEndingLine(node.left); + const opRegExp = node.operator.replace(/([\+\*\/\|\^])/g, "\\$1"); if (rightNodeStartingLine > leftNodeEndingLine) { - let validationRegexOpOnSameLineAsLeftNodeEnd = new RegExp("^[^\\n]*" + opRegExp), - strBetweenLeftAndRightNode = sourceCode.getStringBetweenNodes(node.left, node.right); + const validationRegexOpOnSameLineAsLeftNodeEnd = new RegExp("^[^\\n]*" + opRegExp); + const strBetweenLeftAndRightNode = sourceCode.getStringBetweenNodes(node.left, node.right); if (!validationRegexOpOnSameLineAsLeftNodeEnd.test(strBetweenLeftAndRightNode)) { context.report({ @@ -198,96 +197,85 @@ module.exports = { return; } - // Handle case where left node is a binary expression and right node may be a literal - if (sourceCode.isASTNode(node.left) && node.left.type === "BinaryExpression"){ - leftNode = node.left.right; - } else { - leftNode = node.left; - } - + // Should we handle operator in comments here? const [leftOfOperator, rightOfOperator] = sourceCode - .getStringBetweenNodes(leftNode, node.right) + .getStringBetweenNodes(node.left, node.right) .split(node.operator); - const firstCharOnLeft = leftOfOperator.slice(-1); - const firstCharOnRight = rightOfOperator[0]; - const onlyCharsRegExp = /^[^\s\/]$/; + // Force spacing based on the left side of operator + const forceNoSpacing = leftOfOperator === ""; + const removeRight = forceNoSpacing && rightOfOperator.length > 0; + const insertSpaceOnRight = !forceNoSpacing && rightOfOperator === ""; + if (removeRight || insertSpaceOnRight) { + context.report({ + node: node, + location: { + column: sourceCode.getEndingColumn(node.left) + 1 + }, + message: "Single space should be either on both sides of '" + node.operator + "' or not at all.", + fix(fixer) { + if (removeRight) { + const endOfOperator = node.left.end + node.operator.length; - if (hasWhitespace(firstCharOnLeft) || hasWhitespace(firstCharOnRight)) { - let trimWhitespace = false; + return fixer.removeRange([endOfOperator, endOfOperator + 1]); + } - // Check if either of sides has no whitespace - // and force spacing based on the left side of operator - if (Boolean(firstCharOnLeft) !== Boolean(firstCharOnRight)) { + return fixer.insertTextAt(node.right.start, " "); + } + }); + } + + if (!forceNoSpacing) { + // Handle comments w\o whitespace + if (sourceCode.isASTNode(node.left) && node.left.type.includes("Comment")) { + context.report({ + node: node, + location: { + column: sourceCode.getEndingColumn(node.left) + }, + message: "There should be no comments between left side and '" + node.operator + "'." + }); + } else if (leftOfOperator === "\t" || leftOfOperator.length > 1) { context.report({ node: node, location: { - column: sourceCode.getEndingColumn(node.left) + 1 + column: sourceCode.getEndingColumn(node.left) }, - message: "Single space should be either on both sides of '" + node.operator + "' or not at all.", + message: "There should be a maximum of single space and no comments between left side and '" + node.operator + "'.", fix(fixer) { - const left = leftOfOperator.slice(-1); - const endOfOperator = node.left.end + node.operator.length; - - if (!left) { - trimWhitespace = true; - return fixer.removeRange([endOfOperator, endOfOperator+1]); - } + const start = node.left.end; + const end = start + leftOfOperator.length; - return fixer.insertTextAt(endOfOperator + 1, " "); + return fixer.replaceTextRange([start, end], " "); } }); } - if (!trimWhitespace) { - const secondLastCharOnLeft = leftOfOperator.slice(-2, -1); - const secondCharOnRight = rightOfOperator[1]; - - const whitespaceOnLeft = secondLastCharOnLeft && !onlyCharsRegExp.test(secondLastCharOnLeft); - if (leftOfOperator === "\t" || whitespaceOnLeft) { - context.report({ - node: node, - location: { - column: sourceCode.getEndingColumn(node.left) - }, - message: "There should be a maximum of single space and no comments between left side and '" + node.operator + "'." - }); - } + if (sourceCode.isASTNode(node.right) && node.right.type.includes("Comment")) { + context.report({ + node: node, + location: { + column: sourceCode.getColumn(node.right) + }, + message: "There should be no comments between right side and '" + node.operator + "'." + }); + } else if (rightOfOperator === "\t" || rightOfOperator.length > 1) { + context.report({ + node: node, + location: { + column: sourceCode.getColumn(node.right) + }, + message: "There should be a maximum of single space and no comments between right side and '" + node.operator + "'.", + fix(fixer) { + const start = node.right.start; + const end = start + rightOfOperator.length; - const whitespaceOnRight = secondCharOnRight && !onlyCharsRegExp.test(secondCharOnRight); - if (rightOfOperator === "\t" || whitespaceOnRight) { - context.report({ - node: node, - location: { - column: sourceCode.getColumn(node.right) - }, - message: "There should be a maximum of single space and no comments between right side and '" + node.operator + "'." - }); - } + return fixer.replaceTextRange([start, end], " "); + } + }); } } - - // Handle comments w/o whitespace - if (firstCharOnLeft === "/") { - context.report({ - node: node, - location: { - column: sourceCode.getEndingColumn(node.left) - }, - message: "There should be no comments between left side and '" + node.operator + "'." - }); - } - - if (firstCharOnRight === "/") { - context.report({ - node: node, - location: { - column: sourceCode.getColumn(node.right) - }, - message: "There should be no comments between right side and '" + node.operator + "'." - }); - } } return { @@ -299,9 +287,3 @@ module.exports = { } }; - -const whitespaceRegexp = /\s/; - -function hasWhitespace(str) { - return whitespaceRegexp.test(str); -} diff --git a/test/lib/rules/operator-whitespace/operator-whitespace.js b/test/lib/rules/operator-whitespace/operator-whitespace.js index e5a4223..1d0a8b6 100644 --- a/test/lib/rules/operator-whitespace/operator-whitespace.js +++ b/test/lib/rules/operator-whitespace/operator-whitespace.js @@ -383,32 +383,39 @@ describe("[RULE] operator-whitespace: Fixes", function() { // Multiline binary expressions { - input: "if (foobarMotherfuckers (price, 100)&&\n\n\t++crazyCounter) {\n}", - output: "if (foobarMotherfuckers (price, 100)&&\n++crazyCounter) {\n}" + input: "if (foo (price, 100)&&\n\n\t++bar) {\n}", + output: "if (foo (price, 100)&&\n++bar) {\n}" }, { - input: "if (foobarMotherfuckers (price, 100)\t\n&&++crazyCounter) {\n}", - output: "if (foobarMotherfuckers (price, 100) &&\n++crazyCounter) {\n}" + input: "if (foo (price, 100)\t\n&&++bar) {\n}", + output: "if (foo (price, 100) &&\n++bar) {\n}" }, { - input: "if (foobarMotherfuckers (price, 100)\t\n&&\n\n\n++crazyCounter) {\n}", - output: "if (foobarMotherfuckers (price, 100) &&\n++crazyCounter) {\n}" + input: "if (foo (price, 100)\t\n&&\n\n\n++bar) {\n}", + output: "if (foo (price, 100) &&\n++bar) {\n}" }, // Binary operators { - input: "if (foobarMotherfuckers (price, 100) &&crazyCounter) {\n}", - output: "if (foobarMotherfuckers (price, 100) && crazyCounter) {\n}" + input: "if (foo (price, 100) &&bar) {\n}", + output: "if (foo (price, 100) && bar) {\n}" }, { - input: "if (foobarMotherfuckers (price, 100)&& crazyCounter) {\n}", - output: "if (foobarMotherfuckers (price, 100)&&crazyCounter) {\n}" + input: "if (foo (price, 100)&& bar) {\n}", + output: "if (foo (price, 100)&&bar) {\n}" + }, + { + input: "if (foo (price, 100)\t&& bar) {\n}", + output: "if (foo (price, 100) && bar) {\n}" + }, + { + input: "if (foo (price, 100)\t\t\t && bar) {\n}", + output: "if (foo (price, 100) && bar) {\n}" + }, + { + input: "if (foo (price, 100)\t\t\t &&bar) {\n}", + output: "if (foo (price, 100) && bar) {\n}" } - - // { - // input: "if (foobarMotherfuckers (price, 100)\t\t\t &&crazyCounter) {\n}", - // output: "if (foobarMotherfuckers (price, 100) && ++crazyCounter) {\n}" - // } ]; testCases.forEach(testCase => { From 32aa50fa0988b0e21c61c6d0c42db296b4d55afa Mon Sep 17 00:00:00 2001 From: Ihor Diachenko Date: Wed, 7 Aug 2019 00:07:04 +0300 Subject: [PATCH 5/8] Some more refactoring --- lib/rules/operator-whitespace.js | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/lib/rules/operator-whitespace.js b/lib/rules/operator-whitespace.js index da943d2..6ddd99f 100644 --- a/lib/rules/operator-whitespace.js +++ b/lib/rules/operator-whitespace.js @@ -227,8 +227,7 @@ module.exports = { } if (!forceNoSpacing) { - // Handle comments w\o whitespace - if (sourceCode.isASTNode(node.left) && node.left.type.includes("Comment")) { + if (isComment(node.left)) { context.report({ node: node, location: { @@ -236,7 +235,7 @@ module.exports = { }, message: "There should be no comments between left side and '" + node.operator + "'." }); - } else if (leftOfOperator === "\t" || leftOfOperator.length > 1) { + } else if (leftOfOperator.length > 0 && leftOfOperator !== " ") { context.report({ node: node, location: { @@ -252,7 +251,7 @@ module.exports = { }); } - if (sourceCode.isASTNode(node.right) && node.right.type.includes("Comment")) { + if (isComment(node.right)) { context.report({ node: node, location: { @@ -260,7 +259,7 @@ module.exports = { }, message: "There should be no comments between right side and '" + node.operator + "'." }); - } else if (rightOfOperator === "\t" || rightOfOperator.length > 1) { + } else if (rightOfOperator.length > 0 && rightOfOperator !== " ") { context.report({ node: node, location: { @@ -278,6 +277,10 @@ module.exports = { } } + function isComment(node) { + return sourceCode.isASTNode(node) && node.type.includes("Comment"); + } + return { BinaryExpression: inspectBinaryExpression, VariableDeclaration: inspectVariableDeclaration, From 60fa4e5c84f95d14ec3fe2f39aeecf8cce8f6cc0 Mon Sep 17 00:00:00 2001 From: Ihor Diachenko Date: Wed, 7 Aug 2019 00:31:28 +0300 Subject: [PATCH 6/8] Handle comments and simplify --- lib/rules/operator-whitespace.js | 28 +++--------------- .../operator-whitespace.js | 29 +++++++++++++++++-- 2 files changed, 30 insertions(+), 27 deletions(-) diff --git a/lib/rules/operator-whitespace.js b/lib/rules/operator-whitespace.js index 6ddd99f..f66d2d1 100644 --- a/lib/rules/operator-whitespace.js +++ b/lib/rules/operator-whitespace.js @@ -227,15 +227,7 @@ module.exports = { } if (!forceNoSpacing) { - if (isComment(node.left)) { - context.report({ - node: node, - location: { - column: sourceCode.getEndingColumn(node.left) - }, - message: "There should be no comments between left side and '" + node.operator + "'." - }); - } else if (leftOfOperator.length > 0 && leftOfOperator !== " ") { + if (leftOfOperator.length > 0 && leftOfOperator !== " ") { context.report({ node: node, location: { @@ -251,15 +243,7 @@ module.exports = { }); } - if (isComment(node.right)) { - context.report({ - node: node, - location: { - column: sourceCode.getColumn(node.right) - }, - message: "There should be no comments between right side and '" + node.operator + "'." - }); - } else if (rightOfOperator.length > 0 && rightOfOperator !== " ") { + if (rightOfOperator.length > 0 && rightOfOperator !== " ") { context.report({ node: node, location: { @@ -267,8 +251,8 @@ module.exports = { }, message: "There should be a maximum of single space and no comments between right side and '" + node.operator + "'.", fix(fixer) { - const start = node.right.start; - const end = start + rightOfOperator.length; + const start = node.right.start - rightOfOperator.length; + const end = node.right.start; return fixer.replaceTextRange([start, end], " "); } @@ -277,10 +261,6 @@ module.exports = { } } - function isComment(node) { - return sourceCode.isASTNode(node) && node.type.includes("Comment"); - } - return { BinaryExpression: inspectBinaryExpression, VariableDeclaration: inspectVariableDeclaration, diff --git a/test/lib/rules/operator-whitespace/operator-whitespace.js b/test/lib/rules/operator-whitespace/operator-whitespace.js index 1d0a8b6..a36182e 100644 --- a/test/lib/rules/operator-whitespace/operator-whitespace.js +++ b/test/lib/rules/operator-whitespace/operator-whitespace.js @@ -396,14 +396,23 @@ describe("[RULE] operator-whitespace: Fixes", function() { }, // Binary operators + // - force no spacing if no spacing on the left + { + input: "if (foo (price, 100)&& bar) {\n}", + output: "if (foo (price, 100)&&bar) {\n}" + }, + + // - add missing space on the right { input: "if (foo (price, 100) &&bar) {\n}", output: "if (foo (price, 100) && bar) {\n}" }, { - input: "if (foo (price, 100)&& bar) {\n}", - output: "if (foo (price, 100)&&bar) {\n}" + input: "if (foo (price, 100)\t\t\t &&bar) {\n}", + output: "if (foo (price, 100) && bar) {\n}" }, + + // - trim extra whitespace { input: "if (foo (price, 100)\t&& bar) {\n}", output: "if (foo (price, 100) && bar) {\n}" @@ -412,8 +421,22 @@ describe("[RULE] operator-whitespace: Fixes", function() { input: "if (foo (price, 100)\t\t\t && bar) {\n}", output: "if (foo (price, 100) && bar) {\n}" }, + + // - trim comments { - input: "if (foo (price, 100)\t\t\t &&bar) {\n}", + input: "if (foo (price, 100) /** **/ && bar) {\n}", + output: "if (foo (price, 100) && bar) {\n}" + }, + { + input: "if (foo (price, 100) && /** **/ bar) {\n}", + output: "if (foo (price, 100) && bar) {\n}" + }, + { + input: "if (foo (price, 100) /** **/ && /** **/ bar) {\n}", + output: "if (foo (price, 100) && bar) {\n}" + }, + { + input: "if (foo (price, 100) /** **/ &&bar) {\n}", output: "if (foo (price, 100) && bar) {\n}" } ]; From 9b2721e139a0f8320b10bbe7a04d9ab5a6430c37 Mon Sep 17 00:00:00 2001 From: Ihor Diachenko Date: Wed, 7 Aug 2019 00:48:43 +0300 Subject: [PATCH 7/8] Improve assignment --- lib/rules/operator-whitespace.js | 43 ++++++++++--------- .../operator-whitespace.js | 12 ++++++ 2 files changed, 35 insertions(+), 20 deletions(-) diff --git a/lib/rules/operator-whitespace.js b/lib/rules/operator-whitespace.js index f66d2d1..1357fed 100644 --- a/lib/rules/operator-whitespace.js +++ b/lib/rules/operator-whitespace.js @@ -5,6 +5,14 @@ "use strict"; +// TODO: escape operators in comments? const op = node.operator.replace(/([\+\*\/\|\^])/g, "\\$1"); +/** + * node.operator is refined here by adding backslash before all the 'special' characters. + * 'special' chars are those chars that are part of solidity assignment operators and, if used without backslash in JS RegExp, + * behave as wildcard characters. So to make sure they're treated as simple strings, we add '\' before them. + * As of today, these chars include: * / + | ^ + */ + module.exports = { meta: { @@ -26,32 +34,27 @@ module.exports = { let sourceCode = context.getSourceCode(); function inspectAssignmentExpression(emitted) { - /** - * node.operator is refined here by adding backslash before all the 'special' characters. - * 'special' chars are thos chars that are part of solidity assignment operators and, if used without backslash in JS RegExp, - * behave as wildcard characters. So to make sure they're treated as simple strings, we add '\' before them. - * As of today, these chars include: * / + | ^ - */ const node = emitted.node; - const op = node.operator.replace(/([\+\*\/\|\^])/g, "\\$1"); - const opLength = node.operator.length; - if (emitted.exit) { return; } - // If expression is 'abc *= def;', then charsAfterLeftNode will contain ' *= d'. - const charsAfterLeftNode = sourceCode.getNextChars(node.left, 3 + opLength); - const validationRegexp = new RegExp("^ " + op + " [^\\s]$"); + const [leftOfOperator, rightOfOperator] = sourceCode + .getStringBetweenNodes(node.left, node.right) + .split(node.operator); - (!validationRegexp.test(charsAfterLeftNode)) && context.report({ - node: node.left, - message: "Assignment operator must have exactly single space on both sides of it.", - fix(fixer) { - const fixed = sourceCode.getText(node).replace(/\s+/g, " "); - return [fixer.replaceText(node, fixed)]; - } - }); + if (leftOfOperator !== " " || rightOfOperator !== " ") { + context.report({ + node: node, + message: "Assignment operator must have exactly single space on both sides of it.", + fix(fixer) { + return fixer.replaceTextRange( + [node.left.end, node.right.start], + ` ${node.operator} ` + ); + } + }); + } } //statement like `var x = 10` doesn't come under AssignmentExpression, so needs to be checked separately diff --git a/test/lib/rules/operator-whitespace/operator-whitespace.js b/test/lib/rules/operator-whitespace/operator-whitespace.js index a36182e..a3c881b 100644 --- a/test/lib/rules/operator-whitespace/operator-whitespace.js +++ b/test/lib/rules/operator-whitespace/operator-whitespace.js @@ -361,6 +361,18 @@ describe("[RULE] operator-whitespace: Fixes", function() { input: "foo += bar;", output: "foo += bar;" }, + { + input: "foo /** **/ += bar;", + output: "foo += bar;" + }, + { + input: "foo += /** **/ bar;", + output: "foo += bar;" + }, + { + input: "foo /** **/ += /** **/ bar;", + output: "foo += bar;" + }, // Variable declaration { From b235b63cb23f637edd4b91de951838d99063b3c4 Mon Sep 17 00:00:00 2001 From: Ihor Diachenko Date: Wed, 7 Aug 2019 13:03:27 +0300 Subject: [PATCH 8/8] Escape comments --- lib/rules/operator-whitespace.js | 47 ++++++++++--------- .../operator-whitespace.js | 28 +++++++++++ 2 files changed, 54 insertions(+), 21 deletions(-) diff --git a/lib/rules/operator-whitespace.js b/lib/rules/operator-whitespace.js index 1357fed..7bcdcd6 100644 --- a/lib/rules/operator-whitespace.js +++ b/lib/rules/operator-whitespace.js @@ -5,13 +5,7 @@ "use strict"; -// TODO: escape operators in comments? const op = node.operator.replace(/([\+\*\/\|\^])/g, "\\$1"); -/** - * node.operator is refined here by adding backslash before all the 'special' characters. - * 'special' chars are those chars that are part of solidity assignment operators and, if used without backslash in JS RegExp, - * behave as wildcard characters. So to make sure they're treated as simple strings, we add '\' before them. - * As of today, these chars include: * / + | ^ - */ +const solparse = require("solparse"); module.exports = { @@ -39,9 +33,9 @@ module.exports = { return; } - const [leftOfOperator, rightOfOperator] = sourceCode - .getStringBetweenNodes(node.left, node.right) - .split(node.operator); + const code = sourceCode.getStringBetweenNodes(node.left, node.right); + const escapedCode = commentsToWhitespace(code); + const [leftOfOperator, rightOfOperator] = escapedCode.split(node.operator); if (leftOfOperator !== " " || rightOfOperator !== " ") { context.report({ @@ -59,26 +53,28 @@ module.exports = { //statement like `var x = 10` doesn't come under AssignmentExpression, so needs to be checked separately function inspectVariableDeclaration(emitted) { - let node = emitted.node, code = sourceCode.getText(node); + const node = emitted.node; if (emitted.exit) { return; } + const escapedCode = commentsToWhitespace(sourceCode.getText(node)); + //if a particular character is '=', check its left and right for single space const oneSpaceOnLeft = /^[^\/\s] $/; const whitespaceOnLeft = /(\s+)$/; const oneSpaceOnRight = /^ [^\/\s]$/; const whitespaceOnRight = /^(\s+)/; - for (let i = 2; i < code.length; i++) { - if (code [i] === "=") { - if (!oneSpaceOnLeft.test(code.slice(i - 2, i))) { + for (let i = 2; i < escapedCode.length; i++) { + if (escapedCode [i] === "=") { + if (!oneSpaceOnLeft.test(escapedCode.slice(i - 2, i))) { context.report({ node: node, message: "There should be only a single space between assignment operator '=' and its left side.", fix(fixer) { - const match = code.slice(0, i).match(whitespaceOnLeft); + const match = escapedCode.slice(0, i).match(whitespaceOnLeft); if (!match) { return fixer.insertTextAt(node.start + i, " "); } @@ -92,12 +88,12 @@ module.exports = { }); } - if (!oneSpaceOnRight.test(code.slice(i + 1, i + 3))) { + if (!oneSpaceOnRight.test(escapedCode.slice(i + 1, i + 3))) { context.report({ node: node, message: "There should be only a single space between assignment operator '=' and its right side.", fix(fixer) { - const match = code.slice(i + 1, code.length - 1).match(whitespaceOnRight); + const match = escapedCode.slice(i + 1, escapedCode.length - 1).match(whitespaceOnRight); if (!match) { return fixer.insertTextAt(node.start + i + 1, " "); } @@ -200,10 +196,10 @@ module.exports = { return; } - // Should we handle operator in comments here? - const [leftOfOperator, rightOfOperator] = sourceCode - .getStringBetweenNodes(node.left, node.right) - .split(node.operator); + + const code = sourceCode.getStringBetweenNodes(node.left, node.right); + const escapedCode = commentsToWhitespace(code); + const [leftOfOperator, rightOfOperator] = escapedCode.split(node.operator); // Force spacing based on the left side of operator const forceNoSpacing = leftOfOperator === ""; @@ -273,3 +269,12 @@ module.exports = { } }; + +function commentsToWhitespace(code) { + const comments = solparse.parseComments(code); + comments.forEach(comment => { + code = code.replace(comment.text, " ".repeat(comment.text.length)); + }); + + return code; +} diff --git a/test/lib/rules/operator-whitespace/operator-whitespace.js b/test/lib/rules/operator-whitespace/operator-whitespace.js index a3c881b..7a5ad43 100644 --- a/test/lib/rules/operator-whitespace/operator-whitespace.js +++ b/test/lib/rules/operator-whitespace/operator-whitespace.js @@ -373,6 +373,10 @@ describe("[RULE] operator-whitespace: Fixes", function() { input: "foo /** **/ += /** **/ bar;", output: "foo += bar;" }, + { + input: "foo /** += **/ += /** += **/ bar;", + output: "foo += bar;" + }, // Variable declaration { @@ -392,6 +396,26 @@ describe("[RULE] operator-whitespace: Fixes", function() { input: "var a= \"hello\";", output: "var a = \"hello\";" }, + { + input: "var a/** **/ = \"hello\";", + output: "var a = \"hello\";" + }, + { + input: "var a = /** **/ \"hello\";", + output: "var a = \"hello\";" + }, + { + input: "var a /** **/ = /** **/ \"hello\";", + output: "var a = \"hello\";" + }, + { + input: "var a /** = **/ = /** = **/ \"hello\";", + output: "var a = \"hello\";" + }, + { + input: "var a/****/=/****/\"hello\";", + output: "var a = \"hello\";" + }, // Multiline binary expressions { @@ -450,6 +474,10 @@ describe("[RULE] operator-whitespace: Fixes", function() { { input: "if (foo (price, 100) /** **/ &&bar) {\n}", output: "if (foo (price, 100) && bar) {\n}" + }, + { + input: "if (foo (price, 100) /** && **/ && /** && d**/ bar) {\n}", + output: "if (foo (price, 100) && bar) {\n}" } ];