Merge "AAPT2: Add descriptions of Attributes in Styleables for R.java" into nyc-dev

10 files changed, 285 insertions, 113 deletions

diff --git a/tools/aapt2/ResourceParser.cpp b/tools/aapt2/ResourceParser.cpp
index b100e84f8a46..9704d97029b7 100644
--- a/tools/aapt2/ResourceParser.cpp
+++ b/tools/aapt2/ResourceParser.cpp
@@ -81,6 +81,12 @@ struct ParsedResource {
 
 // Recursively adds resources to the ResourceTable.
 static bool addResourcesToTable(ResourceTable* table, IDiagnostics* diag, ParsedResource* res) {
+    StringPiece16 trimmedComment = util::trimWhitespace(res->comment);
+    if (trimmedComment.size() != res->comment.size()) {
+        // Only if there was a change do we re-assign.
+        res->comment = trimmedComment.toString();
+    }
+
     if (res->symbolState) {
         Symbol symbol;
         symbol.state = res->symbolState.value();
diff --git a/tools/aapt2/integration-tests/StaticLibOne/res/values/values.xml b/tools/aapt2/integration-tests/StaticLibOne/res/values/values.xml
index 2b24544d3d83..d09a4851f7b4 100644
--- a/tools/aapt2/integration-tests/StaticLibOne/res/values/values.xml
+++ b/tools/aapt2/integration-tests/StaticLibOne/res/values/values.xml
@@ -15,8 +15,13 @@
 -->
 
 <resources>
+    <!-- An attribute from StaticLibOne -->
     <attr name="StaticLibOne_attr" format="string" />
 
     <string name="Foo">Foo</string>
     <string name="Foo" product="tablet">Bar</string>
+
+    <declare-styleable name="Widget">
+        <attr name="StaticLibOne_attr" />
+    </declare-styleable>
 </resources>
diff --git a/tools/aapt2/java/AnnotationProcessor.cpp b/tools/aapt2/java/AnnotationProcessor.cpp
index 9c25d4e35975..496e92e0bf93 100644
--- a/tools/aapt2/java/AnnotationProcessor.cpp
+++ b/tools/aapt2/java/AnnotationProcessor.cpp
@@ -38,7 +38,7 @@ void AnnotationProcessor::appendCommentLine(const std::string& comment) {
         mComment << "/**";
     }
 
-    mComment << "\n" << " * " << std::move(comment);
+    mComment << "\n * " << std::move(comment);
 }
 
 void AnnotationProcessor::appendComment(const StringPiece16& comment) {
@@ -60,6 +60,10 @@ void AnnotationProcessor::appendComment(const StringPiece& comment) {
     }
 }
 
+void AnnotationProcessor::appendNewLine() {
+    mComment << "\n *";
+}
+
 void AnnotationProcessor::writeToStream(std::ostream* out, const StringPiece& prefix) {
     if (mHasComments) {
         std::string result = mComment.str();
diff --git a/tools/aapt2/java/AnnotationProcessor.h b/tools/aapt2/java/AnnotationProcessor.h
index e7f2be0dc12b..fadf58440991 100644
--- a/tools/aapt2/java/AnnotationProcessor.h
+++ b/tools/aapt2/java/AnnotationProcessor.h
@@ -61,6 +61,8 @@ public:
     void appendComment(const StringPiece16& comment);
     void appendComment(const StringPiece& comment);
 
+    void appendNewLine();
+
     /**
      * Writes the comments and annotations to the stream, with the given prefix before each line.
      */
diff --git a/tools/aapt2/java/JavaClassGenerator.cpp b/tools/aapt2/java/JavaClassGenerator.cpp
index 1076ffe1b01d..01330dc6b5a1 100644
--- a/tools/aapt2/java/JavaClassGenerator.cpp
+++ b/tools/aapt2/java/JavaClassGenerator.cpp
@@ -23,6 +23,7 @@
 #include "java/AnnotationProcessor.h"
 #include "java/ClassDefinitionWriter.h"
 #include "java/JavaClassGenerator.h"
+#include "process/SymbolTable.h"
 #include "util/StringPiece.h"
 
 #include <algorithm>
@@ -33,8 +34,9 @@
 
 namespace aapt {
 
-JavaClassGenerator::JavaClassGenerator(ResourceTable* table, JavaClassGeneratorOptions options) :
-        mTable(table), mOptions(options) {
+JavaClassGenerator::JavaClassGenerator(IAaptContext* context, ResourceTable* table,
+                                       const JavaClassGeneratorOptions& options) :
+        mContext(context), mTable(table), mOptions(options) {
 }
 
 static void generateHeader(const StringPiece16& packageNameToGenerate, std::ostream* out) {
@@ -103,6 +105,85 @@ static std::string transformNestedAttr(const ResourceNameRef& attrName,
     return output;
 }
 
+static void addAttributeFormatDoc(AnnotationProcessor* processor, Attribute* attr) {
+    const uint32_t typeMask = attr->typeMask;
+    if (typeMask & android::ResTable_map::TYPE_REFERENCE) {
+        processor->appendComment(
+                "<p>May be a reference to another resource, in the form\n"
+                        "\"<code>@[+][<i>package</i>:]<i>type</i>/<i>name</i></code>\" or a theme\n"
+                        "attribute in the form\n"
+                        "\"<code>?[<i>package</i>:]<i>type</i>/<i>name</i></code>\".");
+    }
+
+    if (typeMask & android::ResTable_map::TYPE_STRING) {
+        processor->appendComment(
+                "<p>May be a string value, using '\\\\;' to escape characters such as\n"
+                        "'\\\\n' or '\\\\uxxxx' for a unicode character;");
+    }
+
+    if (typeMask & android::ResTable_map::TYPE_INTEGER) {
+        processor->appendComment("<p>May be an integer value, such as \"<code>100</code>\".");
+    }
+
+    if (typeMask & android::ResTable_map::TYPE_BOOLEAN) {
+        processor->appendComment(
+                "<p>May be a boolean value, such as \"<code>true</code>\" or\n"
+                        "\"<code>false</code>\".");
+    }
+
+    if (typeMask & android::ResTable_map::TYPE_COLOR) {
+        processor->appendComment(
+                "<p>May be a color value, in the form of \"<code>#<i>rgb</i></code>\",\n"
+                        "\"<code>#<i>argb</i></code>\", \"<code>#<i>rrggbb</i></code\", or \n"
+                        "\"<code>#<i>aarrggbb</i></code>\".");
+    }
+
+    if (typeMask & android::ResTable_map::TYPE_FLOAT) {
+        processor->appendComment(
+                "<p>May be a floating point value, such as \"<code>1.2</code>\".");
+    }
+
+    if (typeMask & android::ResTable_map::TYPE_DIMENSION) {
+        processor->appendComment(
+                "<p>May be a dimension value, which is a floating point number appended with a\n"
+                        "unit such as \"<code>14.5sp</code>\".\n"
+                        "Available units are: px (pixels), dp (density-independent pixels),\n"
+                        "sp (scaled pixels based on preferred font size), in (inches), and\n"
+                        "mm (millimeters).");
+    }
+
+    if (typeMask & android::ResTable_map::TYPE_FRACTION) {
+        processor->appendComment(
+                "<p>May be a fractional value, which is a floating point number appended with\n"
+                        "either % or %p, such as \"<code>14.5%</code>\".\n"
+                        "The % suffix always means a percentage of the base size;\n"
+                        "the optional %p suffix provides a size relative to some parent container.");
+    }
+
+    if (typeMask & (android::ResTable_map::TYPE_FLAGS | android::ResTable_map::TYPE_ENUM)) {
+        if (typeMask & android::ResTable_map::TYPE_FLAGS) {
+            processor->appendComment(
+                    "<p>Must be one or more (separated by '|') of the following "
+                            "constant values.</p>");
+        } else {
+            processor->appendComment("<p>Must be one of the following constant values.</p>");
+        }
+
+        processor->appendComment("<table>\n<colgroup align=\"left\" />\n"
+                                         "<colgroup align=\"left\" />\n"
+                                         "<colgroup align=\"left\" />\n"
+                                         "<tr><th>Constant</th><th>Value</th><th>Description</th></tr>\n");
+        for (const Attribute::Symbol& symbol : attr->symbols) {
+            std::stringstream line;
+            line << "<tr><td>" << symbol.symbol.name.value().entry << "</td>"
+            << "<td>" << std::hex << symbol.value << std::dec << "</td>"
+            << "<td>" << util::trimWhitespace(symbol.symbol.getComment()) << "</td></tr>";
+            processor->appendComment(line.str());
+        }
+        processor->appendComment("</table>");
+    }
+}
+
 bool JavaClassGenerator::skipSymbol(SymbolState state) {
     switch (mOptions.types) {
     case JavaClassGeneratorOptions::SymbolTypes::kAll:
@@ -117,6 +198,7 @@ bool JavaClassGenerator::skipSymbol(SymbolState state) {
 
 struct StyleableAttr {
     const Reference* attrRef;
+    std::shared_ptr<Attribute> attribute;
     std::string fieldName;
 };
 
@@ -148,8 +230,29 @@ void JavaClassGenerator::writeStyleableEntryForClass(ClassDefinitionWriter* outC
         assert((!mOptions.useFinal || attr.id) && "no ID set for Styleable entry");
         assert(attr.name && "no name set for Styleable entry");
 
-        sortedAttributes.emplace_back(StyleableAttr{
-                &attr, transformNestedAttr(attr.name.value(), className, packageNameToGenerate) });
+        StyleableAttr styleableAttr = {};
+        styleableAttr.attrRef = &attr;
+        styleableAttr.fieldName = transformNestedAttr(attr.name.value(), className,
+                                                      packageNameToGenerate);
+
+        Reference mangledReference;
+        mangledReference.id = attr.id;
+        mangledReference.name = attr.name;
+        if (mangledReference.name.value().package.empty()) {
+            mangledReference.name.value().package = mContext->getCompilationPackage();
+        }
+
+        if (Maybe<ResourceName> mangledName =
+                mContext->getNameMangler()->mangleName(mangledReference.name.value())) {
+            mangledReference.name = mangledName;
+        }
+
+        const SymbolTable::Symbol* symbol = mContext->getExternalSymbols()->findByReference(
+                mangledReference);
+        if (symbol) {
+            styleableAttr.attribute = symbol->attribute;
+        }
+        sortedAttributes.push_back(std::move(styleableAttr));
     }
     std::sort(sortedAttributes.begin(), sortedAttributes.end(), lessStyleableAttr);
 
@@ -159,16 +262,34 @@ void JavaClassGenerator::writeStyleableEntryForClass(ClassDefinitionWriter* outC
         // 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"
+        if (!styleable->getComment().empty()) {
+            styleableComment << styleable->getComment() << "\n";
+        } else {
+            styleableComment << "Attributes that can be used with a " << className << ".\n";
+        }
+        styleableComment <<
+                "<p>Includes the following attributes:</p>\n"
+                "<table>\n"
+                "<colgroup align=\"left\" />\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 << "<tr><td>";
+            styleableComment << "<code>{@link #"
+                             << entry.fieldName << " "
+                             << (!attrName.package.empty()
+                                    ? attrName.package : mContext->getCompilationPackage())
+                             << ":" << attrName.entry
+                             << "}</code>";
+            styleableComment << "</td>";
+
+            styleableComment << "<td>";
+            if (entry.attribute) {
+                styleableComment << entry.attribute->getComment();
+            }
+            styleableComment << "</td></tr>\n";
         }
         styleableComment << "</table>\n";
         for (const auto& entry : sortedAttributes) {
@@ -189,96 +310,45 @@ void JavaClassGenerator::writeStyleableEntryForClass(ClassDefinitionWriter* outC
 
     // Now we emit the indices into the array.
     for (size_t i = 0; i < attrCount; i++) {
-        const ResourceName& attrName = sortedAttributes[i].attrRef->name.value();
+        const StyleableAttr& styleableAttr = sortedAttributes[i];
+        const ResourceName& attrName = styleableAttr.attrRef->name.value();
 
-        AnnotationProcessor attrProcessor;
-        std::stringstream doclavaComments;
-        doclavaComments << "@attr name ";
-        if (!attrName.package.empty()) {
-            doclavaComments << attrName.package << ":";
+        StringPiece16 packageName = attrName.package;
+        if (packageName.empty()) {
+            packageName = mContext->getCompilationPackage();
         }
-        doclavaComments << attrName.entry;
-        attrProcessor.appendComment(doclavaComments.str());
-        outClassDef->addIntMember(sortedAttributes[i].fieldName, &attrProcessor, i);
-    }
-}
-
-static void addAttributeFormatDoc(AnnotationProcessor* processor, Attribute* attr) {
-    const uint32_t typeMask = attr->typeMask;
-    if (typeMask & android::ResTable_map::TYPE_REFERENCE) {
-        processor->appendComment(
-                "<p>May be a reference to another resource, in the form\n"
-                "\"<code>@[+][<i>package</i>:]<i>type</i>/<i>name</i></code>\" or a theme\n"
-                "attribute in the form\n"
-                "\"<code>?[<i>package</i>:]<i>type</i>/<i>name</i></code>\".");
-    }
-
-    if (typeMask & android::ResTable_map::TYPE_STRING) {
-        processor->appendComment(
-                "<p>May be a string value, using '\\\\;' to escape characters such as\n"
-                "'\\\\n' or '\\\\uxxxx' for a unicode character;");
-    }
-
-    if (typeMask & android::ResTable_map::TYPE_INTEGER) {
-        processor->appendComment("<p>May be an integer value, such as \"<code>100</code>\".");
-    }
-
-    if (typeMask & android::ResTable_map::TYPE_BOOLEAN) {
-        processor->appendComment(
-                "<p>May be a boolean value, such as \"<code>true</code>\" or\n"
-                "\"<code>false</code>\".");
-    }
-
-    if (typeMask & android::ResTable_map::TYPE_COLOR) {
-        processor->appendComment(
-                "<p>May be a color value, in the form of \"<code>#<i>rgb</i></code>\",\n"
-                "\"<code>#<i>argb</i></code>\", \"<code>#<i>rrggbb</i></code\", or \n"
-                "\"<code>#<i>aarrggbb</i></code>\".");
-    }
-
-    if (typeMask & android::ResTable_map::TYPE_FLOAT) {
-        processor->appendComment(
-                "<p>May be a floating point value, such as \"<code>1.2</code>\".");
-    }
 
-    if (typeMask & android::ResTable_map::TYPE_DIMENSION) {
-        processor->appendComment(
-                "<p>May be a dimension value, which is a floating point number appended with a\n"
-                "unit such as \"<code>14.5sp</code>\".\n"
-                "Available units are: px (pixels), dp (density-independent pixels),\n"
-                "sp (scaled pixels based on preferred font size), in (inches), and\n"
-                "mm (millimeters).");
-    }
+        AnnotationProcessor attrProcessor;
 
-    if (typeMask & android::ResTable_map::TYPE_FRACTION) {
-        processor->appendComment(
-                "<p>May be a fractional value, which is a floating point number appended with\n"
-                "either % or %p, such as \"<code>14.5%</code>\".\n"
-                "The % suffix always means a percentage of the base size;\n"
-                "the optional %p suffix provides a size relative to some parent container.");
-    }
+        StringPiece16 comment = styleableAttr.attrRef->getComment();
+        if (styleableAttr.attribute && comment.empty()) {
+            comment = styleableAttr.attribute->getComment();
+        }
 
-    if (typeMask & (android::ResTable_map::TYPE_FLAGS | android::ResTable_map::TYPE_ENUM)) {
-        if (typeMask & android::ResTable_map::TYPE_FLAGS) {
-            processor->appendComment(
-                    "<p>Must be one or more (separated by '|') of the following "
-                    "constant values.</p>");
+        if (!comment.empty()) {
+            attrProcessor.appendComment("<p>\n@attr description");
+            attrProcessor.appendComment(comment);
         } else {
-            processor->appendComment("<p>Must be one of the following constant values.</p>");
+            std::stringstream defaultComment;
+            defaultComment
+                    << "<p>This symbol is the offset where the "
+                    << "{@link " << packageName << ".R.attr#" << transform(attrName.entry) << "}\n"
+                    << "attribute's value can be found in the "
+                    << "{@link #" << className << "} array.";
+            attrProcessor.appendComment(defaultComment.str());
         }
 
-        processor->appendComment("<table>\n<colgroup align=\"left\" />\n"
-                                 "<colgroup align=\"left\" />\n"
-                                 "<colgroup align=\"left\" />\n"
-                                 "<tr><th>Constant</th><th>Value</th><th>Description</th></tr>\n");
-        for (const Attribute::Symbol& symbol : attr->symbols) {
-            std::stringstream line;
-            line << "<tr><td>" << symbol.symbol.name.value().entry << "</td>"
-                 << "<td>" << std::hex << symbol.value << std::dec << "</td>"
-                 << "<td>" << util::trimWhitespace(symbol.symbol.getComment()) << "</td></tr>";
-            processor->appendComment(line.str());
+        attrProcessor.appendNewLine();
+
+        if (styleableAttr.attribute) {
+            addAttributeFormatDoc(&attrProcessor, styleableAttr.attribute.get());
+            attrProcessor.appendNewLine();
         }
-        processor->appendComment("</table>");
+
+        std::stringstream doclavaName;
+        doclavaName << "@attr name " << packageName << ":" << attrName.entry;;
+        attrProcessor.appendComment(doclavaName.str());
+        outClassDef->addIntMember(sortedAttributes[i].fieldName, &attrProcessor, i);
     }
 }
 
diff --git a/tools/aapt2/java/JavaClassGenerator.h b/tools/aapt2/java/JavaClassGenerator.h
index 023d6d635f8c..7e46f8c9043c 100644
--- a/tools/aapt2/java/JavaClassGenerator.h
+++ b/tools/aapt2/java/JavaClassGenerator.h
@@ -19,7 +19,7 @@
 
 #include "ResourceTable.h"
 #include "ResourceValues.h"
-
+#include "process/IResourceTableConsumer.h"
 #include "util/StringPiece.h"
 
 #include <ostream>
@@ -51,7 +51,8 @@ struct JavaClassGeneratorOptions {
  */
 class JavaClassGenerator {
 public:
-    JavaClassGenerator(ResourceTable* table, JavaClassGeneratorOptions options);
+    JavaClassGenerator(IAaptContext* context, ResourceTable* table,
+                       const JavaClassGeneratorOptions& options);
 
     /*
      * Writes the R.java file to `out`. Only symbols belonging to `package` are written.
@@ -82,6 +83,7 @@ private:
 
     bool skipSymbol(SymbolState state);
 
+    IAaptContext* mContext;
     ResourceTable* mTable;
     JavaClassGeneratorOptions mOptions;
     std::string mError;
diff --git a/tools/aapt2/java/JavaClassGenerator_test.cpp b/tools/aapt2/java/JavaClassGenerator_test.cpp
index 63d38a8981f4..4f041b88efbc 100644
--- a/tools/aapt2/java/JavaClassGenerator_test.cpp
+++ b/tools/aapt2/java/JavaClassGenerator_test.cpp
@@ -15,11 +15,9 @@
  */
 
 #include "java/JavaClassGenerator.h"
+#include "test/Test.h"
 #include "util/Util.h"
 
-#include "test/Builders.h"
-
-#include <gtest/gtest.h>
 #include <sstream>
 #include <string>
 
@@ -31,7 +29,11 @@ TEST(JavaClassGeneratorTest, FailWhenEntryIsJavaKeyword) {
             .addSimple(u"@android:id/class", ResourceId(0x01020000))
             .build();
 
-    JavaClassGenerator generator(table.get(), {});
+    std::unique_ptr<IAaptContext> context = test::ContextBuilder()
+            .addSymbolSource(util::make_unique<ResourceTableSymbolSource>(table.get()))
+            .setNameManglerPolicy(NameManglerPolicy{ u"android" })
+            .build();
+    JavaClassGenerator generator(context.get(), table.get(), {});
 
     std::stringstream out;
     EXPECT_FALSE(generator.generate(u"android", &out));
@@ -48,7 +50,11 @@ TEST(JavaClassGeneratorTest, TransformInvalidJavaIdentifierCharacter) {
                               .build())
             .build();
 
-    JavaClassGenerator generator(table.get(), {});
+    std::unique_ptr<IAaptContext> context = test::ContextBuilder()
+            .addSymbolSource(util::make_unique<ResourceTableSymbolSource>(table.get()))
+            .setNameManglerPolicy(NameManglerPolicy{ u"android" })
+            .build();
+    JavaClassGenerator generator(context.get(), table.get(), {});
 
     std::stringstream out;
     EXPECT_TRUE(generator.generate(u"android", &out));
@@ -72,7 +78,11 @@ TEST(JavaClassGeneratorTest, CorrectPackageNameIsUsed) {
             .addSimple(u"@android:id/com.foo$two", ResourceId(0x01020001))
             .build();
 
-    JavaClassGenerator generator(table.get(), {});
+    std::unique_ptr<IAaptContext> context = test::ContextBuilder()
+            .addSymbolSource(util::make_unique<ResourceTableSymbolSource>(table.get()))
+            .setNameManglerPolicy(NameManglerPolicy{ u"android" })
+            .build();
+    JavaClassGenerator generator(context.get(), table.get(), {});
     std::stringstream out;
     ASSERT_TRUE(generator.generate(u"android", u"com.android.internal", &out));
 
@@ -90,7 +100,11 @@ TEST(JavaClassGeneratorTest, AttrPrivateIsWrittenAsAttr) {
             .addSimple(u"@android:^attr-private/one", ResourceId(0x01010000))
             .build();
 
-    JavaClassGenerator generator(table.get(), {});
+    std::unique_ptr<IAaptContext> context = test::ContextBuilder()
+            .addSymbolSource(util::make_unique<ResourceTableSymbolSource>(table.get()))
+            .setNameManglerPolicy(NameManglerPolicy{ u"android" })
+            .build();
+    JavaClassGenerator generator(context.get(), table.get(), {});
     std::stringstream out;
     ASSERT_TRUE(generator.generate(u"android", &out));
 
@@ -110,10 +124,15 @@ TEST(JavaClassGeneratorTest, OnlyWritePublicResources) {
             .setSymbolState(u"@android:id/two", ResourceId(0x01020001), SymbolState::kPrivate)
             .build();
 
+    std::unique_ptr<IAaptContext> context = test::ContextBuilder()
+            .addSymbolSource(util::make_unique<ResourceTableSymbolSource>(table.get()))
+            .setNameManglerPolicy(NameManglerPolicy{ u"android" })
+            .build();
+
     JavaClassGeneratorOptions options;
     options.types = JavaClassGeneratorOptions::SymbolTypes::kPublic;
     {
-        JavaClassGenerator generator(table.get(), options);
+        JavaClassGenerator generator(context.get(), table.get(), options);
         std::stringstream out;
         ASSERT_TRUE(generator.generate(u"android", &out));
         std::string output = out.str();
@@ -124,7 +143,7 @@ TEST(JavaClassGeneratorTest, OnlyWritePublicResources) {
 
     options.types = JavaClassGeneratorOptions::SymbolTypes::kPublicPrivate;
     {
-        JavaClassGenerator generator(table.get(), options);
+        JavaClassGenerator generator(context.get(), table.get(), options);
         std::stringstream out;
         ASSERT_TRUE(generator.generate(u"android", &out));
         std::string output = out.str();
@@ -135,7 +154,7 @@ TEST(JavaClassGeneratorTest, OnlyWritePublicResources) {
 
     options.types = JavaClassGeneratorOptions::SymbolTypes::kAll;
     {
-        JavaClassGenerator generator(table.get(), options);
+        JavaClassGenerator generator(context.get(), table.get(), options);
         std::stringstream out;
         ASSERT_TRUE(generator.generate(u"android", &out));
         std::string output = out.str();
@@ -189,7 +208,11 @@ TEST(JavaClassGeneratorTest, EmitOtherPackagesAttributesInStyleable) {
                                   .build())
                 .build();
 
-    JavaClassGenerator generator(table.get(), {});
+    std::unique_ptr<IAaptContext> context = test::ContextBuilder()
+            .addSymbolSource(util::make_unique<ResourceTableSymbolSource>(table.get()))
+            .setNameManglerPolicy(NameManglerPolicy{ u"android" })
+            .build();
+    JavaClassGenerator generator(context.get(), table.get(), {});
 
     std::stringstream out;
     EXPECT_TRUE(generator.generate(u"android", &out));
@@ -207,8 +230,11 @@ TEST(JavaClassGeneratorTest, CommentsForSimpleResourcesArePresent) {
     test::getValue<Id>(table.get(), u"@android:id/foo")
             ->setComment(std::u16string(u"This is a comment\n@deprecated"));
 
-    JavaClassGenerator generator(table.get(), {});
-
+    std::unique_ptr<IAaptContext> context = test::ContextBuilder()
+            .addSymbolSource(util::make_unique<ResourceTableSymbolSource>(table.get()))
+            .setNameManglerPolicy(NameManglerPolicy{ u"android" })
+            .build();
+    JavaClassGenerator generator(context.get(), table.get(), {});
     std::stringstream out;
     ASSERT_TRUE(generator.generate(u"android", &out));
     std::string actual = out.str();
@@ -241,10 +267,13 @@ TEST(JavaClassGeneratorTest, CommentsForStyleablesAndNestedAttributesArePresent)
                       std::unique_ptr<Styleable>(styleable.clone(nullptr)))
             .build();
 
+    std::unique_ptr<IAaptContext> context = test::ContextBuilder()
+            .addSymbolSource(util::make_unique<ResourceTableSymbolSource>(table.get()))
+            .setNameManglerPolicy(NameManglerPolicy{ u"android" })
+            .build();
     JavaClassGeneratorOptions options;
     options.useFinal = false;
-    JavaClassGenerator generator(table.get(), options);
-
+    JavaClassGenerator generator(context.get(), table.get(), options);
     std::stringstream out;
     ASSERT_TRUE(generator.generate(u"android", &out));
     std::string actual = out.str();
diff --git a/tools/aapt2/link/Link.cpp b/tools/aapt2/link/Link.cpp
index 418a802983c3..b84074d1cb58 100644
--- a/tools/aapt2/link/Link.cpp
+++ b/tools/aapt2/link/Link.cpp
@@ -744,7 +744,7 @@ public:
             return false;
         }
 
-        JavaClassGenerator generator(table, javaOptions);
+        JavaClassGenerator generator(mContext, table, javaOptions);
         if (!generator.generate(packageNameToGenerate, outPackage, &fout)) {
             mContext->getDiagnostics()->error(DiagMessage(outPath) << generator.getError());
             return false;
diff --git a/tools/aapt2/process/SymbolTable.cpp b/tools/aapt2/process/SymbolTable.cpp
index a8f9bfe4fb32..eaaf06f7e530 100644
--- a/tools/aapt2/process/SymbolTable.cpp
+++ b/tools/aapt2/process/SymbolTable.cpp
@@ -51,6 +51,11 @@ const SymbolTable::Symbol* SymbolTable::findByName(const ResourceName& name) {
             // doesn't support unique_ptr.
             std::shared_ptr<Symbol> sharedSymbol = std::shared_ptr<Symbol>(symbol.release());
             mCache.put(name, sharedSymbol);
+
+            if (sharedSymbol->id) {
+                // The symbol has an ID, so we can also cache this!
+                mIdCache.put(sharedSymbol->id.value(), sharedSymbol);
+            }
             return sharedSymbol.get();
         }
     }
@@ -76,6 +81,25 @@ const SymbolTable::Symbol* SymbolTable::findById(ResourceId id) {
     return nullptr;
 }
 
+const SymbolTable::Symbol* SymbolTable::findByReference(const Reference& ref) {
+    // First try the ID. This is because when we lookup by ID, we only fill in the ID cache.
+    // Looking up by name fills in the name and ID cache. So a cache miss will cause a failed
+    // ID lookup, then a successfull name lookup. Subsequent look ups will hit immediately
+    // because the ID is cached too.
+    //
+    // If we looked up by name first, a cache miss would mean we failed to lookup by name, then
+    // succeeded to lookup by ID. Subsequent lookups will miss then hit.
+    const SymbolTable::Symbol* symbol = nullptr;
+    if (ref.id) {
+        symbol = findById(ref.id.value());
+    }
+
+    if (ref.name && !symbol) {
+        symbol = findByName(ref.name.value());
+    }
+    return symbol;
+}
+
 std::unique_ptr<SymbolTable::Symbol> ResourceTableSymbolSource::findByName(
         const ResourceName& name) {
     Maybe<ResourceTable::SearchResult> result = mTable->findResource(name);
@@ -102,7 +126,7 @@ std::unique_ptr<SymbolTable::Symbol> ResourceTableSymbolSource::findByName(
         if (configValue) {
             // This resource has an Attribute.
             if (Attribute* attr = valueCast<Attribute>(configValue->value.get())) {
-                symbol->attribute = util::make_unique<Attribute>(*attr);
+                symbol->attribute = std::make_shared<Attribute>(*attr);
             } else {
                 return {};
             }
@@ -133,7 +157,7 @@ static std::unique_ptr<SymbolTable::Symbol> lookupAttributeInTable(const android
     // Check to see if it is an attribute.
     for (size_t i = 0; i < (size_t) count; i++) {
         if (entry[i].map.name.ident == android::ResTable_map::ATTR_TYPE) {
-            s->attribute = util::make_unique<Attribute>(false);
+            s->attribute = std::make_shared<Attribute>(false);
             s->attribute->typeMask = entry[i].map.value.data;
             break;
         }
@@ -272,4 +296,15 @@ std::unique_ptr<SymbolTable::Symbol> AssetManagerSymbolSource::findById(Resource
     return {};
 }
 
+std::unique_ptr<SymbolTable::Symbol> AssetManagerSymbolSource::findByReference(
+        const Reference& ref) {
+    // AssetManager always prefers IDs.
+    if (ref.id) {
+        return findById(ref.id.value());
+    } else if (ref.name) {
+        return findByName(ref.name.value());
+    }
+    return {};
+}
+
 } // namespace aapt
diff --git a/tools/aapt2/process/SymbolTable.h b/tools/aapt2/process/SymbolTable.h
index 8ea1c757fc68..0a6a4a541a8d 100644
--- a/tools/aapt2/process/SymbolTable.h
+++ b/tools/aapt2/process/SymbolTable.h
@@ -52,7 +52,7 @@ class SymbolTable {
 public:
     struct Symbol {
         Maybe<ResourceId> id;
-        std::unique_ptr<Attribute> attribute;
+        std::shared_ptr<Attribute> attribute;
         bool isPublic;
     };
 
@@ -69,6 +69,12 @@ public:
     const Symbol* findByName(const ResourceName& name);
     const Symbol* findById(ResourceId id);
 
+    /**
+     * Let's the ISymbolSource decide whether looking up by name or ID is faster, if both
+     * are available.
+     */
+    const Symbol* findByReference(const Reference& ref);
+
 private:
     std::vector<std::unique_ptr<ISymbolSource>> mSources;
 
@@ -90,6 +96,18 @@ public:
 
     virtual std::unique_ptr<SymbolTable::Symbol> findByName(const ResourceName& name) = 0;
     virtual std::unique_ptr<SymbolTable::Symbol> findById(ResourceId id) = 0;
+
+    /**
+     * Default implementation tries the name if it exists, else the ID.
+     */
+    virtual std::unique_ptr<SymbolTable::Symbol> findByReference(const Reference& ref) {
+        if (ref.name) {
+            return findByName(ref.name.value());
+        } else if (ref.id) {
+            return findById(ref.id.value());
+        }
+        return {};
+    }
 };
 
 /**
@@ -122,6 +140,7 @@ public:
 
     std::unique_ptr<SymbolTable::Symbol> findByName(const ResourceName& name) override;
     std::unique_ptr<SymbolTable::Symbol> findById(ResourceId id) override;
+    std::unique_ptr<SymbolTable::Symbol> findByReference(const Reference& ref) override;
 
 private:
     android::AssetManager mAssets;