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

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

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 package com.google.javascript.jscomp;
 
 /**
  * Tests {@link ChromePass}.
  */
 public class ChromePassTest extends CompilerTestCase {
@@ skipping 58 matching lines @@
             "  /** I'm function's JSDoc */\n" +
             "  namespace.externalStaticMethod = function internalStaticMethod() {\n" +
             "    alert(42);\n" +
             "  }\n" +
             "  return {\n" +
             "    externalStaticMethod: namespace.externalStaticMethod\n" +
             "  };\n" +
             "});");
     }
 
-    public void testCrDefineReassignsExportedFunctionByQualifiedName() throws Exception {
+    public void testCrDefineReassignsExportedVarByQualifiedName() throws Exception {
         test(
             "cr.define('namespace', function() {\n" +
             "  var internalStaticMethod = function() {\n" +
             "    alert(42);\n" +
             "  }\n" +
             "  return {\n" +
             "    externalStaticMethod: internalStaticMethod\n" +
             "  };\n" +
             "});",
             "var namespace = namespace || {};\n" +
             "cr.define('namespace', function() {\n" +
             "  namespace.externalStaticMethod = function() {\n" +
             "    alert(42);\n" +
             "  }\n" +
             "  return {\n" +
             "    externalStaticMethod: namespace.externalStaticMethod\n" +
             "  };\n" +
             "});");
     }
 
+    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" +
             "  /** I'm function's JSDoc */\n" +
             "  var internalStaticMethod = function() {\n" +
             "    alert(42);\n" +
             "  }\n" +
             "  return {\n" +
             "    externalStaticMethod: internalStaticMethod\n" +
             "  };\n" +
@@ skipping 192 matching lines @@
             "cr.define('cr.ui', function() {\n" +
             "  return {};\n" +
             "});");
     }
 
     public void testCrExportPathInvalidNumberOfArguments() throws Exception {
         test("cr.exportPath();", null, ChromePass.CR_EXPORT_PATH_WRONG_NUMBER_OF_ARGUMENTS);
     }
 
 }