AAPT2: Emit proper doclava comments in R.java

Bug:25958912
Change-Id: I663f2eb5bd54e3c3288ce9bc186c928f0a014f93
diff --git a/tools/aapt2/Resource.h b/tools/aapt2/Resource.h
index c71e249..4d1db5b 100644
--- a/tools/aapt2/Resource.h
+++ b/tools/aapt2/Resource.h
@@ -81,9 +81,6 @@
     ResourceName(const StringPiece16& p, ResourceType t, const StringPiece16& e);
 
     bool isValid() const;
-    bool operator<(const ResourceName& rhs) const;
-    bool operator==(const ResourceName& rhs) const;
-    bool operator!=(const ResourceName& rhs) const;
     std::u16string toString() const;
 };
 
@@ -109,10 +106,6 @@
 
     ResourceName toResourceName() const;
     bool isValid() const;
-
-    bool operator<(const ResourceNameRef& rhs) const;
-    bool operator==(const ResourceNameRef& rhs) const;
-    bool operator!=(const ResourceNameRef& rhs) const;
 };
 
 /**
@@ -138,17 +131,11 @@
     uint8_t packageId() const;
     uint8_t typeId() const;
     uint16_t entryId() const;
-    bool operator<(const ResourceId& rhs) const;
-    bool operator==(const ResourceId& rhs) const;
 };
 
 struct SourcedResourceName {
     ResourceName name;
     size_t line;
-
-    inline bool operator==(const SourcedResourceName& rhs) const {
-        return name == rhs.name && line == rhs.line;
-    }
 };
 
 struct ResourceFile {
@@ -227,16 +214,23 @@
     return static_cast<uint16_t>(id);
 }
 
-inline bool ResourceId::operator<(const ResourceId& rhs) const {
-    return id < rhs.id;
+inline bool operator<(const ResourceId& lhs, const ResourceId& rhs) {
+    return lhs.id < rhs.id;
 }
 
-inline bool ResourceId::operator==(const ResourceId& rhs) const {
-    return id == rhs.id;
+inline bool operator>(const ResourceId& lhs, const ResourceId& rhs) {
+    return lhs.id > rhs.id;
 }
 
-inline ::std::ostream& operator<<(::std::ostream& out,
-        const ResourceId& resId) {
+inline bool operator==(const ResourceId& lhs, const ResourceId& rhs) {
+    return lhs.id == rhs.id;
+}
+
+inline bool operator!=(const ResourceId& lhs, const ResourceId& rhs) {
+    return lhs.id != rhs.id;
+}
+
+inline ::std::ostream& operator<<(::std::ostream& out, const ResourceId& resId) {
     std::ios_base::fmtflags oldFlags = out.flags();
     char oldFill = out.fill();
     out << "0x" << std::internal << std::setfill('0') << std::setw(8)
@@ -266,29 +260,21 @@
     return !package.empty() && !entry.empty();
 }
 
-inline bool ResourceName::operator<(const ResourceName& rhs) const {
-    return std::tie(package, type, entry)
+inline bool operator<(const ResourceName& lhs, const ResourceName& rhs) {
+    return std::tie(lhs.package, lhs.type, lhs.entry)
             < std::tie(rhs.package, rhs.type, rhs.entry);
 }
 
-inline bool operator<(const ResourceName& lhs, const ResourceNameRef& b) {
-    return ResourceNameRef(lhs) < b;
-}
-
-inline bool ResourceName::operator==(const ResourceName& rhs) const {
-    return std::tie(package, type, entry)
+inline bool operator==(const ResourceName& lhs, const ResourceName& rhs) {
+    return std::tie(lhs.package, lhs.type, lhs.entry)
             == std::tie(rhs.package, rhs.type, rhs.entry);
 }
 
-inline bool ResourceName::operator!=(const ResourceName& rhs) const {
-    return std::tie(package, type, entry)
+inline bool operator!=(const ResourceName& lhs, const ResourceName& rhs) {
+    return std::tie(lhs.package, lhs.type, lhs.entry)
             != std::tie(rhs.package, rhs.type, rhs.entry);
 }
 
-inline bool operator!=(const ResourceName& lhs, const ResourceNameRef& rhs) {
-    return ResourceNameRef(lhs) != rhs;
-}
-
 inline std::u16string ResourceName::toString() const {
     std::u16string result;
     if (!package.empty()) {
@@ -333,18 +319,18 @@
     return !package.empty() && !entry.empty();
 }
 
-inline bool ResourceNameRef::operator<(const ResourceNameRef& rhs) const {
-    return std::tie(package, type, entry)
+inline bool operator<(const ResourceNameRef& lhs, const ResourceNameRef& rhs) {
+    return std::tie(lhs.package, lhs.type, lhs.entry)
             < std::tie(rhs.package, rhs.type, rhs.entry);
 }
 
-inline bool ResourceNameRef::operator==(const ResourceNameRef& rhs) const {
-    return std::tie(package, type, entry)
+inline bool operator==(const ResourceNameRef& lhs, const ResourceNameRef& rhs) {
+    return std::tie(lhs.package, lhs.type, lhs.entry)
             == std::tie(rhs.package, rhs.type, rhs.entry);
 }
 
-inline bool ResourceNameRef::operator!=(const ResourceNameRef& rhs) const {
-    return std::tie(package, type, entry)
+inline bool operator!=(const ResourceNameRef& lhs, const ResourceNameRef& rhs) {
+    return std::tie(lhs.package, lhs.type, lhs.entry)
             != std::tie(rhs.package, rhs.type, rhs.entry);
 }
 
@@ -355,6 +341,18 @@
     return out << name.type << "/" << name.entry;
 }
 
+inline bool operator<(const ResourceName& lhs, const ResourceNameRef& b) {
+    return ResourceNameRef(lhs) < b;
+}
+
+inline bool operator!=(const ResourceName& lhs, const ResourceNameRef& rhs) {
+    return ResourceNameRef(lhs) != rhs;
+}
+
+inline bool operator==(const SourcedResourceName& lhs, const SourcedResourceName& rhs) {
+    return lhs.name == rhs.name && lhs.line == rhs.line;
+}
+
 } // namespace aapt
 
 #endif // AAPT_RESOURCE_H
diff --git a/tools/aapt2/java/ClassDefinitionWriter.h b/tools/aapt2/java/ClassDefinitionWriter.h
index 04e1274..cf92c9a 100644
--- a/tools/aapt2/java/ClassDefinitionWriter.h
+++ b/tools/aapt2/java/ClassDefinitionWriter.h
@@ -65,7 +65,7 @@
              << "String " << name << "=\"" << val << "\";\n";
     }
 
-    void addResourceMember(const StringPiece16& name, AnnotationProcessor* processor,
+    void addResourceMember(const StringPiece& name, AnnotationProcessor* processor,
                            const ResourceId id) {
         ensureClassDeclaration();
         if (processor) {
@@ -76,7 +76,7 @@
     }
 
     template <typename Iterator, typename FieldAccessorFunc>
-    void addArrayMember(const StringPiece16& name, AnnotationProcessor* processor,
+    void addArrayMember(const StringPiece& name, AnnotationProcessor* processor,
                         const Iterator begin, const Iterator end, FieldAccessorFunc f) {
         ensureClassDeclaration();
         if (processor) {
diff --git a/tools/aapt2/java/JavaClassGenerator.cpp b/tools/aapt2/java/JavaClassGenerator.cpp
index 9e94d42..1076ffe 100644
--- a/tools/aapt2/java/JavaClassGenerator.cpp
+++ b/tools/aapt2/java/JavaClassGenerator.cpp
@@ -68,16 +68,41 @@
  * Java symbols can not contain . or -, but those are valid in a resource name.
  * Replace those with '_'.
  */
