Merge pull request #6309 from loganfsmyth/assert-module-exports-usage

Support opt-in restrictions on 'module' and 'exports' usage alongside ES6 import/export.

packages/babel-helper-module-transforms/package.json

@@ -9,6 +9,7 @@
   "main": "lib/index.js",
   "dependencies": {
     "babel-helper-module-imports": "7.0.0-beta.2",
+    "babel-helper-simple-access": "7.0.0-beta.2",
     "babel-template": "7.0.0-beta.2",
     "babel-types": "7.0.0-beta.2",
     "lodash": "^4.2.0"

packages/babel-helper-module-transforms/src/rewrite-live-references.js

@@ -1,5 +1,7 @@
+import assert from "assert";
 import * as t from "babel-types";
 import template from "babel-template";
+import simplifyAccess from "babel-helper-simple-access";
 
 import type { ModuleMetadata } from "./";
 
@@ -44,6 +46,12 @@ export default function rewriteLiveReferences(
     exported, // local name => exported name list
   });
 
+  simplifyAccess(
+    programPath,
+    // NOTE(logan): The 'Array.from' calls are to make this code with in loose mode.
+    new Set([...Array.from(imported.keys()), ...Array.from(exported.keys())]),
+  );
+
   // Rewrite reads/writes from imports and exports to have the correct behavior.
   programPath.traverse(rewriteReferencesVisitor, {
     seen: new WeakSet(),
@@ -188,57 +196,6 @@ const rewriteReferencesVisitor = {
     }
   },
 
-  UpdateExpression: {
-    exit(path) {
-      const { scope, imported, exported } = this;
-
-      const arg = path.get("argument");
-      if (!arg.isIdentifier()) return;
-      const localName = arg.node.name;
-
-      if (!imported.has(localName) && !exported.has(localName)) {
-        return;
-      }
-
-      // redeclared in this scope
-      if (scope.getBinding(localName) !== path.scope.getBinding(localName)) {
-        return;
-      }
-
-      const exportedNames = exported.get(localName) || [];
-
-      if (exportedNames.length > 0 || imported.has(localName)) {
-        if (
-          path.node.prefix ||
-          (path.parentPath.isExpressionStatement() &&
-            !path.isCompletionRecord())
-        ) {
-          // ++i => (i += 1);
-          path.replaceWith(
-            t.assignmentExpression("+=", arg.node, t.numericLiteral(1)),
-          );
-        } else {
-          const varName = path.scope.generateDeclaredUidIdentifier("old");
-
-          const assignment = t.binaryExpression(
-            path.node.operator.slice(0, 1),
-            varName,
-            t.numericLiteral(1),
-          );
-
-          // i++ => (_tmp = i, i = _tmp + 1, _tmp)
-          path.replaceWith(
-            t.sequenceExpression([
-              t.assignmentExpression("=", varName, arg.node),
-              t.assignmentExpression("=", arg.node, assignment),
-              varName,
-            ]),
-          );
-        }
-      }
-    },
-  },
-
   AssignmentExpression: {
     exit(path) {
       const {
@@ -267,23 +224,14 @@ const rewriteReferencesVisitor = {
         const exportedNames = exported.get(localName) || [];
         const importData = imported.get(localName);
         if (exportedNames.length > 0 || importData) {
+          assert(path.node.operator === "=", "Path was not simplified");
+
           const assignment = path.node;
 
           if (importData) {
             assignment.left =
               buildImportReference(importData) || assignment.left;
 
-            if (path.node.operator !== "=") {
-              const op = path.node.operator.slice(0, -1);
-              path.node.operator = "=";
-
-              assignment.right = t.binaryExpression(
-                op,
-                assignment.left,
-                assignment.right,
-              );
-            }
-
             assignment.right = t.sequenceExpression([
               assignment.right,
               buildImportThrow(localName),

packages/babel-helper-simple-access/.npmignore

@@ -0,0 +1,3 @@
+src
+test
+*.log

packages/babel-helper-simple-access/README.md

@@ -0,0 +1,26 @@
+# babel-helper-simple-assignment
+
+There are many cases where it is hard to perform transformations because a
+piece of code is using complex structures. Say you want to rewrite all accesses
+to a given variable, and there are cases like
+
+```
+i += 1
+--i;
+```
+
+It is difficult to work with.
+
+This helper can handle converting these to simple access patterns of standard
+assignment. This plugin does _not_ handle
+
+```
+{ a } = foo;
+```
+
+so assignment to patterns still needs to be handled when you are processing
+updates to values.
+
+## Usage
+
+TODO

packages/babel-helper-simple-access/package.json

@@ -0,0 +1,15 @@
+{
+  "name": "babel-helper-simple-access",
+  "version": "7.0.0-beta.2",
+  "description": "Babel helper for ensuring that access to a given value is performed through simple accesses",
+  "author": "Logan Smyth <loganfsmyth@gmail.com>",
+  "homepage": "https://babeljs.io/",
+  "license": "MIT",
+  "repository": "https://github.com/babel/babel/tree/master/packages/babel-helper-simple-access",
+  "main": "lib/index.js",
+  "dependencies": {
+    "babel-template": "7.0.0-beta.2",
+    "babel-types": "7.0.0-beta.2",
+    "lodash": "^4.2.0"
+  }
+}

packages/babel-helper-simple-access/src/index.js

@@ -0,0 +1,87 @@
+import * as t from "babel-types";
+
+export default function simplifyAccess(path: NodePath, bindingNames) {
+  path.traverse(simpleAssignmentVisitor, {
+    scope: path.scope,
+    bindingNames,
+    seen: new WeakSet(),
+  });
+}
+
+const simpleAssignmentVisitor = {
+  UpdateExpression: {
+    exit(path) {
+      const { scope, bindingNames } = this;
+
+      const arg = path.get("argument");
+      if (!arg.isIdentifier()) return;
+      const localName = arg.node.name;
+
+      if (!bindingNames.has(localName)) return;
+
+      // redeclared in this scope
+      if (scope.getBinding(localName) !== path.scope.getBinding(localName)) {
+        return;
+      }
+
+      if (
+        path.node.prefix ||
+        (path.parentPath.isExpressionStatement() && !path.isCompletionRecord())
+      ) {
+        // ++i => (i += 1);
+        path.replaceWith(
+          t.assignmentExpression("+=", arg.node, t.numericLiteral(1)),
+        );
+      } else {
+        const varName = path.scope.generateDeclaredUidIdentifier("old");
+
+        const assignment = t.binaryExpression(
+          path.node.operator.slice(0, 1),
+          varName,
+          t.numericLiteral(1),
+        );
+
+        // i++ => (_tmp = i, i = _tmp + 1, _tmp)
+        path.replaceWith(
+          t.sequenceExpression([
+            t.assignmentExpression("=", varName, arg.node),
+            t.assignmentExpression("=", arg.node, assignment),
+            varName,
+          ]),
+        );
+      }
+    },
+  },
+
+  AssignmentExpression: {
+    exit(path) {
+      const { scope, seen, bindingNames } = this;
+
+      if (path.node.operator === "=") return;
+
+      if (seen.has(path.node)) return;
+      seen.add(path.node);
+
+      const left = path.get("left");
+      if (!left.isIdentifier()) return;
+
+      // Simple update-assign foo += 1;
+      // =>   exports.foo =  (foo += 1);
+      const localName = left.node.name;
+
+      if (!bindingNames.has(localName)) return;
+
+      // redeclared in this scope
+      if (scope.getBinding(localName) !== path.scope.getBinding(localName)) {
+        return;
+      }
+
+      path.node.right = t.binaryExpression(
+        path.node.operator.slice(0, -1),
+        path.node.left,
+        path.node.right,
+      );
+      path.node.operator = "=";
+    },
+  },
+};

packages/babel-plugin-transform-es2015-modules-amd/test/fixtures/amd/remap/expected.js

@@ -8,7 +8,7 @@ define(["exports"], function (_exports) {
   var test = 2;
   _exports.test = test;
   _exports.test = test = 5;
-  _exports.test = test += 1;
+  _exports.test = test = test + 1;
 
   (function () {
     var test = 2;

packages/babel-plugin-transform-es2015-modules-commonjs/package.json

@@ -6,6 +6,7 @@
   "license": "MIT",
   "main": "lib/index.js",
   "dependencies": {
+    "babel-helper-simple-access": "7.0.0-beta.2",
     "babel-helper-module-transforms": "7.0.0-beta.2",
     "babel-types": "7.0.0-beta.2"
   },

packages/babel-plugin-transform-es2015-modules-commonjs/src/index.js

@@ -6,8 +6,82 @@ import {
   ensureStatementsHoisted,
   wrapInterop,
 } from "babel-helper-module-transforms";
+import simplifyAccess from "babel-helper-simple-access";
+
+export default function({ types: t, template }) {
+  const moduleAssertion = template(`
+    (function(){
+      throw new Error("The CommonJS 'module' variable is not available in ES6 modules.");
+    })();
+  `);
+  const exportsAssertion = template(`
+    (function(){
+      throw new Error("The CommonJS 'exports' variable is not available in ES6 modules.");
+    })();
+  `);
+  const getAssertion = localName =>
+    (localName === "module" ? moduleAssertion() : exportsAssertion())
+      .expression;
+
+  const moduleExportsVisitor = {
+    ReferencedIdentifier(path) {
+      const localName = path.node.name;
+      if (localName !== "module" && localName !== "exports") return;
+
+      const localBinding = path.scope.getBinding(localName);
+      const rootBinding = this.scope.getBinding(localName);
+
+      if (
+        // redeclared in this scope
+        rootBinding !== localBinding ||
+        (path.parentPath.isObjectProperty({ value: path.node }) &&
+          path.parentPath.parentPath.isObjectPattern()) ||
+        path.parentPath.isAssignmentExpression({ left: path.node }) ||
+        path.isAssignmentExpression({ left: path.node })
+      ) {
+        return;
+      }
+
+      path.replaceWith(getAssertion(localName));
+    },
+
+    AssignmentExpression(path) {
+      const left = path.get("left");
+      if (left.isIdentifier()) {
+        const localName = path.node.name;
+        if (localName !== "module" && localName !== "exports") return;
+
+        const localBinding = path.scope.getBinding(localName);
+        const rootBinding = this.scope.getBinding(localName);
+
+        // redeclared in this scope
+        if (rootBinding !== localBinding) return;
+
+        const right = path.get("right");
+        right.replaceWith(
+          t.sequenceExpression([right.node, getAssertion(localName)]),
+        );
+      } else if (left.isPattern()) {
+        const ids = left.getOuterBindingIdentifiers();
+        const localName = Object.keys(ids).filter(localName => {
+          if (localName !== "module" && localName !== "exports") return false;
+
+          return (
+            this.scope.getBinding(localName) ===
+            path.scope.getBinding(localName)
+          );
+        })[0];
+
+        if (localName) {
+          const right = path.get("right");
+          right.replaceWith(
+            t.sequenceExpression([right.node, getAssertion(localName)]),
+          );
+        }
+      }
+    },
+  };
 
-export default function({ types: t }) {
   return {
     visitor: {
       Program: {
@@ -22,6 +96,9 @@ export default function({ types: t }) {
             strict,
             strictMode,
             noInterop,
+
+            // Defaulting to 'true' for now. May change before 7.x major.
+            allowCommonJSExports = true,
           } = state.opts;
 
           // Rename the bindings auto-injected into the scope so there is no
@@ -32,6 +109,16 @@ export default function({ types: t }) {
           path.scope.rename("__filename");
           path.scope.rename("__dirname");
 
+          // Rewrite references to 'module' and 'exports' to throw exceptions.
+          // These objects are specific to CommonJS and are not available in
+          // real ES6 implementations.
+          if (!allowCommonJSExports) {
+            simplifyAccess(path, new Set(["module", "exports"]));
+            path.traverse(moduleExportsVisitor, {
+              scope: path.scope,
+            });
+          }
+
           let moduleName = this.getModuleName();
           if (moduleName) moduleName = t.stringLiteral(moduleName);
 

packages/babel-plugin-transform-es2015-modules-commonjs/test/fixtures/interop/remap/expected.js

@@ -7,7 +7,7 @@ exports.f = exports.e = exports.c = exports.a = exports.test = void 0;
 var test = 2;
 exports.test = test;
 exports.test = test = 5;
-exports.test = test += 1;
+exports.test = test = test + 1;
 
 (function () {
   var test = 2;

packages/babel-plugin-transform-es2015-modules-commonjs/test/fixtures/misc/local-exports-decl/actual.js

@@ -0,0 +1,23 @@
+import "foo";
+
+var exports = "local exports";
+var module = "local module";
+
+console.log(exports);
+console.log(exports.prop);
+exports++;
+exports += 4;
+({ exports } = {});
+[ exports ] = [];
+exports = {};
+exports.prop = "";
+
+
+console.log(module);
+console.log(module.exports);
+module++;
+module += 4;
+({ module } = {});
+[ module ] = [];
+module = {};
+module.prop = "";

packages/babel-plugin-transform-es2015-modules-commonjs/test/fixtures/misc/local-exports-decl/expected.js

@@ -0,0 +1,26 @@
+"use strict";
+
+require("foo");
+
+var _exports = "local exports";
+var _module = "local module";
+console.log(_exports);
+console.log(_exports.prop);
+_exports++;
+_exports += 4;
+({
+  exports: _exports
+} = {});
+[_exports] = [];
+_exports = {};
+_exports.prop = "";
+console.log(_module);
+console.log(_module.exports);
+_module++;
+_module += 4;
+({
+  module: _module
+} = {});
+[_module] = [];
+_module = {};
+_module.prop = "";

packages/babel-plugin-transform-es2015-modules-commonjs/test/fixtures/misc/local-exports-decl/options.json

@@ -0,0 +1,3 @@
+{
+  "plugins": ["transform-es2015-modules-commonjs"]
+}

packages/babel-plugin-transform-es2015-modules-commonjs/test/fixtures/misc/module-exports/actual.js

@@ -0,0 +1,22 @@
+import "foo";
+
+console.log(exports);
+console.log(exports.prop);
+exports++;
+exports += 4;
+({ exports } = {});
+[ exports ] = [];
+exports = {};
+exports.prop = "";
+
+
+console.log(module);
+console.log(module.exports);
+module++;
+module += 4;
+({ module } = {});
+[ module ] = [];
+module = {};
+module.prop = "";
+
+

packages/babel-plugin-transform-es2015-modules-commonjs/test/fixtures/misc/module-exports/expected.js

@@ -0,0 +1,58 @@
+"use strict";
+
+require("foo");
+
+console.log(function () {
+  throw new Error("The CommonJS 'exports' variable is not available in ES6 modules.");
+}());
+console.log(function () {
+  throw new Error("The CommonJS 'exports' variable is not available in ES6 modules.");
+}().prop);
+
+exports = function () {
+  throw new Error("The CommonJS 'exports' variable is not available in ES6 modules.");
+}() + 1;
+
+exports = function () {
+  throw new Error("The CommonJS 'exports' variable is not available in ES6 modules.");
+}() + 4;
+
+({
+  exports
+} = ({}, function () {
+  throw new Error("The CommonJS 'exports' variable is not available in ES6 modules.");
+}()));
+[exports] = ([], function () {
+  throw new Error("The CommonJS 'exports' variable is not available in ES6 modules.");
+}());
+exports = {};
+(function () {
+  throw new Error("The CommonJS 'exports' variable is not available in ES6 modules.");
+})().prop = "";
+console.log(function () {
+  throw new Error("The CommonJS 'module' variable is not available in ES6 modules.");
+}());
+console.log(function () {
+  throw new Error("The CommonJS 'module' variable is not available in ES6 modules.");
+}().exports);
+
+module = function () {
+  throw new Error("The CommonJS 'module' variable is not available in ES6 modules.");
+}() + 1;
+
+module = function () {
+  throw new Error("The CommonJS 'module' variable is not available in ES6 modules.");
+}() + 4;
+
+({
+  module
+} = ({}, function () {
+  throw new Error("The CommonJS 'module' variable is not available in ES6 modules.");
+}()));
+[module] = ([], function () {
+  throw new Error("The CommonJS 'module' variable is not available in ES6 modules.");
+}());
+module = {};
+(function () {
+  throw new Error("The CommonJS 'module' variable is not available in ES6 modules.");
+})().prop = "";

packages/babel-plugin-transform-es2015-modules-commonjs/test/fixtures/misc/module-exports/options.json

@@ -0,0 +1,8 @@
+{
+  "plugins": [
+    [
+      "transform-es2015-modules-commonjs",
+      { "allowCommonJSExports": false }
+    ]
+  ]
+}

packages/babel-plugin-transform-es2015-modules-umd/test/fixtures/umd/remap/expected.js

@@ -20,7 +20,7 @@
   var test = 2;
   _exports.test = test;
   _exports.test = test = 5;
-  _exports.test = test += 1;
+  _exports.test = test = test + 1;
 
   (function () {
     var test = 2;