Add --polymer_export_policy flag for PolymerPass, with option for agg…

…ressively preventing renaming/dead code removal of Polymer properties/methods. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=207811101
google · Aug 10, 2018 · b0ecd21 · b0ecd21
1 parent 6b2e4ae
commit b0ecd21
Show file tree

Hide file tree

Showing 15 changed files with 202 additions and 36 deletions.
diff --git a/src/com/google/javascript/jscomp/CommandLineRunner.java b/src/com/google/javascript/jscomp/CommandLineRunner.java
@@ -588,6 +588,13 @@ private static class Flags {
         usage = "Which version of Polymer is being used (1 or 2).")
     private Integer polymerVersion = null;
 
+    @Option(
+        name = "--polymer_export_policy",
+        usage =
+            "How to handle exports/externs for Polymer properties and methods. "
+                + "Values: LEGACY, EXPORT_ALL.")
+    private String polymerExportPolicy = PolymerExportPolicy.LEGACY.name();
+
     @Option(name = "--chrome_pass",
         handler = BooleanOptionHandler.class,
         usage = "Enable Chrome-specific options for handling cr.* functions.",
@@ -1831,6 +1838,13 @@ protected CompilerOptions createOptions() {
     } else {
       options.polymerVersion = flags.polymerVersion;
     }
+    try {
+      options.polymerExportPolicy =
+          PolymerExportPolicy.valueOf(Ascii.toUpperCase(flags.polymerExportPolicy));
+    } catch (IllegalArgumentException ex) {
+      throw new FlagUsageException(
+          "Unknown PolymerExportPolicy `" + flags.polymerExportPolicy + "' specified.");
+    }
 
     options.setChromePass(flags.chromePass);
 

diff --git a/src/com/google/javascript/jscomp/CompilerOptions.java b/src/com/google/javascript/jscomp/CompilerOptions.java
@@ -767,6 +767,9 @@ public void setReplaceMessagesWithChromeI18n(
   @Nullable
   Integer polymerVersion;
 
+  /** How to handle exports/externs for Polymer properties and methods. */
+  PolymerExportPolicy polymerExportPolicy;
+
   /** Processes cr.* functions */
   private boolean chromePass;
 
@@ -1309,6 +1312,7 @@ public CompilerOptions() {
     preserveClosurePrimitives = false;
     angularPass = false;
     polymerVersion = null;
+    polymerExportPolicy = PolymerExportPolicy.LEGACY;
     dartPass = false;
     j2clPassMode = J2clPassMode.AUTO;
     removeAbstractMethods = false;
@@ -3060,6 +3064,7 @@ public String toString() {
             .add("parseJsDocDocumentation", isParseJsDocDocumentation())
             .add("pathEscaper", pathEscaper)
             .add("polymerVersion", polymerVersion)
+            .add("polymerExportPolicy", polymerExportPolicy)
             .add("preferLineBreakAtEndOfFile", preferLineBreakAtEndOfFile)
             .add("preferSingleQuotes", preferSingleQuotes)
             .add("preferStableNames", preferStableNames)

diff --git a/src/com/google/javascript/jscomp/DefaultPassConfig.java b/src/com/google/javascript/jscomp/DefaultPassConfig.java
@@ -3210,6 +3210,7 @@ protected HotSwapCompilerPass create(AbstractCompiler compiler) {
           return new PolymerPass(
               compiler,
               compiler.getOptions().polymerVersion,
+              compiler.getOptions().polymerExportPolicy,
               compiler.getOptions().propertyRenaming == PropertyRenamingPolicy.ALL_UNQUOTED);
         }
 

diff --git a/src/com/google/javascript/jscomp/PolymerBehaviorExtractor.java b/src/com/google/javascript/jscomp/PolymerBehaviorExtractor.java
@@ -267,7 +267,7 @@ static final class BehaviorDefinition {
     final List<MemberDefinition> nonPropertyMembersToCopy;
 
     /**
-     * Whether this Behvaior is declared in the global scope.
+     * Whether this Behavior is declared in the global scope.
      */
     final boolean isGlobalDeclaration;
 

diff --git a/src/com/google/javascript/jscomp/PolymerClassDefinition.java b/src/com/google/javascript/jscomp/PolymerClassDefinition.java
@@ -61,6 +61,9 @@ static enum DefinitionType {
   /** Properties declared in the Polymer "properties" block. */
   final List<MemberDefinition> props;
 
+  /** Methods on the element. */
+  @Nullable final List<MemberDefinition> methods;
+
   /** Flattened list of behavior definitions used by this element. */
   @Nullable final ImmutableList<BehaviorDefinition> behaviors;
 
@@ -76,6 +79,7 @@ static enum DefinitionType {
       MemberDefinition constructor,
       String nativeBaseElement,
       List<MemberDefinition> props,
+      List<MemberDefinition> methods,
       ImmutableList<BehaviorDefinition> behaviors,
       FeatureSet features) {
     this.defType = defType;
@@ -86,6 +90,7 @@ static enum DefinitionType {
     this.constructor = constructor;
     this.nativeBaseElement = nativeBaseElement;
     this.props = props;
+    this.methods = methods;
     this.behaviors = behaviors;
     this.features = features;
   }
@@ -166,6 +171,18 @@ static enum DefinitionType {
       }
     }
 
+    List<MemberDefinition> methods = new ArrayList<>();
+    for (Node keyNode : descriptor.children()) {
+      boolean isFunctionDefinition =
+          keyNode.isMemberFunctionDef()
+              || (keyNode.isStringKey() && keyNode.getFirstChild().isFunction());
+      if (isFunctionDefinition) {
+        methods.add(
+            new MemberDefinition(
+                NodeUtil.getBestJSDocInfo(keyNode), keyNode, keyNode.getFirstChild()));
+      }
+    }
+
     return new PolymerClassDefinition(
         DefinitionType.ObjectLiteral,
         callNode,
@@ -175,6 +192,7 @@ static enum DefinitionType {
         new MemberDefinition(ctorInfo, null, constructor),
         nativeBaseElement,
         allProperties,
+        methods,
         behaviors,
         newFeatures);
   }
@@ -240,6 +258,15 @@ static PolymerClassDefinition extractFromClassNode(
         PolymerPassStaticUtils.extractProperties(
             propertiesDescriptor, DefinitionType.ES6Class, compiler);
 
+    List<MemberDefinition> methods = new ArrayList<>();
+    for (Node keyNode : NodeUtil.getClassMembers(classNode).children()) {
+      if (!keyNode.isMemberFunctionDef()) {
+        continue;
+      }
+      methods.add(new MemberDefinition(
+                      NodeUtil.getBestJSDocInfo(keyNode), keyNode, keyNode.getFirstChild()));
+    }
+
     return new PolymerClassDefinition(
         DefinitionType.ES6Class,
         classNode,
@@ -249,6 +276,7 @@ static PolymerClassDefinition extractFromClassNode(
         new MemberDefinition(ctorInfo, null, constructor),
         null,
         allProperties,
+        methods,
         null,
         null);
   }

diff --git a/src/com/google/javascript/jscomp/PolymerClassRewriter.java b/src/com/google/javascript/jscomp/PolymerClassRewriter.java
@@ -43,6 +43,7 @@ final class PolymerClassRewriter {
   private static final String VIRTUAL_FILE = "<PolymerClassRewriter.java>";
   private final AbstractCompiler compiler;
   private final int polymerVersion;
+  private final PolymerExportPolicy polymerExportPolicy;
   private final boolean propertyRenamingEnabled;
 
   @VisibleForTesting
@@ -54,10 +55,12 @@ final class PolymerClassRewriter {
       AbstractCompiler compiler,
       Node polymerElementExterns,
       int polymerVersion,
+      PolymerExportPolicy polymerExportPolicy,
       boolean propertyRenamingEnabled) {
     this.compiler = compiler;
     this.polymerElementExterns = polymerElementExterns;
     this.polymerVersion = polymerVersion;
+    this.polymerExportPolicy = polymerExportPolicy;
     this.propertyRenamingEnabled = propertyRenamingEnabled;
   }
 
@@ -118,7 +121,7 @@ void rewritePolymerCall(
       block.addChildToBack(var);
     }
 
-    appendPropertiesToBlock(cls, block, cls.target.getQualifiedName() + ".prototype.");
+    appendPropertiesToBlock(cls.props, block, cls.target.getQualifiedName() + ".prototype.");
     appendBehaviorMembersToBlock(cls, block);
     ImmutableList<MemberDefinition> readOnlyProps = parseReadOnlyProperties(cls, block);
     ImmutableList<MemberDefinition> attributeReflectedProps =
@@ -206,14 +209,16 @@ void rewritePolymerClassDeclaration(
     // For simplicity add everything into a block, before adding it to the AST.
     Node block = IR.block();
 
-    appendPropertiesToBlock(cls, block, cls.target.getQualifiedName() + ".prototype.");
+    appendPropertiesToBlock(cls.props, block, cls.target.getQualifiedName() + ".prototype.");
     ImmutableList<MemberDefinition> readOnlyProps = parseReadOnlyProperties(cls, block);
     ImmutableList<MemberDefinition> attributeReflectedProps =
         parseAttributeReflectedProperties(cls);
     addInterfaceExterns(cls, readOnlyProps, attributeReflectedProps);
 
     // If an external interface is required, mark the class as implementing it
-    if (!readOnlyProps.isEmpty() || !attributeReflectedProps.isEmpty()) {
+    if (polymerExportPolicy == PolymerExportPolicy.EXPORT_ALL
+        || !readOnlyProps.isEmpty()
+        || !attributeReflectedProps.isEmpty()) {
       Node jsDocInfoNode = NodeUtil.getBestJSDocInfoNode(clazz);
       JSDocInfoBuilder classInfo = JSDocInfoBuilder.maybeCopyFrom(jsDocInfoNode.getJSDocInfo());
       String interfaceName = getInterfaceName(cls);
@@ -384,11 +389,11 @@ private JSDocInfoBuilder getConstructorDoc(final PolymerClassDefinition cls) {
   }
 
   /**
-   * Appends all properties in the ClassDefinition to the prototype of the custom element.
+   * Appends all of the given properties to the given block.
    */
   private void appendPropertiesToBlock(
-      final PolymerClassDefinition cls, Node block, String basePath) {
-    for (MemberDefinition prop : cls.props) {
+      List<MemberDefinition> props, Node block, String basePath) {
+    for (MemberDefinition prop : props) {
       Node propertyNode = IR.exprResult(
           NodeUtil.newQName(compiler, basePath + prop.name.getString()));
 
@@ -411,6 +416,21 @@ private void appendPropertiesToBlock(
     }
   }
 
+  /**
+   * Appends all of the given methods to the given block.
+   */
+  private void appendMethodsToBlock(
+      final List<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) {
@@ -534,42 +554,36 @@ private void addInterfaceExterns(
     info.recordInterface();
     varNode.setJSDocInfo(info.build());
     block.addChildToBack(varNode);
-
-    if (polymerVersion == 1) {
+    String interfaceBasePath = interfaceName + ".prototype.";
+
+    if (polymerExportPolicy == PolymerExportPolicy.EXPORT_ALL) {
+      appendPropertiesToBlock(cls.props, block, interfaceBasePath);
+      appendMethodsToBlock(cls.methods, block, interfaceBasePath);
+      if (cls.behaviors != null) {
+        for (BehaviorDefinition behavior : cls.behaviors) {
+          appendMethodsToBlock(behavior.functionsToCopy, block, interfaceBasePath);
+        }
+      }
+    } else if (polymerVersion == 1) {
       // For Polymer 1, all declared properties are non-renameable
-      appendPropertiesToBlock(cls, block, interfaceName + ".prototype.");
+      appendPropertiesToBlock(cls.props, block, interfaceBasePath);
     } else {
+      // For Polymer 2, only read-only properties and reflectToAttribute properties are
+      // non-renameable. Other properties follow the ALL_UNQUOTED renaming rules.
       List<MemberDefinition> interfaceProperties = new ArrayList<>();
       interfaceProperties.addAll(readOnlyProps);
       if (attributeReflectedProps != null) {
         interfaceProperties.addAll(attributeReflectedProps);
       }
-
-      // For Polymer 2, only read-only properties and reflectToAttribute properties are
-      // non-renameable. Other properties follow the ALL_UNQUOTED renaming rules.
-      PolymerClassDefinition tmpDef =
-          new PolymerClassDefinition(
-              cls.defType,
-              cls.definition,
-              cls.target,
-              cls.descriptor,
-              null,
-              null,
-              null,
-              interfaceProperties,
-              null,
-              null);
-
-      // disallow renaming of readonly properties
-      appendPropertiesToBlock(tmpDef, block, interfaceName + ".prototype.");
+      appendPropertiesToBlock(interfaceProperties, block, interfaceBasePath);
     }
 
     for (MemberDefinition prop : readOnlyProps) {
       // Add all _set* functions to avoid renaming.
       String propName = prop.name.getString();
       String setterName = "_set" + propName.substring(0, 1).toUpperCase() + propName.substring(1);
       Node setterExprNode = IR.exprResult(
-          NodeUtil.newQName(compiler, interfaceName + ".prototype." + setterName));
+          NodeUtil.newQName(compiler, interfaceBasePath + setterName));
 
       JSDocInfoBuilder setterInfo = new JSDocInfoBuilder(true);
       JSTypeExpression propType = PolymerPassStaticUtils.getTypeFromProperty(prop, compiler);

diff --git a/src/com/google/javascript/jscomp/PolymerExportPolicy.java b/src/com/google/javascript/jscomp/PolymerExportPolicy.java
@@ -0,0 +1,42 @@
+/*
+ * Copyright 2018 The Closure Compiler Authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.javascript.jscomp;
+
+/**
+ * How to handle exports/externs for Polymer properties and methods.
+ */
+public enum PolymerExportPolicy {
+  /**
+   * If --polymer_version=1, add all Polymer properties (but not methods) to the externs.
+   * If --polymer_version=2, add readOnly and reflectToAttribute Polymer properties (but not
+   * methods) to the externs.
+   *
+   * This policy is is not generally safe for use with renaming and unused code optimizations,
+   * unless additional steps are taken (e.g. manual exports, goog.reflect.objectProperty, the
+   * PolymerRenamer post-processor).
+   */
+  LEGACY,
+
+  /**
+   * Add all Polymer properties and methods to the externs.
+   *
+   * Since any of these definitions could be referenced by string in HTML templates, observer
+   * definitions, or computed property definitions, this is a blunt but safe way to allow Polymer
+   * code to be used with renaming and unused code optimizations enabled.
+   */
+  EXPORT_ALL
+}
diff --git a/src/com/google/javascript/jscomp/PolymerPass.java b/src/com/google/javascript/jscomp/PolymerPass.java
@@ -50,6 +50,7 @@ final class PolymerPass extends AbstractPostOrderCallback implements HotSwapComp
   private final AbstractCompiler compiler;
   private final ImmutableMap<String, String> tagNameMap;
   private final int polymerVersion;
+  private final PolymerExportPolicy polymerExportPolicy;
   private final boolean propertyRenamingEnabled;
 
   private Node polymerElementExterns;
@@ -59,7 +60,11 @@ final class PolymerPass extends AbstractPostOrderCallback implements HotSwapComp
   private GlobalNamespace globalNames;
   private boolean warnedPolymer1ExternsMissing = false;
 
-  PolymerPass(AbstractCompiler compiler, Integer polymerVersion, boolean propertyRenamingEnabled) {
+  PolymerPass(
+      AbstractCompiler compiler,
+      Integer polymerVersion,
+      PolymerExportPolicy polymerExportPolicy,
+      boolean propertyRenamingEnabled) {
     checkArgument(
         polymerVersion == null || polymerVersion == 1 || polymerVersion == 2,
         "Invalid Polymer version:",
@@ -68,6 +73,8 @@ final class PolymerPass extends AbstractPostOrderCallback implements HotSwapComp
     tagNameMap = TagNameToType.getMap();
     nativeExternsAdded = new HashSet<>();
     this.polymerVersion = polymerVersion == null ? 1 : polymerVersion;
+    this.polymerExportPolicy =
+        polymerExportPolicy == null ? PolymerExportPolicy.LEGACY : polymerExportPolicy;
     this.propertyRenamingEnabled = propertyRenamingEnabled;
   }
 
@@ -132,7 +139,11 @@ private void rewritePolymer1ClassDefinition(Node node, Node parent, NodeTraversa
       }
       PolymerClassRewriter rewriter =
           new PolymerClassRewriter(
-              compiler, getExtensInsertionRef(), polymerVersion, this.propertyRenamingEnabled);
+              compiler,
+              getExtensInsertionRef(),
+              polymerVersion,
+              polymerExportPolicy,
+              this.propertyRenamingEnabled);
       if (NodeUtil.isNameDeclaration(grandparent) || parent.isAssign()) {
         rewriter.rewritePolymerCall(grandparent, def, traversal.inGlobalScope());
       } else {
@@ -148,7 +159,11 @@ private void rewritePolymer2ClassDefinition(Node node, NodeTraversal traversal)
     if (def != null) {
       PolymerClassRewriter rewriter =
           new PolymerClassRewriter(
-              compiler, getExtensInsertionRef(), polymerVersion, this.propertyRenamingEnabled);
+              compiler,
+              getExtensInsertionRef(),
+              polymerVersion,
+              polymerExportPolicy,
+              this.propertyRenamingEnabled);
       rewriter.rewritePolymerClassDeclaration(node, def, traversal.inGlobalScope());
     }
   }