Merge "AAPT2: Record source/comments for compound values' children"
diff --git a/tools/aapt2/ResourceParser.cpp b/tools/aapt2/ResourceParser.cpp
index f6a720a..c2ddb5c 100644
--- a/tools/aapt2/ResourceParser.cpp
+++ b/tools/aapt2/ResourceParser.cpp
@@ -919,6 +919,7 @@
}
transformReferenceFromNamespace(parser, u"", &maybeKey.value());
+ maybeKey.value().setSource(source);
std::unique_ptr<Item> value = parseXml(parser, 0, kAllowRawString);
if (!value) {
diff --git a/tools/aapt2/flatten/ResourceTypeExtensions.h b/tools/aapt2/flatten/ResourceTypeExtensions.h
index acf5bb5..02bff2c 100644
--- a/tools/aapt2/flatten/ResourceTypeExtensions.h
+++ b/tools/aapt2/flatten/ResourceTypeExtensions.h
@@ -67,6 +67,28 @@
};
/**
+ * New types for a ResTable_map.
+ */
+struct ExtendedResTableMapTypes {
+ enum {
+ /**
+ * Type that contains the source path of the next item in the map.
+ */
+ ATTR_SOURCE_PATH = Res_MAKEINTERNAL(0xffff),
+
+ /**
+ * Type that contains the source line of the next item in the map.
+ */
+ ATTR_SOURCE_LINE = Res_MAKEINTERNAL(0xfffe),
+
+ /**
+ * Type that contains the comment of the next item in the map.
+ */
+ ATTR_COMMENT = Res_MAKEINTERNAL(0xfffd)
+ };
+};
+
+/**
* Followed by exportedSymbolCount ExportedSymbol structs, followed by the string pool.
*/
struct FileExport_header {
diff --git a/tools/aapt2/flatten/TableFlattener.cpp b/tools/aapt2/flatten/TableFlattener.cpp
index 0590610..f42e6b7 100644
--- a/tools/aapt2/flatten/TableFlattener.cpp
+++ b/tools/aapt2/flatten/TableFlattener.cpp
@@ -108,15 +108,19 @@
SymbolWriter* mSymbols;
FlatEntry* mEntry;
BigBuffer* mBuffer;
+ StringPool* mSourcePool;
+ StringPool* mCommentPool;
bool mUseExtendedChunks;
+
size_t mEntryCount = 0;
Maybe<uint32_t> mParentIdent;
Maybe<ResourceNameRef> mParentName;
MapFlattenVisitor(SymbolWriter* symbols, FlatEntry* entry, BigBuffer* buffer,
+ StringPool* sourcePool, StringPool* commentPool,
bool useExtendedChunks) :
- mSymbols(symbols), mEntry(entry), mBuffer(buffer),
- mUseExtendedChunks(useExtendedChunks) {
+ mSymbols(symbols), mEntry(entry), mBuffer(buffer), mSourcePool(sourcePool),
+ mCommentPool(commentPool), mUseExtendedChunks(useExtendedChunks) {
}
void flattenKey(Reference* key, ResTable_map* outEntry) {
@@ -158,6 +162,32 @@
mEntryCount++;
}
+ void flattenMetaData(Value* value) {
+ if (!mUseExtendedChunks) {
+ return;
+ }
+
+ Reference key(ResourceId{ ExtendedResTableMapTypes::ATTR_SOURCE_PATH });
+ StringPool::Ref sourcePathRef = mSourcePool->makeRef(
+ util::utf8ToUtf16(value->getSource().path));
+ BinaryPrimitive val(Res_value::TYPE_INT_DEC,
+ static_cast<uint32_t>(sourcePathRef.getIndex()));
+ flattenEntry(&key, &val);
+
+ if (value->getSource().line) {
+ key.id = ResourceId(ExtendedResTableMapTypes::ATTR_SOURCE_LINE);
+ val.value.data = static_cast<uint32_t>(value->getSource().line.value());
+ flattenEntry(&key, &val);
+ }
+
+ if (!value->getComment().empty()) {
+ key.id = ResourceId(ExtendedResTableMapTypes::ATTR_COMMENT);
+ StringPool::Ref commentRef = mCommentPool->makeRef(value->getComment());
+ val.value.data = static_cast<uint32_t>(commentRef.getIndex());
+ flattenEntry(&key, &val);
+ }
+ }
+
void visit(Attribute* attr) override {
{
Reference key(ResourceId{ ResTable_map::ATTR_TYPE });
@@ -211,6 +241,7 @@
for (Style::Entry& entry : style->entries) {
flattenEntry(&entry.key, entry.value.get());
+ flattenMetaData(&entry.key);
}
}
@@ -218,6 +249,7 @@
for (auto& attrRef : styleable->entries) {
BinaryPrimitive val(Res_value{});
flattenEntry(&attrRef, &val);
+ flattenMetaData(&attrRef);
}
}
@@ -227,6 +259,7 @@
flattenValue(item.get(), outEntry);
outEntry->value.size = util::hostToDevice16(sizeof(outEntry->value));
mEntryCount++;
+ flattenMetaData(item.get());
}
}
@@ -270,6 +303,7 @@
Reference key(q);
flattenEntry(&key, plural->values[i].get());
+ flattenMetaData(plural->values[i].get());
}
}
};
@@ -389,7 +423,8 @@
} else {
const size_t beforeEntry = buffer->size();
ResTable_entry_ext* outEntry = writeEntry<ResTable_entry_ext, false>(entry, buffer);
- MapFlattenVisitor visitor(mSymbols, entry, buffer, mOptions.useExtendedChunks);
+ MapFlattenVisitor visitor(mSymbols, entry, buffer, mSourcePool, mSourcePool,
+ mOptions.useExtendedChunks);
entry->value->accept(&visitor);
outEntry->count = util::hostToDevice32(visitor.mEntryCount);
if (visitor.mParentName) {
diff --git a/tools/aapt2/link/ReferenceLinker.cpp b/tools/aapt2/link/ReferenceLinker.cpp
index 3f64d7b..2743539 100644
--- a/tools/aapt2/link/ReferenceLinker.cpp
+++ b/tools/aapt2/link/ReferenceLinker.cpp
@@ -133,7 +133,7 @@
// fast and we avoid creating a DiagMessage when the match is successful.
if (!symbol->attribute->matches(entry.value.get(), nullptr)) {
// The actual type of this item is incompatible with the attribute.
- DiagMessage msg(style->getSource());
+ DiagMessage msg(entry.key.getSource());
// Call the matches method again, this time with a DiagMessage so we fill
// in the actual error message.
@@ -143,16 +143,11 @@
}
} else {
- DiagMessage msg(style->getSource());
+ DiagMessage msg(entry.key.getSource());
msg << "style attribute '";
- if (entry.key.name) {
- msg << entry.key.name.value().package << ":" << entry.key.name.value().entry;
- } else {
- msg << entry.key.id.value();
- }
+ ReferenceLinker::writeResourceName(&msg, entry.key, transformedReference);
msg << "' " << errStr;
mContext->getDiagnostics()->error(msg);
- mContext->getDiagnostics()->note(DiagMessage(style->getSource()) << entry.key);
mError = true;
}
}
@@ -201,16 +196,12 @@
CallSite* callSite, std::string* outError) {
const ISymbolTable::Symbol* symbol = resolveSymbol(reference, nameMangler, symbols);
if (!symbol) {
- std::stringstream errStr;
- errStr << "not found";
- if (outError) *outError = errStr.str();
+ if (outError) *outError = "not found";
return nullptr;
}
if (!isSymbolVisible(*symbol, reference, *callSite)) {
- std::stringstream errStr;
- errStr << "is private";
- if (outError) *outError = errStr.str();
+ if (outError) *outError = "is private";
return nullptr;
}
return symbol;
@@ -227,9 +218,7 @@
}
if (!symbol->attribute) {
- std::stringstream errStr;
- errStr << "is not an attribute";
- if (outError) *outError = errStr.str();
+ if (outError) *outError = "is not an attribute";
return nullptr;
}
return symbol;
@@ -246,14 +235,26 @@
}
if (!symbol->attribute) {
- std::stringstream errStr;
- errStr << "is not an attribute";
- if (outError) *outError = errStr.str();
+ if (outError) *outError = "is not an attribute";
return {};
}
return xml::AaptAttribute{ symbol->id, *symbol->attribute };
}
+void ReferenceLinker::writeResourceName(DiagMessage* outMsg, const Reference& orig,
+ const Reference& transformed) {
+ assert(outMsg);
+
+ if (orig.name) {
+ *outMsg << orig.name.value();
+ if (transformed.name.value() != orig.name.value()) {
+ *outMsg << " (aka " << transformed.name.value() << ")";
+ }
+ } else {
+ *outMsg << orig.id.value();
+ }
+}
+
bool ReferenceLinker::linkReference(Reference* reference, IAaptContext* context,
ISymbolTable* symbols, xml::IPackageDeclStack* decls,
CallSite* callSite) {
@@ -274,15 +275,7 @@
DiagMessage errorMsg(reference->getSource());
errorMsg << "resource ";
- if (reference->name) {
- errorMsg << reference->name.value();
- if (transformedReference.name.value() != reference->name.value()) {
- errorMsg << " (aka " << transformedReference.name.value() << ")";
- }
- } else {
- errorMsg << reference->id.value();
- }
-
+ writeResourceName(&errorMsg, *reference, transformedReference);
errorMsg << " " << errStr;
context->getDiagnostics()->error(errorMsg);
return false;
diff --git a/tools/aapt2/link/ReferenceLinker.h b/tools/aapt2/link/ReferenceLinker.h
index 6f11d58..a0eb00c 100644
--- a/tools/aapt2/link/ReferenceLinker.h
+++ b/tools/aapt2/link/ReferenceLinker.h
@@ -80,6 +80,13 @@
std::string* outError);
/**
+ * Writes the resource name to the DiagMessage, using the "orig_name (aka <transformed_name>)"
+ * syntax.
+ */
+ static void writeResourceName(DiagMessage* outMsg, const Reference& orig,
+ const Reference& transformed);
+
+ /**
* Transforms the package name of the reference to the fully qualified package name using
* the xml::IPackageDeclStack, then mangles and looks up the symbol. If the symbol is visible
* to the reference at the callsite, the reference is updated with an ID.
diff --git a/tools/aapt2/unflatten/BinaryResourceParser.cpp b/tools/aapt2/unflatten/BinaryResourceParser.cpp
index bd42b67..5cc7aa7 100644
--- a/tools/aapt2/unflatten/BinaryResourceParser.cpp
+++ b/tools/aapt2/unflatten/BinaryResourceParser.cpp
@@ -575,11 +575,10 @@
Source source = mSource;
if (sourceBlock) {
- size_t len;
- const char* str = mSourcePool.string8At(util::deviceToHost32(sourceBlock->path.index),
- &len);
- if (str) {
- source.path.assign(str, len);
+ StringPiece path = util::getString8(mSourcePool,
+ util::deviceToHost32(sourceBlock->path.index));
+ if (!path.empty()) {
+ source.path = path.toString();
}
source.line = util::deviceToHost32(sourceBlock->line);
}
@@ -728,6 +727,16 @@
}
for (const ResTable_map& mapEntry : map) {
+ if (Res_INTERNALID(util::deviceToHost32(mapEntry.name.ident))) {
+ if (style->entries.empty()) {
+ mContext->getDiagnostics()->error(DiagMessage(mSource)
+ << "out-of-sequence meta data in style");
+ return {};
+ }
+ collectMetaData(mapEntry, &style->entries.back().key);
+ continue;
+ }
+
style->entries.emplace_back();
Style::Entry& styleEntry = style->entries.back();
@@ -811,11 +820,53 @@
return attr;
}
+static bool isMetaDataEntry(const ResTable_map& mapEntry) {
+ switch (util::deviceToHost32(mapEntry.name.ident)) {
+ case ExtendedResTableMapTypes::ATTR_SOURCE_PATH:
+ case ExtendedResTableMapTypes::ATTR_SOURCE_LINE:
+ case ExtendedResTableMapTypes::ATTR_COMMENT:
+ return true;
+ }
+ return false;
+}
+
+bool BinaryResourceParser::collectMetaData(const ResTable_map& mapEntry, Value* value) {
+ switch (util::deviceToHost32(mapEntry.name.ident)) {
+ case ExtendedResTableMapTypes::ATTR_SOURCE_PATH:
+ value->setSource(Source(util::getString8(mSourcePool,
+ util::deviceToHost32(mapEntry.value.data))));
+ return true;
+ break;
+
+ case ExtendedResTableMapTypes::ATTR_SOURCE_LINE:
+ value->setSource(value->getSource().withLine(util::deviceToHost32(mapEntry.value.data)));
+ return true;
+ break;
+
+ case ExtendedResTableMapTypes::ATTR_COMMENT:
+ value->setComment(util::getString(mSourcePool, util::deviceToHost32(mapEntry.value.data)));
+ return true;
+ break;
+ }
+ return false;
+}
+
std::unique_ptr<Array> BinaryResourceParser::parseArray(const ResourceNameRef& name,
const ConfigDescription& config,
const ResTable_map_entry* map) {
std::unique_ptr<Array> array = util::make_unique<Array>();
+ Source source;
for (const ResTable_map& mapEntry : map) {
+ if (isMetaDataEntry(mapEntry)) {
+ if (array->items.empty()) {
+ mContext->getDiagnostics()->error(DiagMessage(mSource)
+ << "out-of-sequence meta data in array");
+ return {};
+ }
+ collectMetaData(mapEntry, array->items.back().get());
+ continue;
+ }
+
array->items.push_back(parseValue(name, config, &mapEntry.value, 0));
}
return array;
@@ -826,6 +877,16 @@
const ResTable_map_entry* map) {
std::unique_ptr<Styleable> styleable = util::make_unique<Styleable>();
for (const ResTable_map& mapEntry : map) {
+ if (isMetaDataEntry(mapEntry)) {
+ if (styleable->entries.empty()) {
+ mContext->getDiagnostics()->error(DiagMessage(mSource)
+ << "out-of-sequence meta data in styleable");
+ return {};
+ }
+ collectMetaData(mapEntry, &styleable->entries.back());
+ continue;
+ }
+
if (util::deviceToHost32(mapEntry.name.ident) == 0) {
// The map entry's key (attribute) is not set. This must be
// a symbol reference, so resolve it.
@@ -849,12 +910,25 @@
const ConfigDescription& config,
const ResTable_map_entry* map) {
std::unique_ptr<Plural> plural = util::make_unique<Plural>();
+ Item* lastEntry = nullptr;
for (const ResTable_map& mapEntry : map) {
+ if (isMetaDataEntry(mapEntry)) {
+ if (!lastEntry) {
+ mContext->getDiagnostics()->error(DiagMessage(mSource)
+ << "out-of-sequence meta data in plural");
+ return {};
+ }
+ collectMetaData(mapEntry, lastEntry);
+ continue;
+ }
+
std::unique_ptr<Item> item = parseValue(name, config, &mapEntry.value, 0);
if (!item) {
return {};
}
+ lastEntry = item.get();
+
switch (util::deviceToHost32(mapEntry.name.ident)) {
case ResTable_map::ATTR_ZERO:
plural->values[Plural::Zero] = std::move(item);
diff --git a/tools/aapt2/unflatten/BinaryResourceParser.h b/tools/aapt2/unflatten/BinaryResourceParser.h
index 73fcf52..0745a59 100644
--- a/tools/aapt2/unflatten/BinaryResourceParser.h
+++ b/tools/aapt2/unflatten/BinaryResourceParser.h
@@ -91,6 +91,13 @@
const ConfigDescription& config,
const android::ResTable_map_entry* map);
+ /**
+ * If the mapEntry is a special type that denotes meta data (source, comment), then it is
+ * read and added to the Value.
+ * Returns true if the mapEntry was meta data.
+ */
+ bool collectMetaData(const android::ResTable_map& mapEntry, Value* value);
+
IAaptContext* mContext;
ResourceTable* mTable;
diff --git a/tools/aapt2/util/Util.h b/tools/aapt2/util/Util.h
index a898619..0dacbd7 100644
--- a/tools/aapt2/util/Util.h
+++ b/tools/aapt2/util/Util.h
@@ -158,6 +158,15 @@
return StringPiece16();
}
+inline StringPiece getString8(const android::ResStringPool& pool, size_t idx) {
+ size_t len;
+ const char* str = pool.string8At(idx, &len);
+ if (str != nullptr) {
+ return StringPiece(str, len);
+ }
+ return StringPiece();
+}
+
/**
* Checks that the Java string format contains no non-positional arguments (arguments without
* explicitly specifying an index) when there are more than one argument. This is an error