From 1744faba3c93c869f7dbbf0a704d32e2692d6856 Mon Sep 17 00:00:00 2001 From: Milos Djermanovic Date: Fri, 25 Oct 2019 20:07:44 +0200 Subject: [PATCH] Fix: operator-assignment removes and duplicates comments (#12485) --- lib/rules/operator-assignment.js | 11 ++++ tests/lib/rules/operator-assignment.js | 80 ++++++++++++++++++++++++++ 2 files changed, 91 insertions(+) diff --git a/lib/rules/operator-assignment.js b/lib/rules/operator-assignment.js index ea6d2905c46..e294668b42b 100644 --- a/lib/rules/operator-assignment.js +++ b/lib/rules/operator-assignment.js @@ -150,6 +150,11 @@ module.exports = { const leftText = sourceCode.getText().slice(node.range[0], equalsToken.range[0]); const rightText = sourceCode.getText().slice(operatorToken.range[1], node.right.range[1]); + // Check for comments that would be removed. + if (sourceCode.commentsExistBetween(equalsToken, operatorToken)) { + return null; + } + return fixer.replaceText(node, `${leftText}${expr.operator}=${rightText}`); } return null; @@ -182,11 +187,17 @@ module.exports = { messageId: "unexpected", fix(fixer) { if (canBeFixed(node.left)) { + const firstToken = sourceCode.getFirstToken(node); const operatorToken = getOperatorToken(node); const leftText = sourceCode.getText().slice(node.range[0], operatorToken.range[0]); const newOperator = node.operator.slice(0, -1); let rightText; + // Check for comments that would be duplicated. + if (sourceCode.commentsExistBetween(firstToken, operatorToken)) { + return null; + } + // If this change would modify precedence (e.g. `foo *= bar + 1` => `foo = foo * (bar + 1)`), parenthesize the right side. if ( astUtils.getPrecedence(node.right) <= astUtils.getPrecedence({ type: "BinaryExpression", operator: newOperator }) && diff --git a/tests/lib/rules/operator-assignment.js b/tests/lib/rules/operator-assignment.js index 06a0e40e707..2f87532535e 100644 --- a/tests/lib/rules/operator-assignment.js +++ b/tests/lib/rules/operator-assignment.js @@ -191,6 +191,86 @@ ruleTester.run("operator-assignment", rule, { code: "foo[5] = foo[5] / baz", output: "foo[5] /= baz", // this is ok because 5 is a literal, so toString won't get called errors: EXPECTED_OPERATOR_ASSIGNMENT + }, { + code: "/*1*/x/*2*/./*3*/y/*4*/= x.y +/*5*/z/*6*/./*7*/w/*8*/;", + output: "/*1*/x/*2*/./*3*/y/*4*/+=/*5*/z/*6*/./*7*/w/*8*/;", // these comments are preserved + options: ["always"], + errors: EXPECTED_OPERATOR_ASSIGNMENT + }, { + code: "x // 1\n . // 2\n y // 3\n = x.y + //4\n z //5\n . //6\n w;", + output: "x // 1\n . // 2\n y // 3\n += //4\n z //5\n . //6\n w;", // these comments are preserved + options: ["always"], + errors: EXPECTED_OPERATOR_ASSIGNMENT + }, { + code: "x = /*1*/ x + y", + output: null, // not fixed; fixing would remove this comment + options: ["always"], + errors: EXPECTED_OPERATOR_ASSIGNMENT + }, { + code: "x = //1\n x + y", + output: null, // not fixed; fixing would remove this comment + options: ["always"], + errors: EXPECTED_OPERATOR_ASSIGNMENT + }, { + code: "x.y = x/*1*/.y + z", + output: null, // not fixed; fixing would remove this comment + options: ["always"], + errors: EXPECTED_OPERATOR_ASSIGNMENT + }, { + code: "x.y = x. //1\n y + z", + output: null, // not fixed; fixing would remove this comment + options: ["always"], + errors: EXPECTED_OPERATOR_ASSIGNMENT + }, { + code: "x = x /*1*/ + y", + output: null, // not fixed; fixing would remove this comment + options: ["always"], + errors: EXPECTED_OPERATOR_ASSIGNMENT + }, { + code: "x = x //1\n + y", + output: null, // not fixed; fixing would remove this comment + options: ["always"], + errors: EXPECTED_OPERATOR_ASSIGNMENT + }, { + code: "/*1*/x +=/*2*/y/*3*/;", + output: "/*1*/x = x +/*2*/y/*3*/;", // these comments are preserved and not duplicated + options: ["never"], + errors: UNEXPECTED_OPERATOR_ASSIGNMENT + }, { + code: "x +=//1\n y", + output: "x = x +//1\n y", // this comment is preserved and not duplicated + options: ["never"], + errors: UNEXPECTED_OPERATOR_ASSIGNMENT + }, { + code: "(/*1*/x += y)", + output: "(/*1*/x = x + y)", // this comment is preserved and not duplicated + options: ["never"], + errors: UNEXPECTED_OPERATOR_ASSIGNMENT + }, { + code: "x/*1*/+= y", + output: null, // not fixed; fixing would duplicate this comment + options: ["never"], + errors: UNEXPECTED_OPERATOR_ASSIGNMENT + }, { + code: "x //1\n += y", + output: null, // not fixed; fixing would duplicate this comment + options: ["never"], + errors: UNEXPECTED_OPERATOR_ASSIGNMENT + }, { + code: "(/*1*/x) += y", + output: null, // not fixed; fixing would duplicate this comment + options: ["never"], + errors: UNEXPECTED_OPERATOR_ASSIGNMENT + }, { + code: "x/*1*/.y += z", + output: null, // not fixed; fixing would duplicate this comment + options: ["never"], + errors: UNEXPECTED_OPERATOR_ASSIGNMENT + }, { + code: "x.//1\n y += z", + output: null, // not fixed; fixing would duplicate this comment + options: ["never"], + errors: UNEXPECTED_OPERATOR_ASSIGNMENT }, { code: "(foo.bar) ^= ((((((((((((((((baz))))))))))))))))", output: "(foo.bar) = (foo.bar) ^ ((((((((((((((((baz))))))))))))))))",