-static std::u16string transform(const StringPiece16& symbol) {
-    std::u16string output = symbol.toString();
-    for (char16_t& c : output) {
-        if (c == u'.' || c == u'-') {
-            c = u'_';
+static std::string transform(const StringPiece16& symbol) {
+    std::string output = util::utf16ToUtf8(symbol);
+    for (char& c : output) {
+        if (c == '.' || c == '-') {
+            c = '_';
         }
     }
     return output;
 }
 
+/**
+ * Transforms an attribute in a styleable to the Java field name:
+ *
+ * <declare-styleable name="Foo">
+ *   <attr name="android:bar" />
+ *   <attr name="bar" />
+ * </declare-styleable>
+ *
+ * Foo_android_bar
+ * Foo_bar
+ */
+static std::string transformNestedAttr(const ResourceNameRef& attrName,
+                                       const std::string& styleableClassName,
+                                       const StringPiece16& packageNameToGenerate) {
+    std::string output = styleableClassName;
+
+    // We may reference IDs from other packages, so prefix the entry name with
+    // the package.
+    if (!attrName.package.empty() && packageNameToGenerate != attrName.package) {
+        output += "_" + transform(attrName.package);
+    }
+    output += "_" + transform(attrName.entry);
+    return output;
+}
+
 bool JavaClassGenerator::skipSymbol(SymbolState state) {
     switch (mOptions.types) {
     case JavaClassGeneratorOptions::SymbolTypes::kAll:
@@ -90,48 +115,91 @@
     return true;
 }
 
+struct StyleableAttr {
+    const Reference* attrRef;
+    std::string fieldName;
+};
+
+static bool lessStyleableAttr(const StyleableAttr& lhs, const StyleableAttr& rhs) {
+    const ResourceId lhsId = lhs.attrRef->id ? lhs.attrRef->id.value() : ResourceId(0);
+    const ResourceId rhsId = rhs.attrRef->id ? rhs.attrRef->id.value() : ResourceId(0);
+    if (lhsId < rhsId) {
+        return true;
+    } else if (lhsId > rhsId) {
+        return false;
+    } else {
+        return lhs.attrRef->name.value() < rhs.attrRef->name.value();
+    }
+}
+
 void JavaClassGenerator::writeStyleableEntryForClass(ClassDefinitionWriter* outClassDef,
                                                      AnnotationProcessor* processor,
                                                      const StringPiece16& packageNameToGenerate,
                                                      const std::u16string& entryName,
                                                      const Styleable* styleable) {
+    const std::string className = transform(entryName);
+
     // This must be sorted by resource ID.
-    std::vector<std::pair<ResourceId, ResourceNameRef>> sortedAttributes;
+    std::vector<StyleableAttr> sortedAttributes;
     sortedAttributes.reserve(styleable->entries.size());
     for (const auto& attr : styleable->entries) {
         // If we are not encoding final attributes, the styleable entry may have no ID
         // if we are building a static library.
         assert((!mOptions.useFinal || attr.id) && "no ID set for Styleable entry");
         assert(attr.name && "no name set for Styleable entry");
-        sortedAttributes.emplace_back(attr.id ? attr.id.value() : ResourceId(0), attr.name.value());
-    }
-    std::sort(sortedAttributes.begin(), sortedAttributes.end());
 
-    auto accessorFunc = [](const std::pair<ResourceId, ResourceNameRef>& a) -> ResourceId {
-        return a.first;
+        sortedAttributes.emplace_back(StyleableAttr{
+                &attr, transformNestedAttr(attr.name.value(), className, packageNameToGenerate) });
+    }
+    std::sort(sortedAttributes.begin(), sortedAttributes.end(), lessStyleableAttr);
+
+    const size_t attrCount = sortedAttributes.size();
+
+    if (attrCount > 0) {
+        // Build the comment string for the Styleable. It includes details about the
+        // child attributes.
+        std::stringstream styleableComment;
+        styleableComment << "Attributes that can be used with a " << className << ".\n";
+        styleableComment << "<table>\n"
+                "<colgroup align=\"left\" />\n"
+                "<colgroup align=\"left\">\n"
+                "<tr><th>Attribute</th><th>Description</th></tr>\n";
+        for (const auto& entry : sortedAttributes) {
+            const ResourceName& attrName = entry.attrRef->name.value();
+            styleableComment << "<tr><td><code>{@link #" << entry.fieldName << " "
+                    << attrName.package << ":" << attrName.entry
+                    << "}</code></td><td></td></tr>\n";
+        }
+        styleableComment << "</table>\n";
+        for (const auto& entry : sortedAttributes) {
+            styleableComment << "@see #" << entry.fieldName << "\n";
+        }
+        processor->appendComment(styleableComment.str());
+    }
+
+    auto accessorFunc = [](const StyleableAttr& a) -> ResourceId {
+        return a.attrRef->id ? a.attrRef->id.value() : ResourceId(0);
     };
 
     // First we emit the array containing the IDs of each attribute.
-    outClassDef->addArrayMember(transform(entryName), processor,
+    outClassDef->addArrayMember(className, processor,
                                 sortedAttributes.begin(),
                                 sortedAttributes.end(),
                                 accessorFunc);
 
     // Now we emit the indices into the array.
-    size_t attrCount = sortedAttributes.size();
     for (size_t i = 0; i < attrCount; i++) {
-        std::stringstream name;
-        name << transform(entryName);
+        const ResourceName& attrName = sortedAttributes[i].attrRef->name.value();
 
-        // We may reference IDs from other packages, so prefix the entry name with
-        // the package.
-        const ResourceNameRef& itemName = sortedAttributes[i].second;
-        if (!itemName.package.empty() && packageNameToGenerate != itemName.package) {
-            name << "_" << transform(itemName.package);
+        AnnotationProcessor attrProcessor;
+        std::stringstream doclavaComments;
+        doclavaComments << "@attr name ";
+        if (!attrName.package.empty()) {
+            doclavaComments << attrName.package << ":";
         }
-        name << "_" << transform(itemName.entry);
-
-        outClassDef->addIntMember(name.str(), nullptr, i);
+        doclavaComments << attrName.entry;
+        attrProcessor.appendComment(doclavaComments.str());
+        outClassDef->addIntMember(sortedAttributes[i].fieldName, &attrProcessor, i);
     }
 }
 
diff --git a/tools/aapt2/java/JavaClassGenerator_test.cpp b/tools/aapt2/java/JavaClassGenerator_test.cpp
index e9e7881..63d38a8 100644
--- a/tools/aapt2/java/JavaClassGenerator_test.cpp
+++ b/tools/aapt2/java/JavaClassGenerator_test.cpp
@@ -227,7 +227,32 @@
 }
 
 TEST(JavaClassGeneratorTest, CommentsForStyleablesAndNestedAttributesArePresent) {
+    Attribute attr(false);
+    attr.setComment(StringPiece16(u"This is an attribute"));
 
+    Styleable styleable;
+    styleable.entries.push_back(Reference(test::parseNameOrDie(u"@android:attr/one")));
+    styleable.setComment(StringPiece16(u"This is a styleable"));
+
+    std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder()
+            .setPackageId(u"android", 0x01)
+            .addValue(u"@android:attr/one", util::make_unique<Attribute>(attr))
+            .addValue(u"@android:styleable/Container",
+                      std::unique_ptr<Styleable>(styleable.clone(nullptr)))
+            .build();
+
+    JavaClassGeneratorOptions options;
+    options.useFinal = false;
+    JavaClassGenerator generator(table.get(), options);
+
+    std::stringstream out;
+    ASSERT_TRUE(generator.generate(u"android", &out));
+    std::string actual = out.str();
+
+    EXPECT_NE(std::string::npos, actual.find("@attr name android:one"));
+    EXPECT_NE(std::string::npos, actual.find("@attr description"));
+    EXPECT_NE(std::string::npos, actual.find(util::utf16ToUtf8(attr.getComment())));
+    EXPECT_NE(std::string::npos, actual.find(util::utf16ToUtf8(styleable.getComment())));
 }
 
 } // namespace aapt