AAPT2: Fix inclusion of comments in R.java javadoc

Comments weren't being copied when merged from the various
resource tables.

Also refactored the JavaClassGenerator to omit a class
if no entries exist for it.

Change-Id: I6eaa89b7b3715bc05403635a2baf0d1db3efd142

9 files changed, 390 insertions, 181 deletions

diff --git a/tools/aapt2/java/AnnotationProcessor.cpp b/tools/aapt2/java/AnnotationProcessor.cpp
index b36682d98bfd..9c25d4e35975 100644
--- a/tools/aapt2/java/AnnotationProcessor.cpp
+++ b/tools/aapt2/java/AnnotationProcessor.cpp
@@ -25,33 +25,20 @@ void AnnotationProcessor::appendCommentLine(const std::string& comment) {
     static const std::string sDeprecated = "@deprecated";
     static const std::string sSystemApi = "@SystemApi";
 
-    if (comment.find(sDeprecated) != std::string::npos && !mDeprecated) {
-        mDeprecated = true;
-        if (!mAnnotations.empty()) {
-            mAnnotations += "\n";
-        }
-        mAnnotations += mPrefix;
-        mAnnotations += "@Deprecated";
+    if (comment.find(sDeprecated) != std::string::npos) {
+        mAnnotationBitMask |= kDeprecated;
     }
 
-    if (comment.find(sSystemApi) != std::string::npos && !mSystemApi) {
-        mSystemApi = true;
-        if (!mAnnotations.empty()) {
-            mAnnotations += "\n";
-        }
-        mAnnotations += mPrefix;
-        mAnnotations += "@android.annotations.SystemApi";
+    if (comment.find(sSystemApi) != std::string::npos) {
+        mAnnotationBitMask |= kSystemApi;
     }
 
-    if (mComment.empty()) {
-        mComment += mPrefix;
-        mComment += "/**";
+    if (!mHasComments) {
+        mHasComments = true;
+        mComment << "/**";
     }
 
-    mComment += "\n";
-    mComment += mPrefix;
-    mComment += " * ";
-    mComment += std::move(comment);
+    mComment << "\n" << " * " << std::move(comment);
 }
 
 void AnnotationProcessor::appendComment(const StringPiece16& comment) {
@@ -73,17 +60,22 @@ void AnnotationProcessor::appendComment(const StringPiece& comment) {
     }
 }
 
-std::string AnnotationProcessor::buildComment() {
-    if (!mComment.empty()) {
-        mComment += "\n";
-        mComment += mPrefix;
-        mComment += " */";
+void AnnotationProcessor::writeToStream(std::ostream* out, const StringPiece& prefix) {
+    if (mHasComments) {
+        std::string result = mComment.str();
+        for (StringPiece line : util::tokenize<char>(result, '\n')) {
+           *out << prefix << line << "\n";
+        }
+        *out << prefix << " */" << "\n";
     }
-    return std::move(mComment);
-}
 
-std::string AnnotationProcessor::buildAnnotations() {
-    return std::move(mAnnotations);
+    if (mAnnotationBitMask & kDeprecated) {
+        *out << prefix << "@Deprecated\n";
+    }
+
+    if (mAnnotationBitMask & kSystemApi) {
+        *out << prefix << "@android.annotation.SystemApi\n";
+    }
 }
 
 } // namespace aapt
diff --git a/tools/aapt2/java/AnnotationProcessor.h b/tools/aapt2/java/AnnotationProcessor.h
index 81a6f6e42759..e7f2be0dc12b 100644
--- a/tools/aapt2/java/AnnotationProcessor.h
+++ b/tools/aapt2/java/AnnotationProcessor.h
@@ -19,6 +19,7 @@
 
 #include "util/StringPiece.h"
 
+#include <sstream>
 #include <string>
 
 namespace aapt {
@@ -54,13 +55,6 @@ namespace aapt {
 class AnnotationProcessor {
 public:
     /**
-     * Creates an AnnotationProcessor with a given prefix for each line generated.
-     * This is usually a set of spaces for indentation.
-     */
-    AnnotationProcessor(const StringPiece& prefix) : mPrefix(prefix.toString()) {
-    }
-
-    /**
      * Adds more comments. Since resources can have various values with different configurations,
      * we need to collect all the comments.
      */
@@ -68,23 +62,20 @@ public:
     void appendComment(const StringPiece& comment);
 
     /**
-     * Finishes the comment and moves it to the caller. Subsequent calls to buildComment() have
-     * undefined results.
+     * Writes the comments and annotations to the stream, with the given prefix before each line.
      */
-    std::string buildComment();
-
-    /**
-     * Finishes the annotation and moves it to the caller. Subsequent calls to buildAnnotations()
-     * have undefined results.
-     */
-    std::string buildAnnotations();
+    void writeToStream(std::ostream* out, const StringPiece& prefix);
 
 private:
-    std::string mPrefix;
-    std::string mComment;
-    std::string mAnnotations;
-    bool mDeprecated = false;
-    bool mSystemApi = false;
+    enum : uint32_t {
+        kDeprecated = 0x01,
+        kSystemApi = 0x02,
+    };
+
+    std::stringstream mComment;
+    std::stringstream mAnnotations;
+    bool mHasComments = false;
+    uint32_t mAnnotationBitMask = 0;
 
     void appendCommentLine(const std::string& line);
 };
diff --git a/tools/aapt2/java/AnnotationProcessor_test.cpp b/tools/aapt2/java/AnnotationProcessor_test.cpp
new file mode 100644
index 000000000000..d5a2b38bcbc4
--- /dev/null
+++ b/tools/aapt2/java/AnnotationProcessor_test.cpp
@@ -0,0 +1,80 @@
+/*
+ * Copyright (C) 2015 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "ResourceParser.h"
+#include "ResourceTable.h"
+#include "ResourceValues.h"
+#include "XmlPullParser.h"
+
+#include "java/AnnotationProcessor.h"
+
+#include "test/Builders.h"
+#include "test/Context.h"
+
+#include <gtest/gtest.h>
+
+namespace aapt {
+
+struct AnnotationProcessorTest : public ::testing::Test {
+    std::unique_ptr<IAaptContext> mContext;
+    ResourceTable mTable;
+
+    void SetUp() override {
+        mContext = test::ContextBuilder().build();
+    }
+
+    ::testing::AssertionResult parse(const StringPiece& str) {
+        ResourceParserOptions options;
+        ResourceParser parser(mContext->getDiagnostics(), &mTable, Source{}, ConfigDescription{},
+                              options);
+        std::stringstream in;
+        in << "<?xml version=\"1.0\" encoding=\"utf-8\"?>\n" << str;
+        XmlPullParser xmlParser(in);
+        if (parser.parse(&xmlParser)) {
+            return ::testing::AssertionSuccess();
+        }
+        return ::testing::AssertionFailure();
+    }
+};
+
+TEST_F(AnnotationProcessorTest, EmitsDeprecated) {
+    ASSERT_TRUE(parse(R"EOF(
+    <resources>
+      <declare-styleable name="foo">      
+        <!-- Some comment, and it should contain
+             a marker word, something that marks
+             this resource as nor needed.
+             {@deprecated That's the marker! } -->
+        <attr name="autoText" format="boolean" />
+      </declare-styleable>
+    </resources>)EOF"));
+
+    Attribute* attr = test::getValue<Attribute>(&mTable, u"@attr/autoText");
+    ASSERT_NE(nullptr, attr);
+
+    AnnotationProcessor processor;
+    processor.appendComment(attr->getComment());
+
+    std::stringstream result;
+    processor.writeToStream(&result, "");
+    std::string annotations = result.str();
+
+    EXPECT_NE(std::string::npos, annotations.find("@Deprecated"));
+}
+
+} // namespace aapt
+
+
diff --git a/tools/aapt2/java/ClassDefinitionWriter.h b/tools/aapt2/java/ClassDefinitionWriter.h
new file mode 100644
index 000000000000..b8886f90c6c5
--- /dev/null
+++ b/tools/aapt2/java/ClassDefinitionWriter.h
@@ -0,0 +1,141 @@
+/*
+ * Copyright (C) 2015 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef AAPT_JAVA_CLASSDEFINITION_H
+#define AAPT_JAVA_CLASSDEFINITION_H
+
+#include "java/AnnotationProcessor.h"
+#include "util/StringPiece.h"
+#include "util/Util.h"
+
+#include <sstream>
+#include <string>
+
+namespace aapt {
+
+struct ClassDefinitionWriterOptions {
+    bool useFinalQualifier = false;
+    bool forceCreationIfEmpty = false;
+};
+
+/**
+ * Writes a class for use in R.java or Manifest.java.
+ */
+class ClassDefinitionWriter {
+public:
+    ClassDefinitionWriter(const StringPiece& name, const ClassDefinitionWriterOptions& options) :
+            mName(name.toString()), mOptions(options), mStarted(false) {
+    }
+
+    ClassDefinitionWriter(const StringPiece16& name, const ClassDefinitionWriterOptions& options) :
+            mName(util::utf16ToUtf8(name)), mOptions(options), mStarted(false) {
+    }
+
+    void addIntMember(const StringPiece& name, AnnotationProcessor* processor,
+                      const uint32_t val) {
+        ensureClassDeclaration();
+        if (processor) {
+            processor->writeToStream(&mOut, kIndent);
+        }
+        mOut << kIndent << "public static " << (mOptions.useFinalQualifier ? "final " : "")
+             << "int " << name << "=" << val << ";\n";
+    }
+
+    void addStringMember(const StringPiece16& name, AnnotationProcessor* processor,
+                         const StringPiece16& val) {
+        ensureClassDeclaration();
+        if (processor) {
+            processor->writeToStream(&mOut, kIndent);
+        }
+        mOut << kIndent << "public static " << (mOptions.useFinalQualifier ? "final " : "")
+             << "String " << name << "=\"" << val << "\";\n";
+    }
+
+    void addResourceMember(const StringPiece16& name, AnnotationProcessor* processor,
+                           const ResourceId id) {
+        ensureClassDeclaration();
+        if (processor) {
+            processor->writeToStream(&mOut, kIndent);
+        }
+        mOut << kIndent << "public static " << (mOptions.useFinalQualifier ? "final " : "")
+             << "int " << name << "=" << id <<";\n";
+    }
+
+    template <typename Iterator, typename FieldAccessorFunc>
+    void addArrayMember(const StringPiece16& name, AnnotationProcessor* processor,
+                        const Iterator begin, const Iterator end, FieldAccessorFunc f) {
+        ensureClassDeclaration();
+        if (processor) {
+            processor->writeToStream(&mOut, kIndent);
+        }
+        mOut << kIndent << "public static final int[] " << name << "={";
+
+        for (Iterator current = begin; current != end; ++current) {
+            if (std::distance(begin, current) % kAttribsPerLine == 0) {
+                mOut << "\n" << kIndent << kIndent;
+            }
+
+            mOut << f(*current);
+            if (std::distance(current, end) > 1) {
+                mOut << ", ";
+            }
+        }
+        mOut << "\n" << kIndent <<"};\n";
+    }
+
+    void writeToStream(std::ostream* out, const StringPiece& prefix,
+                       AnnotationProcessor* processor=nullptr) {
+        if (mOptions.forceCreationIfEmpty) {
+            ensureClassDeclaration();
+        }
+
+        if (!mStarted) {
+            return;
+        }
+
+        if (processor) {
+            processor->writeToStream(out, prefix);
+        }
+
+        std::string result = mOut.str();
+        for (StringPiece line : util::tokenize<char>(result, '\n')) {
+            *out << prefix << line << "\n";
+        }
+        *out << prefix << "}\n";
+    }
+
+private:
+    constexpr static const char* kIndent = "  ";
+
+    // The number of attributes to emit per line in a Styleable array.
+    constexpr static size_t kAttribsPerLine = 4;
+
+    void ensureClassDeclaration() {
+        if (!mStarted) {
+            mStarted = true;
+            mOut << "public static final class " << mName << " {\n";
+        }
+    }
+
+    std::stringstream mOut;
+    std::string mName;
+    ClassDefinitionWriterOptions mOptions;
+    bool mStarted;
+};
+
+} // namespace aapt
+
+#endif /* AAPT_JAVA_CLASSDEFINITION_H */
diff --git a/tools/aapt2/java/JavaClassGenerator.cpp b/tools/aapt2/java/JavaClassGenerator.cpp
index dfd2ef6f5372..7280f3a968a0 100644
--- a/tools/aapt2/java/JavaClassGenerator.cpp
+++ b/tools/aapt2/java/JavaClassGenerator.cpp
@@ -21,7 +21,9 @@
 #include "ValueVisitor.h"
 
 #include "java/AnnotationProcessor.h"
+#include "java/ClassDefinitionWriter.h"
 #include "java/JavaClassGenerator.h"
+#include "util/Comparators.h"
 #include "util/StringPiece.h"
 
 #include <algorithm>
@@ -32,9 +34,6 @@
 
 namespace aapt {
 
-// The number of attributes to emit per line in a Styleable array.
-constexpr size_t kAttribsPerLine = 4;
-
 JavaClassGenerator::JavaClassGenerator(ResourceTable* table, JavaClassGeneratorOptions options) :
         mTable(table), mOptions(options) {
 }
@@ -92,12 +91,11 @@ bool JavaClassGenerator::skipSymbol(SymbolState state) {
     return true;
 }
 
-void JavaClassGenerator::generateStyleable(const StringPiece16& packageNameToGenerate,
-                                           const std::u16string& entryName,
-                                           const Styleable* styleable,
-                                           std::ostream* out) {
-    const StringPiece finalModifier = mOptions.useFinal ? " final" : "";
-
+void JavaClassGenerator::writeStyleableEntryForClass(ClassDefinitionWriter* outClassDef,
+                                                     AnnotationProcessor* processor,
+                                                     const StringPiece16& packageNameToGenerate,
+                                                     const std::u16string& entryName,
+                                                     const Styleable* styleable) {
     // This must be sorted by resource ID.
     std::vector<std::pair<ResourceId, ResourceNameRef>> sortedAttributes;
     sortedAttributes.reserve(styleable->entries.size());
@@ -110,36 +108,31 @@ void JavaClassGenerator::generateStyleable(const StringPiece16& packageNameToGen
     }
     std::sort(sortedAttributes.begin(), sortedAttributes.end());
 
-    // First we emit the array containing the IDs of each attribute.
-    *out << "    "
-         << "public static final int[] " << transform(entryName) << " = {";
-
-    const size_t attrCount = sortedAttributes.size();
-    for (size_t i = 0; i < attrCount; i++) {
-        if (i % kAttribsPerLine == 0) {
-            *out << "\n      ";
-        }
+    auto accessorFunc = [](const std::pair<ResourceId, ResourceNameRef>& a) -> ResourceId {
+        return a.first;
+    };
 
-        *out << sortedAttributes[i].first;
-        if (i != attrCount - 1) {
-            *out << ", ";
-        }
-    }
-    *out << "\n    };\n";
+    // First we emit the array containing the IDs of each attribute.
+    outClassDef->addArrayMember(transform(entryName), 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++) {
-        *out << "    "
-             << "public static" << finalModifier
-             << " int " << transform(entryName);
+        std::stringstream name;
+        name << transform(entryName);
 
         // 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) {
-            *out << "_" << transform(itemName.package);
+            name << "_" << transform(itemName.package);
         }
-        *out << "_" << transform(itemName.entry) << " = " << i << ";\n";
+        name << "_" << transform(itemName.entry);
+
+        outClassDef->addIntMember(name.str(), nullptr, i);
     }
 }
 
@@ -155,8 +148,8 @@ static void addAttributeFormatDoc(AnnotationProcessor* processor, Attribute* att
 
     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;");
+                "<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) {
@@ -222,14 +215,10 @@ static void addAttributeFormatDoc(AnnotationProcessor* processor, Attribute* att
     }
 }
 
-bool JavaClassGenerator::generateType(const StringPiece16& packageNameToGenerate,
-                                      const ResourceTablePackage* package,
-                                      const ResourceTableType* type,
-                                      std::ostream* out) {
-    const StringPiece finalModifier = mOptions.useFinal ? " final" : "";
-
-    std::u16string unmangledPackage;
-    std::u16string unmangledName;
+bool JavaClassGenerator::writeEntriesForClass(ClassDefinitionWriter* outClassDef,
+                                              const StringPiece16& packageNameToGenerate,
+                                              const ResourceTablePackage* package,
+                                              const ResourceTableType* type) {
     for (const auto& entry : type->entries) {
         if (skipSymbol(entry->symbolStatus.state)) {
             continue;
@@ -238,7 +227,8 @@ bool JavaClassGenerator::generateType(const StringPiece16& packageNameToGenerate
         ResourceId id(package->id.value(), type->id.value(), entry->id.value());
         assert(id.isValid());
 
-        unmangledName = entry->name;
+        std::u16string unmangledPackage;
+        std::u16string unmangledName = entry->name;
         if (NameMangler::unmangle(&unmangledName, &unmangledPackage)) {
             // The entry name was mangled, and we successfully unmangled it.
             // Check that we want to emit this symbol.
@@ -246,12 +236,10 @@ bool JavaClassGenerator::generateType(const StringPiece16& packageNameToGenerate
                 // Skip the entry if it doesn't belong to the package we're writing.
                 continue;
             }
-        } else {
-            if (packageNameToGenerate != package->name) {
-                // We are processing a mangled package name,
-                // but this is a non-mangled resource.
-                continue;
-            }
+        } else if (packageNameToGenerate != package->name) {
+            // We are processing a mangled package name,
+            // but this is a non-mangled resource.
+            continue;
         }
 
         if (!isValidSymbol(unmangledName)) {
@@ -262,39 +250,33 @@ bool JavaClassGenerator::generateType(const StringPiece16& packageNameToGenerate
             return false;
         }
 
-        if (type->type == ResourceType::kStyleable) {
-            assert(!entry->values.empty());
-            generateStyleable(packageNameToGenerate, unmangledName, static_cast<const Styleable*>(
-                    entry->values.front().value.get()), out);
-        } else {
-            AnnotationProcessor processor("    ");
-            if (entry->symbolStatus.state != SymbolState::kUndefined) {
-                processor.appendComment(entry->symbolStatus.comment);
-            }
+        // Build the comments and annotations for this entry.
 
-            for (const auto& configValue : entry->values) {
-                processor.appendComment(configValue.value->getComment());
-            }
-
-            if (!entry->values.empty()) {
-                if (Attribute* attr = valueCast<Attribute>(entry->values.front().value.get())) {
-                    // We list out the available values for the given attribute.
-                    addAttributeFormatDoc(&processor, attr);
-                }
-            }
+        AnnotationProcessor processor;
+        if (entry->symbolStatus.state != SymbolState::kUndefined) {
+            processor.appendComment(entry->symbolStatus.comment);
+        }
 
-            std::string comment = processor.buildComment();
-            if (!comment.empty()) {
-                *out << comment << "\n";
-            }
+        for (const auto& configValue : entry->values) {
+            processor.appendComment(configValue.value->getComment());
+        }
 
-            std::string annotations = processor.buildAnnotations();
-            if (!annotations.empty()) {
-                *out << annotations << "\n";
+        // If this is an Attribute, append the format Javadoc.
+        if (!entry->values.empty()) {
+            if (Attribute* attr = valueCast<Attribute>(entry->values.front().value.get())) {
+                // We list out the available values for the given attribute.
+                addAttributeFormatDoc(&processor, attr);
             }
+        }
 
-            *out << "    " << "public static" << finalModifier
-                 << " int " << transform(unmangledName) << " = " << id << ";\n";
+        if (type->type == ResourceType::kStyleable) {
+            assert(!entry->values.empty());
+            const Styleable* styleable = static_cast<const Styleable*>(
+                    entry->values.front().value.get());
+            writeStyleableEntryForClass(outClassDef, &processor, packageNameToGenerate,
+                                        unmangledName, styleable);
+        } else {
+            outClassDef->addResourceMember(transform(unmangledName), &processor, id);
         }
     }
     return true;
@@ -312,17 +294,42 @@ bool JavaClassGenerator::generate(const StringPiece16& packageNameToGenerate,
 
     for (const auto& package : mTable->packages) {
         for (const auto& type : package->types) {
-            StringPiece16 typeStr;
             if (type->type == ResourceType::kAttrPrivate) {
-                typeStr = toString(ResourceType::kAttr);
-            } else {
-                typeStr = toString(type->type);
+                continue;
             }
-            *out << "  public static final class " << typeStr << " {\n";
-            if (!generateType(packageNameToGenerate, package.get(), type.get(), out)) {
+
+            ClassDefinitionWriterOptions classOptions;
+            classOptions.useFinalQualifier = mOptions.useFinal;
+            classOptions.forceCreationIfEmpty =
+                    (mOptions.types == JavaClassGeneratorOptions::SymbolTypes::kPublic);
+            ClassDefinitionWriter classDef(toString(type->type), classOptions);
+            bool result = writeEntriesForClass(&classDef, packageNameToGenerate,
+                                               package.get(), type.get());
+            if (!result) {
                 return false;
             }
-            *out << "  }\n";
+
+            if (type->type == ResourceType::kAttr) {
+                // Also include private attributes in this same class.
+                auto iter = std::lower_bound(package->types.begin(), package->types.end(),
+                                             ResourceType::kAttrPrivate, cmp::lessThanType);
+                if (iter != package->types.end() && (*iter)->type == ResourceType::kAttrPrivate) {
+                    result = writeEntriesForClass(&classDef, packageNameToGenerate,
+                                                  package.get(), iter->get());
+                    if (!result) {
+                        return false;
+                    }
+                }
+            }
+
+            AnnotationProcessor processor;
+            if (type->type == ResourceType::kStyleable &&
+                    mOptions.types == JavaClassGeneratorOptions::SymbolTypes::kPublic) {
+                // When generating a public R class, we don't want Styleable to be part of the API.
+                // It is only emitted for documentation purposes.
+                processor.appendComment("@doconly");
+            }
+            classDef.writeToStream(out, "  ", &processor);
         }
     }
 
diff --git a/tools/aapt2/java/JavaClassGenerator.h b/tools/aapt2/java/JavaClassGenerator.h
index e53a765ae0d8..023d6d635f8c 100644
--- a/tools/aapt2/java/JavaClassGenerator.h
+++ b/tools/aapt2/java/JavaClassGenerator.h
@@ -27,6 +27,9 @@
 
 namespace aapt {
 
+class AnnotationProcessor;
+class ClassDefinitionWriter;
+
 struct JavaClassGeneratorOptions {
     /*
      * Specifies whether to use the 'final' modifier
@@ -40,9 +43,6 @@ struct JavaClassGeneratorOptions {
         kPublic,
     };
 
-    /*
-     *
-     */
     SymbolTypes types = SymbolTypes::kAll;
 };
 
@@ -69,15 +69,16 @@ public:
     const std::string& getError() const;
 
 private:
-    bool generateType(const StringPiece16& packageNameToGenerate,
-                      const ResourceTablePackage* package,
-                      const ResourceTableType* type,
-                      std::ostream* out);
-
-    void generateStyleable(const StringPiece16& packageNameToGenerate,
-                           const std::u16string& entryName,
-                           const Styleable* styleable,
-                           std::ostream* out);
+    bool writeEntriesForClass(ClassDefinitionWriter* outClassDef,
+                              const StringPiece16& packageNameToGenerate,
+                              const ResourceTablePackage* package,
+                              const ResourceTableType* type);
+
+    void writeStyleableEntryForClass(ClassDefinitionWriter* outClassDef,
+                                     AnnotationProcessor* processor,
+                                     const StringPiece16& packageNameToGenerate,
+                                     const std::u16string& entryName,
+                                     const Styleable* styleable);
 
     bool skipSymbol(SymbolState state);
 
diff --git a/tools/aapt2/java/JavaClassGenerator_test.cpp b/tools/aapt2/java/JavaClassGenerator_test.cpp
index 2dc387bea8b9..e9e788167966 100644
--- a/tools/aapt2/java/JavaClassGenerator_test.cpp
+++ b/tools/aapt2/java/JavaClassGenerator_test.cpp
@@ -56,13 +56,13 @@ TEST(JavaClassGeneratorTest, TransformInvalidJavaIdentifierCharacter) {
     std::string output = out.str();
 
     EXPECT_NE(std::string::npos,
-              output.find("public static final int hey_man = 0x01020000;"));
+              output.find("public static final int hey_man=0x01020000;"));
 
     EXPECT_NE(std::string::npos,
-              output.find("public static final int[] hey_dude = {"));
+              output.find("public static final int[] hey_dude={"));
 
     EXPECT_NE(std::string::npos,
-              output.find("public static final int hey_dude_cool_attr = 0;"));
+              output.find("public static final int hey_dude_cool_attr=0;"));
 }
 
 TEST(JavaClassGeneratorTest, CorrectPackageNameIsUsed) {
@@ -78,7 +78,7 @@ TEST(JavaClassGeneratorTest, CorrectPackageNameIsUsed) {
 
     std::string output = out.str();
     EXPECT_NE(std::string::npos, output.find("package com.android.internal;"));
-    EXPECT_NE(std::string::npos, output.find("public static final int one = 0x01020000;"));
+    EXPECT_NE(std::string::npos, output.find("public static final int one=0x01020000;"));
     EXPECT_EQ(std::string::npos, output.find("two"));
     EXPECT_EQ(std::string::npos, output.find("com_foo$two"));
 }
@@ -86,6 +86,7 @@ TEST(JavaClassGeneratorTest, CorrectPackageNameIsUsed) {
 TEST(JavaClassGeneratorTest, AttrPrivateIsWrittenAsAttr) {
     std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder()
             .setPackageId(u"android", 0x01)
+            .addSimple(u"@android:attr/two", ResourceId(0x01010001))
             .addSimple(u"@android:^attr-private/one", ResourceId(0x01010000))
             .build();
 
@@ -116,7 +117,7 @@ TEST(JavaClassGeneratorTest, OnlyWritePublicResources) {
         std::stringstream out;
         ASSERT_TRUE(generator.generate(u"android", &out));
         std::string output = out.str();
-        EXPECT_NE(std::string::npos, output.find("public static final int one = 0x01020000;"));
+        EXPECT_NE(std::string::npos, output.find("public static final int one=0x01020000;"));
         EXPECT_EQ(std::string::npos, output.find("two"));
         EXPECT_EQ(std::string::npos, output.find("three"));
     }
@@ -127,8 +128,8 @@ TEST(JavaClassGeneratorTest, OnlyWritePublicResources) {
         std::stringstream out;
         ASSERT_TRUE(generator.generate(u"android", &out));
         std::string output = out.str();
-        EXPECT_NE(std::string::npos, output.find("public static final int one = 0x01020000;"));
-        EXPECT_NE(std::string::npos, output.find("public static final int two = 0x01020001;"));
+        EXPECT_NE(std::string::npos, output.find("public static final int one=0x01020000;"));
+        EXPECT_NE(std::string::npos, output.find("public static final int two=0x01020001;"));
         EXPECT_EQ(std::string::npos, output.find("three"));
     }
 
@@ -138,9 +139,9 @@ TEST(JavaClassGeneratorTest, OnlyWritePublicResources) {
         std::stringstream out;
         ASSERT_TRUE(generator.generate(u"android", &out));
         std::string output = out.str();
-        EXPECT_NE(std::string::npos, output.find("public static final int one = 0x01020000;"));
-        EXPECT_NE(std::string::npos, output.find("public static final int two = 0x01020001;"));
-        EXPECT_NE(std::string::npos, output.find("public static final int three = 0x01020002;"));
+        EXPECT_NE(std::string::npos, output.find("public static final int one=0x01020000;"));
+        EXPECT_NE(std::string::npos, output.find("public static final int two=0x01020001;"));
+        EXPECT_NE(std::string::npos, output.find("public static final int three=0x01020002;"));
     }
 }
 
@@ -194,8 +195,8 @@ TEST(JavaClassGeneratorTest, EmitOtherPackagesAttributesInStyleable) {
     EXPECT_TRUE(generator.generate(u"android", &out));
 
     std::string output = out.str();
-    EXPECT_NE(std::string::npos, output.find("int foo_bar ="));
-    EXPECT_NE(std::string::npos, output.find("int foo_com_lib_bar ="));
+    EXPECT_NE(std::string::npos, output.find("int foo_bar="));
+    EXPECT_NE(std::string::npos, output.find("int foo_com_lib_bar="));
 }
 
 TEST(JavaClassGeneratorTest, CommentsForSimpleResourcesArePresent) {
@@ -218,7 +219,7 @@ TEST(JavaClassGeneratorTest, CommentsForSimpleResourcesArePresent) {
      * @deprecated
      */
     @Deprecated
-    public static final int foo = 0x01010000;)EOF"));
+    public static final int foo=0x01010000;)EOF"));
 }
 
 TEST(JavaClassGeneratorTest, CommentsForEnumAndFlagAttributesArePresent) {
diff --git a/tools/aapt2/java/ManifestClassGenerator.cpp b/tools/aapt2/java/ManifestClassGenerator.cpp
index 901a344881fc..d963d8948f8d 100644
--- a/tools/aapt2/java/ManifestClassGenerator.cpp
+++ b/tools/aapt2/java/ManifestClassGenerator.cpp
@@ -18,6 +18,7 @@
 #include "XmlDom.h"
 
 #include "java/AnnotationProcessor.h"
+#include "java/ClassDefinitionWriter.h"
 #include "java/ManifestClassGenerator.h"
 #include "util/Maybe.h"
 
@@ -58,8 +59,8 @@ static Maybe<StringPiece16> extractJavaIdentifier(IDiagnostics* diag, const Sour
     return result;
 }
 
-static bool writeSymbol(IDiagnostics* diag, const Source& source, xml::Element* el,
-                        std::ostream* out) {
+static bool writeSymbol(IDiagnostics* diag, ClassDefinitionWriter* outClassDef, const Source& source,
+                        xml::Element* el) {
     xml::Attribute* attr = el->findAttribute(xml::kSchemaAndroid, u"name");
     if (!attr) {
         diag->error(DiagMessage(source) << "<" << el->name << "> must define 'android:name'");
@@ -72,18 +73,9 @@ static bool writeSymbol(IDiagnostics* diag, const Source& source, xml::Element*
         return false;
     }
 
-    *out << "\n";
-
-    if (!util::trimWhitespace(el->comment).empty()) {
-        AnnotationProcessor processor("    ");
-        processor.appendComment(el->comment);
-        *out << processor.buildComment() << "\n";
-        std::string annotations = processor.buildAnnotations();
-        if (!annotations.empty()) {
-            *out << annotations << "\n";
-        }
-    }
-    *out << "    public static final String " << result.value() << "=\"" << attr->value << "\";\n";
+    AnnotationProcessor processor;
+    processor.appendComment(el->comment);
+    outClassDef->addStringMember(result.value(), &processor, attr->value);
     return true;
 }
 
@@ -100,29 +92,32 @@ bool ManifestClassGenerator::generate(IDiagnostics* diag, const StringPiece16& p
     }
 
     *out << "package " << package << ";\n\n"
-         << "public class Manifest {\n";
+         << "public final class Manifest {\n";
 
     bool error = false;
     std::vector<xml::Element*> children = el->getChildElements();
 
+    ClassDefinitionWriterOptions classOptions;
+    classOptions.useFinalQualifier = true;
+    classOptions.forceCreationIfEmpty = false;
 
     // First write out permissions.
-    *out << "  public static class permission {\n";
+    ClassDefinitionWriter classDef("permission", classOptions);
     for (xml::Element* childEl : children) {
         if (childEl->namespaceUri.empty() && childEl->name == u"permission") {
-            error |= !writeSymbol(diag, res->file.source, childEl, out);
+            error |= !writeSymbol(diag, &classDef, res->file.source, childEl);
         }
     }
-    *out << "  }\n";
+    classDef.writeToStream(out, "  ");
 
     // Next write out permission groups.
-    *out << "  public static class permission_group {\n";
+    classDef = ClassDefinitionWriter("permission_group", classOptions);
     for (xml::Element* childEl : children) {
         if (childEl->namespaceUri.empty() && childEl->name == u"permission-group") {
-            error |= !writeSymbol(diag, res->file.source, childEl, out);
+            error |= !writeSymbol(diag, &classDef, res->file.source, childEl);
         }
     }
-    *out << "  }\n";
+    classDef.writeToStream(out, "  ");
 
     *out << "}\n";
     return !error;
diff --git a/tools/aapt2/java/ManifestClassGenerator_test.cpp b/tools/aapt2/java/ManifestClassGenerator_test.cpp
index 1b5bc0586f43..4081287a1852 100644
--- a/tools/aapt2/java/ManifestClassGenerator_test.cpp
+++ b/tools/aapt2/java/ManifestClassGenerator_test.cpp
@@ -39,8 +39,9 @@ TEST(ManifestClassGeneratorTest, NameIsProperlyGeneratedFromSymbol) {
 
     std::string actual = out.str();
 
-    const size_t permissionClassPos = actual.find("public static class permission {");
-    const size_t permissionGroupClassPos = actual.find("public static class permission_group {");
+    const size_t permissionClassPos = actual.find("public static final class permission {");
+    const size_t permissionGroupClassPos =
+            actual.find("public static final class permission_group {");
     ASSERT_NE(std::string::npos, permissionClassPos);
     ASSERT_NE(std::string::npos, permissionGroupClassPos);
 
@@ -113,7 +114,7 @@ R"EOF(    /**
      * @hide
      * @SystemApi
      */
-    @android.annotations.SystemApi
+    @android.annotation.SystemApi
     public static final String SECRET="android.permission.SECRET";)EOF"));
 }