Skip to content

Commit

Permalink
Fix: don't modify operator precedence in operator-assignment autofixer (
Browse files Browse the repository at this point in the history
#8358)

* Fix: don't modify operator precedence in operator-assignment autofixer

Previously, the operator-assignment autofixer could sometimes modify semantics or produce a syntax error due to different operator precedence. This commit updates the fixer to surround the right side of an assignment with parentheses if it has lower precedence than its new neighbor.

* Add additional test
  • Loading branch information
not-an-aardvark committed Mar 31, 2017
1 parent f88375f commit 3342e9f
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 3 deletions.
21 changes: 19 additions & 2 deletions lib/rules/operator-assignment.js
Expand Up @@ -4,6 +4,12 @@
*/
"use strict";

//------------------------------------------------------------------------------
// Requirements
//------------------------------------------------------------------------------

const astUtils = require("../ast-utils");

//------------------------------------------------------------------------------
// Helpers
//------------------------------------------------------------------------------
Expand Down Expand Up @@ -171,9 +177,20 @@ module.exports = {
if (canBeFixed(node.left)) {
const operatorToken = getOperatorToken(node);
const leftText = sourceCode.getText().slice(node.range[0], operatorToken.range[0]);
const rightText = sourceCode.getText().slice(operatorToken.range[1], node.range[1]);
const newOperator = node.operator.slice(0, -1);
let rightText;

// 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 }) &&
!astUtils.isParenthesised(sourceCode, node.right)
) {
rightText = `${sourceCode.text.slice(operatorToken.range[1], node.right.range[0])}(${sourceCode.getText(node.right)})`;
} else {
rightText = sourceCode.text.slice(operatorToken.range[1], node.range[1]);
}

return fixer.replaceText(node, `${leftText}= ${leftText}${node.operator.slice(0, -1)}${rightText}`);
return fixer.replaceText(node, `${leftText}= ${leftText}${newOperator}${rightText}`);
}
return null;
}
Expand Down
28 changes: 27 additions & 1 deletion tests/lib/rules/operator-assignment.js
Expand Up @@ -81,7 +81,8 @@ ruleTester.run("operator-assignment", rule, {
"x = x === y",
"x = x !== y",
"x = x && y",
"x = x || y"
"x = x || y",
"x = x * y + z"
],

invalid: [{
Expand Down Expand Up @@ -214,6 +215,31 @@ ruleTester.run("operator-assignment", rule, {
output: "foo = foo ** bar",
options: ["never"],
errors: UNEXPECTED_OPERATOR_ASSIGNMENT
}, {
code: "foo *= bar + 1",
output: "foo = foo * (bar + 1)",
options: ["never"],
errors: UNEXPECTED_OPERATOR_ASSIGNMENT
}, {
code: "foo -= bar - baz",
output: "foo = foo - (bar - baz)",
options: ["never"],
errors: UNEXPECTED_OPERATOR_ASSIGNMENT
}, {
code: "foo += bar + baz",
output: "foo = foo + (bar + baz)", // addition is not associative in JS, e.g. (1 + 2) + '3' !== 1 + (2 + '3')
options: ["never"],
errors: UNEXPECTED_OPERATOR_ASSIGNMENT
}, {
code: "foo += bar = 1",
output: "foo = foo + (bar = 1)",
options: ["never"],
errors: UNEXPECTED_OPERATOR_ASSIGNMENT
}, {
code: "foo *= (bar + 1)",
output: "foo = foo * (bar + 1)",
options: ["never"],
errors: UNEXPECTED_OPERATOR_ASSIGNMENT
}]

});

0 comments on commit 3342e9f

Please sign in to comment.