Merge "AAPT2: Clean up R JavaDoc generation" into nyc-dev

tools/aapt2/java/AnnotationProcessor.cpp

@@ -21,7 +21,7 @@
 
 namespace aapt {
 
-void AnnotationProcessor::appendCommentLine(const std::string& comment) {
+void AnnotationProcessor::appendCommentLine(std::string& comment) {
     static const std::string sDeprecated = "@deprecated";
     static const std::string sSystemApi = "@SystemApi";
 
@@ -29,8 +29,14 @@ void AnnotationProcessor::appendCommentLine(const std::string& comment) {
         mAnnotationBitMask |= kDeprecated;
     }
 
-    if (comment.find(sSystemApi) != std::string::npos) {
+    std::string::size_type idx = comment.find(sSystemApi);
+    if (idx != std::string::npos) {
         mAnnotationBitMask |= kSystemApi;
+        comment.erase(comment.begin() + idx, comment.begin() + idx + sSystemApi.size());
+    }
+
+    if (util::trimWhitespace(comment).empty()) {
+        return;
     }
 
     if (!mHasComments) {
@@ -46,7 +52,8 @@ void AnnotationProcessor::appendComment(const StringPiece16& comment) {
     for (StringPiece16 line : util::tokenize(comment, u'\n')) {
         line = util::trimWhitespace(line);
         if (!line.empty()) {
-            appendCommentLine(util::utf16ToUtf8(line));
+            std::string utf8Line = util::utf16ToUtf8(line);
+            appendCommentLine(utf8Line);
         }
     }
 }
@@ -55,7 +62,8 @@ void AnnotationProcessor::appendComment(const StringPiece& comment) {
     for (StringPiece line : util::tokenize(comment, '\n')) {
         line = util::trimWhitespace(line);
         if (!line.empty()) {
-            appendCommentLine(line.toString());
+            std::string utf8Line = line.toString();
+            appendCommentLine(utf8Line);
         }
     }
 }

tools/aapt2/java/AnnotationProcessor.h

@@ -43,7 +43,6 @@ namespace aapt {
  *  /\*
  *   * This is meant to be hidden because
  *   * It is system api. Also it is @deprecated
- *   * @SystemApi
  *   *\/
  *
  * Output Annotations:
@@ -79,7 +78,7 @@ private:
     bool mHasComments = false;
     uint32_t mAnnotationBitMask = 0;
 
-    void appendCommentLine(const std::string& line);
+    void appendCommentLine(std::string& line);
 };
 
 } // namespace aapt

tools/aapt2/java/AnnotationProcessor_test.cpp

@@ -14,59 +14,18 @@
  * limitations under the License.
  */
 
-#include "ResourceParser.h"
-#include "ResourceTable.h"
-#include "ResourceValues.h"
 #include "java/AnnotationProcessor.h"
-#include "test/Builders.h"
+#include "test/Test.h"
-#include "test/Context.h"
-#include "xml/XmlPullParser.h"
-
-#include <gtest/gtest.h>
 
 namespace aapt {
 
-struct AnnotationProcessorTest : public ::testing::Test {
+TEST(AnnotationProcessorTest, EmitsDeprecated) {
-    std::unique_ptr<IAaptContext> mContext;
+    const char* comment = "Some comment, and it should contain a marker word, "
-    ResourceTable mTable;
+                          "something that marks this resource as nor needed. "
-
+                          "{@deprecated That's the marker! }";
-    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;
-        xml::XmlPullParser xmlParser(in);
-        if (parser.parse(&xmlParser)) {
-            return ::testing::AssertionSuccess();
-        }
-        return ::testing::AssertionFailure();
-    }
-};
-
-TEST_F(AnnotationProcessorTest, EmitsDeprecated) {
-    const char* xmlInput = 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";
-
-    ASSERT_TRUE(parse(xmlInput));
-
-    Attribute* attr = test::getValue<Attribute>(&mTable, u"@attr/autoText");
-    ASSERT_NE(nullptr, attr);
 
     AnnotationProcessor processor;
-    processor.appendComment(attr->getComment());
+    processor.appendComment(comment);
 
     std::stringstream result;
     processor.writeToStream(&result, "");
@@ -75,6 +34,19 @@ TEST_F(AnnotationProcessorTest, EmitsDeprecated) {
     EXPECT_NE(std::string::npos, annotations.find("@Deprecated"));
 }
 
+TEST(AnnotationProcessorTest, EmitsSystemApiAnnotationAndRemovesFromComment) {
+    AnnotationProcessor processor;
+    processor.appendComment("@SystemApi This is a system API");
+
+    std::stringstream result;
+    processor.writeToStream(&result, "");
+    std::string annotations = result.str();
+
+    EXPECT_NE(std::string::npos, annotations.find("@android.annotation.SystemApi"));
+    EXPECT_EQ(std::string::npos, annotations.find("@SystemApi"));
+    EXPECT_NE(std::string::npos, annotations.find("This is a system API"));
+}
+
 } // namespace aapt
 
 

tools/aapt2/java/ClassDefinition.h

@@ -125,7 +125,7 @@ public:
     void writeToStream(const StringPiece& prefix, bool final, std::ostream* out) const override {
         ClassMember::writeToStream(prefix, final, out);
 
-        *out << "public static final int[] " << mName << "={";
+        *out << prefix << "public static final int[] " << mName << "={";
 
         const auto begin = mElements.begin();
         const auto end = mElements.end();

tools/aapt2/java/JavaClassGenerator.cpp

@@ -188,8 +188,8 @@ bool JavaClassGenerator::skipSymbol(SymbolState state) {
 
 struct StyleableAttr {
     const Reference* attrRef;
-    std::shared_ptr<Attribute> attribute;
     std::string fieldName;
+    std::unique_ptr<SymbolTable::Symbol> symbol;
 };
 
 static bool lessStyleableAttr(const StyleableAttr& lhs, const StyleableAttr& rhs) {
@@ -245,8 +245,9 @@ void JavaClassGenerator::addMembersToStyleableClass(const StringPiece16& package
         // legal values for this attribute.
         const SymbolTable::Symbol* symbol = mContext->getExternalSymbols()->findByReference(
                 mangledReference);
-        if (symbol) {
+        if (symbol && symbol->attribute) {
-            styleableAttr.attribute = symbol->attribute;
+            // Copy the symbol data structure because the returned instance can be destroyed.
+            styleableAttr.symbol = util::make_unique<SymbolTable::Symbol>(*symbol);
         }
         sortedAttributes.push_back(std::move(styleableAttr));
     }
@@ -273,6 +274,16 @@ void JavaClassGenerator::addMembersToStyleableClass(const StringPiece16& package
                 "<tr><th>Attribute</th><th>Description</th></tr>\n";
 
         for (const StyleableAttr& entry : sortedAttributes) {
+            if (!entry.symbol) {
+                continue;
+            }
+
+            if (mOptions.types == JavaClassGeneratorOptions::SymbolTypes::kPublic &&
+                    !entry.symbol->isPublic) {
+                // Don't write entries for non-public attributes.
+                continue;
+            }
+
             const ResourceName& attrName = entry.attrRef->name.value();
             styleableComment << "<tr><td>";
             styleableComment << "<code>{@link #"
@@ -284,14 +295,30 @@ void JavaClassGenerator::addMembersToStyleableClass(const StringPiece16& package
             styleableComment << "</td>";
 
             styleableComment << "<td>";
-            if (entry.attribute) {
+
-                styleableComment << entry.attribute->getComment();
+            // Only use the comment up until the first '.'. This is to stay compatible with
+            // the way old AAPT did it (presumably to keep it short and to avoid including
+            // annotations like @hide which would affect this Styleable).
+            StringPiece16 attrCommentLine = entry.symbol->attribute->getComment();
+            auto iter = std::find(attrCommentLine.begin(), attrCommentLine.end(), u'.');
+            if (iter != attrCommentLine.end()) {
+                attrCommentLine = attrCommentLine.substr(
+                        0, (iter - attrCommentLine.begin()) + 1);
             }
-            styleableComment << "</td></tr>\n";
+            styleableComment << attrCommentLine << "</td></tr>\n";
         }
         styleableComment << "</table>\n";
 
         for (const StyleableAttr& entry : sortedAttributes) {
+            if (!entry.symbol) {
+                continue;
+            }
+
+            if (mOptions.types == JavaClassGeneratorOptions::SymbolTypes::kPublic &&
+                    !entry.symbol->isPublic) {
+                // Don't write entries for non-public attributes.
+                continue;
+            }
             styleableComment << "@see #" << entry.fieldName << "\n";
         }
 
@@ -310,6 +337,17 @@ void JavaClassGenerator::addMembersToStyleableClass(const StringPiece16& package
     // Now we emit the indices into the array.
     for (size_t i = 0; i < attrCount; i++) {
         const StyleableAttr& styleableAttr = sortedAttributes[i];
+
+        if (!styleableAttr.symbol) {
+            continue;
+        }
+
+        if (mOptions.types == JavaClassGeneratorOptions::SymbolTypes::kPublic &&
+                !styleableAttr.symbol->isPublic) {
+            // Don't write entries for non-public attributes.
+            continue;
+        }
+
         const ResourceName& attrName = styleableAttr.attrRef->name.value();
 
         StringPiece16 packageName = attrName.package;
@@ -323,8 +361,8 @@ void JavaClassGenerator::addMembersToStyleableClass(const StringPiece16& package
         AnnotationProcessor* attrProcessor = indexMember->getCommentBuilder();
 
         StringPiece16 comment = styleableAttr.attrRef->getComment();
-        if (styleableAttr.attribute && comment.empty()) {
+        if (styleableAttr.symbol->attribute && comment.empty()) {
-            comment = styleableAttr.attribute->getComment();
+            comment = styleableAttr.symbol->attribute->getComment();
         }
 
         if (!comment.empty()) {
@@ -342,10 +380,8 @@ void JavaClassGenerator::addMembersToStyleableClass(const StringPiece16& package
 
         attrProcessor->appendNewLine();
 
-        if (styleableAttr.attribute) {
+        addAttributeFormatDoc(attrProcessor, styleableAttr.symbol->attribute.get());
-            addAttributeFormatDoc(attrProcessor, styleableAttr.attribute.get());
         attrProcessor->appendNewLine();
-        }
 
         std::stringstream doclavaName;
         doclavaName << "@attr name " << packageName << ":" << attrName.entry;;

tools/aapt2/java/JavaClassGenerator_test.cpp

@@ -43,7 +43,8 @@ TEST(JavaClassGeneratorTest, TransformInvalidJavaIdentifierCharacter) {
     std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder()
             .setPackageId(u"android", 0x01)
             .addSimple(u"@android:id/hey-man", ResourceId(0x01020000))
-            .addSimple(u"@android:attr/cool.attr", ResourceId(0x01010000))
+            .addValue(u"@android:attr/cool.attr", ResourceId(0x01010000),
+                      test::AttributeBuilder(false).build())
             .addValue(u"@android:styleable/hey.dude", ResourceId(0x01030000),
                       test::StyleableBuilder()
                               .addItem(u"@android:attr/cool.attr", ResourceId(0x01010000))
@@ -199,8 +200,10 @@ TEST(JavaClassGeneratorTest, EmitOtherPackagesAttributesInStyleable) {
     std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder()
                 .setPackageId(u"android", 0x01)
                 .setPackageId(u"com.lib", 0x02)
-                .addSimple(u"@android:attr/bar", ResourceId(0x01010000))
+                .addValue(u"@android:attr/bar", ResourceId(0x01010000),
-                .addSimple(u"@com.lib:attr/bar", ResourceId(0x02010000))
+                          test::AttributeBuilder(false).build())
+                .addValue(u"@com.lib:attr/bar", ResourceId(0x02010000),
+                           test::AttributeBuilder(false).build())
                 .addValue(u"@android:styleable/foo", ResourceId(0x01030000),
                           test::StyleableBuilder()
                                   .addItem(u"@android:attr/bar", ResourceId(0x01010000))

tools/aapt2/java/ManifestClassGenerator_test.cpp

@@ -126,7 +126,6 @@ R"EOF(    /**
 R"EOF(    /**
      * This is a private permission for system only!
      * @hide
-     * @SystemApi
      */
     @android.annotation.SystemApi
     public static final String SECRET="android.permission.SECRET";)EOF";

tools/aapt2/process/SymbolTable.h

@@ -51,9 +51,28 @@ class ISymbolSource;
 class SymbolTable {
 public:
     struct Symbol {
+        Symbol() : Symbol(Maybe<ResourceId>{}) {
+        }
+
+        Symbol(const Maybe<ResourceId>& i) : Symbol(i, nullptr) {
+        }
+
+        Symbol(const Maybe<ResourceId>& i, const std::shared_ptr<Attribute>& attr) :
+                Symbol(i, attr, false) {
+        }
+
+        Symbol(const Maybe<ResourceId>& i, const std::shared_ptr<Attribute>& attr, bool pub) :
+                id(i), attribute(attr), isPublic(pub) {
+        }
+
+        Symbol(const Symbol&) = default;
+        Symbol(Symbol&&) = default;
+        Symbol& operator=(const Symbol&) = default;
+        Symbol& operator=(Symbol&&) = default;
+
         Maybe<ResourceId> id;
         std::shared_ptr<Attribute> attribute;
-        bool isPublic;
+        bool isPublic = false;
     };
 
     SymbolTable() : mCache(200), mIdCache(200) {