Add goog.reflect.objectProperty as a first class primitive with the s…

…ame functionality as JSCompiler_renameProperty.

Rolling forward with a slight change to allow JSCompiler_renameProperty to be called with only one argument, because there are projects currently doing that.

Use an optimization to replace property renaming function calls with the internal JSCompiler_renameProperty method name, in ClosureOptimizePrimitives.  This lets optimizations passes check a single, simple name rather than a namespaced function name which could be heavily optimized.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=122674640

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

@@ -455,6 +455,57 @@ public void visit(NodeTraversal t, Node n, Node parent) {
           maybeMarkCandidate(propNode, jstype);
           break;
         }
+        case Token.CALL: {
+          Node target = n.getFirstChild();
+          if (!target.isName()) {
+            break;
+          }
+
+          String renameFunctionName = target.getOriginalName();
+          if (renameFunctionName == null) {
+            renameFunctionName = target.getString();
+          }
+          if (renameFunctionName == null
+              || !compiler.getCodingConvention().isPropertyRenameFunction(renameFunctionName)) {
+            break;
+          }
+
+          if (n.getChildCount() != 2 && n.getChildCount() != 3) {
+            compiler.report(
+                JSError.make(
+                    n,
+                    DisambiguateProperties.Warnings.INVALID_RENAME_FUNCTION,
+                    renameFunctionName,
+                    " Must be called with 1 or 2 arguments."));
+            break;
+          }
+
+          Node propName = n.getSecondChild();
+          if (!propName.isString()) {
+            compiler.report(
+                JSError.make(
+                    n,
+                    DisambiguateProperties.Warnings.INVALID_RENAME_FUNCTION,
+                    renameFunctionName,
+                    " The first argument must be a string literal."));
+            break;
+          }
+
+          if (propName.getString().contains(".")) {
+            compiler.report(
+                JSError.make(
+                    n,
+                    DisambiguateProperties.Warnings.INVALID_RENAME_FUNCTION,
+                    renameFunctionName,
+                    " The first argument must not be a property path."));
+            break;
+          }
+
+          JSType jstype = getJSType(n.getChildAtIndex(2));
+
+          maybeMarkCandidate(propName, jstype);
+          break;
+        }
         case Token.OBJECTLIT:
           // The children of an OBJECTLIT node are keys, where the values
           // are the children of the keys.
