Skip to content

Commit

Permalink
Update: add fixer for operator-assignment (#7517)
Browse files Browse the repository at this point in the history
  • Loading branch information
@not-an-aardvark @nzakas
not-an-aardvark authored and nzakas committed Nov 25, 2016
1 parent faf5f56 commit 7185567
Show file tree
Hide file tree
Showing 3 changed files with 141 additions and 76 deletions.
2 changes: 2 additions & 0 deletions docs/rules/operator-assignment.md
View file
@@ -1,5 +1,7 @@
# require or disallow assignment operator shorthand where possible (operator-assignment)

(fixable) The `--fix` option on the [command line](../user-guide/command-line-interface#fix) automatically fixes problems reported by this rule.

JavaScript provides shorthand operators that combine variable assignment and some simple mathematical operations. For example, `x = x + 4` can be shortened to `x += 4`. The supported shorthand forms are as follows:

```text
Expand Down
74 changes: 66 additions & 8 deletions lib/rules/operator-assignment.js
View file
Expand Up @@ -70,6 +70,17 @@ function same(a, b) {
}
}

/**
* Determines if the left side of a node can be safely fixed (i.e. if it activates the same getters/setters and)
* toString calls regardless of whether assignment shorthand is used)
* @param {ASTNode} node The node on the left side of the expression
* @returns {boolean} `true` if the node can be fixed
*/
function canBeFixed(node) {
return node.type === "Identifier" ||
node.type === "MemberExpression" && node.object.type === "Identifier" && (!node.computed || node.property.type === "Literal");
}

module.exports = {
meta: {
docs: {
Expand All @@ -82,11 +93,24 @@ module.exports = {
{
enum: ["always", "never"]
}
]
],

fixable: "code"
},

create(context) {

const sourceCode = context.getSourceCode();

/**
* Returns the operator token of an AssignmentExpression or BinaryExpression
* @param {ASTNode} node An AssignmentExpression or BinaryExpression node
* @returns {Token} The operator token in the node
*/
function getOperatorToken(node) {
return sourceCode.getTokensBetween(node.left, node.right).find(token => token.value === node.operator);
}

/**
* Ensures that an assignment uses the shorthand form where possible.
* @param {ASTNode} node An AssignmentExpression node.
Expand All @@ -101,13 +125,34 @@ module.exports = {
const expr = node.right;
const operator = expr.operator;

if (isCommutativeOperatorWithShorthand(operator)) {
if (same(left, expr.left) || same(left, expr.right)) {
context.report(node, "Assignment can be replaced with operator assignment.");
}
} else if (isNonCommutativeOperatorWithShorthand(operator)) {
if (isCommutativeOperatorWithShorthand(operator) || isNonCommutativeOperatorWithShorthand(operator)) {
if (same(left, expr.left)) {
context.report(node, "Assignment can be replaced with operator assignment.");
context.report({
node,
message: "Assignment can be replaced with operator assignment.",
fix(fixer) {
if (canBeFixed(left)) {
const equalsToken = getOperatorToken(node);
const operatorToken = getOperatorToken(expr);
const leftText = sourceCode.getText().slice(node.range[0], equalsToken.range[0]);
const rightText = sourceCode.getText().slice(operatorToken.range[1], node.range[1]);

return fixer.replaceText(node, `${leftText}${expr.operator}=${rightText}`);
}
return null;
}
});
} else if (same(left, expr.right) && isCommutativeOperatorWithShorthand(operator)) {

/*
* This case can't be fixed safely.
* If `a` and `b` both have custom valueOf() behavior, then fixing `a = b * a` to `a *= b` would
* change the execution order of the valueOf() functions.
*/
context.report({
node,
message: "Assignment can be replaced with operator assignment."
});
}
}
}
Expand All @@ -119,7 +164,20 @@ module.exports = {
*/
function prohibit(node) {
if (node.operator !== "=") {
context.report(node, "Unexpected operator assignment shorthand.");
context.report({
node,
message: "Unexpected operator assignment shorthand.",
fix(fixer) {
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]);

return fixer.replaceText(node, `${leftText}= ${leftText}${node.operator.slice(0, -1)}${rightText}`);
}
return null;
}
});
}
}

Expand Down
141 changes: 73 additions & 68 deletions tests/lib/rules/operator-assignment.js
View file
Expand Up @@ -18,6 +18,9 @@ const rule = require("../../../lib/rules/operator-assignment"),

const ruleTester = new RuleTester();

const EXPECTED_OPERATOR_ASSIGNMENT = [{ message: "Assignment can be replaced with operator assignment.", type: "AssignmentExpression" }];
const UNEXPECTED_OPERATOR_ASSIGNMENT = [{ message: "Unexpected operator assignment shorthand.", type: "AssignmentExpression" }];

ruleTester.run("operator-assignment", rule, {

valid: [
Expand Down Expand Up @@ -77,108 +80,110 @@ ruleTester.run("operator-assignment", rule, {

invalid: [{
code: "x = x + y",
errors: [{
message: "Assignment can be replaced with operator assignment.",
type: "AssignmentExpression"
}]
output: "x += y",
errors: EXPECTED_OPERATOR_ASSIGNMENT
}, {
code: "x = x - y",
errors: [{
message: "Assignment can be replaced with operator assignment.",
type: "AssignmentExpression"
}]
output: "x -= y",
errors: EXPECTED_OPERATOR_ASSIGNMENT
}, {
code: "x = x * y",
errors: [{
message: "Assignment can be replaced with operator assignment.",
type: "AssignmentExpression"
}]
output: "x *= y",
errors: EXPECTED_OPERATOR_ASSIGNMENT
}, {
code: "x = y * x",
errors: [{
message: "Assignment can be replaced with operator assignment.",
type: "AssignmentExpression"
}]
output: "x = y * x", // not fixed (possible change in behavior if y and x have valueOf() functions)
errors: EXPECTED_OPERATOR_ASSIGNMENT
}, {
code: "x = (y * z) * x",
errors: [{
message: "Assignment can be replaced with operator assignment.",
type: "AssignmentExpression"
}]
output: "x = (y * z) * x", // not fixed (possible change in behavior if y/z and x have valueOf() functions)
errors: EXPECTED_OPERATOR_ASSIGNMENT
}, {
code: "x = x / y",
errors: [{
message: "Assignment can be replaced with operator assignment.",
type: "AssignmentExpression"
}]
output: "x /= y",
errors: EXPECTED_OPERATOR_ASSIGNMENT
}, {
code: "x = x % y",
errors: [{
message: "Assignment can be replaced with operator assignment.",
type: "AssignmentExpression"
}]
output: "x %= y",
errors: EXPECTED_OPERATOR_ASSIGNMENT
}, {
code: "x = x << y",
errors: [{
message: "Assignment can be replaced with operator assignment.",
type: "AssignmentExpression"
}]
output: "x <<= y",
errors: EXPECTED_OPERATOR_ASSIGNMENT
}, {
code: "x = x >> y",
errors: [{
message: "Assignment can be replaced with operator assignment.",
type: "AssignmentExpression"
}]
output: "x >>= y",
errors: EXPECTED_OPERATOR_ASSIGNMENT
}, {
code: "x = x >>> y",
errors: [{
message: "Assignment can be replaced with operator assignment.",
type: "AssignmentExpression"
}]
output: "x >>>= y",
errors: EXPECTED_OPERATOR_ASSIGNMENT
}, {
code: "x = x & y",
errors: [{
message: "Assignment can be replaced with operator assignment.",
type: "AssignmentExpression"
}]
output: "x &= y",
errors: EXPECTED_OPERATOR_ASSIGNMENT
}, {
code: "x = x ^ y",
errors: [{
message: "Assignment can be replaced with operator assignment.",
type: "AssignmentExpression"
}]
output: "x ^= y",
errors: EXPECTED_OPERATOR_ASSIGNMENT
}, {
code: "x = x | y",
errors: [{
message: "Assignment can be replaced with operator assignment.",
type: "AssignmentExpression"
}]
output: "x |= y",
errors: EXPECTED_OPERATOR_ASSIGNMENT
}, {
code: "x[0] = x[0] - y",
errors: [{
message: "Assignment can be replaced with operator assignment.",
type: "AssignmentExpression"
}]
output: "x[0] -= y",
errors: EXPECTED_OPERATOR_ASSIGNMENT
}, {
code: "x.y[z['a']][0].b = x.y[z['a']][0].b * 2",
errors: [{
message: "Assignment can be replaced with operator assignment.",
type: "AssignmentExpression"
}]
output: "x.y[z['a']][0].b = x.y[z['a']][0].b * 2", // not fixed; might activate getters more than before
errors: EXPECTED_OPERATOR_ASSIGNMENT
}, {
code: "x = x + y",
output: "x += y",
options: ["always"],
errors: [{
message: "Assignment can be replaced with operator assignment.",
type: "AssignmentExpression"
}]
errors: EXPECTED_OPERATOR_ASSIGNMENT
}, {
code: "x += y",
output: "x = x + y",
options: ["never"],
errors: UNEXPECTED_OPERATOR_ASSIGNMENT
}, {
code: "foo.bar = foo.bar + baz",
output: "foo.bar += baz",
errors: EXPECTED_OPERATOR_ASSIGNMENT
}, {
code: "foo.bar += baz",
output: "foo.bar = foo.bar + baz",
options: ["never"],
errors: UNEXPECTED_OPERATOR_ASSIGNMENT
}, {
code: "foo.bar.baz = foo.bar.baz + qux",
output: "foo.bar.baz = foo.bar.baz + qux", // not fixed; fixing would cause a foo.bar getter to activate once rather than twice
errors: EXPECTED_OPERATOR_ASSIGNMENT
}, {
code: "foo.bar.baz += qux",
output: "foo.bar.baz += qux", // not fixed; fixing would cause a foo.bar getter to activate twice rather than once
options: ["never"],
errors: UNEXPECTED_OPERATOR_ASSIGNMENT
}, {
code: "foo[bar] = foo[bar] + baz",
output: "foo[bar] = foo[bar] + baz", // not fixed; fixing would cause bar.toString() to get called once instead of twice
errors: EXPECTED_OPERATOR_ASSIGNMENT
}, {
code: "foo[bar] >>>= baz",
output: "foo[bar] >>>= baz", // not fixed; fixing would cause bar.toString() to get called twice instead of once
options: ["never"],
errors: UNEXPECTED_OPERATOR_ASSIGNMENT
}, {
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: "(foo.bar) ^= ((((((((((((((((baz))))))))))))))))",
output: "(foo.bar) = (foo.bar) ^ ((((((((((((((((baz))))))))))))))))",
options: ["never"],
errors: [{
message: "Unexpected operator assignment shorthand.",
type: "AssignmentExpression"
}]
errors: UNEXPECTED_OPERATOR_ASSIGNMENT
}]

});

0 comments on commit 7185567

Please sign in to comment.