Disallow local type declarations inside goog.scope

The .i.js generation step removes local goog.scope declarations, so without this it is possible to be left with dangling references to types declared inside goog.scopes. This pattern is fully supported inside goog.module/ES6 modules, however. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=187519420
google · Mar 3, 2018 · 723b053 · 723b053
1 parent c80da22
commit 723b053
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 2 deletions.
diff --git a/src/com/google/javascript/jscomp/ijs/ConvertToTypedInterface.java b/src/com/google/javascript/jscomp/ijs/ConvertToTypedInterface.java
@@ -57,6 +57,13 @@ public class ConvertToTypedInterface implements CompilerPass {
           "JSC_CONSTANT_WITHOUT_EXPLICIT_TYPE",
           "Constants in top-level should have types explicitly specified.");
 
+  static final DiagnosticType GOOG_SCOPE_HIDDEN_TYPE =
+      DiagnosticType.warning(
+          "JSC_GOOG_SCOPE_HIDDEN_TYPE",
+          "Please do not use goog.scope to hide declarations.\n"
+      + "It is preferable to either create an @private namespaced declaration, or migrate "
+              + "to goog.module.");
+
   private static final ImmutableSet<String> CALLS_TO_PRESERVE =
       ImmutableSet.of(
           "goog.addSingletonGetter",
@@ -541,6 +548,10 @@ private boolean shouldRemove(String name, PotentialDeclaration decl) {
       if ("$jscomp".equals(rootName(name))) {
         // These are created by goog.scope processing, but clash with each other
         // and should not be depended on.
+        if (decl.getRhs() != null && decl.getRhs().isClass()
+            || decl.getJsDoc() != null && decl.getJsDoc().containsTypeDefinition()) {
+          compiler.report(JSError.make(decl.getLhs(), GOOG_SCOPE_HIDDEN_TYPE));
+        }
         return true;
       }
       // This looks like an update rather than a declaration in this file.

diff --git a/test/com/google/javascript/jscomp/ijs/ConvertToTypedInterfaceTest.java b/test/com/google/javascript/jscomp/ijs/ConvertToTypedInterfaceTest.java
@@ -1336,7 +1336,8 @@ public void testGoogScopeLeftoversAreRemoved() {
         lines(
             "goog.provide('a.b.c.d.e.f.g');",
             "",
-            "a.b.c.d.e.f.g.Foo = class {};"));
+            "a.b.c.d.e.f.g.Foo = class {};"),
+        warning(ConvertToTypedInterface.GOOG_SCOPE_HIDDEN_TYPE));
 
     test(
         lines(
@@ -1354,7 +1355,8 @@ public void testGoogScopeLeftoversAreRemoved() {
         lines(
             "goog.provide('a.b.c.d.e.f.g');",
             "",
-            "a.b.c.d.e.f.g.Foo = class {};"));
+            "a.b.c.d.e.f.g.Foo = class {};"),
+    warning(ConvertToTypedInterface.GOOG_SCOPE_HIDDEN_TYPE));
   }
 
   public void testDestructuringDoesntCrash() {