Delete support for goog.module.declareNamespace

------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=244438900
google · Apr 20, 2019 · 207d84a · 207d84a
1 parent ff73f3b
commit 207d84a
Show file tree

Hide file tree

Showing 13 changed files with 41 additions and 205 deletions.
diff --git a/src/com/google/javascript/jscomp/CheckMissingAndExtraRequires.java b/src/com/google/javascript/jscomp/CheckMissingAndExtraRequires.java
@@ -319,13 +319,10 @@ private void visitScriptNode() {
   private boolean isMissingRequire(String namespace, Node node) {
     if (namespace.startsWith("goog.global.")
         // Most functions in base.js are goog.someName, but
-        // goog.module.{get,declareLegacyNamespace,declareNamespace} are the exceptions, so just
+        // goog.module.{get,declareLegacyNamespace} are the exceptions, so just
         // check for them explicitly.
         || namespace.equals("goog.module.get")
-        || namespace.equals("goog.module.declareLegacyNamespace")
+        || namespace.equals("goog.module.declareLegacyNamespace")) {
-        // TODO(johnplaisted): Consolidate on declareModuleId.
-        || namespace.equals("goog.module.declareNamespace")
-        || namespace.equals("goog.declareModuleId")) {
       return false;
     }
 

diff --git a/src/com/google/javascript/jscomp/CompilerInput.java b/src/com/google/javascript/jscomp/CompilerInput.java
@@ -426,8 +426,7 @@ void visitSubtree(Node n, Node parent) {
             }
           } else if (parent.isGetProp()
               // TODO(johnplaisted): Consolidate on declareModuleId
-              && (parent.matchesQualifiedName("goog.module.declareNamespace")
+              && parent.matchesQualifiedName("goog.declareModuleId")
-                  || parent.matchesQualifiedName("goog.declareModuleId"))
               && parent.getParent().isCall()) {
             Node argument = parent.getParent().getSecondChild();
             if (!argument.isString()) {

diff --git a/src/com/google/javascript/jscomp/Es6RewriteModules.java b/src/com/google/javascript/jscomp/Es6RewriteModules.java
@@ -378,8 +378,7 @@ public void visit(NodeTraversal t, Node n, Node parent) {
       visitScript(t, n);
     } else if (n.isCall()) {
       // TODO(johnplaisted): Consolidate on declareModuleId.
-      if (n.getFirstChild().matchesQualifiedName("goog.module.declareNamespace")
+      if (n.getFirstChild().matchesQualifiedName("goog.declareModuleId")) {
-          || n.getFirstChild().matchesQualifiedName("goog.declareModuleId")) {
         n.getParent().detach();
       }
     }

diff --git a/src/com/google/javascript/jscomp/conformance.proto b/src/com/google/javascript/jscomp/conformance.proto
@@ -85,7 +85,7 @@ message Requirement {
     //  - A namespaced value or type:  namespace.Banned
     //  - A 'static' property:  "namespace.Foo.banned"
     // TODO(b/112325992): If namespace.Banned is a goog.module that does not
-    // call goog.module.declareNamespace, the rule will not match.
+    // call goog.declareModuleId, the rule will not match.
     BANNED_NAME = 3;
 
     // A banned instance property, for example:

diff --git a/src/com/google/javascript/jscomp/deps/JsFileParser.java b/src/com/google/javascript/jscomp/deps/JsFileParser.java
@@ -54,7 +54,7 @@ public final class JsFileParser extends JsFileLineParser {
           "(?:^|;)(?:[a-zA-Z0-9$_,:{}\\s]+=)?\\s*"
               + "goog\\.(?<func>provide|module|require|requireType|addDependency|declareModuleId)"
               // TODO(johnplaisted): Remove declareNamespace.
-              + "(?<subfunc>\\.declareNamespace)?\\s*\\((?<args>.*?)\\)");
+              + "\\s*\\((?<args>.*?)\\)");
 
   /**
    * Pattern for matching import ... from './path/to/file'.
@@ -301,8 +301,7 @@ protected boolean parseLine(String line) throws ParseException {
         // See if it's a require or provide.
         String methodName = googMatcher.group("func");
         char firstChar = methodName.charAt(0);
-        boolean isDeclareModuleNamespace =
+        boolean isDeclareModuleNamespace = firstChar == 'd';
-            firstChar == 'd' || (firstChar == 'm' && googMatcher.group("subfunc") != null);
         boolean isModule = !isDeclareModuleNamespace && firstChar == 'm';
         boolean isProvide = firstChar == 'p';
         boolean providesNamespace = isProvide || isModule || isDeclareModuleNamespace;

diff --git a/src/com/google/javascript/jscomp/ijs/ConvertToTypedInterface.java b/src/com/google/javascript/jscomp/ijs/ConvertToTypedInterface.java
@@ -76,9 +76,7 @@ public class ConvertToTypedInterface implements CompilerPass {
           "goog.forwardDeclare",
           "goog.module",
           "goog.module.declareLegacyNamespace",
-          // TODO(johnplaisted): Consolidate on declareModuleId / delete declareNamespace.
           "goog.declareModuleId",
-          "goog.module.declareNamespace",
           "goog.provide",
           "goog.require",
           "goog.requireType");

diff --git a/src/com/google/javascript/jscomp/lint/CheckEs6ModuleFileStructure.java b/src/com/google/javascript/jscomp/lint/CheckEs6ModuleFileStructure.java
@@ -98,9 +98,7 @@ private void checkOrder(NodeTraversal t, Node n, OrderedStatement statement) {
   private boolean visitExprResult(NodeTraversal t, Node exprResult, Node parent) {
     if (parent.isModuleBody() && exprResult.getFirstChild().isCall()) {
       Node call = exprResult.getFirstChild();
-      // TODO(johnplaisted): Remove declareNamespace.
+      if (call.getFirstChild().matchesQualifiedName("goog.declareModuleId")) {
-      if (call.getFirstChild().matchesQualifiedName("goog.module.declareNamespace")
-          || call.getFirstChild().matchesQualifiedName("goog.declareModuleId")) {
         checkOrder(t, call, OrderedStatement.DECLARE_MODULE_ID);
         return false;
       } else if (call.getFirstChild().matchesQualifiedName("goog.require")) {

diff --git a/test/com/google/javascript/jscomp/Es6RewriteModulesWithGoogInteropTest.java b/test/com/google/javascript/jscomp/Es6RewriteModulesWithGoogInteropTest.java
@@ -373,18 +373,7 @@ public void testGoogModuleGetForEs6Module() {
   public void testGoogModuleGetForEs6ModuleDeclaresNamespace() {
     test(
         srcs(
-            SourceFile.fromCode("es6.js", "export {}; goog.module.declareNamespace('es6');"),
+            SourceFile.fromCode("es6.js", "export let y; goog.declareModuleId('es6');"),
-            SourceFile.fromCode(
-                "closure.js",
-                "goog.module('my.module'); function f() { const y = goog.module.get('es6'); }")),
-        expected(
-            SourceFile.fromCode("es6.js", "/** @const */ var module$es6 = {};"),
-            SourceFile.fromCode(
-                "closure.js", "goog.module('my.module'); function f() { const y = module$es6; }")));
-
-    test(
-        srcs(
-            SourceFile.fromCode("es6.js", "export let y; goog.module.declareNamespace('es6');"),
             SourceFile.fromCode(
                 "closure.js",
                 "goog.module('my.module'); function f() { return goog.module.get('es6').y; }")),
@@ -415,10 +404,10 @@ public void testDeclareModuleId() {
   }
 
   @Test
-  public void testGoogRequireForDeclareNamespace() {
+  public void testGoogRequireForDeclareModuleId() {
     test(
         srcs(
-            SourceFile.fromCode("es6.js", "export var x; goog.module.declareNamespace('my.es6');"),
+            SourceFile.fromCode("es6.js", "export var x; goog.declareModuleId('my.es6');"),
             SourceFile.fromCode("goog.js", "const es6 = goog.require('my.es6'); use(es6, es6.x);")),
         expected(
             SourceFile.fromCode(
@@ -430,10 +419,10 @@ public void testGoogRequireForDeclareNamespace() {
   }
 
   @Test
-  public void testGoogRequireTypeForDeclareNamespace() {
+  public void testGoogRequireTypeForDeclareModuleId() {
     test(
         srcs(
-            SourceFile.fromCode("es6.js", "export var x; goog.module.declareNamespace('my.es6');"),
+            SourceFile.fromCode("es6.js", "export var x; goog.declareModuleId('my.es6');"),
             SourceFile.fromCode(
                 "goog.js",
                 lines(
@@ -462,11 +451,10 @@ public void testGoogRequireTypeForDeclareNamespace() {
   }
 
   @Test
-  public void testGoogRequireForDeclareNamespaceWithDestructure() {
+  public void testGoogRequireForDeclareModuleIdWithDestructure() {
     test(
         srcs(
-            SourceFile.fromCode(
+            SourceFile.fromCode("es6.js", "export var x, z; goog.declareModuleId('my.es6');"),
-                "es6.js", "export var x, z; goog.module.declareNamespace('my.es6');"),
             SourceFile.fromCode("goog.js", "const {x, z: y} = goog.require('my.es6'); use(x, y);")),
         expected(
             SourceFile.fromCode(
@@ -482,11 +470,10 @@ public void testGoogRequireForDeclareNamespaceWithDestructure() {
   }
 
   @Test
-  public void testGoogRequireTypeForDeclareNamespaceWithDestructure() {
+  public void testGoogRequireTypeForDeclareModuleIdWithDestructure() {
     test(
         srcs(
-            SourceFile.fromCode(
+            SourceFile.fromCode("es6.js", "export var x, z; goog.declareModuleId('my.es6');"),
-                "es6.js", "export var x, z; goog.module.declareNamespace('my.es6');"),
             SourceFile.fromCode(
                 "goog.js",
                 lines(
@@ -806,58 +793,11 @@ public void testForwardDeclareEs6Module() {
         FORWARD_DECLARE_FOR_ES6_SHOULD_BE_CONST);
   }
 
-  @Test
-  public void testForwardDeclareEs6ModuleDeclareNamespace() {
-    test(
-        srcs(
-            SourceFile.fromCode("es6.js", "export {}; goog.module.declareNamespace('es6');"),
-            SourceFile.fromCode(
-                "closure.js",
-                lines(
-                    "goog.module('my.module');",
-                    "const alias = goog.forwardDeclare('es6');",
-                    "let /** !alias.Type */ x;",
-                    "alias;"))),
-        expected(
-            SourceFile.fromCode("es6.js", "/** @const */ var module$es6 = {};"),
-            SourceFile.fromCode(
-                "closure.js",
-                lines("goog.module('my.module');", "let /** !module$es6.Type */ x;", "alias;"))));
-
-    test(
-        srcs(
-            SourceFile.fromCode("es6.js", "export {}; goog.module.declareNamespace('es6');"),
-            SourceFile.fromCode(
-                "closure.js",
-                lines(
-                    "goog.module('my.module');",
-                    "goog.forwardDeclare('es6');",
-                    "let /** !es6.Type */ x;",
-                    "es6;"))),
-        expected(
-            SourceFile.fromCode("es6.js", "/** @const */ var module$es6 = {};"),
-            SourceFile.fromCode(
-                "closure.js",
-                lines("goog.module('my.module');", "let /** !module$es6.Type */ x;", "es6;"))));
-
-    testError(
-        ImmutableList.of(
-            SourceFile.fromCode("es6.js", "export {}; goog.module.declareNamespace('es6');"),
-            SourceFile.fromCode(
-                "closure.js",
-                lines(
-                    "goog.module('my.module');",
-                    "let alias = goog.forwardDeclare('es6');",
-                    "let /** !alias.Type */ x;",
-                    "alias = goog.modle.get('es6');"))),
-        FORWARD_DECLARE_FOR_ES6_SHOULD_BE_CONST);
-  }
-
   @Test
   public void testWarnAboutRequireEs6FromEs6() {
     test(
         srcs(
-            SourceFile.fromCode("first.js", "goog.module.declareNamespace('first'); export {};"),
+            SourceFile.fromCode("first.js", "goog.declareModuleId('first'); export {};"),
             SourceFile.fromCode(
                 "second.js", "const first = goog.require('first'); export {first};")),
         expected(
@@ -872,7 +812,7 @@ public void testWarnAboutRequireEs6FromEs6() {
 
     test(
         srcs(
-            SourceFile.fromCode("first.js", "goog.module.declareNamespace('no.alias'); export {};"),
+            SourceFile.fromCode("first.js", "goog.declareModuleId('no.alias'); export {};"),
             SourceFile.fromCode("second.js", "goog.require('no.alias'); export {};")),
         expected(
             SourceFile.fromCode("first.js", "/** @const */ var module$first = {};"),
@@ -886,7 +826,7 @@ public void testGoogRequireTypeEs6ModuleInEs6Module() {
 
     test(
         srcs(
-            SourceFile.fromCode("es6.js", "export {}; goog.module.declareNamespace('es6');"),
+            SourceFile.fromCode("es6.js", "export {}; goog.declareModuleId('es6');"),
             SourceFile.fromCode(
                 "requiretype.js",
                 lines(
@@ -903,7 +843,7 @@ public void testGoogRequireTypeEs6ModuleInEs6Module() {
 
     test(
         srcs(
-            SourceFile.fromCode("es6.js", "export {}; goog.module.declareNamespace('es6');"),
+            SourceFile.fromCode("es6.js", "export {}; goog.declareModuleId('es6');"),
             SourceFile.fromCode(
                 "requiretype.js",
                 lines("export {};", "goog.requireType('es6');", "let /** !es6.Type */ x;"))),
@@ -920,7 +860,7 @@ public void testGoogRequireTypeEs6ModuleInEs6Module() {
   public void testGoogModuleGetEs6ModuleInEs6Module() {
     test(
         srcs(
-            SourceFile.fromCode("first.js", "goog.module.declareNamespace('first'); export let x;"),
+            SourceFile.fromCode("first.js", "goog.declareModuleId('first'); export let x;"),
             SourceFile.fromCode(
                 "second.js", "export function foo() { return goog.module.get('first').x; }")),
         expected(
@@ -944,7 +884,7 @@ public void testGoogModuleGetEs6ModuleInEs6Module() {
   public void testForwardDeclareEs6ModuleInEs6Module() {
     test(
         srcs(
-            SourceFile.fromCode("es6.js", "export {}; goog.module.declareNamespace('es6');"),
+            SourceFile.fromCode("es6.js", "export {}; goog.declareModuleId('es6');"),
             SourceFile.fromCode(
                 "forwarddeclare.js",
                 lines(
@@ -963,7 +903,7 @@ public void testForwardDeclareEs6ModuleInEs6Module() {
 
     test(
         srcs(
-            SourceFile.fromCode("es6.js", "export {}; goog.module.declareNamespace('es6');"),
+            SourceFile.fromCode("es6.js", "export {}; goog.declareModuleId('es6');"),
             SourceFile.fromCode(
                 "forwarddeclare.js",
                 lines(
@@ -982,7 +922,7 @@ public void testForwardDeclareEs6ModuleInEs6Module() {
 
     testError(
         ImmutableList.of(
-            SourceFile.fromCode("es6.js", "export {}; goog.module.declareNamespace('es6');"),
+            SourceFile.fromCode("es6.js", "export {}; goog.declareModuleId('es6');"),
             SourceFile.fromCode(
                 "closure.js",
                 lines(

diff --git a/test/com/google/javascript/jscomp/GatherModuleMetadataTest.java b/test/com/google/javascript/jscomp/GatherModuleMetadataTest.java
@@ -379,25 +379,16 @@ public void testEs6ModuleDeclareModuleId() {
   }
 
   @Test
-  public void testEs6ModuleDeclareNamespace() {
+  public void testEs6ModuleDeclareModuleIdImportedGoog() {
-    testSame("export var x; goog.module.declareNamespace('my.module');");
-    assertThat(metadataMap().getModulesByGoogNamespace().keySet()).containsExactly("my.module");
-    ModuleMetadata m = metadataMap().getModulesByGoogNamespace().get("my.module");
-    assertThat(m.googNamespaces()).containsExactly("my.module");
-    assertThat(m.isEs6Module()).isTrue();
-    assertThat(m.isGoogModule()).isFalse();
-  }
-
-  @Test
-  public void testEs6ModuleDeclareNamespaceImportedGoog() {
     testSame(
         ImmutableList.of(
             SourceFile.fromCode("goog.js", ""),
             SourceFile.fromCode(
-                "testcode", lines(
+                "testcode",
+                lines(
                     "import * as goog from './goog.js';",
                     "export var x;",
-                    "goog.module.declareNamespace('my.module');"))));
+                    "goog.declareModuleId('my.module');"))));
     assertThat(metadataMap().getModulesByGoogNamespace().keySet()).containsExactly("my.module");
     ModuleMetadata m = metadataMap().getModulesByGoogNamespace().get("my.module");
     assertThat(m.googNamespaces()).containsExactly("my.module");
@@ -441,12 +432,12 @@ public void testDuplicateProvideAndGoogModule() {
   public void testDuplicateProvideAndEs6Module() {
     testError(
         new String[] {
-          "goog.provide('duplciated');", "export {}; goog.module.declareNamespace('duplciated');"
+          "goog.provide('duplciated');", "export {}; goog.declareModuleId('duplciated');"
         },
         ClosureRewriteModule.DUPLICATE_NAMESPACE);
     testError(
         new String[] {
-          "export {}; goog.module.declareNamespace('duplciated');", "goog.provide('duplciated');"
+          "export {}; goog.declareModuleId('duplciated');", "goog.provide('duplciated');"
         },
         ClosureRewriteModule.DUPLICATE_MODULE);
   }
@@ -462,12 +453,12 @@ public void testDuplicateGoogModules() {
   public void testDuplicateGoogAndEs6Module() {
     testError(
         new String[] {
-            "goog.module('duplciated');", "export {}; goog.module.declareNamespace('duplciated');"
+          "goog.module('duplciated');", "export {}; goog.declareModuleId('duplciated');"
         },
         ClosureRewriteModule.DUPLICATE_MODULE);
     testError(
         new String[] {
-            "export {}; goog.module.declareNamespace('duplciated');", "goog.module('duplciated');"
+          "export {}; goog.declareModuleId('duplciated');", "goog.module('duplciated');"
         },
         ClosureRewriteModule.DUPLICATE_MODULE);
   }
@@ -476,14 +467,14 @@ public void testDuplicateGoogAndEs6Module() {
   public void testDuplicatEs6Modules() {
     testError(
         new String[] {
-          "export {}; goog.module.declareNamespace('duplciated');",
+          "export {}; goog.declareModuleId('duplciated');",
-          "export {}; goog.module.declareNamespace('duplciated');"
+          "export {}; goog.declareModuleId('duplciated');"
         },
         ClosureRewriteModule.DUPLICATE_MODULE);
     testError(
         new String[] {
-          "export {}; goog.module.declareNamespace('duplciated');",
+          "export {}; goog.declareModuleId('duplciated');",
-          "export {}; goog.module.declareNamespace('duplciated');"
+          "export {}; goog.declareModuleId('duplciated');"
         },
         ClosureRewriteModule.DUPLICATE_MODULE);
   }
@@ -596,7 +587,7 @@ public void testSetTestOnlyWithInvalidArg() {
   @Test
   public void testGatherFromExterns() {
     // js_lib will put data in externs for .i.js files.
-    test(externs("export var x; goog.module.declareNamespace('my.module');"), srcs(""));
+    test(externs("export var x; goog.declareModuleId('my.module');"), srcs(""));
     assertThat(metadataMap().getModulesByGoogNamespace().keySet()).containsExactly("my.module");
     ModuleMetadata m = metadataMap().getModulesByGoogNamespace().get("my.module");
     assertThat(m.googNamespaces()).containsExactly("my.module");