From feaf99fa1b7563f15dbd4211718a6cfb7a3cc3c8 Mon Sep 17 00:00:00 2001 From: Michael Wright Date: Fri, 6 May 2016 17:16:06 +0100 Subject: [PATCH] Stop emitting javadoc for @removed attributes. We need the attributes to remain public because people might still be linking against them, but we don't want them showing up in the documentation any more. Them showing up in the documentation also had the side effect that it would accidentally mark the parent class of attributes as @removed, which was not intended. Bug: 28663748 Change-Id: I2f6eb09455fddf1086e6b24bc3bea5292e8e32b7 --- tools/aapt/Resource.cpp | 3 ++ tools/aapt2/java/JavaClassGenerator.cpp | 24 ++++++++++---- tools/aapt2/java/JavaClassGenerator_test.cpp | 33 ++++++++++++++++++++ tools/aapt2/util/StringPiece.h | 22 +++++++++++++ tools/aapt2/util/StringPiece_test.cpp | 32 +++++++++++++++++++ 5 files changed, 108 insertions(+), 6 deletions(-) diff --git a/tools/aapt/Resource.cpp b/tools/aapt/Resource.cpp index 8e7045bd07e11..e6407332bb900 100644 --- a/tools/aapt/Resource.cpp +++ b/tools/aapt/Resource.cpp @@ -2255,6 +2255,9 @@ static status_t writeLayoutClasses( if (comment.size() <= 0) { comment = getAttributeComment(assets, name8); } + if (comment.contains(u"@removed")) { + continue; + } if (comment.size() > 0) { const char16_t* p = comment.string(); while (*p != 0 && *p != '.') { diff --git a/tools/aapt2/java/JavaClassGenerator.cpp b/tools/aapt2/java/JavaClassGenerator.cpp index 24347a1cdc1ef..84df0b429fc5b 100644 --- a/tools/aapt2/java/JavaClassGenerator.cpp +++ b/tools/aapt2/java/JavaClassGenerator.cpp @@ -284,6 +284,13 @@ void JavaClassGenerator::addMembersToStyleableClass(const StringPiece16& package continue; } + StringPiece16 attrCommentLine = entry.symbol->attribute->getComment(); + if (attrCommentLine.contains(StringPiece16(u"@removed"))) { + // Removed attributes are public but hidden from the documentation, so don't emit + // them as part of the class documentation. + continue; + } + const ResourceName& attrName = entry.attrRef->name.value(); styleableComment << ""; styleableComment << "{@link #" @@ -299,7 +306,6 @@ void JavaClassGenerator::addMembersToStyleableClass(const StringPiece16& package // 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( @@ -348,6 +354,17 @@ void JavaClassGenerator::addMembersToStyleableClass(const StringPiece16& package continue; } + StringPiece16 comment = styleableAttr.attrRef->getComment(); + if (styleableAttr.symbol->attribute && comment.empty()) { + comment = styleableAttr.symbol->attribute->getComment(); + } + + if (comment.contains(StringPiece16(u"@removed"))) { + // Removed attributes are public but hidden from the documentation, so don't emit them + // as part of the class documentation. + continue; + } + const ResourceName& attrName = styleableAttr.attrRef->name.value(); StringPiece16 packageName = attrName.package; @@ -360,11 +377,6 @@ void JavaClassGenerator::addMembersToStyleableClass(const StringPiece16& package AnnotationProcessor* attrProcessor = indexMember->getCommentBuilder(); - StringPiece16 comment = styleableAttr.attrRef->getComment(); - if (styleableAttr.symbol->attribute && comment.empty()) { - comment = styleableAttr.symbol->attribute->getComment(); - } - if (!comment.empty()) { attrProcessor->appendComment("

