Spec compliancy of check-es2015-constants plugin (#5930)

* Figuring out where to place throw statement

* Restored path argument destructuring

* New approach using comma expressions for assignments

* Moved throwNode into body of forXstatements

* Refactored with helper function and ensureBlock, additional tests for update exprs

* Added exec tests for all violations

* Hoisted helper function for comma expression outside of visitor

packages/babel-plugin-check-es2015-constants/src/index.js

@@ -1,4 +1,18 @@
-export default function({ messages }) {
+export default function({ messages, types: t }) {
+  /**
+   * Helper function to run a statement before an expression by replacing it with a comma expression
+   * and wrapping the statement in an IIFE as the first operand.
+   */
+  function statementBeforeExpression(statement, expression) {
+    return t.sequenceExpression([
+      t.callExpression(
+        t.functionExpression(null, [], t.blockStatement([statement])),
+        [],
+      ),
+      expression,
+    ]);
+  }
+
   return {
     visitor: {
       Scope({ scope }) {
@@ -7,7 +21,29 @@ export default function({ messages }) {
           if (binding.kind !== "const" && binding.kind !== "module") continue;
 
           for (const violation of (binding.constantViolations: Array)) {
-            throw violation.buildCodeFrameError(messages.get("readOnly", name));
+            const throwNode = t.throwStatement(
+              t.newExpression(t.identifier("Error"), [
+                t.stringLiteral(messages.get("readOnly", name)),
+              ]),
+            );
+
+            if (violation.isAssignmentExpression()) {
+              violation
+                .get("right")
+                .replaceWith(
+                  statementBeforeExpression(
+                    throwNode,
+                    violation.get("right").node,
+                  ),
+                );
+            } else if (violation.parentPath.isUpdateExpression()) {
+              violation.parentPath.replaceWith(
+                statementBeforeExpression(throwNode, violation.parent),
+              );
+            } else if (violation.parentPath.isForXStatement()) {
+              violation.parentPath.ensureBlock();
+              violation.parentPath.node.body.body.unshift(throwNode);
+            }
           }
         }
       },

packages/babel-plugin-check-es2015-constants/test/fixtures/general/deadcode-violation/actual.js

@@ -0,0 +1,7 @@
+(function(){
+  const a = "foo";
+
+  if (false) a = "false";
+
+  return a;
+})();

packages/babel-plugin-check-es2015-constants/test/fixtures/general/deadcode-violation/exec.js

@@ -0,0 +1,9 @@
+function f() {
+  const a = "foo";
+
+  if (false) a = "false";
+
+  return a;
+}
+
+assert.equal(f(), "foo", 'Const violation in not taken branch should be ignored.')

packages/babel-plugin-check-es2015-constants/test/fixtures/general/deadcode-violation/expected.js

@@ -0,0 +1,7 @@
+(function () {
+  var a = "foo";
+  if (false) a = (function () {
+    throw new Error("\"a\" is read-only");
+  }(), "false");
+  return a;
+})();

packages/babel-plugin-check-es2015-constants/test/fixtures/general/destructuring-assignment/exec.js

@@ -0,0 +1,14 @@
+assert.throws(function() {
+  const [a, b] = [1, 2];
+  a = 3;
+}, '"a" is read-only')
+
+assert.throws(function() {
+  const a = 1;
+  [a] = [2];
+}, '"a" is read-only');
+
+assert.throws(function() {
+  const b = 1;
+  ({b} = {b: 2});
+}, '"b" is read-only');

packages/babel-plugin-check-es2015-constants/test/fixtures/general/destructuring-assignment/expected.js

@@ -0,0 +1,5 @@
+var a = 1,
+    b = 2;
+a = (function () {
+  throw new Error("\"a\" is read-only");
+}(), 3);

packages/babel-plugin-check-es2015-constants/test/fixtures/general/destructuring-assignment/options.json

@@ -1,3 +0,0 @@
-{
-  "throws": "\"a\" is read-only"
-}

packages/babel-plugin-check-es2015-constants/test/fixtures/general/loop/exec.js

@@ -0,0 +1,5 @@
+assert.throws(function() {
+  for (const i = 0; i < 3; i = i + 1) {
+    // whatever
+  }
+}, '"i" is read-only');

packages/babel-plugin-check-es2015-constants/test/fixtures/general/loop/expected.js

@@ -0,0 +1,5 @@
+for (var i = 0; i < 3; i = (function () {
+  throw new Error("\"i\" is read-only");
+}(), i + 1)) {
+  console.log(i);
+}

packages/babel-plugin-check-es2015-constants/test/fixtures/general/loop/options.json

@@ -1,3 +0,0 @@
-{
-  "throws": "\"i\" is read-only"
-}

packages/babel-plugin-check-es2015-constants/test/fixtures/general/nested-update-expressions/actual.js

@@ -0,0 +1,6 @@
+const c = 17;
+let a = 0;
+
+function f() {
+  return ++c+--a;
+}

packages/babel-plugin-check-es2015-constants/test/fixtures/general/nested-update-expressions/exec.js

@@ -0,0 +1,11 @@
+assert.throws(function() {
+  const c = 17;
+  let a = 0;
+
+  function f() {
+    return ++c+--a;
+  }
+
+  f();
+
+}, '"c" is read-only');

packages/babel-plugin-check-es2015-constants/test/fixtures/general/nested-update-expressions/expected.js

@@ -0,0 +1,8 @@
+var c = 17;
+var a = 0;
+
+function f() {
+  return (function () {
+    throw new Error("\"c\" is read-only");
+  }(), ++c) + --a;
+}

packages/babel-plugin-check-es2015-constants/test/fixtures/general/no-assignment/exec.js

@@ -0,0 +1,4 @@
+assert.throws(function() {
+  const a = 3;
+  a = 7;
+}, '"a" is read-only');

packages/babel-plugin-check-es2015-constants/test/fixtures/general/no-assignment/expected.js

@@ -0,0 +1,4 @@
+var MULTIPLIER = 5;
+MULTIPLIER = (function () {
+  throw new Error("\"MULTIPLIER\" is read-only");
+}(), "overwrite");

packages/babel-plugin-check-es2015-constants/test/fixtures/general/no-assignment/options.json

@@ -1,3 +0,0 @@
-{
-  "throws": "\"MULTIPLIER\" is read-only"
-}

packages/babel-plugin-check-es2015-constants/test/fixtures/general/no-for-in/exec.js

@@ -0,0 +1,12 @@
+function f(arr) {
+  const MULTIPLIER = 5;
+  for (MULTIPLIER in arr);
+
+  return 'survived';
+}
+
+assert.throws(function() {
+  f([1,2,3]);
+}, '"MULTIPLIER" is read-only');
+
+assert.equal(f([]), 'survived', 'For-in over empty array should not throw.');

packages/babel-plugin-check-es2015-constants/test/fixtures/general/no-for-in/expected.js

@@ -0,0 +1,5 @@
+var MULTIPLIER = 5;
+
+for (MULTIPLIER in arr) {
+  throw new Error("\"MULTIPLIER\" is read-only");
+}

packages/babel-plugin-check-es2015-constants/test/fixtures/general/no-for-in/options.json

@@ -1,3 +0,0 @@
-{
-  "throws": "\"MULTIPLIER\" is read-only"
-}

packages/babel-plugin-check-es2015-constants/test/fixtures/general/update-expression-prefix/actual.js

@@ -0,0 +1,2 @@
+const a = "str";
+--a;

packages/babel-plugin-check-es2015-constants/test/fixtures/general/update-expression-prefix/exec.js

@@ -0,0 +1,4 @@
+assert.throws(function() {
+  const a = "str";
+  --a;
+}, '"a" is read-only');

packages/babel-plugin-check-es2015-constants/test/fixtures/general/update-expression-prefix/expected.js

@@ -0,0 +1,4 @@
+var a = "str";
+(function () {
+  throw new Error("\"a\" is read-only");
+})(), --a;

packages/babel-plugin-check-es2015-constants/test/fixtures/general/update-expression/exec.js

@@ -0,0 +1,4 @@
+assert.throws(function() {
+  const foo = 1;
+  foo++;
+}, '"foo" is read-only');

packages/babel-plugin-check-es2015-constants/test/fixtures/general/update-expression/expected.js

@@ -1,4 +1,4 @@
 var foo = 1;
 (function () {
-  throw new TypeError("\"foo\" is read-only");
-})();
+  throw new Error("\"foo\" is read-only");
+})(), foo++;

packages/babel-plugin-check-es2015-constants/test/fixtures/general/update-expression/options.json

@@ -1,3 +0,0 @@
-{
-  "throws": "\"foo\" is read-only"
-}