Handle some cases where function parameter details are lost and

end up being rendered with generic names like "arg0". At this point, the logic in TypeInspector for finding parameters is getting pretty gross and I should, in my infinite spare time, sit down and refactor things.
jleyba · Mar 22, 2019 · def9657 · def9657
1 parent bfd338c
commit def9657
Show file tree

Hide file tree

Showing 6 changed files with 43 additions and 43 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -5,6 +5,8 @@
 -  Fixed a rendering issue with arguments that do not have a doc comment.
 -  Fixed a bug that caused optional parameters in function type expressions to
    be discarded.
+-  Fixed some cases where function parameter details were lost and rendered
+   with generic `argN` names.
 
 ## Changes
 

diff --git a/src/java/com/github/jsdossier/RenderDocumentationTaskSupplier.java b/src/java/com/github/jsdossier/RenderDocumentationTaskSupplier.java
@@ -16,7 +16,6 @@
 
 package com.github.jsdossier;
 
-import static com.github.jsdossier.TypeInspector.fakeNodeForType;
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Strings.isNullOrEmpty;
@@ -414,16 +413,14 @@ private void addMainFunctionInfo(JsType.Builder spec) {
         }
       }
 
-      // TODO: should not be using Node here.
-      Node fakeNode = fakeNodeForType(type);
       FunctionType mainFn =
           checkNotNull(
               type.getType().toMaybeFunctionType(),
               "Expected %s to be a function: %s",
               type.getName(),
               type.getType());
       spec.setMainFunction(
-          typeInspector.getFunctionData(getBasename(type), mainFn, fakeNode, context, docs));
+          typeInspector.getFunctionData(getBasename(type), mainFn, type.getNode(), context, docs));
     }
 
     private void addTypeInheritanceInfo(JsType.Builder spec) {

diff --git a/src/java/com/github/jsdossier/TypeInspector.java b/src/java/com/github/jsdossier/TypeInspector.java
@@ -56,11 +56,9 @@
 import com.google.common.html.types.SafeUrl;
 import com.google.common.html.types.SafeUrlProto;
 import com.google.common.html.types.SafeUrls;
-import com.google.javascript.rhino.IR;
 import com.google.javascript.rhino.JSDocInfo;
 import com.google.javascript.rhino.JSTypeExpression;
 import com.google.javascript.rhino.Node;
-import com.google.javascript.rhino.StaticSourceFile;
 import com.google.javascript.rhino.Token;
 import com.google.javascript.rhino.jstype.FunctionType;
 import com.google.javascript.rhino.jstype.JSType;
@@ -825,11 +823,24 @@ private Function getFunctionData(
     return builder.build();
   }
 
+  // TODO(jleyba): this method is crazy and full of a bunch hacks. Need to refactor.
   private Iterable<Function.Detail> getParameters(
       FunctionType type, Node node, PropertyDocs docs, Iterable<InstanceProperty> overrides) {
     NominalType contextType = docs.getContextType();
     Collection<Parameter> parameters = docs.getJsDoc().getParameters();
 
+    if (parameters.isEmpty()
+        && type.isConstructor()
+        && !type.getTypeOfThis().isUnknownType() // Short-circuit for the global Function type.
+        && type.getJSDocInfo() != null
+        && type.getJSDocInfo().getParameterCount() > 0) {
+      parameters = extractParameters(type.getJSDocInfo());
+    }
+
+    if (node.isAssign() && node.getFirstChild() != null && node.getSecondChild() != null) {
+      node = node.getSecondChild();
+    }
+
     if (node.isClass()) {
       JSDocInfo info = findClassConstructorDocs(node);
       if (info != null) {
@@ -1347,40 +1358,6 @@ private void resolveNames(Node node) {
     }
   }
 
-  static Node fakeNodeForType(final NominalType type) {
-    Node fakeNode = IR.script();
-    fakeNode.getSourceFileName();
-    fakeNode.setStaticSourceFile(
-        new StaticSourceFile() {
-          @Override
-          public String getName() {
-            return type.getSourceFile().toString();
-          }
-
-          @Override
-          public SourceKind getKind() {
-            return SourceKind.STRONG;
-          }
-
-          @Override
-          public int getLineOffset(int lineNumber) {
-            return 0;
-          }
-
-          @Override
-          public int getLineOfOffset(int offset) {
-            return 0;
-          }
-
-          @Override
-          public int getColumnOfOffset(int offset) {
-            return 0;
-          }
-        });
-    fakeNode.setLineno(type.getSourcePosition().getLine());
-    return fakeNode;
-  }
-
   private static JSType stripTemplateTypeInformation(JSType type) {
     if (type.isTemplatizedType()) {
       return ((TemplatizedType) type).getReferencedType();

diff --git a/test/java/com/github/jsdossier/resources/golden/Calculator.json b/test/java/com/github/jsdossier/resources/golden/Calculator.json
@@ -245,7 +245,7 @@
           1,
           [
             [
-              "arg0",
+              "radix",
               [
                 null,
                 null,
@@ -257,6 +257,14 @@
                   1,
                   []
                 ]
+              ],
+              [
+                [
+                  [
+                    null,
+                    "\u003cp\u003ethe radix to use.\u003c/p\u003e\n"
+                  ]
+                ]
               ]
             ]
           ],

diff --git a/test/java/com/github/jsdossier/resources/golden/sample.inheritance.StringClass.json b/test/java/com/github/jsdossier/resources/golden/sample.inheritance.StringClass.json
@@ -144,7 +144,7 @@
           1,
           [
             [
-              "arg0",
+              "value",
               [
                 null,
                 null,
@@ -156,6 +156,14 @@
                   1,
                   []
                 ]
+              ],
+              [
+                [
+                  [
+                    null,
+                    "\u003cp\u003eThe initial value.\u003c/p\u003e\n"
+                  ]
+                ]
               ]
             ]
           ],

diff --git a/test/java/com/github/jsdossier/resources/golden/vis.InheritsVis.json b/test/java/com/github/jsdossier/resources/golden/vis.InheritsVis.json
@@ -71,7 +71,7 @@
           1,
           [
             [
-              "arg0",
+              "x",
               [
                 null,
                 null,
@@ -83,6 +83,14 @@
                   1,
                   []
                 ]
+              ],
+              [
+                [
+                  [
+                    null,
+                    "\u003cp\u003eA number.\u003c/p\u003e\n"
+                  ]
+                ]
               ]
             ]
           ],