Make PolymerClassRewriter insert new nodes in MODULE_BODY nodes, not …

…SCRIPTs

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

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

@@ -41,7 +41,7 @@ final class ModuleImportResolver {
   }
 
   /** Returns whether this is a CALL node for goog.require(Type) or goog.forwardDeclare */
-  boolean isGoogModuleDependencyCall(Node value) {
+  static boolean isGoogModuleDependencyCall(Node value) {
     if (value == null || !value.isCall()) {
       return false;
     }

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

@@ -73,9 +73,10 @@ final class PolymerClassRewriter {
    * @param exprRoot The root expression of the call to Polymer({}).
    * @param cls The extracted {@link PolymerClassDefinition} for the Polymer element created by this
    *     call.
+   * @param isInGlobalOrModuleScope whether this call is either directly in a script or module body
    */
   void rewritePolymerCall(
-      Node exprRoot, final PolymerClassDefinition cls, boolean isInGlobalScope) {
+      Node exprRoot, final PolymerClassDefinition cls, boolean isInGlobalOrModuleScope) {
     Node objLit = checkNotNull(cls.descriptor);
 
     // Add {@code @lends} to the object literal.
@@ -135,17 +136,22 @@ void rewritePolymerCall(
     Node statements = block.removeChildren();
     Node parent = exprRoot.getParent();
 
-    // If the call to Polymer() is not in the global scope and the assignment target
-    // is not namespaced (which likely means it's exported to the global scope), put the type
-    // declaration into the global scope at the start of the current script.
-    //
-    // This avoids unknown type warnings which are a result of the compiler's poor understanding of
-    // types declared inside IIFEs or any non-global scope. We should revisit this decision as
-    // the typechecker's support for non-global types improves.
-    if (!isInGlobalScope && !cls.target.isGetProp()) {
-      Node scriptNode = NodeUtil.getEnclosingScript(parent);
-      scriptNode.addChildrenToFront(statements);
-      compiler.reportChangeToChangeScope(scriptNode);
+    // Put the type declaration in to either the enclosing module scope, if in a module, or the
+    // enclosing script node. Compiler support for local scopes like IIFEs is sometimes lacking but
+    // module scopes are well-supported. If this is not in a module or the global scope it is likely
+    // exported.
+    if (!isInGlobalOrModuleScope && !cls.target.isGetProp()) {
+      Node scriptOrModuleNode =
+          NodeUtil.getEnclosingNode(parent, node -> node.isScript() || node.isModuleBody());
+      if (scriptOrModuleNode.isModuleBody()
+          && scriptOrModuleNode.getParent().getBooleanProp(Node.GOOG_MODULE)) {
+        // The goog.module('ns'); call must remain the first statement in the module.
+        Node insertionPoint = getInsertionPointForGoogModule(scriptOrModuleNode);
+        scriptOrModuleNode.addChildrenAfter(statements, insertionPoint);
+      } else {
+        scriptOrModuleNode.addChildrenToFront(statements);
+        compiler.reportChangeToChangeScope(NodeUtil.getEnclosingScript(scriptOrModuleNode));
+      }
     } else {
       Node beforeRoot = exprRoot.getPrevious();
       if (beforeRoot == null) {
@@ -988,4 +994,19 @@ private static boolean isParamLiteral(String param) {
     }
     return false;
   }
+
+  private static Node getInsertionPointForGoogModule(Node moduleBody) {
+    checkArgument(moduleBody.isModuleBody(), moduleBody);
+    Node insertionPoint = moduleBody.getFirstChild(); // goog.module('ns');
+    Node next = insertionPoint.getNext();
+    while ((NodeUtil.isNameDeclaration(next)
+            && next.hasOneChild()
+            && ModuleImportResolver.isGoogModuleDependencyCall(next.getFirstFirstChild()))
+        || (NodeUtil.isExprCall(next)
+            && ModuleImportResolver.isGoogModuleDependencyCall(next.getOnlyChild()))) {
+      insertionPoint = next;
+      next = next.getNext();
+    }
+    return insertionPoint;
+  }
 }

test/com/google/javascript/jscomp/PolymerClassRewriterTest.java

@@ -64,13 +64,15 @@ public final class PolymerClassRewriterTest extends CompilerTypeTestCase {
   private Node rootNode;
   private GlobalNamespace globalNamespace;
   private Node polymerCall;
+  private boolean inGlobalScope;
 
   @Override
   @Before
   public void setUp() throws Exception {
     super.setUp();
     polymerCall = null;
     rootNode = null;
+    inGlobalScope = true;
   }
 
   // TODO(jlklein): Add tests for non-global definitions, interface externs, read-only setters, etc.
@@ -98,6 +100,103 @@ public void testVarTarget() {
             "};"));
   }
 
+  @Test
+  public void testVarTarget_inGoogModule() {
+    inGlobalScope = false;
+    test(
+        lines(
+            "goog.module('mod');", //
+            "var X = Polymer({",
+            "  is: 'x-element',",
+            "});"),
+        lines(
+            "goog.module('mod');",
+            "/** @constructor @extends {PolymerElement} @implements {PolymerXInterface} */",
+            "var X = function() {};",
+            "X = Polymer(/** @lends {X.prototype} */ {",
+            "  is: 'x-element',",
+            "});"));
+
+    testSame(
+        lines(
+            "goog.module('mod');",
+            "var X = class extends Polymer.Element {",
+            "  static get is() { return 'x-element'; }",
+            "  static get properties { return { }; }",
+            "};"));
+  }
+
+  @Test
+  public void testVarTarget_inIifeInGoogModule() {
+    inGlobalScope = false;
+    test(
+        lines(
+            "goog.module('mod');", //
+            "(function() {",
+            "  var X = Polymer({",
+            "    is: 'x-element',",
+            "  });",
+            "})();"),
+        lines(
+            "goog.module('mod');",
+            "/** @constructor @extends {PolymerElement} @implements {PolymerXInterface} */",
+            "var X = function() {};",
+            "(function() {",
+            "    X = Polymer(/** @lends {X.prototype} */ {",
+            "    is: 'x-element',",
+            "  });",
+            "})();"));
+  }
+
+  @Test
+  public void testVarTarget_inGoogModuleWithRequires() {
+    inGlobalScope = false;
+    test(
+        lines(
+            "goog.module('mod');", //
+            "const Component = goog.require('goog.Component');",
+            "goog.forwardDeclare('something.else');",
+            "const someLocal = (function() { return 0; })();",
+            "var X = Polymer({",
+            "  is: 'x-element',",
+            "});"),
+        lines(
+            "goog.module('mod');",
+            "const Component = goog.require('goog.Component');",
+            "goog.forwardDeclare('something.else');",
+            "/** @constructor @extends {PolymerElement} @implements {PolymerXInterface} */",
+            "var X = function() {};",
+            "const someLocal = (function() { return 0; })();",
+            "X = Polymer(/** @lends {X.prototype} */ {",
+            "  is: 'x-element',",
+            "});"));
+  }
+
+  @Test
+  public void testVarTarget_inEsModule() {
+    test(
+        lines(
+            "var X = Polymer({", //
+            "  is: 'x-element',",
+            "});",
+            "export {X};"),
+        lines(
+            "/** @constructor @extends {PolymerElement} @implements {PolymerXInterface} */",
+            "var X = function() {};",
+            "X = Polymer(/** @lends {X.prototype} */ {",
+            "  is: 'x-element',",
+            "});",
+            "export {X};"));
+
+    testSame(
+        lines(
+            "var X = class extends Polymer.Element {",
+            "  static get is() { return 'x-element'; }",
+            "  static get properties { return { }; }",
+            "};",
+            "export {X};"));
+  }
+
   @Test
   public void testDefaultTypeNameTarget() {
     test(
@@ -144,6 +243,7 @@ protected CompilerOptions getDefaultOptions() {
     options.setWarningLevel(
         DiagnosticGroups.INVALID_CASTS, CheckLevel.WARNING);
     options.setWarningLevel(DiagnosticGroups.LINT_CHECKS, CheckLevel.WARNING);
+    options.setWarningLevel(DiagnosticGroups.MODULE_LOAD, CheckLevel.OFF);
     options.setCodingConvention(getCodingConvention());
     options.setPreserveTypeAnnotations(true);
     options.setPrettyPrint(true);
@@ -153,21 +253,21 @@ protected CompilerOptions getDefaultOptions() {
   private void test(String originalCode, String expectedResult) {
     parseAndRewrite(originalCode, 1);
     Node expectedNode = compiler.parseSyntheticCode(expectedResult);
-    assertNode(expectedNode).isEqualTo(rootNode);
+    assertNode(rootNode).isEqualTo(expectedNode);
 
     parseAndRewrite(originalCode, 2);
     expectedNode = compiler.parseSyntheticCode(expectedResult);
-    assertNode(expectedNode).isEqualTo(rootNode);
+    assertNode(rootNode).isEqualTo(expectedNode);
   }
 
   private void testSame(String originalCode) {
     parseAndRewrite(originalCode, 1);
     Node expectedNode = compiler.parseSyntheticCode(originalCode);
-    assertNode(expectedNode).isEqualTo(rootNode);
+    assertNode(rootNode).isEqualTo(expectedNode);
 
     parseAndRewrite(originalCode, 2);
     expectedNode = compiler.parseSyntheticCode(originalCode);
-    assertNode(expectedNode).isEqualTo(rootNode);
+    assertNode(rootNode).isEqualTo(expectedNode);
   }
 
   private void parseAndRewrite(String code, int version) {
@@ -203,9 +303,9 @@ public void visit(Node node) {
     Node parent = polymerCall.getParent();
     Node grandparent = parent.getParent();
     if (NodeUtil.isNameDeclaration(grandparent) || parent.isAssign()) {
-      rewriter.rewritePolymerCall(grandparent, classDef, true);
+      rewriter.rewritePolymerCall(grandparent, classDef, inGlobalScope);
     } else {
-      rewriter.rewritePolymerCall(parent, classDef, true);
+      rewriter.rewritePolymerCall(parent, classDef, inGlobalScope);
     }
   }