@@ -542,14 +593,17 @@ private Property getProperty(String name) {
    * present.
    */
   private JSType getJSType(Node n) {
+    if (n == null) {
+      return compiler.getTypeRegistry().getNativeType(JSTypeNative.UNKNOWN_TYPE);
+    }
+
     JSType jsType = n.getJSType();
     if (jsType == null) {
       // TODO(user): This branch indicates a compiler bug, not worthy of
       // halting the compilation but we should log this and analyze to track
       // down why it happens. This is not critical and will be resolved over
       // time as the type checker is extended.
-      return compiler.getTypeRegistry().getNativeType(
-          JSTypeNative.UNKNOWN_TYPE);
+      return compiler.getTypeRegistry().getNativeType(JSTypeNative.UNKNOWN_TYPE);
     } else {
       return jsType;
     }

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

@@ -134,12 +134,13 @@ public void visit(NodeTraversal t, Node n, Node parent) {
          break;
        }
 
-       case Token.CALL:
-         // Look for properties referenced through "JSCompiler_propertyRename".
-         Node target = n.getFirstChild();
-         if (n.hasMoreThanOneChild()
-             && target.isName()
-             && target.getString().equals(NodeUtil.JSC_PROPERTY_NAME_FN)) {
+      case Token.CALL:
+        // Look for properties referenced through a property rename function.
+        Node target = n.getFirstChild();
+        if (n.hasMoreThanOneChild()
+            && compiler
+                .getCodingConvention()
+                .isPropertyRenameFunction(target.getOriginalQualifiedName())) {
            Node propName = target.getNext();
            if (propName.isString()) {
              used.add(propName.getString());

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

@@ -355,6 +355,11 @@ public boolean isPropertyTestFunction(Node call) {
         super.isPropertyTestFunction(call);
   }
 
+  @Override
+  public boolean isPropertyRenameFunction(String name) {
+    return super.isPropertyRenameFunction(name) || "goog.reflect.objectProperty".equals(name);
+  }
+
   @Override
   public boolean isFunctionCallThatAlwaysThrows(Node n) {
     return CodingConventions.defaultIsFunctionCallThatAlwaysThrows(

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

@@ -22,9 +22,14 @@
 import com.google.javascript.rhino.Token;
 
 /**
- * <p>Compiler pass that converts all calls to:
+ * Compiler pass that converts primitive calls:
+ *
+ * Converts:
  *   goog.object.create(key1, val1, key2, val2, ...) where all of the keys
- *   are literals into object literals.</p>
+ *   are literals into object literals.
+ *
+ *   goog.reflect.objectProperty(propName, object) to
+ *   JSCompiler_renameProperty
  *
  * @author agrieve@google.com (Andrew Grieve)
  */
@@ -34,15 +39,19 @@ final class ClosureOptimizePrimitives implements CompilerPass {
   private final AbstractCompiler compiler;
 
   /**
-   * Identifies all calls to goog.object.create.
+   * Identifies all calls to closure primitive functions
    */
-  private class FindObjectCreateCalls extends AbstractPostOrderCallback {
+  private class FindPrimitives extends AbstractPostOrderCallback {
 
     @Override
     public void visit(NodeTraversal t, Node n, Node parent) {
       if (n.isCall()) {
         Node fn = n.getFirstChild();
-        if (fn.matchesQualifiedName("goog$object$create")
+        if (compiler
+            .getCodingConvention()
+            .isPropertyRenameFunction(fn.getOriginalQualifiedName())) {
+          processRenamePropertyCall(n);
+        } else if (fn.matchesQualifiedName("goog$object$create")
             || fn.matchesQualifiedName("goog.object.create")) {
           processObjectCreateCall(n);
         } else if (fn.matchesQualifiedName("goog$object$createSet")
@@ -62,7 +71,7 @@ public void visit(NodeTraversal t, Node n, Node parent) {
 
   @Override
   public void process(Node externs, Node root) {
-    FindObjectCreateCalls pass = new FindObjectCreateCalls();
+    FindPrimitives pass = new FindPrimitives();
     NodeTraversal.traverseEs6(compiler, root, pass);
   }
 
@@ -95,6 +104,24 @@ private void processObjectCreateCall(Node callNode) {
     }
   }
 
+  /**
+   * Converts all of the given call nodes to object literals that are safe to
+   * do so.
+   */
+  private void processRenamePropertyCall(Node callNode) {
+    Node nameNode = callNode.getFirstChild();
+    if (nameNode.matchesQualifiedName(NodeUtil.JSC_PROPERTY_NAME_FN)) {
+      return;
+    }
+
+    Node newTarget = IR.name(NodeUtil.JSC_PROPERTY_NAME_FN).copyInformationFrom(nameNode);
+    newTarget.setOriginalName(nameNode.getOriginalQualifiedName());
+
+    callNode.replaceChild(nameNode, newTarget);
+    callNode.putBooleanProp(Node.FREE_CALL, true);
+    compiler.reportCodeChange();
+  }
+
   /**
    * Returns whether the given call to goog.object.create can be converted to an
    * object literal.

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

@@ -372,6 +372,11 @@ public Cache(Node cacheObj, Node key, Node valueFn, Node keyFn) {
    */
   public boolean isPrototypeAlias(Node getProp);
 
+  /**
+   * Whether this CALL function is returning the string name for a property, but allows renaming.
+   */
+  public boolean isPropertyRenameFunction(String name);
+
   /**
    * Checks if the given method performs a object literal cast, and if it does,
    * returns information on the cast. By default, always returns null. Meant

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

@@ -289,6 +289,11 @@ public boolean isPropertyTestFunction(Node call) {
       return nextConvention.isPropertyTestFunction(call);
     }
 
+    @Override
+    public boolean isPropertyRenameFunction(String name) {
+      return nextConvention.isPropertyRenameFunction(name);
+    }
+
     @Override
     public boolean isPrototypeAlias(Node getProp) {
       return false;
@@ -511,6 +516,11 @@ public boolean isPropertyTestFunction(Node call) {
       return "Array.isArray".equals(call.getFirstChild().getQualifiedName());
     }
 
+    @Override
+    public boolean isPropertyRenameFunction(String name) {
+      return NodeUtil.JSC_PROPERTY_NAME_FN.equals(name);
+    }
+
     @Override
     public boolean isPrototypeAlias(Node getProp) {
       return false;

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

@@ -95,12 +95,16 @@ static class Warnings {
     static final DiagnosticType INVALIDATION = DiagnosticType.disabled(
         "JSC_INVALIDATION",
         "Property disambiguator skipping all instances of property {0} "
-        + "because of type {1} node {2}. {3}");
+            + "because of type {1} node {2}. {3}");
 
     static final DiagnosticType INVALIDATION_ON_TYPE = DiagnosticType.disabled(
         "JSC_INVALIDATION_TYPE",
-        "Property disambiguator skipping instances of property {0} "
-        + "on type {1}. {2}");
+        "Property disambiguator skipping instances of property {0} on type {1}. {2}");
+
+    // TODO(tbreisacher): Check this in a separate pass, so that users get the error even if
+    // optimizations are not running.
+    static final DiagnosticType INVALID_RENAME_FUNCTION =
+        DiagnosticType.error("JSC_INVALID_RENAME_FUNCTION", "{0} call is invalid: {1}");
   }
 
   private final AbstractCompiler compiler;
@@ -461,6 +465,8 @@ public void visit(NodeTraversal t, Node n, Node parent) {
         handleGetProp(t, n);
       } else if (n.isObjectLit()) {
         handleObjectLit(t, n);
+      } else if (n.isCall()) {
+        handleCall(t, n);
       }
     }
 
@@ -521,6 +527,84 @@ private void handleObjectLit(NodeTraversal t, Node n) {
       }
     }
 
+    private void handleCall(NodeTraversal t, Node call) {
+      Node target = call.getFirstChild();
+      if (!target.isName()) {
+        return;
+      }
+
+      String renameFunctionName = target.getOriginalQualifiedName();
+      if (renameFunctionName == null
+          || !compiler.getCodingConvention().isPropertyRenameFunction(renameFunctionName)) {
+        return;
+      }
+
+      if (call.getChildCount() != 2 && call.getChildCount() != 3) {
+        compiler.report(
+            JSError.make(
+                call,
+                Warnings.INVALID_RENAME_FUNCTION,
+                renameFunctionName,
+                " Must be called with 1 or 2 arguments"));
+        return;
+      }
+
+      if (!call.getSecondChild().isString()) {
+        compiler.report(
+            JSError.make(
+                call,
+                Warnings.INVALID_RENAME_FUNCTION,
+                renameFunctionName,
+                " The first argument must be a string literal."));
+        return;
+      }
+
+      String propName = call.getSecondChild().getString();
+
+      if (propName.contains(".")) {
+        compiler.report(
+            JSError.make(
+                call,
+                Warnings.INVALID_RENAME_FUNCTION,
+                renameFunctionName,
+                " The first argument must not be a property path."));
+        return;
+      }
+
+      Node obj = call.getChildAtIndex(2);
+      JSType type = getType(obj);
+      Property prop = getProperty(propName);
+      if (!prop.scheduleRenaming(call.getSecondChild(), processProperty(t, prop, type, null))
+          && propertiesToErrorFor.containsKey(propName)) {
+        String suggestion = "";
+        if (type.isAllType() || type.isUnknownType()) {
+          if (obj.isThis()) {
+            suggestion = "The \"this\" object is unknown in the function, consider using @this";
+          } else {
+            String qName = obj.getQualifiedName();
+            suggestion = "Consider casting " + qName + " if you know its type.";
+          }
+        } else {
+          List<String> errors = new ArrayList<>();
+          printErrorLocations(errors, type);
+          if (!errors.isEmpty()) {
+            suggestion = "Consider fixing errors for the following types:\n";
+            suggestion += Joiner.on("\n").join(errors);
+          }
+        }
+
+        compiler.report(
+            JSError.make(
+                call,
+                propertiesToErrorFor.get(propName),
+                Warnings.INVALIDATION,
+                propName,
+                String.valueOf(type),
+                renameFunctionName,
+                suggestion));
+      }
+    }
+
     private void printErrorLocations(List<String> errors, JSType t) {
       if (!t.isObject() || t.isAllType()) {
         return;
@@ -680,7 +764,7 @@ private void addInvalidatingType(JSType type) {
   }
 
   private JSType getType(Node node) {
-    if (node.getJSType() == null) {
+    if (node == null || node.getJSType() == null) {
       return registry.getNativeType(JSTypeNative.UNKNOWN_TYPE);
     }
     return node.getJSType();

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

@@ -369,7 +369,7 @@ private boolean isCandidateFunction(Function fn) {
     }
 
     // Don't inline this special function
-    if (RenameProperties.RENAME_PROPERTY_FUNCTION_NAME.equals(fnName)) {
+    if (compiler.getCodingConvention().isPropertyRenameFunction(fnName)) {
       return false;
     }
 

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

@@ -140,7 +140,9 @@ private boolean isVarInlineForbidden(Var var) {
       return var.isGlobal()
           || var.isExtern()
           || compiler.getCodingConvention().isExported(var.name)
-          || RenameProperties.RENAME_PROPERTY_FUNCTION_NAME.equals(var.name)
+          || compiler
+              .getCodingConvention()
+              .isPropertyRenameFunction(var.nameNode.getQualifiedName())
           || staleVars.contains(var);
     }
 

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

@@ -345,10 +345,12 @@ private boolean isVarInlineForbidden(Var var) {
       // 2) A reference to the variable has been inlined. We're downstream
       //    of the mechanism that creates variable references, so we don't
       //    have a good way to update the reference. Just punt on it.
-      // 3) Don't inline the special RENAME_PROPERTY_FUNCTION_NAME
+      // 3) Don't inline the special property rename functions.
       return var.isExtern()
           || compiler.getCodingConvention().isExported(var.name)
-          || RenameProperties.RENAME_PROPERTY_FUNCTION_NAME.equals(var.name)
+          || compiler
+              .getCodingConvention()
+              .isPropertyRenameFunction(var.nameNode.getOriginalQualifiedName())
           || staleVars.contains(var);
     }
 

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

@@ -147,12 +147,13 @@ public void visit(NodeTraversal t, Node n, Node parent) {
          break;
        }
 
-       case Token.CALL:
-         // Look for properties referenced through "JSCompiler_propertyRename".
-         Node target = n.getFirstChild();
-         if (n.hasMoreThanOneChild()
-             && target.isName()
-             && target.getString().equals(NodeUtil.JSC_PROPERTY_NAME_FN)) {
+      case Token.CALL:
+        // Look for properties referenced through the property rename functions.
+        Node target = n.getFirstChild();
+        if (n.hasMoreThanOneChild()
+            && compiler
+                .getCodingConvention()
+                .isPropertyRenameFunction(target.getOriginalQualifiedName())) {
            Node propName = target.getNext();
            if (propName.isString()) {
              used.add(propName.getString());