Add attribute information to skydoc output

RELNOTES: None.
PiperOrigin-RevId: 204147228

6 files changed, 122 insertions, 31 deletions

diff --git a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeDescriptor.java b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeDescriptor.java
index 249ee60669..3dc7a434f4 100644
--- a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeDescriptor.java
+++ b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeDescriptor.java
@@ -21,19 +21,60 @@ import com.google.devtools.build.lib.skylarkinterface.SkylarkPrinter;
  * Fake implementation of {@link Descriptor}.
  */
 public class FakeDescriptor implements Descriptor {
+  private final Type type;
   private final String docString;
+  private final boolean mandatory;
 
-  public FakeDescriptor(String docString) {
+  public FakeDescriptor(Type type, String docString, boolean mandatory) {
+    this.type = type;
     this.docString = docString;
+    this.mandatory = mandatory;
+  }
+
+  public Type getType() {
+    return type;
   }
 
   public String getDocString() {
     return docString;
   }
 
+  public boolean isMandatory() {
+    return mandatory;
+  }
+
   @Override
   public void repr(SkylarkPrinter printer) {}
 
-  // TODO(cparsons): This class should store information about the attribute definition, for
-  // example, the attribute type.
+  /**
+   * Attribute type. For example, an attribute described by attr.label() will be of type LABEL.
+   */
+  public enum Type {
+    INT("Integer"),
+    LABEL("Label"),
+    STRING("String"),
+    STRING_LIST("List of strings"),
+    INT_LIST("List of integers"),
+    LABEL_LIST("List of labels"),
+    BOOLEAN("Boolean"),
+    LICENSE("List of strings"),
+    LABEL_STRING_DICT("Dictionary: Label -> String"),
+    STRING_DICT("Dictionary: String -> String"),
+    STRING_LIST_DICT("Dictionary: String -> List of strings"),
+    OUTPUT("Label"),
+    OUTPUT_LIST("List of labels");
+
+    private final String description;
+
+    Type(String description) {
+      this.description = description;
+    }
+
+    /**
+     * Returns a human-readable string representing this attribute type.
+     */
+    public String getDescription() {
+      return description;
+    }
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeSkylarkAttrApi.java b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeSkylarkAttrApi.java
index 3dcfaa2d68..f0ea8a6701 100644
--- a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeSkylarkAttrApi.java
+++ b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeSkylarkAttrApi.java
@@ -21,6 +21,7 @@ import com.google.devtools.build.lib.syntax.EvalException;
 import com.google.devtools.build.lib.syntax.FuncallExpression;
 import com.google.devtools.build.lib.syntax.SkylarkDict;
 import com.google.devtools.build.lib.syntax.SkylarkList;
+import com.google.devtools.build.skydoc.fakebuildapi.FakeDescriptor.Type;
 
 /**
  * Fake implementation of {@link SkylarkAttrApi}.
@@ -30,13 +31,13 @@ public class FakeSkylarkAttrApi implements SkylarkAttrApi {
   @Override
   public Descriptor intAttribute(Integer defaultInt, String doc, Boolean mandatory,
       SkylarkList<?> values, FuncallExpression ast, Environment env) throws EvalException {
-    return new FakeDescriptor(doc);
+    return new FakeDescriptor(Type.INT, doc, mandatory);
   }
 
   @Override
   public Descriptor stringAttribute(String defaultString, String doc, Boolean mandatory,
       SkylarkList<?> values, FuncallExpression ast, Environment env) throws EvalException {
-    return new FakeDescriptor(doc);
+    return new FakeDescriptor(Type.STRING, doc, mandatory);
   }
 
   @Override
@@ -44,21 +45,21 @@ public class FakeSkylarkAttrApi implements SkylarkAttrApi {
       Object allowFiles, Object allowSingleFile, Boolean mandatory, SkylarkList<?> providers,
       Object allowRules, Boolean singleFile, Object cfg, SkylarkList<?> aspects,
       FuncallExpression ast, Environment env) throws EvalException {
-    return new FakeDescriptor(doc);
+    return new FakeDescriptor(Type.LABEL, doc, mandatory);
   }
 
   @Override
   public Descriptor stringListAttribute(Boolean mandatory, Boolean nonEmpty, Boolean allowEmpty,
       SkylarkList<?> defaultList, String doc, FuncallExpression ast, Environment env)
       throws EvalException {
-    return new FakeDescriptor(doc);
+    return new FakeDescriptor(Type.STRING_LIST, doc, mandatory);
   }
 
   @Override
   public Descriptor intListAttribute(Boolean mandatory, Boolean nonEmpty, Boolean allowEmpty,
       SkylarkList<?> defaultList, String doc, FuncallExpression ast, Environment env)
       throws EvalException {
-    return new FakeDescriptor(doc);
+    return new FakeDescriptor(Type.INT_LIST, doc, mandatory);
   }
 
   @Override
@@ -66,7 +67,7 @@ public class FakeSkylarkAttrApi implements SkylarkAttrApi {
       Object allowFiles, Object allowRules, SkylarkList<?> providers, SkylarkList<?> flags,
       Boolean mandatory, Boolean nonEmpty, Object cfg, SkylarkList<?> aspects,
       FuncallExpression ast, Environment env) throws EvalException {
-    return new FakeDescriptor(doc);
+    return new FakeDescriptor(Type.LABEL_LIST, doc, mandatory);
   }
 
   @Override
@@ -74,46 +75,46 @@ public class FakeSkylarkAttrApi implements SkylarkAttrApi {
       String doc, Object allowFiles, Object allowRules, SkylarkList<?> providers,
       SkylarkList<?> flags, Boolean mandatory, Boolean nonEmpty, Object cfg, SkylarkList<?> aspects,
       FuncallExpression ast, Environment env) throws EvalException {
-    return new FakeDescriptor(doc);
+    return new FakeDescriptor(Type.LABEL_STRING_DICT, doc, mandatory);
   }
 
   @Override
   public Descriptor boolAttribute(Boolean defaultO, String doc, Boolean mandatory,
       FuncallExpression ast, Environment env) throws EvalException {
-    return new FakeDescriptor(doc);
+    return new FakeDescriptor(Type.BOOLEAN, doc, mandatory);
   }
 
   @Override
   public Descriptor outputAttribute(Object defaultO, String doc, Boolean mandatory,
       FuncallExpression ast, Environment env) throws EvalException {
-    return new FakeDescriptor(doc);
+    return new FakeDescriptor(Type.OUTPUT, doc, mandatory);
   }
 
   @Override
   public Descriptor outputListAttribute(Boolean allowEmpty, SkylarkList<?> defaultList, String doc,
       Boolean mandatory, Boolean nonEmpty, FuncallExpression ast, Environment env)
       throws EvalException {
-    return new FakeDescriptor(doc);
+    return new FakeDescriptor(Type.OUTPUT_LIST, doc, mandatory);
   }
 
   @Override
   public Descriptor stringDictAttribute(Boolean allowEmpty, SkylarkDict<?, ?> defaultO, String doc,
       Boolean mandatory, Boolean nonEmpty, FuncallExpression ast, Environment env)
       throws EvalException {
-    return new FakeDescriptor(doc);
+    return new FakeDescriptor(Type.STRING_DICT, doc, mandatory);
   }
 
   @Override
   public Descriptor stringListDictAttribute(Boolean allowEmpty, SkylarkDict<?, ?> defaultO,
       String doc, Boolean mandatory, Boolean nonEmpty, FuncallExpression ast, Environment env)
       throws EvalException {
-    return new FakeDescriptor(doc);
+    return new FakeDescriptor(Type.STRING_LIST_DICT, doc, mandatory);
   }
 
   @Override
   public Descriptor licenseAttribute(Object defaultO, String doc, Boolean mandatory,
       FuncallExpression ast, Environment env) throws EvalException {
-    return new FakeDescriptor(doc);
+    return new FakeDescriptor(Type.LICENSE, doc, mandatory);
   }
 
   @Override
diff --git a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeSkylarkRuleFunctionsApi.java b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeSkylarkRuleFunctionsApi.java
index 9b1081b6f1..4260032ad8 100644
--- a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeSkylarkRuleFunctionsApi.java
+++ b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeSkylarkRuleFunctionsApi.java
@@ -14,7 +14,7 @@
 
 package com.google.devtools.build.skydoc.fakebuildapi;
 
-import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.events.Location;
 import com.google.devtools.build.lib.skylarkbuildapi.FileApi;
@@ -29,10 +29,11 @@ import com.google.devtools.build.lib.syntax.FuncallExpression;
 import com.google.devtools.build.lib.syntax.Runtime;
 import com.google.devtools.build.lib.syntax.SkylarkDict;
 import com.google.devtools.build.lib.syntax.SkylarkList;
+import com.google.devtools.build.skydoc.fakebuildapi.FakeDescriptor.Type;
 import com.google.devtools.build.skydoc.rendering.AttributeInfo;
 import com.google.devtools.build.skydoc.rendering.RuleInfo;
+import java.util.Comparator;
 import java.util.List;
-import java.util.Map;
 import java.util.stream.Collectors;
 import javax.annotation.Nullable;
 
@@ -44,6 +45,8 @@ import javax.annotation.Nullable;
  */
 public class FakeSkylarkRuleFunctionsApi implements SkylarkRuleFunctionsApi<FileApi> {
 
+  private static final FakeDescriptor IMPLICIT_NAME_ATTRIBUTE_DESCRIPTOR =
+      new FakeDescriptor(Type.STRING, "A unique name for this rule.", true);
   private final List<RuleInfo> ruleInfoList;
 
   /**
@@ -69,20 +72,22 @@ public class FakeSkylarkRuleFunctionsApi implements SkylarkRuleFunctionsApi<File
       Boolean executionPlatformConstraintsAllowed, SkylarkList<?> execCompatibleWith,
       FuncallExpression ast, Environment funcallEnv) throws EvalException {
     List<AttributeInfo> attrInfos;
-    // TODO(cparsons): Include implicit "Name" attribute.
+    ImmutableMap.Builder<String, FakeDescriptor> attrsMapBuilder = ImmutableMap.builder();
     if (attrs != null && attrs != Runtime.NONE) {
       SkylarkDict<?, ?> attrsDict = (SkylarkDict<?, ?>) attrs;
-      Map<String, FakeDescriptor> attrsMap =
-          attrsDict.getContents(String.class, FakeDescriptor.class, "attrs");
-      // TODO(cparsons): Include better attribute details. For example, attribute type.
-      attrInfos = attrsMap.entrySet().stream()
-          .map(entry -> new AttributeInfo(entry.getKey(), entry.getValue().getDocString()))
-          .sorted((o1, o2) -> o1.getName().compareTo(o2.getName()))
-          .collect(Collectors.toList());
-    } else {
-      attrInfos = ImmutableList.of();
+      attrsMapBuilder.putAll(attrsDict.getContents(String.class, FakeDescriptor.class, "attrs"));
     }
 
+    attrsMapBuilder.put("name", IMPLICIT_NAME_ATTRIBUTE_DESCRIPTOR);
+    attrInfos = attrsMapBuilder.build().entrySet().stream()
+        .map(entry -> new AttributeInfo(
+            entry.getKey(),
+            entry.getValue().getDocString(),
+            entry.getValue().getType().getDescription(),
+            entry.getValue().isMandatory()))
+        .collect(Collectors.toList());
+    attrInfos.sort(new AttributeNameComparator());
+
     RuleDefinitionIdentifier functionIdentifier = new RuleDefinitionIdentifier();
 
     ruleInfoList.add(new RuleInfo(functionIdentifier, ast.getLocation(), doc, attrInfos));
@@ -128,4 +133,22 @@ public class FakeSkylarkRuleFunctionsApi implements SkylarkRuleFunctionsApi<File
       return this == other;
     }
   }
+
+  /**
+   * A comparator for {@link AttributeInfo} objects which sorts by attribute name alphabetically,
+   * except that any attribute named "name" is placed first.
+   */
+  private static class AttributeNameComparator implements Comparator<AttributeInfo> {
+
+    @Override
+    public int compare(AttributeInfo o1, AttributeInfo o2) {
+      if (o1.getName().equals("name")) {
+        return o2.getName().equals("name") ? 0 : -1;
+      } else if (o2.getName().equals("name")) {
+        return 1;
+      } else {
+        return o1.getName().compareTo(o2.getName());
+      }
+    }
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/skydoc/rendering/AttributeInfo.java b/src/main/java/com/google/devtools/build/skydoc/rendering/AttributeInfo.java
index 8051057321..696fe2deb6 100644
--- a/src/main/java/com/google/devtools/build/skydoc/rendering/AttributeInfo.java
+++ b/src/main/java/com/google/devtools/build/skydoc/rendering/AttributeInfo.java
@@ -21,17 +21,36 @@ public class AttributeInfo {
 
   private final String name;
   private final String docString;
+  private final String typeString;
+  private final boolean mandatory;
 
-  public AttributeInfo(String name, String docString) {
+  public AttributeInfo(String name, String docString, String typeString, boolean mandatory) {
     this.name = name;
     this.docString = docString;
+    this.typeString = typeString;
+    this.mandatory = mandatory;
   }
 
+  @SuppressWarnings("unused") // Used by markdown template.
   public String getName() {
     return name;
   }
 
+  @SuppressWarnings("unused") // Used by markdown template.
   public String getDocString() {
     return docString;
   }
+
+  @SuppressWarnings("unused") // Used by markdown template.
+  public String getTypeString() {
+    return typeString;
+  }
+
+  /**
+   * Returns a string representing whether this attribute is required or optional.
+   */
+  @SuppressWarnings("unused") // Used by markdown template.
+  public String getMandatoryString() {
+    return mandatory ? "required" : "optional";
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/skydoc/rendering/MarkdownRenderer.java b/src/main/java/com/google/devtools/build/skydoc/rendering/MarkdownRenderer.java
index f9f5495134..9632d5342f 100644
--- a/src/main/java/com/google/devtools/build/skydoc/rendering/MarkdownRenderer.java
+++ b/src/main/java/com/google/devtools/build/skydoc/rendering/MarkdownRenderer.java
@@ -33,7 +33,7 @@ import org.apache.velocity.runtime.resource.loader.JarResourceLoader;
 public class MarkdownRenderer {
 
   private static final String TEMPLATE_FILENAME =
-      "com/google/devtools/build/skydoc/rendering/templates/test.vm";
+      "com/google/devtools/build/skydoc/rendering/templates/rule.vm";
 
   private final VelocityEngine velocityEngine;
 
diff --git a/src/main/java/com/google/devtools/build/skydoc/rendering/templates/test.vm b/src/main/java/com/google/devtools/build/skydoc/rendering/templates/rule.vm
index b1600e9a7f..cd5ebafccb 100644
--- a/src/main/java/com/google/devtools/build/skydoc/rendering/templates/test.vm
+++ b/src/main/java/com/google/devtools/build/skydoc/rendering/templates/rule.vm
@@ -19,7 +19,14 @@ ${ruleInfo.docString}
 #foreach ($attribute in $ruleInfo.attributes)
     <tr id="#${ruleName}_${attribute.name}">
       <td><code>${attribute.name}</code></td>
-      <td>${attribute.docString}</td>
+      <td>
+        ${attribute.typeString}; ${attribute.mandatoryString}
+#if (!$attribute.docString.isEmpty())
+        <p>
+          ${attribute.docString}
+        </p>
+#end
+      </td>
     </tr>
 #end
   </tbody>