From 626b3dbf74f02ae630ae0089632f5962340694dc Mon Sep 17 00:00:00 2001 From: Adam Lesinski Date: Thu, 7 Apr 2016 13:24:59 -0700 Subject: AAPT2: Clean up R JavaDoc generation - Don't generate private attributes in public R.java - Strip out @SystemApi from comment when generating @android.annotation.SystemApi - Only emit a single line (up to the first period) of an attribute's comment within a styleable's attribute table. Change-Id: Id6316a6861540325934133958939a12074ad4428 --- tools/aapt2/java/AnnotationProcessor.cpp | 16 ++++-- tools/aapt2/java/AnnotationProcessor.h | 3 +- tools/aapt2/java/AnnotationProcessor_test.cpp | 62 +++++++----------------- tools/aapt2/java/ClassDefinition.h | 2 +- tools/aapt2/java/JavaClassGenerator.cpp | 60 ++++++++++++++++++----- tools/aapt2/java/JavaClassGenerator_test.cpp | 9 ++-- tools/aapt2/java/ManifestClassGenerator_test.cpp | 1 - 7 files changed, 85 insertions(+), 68 deletions(-) (limited to 'tools/aapt2/java') diff --git a/tools/aapt2/java/AnnotationProcessor.cpp b/tools/aapt2/java/AnnotationProcessor.cpp index ba744395ece0..b7e7f903a2b1 100644 --- a/tools/aapt2/java/AnnotationProcessor.cpp +++ b/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); } } } diff --git a/tools/aapt2/java/AnnotationProcessor.h b/tools/aapt2/java/AnnotationProcessor.h index 0fc5b08e0bf3..8309dd978175 100644 --- a/tools/aapt2/java/AnnotationProcessor.h +++ b/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 diff --git a/tools/aapt2/java/AnnotationProcessor_test.cpp b/tools/aapt2/java/AnnotationProcessor_test.cpp index d3860a5825b2..5a39add48fbd 100644 --- a/tools/aapt2/java/AnnotationProcessor_test.cpp +++ b/tools/aapt2/java/AnnotationProcessor_test.cpp @@ -14,65 +14,37 @@ * limitations under the License. */ -#include "ResourceParser.h" -#include "ResourceTable.h" -#include "ResourceValues.h" #include "java/AnnotationProcessor.h" -#include "test/Builders.h" -#include "test/Context.h" -#include "xml/XmlPullParser.h" - -#include +#include "test/Test.h" namespace aapt { -struct AnnotationProcessorTest : public ::testing::Test { - std::unique_ptr 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 << "\n" << str; - xml::XmlPullParser xmlParser(in); - if (parser.parse(&xmlParser)) { - return ::testing::AssertionSuccess(); - } - return ::testing::AssertionFailure(); - } -}; +TEST(AnnotationProcessorTest, EmitsDeprecated) { + const char* comment = "Some comment, and it should contain a marker word, " + "something that marks this resource as nor needed. " + "{@deprecated That's the marker! }"; -TEST_F(AnnotationProcessorTest, EmitsDeprecated) { - const char* xmlInput = R"EOF( - - - - - - )EOF"; + AnnotationProcessor processor; + processor.appendComment(comment); - ASSERT_TRUE(parse(xmlInput)); + std::stringstream result; + processor.writeToStream(&result, ""); + std::string annotations = result.str(); - Attribute* attr = test::getValue(&mTable, u"@attr/autoText"); - ASSERT_NE(nullptr, attr); + EXPECT_NE(std::string::npos, annotations.find("@Deprecated")); +} +TEST(AnnotationProcessorTest, EmitsSystemApiAnnotationAndRemovesFromComment) { AnnotationProcessor processor; - processor.appendComment(attr->getComment()); + 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("@Deprecated")); + 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 diff --git a/tools/aapt2/java/ClassDefinition.h b/tools/aapt2/java/ClassDefinition.h index 53e0f6fe2f1e..d45328fedba2 100644 --- a/tools/aapt2/java/ClassDefinition.h +++ b/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(); diff --git a/tools/aapt2/java/JavaClassGenerator.cpp b/tools/aapt2/java/JavaClassGenerator.cpp index 32b8600358b8..24347a1cdc1e 100644 --- a/tools/aapt2/java/JavaClassGenerator.cpp +++ b/tools/aapt2/java/JavaClassGenerator.cpp @@ -188,8 +188,8 @@ bool JavaClassGenerator::skipSymbol(SymbolState state) { struct StyleableAttr { const Reference* attrRef; - std::shared_ptr attribute; std::string fieldName; + std::unique_ptr 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) { - styleableAttr.attribute = symbol->attribute; + if (symbol && symbol->attribute) { + // Copy the symbol data structure because the returned instance can be destroyed. + styleableAttr.symbol = util::make_unique(*symbol); } sortedAttributes.push_back(std::move(styleableAttr)); } @@ -273,6 +274,16 @@ void JavaClassGenerator::addMembersToStyleableClass(const StringPiece16& package "AttributeDescription\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 << ""; styleableComment << "{@link #" @@ -284,14 +295,30 @@ void JavaClassGenerator::addMembersToStyleableClass(const StringPiece16& package styleableComment << ""; styleableComment << ""; - 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 << "\n"; + styleableComment << attrCommentLine << "\n"; } styleableComment << "\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()) { - comment = styleableAttr.attribute->getComment(); + if (styleableAttr.symbol->attribute && comment.empty()) { + 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.attribute.get()); - attrProcessor->appendNewLine(); - } + addAttributeFormatDoc(attrProcessor, styleableAttr.symbol->attribute.get()); + attrProcessor->appendNewLine(); std::stringstream doclavaName; doclavaName << "@attr name " << packageName << ":" << attrName.entry;; diff --git a/tools/aapt2/java/JavaClassGenerator_test.cpp b/tools/aapt2/java/JavaClassGenerator_test.cpp index 370e78a7b9d2..7d0aa83a0fb8 100644 --- a/tools/aapt2/java/JavaClassGenerator_test.cpp +++ b/tools/aapt2/java/JavaClassGenerator_test.cpp @@ -43,7 +43,8 @@ TEST(JavaClassGeneratorTest, TransformInvalidJavaIdentifierCharacter) { std::unique_ptr 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 table = test::ResourceTableBuilder() .setPackageId(u"android", 0x01) .setPackageId(u"com.lib", 0x02) - .addSimple(u"@android:attr/bar", ResourceId(0x01010000)) - .addSimple(u"@com.lib:attr/bar", ResourceId(0x02010000)) + .addValue(u"@android:attr/bar", ResourceId(0x01010000), + 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)) diff --git a/tools/aapt2/java/ManifestClassGenerator_test.cpp b/tools/aapt2/java/ManifestClassGenerator_test.cpp index e7210db8b743..d3bca7068cb2 100644 --- a/tools/aapt2/java/ManifestClassGenerator_test.cpp +++ b/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"; -- cgit v1.2.3-59-g8ed1b