Handle property definition by {cr|Object}.defineProperty() in compiler pass

third_party/closure_compiler/runner/src/com/google/javascript/jscomp/ChromePass.java

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 package com.google.javascript.jscomp;
 
 import com.google.javascript.jscomp.NodeTraversal.AbstractPostOrderCallback;
 import com.google.javascript.rhino.IR;
+import com.google.javascript.rhino.JSDocInfoBuilder;
+import com.google.javascript.rhino.JSTypeExpression;
 import com.google.javascript.rhino.Node;
+import com.google.javascript.rhino.Token;
 
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
 /**
- * Compiler pass for Chrome-specific needs. Right now it allows the compiler to check types with
- * methods defined inside Chrome namespaces.
+ * Compiler pass for Chrome-specific needs. It handles the following Chrome JS features:
+ * <ul>
+ * <li>namespace declaration using {@code cr.define()},
+ * <li>unquoted property declaration using {@code {cr|Object}.defineProperty()}.
+ * </ul>
  *
- * <p>The namespaces in Chrome JS are declared as follows:
- * <pre>
- * cr.define('my.namespace.name', function() {
- *   /** @param {number} arg
- *   function internalStaticMethod(arg) {}
- *
- *   /** @constructor
- *   function InternalClass() {}
- *
- *   InternalClass.prototype = {
- *     /** @param {number} arg
- *     method: function(arg) {
- *       internalStaticMethod(arg);  // let's demonstrate the change of local names after our pass
- *     }
- *   };
- *
- *   return {
- *     externalStaticMethod: internalStaticMethod,
- *     ExternalClass: InternalClass
- *   }
- * });
- * </pre>
- *
- * <p>Outside of cr.define() statement the function can be called like this:
- * {@code my.namespace.name.externalStaticMethod(42);}.
- *
- * <p>We need to check the types of arguments and return values of such functions. However, the
- * function is assigned to its namespace dictionary only at run-time and the original Closure
- * Compiler isn't smart enough to predict behavior in that case. Therefore, we need to modify the
- * AST before any compiler checks. That's how we modify it to tell the compiler what's going on:
- *
- * <pre>
- * var my = my || {};
- * my.namespace = my.namespace || {};
- * my.namespace.name = my.namespace.name || {};
- *
- * cr.define('my.namespace.name', function() {
- *   /** @param {number} arg
- *   my.namespace.name.externalStaticMethod(arg) {}
- *
- *   /** @constructor
- *   my.namespace.name.ExternalClass = function() {};
- *
- *   my.namespace.name.ExternalClass.prototype = {
- *     /** @param {number} arg
- *     method: function(arg) {
- *       my.namespace.name.externalStaticMethod(arg);  // see, it has been changed
- *     }
- *   };
- *
- *   return {
- *     externalStaticMethod: my.namespace.name.externalStaticMethod,
- *     ExternalClass: my.namespace.name.ExternalClass
- *   }
- * });
- * </pre>
+ * <p>For the details, see tests inside ChromePassTest.java.
  */
 public class ChromePass extends AbstractPostOrderCallback implements CompilerPass {
     final AbstractCompiler compiler;
 
     private static final String CR_DEFINE = "cr.define";
 
+    private static final String OBJECT_DEFINE_PROPERTY = "Object.defineProperty";
+
+    private static final String CR_DEFINE_PROPERTY = "cr.defineProperty";
+
     private static final String CR_DEFINE_COMMON_EXPLANATION = "It should be called like this: " +
             "cr.define('name.space', function() '{ ... return {Export: Internal}; }');";
 
     static final DiagnosticType CR_DEFINE_WRONG_NUMBER_OF_ARGUMENTS =
             DiagnosticType.error("JSC_CR_DEFINE_WRONG_NUMBER_OF_ARGUMENTS",
                     "cr.define() should have exactly 2 arguments. " + CR_DEFINE_COMMON_EXPLANATION);
 
     static final DiagnosticType CR_DEFINE_INVALID_FIRST_ARGUMENT =
             DiagnosticType.error("JSC_CR_DEFINE_INVALID_FIRST_ARGUMENT",
                     "Invalid first argument for cr.define(). " + CR_DEFINE_COMMON_EXPLANATION);
 
     static final DiagnosticType CR_DEFINE_INVALID_SECOND_ARGUMENT =
             DiagnosticType.error("JSC_CR_DEFINE_INVALID_SECOND_ARGUMENT",
                     "Invalid second argument for cr.define(). " + CR_DEFINE_COMMON_EXPLANATION);
 
     static final DiagnosticType CR_DEFINE_INVALID_RETURN_IN_FUNCTION =
             DiagnosticType.error("JSC_CR_DEFINE_INVALID_RETURN_IN_SECOND_ARGUMENT",
                     "Function passed as second argument of cr.define() should return the " +
                     "dictionary in its last statement. " + CR_DEFINE_COMMON_EXPLANATION);
 
+    static final DiagnosticType CR_DEFINE_PROPERTY_INVALID_PROPERTY_KIND =
+            DiagnosticType.error("JSC_CR_DEFINE_PROPERTY_INVALID_PROPERTY_KIND",
+                    "Invalid cr.PropertyKind passed to cr.defineProperty(): expected ATTR, " +

[Tyler Breisacher (Chromium), 2014/08/12 15:57:29]

If you define PropertyKind as an @enum then the typechecker should be able to
catch this for you, I think. But no harm in catching it here too I guess.

[Vitaly Pavlenko, 2014/08/12 17:44:17]

Acknowledged.

+                    "BOOL_ATTR or JS, found \"{0}\".");

[Tyler Breisacher (Chromium), 2014/08/12 15:57:29]

nit: Put the space and the '+' on the next line:

...ATTR,"
+ " BOOL_ATTR or...

[Vitaly Pavlenko, 2014/08/12 17:44:17]

How does one select between two possibilities?

+
     public ChromePass(AbstractCompiler compiler) {
         this.compiler = compiler;
     }
 
     @Override
     public void process(Node externs, Node root) {
         NodeTraversal.traverse(compiler, root, this);
     }
 
     @Override
     public void visit(NodeTraversal t, Node node, Node parent) {
         if (node.isCall()) {
             Node callee = node.getFirstChild();
             if (callee.matchesQualifiedName(CR_DEFINE)) {
                 visitNamespaceDefinition(node, parent);
                 compiler.reportCodeChange();
+            } else if (callee.matchesQualifiedName(OBJECT_DEFINE_PROPERTY) ||
+                    callee.matchesQualifiedName(CR_DEFINE_PROPERTY)) {
+                visitPropertyDefinition(node, parent);
+                compiler.reportCodeChange();
             }
         }
     }
 
+    private void visitPropertyDefinition(Node call, Node parent) {
+        Node callee = call.getFirstChild();
+        Node target = call.getChildAtIndex(1);
+        Node property = call.getChildAtIndex(2);
+
+        Node getPropNode = NodeUtil.newQualifiedNameNode(compiler.getCodingConvention(),
+                target.getQualifiedName() + "." + property.getString());
+
+        if (callee.matchesQualifiedName(CR_DEFINE_PROPERTY)) {
+            setJsDocWithType(getPropNode, getTypeByCrPropertyKind(call.getChildAtIndex(3)));
+        } else {
+            setJsDocWithType(getPropNode, new Node(Token.STAR));
+        }
+
+        Node definitionNode = IR.exprResult(getPropNode).srcrefTree(parent);
+
+        parent.getParent().addChildAfter(definitionNode, parent);
+    }
+
+    private Node getTypeByCrPropertyKind(Node propertyKind) {
+        if (propertyKind.matchesQualifiedName("cr.PropertyKind.ATTR")) {
+            return IR.string("string");
+        } else if (propertyKind.matchesQualifiedName("cr.PropertyKind.BOOL_ATTR")) {
+            return IR.string("boolean");
+        } else if (propertyKind.matchesQualifiedName("cr.PropertyKind.JS")) {
+            return new Node(Token.STAR);

[Tyler Breisacher (Chromium), 2014/08/12 16:32:49]

I was discussing this with dimvar@ a little bit and he pointed out that this
should probably be @type {?} rather than @type {*} otherwise you'll end up
having to add a zillion casts elsewhere.

[Vitaly Pavlenko, 2014/08/12 17:44:17]

Done.

+        } else {
+            compiler.report(JSError.make(propertyKind, CR_DEFINE_PROPERTY_INVALID_PROPERTY_KIND,
+                    propertyKind.getQualifiedName()));
+            return null;
+        }
+    }
+
+    private void setJsDocWithType(Node target, Node type) {
+        JSDocInfoBuilder builder = new JSDocInfoBuilder(false);
+        builder.recordType(new JSTypeExpression(type, ""));
+        target.setJSDocInfo(builder.build(target));
+    }
+
     private void visitNamespaceDefinition(Node crDefineCallNode, Node parent) {
         if (crDefineCallNode.getChildCount() != 3) {
             compiler.report(JSError.make(crDefineCallNode, CR_DEFINE_WRONG_NUMBER_OF_ARGUMENTS));
         }
 
         Node namespaceArg = crDefineCallNode.getChildAtIndex(1);
         Node function = crDefineCallNode.getChildAtIndex(2);
 
         if (!namespaceArg.isString()) {
             compiler.report(JSError.make(namespaceArg, CR_DEFINE_INVALID_FIRST_ARGUMENT));
@@ skipping 106 matching lines @@
                 //
                 // to
                 //
                 //   /** Some doc */
                 //   my.namespace.name.externalName = function internalName() {};
                 //
                 // by looking up in this.exports for internalName to find the correspondent
                 // externalName.
                 Node functionTree = n.cloneTree();
                 Node exprResult = IR.exprResult(
-                        IR.assign(buildQualifiedName(n.getFirstChild()), functionTree));
-                NodeUtil.setDebugInformation(exprResult, n, n.getFirstChild().getString());
+                            IR.assign(buildQualifiedName(n.getFirstChild()), functionTree)
+                        ).srcrefTree(n);
+
                 if (n.getJSDocInfo() != null) {
                     exprResult.getFirstChild().setJSDocInfo(n.getJSDocInfo());
                     functionTree.removeProp(Node.JSDOC_INFO_PROP);
                 }
                 this.namespaceBlock.replaceChild(n, exprResult);
             } else if (n.isName() && this.exports.containsKey(n.getString()) &&
                     !parent.isFunction()) {
                 if (parent.isVar()) {
                     if (parent.getParent() == this.namespaceBlock) {
                         // It's a top-level exported variable definition.
                         // Change
                         //
                         //   var enum = { 'one': 1, 'two': 2 };
                         //
                         // to
                         //
                         //   my.namespace.name.enum = { 'one': 1, 'two': 2 };
                         Node varContent = n.removeFirstChild();
                         Node exprResult = IR.exprResult(
-                                IR.assign(buildQualifiedName(n), varContent));
-                        NodeUtil.setDebugInformation(exprResult, parent, n.getString());
+                                IR.assign(buildQualifiedName(n), varContent)).srcrefTree(parent);
+
                         if (parent.getJSDocInfo() != null) {
                             exprResult.getFirstChild().setJSDocInfo(parent.getJSDocInfo().clone());
                         }
                         this.namespaceBlock.replaceChild(parent, exprResult);
                     }
                 } else {
                     // It's a local name referencing exported entity. Change to its global name.
                     Node newNode = buildQualifiedName(n);
                     if (n.getJSDocInfo() != null) {
                         newNode.setJSDocInfo(n.getJSDocInfo().clone());
                     }
 
                     // If we alter the name of a called function, then it gets an explicit "this"
                     // value.
                     if (parent.isCall()) {
                         parent.putBooleanProp(Node.FREE_CALL, false);
                     }
 
                     parent.replaceChild(n, newNode);
                 }
             }
         }
 
         private Node buildQualifiedName(Node internalName) {
             String externalName = this.exports.get(internalName.getString());
             return NodeUtil.newQualifiedNameNode(compiler.getCodingConvention(),
-                                                 this.namespaceName + "." + externalName,
-                                                 internalName,
-                                                 internalName.getString());
+                    this.namespaceName + "." + externalName).srcrefTree(internalName);
         }
     }
+
 }