\n@attr description"); attrProcessor->appendComment(comment); diff --git a/tools/aapt2/java/JavaClassGenerator_test.cpp b/tools/aapt2/java/JavaClassGenerator_test.cpp index 7d0aa83a0fb84..46266b3f3e89f 100644 --- a/tools/aapt2/java/JavaClassGenerator_test.cpp +++ b/tools/aapt2/java/JavaClassGenerator_test.cpp @@ -289,4 +289,37 @@ TEST(JavaClassGeneratorTest, CommentsForStyleablesAndNestedAttributesArePresent) EXPECT_NE(std::string::npos, actual.find(util::utf16ToUtf8(styleable.getComment()))); } +TEST(JavaClassGeneratorTest, CommentsForRemovedAttributesAreNotPresentInClass) { + Attribute attr(false); + attr.setComment(StringPiece16(u"@removed")); + + + std::unique_ptr table = test::ResourceTableBuilder() + .setPackageId(u"android", 0x01) + .addValue(u"@android:attr/one", util::make_unique(attr)) + .build(); + + std::unique_ptr context = test::ContextBuilder() + .addSymbolSource(util::make_unique(table.get())) + .setNameManglerPolicy(NameManglerPolicy{ u"android" }) + .build(); + JavaClassGeneratorOptions options; + options.useFinal = false; + JavaClassGenerator generator(context.get(), table.get(), options); + std::stringstream out; + ASSERT_TRUE(generator.generate(u"android", &out)); + std::string actual = out.str(); + + std::cout << actual << std::endl; + + EXPECT_EQ(std::string::npos, actual.find("@attr name android:one")); + EXPECT_EQ(std::string::npos, actual.find("@attr description")); + + // We should find @removed only in the attribute javadoc and not anywhere else (i.e. the class + // javadoc). + const size_t pos = actual.find("@removed"); + EXPECT_NE(std::string::npos, pos); + EXPECT_EQ(std::string::npos, actual.find("@removed", pos + 1)); +} + } // namespace aapt diff --git a/tools/aapt2/util/StringPiece.h b/tools/aapt2/util/StringPiece.h index 31deb452b53c4..f91bccc93019e 100644 --- a/tools/aapt2/util/StringPiece.h +++ b/tools/aapt2/util/StringPiece.h @@ -57,6 +57,7 @@ public: bool empty() const; std::basic_string toString() const; + bool contains(const BasicStringPiece& rhs) const; int compare(const BasicStringPiece& rhs) const; bool operator<(const BasicStringPiece& rhs) const; bool operator>(const BasicStringPiece& rhs) const; @@ -163,6 +164,17 @@ inline std::basic_string BasicStringPiece::toString() const { return std::basic_string(mData, mLength); } +template <> +inline bool BasicStringPiece::contains(const BasicStringPiece& rhs) const { + if (!mData || !rhs.mData) { + return false; + } + if (rhs.mLength > mLength) { + return false; + } + return strstr(mData, rhs.mData) != nullptr; +} + template <> inline int BasicStringPiece::compare(const BasicStringPiece& rhs) const { const char nullStr = '\0'; @@ -185,6 +197,16 @@ inline ::std::ostream& operator<<(::std::ostream& out, const BasicStringPiece +inline bool BasicStringPiece::contains(const BasicStringPiece& rhs) const { + if (!mData || !rhs.mData) { + return false; + } + if (rhs.mLength > mLength) { + return false; + } + return strstr16(mData, rhs.mData) != nullptr; +} template <> inline int BasicStringPiece::compare(const BasicStringPiece& rhs) const { diff --git a/tools/aapt2/util/StringPiece_test.cpp b/tools/aapt2/util/StringPiece_test.cpp index d49b67ffa88eb..853a9a46fde82 100644 --- a/tools/aapt2/util/StringPiece_test.cpp +++ b/tools/aapt2/util/StringPiece_test.cpp @@ -59,4 +59,36 @@ TEST(StringPieceTest, PiecesHaveCorrectSortOrderUtf8) { EXPECT_TRUE(StringPiece(car) > banana); } +TEST(StringPieceTest, ContainsOtherStringPiece) { + StringPiece text("I am a leaf on the wind."); + StringPiece startNeedle("I am"); + StringPiece endNeedle("wind."); + StringPiece middleNeedle("leaf"); + StringPiece emptyNeedle(""); + StringPiece missingNeedle("soar"); + StringPiece longNeedle("This string is longer than the text."); + + EXPECT_TRUE(text.contains(startNeedle)); + EXPECT_TRUE(text.contains(endNeedle)); + EXPECT_TRUE(text.contains(middleNeedle)); + EXPECT_TRUE(text.contains(emptyNeedle)); + EXPECT_FALSE(text.contains(missingNeedle)); + EXPECT_FALSE(text.contains(longNeedle)); + + StringPiece16 text16(u"I am a leaf on the wind."); + StringPiece16 startNeedle16(u"I am"); + StringPiece16 endNeedle16(u"wind."); + StringPiece16 middleNeedle16(u"leaf"); + StringPiece16 emptyNeedle16(u""); + StringPiece16 missingNeedle16(u"soar"); + StringPiece16 longNeedle16(u"This string is longer than the text."); + + EXPECT_TRUE(text16.contains(startNeedle16)); + EXPECT_TRUE(text16.contains(endNeedle16)); + EXPECT_TRUE(text16.contains(middleNeedle16)); + EXPECT_TRUE(text16.contains(emptyNeedle16)); + EXPECT_FALSE(text16.contains(missingNeedle16)); + EXPECT_FALSE(text16.contains(longNeedle16)); +} + } // namespace aapt