Automated g4 rollback of changelist 113913331.

*** Reason for rollback *** Losing generated types for some enums *** Original change description *** Miscs improvements to TypedCodeGenerator Addresses several crashes and failures mentioned in #1369. Closes #1490 Fixes #1369 Patch from Michael Zhou *** ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=114149048
google · Feb 8, 2016 · 101e0a4 · 101e0a4
1 parent 19f3a89
commit 101e0a4
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 132 deletions.
diff --git a/src/com/google/javascript/jscomp/TypedCodeGenerator.java b/src/com/google/javascript/jscomp/TypedCodeGenerator.java
@@ -44,25 +44,16 @@ class TypedCodeGenerator extends CodeGenerator {
 
   @Override
   void add(Node n, Context context) {
-    if (n.isCast()) {
-      add(getTypeAnnotation(n));
-    }
-
     Node parent = n.getParent();
     if (parent != null
         && (parent.isBlock()
             || parent.isScript())) {
       if (n.isFunction()) {
         add(getFunctionAnnotation(n));
-      } else if (n.isExprResult()) {
-        Node firstChild = n.getFirstChild();
-        if (firstChild.isAssign()
-            && firstChild.getLastChild().isFunction()) {
-          Node rhs = firstChild.getLastChild();
-          add(getTypeAnnotation(rhs));
-        } else {
-          add(getTypeAnnotation(firstChild));
-        }
+      } else if (n.isExprResult()
+          && n.getFirstChild().isAssign()) {
+        Node rhs = n.getFirstChild().getLastChild();
+        add(getTypeAnnotation(rhs));
       } else if (n.isVar()
           && n.getFirstFirstChild() != null) {
         add(getTypeAnnotation(n.getFirstFirstChild()));
@@ -94,10 +85,7 @@ private String getTypeAnnotation(Node node) {
         && !type.isFunctionPrototypeType()) {
       return "/** @type {" + node.getJSType().toAnnotationString() + "} */\n";
     } else {
-      // TODO(moz): Currently we use JSDocInfoPrinter as a fallback， which means
-      // we might miss things like @const and @private. We should be able to tell
-      // JSDocInfoPrinter to only print things that are not covered by JSType.
-      return jsdoc == null ? "" : JSDocInfoPrinter.print(jsdoc) + "\n";
+      return "";
     }
   }
 
@@ -122,30 +110,28 @@ private String getFunctionAnnotation(Node fnNode) {
     StringBuilder sb = new StringBuilder("/**\n");
 
 
-    Node paramNode = null;
     // We need to use the child nodes of the function as the nodes for the
     // parameters of the function type do not have the real parameter names.
     // FUNCTION
     //   NAME
     //   LP
     //     NAME param1
     //     NAME param2
-    if (fnNode != null && fnNode.isFunction()) {
-      paramNode = NodeUtil.getFunctionParameters(fnNode).getFirstChild();
-    }
-
-    // Param types
-    int i = 0;
-    for (Node n : funType.getParameters()) {
-      sb.append(" * ");
-      appendAnnotation(sb, "param", getParameterNodeJSDocType(n));
-      sb.append(" ")
-          .append(paramNode == null ? "p" + i : paramNode.getString())
-          .append("\n");
-      if (paramNode != null) {
+    if (fnNode != null) {
+      Node paramNode = NodeUtil.getFunctionParameters(fnNode).getFirstChild();
+
+      // Param types
+      for (Node n : funType.getParameters()) {
+        // Bail out if the paramNode is not there.
+        if (paramNode == null) {
+          break;
+        }
+        sb.append(" * ");
+        appendAnnotation(sb, "param", getParameterNodeJSDocType(n));
+        sb.append(" ")
+            .append(paramNode.getString())
+            .append("\n");
         paramNode = paramNode.getNext();
-      } else {
-        i++;
       }
     }
 

diff --git a/test/com/google/javascript/jscomp/CodePrinterTest.java b/test/com/google/javascript/jscomp/CodePrinterTest.java
@@ -977,21 +977,19 @@ public void testNonNullTypes() {
   }
 
   public void testTypeAnnotationsTypeDef() {
+    // TODO(johnlenz): It would be nice if there were some way to preserve
+    // typedefs but currently they are resolved into the basic types in the
+    // type registry.
     assertTypeAnnotations(
-        LINE_JOINER.join(
-            "/** @typedef {Array<number>} */ goog.java.Long;",
-            "/** @param {!goog.java.Long} a*/",
-            "function f(a){};"),
-        LINE_JOINER.join(
-            "/**@typedef {Array<number>} */",
-            "goog.java.Long;",
-            "/**",
-            " * @param {(Array<number>|null)} a",
-            " * @return {undefined}",
-            " */",
-            "function f(a) {",
-            "}",
-            ""));
+        "/** @typedef {Array<number>} */ goog.java.Long;\n"
+        + "/** @param {!goog.java.Long} a*/\n"
+        + "function f(a){};\n",
+        "goog.java.Long;\n"
+        + "/**\n"
+        + " * @param {(Array<number>|null)} a\n"
+        + " * @return {undefined}\n"
+        + " */\n"
+        + "function f(a) {\n}\n");
   }
 
   public void testTypeAnnotationsAssign() {
@@ -1070,22 +1068,15 @@ public void testTypeAnnotationsMember() {
   }
 
   public void testTypeAnnotationsMemberStub() {
-    assertTypeAnnotations(
-        LINE_JOINER.join(
-            "/** @interface */ function I(){};",
-            "/** @return {undefined} @param {number} x */ I.prototype.method;"),
-        LINE_JOINER.join(
-            "/**",
-            " * @interface",
-            " */",
-            "function I() {",
-            "}",
-            "/**",
-            " * @param {number} p0",
-            " * @return {undefined}",
-            " */",
-            "I.prototype.method;",
-            ""));
+    // TODO(blickly): Investigate why the method's type isn't preserved.
+    assertTypeAnnotations("/** @interface */ function I(){};"
+        + "/** @return {undefined} @param {number} x */ I.prototype.method;",
+        "/**\n"
+        + " * @interface\n"
+        + " */\n"
+        + "function I() {\n"
+        + "}\n"
+        + "I.prototype.method;\n");
   }
 
   public void testTypeAnnotationsImplements() {
@@ -1191,75 +1182,6 @@ public void testEnumAnnotation2() {
         "/** @type {(Object|{})} */\ngoog.Enum2 = goog.x ? {} : goog.Enum;\n");
   }
 
-  public void testClosureLibraryTypeAnnotationExamples() {
-    assertTypeAnnotations(
-        LINE_JOINER.join(
-            "/** @param {Object} obj */goog.removeUid = function(obj) {};",
-            "/** @param {Object} obj The object to remove the field from. */",
-            "goog.removeHashCode = goog.removeUid;"),
-        LINE_JOINER.join(
-            "/**",
-            " * @param {(Object|null)} obj",
-            " * @return {undefined}",
-            " */",
-            "goog.removeUid = function(obj) {",
-            "};",
-            "/**",
-            " * @param {(Object|null)} p0",
-            " * @return {undefined}",
-            " */",
-            "goog.removeHashCode = goog.removeUid;",
-            ""));
-
-    // TODO(moz): Preserve @const
-    assertTypeAnnotations(
-        LINE_JOINER.join(
-            "/** @const */",
-            "goog.TRUSTED_SITE = true;",
-            "/** @return {number} */",
-            "goog.now = (goog.TRUSTED_SITE && Date.now) || (function() {})"),
-        LINE_JOINER.join(
-            "/** @type {boolean} */",
-            "goog.TRUSTED_SITE = true;",
-            "/**",
-            "@return {number} */",
-            "goog.now = goog.TRUSTED_SITE && Date.now || function() {",
-            "};",
-            ""));
-
-    // TODO(moz): Preserve @private
-    assertTypeAnnotations(
-        LINE_JOINER.join(
-            "/** @private */",
-            "this.tick_ = function() {};",
-            "/** @private {Function} @const */",
-            "this.boundTick_ = goog.bind(this.tick_, this);"),
-        LINE_JOINER.join(
-            "/**",
-            " * @return {undefined}",
-            " */",
-            "this.tick_ = function() {",
-            "};",
-            "/**@const @private @type {Function} */",
-            "this.boundTick_ = goog.bind(this.tick_, this);",
-            ""));
-
-    // Cast similar to vec2.js:
-    assertTypeAnnotations(
-        LINE_JOINER.join(
-            "x = function() {};",
-            "y = /** @type {Function} */ (x)"),
-        LINE_JOINER.join(
-            "/**",
-            " * @return {undefined}",
-            " */",
-            "x = function() {",
-            "};",
-            "y = /** @type {(Function|null)} */",
-            "(x);",
-            ""));
-  }
-
   public void testDeprecatedAnnotationIncludesNewline() {
     String js = LINE_JOINER.join(
         "/**@deprecated See {@link replacementClass} for more details.",