Remove IIFE wrappers from CommonJS modules that have UMD patterns

Closes #2146

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=140774013

src/com/google/javascript/jscomp/FunctionToBlockMutator.java

@@ -63,16 +63,62 @@ class FunctionToBlockMutator {
    */
   Node mutate(String fnName, Node fnNode, Node callNode,
       String resultName, boolean needsDefaultResult, boolean isCallInLoop) {
+    return mutateInternal(
+        fnName, fnNode, callNode, resultName, needsDefaultResult, isCallInLoop,
+        /* renameLocals */ true);
+  }
+
+  /**
+   * Used when an IIFE wrapper is being removed
+   *
+   * @param fnName The name to use when preparing human readable names.
+   * @param fnNode The function to prepare.
+   * @param callNode The call node that will be replaced.
+   * @return A clone of the function body mutated to be suitable for injection as a statement into a
+   *     script root
+   */
+  Node unwrapIifeInModule(String fnName, Node fnNode, Node callNode) {
+    return mutateInternal(fnName, fnNode, callNode,
+        /* resultName */ null,
+        /* needsDefaultResult */ false,
+        /* isCallInLoop */ false,
+        /* renameLocals */ false);
+  }
+
+  /**
+   * @param fnName The name to use when preparing human readable names.
+   * @param fnNode The function to prepare.
+   * @param callNode The call node that will be replaced.
+   * @param resultName Function results should be assigned to this name.
+   * @param needsDefaultResult Whether the result value must be set.
+   * @param isCallInLoop Whether the function body must be prepared to be injected into the body of
+   *     a loop.
+   * @param renameLocals If the inlining is part of module rewriting and doesn't require making
+   *     local names unique
+   * @return A clone of the function body mutated to be suitable for injection as a statement into
+   *     another code block.
+   */
+  private Node mutateInternal(
+      String fnName,
+      Node fnNode,
+      Node callNode,
+      String resultName,
+      boolean needsDefaultResult,
+      boolean isCallInLoop,
+      boolean renameLocals) {
     Node newFnNode = fnNode.cloneTree();
-    // Now that parameter names have been replaced, make sure all the local
-    // names are unique, to allow functions to be inlined multiple times
-    // without causing conflicts.
-    makeLocalNamesUnique(newFnNode, isCallInLoop);
-
-    // Function declarations must be rewritten as function expressions as
-    // they will be within a block and normalization prevents function
-    // declarations within block as browser implementations vary.
-    rewriteFunctionDeclarations(newFnNode.getLastChild());
+
+    if (renameLocals) {
+      // Now that parameter names have been replaced, make sure all the local
+      // names are unique, to allow functions to be inlined multiple times
+      // without causing conflicts.
+      makeLocalNamesUnique(newFnNode, isCallInLoop);
+
+      // Function declarations must be rewritten as function expressions as
+      // they will be within a block and normalization prevents function
+      // declarations within block as browser implementations vary.
+      rewriteFunctionDeclarations(newFnNode.getLastChild());
+    }
 
     // TODO(johnlenz): Mark NAME nodes constant for parameters that are not
     // modified.

src/com/google/javascript/jscomp/ProcessCommonJSModules.java

@@ -101,7 +101,17 @@ public void process(Node externs, Node root) {
     ImmutableList.Builder<ExportInfo> exports = ImmutableList.builder();
     if (finder.isCommonJsModule()) {
       finder.reportModuleErrors();
-      finder.replaceUmdPatterns();
+
+      if (!finder.umdPatterns.isEmpty()) {
+        finder.replaceUmdPatterns();
+
+        // Removing the IIFE rewrites vars. We need to re-traverse
+        // to get the new references.
+        if (removeIIFEWrapper(root)) {
+          finder = new FindImportsAndExports();
+          NodeTraversal.traverseEs6(compiler, root, finder);
+        }
+      }
 
       //UMD pattern replacement can leave detached export references - don't include those
       for (ExportInfo export : finder.getModuleExports()) {
@@ -156,6 +166,72 @@ private Node getBaseQualifiedNameNode(Node n) {
     return refParent;
   }
 
+  /**
+   * UMD modules are often wrapped in an IIFE for cases where they are used as scripts instead of
+   * modules. Remove the wrapper.
+   * @return Whether an IIFE wrapper was found and removed.
+   */
+  private boolean removeIIFEWrapper(Node root) {
+    Preconditions.checkState(root.isScript());
+    Node n = root.getFirstChild();
+
+    // Sometimes scripts start with a semicolon for easy concatenation.
+    // Skip any empty statements from those
+    while (n != null && n.isEmpty()) {
+      n = n.getNext();
+    }
+
+    // An IIFE wrapper must be the only non-empty statement in the script,
+    // and it must be an expression statement.
+    if (n == null || !n.isExprResult() || n.getNext() != null) {
+      return false;
+    }
+
+    Node call = n.getFirstChild();
+    if (call == null || !call.isCall()) {
+      return false;
+    }
+
+    // Find the IIFE call and function nodes
+    Node fnc;
+    if (call.getFirstChild().isFunction()) {
+      fnc = n.getFirstFirstChild();
+    } else if (call.getFirstChild().isGetProp()
+        && call.getFirstFirstChild().isFunction()
+        && call.getFirstFirstChild().getNext().isString()
+        && call.getFirstFirstChild().getNext().getString().equals("call")) {
+      fnc = call.getFirstFirstChild();
+
+      // We only support explicitly binding "this" to the parent "this"
+      if (!(call.getSecondChild() != null && call.getSecondChild().isThis())) {
+        return false;
+      }
+    } else {
+      return false;
+    }
+
+    if (NodeUtil.isVarArgsFunction(fnc)) {
+      return false;
+    }
+
+    CompilerInput ci = compiler.getInput(root.getInputId());
+    ModulePath modulePath = ci.getPath();
+    if (modulePath == null) {
+      return false;
+    }
+
+    String iifeLabel = modulePath.toModuleName() + "_iifeWrapper";
+
+    FunctionToBlockMutator mutator =
+        new FunctionToBlockMutator(compiler, compiler.getUniqueNameIdSupplier());
+    Node block = mutator.unwrapIifeInModule(iifeLabel, fnc, call);
+    root.removeChildren();
+    root.addChildrenToFront(block.removeChildren());
+    compiler.reportCodeChange();
+
+    return true;
+  }
+
   /**
    * Traverse the script. Find all references to CommonJS require (import) and module.exports or
    * export statements. Add goog.require statements for any require statements. Rewrites any require
@@ -503,6 +579,11 @@ void replaceUmdPatterns() {
         }
         p.replaceChild(umdPattern.ifRoot, newNode);
       }
+
+
+      if (!umdPatterns.isEmpty()) {
+        compiler.reportCodeChange();
+      }
     }
   }
 
@@ -658,11 +739,23 @@ private void visitExport(NodeTraversal t, Node export) {
         }
       }
 
+      ModulePath modulePath = t.getInput().getPath();
+      String moduleName = modulePath.toModuleName();
+
       // If this is an assignment to module.exports or exports, renaming
       // has already handled this case. Remove the export.
       Var rValueVar = null;
       if (rValue != null && rValue.isQualifiedName()) {
         rValueVar = t.getScope().getVar(rValue.getQualifiedName());
+
+        // If the exported name is not found and this is a direct assignment
+        // to modules.exports, look to see if the module name has a var definition
+        if (rValueVar == null && root == export) {
+          rValueVar = t.getScope().getVar(moduleName);
+          if (rValueVar != null && rValueVar.getNode() == root) {
+            rValueVar = null;
+          }
+        }
       }
 
       if (root.getParent().isAssign()
@@ -674,21 +767,20 @@ private void visitExport(NodeTraversal t, Node export) {
         return;
       }
 
-      ModulePath modulePath = t.getInput().getPath();
-      String moduleName = modulePath.toModuleName();
       Node updatedExport =
           NodeUtil.newName(compiler, moduleName, export, export.getQualifiedName());
 
-      // Ensure that direct assignments to "module.exports" have var definitions
       if ("module.exports".equals(root.getQualifiedName())
           && rValue != null
           && t.getScope().getVar("module.exports") == null
           && root.getParent().isAssign()
           && root.getParent().getParent().isExprResult()) {
+        // Rewrite "module.exports = foo;" to "var moduleName = foo;"
         Node parent = root.getParent();
         Node var = IR.var(updatedExport, rValue.detach()).useSourceInfoFrom(root.getParent());
         parent.getParent().replaceWith(var);
       } else {
+        // Other references to "module.exports" are just replaced with the module name.
         export.replaceWith(updatedExport);
       }
       compiler.reportCodeChange();