AAPT2: Add support for comments in R.java
Change-Id: Iaa5f3b75bf7de9dbf458fa5c452f7312989f4c4f
diff --git a/tools/aapt2/flatten/ResourceTypeExtensions.h b/tools/aapt2/flatten/ResourceTypeExtensions.h
index 38cda09..c1ff556 100644
--- a/tools/aapt2/flatten/ResourceTypeExtensions.h
+++ b/tools/aapt2/flatten/ResourceTypeExtensions.h
@@ -132,6 +132,32 @@
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 @@
uint16_t state;
android::ResStringPool_ref key;
- android::ResStringPool_ref source;
- uint32_t sourceLine;
+ ResTable_entry_source source;
};
/**
@@ -173,28 +198,7 @@
* 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 47fa2a6..6b90fb2 100644
--- a/tools/aapt2/flatten/TableFlattener.cpp
+++ b/tools/aapt2/flatten/TableFlattener.cpp
@@ -54,9 +54,16 @@
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 @@
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 @@
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 @@
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 @@
// 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 16440bc..b36682d 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::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 b472109..81a6f6e 100644
--- a/tools/aapt2/java/AnnotationProcessor.h
+++ b/tools/aapt2/java/AnnotationProcessor.h
@@ -65,6 +65,7 @@
* 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 @@
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 0175489..dfd2ef6 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 @@
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 @@
*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 @@
}
}
+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 @@
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 @@
} 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 3625f9c..2dc387b 100644
--- a/tools/aapt2/java/JavaClassGenerator_test.cpp
+++ b/tools/aapt2/java/JavaClassGenerator_test.cpp
@@ -198,4 +198,35 @@
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 30c6091..0d17e84 100644
--- a/tools/aapt2/unflatten/BinaryResourceParser.cpp
+++ b/tools/aapt2/unflatten/BinaryResourceParser.cpp
@@ -116,7 +116,7 @@
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 @@
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 @@
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 @@
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 f219b65..6ef4ce5 100644
--- a/tools/aapt2/util/Util.cpp
+++ b/tools/aapt2/util/Util.cpp
@@ -76,6 +76,25 @@
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 402147d..fd3fbb4 100644
--- a/tools/aapt2/util/Util.h
+++ b/tools/aapt2/util/Util.h
@@ -62,6 +62,8 @@
*/
StringPiece16 trimWhitespace(const StringPiece16& str);
+StringPiece trimWhitespace(const StringPiece& str);
+
/**
* UTF-16 isspace(). It basically checks for lower range characters that are
* whitespace.