From 34d73ebef09e5b806e50a30ef7d268e5d663dada Mon Sep 17 00:00:00 2001 From: Justin Ridgewell Date: Sun, 22 Apr 2018 13:50:11 -0400 Subject: [PATCH] Correct update expression Number coercion (#7766) * Correct update expression Number coercion You have to `ToNumber` whatever the `UpdateExpression` argument is. * Fix systemjs update expression --- .../babel-helper-simple-access/src/index.js | 34 +++++++++++++++---- .../fixtures/regression/4462-T7565/output.js | 4 +-- .../negative-suffix/output.js | 2 +- .../positive-suffix/output.js | 2 +- .../src/index.js | 22 ++++-------- .../systemjs/hoisting-bindings/output.mjs | 2 +- .../test/fixtures/systemjs/remap/output.mjs | 2 +- 7 files changed, 40 insertions(+), 28 deletions(-) diff --git a/packages/babel-helper-simple-access/src/index.js b/packages/babel-helper-simple-access/src/index.js index b26404223c..9f02df0ec5 100644 --- a/packages/babel-helper-simple-access/src/index.js +++ b/packages/babel-helper-simple-access/src/index.js @@ -25,27 +25,49 @@ const simpleAssignmentVisitor = { } if ( - path.node.prefix || - (path.parentPath.isExpressionStatement() && !path.isCompletionRecord()) + path.parentPath.isExpressionStatement() && + !path.isCompletionRecord() ) { // ++i => (i += 1); const operator = path.node.operator == "++" ? "+=" : "-="; path.replaceWith( t.assignmentExpression(operator, arg.node, t.numericLiteral(1)), ); + } else if (path.node.prefix) { + // ++i => (i = (+i) + 1); + path.replaceWith( + t.assignmentExpression( + "=", + t.identifier(localName), + t.binaryExpression( + path.node.operator[0], + t.unaryExpression("+", arg.node), + t.numericLiteral(1), + ), + ), + ); } else { - const varName = path.scope.generateDeclaredUidIdentifier("old").name; + const old = path.scope.generateUidIdentifierBasedOnNode( + arg.node, + "old", + ); + const varName = old.name; + path.scope.push({ id: old }); const binary = t.binaryExpression( - path.node.operator.slice(0, 1), + path.node.operator[0], t.identifier(varName), t.numericLiteral(1), ); - // i++ => (_tmp = i, i = _tmp + 1, _tmp) + // i++ => (_old = (+i), i = _old + 1, _old) path.replaceWith( t.sequenceExpression([ - t.assignmentExpression("=", t.identifier(varName), arg.node), + t.assignmentExpression( + "=", + t.identifier(varName), + t.unaryExpression("+", arg.node), + ), t.assignmentExpression("=", t.cloneNode(arg.node), binary), t.identifier(varName), ]), diff --git a/packages/babel-plugin-transform-modules-commonjs/test/fixtures/regression/4462-T7565/output.js b/packages/babel-plugin-transform-modules-commonjs/test/fixtures/regression/4462-T7565/output.js index d9c0a4a724..ac7264e25d 100644 --- a/packages/babel-plugin-transform-modules-commonjs/test/fixtures/regression/4462-T7565/output.js +++ b/packages/babel-plugin-transform-modules-commonjs/test/fixtures/regression/4462-T7565/output.js @@ -5,9 +5,9 @@ Object.defineProperty(exports, "__esModule", { }); exports.zz = exports.yy = void 0; -var _old; +var _yy; var yy = 0; exports.yy = yy; -var zz = (_old = yy, exports.yy = yy = _old + 1, _old); +var zz = (_yy = +yy, exports.yy = yy = _yy + 1, _yy); exports.zz = zz; diff --git a/packages/babel-plugin-transform-modules-commonjs/test/fixtures/update-expression/negative-suffix/output.js b/packages/babel-plugin-transform-modules-commonjs/test/fixtures/update-expression/negative-suffix/output.js index c881117d50..e6ac924358 100644 --- a/packages/babel-plugin-transform-modules-commonjs/test/fixtures/update-expression/negative-suffix/output.js +++ b/packages/babel-plugin-transform-modules-commonjs/test/fixtures/update-expression/negative-suffix/output.js @@ -9,7 +9,7 @@ let diffLevel = 0; exports.diffLevel = diffLevel; function diff() { - if (!(exports.diffLevel = diffLevel = diffLevel - 1)) { + if (!(exports.diffLevel = diffLevel = +diffLevel - 1)) { console.log("hey"); } } diff --git a/packages/babel-plugin-transform-modules-commonjs/test/fixtures/update-expression/positive-suffix/output.js b/packages/babel-plugin-transform-modules-commonjs/test/fixtures/update-expression/positive-suffix/output.js index 6fd88b919d..b78cd08a8e 100644 --- a/packages/babel-plugin-transform-modules-commonjs/test/fixtures/update-expression/positive-suffix/output.js +++ b/packages/babel-plugin-transform-modules-commonjs/test/fixtures/update-expression/positive-suffix/output.js @@ -9,7 +9,7 @@ let diffLevel = 0; exports.diffLevel = diffLevel; function diff() { - if (!(exports.diffLevel = diffLevel = diffLevel + 1)) { + if (!(exports.diffLevel = diffLevel = +diffLevel + 1)) { console.log("hey"); } } diff --git a/packages/babel-plugin-transform-modules-systemjs/src/index.js b/packages/babel-plugin-transform-modules-systemjs/src/index.js index a323250359..3111665fa9 100644 --- a/packages/babel-plugin-transform-modules-systemjs/src/index.js +++ b/packages/babel-plugin-transform-modules-systemjs/src/index.js @@ -50,23 +50,13 @@ export default declare((api, options) => { // if it is a non-prefix update expression (x++ etc) // then we must replace with the expression (_export('x', x + 1), x++) // in order to ensure the same update expression value - let isPostUpdateExpression = path.isUpdateExpression() && !node.prefix; + const isPostUpdateExpression = path.isUpdateExpression({ prefix: false }); if (isPostUpdateExpression) { - if (node.operator === "++") { - node = t.binaryExpression( - "+", - t.cloneNode(node.argument), - t.numericLiteral(1), - ); - } else if (node.operator === "--") { - node = t.binaryExpression( - "-", - t.cloneNode(node.argument), - t.numericLiteral(1), - ); - } else { - isPostUpdateExpression = false; - } + node = t.binaryExpression( + node.operator[0], + t.unaryExpression("+", t.cloneNode(node.argument)), + t.numericLiteral(1), + ); } for (const exportedName of exportedNames) { diff --git a/packages/babel-plugin-transform-modules-systemjs/test/fixtures/systemjs/hoisting-bindings/output.mjs b/packages/babel-plugin-transform-modules-systemjs/test/fixtures/systemjs/hoisting-bindings/output.mjs index 2da64b6f6c..95452de79c 100644 --- a/packages/babel-plugin-transform-modules-systemjs/test/fixtures/systemjs/hoisting-bindings/output.mjs +++ b/packages/babel-plugin-transform-modules-systemjs/test/fixtures/systemjs/hoisting-bindings/output.mjs @@ -5,7 +5,7 @@ System.register([], function (_export, _context) { function a() { alert("a"); - _export("c", c + 1), c++; + _export("c", +c + 1), c++; } _export("a", a); diff --git a/packages/babel-plugin-transform-modules-systemjs/test/fixtures/systemjs/remap/output.mjs b/packages/babel-plugin-transform-modules-systemjs/test/fixtures/systemjs/remap/output.mjs index 0b80944a95..3e31c01465 100644 --- a/packages/babel-plugin-transform-modules-systemjs/test/fixtures/systemjs/remap/output.mjs +++ b/packages/babel-plugin-transform-modules-systemjs/test/fixtures/systemjs/remap/output.mjs @@ -11,7 +11,7 @@ System.register([], function (_export, _context) { _export("test", test = 5); - _export("test", test + 1), test++; + _export("test", +test + 1), test++; (function () { var test = 2;