Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shorten assignments to itself #230

Merged
merged 16 commits into from
Nov 13, 2016
132 changes: 132 additions & 0 deletions packages/babel-plugin-minify-simplify/__tests__/simplify-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2352,4 +2352,136 @@ describe("simplify-plugin", () => {
const expected = source;
expect(transform(source)).toBe(expected);
});

it("should simplify assignments", () => {

const source = unpad(`
x = x + 1,
x = x - 1,
x = x * 1,
x = x % 1,
x = x << 1,
x = x >> 1,
x = x >>> 1,
x = x & 1,
x = x | 1,
x = x ^ 1,
x = x / 1,
x = x ** 1;
`);
const expected = unpad(`
++x,
--x,
x *= 1,
x %= 1,
x <<= 1,
x >>= 1,
x >>>= 1,
x &= 1,
x |= 1,
x ^= 1,
x /= 1,
x **= 1;
`).replace(/\s+/g, " ");

expect(transform(source)).toBe(expected);
});

it("should simplify assignments 2", () => {

const source = unpad(`
foo = foo + bar,
foo = foo * function(){},
foo += 123,
foo = 1 + foo,
x = x = x + 1,
foo = foo + bar + baz
`);
const expected = unpad(`
foo += bar,
foo *= function () {},
foo += 123,
foo = 1 + foo,
x = ++x,
foo = foo + bar + baz;
`).replace(/\s+/g, " ");

expect(transform(source)).toBe(expected);
});

it("should simplify assignments w. member expressions", () => {

const source = unpad(`
foo.bar = foo.bar + 1,
foo.bar = foo.bar + 2,
foo["x"] = foo[x] + 2,
foo[x] = foo[x] + 2,
foo[x] = foo["x"] + 2,
foo["x"] = foo["x"] + 2,
foo[1] = foo["1"] + 2,
foo["bar"] = foo["bar"] + 2,
foo[bar()] = foo[bar()] + 2,
foo[""] = foo[""] + 2,
foo[2] = foo[2] + 2,
foo[{}] = foo[{}] + 1,
foo[function(){}] = foo[function(){}] + 1,
foo[false] = foo[false] + 1,
foo.bar.baz = foo.bar.baz + 321,
this.hello = this.hello + 1,
foo[null] = foo[null] + 1,
foo[undefined] = foo[undefined] + 1,
foo.bar = foo.bar || {};
`);
// TODO: foo[void 0] = foo[void 0] + 1;
const expected = unpad(`
++foo.bar,
foo.bar += 2,
foo["x"] = foo[x] + 2,
foo[x] += 2,
foo[x] = foo["x"] + 2,
foo["x"] += 2,
foo[1] += 2,
foo["bar"] += 2,
foo[bar()] = foo[bar()] + 2,
foo[""] += 2,
foo[2] += 2,
foo[{}] = foo[{}] + 1,
foo[function () {}] = foo[function () {}] + 1,
++foo[false],
foo.bar.baz += 321,
++this.hello,
++foo[null],
++foo[undefined],
foo.bar = foo.bar || {};
`).replace(/\s+/g, " ");

expect(transform(source)).toBe(expected);
});

it("should simplify assignments w. super", () => {

const source = unpad(`
class Foo {
foo() {
super.foo = super.foo + 1;
}
};
`);
const expected = unpad(`
class Foo {
foo() {
++super.foo;
}
};
`);

expect(transform(source)).toBe(expected);
});

it("should not simplify assignments w. template literals", () => {

const source = unpad("foo[`x`] = foo[`x`] + 1;");

expect(transform(source)).toBe(source);
});
});
107 changes: 107 additions & 0 deletions packages/babel-plugin-minify-simplify/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,62 @@ module.exports = ({ types: t }) => {
const or = (a, b) => t.logicalExpression("||", a, b);
const and = (a, b) => t.logicalExpression("&&", a, b);

const operators = new Set([
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to consider splitting plugins into multiple modules. This kind of mixing of helpers and supplemental stuff is not great ^. I think we could even strive for 1 type-of-transform per file.

"+", "-", "*", "%",
"<<", ">>", ">>>",
"&", "|", "^", "/",
"**"
]);

const updateOperators = new Set([
"+", "-"
]);

function isEqual(arr1, arr2) {
Copy link
Contributor

@shinew shinew Nov 1, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function should be isEqualToPrefix rather than isEqual. isEqual([1], [1, 2]) === true given this implementation -- is this expected?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh, good catch. So the reason it works is because property names are resolved RTL so foo.bar won't match foo.bar.baz due to arrays being ['bar', 'foo'] and ['baz', 'bar', 'foo']. We can maybe do an additional length check which could be a nice compromise between speed and correctness?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is specific to this feature, we should have a comment or change name to indicate that, since this is at the top level (nearly).

return arr1.every((value, index) => {
return String(value) === String(arr2[index]);
});
}

function getName(node) {
if (node.type === "ThisExpression") {
return "this";
}
if (node.type === "Super") {
return "super";
}
if (node.type === "NullLiteral") {
return "null";
}
// augment identifiers so that they don't match
// string/number literals
// but still match against each other
return node.name
? node.name + "_"
: node.value /* Literal */;
}

function getPropNames(path) {
if (!path.isMemberExpression()) {
return;
}

let obj = path.get("object");

const prop = path.get("property");
const propNames = [getName(prop.node)];

while (obj.type === "MemberExpression") {
const node = obj.get("property").node;
if (node) {
propNames.push(getName(node));
}
obj = obj.get("object");
}
propNames.push(getName(obj.node));

return propNames;
}
const OP_AND = (input) => input === "&&";
const OP_OR = (input) => input === "||";

Expand Down Expand Up @@ -194,6 +250,57 @@ module.exports = ({ types: t }) => {
}
},

AssignmentExpression(path) {

const rightExpr = path.get("right");
const leftExpr = path.get("left");

const canBeUpdateExpression = (
rightExpr.get("right").isNumericLiteral() &&
rightExpr.get("right").node.value === 1 &&
updateOperators.has(rightExpr.node.operator));

if (leftExpr.isMemberExpression()) {

const leftPropNames = getPropNames(leftExpr);
const rightPropNames = getPropNames(rightExpr.get("left"));

if (!leftPropNames ||
leftPropNames.indexOf(undefined) > -1 ||
!rightPropNames ||
rightPropNames.indexOf(undefined) > -1 ||
!operators.has(rightExpr.node.operator) ||
!isEqual(leftPropNames, rightPropNames)) {
return;
}
}
else {
if (!rightExpr.isBinaryExpression() ||
!operators.has(rightExpr.node.operator) ||
leftExpr.node.name !== rightExpr.node.left.name) {
return;
}
}

let newExpression;

// special case x=x+1 --> ++x
if (canBeUpdateExpression) {
newExpression = t.updateExpression(
rightExpr.node.operator + rightExpr.node.operator,
t.clone(leftExpr.node),
true /* prefix */);
}
else {
newExpression = t.assignmentExpression(
rightExpr.node.operator + "=",
t.clone(leftExpr.node),
t.clone(rightExpr.node.right));
}

path.replaceWith(newExpression);
},

ConditionalExpression: {
enter: [
// !foo ? 'foo' : 'bar' -> foo ? 'bar' : 'foo'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ module.exports = function({ types: t }) {
"";

pattern = pattern
.replace(/\n/g, '\\n')
.replace(/\r/g, '\\r')
.replace(/\//g, '\\/');
.replace(/\n/g, "\\n")
.replace(/\r/g, "\\r")
.replace(/\//g, "\\/");

path.replaceWith(t.regExpLiteral(pattern, flags));
}
Expand Down