diff options
-rw-r--r-- | tools/aapt2/flatten/ResourceTypeExtensions.h | 52 | ||||
-rw-r--r-- | tools/aapt2/flatten/TableFlattener.cpp | 23 | ||||
-rw-r--r-- | tools/aapt2/java/AnnotationProcessor.cpp | 30 | ||||
-rw-r--r-- | tools/aapt2/java/AnnotationProcessor.h | 3 | ||||
-rw-r--r-- | tools/aapt2/java/JavaClassGenerator.cpp | 121 | ||||
-rw-r--r-- | tools/aapt2/java/JavaClassGenerator_test.cpp | 31 | ||||
-rw-r--r-- | tools/aapt2/unflatten/BinaryResourceParser.cpp | 20 | ||||
-rw-r--r-- | tools/aapt2/util/Util.cpp | 19 | ||||
-rw-r--r-- | tools/aapt2/util/Util.h | 2 |
9 files changed, 249 insertions, 52 deletions
diff --git a/tools/aapt2/flatten/ResourceTypeExtensions.h b/tools/aapt2/flatten/ResourceTypeExtensions.h index 38cda097c927..c1ff556ac0cf 100644 --- a/tools/aapt2/flatten/ResourceTypeExtensions.h +++ b/tools/aapt2/flatten/ResourceTypeExtensions.h @@ -132,6 +132,32 @@ struct Public_header { uint32_t count; }; +/** + * A structure representing source data for a resource entry. + * Appears after an android::ResTable_entry or android::ResTable_map_entry. + * + * TODO(adamlesinski): This causes some issues when runtime code checks + * the size of an android::ResTable_entry. It assumes it is an + * android::ResTable_map_entry if the size is bigger than an android::ResTable_entry + * which may not be true if this structure is present. + */ +struct ResTable_entry_source { + /** + * File path reference. + */ + android::ResStringPool_ref path; + + /** + * Line number this resource was defined on. + */ + uint32_t line; + + /** + * Comment string reference. + */ + android::ResStringPool_ref comment; +}; + struct Public_entry { uint16_t entryId; @@ -143,8 +169,7 @@ struct Public_entry { uint16_t state; android::ResStringPool_ref key; - android::ResStringPool_ref source; - uint32_t sourceLine; + ResTable_entry_source source; }; /** @@ -173,28 +198,7 @@ struct SymbolTable_entry { * The index into the string pool where the name of this * symbol exists. */ - uint32_t stringIndex; -}; - -/** - * A structure representing the source of a resourc entry. - * Appears after an android::ResTable_entry or android::ResTable_map_entry. - * - * TODO(adamlesinski): This causes some issues when runtime code checks - * the size of an android::ResTable_entry. It assumes it is an - * android::ResTable_map_entry if the size is bigger than an android::ResTable_entry - * which may not be true if this structure is present. - */ -struct ResTable_entry_source { - /** - * Index into the source string pool. - */ - uint32_t pathIndex; - - /** - * Line number this resource was defined on. - */ - uint32_t line; + android::ResStringPool_ref name; }; /** diff --git a/tools/aapt2/flatten/TableFlattener.cpp b/tools/aapt2/flatten/TableFlattener.cpp index 47fa2a63d553..6b90fb276b6d 100644 --- a/tools/aapt2/flatten/TableFlattener.cpp +++ b/tools/aapt2/flatten/TableFlattener.cpp @@ -54,9 +54,16 @@ static void strcpy16_htod(uint16_t* dst, size_t len, const StringPiece16& src) { struct FlatEntry { ResourceEntry* entry; Value* value; + + // The entry string pool index to the entry's name. uint32_t entryKey; + + // The source string pool index to the source file path. uint32_t sourcePathKey; uint32_t sourceLine; + + // The source string pool index to the comment. + uint32_t commentKey; }; class SymbolWriter { @@ -318,8 +325,9 @@ private: if (mOptions.useExtendedChunks) { // Write the extra source block. This will be ignored by the Android runtime. ResTable_entry_source* sourceBlock = buffer->nextBlock<ResTable_entry_source>(); - sourceBlock->pathIndex = util::hostToDevice32(entry->sourcePathKey); + sourceBlock->path.index = util::hostToDevice32(entry->sourcePathKey); sourceBlock->line = util::hostToDevice32(entry->sourceLine); + sourceBlock->comment.index = util::hostToDevice32(entry->commentKey); outEntry->size += sizeof(*sourceBlock); } @@ -486,12 +494,14 @@ private: publicEntry->entryId = util::hostToDevice32(entry->id.value()); publicEntry->key.index = util::hostToDevice32(mKeyPool.makeRef( entry->name).getIndex()); - publicEntry->source.index = util::hostToDevice32(mSourcePool->makeRef( + publicEntry->source.path.index = util::hostToDevice32(mSourcePool->makeRef( util::utf8ToUtf16(entry->symbolStatus.source.path)).getIndex()); if (entry->symbolStatus.source.line) { - publicEntry->sourceLine = util::hostToDevice32( + publicEntry->source.line = util::hostToDevice32( entry->symbolStatus.source.line.value()); } + publicEntry->source.comment.index = util::hostToDevice32(mSourcePool->makeRef( + entry->symbolStatus.comment).getIndex()); switch (entry->symbolStatus.state) { case SymbolState::kPrivate: @@ -565,13 +575,16 @@ private: lineNumber = value->getSource().line.value(); } + const StringPool::Ref commentRef = mSourcePool->makeRef(value->getComment()); + configToEntryListMap[configValue.config] .push_back(FlatEntry{ entry, value, keyIndex, (uint32_t) sourceRef.getIndex(), - lineNumber }); + lineNumber, + (uint32_t) commentRef.getIndex() }); } } @@ -680,7 +693,7 @@ bool TableFlattener::consume(IAaptContext* context, ResourceTable* table) { // Update the offsets to their final values. if (symbolEntryData) { for (SymbolWriter::Entry& entry : symbolOffsets) { - symbolEntryData->stringIndex = util::hostToDevice32(entry.name.getIndex()); + symbolEntryData->name.index = util::hostToDevice32(entry.name.getIndex()); // The symbols were all calculated with the packageBuffer offset. We need to // add the beginning of the output buffer. diff --git a/tools/aapt2/java/AnnotationProcessor.cpp b/tools/aapt2/java/AnnotationProcessor.cpp index 16440bcf8ad7..b36682d98bfd 100644 --- a/tools/aapt2/java/AnnotationProcessor.cpp +++ b/tools/aapt2/java/AnnotationProcessor.cpp @@ -21,16 +21,10 @@ namespace aapt { -void AnnotationProcessor::appendCommentLine(const StringPiece16& line) { +void AnnotationProcessor::appendCommentLine(const std::string& comment) { static const std::string sDeprecated = "@deprecated"; static const std::string sSystemApi = "@SystemApi"; - if (line.empty()) { - return; - } - - std::string comment = util::utf16ToUtf8(line); - if (comment.find(sDeprecated) != std::string::npos && !mDeprecated) { mDeprecated = true; if (!mAnnotations.empty()) { @@ -63,14 +57,28 @@ void AnnotationProcessor::appendCommentLine(const StringPiece16& line) { void AnnotationProcessor::appendComment(const StringPiece16& comment) { // We need to process line by line to clean-up whitespace and append prefixes. for (StringPiece16 line : util::tokenize(comment, u'\n')) { - appendCommentLine(util::trimWhitespace(line)); + line = util::trimWhitespace(line); + if (!line.empty()) { + appendCommentLine(util::utf16ToUtf8(line)); + } + } +} + +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 AnnotationProcessor::buildComment() { - mComment += "\n"; - mComment += mPrefix; - mComment += " */"; + if (!mComment.empty()) { + mComment += "\n"; + mComment += mPrefix; + mComment += " */"; + } return std::move(mComment); } diff --git a/tools/aapt2/java/AnnotationProcessor.h b/tools/aapt2/java/AnnotationProcessor.h index b47210939d06..81a6f6e42759 100644 --- a/tools/aapt2/java/AnnotationProcessor.h +++ b/tools/aapt2/java/AnnotationProcessor.h @@ -65,6 +65,7 @@ public: * we need to collect all the comments. */ void appendComment(const StringPiece16& comment); + void appendComment(const StringPiece& comment); /** * Finishes the comment and moves it to the caller. Subsequent calls to buildComment() have @@ -85,7 +86,7 @@ private: bool mDeprecated = false; bool mSystemApi = false; - void appendCommentLine(const StringPiece16& line); + void appendCommentLine(const std::string& line); }; } // namespace aapt diff --git a/tools/aapt2/java/JavaClassGenerator.cpp b/tools/aapt2/java/JavaClassGenerator.cpp index 0175489cf6ea..dfd2ef6f5372 100644 --- a/tools/aapt2/java/JavaClassGenerator.cpp +++ b/tools/aapt2/java/JavaClassGenerator.cpp @@ -18,6 +18,8 @@ #include "Resource.h" #include "ResourceTable.h" #include "ResourceValues.h" +#include "ValueVisitor.h" + #include "java/AnnotationProcessor.h" #include "java/JavaClassGenerator.h" #include "util/StringPiece.h" @@ -109,13 +111,13 @@ void JavaClassGenerator::generateStyleable(const StringPiece16& packageNameToGen std::sort(sortedAttributes.begin(), sortedAttributes.end()); // First we emit the array containing the IDs of each attribute. - *out << " " + *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 "; + *out << "\n "; } *out << sortedAttributes[i].first; @@ -123,11 +125,11 @@ void JavaClassGenerator::generateStyleable(const StringPiece16& packageNameToGen *out << ", "; } } - *out << "\n };\n"; + *out << "\n };\n"; // Now we emit the indices into the array. for (size_t i = 0; i < attrCount; i++) { - *out << " " + *out << " " << "public static" << finalModifier << " int " << transform(entryName); @@ -141,6 +143,85 @@ void JavaClassGenerator::generateStyleable(const StringPiece16& packageNameToGen } } +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::generateType(const StringPiece16& packageNameToGenerate, const ResourceTablePackage* package, const ResourceTableType* type, @@ -186,7 +267,33 @@ bool JavaClassGenerator::generateType(const StringPiece16& packageNameToGenerate generateStyleable(packageNameToGenerate, unmangledName, static_cast<const Styleable*>( entry->values.front().value.get()), out); } else { - *out << " " << "public static" << finalModifier + AnnotationProcessor processor(" "); + if (entry->symbolStatus.state != SymbolState::kUndefined) { + processor.appendComment(entry->symbolStatus.comment); + } + + 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); + } + } + + std::string comment = processor.buildComment(); + if (!comment.empty()) { + *out << comment << "\n"; + } + + std::string annotations = processor.buildAnnotations(); + if (!annotations.empty()) { + *out << annotations << "\n"; + } + + *out << " " << "public static" << finalModifier << " int " << transform(unmangledName) << " = " << id << ";\n"; } } @@ -211,11 +318,11 @@ bool JavaClassGenerator::generate(const StringPiece16& packageNameToGenerate, } else { typeStr = toString(type->type); } - *out << " public static final class " << typeStr << " {\n"; + *out << " public static final class " << typeStr << " {\n"; if (!generateType(packageNameToGenerate, package.get(), type.get(), out)) { return false; } - *out << " }\n"; + *out << " }\n"; } } diff --git a/tools/aapt2/java/JavaClassGenerator_test.cpp b/tools/aapt2/java/JavaClassGenerator_test.cpp index 3625f9c340ed..2dc387bea8b9 100644 --- a/tools/aapt2/java/JavaClassGenerator_test.cpp +++ b/tools/aapt2/java/JavaClassGenerator_test.cpp @@ -198,4 +198,35 @@ TEST(JavaClassGeneratorTest, EmitOtherPackagesAttributesInStyleable) { EXPECT_NE(std::string::npos, output.find("int foo_com_lib_bar =")); } +TEST(JavaClassGeneratorTest, CommentsForSimpleResourcesArePresent) { + std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() + .setPackageId(u"android", 0x01) + .addSimple(u"@android:id/foo", ResourceId(0x01010000)) + .build(); + test::getValue<Id>(table.get(), u"@android:id/foo") + ->setComment(std::u16string(u"This is a comment\n@deprecated")); + + JavaClassGenerator generator(table.get(), {}); + + std::stringstream out; + ASSERT_TRUE(generator.generate(u"android", &out)); + std::string actual = out.str(); + + EXPECT_NE(std::string::npos, actual.find( + R"EOF(/** + * This is a comment + * @deprecated + */ + @Deprecated + public static final int foo = 0x01010000;)EOF")); +} + +TEST(JavaClassGeneratorTest, CommentsForEnumAndFlagAttributesArePresent) { + +} + +TEST(JavaClassGeneratorTest, CommentsForStyleablesAndNestedAttributesArePresent) { + +} + } // namespace aapt diff --git a/tools/aapt2/unflatten/BinaryResourceParser.cpp b/tools/aapt2/unflatten/BinaryResourceParser.cpp index 30c60918d4e8..0d17e8467d32 100644 --- a/tools/aapt2/unflatten/BinaryResourceParser.cpp +++ b/tools/aapt2/unflatten/BinaryResourceParser.cpp @@ -116,7 +116,7 @@ bool BinaryResourceParser::getSymbol(const void* data, ResourceNameRef* outSymbo if (util::deviceToHost32(mSymbolEntries[i].offset) == offset) { // This offset is a symbol! const StringPiece16 str = util::getString( - mSymbolPool, util::deviceToHost32(mSymbolEntries[i].stringIndex)); + mSymbolPool, util::deviceToHost32(mSymbolEntries[i].name.index)); StringPiece16 typeStr; ResourceUtils::extractResourceName(str, &outSymbol->package, &typeStr, @@ -425,8 +425,14 @@ bool BinaryResourceParser::parsePublic(const ResourceTablePackage* package, Symbol symbol; if (mSourcePool.getError() == NO_ERROR) { symbol.source.path = util::utf16ToUtf8(util::getString( - mSourcePool, util::deviceToHost32(entry->source.index))); - symbol.source.line = util::deviceToHost32(entry->sourceLine); + mSourcePool, util::deviceToHost32(entry->source.path.index))); + symbol.source.line = util::deviceToHost32(entry->source.line); + } + + StringPiece16 comment = util::getString(mSourcePool, + util::deviceToHost32(entry->source.comment.index)); + if (!comment.empty()) { + symbol.comment = comment.toString(); } switch (util::deviceToHost16(entry->state)) { @@ -560,7 +566,7 @@ bool BinaryResourceParser::parseType(const ResourceTablePackage* package, Source source = mSource; if (sourceBlock) { size_t len; - const char* str = mSourcePool.string8At(util::deviceToHost32(sourceBlock->pathIndex), + const char* str = mSourcePool.string8At(util::deviceToHost32(sourceBlock->path.index), &len); if (str) { source.path.assign(str, len); @@ -568,6 +574,12 @@ bool BinaryResourceParser::parseType(const ResourceTablePackage* package, source.line = util::deviceToHost32(sourceBlock->line); } + StringPiece16 comment = util::getString(mSourcePool, + util::deviceToHost32(sourceBlock->comment.index)); + if (!comment.empty()) { + resourceValue->setComment(comment); + } + resourceValue->setSource(source); if (!mTable->addResourceAllowMangled(name, config, std::move(resourceValue), mContext->getDiagnostics())) { diff --git a/tools/aapt2/util/Util.cpp b/tools/aapt2/util/Util.cpp index f219b65378ff..6ef4ce504a63 100644 --- a/tools/aapt2/util/Util.cpp +++ b/tools/aapt2/util/Util.cpp @@ -76,6 +76,25 @@ StringPiece16 trimWhitespace(const StringPiece16& str) { return StringPiece16(start, end - start); } +StringPiece trimWhitespace(const StringPiece& str) { + if (str.size() == 0 || str.data() == nullptr) { + return str; + } + + const char* start = str.data(); + const char* end = str.data() + str.length(); + + while (start != end && isspace(*start)) { + start++; + } + + while (end != start && isspace(*(end - 1))) { + end--; + } + + return StringPiece(start, end - start); +} + StringPiece16::const_iterator findNonAlphaNumericAndNotInSet(const StringPiece16& str, const StringPiece16& allowedChars) { const auto endIter = str.end(); diff --git a/tools/aapt2/util/Util.h b/tools/aapt2/util/Util.h index 402147de32b2..fd3fbb4573a0 100644 --- a/tools/aapt2/util/Util.h +++ b/tools/aapt2/util/Util.h @@ -62,6 +62,8 @@ bool stringEndsWith(const BasicStringPiece<T>& str, const BasicStringPiece<T>& s */ StringPiece16 trimWhitespace(const StringPiece16& str); +StringPiece trimWhitespace(const StringPiece& str); + /** * UTF-16 isspace(). It basically checks for lower range characters that are * whitespace. |