From f166b7ae58476266bfc6f2759cb86a3b9c947217 Mon Sep 17 00:00:00 2001 From: overlookmotel Date: Mon, 3 May 2021 16:47:25 +0100 Subject: [PATCH] Fix plugin-transform-block-scoping const violations (#13248) * Fix plugin-transform-block-scoping const violations Fixes #13245 * Replace `a++` with `+a` where const violation * Remove assignment where const violation * Remove assignment for `&&=`, `||=`, `??=` where const violation * Shorten test --- .../src/index.js | 42 +++++++++++-- .../assignment-expression/exec.js | 62 +++++++++++++++++++ .../assignment-expression/input.js | 7 +++ .../assignment-expression/output.js | 7 +++ .../deadcode-violation/output.js | 2 +- .../destructuring-assignment/output.js | 2 +- .../fixtures/const-violations/loop/input.js | 4 ++ .../fixtures/const-violations/loop/output.js | 6 +- .../nested-update-expressions/output.js | 2 +- .../const-violations/no-assignment/exec.js | 16 +++++ .../const-violations/no-assignment/input.js | 5 +- .../const-violations/no-assignment/output.js | 5 +- .../const-violations/no-for-in/exec.js | 4 ++ .../const-violations/no-for-in/output.js | 2 +- .../update-expression-prefix/exec.js | 19 ++++++ .../update-expression-prefix/output.js | 2 +- .../update-expression/exec.js | 19 ++++++ .../update-expression/output.js | 2 +- 18 files changed, 194 insertions(+), 14 deletions(-) create mode 100644 packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/assignment-expression/exec.js create mode 100644 packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/assignment-expression/input.js create mode 100644 packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/assignment-expression/output.js diff --git a/packages/babel-plugin-transform-block-scoping/src/index.js b/packages/babel-plugin-transform-block-scoping/src/index.js index 33b5ac8fb5..ae51589520 100644 --- a/packages/babel-plugin-transform-block-scoping/src/index.js +++ b/packages/babel-plugin-transform-block-scoping/src/index.js @@ -433,17 +433,49 @@ class BlockScoping { ]); if (violation.isAssignmentExpression()) { - violation - .get("right") - .replaceWith( - t.sequenceExpression([throwNode, violation.get("right").node]), + const { operator } = violation.node; + if (operator === "=") { + violation.replaceWith( + t.sequenceExpression([violation.get("right").node, throwNode]), ); + } else if (["&&=", "||=", "??="].includes(operator)) { + violation.replaceWith( + t.logicalExpression( + operator.slice(0, -1), + violation.get("left").node, + t.sequenceExpression([violation.get("right").node, throwNode]), + ), + ); + } else { + violation.replaceWith( + t.sequenceExpression([ + t.binaryExpression( + operator.slice(0, -1), + violation.get("left").node, + violation.get("right").node, + ), + throwNode, + ]), + ); + } } else if (violation.isUpdateExpression()) { violation.replaceWith( - t.sequenceExpression([throwNode, violation.node]), + t.sequenceExpression([ + t.unaryExpression("+", violation.get("argument").node), + throwNode, + ]), ); } else if (violation.isForXStatement()) { violation.ensureBlock(); + violation + .get("left") + .replaceWith( + t.variableDeclaration("var", [ + t.variableDeclarator( + violation.scope.generateUidIdentifier(name), + ), + ]), + ); violation.node.body.body.unshift(t.expressionStatement(throwNode)); } } diff --git a/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/assignment-expression/exec.js b/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/assignment-expression/exec.js new file mode 100644 index 0000000000..bec4bfac0f --- /dev/null +++ b/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/assignment-expression/exec.js @@ -0,0 +1,62 @@ +const state1 = {}; +expect(function() { + const a = 3; + let b = 1; + state1.getA = () => a; + state1.getB = () => b; + + a += b++; +}).toThrow('"a" is read-only'); +expect(state1.getA()).toBe(3); // Assignment did not succeed +expect(state1.getB()).toBe(2); // `b++` was evaluated before error thrown + +const state2 = {}; +expect(function() { + const a = { + valueOf() { + state2.valueOfIsCalled = true; + } + }; + state2.a = a; + state2.getA = () => a; + state2.getB = () => b; + + let b = 1; + a += b++; +}).toThrow('"a" is read-only'); +expect(state2.getA()).toBe(state2.a); // Assignment did not succeed +expect(state2.getB()).toBe(2); // `b++` was evaluated before error thrown +expect(state2.valueOfIsCalled).toBe(true); // `a` was read before error thrown + +const state3 = {}; +expect(function() { + const a = 32; + let b = 1; + state3.getA = () => a; + state3.getB = () => b; + + a >>>= ++b; +}).toThrow('"a" is read-only'); +expect(state3.getA()).toBe(32); // Assignment did not succeed +expect(state3.getB()).toBe(2); // `++b` was evaluated before error thrown + +const state4 = {}; +expect(function() { + const a = 1; + let b = 1; + state4.getA = () => a; + state4.getB = () => b; + + a &&= ++b; +}).toThrow('"a" is read-only'); +expect(state4.getA()).toBe(1); // Assignment did not succeed +expect(state4.getB()).toBe(2); // `++b` was evaluated before error thrown + +{ + const a = 1; + let b = 1; + a ||= ++b; + + expect(a).toBe(1); // Assignment not made + expect(b).toBe(1); // `++b` was not evaluated +} diff --git a/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/assignment-expression/input.js b/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/assignment-expression/input.js new file mode 100644 index 0000000000..96802f76f7 --- /dev/null +++ b/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/assignment-expression/input.js @@ -0,0 +1,7 @@ +const a = 5; +let b = 0; +a += b++; +a >>>= b++; +a ||= b++; +a &&= b++; +a ??= b++; diff --git a/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/assignment-expression/output.js b/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/assignment-expression/output.js new file mode 100644 index 0000000000..9bccb23e3d --- /dev/null +++ b/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/assignment-expression/output.js @@ -0,0 +1,7 @@ +var a = 5; +var b = 0; +a + b++, babelHelpers.readOnlyError("a"); +a >>> b++, babelHelpers.readOnlyError("a"); +a || (b++, babelHelpers.readOnlyError("a")); +a && (b++, babelHelpers.readOnlyError("a")); +a ?? (b++, babelHelpers.readOnlyError("a")); diff --git a/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/deadcode-violation/output.js b/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/deadcode-violation/output.js index a9ecc00566..e1e5e16024 100644 --- a/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/deadcode-violation/output.js +++ b/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/deadcode-violation/output.js @@ -1,5 +1,5 @@ (function () { var a = "foo"; - if (false) a = (babelHelpers.readOnlyError("a"), "false"); + if (false) "false", babelHelpers.readOnlyError("a"); return a; })(); diff --git a/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/destructuring-assignment/output.js b/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/destructuring-assignment/output.js index e055105133..455758feeb 100644 --- a/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/destructuring-assignment/output.js +++ b/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/destructuring-assignment/output.js @@ -1,3 +1,3 @@ var a = 1, b = 2; -a = (babelHelpers.readOnlyError("a"), 3); +3, babelHelpers.readOnlyError("a"); diff --git a/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/loop/input.js b/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/loop/input.js index 2bef733ec1..1f1545a8bb 100644 --- a/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/loop/input.js +++ b/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/loop/input.js @@ -1,3 +1,7 @@ for (const i = 0; i < 3; i = i + 1) { console.log(i); } + +for (const j = 0; j < 3; j++) { + console.log(j); +} diff --git a/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/loop/output.js b/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/loop/output.js index f1b882b2e4..e5f5014c16 100644 --- a/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/loop/output.js +++ b/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/loop/output.js @@ -1,3 +1,7 @@ -for (var i = 0; i < 3; i = (babelHelpers.readOnlyError("i"), i + 1)) { +for (var i = 0; i < 3; i + 1, babelHelpers.readOnlyError("i")) { console.log(i); } + +for (var j = 0; j < 3; +j, babelHelpers.readOnlyError("j")) { + console.log(j); +} diff --git a/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/nested-update-expressions/output.js b/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/nested-update-expressions/output.js index 4f5d9284d7..9dfbb9b9ee 100644 --- a/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/nested-update-expressions/output.js +++ b/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/nested-update-expressions/output.js @@ -2,5 +2,5 @@ var c = 17; var a = 0; function f() { - return (babelHelpers.readOnlyError("c"), ++c) + --a; + return (+c, babelHelpers.readOnlyError("c")) + --a; } diff --git a/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/no-assignment/exec.js b/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/no-assignment/exec.js index f2094267f2..72169ff1cd 100644 --- a/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/no-assignment/exec.js +++ b/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/no-assignment/exec.js @@ -1,4 +1,20 @@ +const state1 = {}; expect(function() { const a = 3; + state1.getA = () => a; + a = 7; }).toThrow('"a" is read-only'); +expect(state1.getA()).toBe(3); // Assignment did not succeed + +const state2 = {}; +expect(function() { + const a = 3; + let b = 0; + state2.getA = () => a; + state2.getB = () => b; + + a = b++; +}).toThrow('"a" is read-only'); +expect(state2.getA()).toBe(3); // Assignment did not succeed +expect(state2.getB()).toBe(1); // `b++` was evaluated before error thrown diff --git a/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/no-assignment/input.js b/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/no-assignment/input.js index 296ce9b558..19dd65d97b 100644 --- a/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/no-assignment/input.js +++ b/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/no-assignment/input.js @@ -1,3 +1,6 @@ const MULTIPLIER = 5; - MULTIPLIER = "overwrite"; + +const a = 5; +let b = 0; +a = b++; diff --git a/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/no-assignment/output.js b/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/no-assignment/output.js index fe6f609dbd..0c6dfddc7f 100644 --- a/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/no-assignment/output.js +++ b/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/no-assignment/output.js @@ -1,2 +1,5 @@ var MULTIPLIER = 5; -MULTIPLIER = (babelHelpers.readOnlyError("MULTIPLIER"), "overwrite"); +"overwrite", babelHelpers.readOnlyError("MULTIPLIER"); +var a = 5; +var b = 0; +b++, babelHelpers.readOnlyError("a"); diff --git a/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/no-for-in/exec.js b/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/no-for-in/exec.js index 0b4cd0db9e..088178e1f7 100644 --- a/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/no-for-in/exec.js +++ b/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/no-for-in/exec.js @@ -1,5 +1,8 @@ +const state = {}; function f(arr) { const MULTIPLIER = 5; + state.getMultiplier = () => MULTIPLIER; + for (MULTIPLIER in arr); return 'survived'; @@ -8,5 +11,6 @@ function f(arr) { expect(function() { f([1,2,3]); }).toThrow('"MULTIPLIER" is read-only'); +expect(state.getMultiplier()).toBe(5); // Assignment did not succeed expect(f([])).toBe('survived'); diff --git a/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/no-for-in/output.js b/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/no-for-in/output.js index 71b72be5ca..b54d76d1f2 100644 --- a/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/no-for-in/output.js +++ b/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/no-for-in/output.js @@ -1,6 +1,6 @@ var MULTIPLIER = 5; -for (MULTIPLIER in arr) { +for (var _MULTIPLIER in arr) { babelHelpers.readOnlyError("MULTIPLIER"); ; } diff --git a/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/update-expression-prefix/exec.js b/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/update-expression-prefix/exec.js index 9b0f1ca2c2..6588bdea28 100644 --- a/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/update-expression-prefix/exec.js +++ b/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/update-expression-prefix/exec.js @@ -1,4 +1,23 @@ +const state1 = {}; expect(function() { const a = "str"; + state1.getA = () => a; + --a; }).toThrow('"a" is read-only'); +expect(state1.getA()).toBe("str"); // Assignment did not succeed + +const state2 = {}; +expect(function() { + const b = { + valueOf() { + state2.valueOfIsCalled = true; + } + }; + state2.b = b; + state2.getB = () => b; + + --b; +}).toThrow('"b" is read-only'); +expect(state2.getB()).toBe(state2.b); // Assignment did not succeed +expect(state2.valueOfIsCalled).toBe(true); // `bar` was read before error thrown diff --git a/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/update-expression-prefix/output.js b/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/update-expression-prefix/output.js index b9a3ee122b..fe84098443 100644 --- a/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/update-expression-prefix/output.js +++ b/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/update-expression-prefix/output.js @@ -1,2 +1,2 @@ var a = "str"; -babelHelpers.readOnlyError("a"), --a; ++a, babelHelpers.readOnlyError("a"); diff --git a/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/update-expression/exec.js b/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/update-expression/exec.js index 3de129614e..5734e67434 100644 --- a/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/update-expression/exec.js +++ b/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/update-expression/exec.js @@ -1,4 +1,23 @@ +const state1 = {}; expect(function() { const foo = 1; + state1.getFoo = () => foo; + foo++; }).toThrow('"foo" is read-only'); +expect(state1.getFoo()).toBe(1); // Assignment did not succeed + +const state2 = {}; +expect(function() { + const bar = { + valueOf() { + state2.valueOfIsCalled = true; + } + }; + state2.bar = bar; + state2.getBar = () => bar; + + bar++; +}).toThrow('"bar" is read-only'); +expect(state2.getBar()).toBe(state2.bar); // Assignment did not succeed +expect(state2.valueOfIsCalled).toBe(true); // `bar` was read before error thrown diff --git a/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/update-expression/output.js b/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/update-expression/output.js index 355150f7ec..4f0f5202a7 100644 --- a/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/update-expression/output.js +++ b/packages/babel-plugin-transform-block-scoping/test/fixtures/const-violations/update-expression/output.js @@ -1,2 +1,2 @@ var foo = 1; -babelHelpers.readOnlyError("foo"), foo++; ++foo, babelHelpers.readOnlyError("foo");