Allow assigning the result of goog.define

Adds a new AST property to store the identifier passed to goog.define, since the string is otherwise thrown away from the AST.  Updates ProcessClosurePrimitives to no longer throw an error for this case.  Updates ProcessDefines to look for and use the property.

This is important to support using goog.define in modules, where we no longer want to assign a global value.

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

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

@@ -32,7 +32,6 @@
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.javascript.jscomp.AbstractCompiler.LifeCycleStage;
 import com.google.javascript.jscomp.CompilerOptions.ExtractPrototypeMemberDeclarationsMode;
@@ -2158,18 +2157,13 @@ protected FeatureSet featureSet() {
       new PassFactory("processDefines", true) {
         @Override
         protected CompilerPass create(final AbstractCompiler compiler) {
-          return new CompilerPass() {
-            @Override
-            public void process(Node externs, Node jsRoot) {
-              HashMap<String, Node> replacements = new HashMap<>();
-              replacements.putAll(compiler.getDefaultDefineValues());
-              replacements.putAll(getAdditionalReplacements(options));
-              replacements.putAll(options.getDefineReplacements());
-              new ProcessDefines(compiler, ImmutableMap.copyOf(replacements), !options.checksOnly)
-                  .injectNamespace(namespaceForChecks)
-                  .process(externs, jsRoot);
-            }
-          };
+          return new ProcessDefines.Builder(compiler)
+              .putReplacements(compiler.getDefaultDefineValues())
+              .putReplacements(getAdditionalReplacements(options))
+              .putReplacements(options.getDefineReplacements())
+              .checksOnly(options.checksOnly)
+              .injectNamespace(() -> namespaceForChecks)
+              .build();
         }
 
         @Override

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

@@ -35,7 +35,7 @@ public void process(Node externs, Node root) {
     if (!J2clSourceFileChecker.shouldRunJ2clPasses(compiler)) {
       return;
     }
-    defines = new ProcessDefines(compiler, null, true).collectDefines(externs, root).keySet();
+    defines = new ProcessDefines.Builder(compiler).build().collectDefines(externs, root).keySet();
     NodeTraversal.traverse(compiler, root, this);
   }
 

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

@@ -272,16 +272,32 @@ private Node getAnyValueOfType(JSDocInfo jsdoc) {
    */
   private void replaceGoogDefines(Node n) {
     Node parent = n.getParent();
-    checkState(parent.isExprResult());
     String name = n.getSecondChild().getString();
     JSDocInfo jsdoc = n.getJSDocInfo();
     Node value =
         n.isFromExterns() ? getAnyValueOfType(jsdoc).srcref(n) : n.getChildAtIndex(2).detach();
 
-    Node replacement = NodeUtil.newQNameDeclaration(compiler, name, value, jsdoc);
-    replacement.useSourceInfoIfMissingFromForTree(parent);
-    parent.replaceWith(replacement);
-    compiler.reportChangeToEnclosingScope(replacement);
+    switch (parent.getToken()) {
+      case EXPR_RESULT:
+        Node replacement = NodeUtil.newQNameDeclaration(compiler, name, value, jsdoc);
+        replacement.useSourceInfoIfMissingFromForTree(parent);
+        parent.replaceWith(replacement);
+        compiler.reportChangeToEnclosingScope(replacement);
+        break;
+      case NAME:
+        parent.setDefineName(name);
+        n.replaceWith(value);
+        compiler.reportChangeToEnclosingScope(parent);
+        break;
+      case ASSIGN:
+        checkState(n == parent.getLastChild());
+        parent.getFirstChild().setDefineName(name);
+        n.replaceWith(value);
+        compiler.reportChangeToEnclosingScope(parent);
+        break;
+      default:
+        throw new IllegalStateException("goog.define outside of EXPR_RESULT, NAME, or ASSIGN");
+    }
   }
 
   @Override
@@ -308,9 +324,7 @@ public void visit(NodeTraversal t, Node n, Node parent) {
                 processBaseClassCall(t, n);
                 break;
               case "define":
-                if (validateUnaliasablePrimitiveCall(t, n, methodName)) {
-                  processDefineCall(t, n, parent);
-                }
+                processDefineCall(t, n, parent);
                 break;
               case "require":
               case "requireType":
@@ -448,7 +462,7 @@ private boolean validatePrimitiveCallWithMessage(
     if (!t.inGlobalHoistScope()) {
       compiler.report(t.makeError(n, INVALID_CLOSURE_CALL_SCOPE_ERROR));
       return false;
-    } else if (!n.getParent().isExprResult()) {
+    } else if (!n.getParent().isExprResult() && !"goog.define".equals(methodName)) {
       // If the call is in the global hoist scope, but the result is used
       compiler.report(t.makeError(n, invalidAliasingError, GOOG + "." + methodName));
       return false;
@@ -1142,15 +1156,30 @@ private boolean verifyProvide(NodeTraversal t, Node methodName, Node arg) {
   }
 
   /**
-   * Verifies that a provide method call has exactly one argument,
-   * and that it's a string literal and that the contents of the string are
-   * valid JS tokens. Reports a compile error if it doesn't.
+   * Verifies that a goog.define method call has exactly two arguments, with the first a string
+   * literal whose contents is a valid JS qualified name. Reports a compile error if it doesn't.
    *
    * @return Whether the argument checked out okay
    */
-  private boolean verifyDefine(NodeTraversal t,
-      Node expr,
-      Node methodName, Node args) {
+  private boolean verifyDefine(NodeTraversal t, Node parent, Node methodName, Node args) {
+    // Calls to goog.define must be in the global hoist scope.  This is copied from
+    // validate(Un)aliasablePrimitiveCall.
+    // TODO(sdh): loosen this restriction if the results are assigned?
+    if (!compiler.getOptions().shouldPreserveGoogModule() && !t.inGlobalHoistScope()) {
+      compiler.report(t.makeError(methodName.getParent(), INVALID_CLOSURE_CALL_SCOPE_ERROR));
+      return false;
+    }
+
+    // It is an error for goog.define to show up anywhere except on its own or immediately after =.
+    if (parent.isAssign() && parent.getParent().isExprResult()) {
+      parent = parent.getParent();
+    }
+    if (parent.isName() && NodeUtil.isNameDeclaration(parent.getParent())) {
+      parent = parent.getGrandparent();
+    } else if (!parent.isExprResult()) {
+      compiler.report(t.makeError(methodName.getParent(), INVALID_CLOSURE_CALL_SCOPE_ERROR));
+      return false;
+    }
 
     // Verify first arg
     Node arg = args;
@@ -1172,9 +1201,9 @@ private boolean verifyDefine(NodeTraversal t,
       return false;
     }
 
-    JSDocInfo info = expr.getFirstChild().getJSDocInfo();
+    JSDocInfo info = parent.getFirstChild().getJSDocInfo();
     if (info == null || !info.isDefine()) {
-      compiler.report(t.makeError(expr, MISSING_DEFINE_ANNOTATION));
+      compiler.report(t.makeError(parent, MISSING_DEFINE_ANNOTATION));
       return false;
     }
     return true;