Fix in compiler pass: cr.define() can export variable without assigned value

third_party/closure_compiler/runner/test/com/google/javascript/jscomp/ChromePassTest.java

Index: third_party/closure_compiler/runner/test/com/google/javascript/jscomp/ChromePassTest.java
diff --git a/third_party/closure_compiler/runner/test/com/google/javascript/jscomp/ChromePassTest.java b/third_party/closure_compiler/runner/test/com/google/javascript/jscomp/ChromePassTest.java
index b8db07d9b2f19f9d739cb76339ca3cd6f1c3c900..267b509d86167789cb7ce35fc23451e572b94a35 100644
--- a/third_party/closure_compiler/runner/test/com/google/javascript/jscomp/ChromePassTest.java
+++ b/third_party/closure_compiler/runner/test/com/google/javascript/jscomp/ChromePassTest.java
@@ -76,7 +76,7 @@ public class ChromePassTest extends CompilerTestCase {
             "});");
     }
 
-    public void testCrDefineReassignsExportedFunctionByQualifiedName() throws Exception {
+    public void testCrDefineReassignsExportedVarByQualifiedName() throws Exception {
         test(
             "cr.define('namespace', function() {\n" +
             "  var internalStaticMethod = function() {\n" +
@@ -97,6 +97,42 @@ public class ChromePassTest extends CompilerTestCase {
             "});");
     }
 
+    public void testCrDefineExportsVarsWithoutAssignment() throws Exception {
+        test(
+            "cr.define('namespace', function() {\n" +
+            "  var a;" +
+            "  return {\n" +
+            "    a: a\n" +
+            "  };\n" +
+            "});\n",

[Tyler Breisacher (Chromium), 2014/08/18 16:51:00]

You're already using Guava elsewhere so you should be able to use a Joiner here,
I think.

test(Joiner.on('\n').join(
   "first line of code",
   "second line",
   "etc."), Joiner.on('\n').join(
   "var namespace = ...",
   "etc."));

A lot nicer to read than all the \n's and +'s :)

[Dan Beam, 2014/08/19 21:14:19]

where are we using Guava?  could we avoid adding new dependencies if we use
compiler.jar to the Class-path?
eh

[Tyler Breisacher (Chromium), 2014/08/19 21:19:47]

https://code.google.com/p/chromium/codesearch#chromium/src/third_party/closur...

I'm not sure how the dependencies work in Chrome, but there is also a copy of
Guava at
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/guava/

[Dan Beam, 2014/08/19 21:42:09]

i assume that works because of:
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/closur...

+            "var namespace = namespace || {};\n" +
+            "cr.define('namespace', function() {\n" +
+            "  namespace.a;\n" +
+            "  return {\n" +
+            "    a: namespace.a\n" +
+            "  };\n" +
+            "});\n");
+    }
+
+    public void testCrDefineExportsVarsWithoutAssignmentWithJSDoc() throws Exception {
+        test(
+            "cr.define('namespace', function() {\n" +
+            "  /** @type {number} */\n" +
+            "  var a;" +

[Dan Beam, 2014/08/18 18:36:46]

so is this only change from a => namespace.a in the case that it's exported?

[Dan Beam, 2014/08/18 18:57:45]

said in another way

  cr.define('ui', function() {
    var a = 1;
    var b = 2;
    return {b: b};
  });

|ui.a| will be undefined and |ui.b| == 2, right?

+            "  return {\n" +
+            "    a: a\n" +
+            "  };\n" +
+            "});\n",
+            "var namespace = namespace || {};\n" +
+            "cr.define('namespace', function() {\n" +
+            "  /** @type {number} */\n" +
+            "  namespace.a;\n" +
+            "  return {\n" +
+            "    a: namespace.a\n" +
+            "  };\n" +
+            "});\n");
+    }
+
     public void testCrDefineCopiesJSDocForExportedVariable() throws Exception {
         test(
             "cr.define('namespace', function() {\n" +