PolymerPass now generates goog.exportProperty calls instead of augmen…

…ting the externs when protecting Polymer element methods from renaming and dead code removal.

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

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

@@ -31,7 +31,6 @@
 import com.google.javascript.rhino.Node;
 import com.google.javascript.rhino.Token;
 import java.util.ArrayList;
-import java.util.Collection;
 import java.util.HashMap;
 import java.util.LinkedHashMap;
 import java.util.List;
@@ -128,7 +127,7 @@ void rewritePolymerCall(
     ImmutableList<MemberDefinition> readOnlyProps = parseReadOnlyProperties(cls, block);
     ImmutableList<MemberDefinition> attributeReflectedProps =
         parseAttributeReflectedProperties(cls);
-    addInterfaceExterns(cls, readOnlyProps, attributeReflectedProps);
+    createExportsAndExterns(cls, readOnlyProps, attributeReflectedProps);
     removePropertyDocs(objLit, PolymerClassDefinition.DefinitionType.ObjectLiteral);
 
     Node statements = block.removeChildren();
@@ -218,7 +217,7 @@ void rewritePolymerClassDeclaration(
     ImmutableList<MemberDefinition> readOnlyProps = parseReadOnlyProperties(cls, block);
     ImmutableList<MemberDefinition> attributeReflectedProps =
         parseAttributeReflectedProperties(cls);
-    addInterfaceExterns(cls, readOnlyProps, attributeReflectedProps);
+    createExportsAndExterns(cls, readOnlyProps, attributeReflectedProps);
 
     // If an external interface is required, mark the class as implementing it
     if (polymerExportPolicy == PolymerExportPolicy.EXPORT_ALL
@@ -426,19 +425,6 @@ private void appendPropertiesToBlock(
     }
   }
 
-  /** Appends all of the given methods to the given block. */
-  private void appendMethodsToBlock(
-      final Collection<MemberDefinition> methods, Node block, String basePath) {
-    for (MemberDefinition method : methods) {
-      Node propertyNode = IR.exprResult(
-          NodeUtil.newQName(compiler, basePath + method.name.getString()));
-      propertyNode.useSourceInfoIfMissingFromForTree(method.name);
-      JSDocInfoBuilder info = JSDocInfoBuilder.maybeCopyFrom(method.info);
-      propertyNode.getFirstChild().setJSDocInfo(info.build());
-      block.addChildToBack(propertyNode);
-    }
-  }
-
   /** Remove all JSDocs from properties of a class definition */
   private void removePropertyDocs(
       final Node objLit, PolymerClassDefinition.DefinitionType defType) {
@@ -539,20 +525,33 @@ private Node makeReadOnlySetter(String propName, String qualifiedPath) {
 
     JSDocInfoBuilder info = new JSDocInfoBuilder(true);
     // This is overriding a generated function which was added to the interface in
-    // {@code addInterfaceExterns}.
+    // {@code createExportsAndExterns}.
     info.recordOverride();
     exprResNode.getFirstChild().setJSDocInfo(info.build());
 
     return exprResNode;
   }
 
   /**
-   * Adds an interface for the given ClassDefinition to externs. This allows generated setter
-   * functions for read-only properties to avoid renaming altogether.
+   * Create exports and externs to protect element properties and methods from renaming and dead
+   * code removal.
    *
-   * @see https://www.polymer-project.org/0.8/docs/devguide/properties.html#read-only
+   * <p>Since Polymer templates, observers, and computed properties rely on string references to
+   * element properties and methods, and because we don't have a way to update those references
+   * reliably, we instead export or extern them.
+   *
+   * <p>For properties, we create a new interface called {@code Polymer<ElementName>Interface}, add
+   * all element properties to it, mark that the element class {@code @implements} this interface,
+   * and add the interface to the Closure externs. The specific set of properties we add to this
+   * interface is determined by the value of {@code polymerExportPolicy}.
+   *
+   * <p>For methods, when {@code polymerExportPolicy = EXPORT_ALL}, we instead generate {@code
+   * goog.exportProperty} calls. This is preferable to using the externs, since it can result in
+   * better minified code (since compiled code can still use the minified name), avoids unnecessary
+   * {@code missingOverride} checks, and avoids needing to keep the signatures of the element class
+   * and the generated interface synchronized (previously the source of a bug).
    */
-  private void addInterfaceExterns(
+  private void createExportsAndExterns(
       final PolymerClassDefinition cls,
       List<MemberDefinition> readOnlyProps,
       List<MemberDefinition> attributeReflectedProps) {
@@ -591,7 +590,14 @@ private void addInterfaceExterns(
       for (MemberDefinition method : cls.methods) {
         uniqueMethods.put(method.name.getString(), method);
       }
-      appendMethodsToBlock(uniqueMethods.values(), block, interfaceBasePath);
+      for (MemberDefinition method : uniqueMethods.values()) {
+        addExportPropertyCall(
+            cls.target.getQualifiedName() + ".prototype",
+            method.name.getString(),
+            cls.target.getQualifiedName() + ".prototype." + method.name.getString(),
+            method.value,
+            cls.definition);
+      }
 
     } else if (polymerVersion == 1) {
       // For Polymer 1, all declared properties are non-renameable
@@ -634,6 +640,47 @@ private void addInterfaceExterns(
     compiler.reportChangeToEnclosingScope(stmts);
   }
 
+  /**
+   * Insert a call to {@code goog.exportProperty} (or equivalent function depending on current
+   * coding convention).
+   *
+   * <p>Note this function is very similar to {@link GenerateExports#addExportPropertyCall}.
+   *
+   * @param object Object whose static property is being exported (e.g. @{code MyClass.prototype}).
+   * @param publicName Unobfuscated name to export (e.g. @{code "myMethod"}).
+   * @param symbol Object the name should point to (e.g. @{code MyClass.prototype.myMethod}).
+   * @param source The context node to use for source info.
+   * @param insertAfter The call will be inserted as the next sibling of this node (or as close as
+   *     possible).
+   */
+  private void addExportPropertyCall(
+      String object, String publicName, String symbol, Node source, Node insertAfter) {
+    CodingConvention convention = compiler.getCodingConvention();
+
+    // Generate the `goog.exportProperty(object, publicName, symbol);` call.
+    String exportPropertyFunction = convention.getExportPropertyFunction();
+    Node call =
+        IR.call(
+            NodeUtil.newQName(
+                compiler, exportPropertyFunction,
+                source, exportPropertyFunction),
+            NodeUtil.newQName(
+                compiler, object,
+                source, exportPropertyFunction),
+            IR.string(publicName),
+            NodeUtil.newQName(
+                compiler, symbol,
+                source, exportPropertyFunction));
+    Node expression = IR.exprResult(call).useSourceInfoIfMissingFromForTree(source);
+
+    // Walk up until we find a statement we can insert after.
+    while (!NodeUtil.isStatementBlock(insertAfter.getParent())) {
+      insertAfter = insertAfter.getParent();
+    }
+    insertAfter.getParent().addChildAfter(expression, insertAfter);
+    compiler.reportChangeToEnclosingScope(expression);
+  }
+
   /**
    * @return The name of the generated extern interface which the element implements.
    */

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

@@ -55,6 +55,12 @@ public final class IntegrationTest extends IntegrationTestCase {
   private static final String CLOSURE_COMPILED =
       "var COMPILED = true; var goog$exportSymbol = function() {};";
 
+  private static final String EXPORT_PROPERTY_DEF =
+      lines(
+          "goog.exportProperty = function(object, publicName, symbol) {",
+          "  object[publicName] = symbol;",
+          "};");
+
   @Test
   public void testStaticMemberClass() {
     CompilerOptions options = createCompilerOptions();
@@ -1034,18 +1040,20 @@ public void testPolymerExportPolicyExportAll() {
     options.polymerExportPolicy = PolymerExportPolicy.EXPORT_ALL;
 
     Compiler compiler =
-        compile(options,
-        lines(
-            "class FooElement extends PolymerElement {",
-            "  static get properties() {",
-            "    return {",
-            "      longUnusedProperty: String,",
-            "    }",
-            "  }",
-            "  longUnusedMethod() {",
-            "    return this.longUnusedProperty;",
-            "  }",
-            "}"));
+        compile(
+            options,
+            lines(
+                EXPORT_PROPERTY_DEF,
+                "class FooElement extends PolymerElement {",
+                "  static get properties() {",
+                "    return {",
+                "      longUnusedProperty: String,",
+                "    }",
+                "  }",
+                "  longUnusedMethod() {",
+                "    return this.longUnusedProperty;",
+                "  }",
+                "}"));
     String source = compiler.getCurrentJsSource();
 
     // If we see these identifiers anywhere in the output source, we know that we successfully
@@ -1074,6 +1082,7 @@ public void testPolymerPropertyDeclarationsWithConstructor() {
         compile(
             options,
             lines(
+                EXPORT_PROPERTY_DEF,
                 "class FooElement extends PolymerElement {",
                 "  constructor() {",
                 "    super();",

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

@@ -121,6 +121,12 @@ public class PolymerPassTest extends CompilerTestCase {
           " */",
           "$jscomp.reflectObject = function (type, object) { return object; };");
 
+  private static final String EXPORT_PROPERTY_DEF =
+      lines(
+          "goog.exportProperty = function(object, publicName, symbol) {",
+          "  object[publicName] = symbol;",
+          "};");
+
   private int polymerVersion = 1;
   private PolymerExportPolicy polymerExportPolicy = PolymerExportPolicy.LEGACY;
   private boolean propertyRenamingEnabled = false;
@@ -3549,18 +3555,42 @@ public void testObjectReflectionAddedToConfigProperties3() {
             "XElement.prototype.thingToDo;"));
   }
 
+  @Test
+  public void textExportsMethodsFromClassBasedElement() {
+    polymerExportPolicy = PolymerExportPolicy.EXPORT_ALL;
+    test(
+        2,
+        lines(
+            EXPORT_PROPERTY_DEF,
+            "class TestElement extends PolymerElement {",
+            "  method1() {}",
+            "  method2() {}",
+            "}"),
+        lines(
+            EXPORT_PROPERTY_DEF,
+            "/** @implements {PolymerTestElementInterface} */",
+            "class TestElement extends PolymerElement {",
+            "  method1() {}",
+            "  method2() {}",
+            "}",
+            "goog.exportProperty(TestElement.prototype, \"method2\",",
+            "    TestElement.prototype.method2);",
+            "goog.exportProperty(TestElement.prototype, \"method1\",",
+            "    TestElement.prototype.method1);"));
+  }
+
   /**
-   * When --polymer_export_policy=EXPORT_ALL, the PolymerPass will add all methods of an element to
-   * that element's generated interface (which is injected into the externs), including methods
-   * inherited from Polymer Behaviors. Ensure that each method is included on the interface only
-   * once, even when it is implemented in multiple places.
+   * When --polymer_export_policy=EXPORT_ALL, the PolymerPass will export all methods of an element
+   * including methods inherited from Polymer Behaviors. Ensure that each method is included on the
+   * interface only once, even when it is implemented in multiple places.
    */
   @Test
-  public void testExportAllOnlyExportsEachMethodOnce() {
+  public void textExportsUniqueMethodsFromLegacyElementAndBehaviors() {
     polymerExportPolicy = PolymerExportPolicy.EXPORT_ALL;
-
-    String js =
+    test(
+        2,
         lines(
+            EXPORT_PROPERTY_DEF,
             "/** @polymerBehavior */",
             "const Behavior1 = {",
             "  onAll: function() {},",
@@ -3576,18 +3606,43 @@ public void testExportAllOnlyExportsEachMethodOnce() {
             "  behaviors: [Behavior1, Behavior2],",
             "  onAll() {},",
             "  onElement: function() {},",
-            "});");
-
-    String newExterns =
+            "});"),
         lines(
-            EXTERNS,
-            "/** @interface */ var PolymerTestElementElementInterface=function(){};",
-            "PolymerTestElementElementInterface.prototype.onAll;",
-            "PolymerTestElementElementInterface.prototype.onBehavior1;",
-            "PolymerTestElementElementInterface.prototype.onBehavior2;",
-            "PolymerTestElementElementInterface.prototype.onElement");
-
-    testExternChanges(EXTERNS, js, newExterns);
+            EXPORT_PROPERTY_DEF,
+            "/** @nocollapse @polymerBehavior */",
+            "const Behavior1 = {",
+            "  /** @suppress {checkTypes,globalThis,visibility} */ onAll: function() {},",
+            "  /** @suppress {checkTypes,globalThis,visibility} */ onBehavior1: function() {}",
+            "};",
+            "/** @nocollapse @polymerBehavior */",
+            "const Behavior2 = {",
+            "  /** @suppress {checkTypes,globalThis,visibility} */ onAll: function() {},",
+            "  /** @suppress {checkTypes,globalThis,visibility} */ onBehavior2: function() {}",
+            "};",
+            "/**",
+            " * @constructor",
+            " * @extends {PolymerElement}",
+            " * @implements {PolymerTestElementElementInterface}",
+            " */",
+            "var TestElementElement = function() {};",
+            "/** @suppress {unusedPrivateMembers} */",
+            "TestElementElement.prototype.onBehavior1 = function() {};",
+            "/** @suppress {unusedPrivateMembers} */",
+            "TestElementElement.prototype.onBehavior2 = function() {};",
+            "Polymer(/** @lends {TestElementElement.prototype} */ {",
+            "  is: \"test-element\",",
+            "  behaviors: [Behavior1, Behavior2],",
+            "  /** @this {TestElementElement} */ onAll() {},",
+            "  /** @this {TestElementElement} */ onElement: function() {}",
+            "});",
+            "goog.exportProperty(TestElementElement.prototype, \"onElement\",",
+            "    TestElementElement.prototype.onElement);",
+            "goog.exportProperty(TestElementElement.prototype, \"onBehavior2\",",
+            "    TestElementElement.prototype.onBehavior2);",
+            "goog.exportProperty(TestElementElement.prototype, \"onBehavior1\",",
+            "    TestElementElement.prototype.onBehavior1);",
+            "goog.exportProperty(TestElementElement.prototype, \"onAll\",",
+            "    TestElementElement.prototype.onAll);"));
   }
 
   @Override