OSDN Git Service

AAPT2: Record source/comments for compound values' children
authorAdam Lesinski <adamlesinski@google.com>
Mon, 23 Nov 2015 22:22:47 +0000 (14:22 -0800)
committerAdam Lesinski <adamlesinski@google.com>
Mon, 23 Nov 2015 22:22:47 +0000 (14:22 -0800)
Values like styles or arrays have children which need to have
their source and comments preserved.

Change-Id: I6fc713ba36627e6d66c7930b4080cc1403bac207

tools/aapt2/ResourceParser.cpp
tools/aapt2/flatten/ResourceTypeExtensions.h
tools/aapt2/flatten/TableFlattener.cpp
tools/aapt2/link/ReferenceLinker.cpp
tools/aapt2/link/ReferenceLinker.h
tools/aapt2/unflatten/BinaryResourceParser.cpp
tools/aapt2/unflatten/BinaryResourceParser.h
tools/aapt2/util/Util.h

index f6a720a..c2ddb5c 100644 (file)
@@ -919,6 +919,7 @@ bool ResourceParser::parseStyleItem(xml::XmlPullParser* parser, Style* style) {
     }
 
     transformReferenceFromNamespace(parser, u"", &maybeKey.value());
+    maybeKey.value().setSource(source);
 
     std::unique_ptr<Item> value = parseXml(parser, 0, kAllowRawString);
     if (!value) {
index acf5bb5..02bff2c 100644 (file)
@@ -67,6 +67,28 @@ struct ExtendedTypes {
 };
 
 /**
+ * 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 {
index 0590610..f42e6b7 100644 (file)
@@ -108,15 +108,19 @@ struct MapFlattenVisitor : public RawValueVisitor {
     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 @@ struct MapFlattenVisitor : public RawValueVisitor {
         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 @@ struct MapFlattenVisitor : public RawValueVisitor {
 
         for (Style::Entry& entry : style->entries) {
             flattenEntry(&entry.key, entry.value.get());
+            flattenMetaData(&entry.key);
         }
     }
 
@@ -218,6 +249,7 @@ struct MapFlattenVisitor : public RawValueVisitor {
         for (auto& attrRef : styleable->entries) {
             BinaryPrimitive val(Res_value{});
             flattenEntry(&attrRef, &val);
+            flattenMetaData(&attrRef);
         }
     }
 
@@ -227,6 +259,7 @@ struct MapFlattenVisitor : public RawValueVisitor {
             flattenValue(item.get(), outEntry);
             outEntry->value.size = util::hostToDevice16(sizeof(outEntry->value));
             mEntryCount++;
+            flattenMetaData(item.get());
         }
     }
 
@@ -270,6 +303,7 @@ struct MapFlattenVisitor : public RawValueVisitor {
 
             Reference key(q);
             flattenEntry(&key, plural->values[i].get());
+            flattenMetaData(plural->values[i].get());
         }
     }
 };
@@ -389,7 +423,8 @@ private:
         } 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) {
index 3f64d7b..2743539 100644 (file)
@@ -133,7 +133,7 @@ public:
                 // 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 @@ public:
                 }
 
             } 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 @@ const ISymbolTable::Symbol* ReferenceLinker::resolveSymbolCheckVisibility(
         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 @@ const ISymbolTable::Symbol* ReferenceLinker::resolveAttributeCheckVisibility(
     }
 
     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 @@ Maybe<xml::AaptAttribute> ReferenceLinker::compileXmlAttribute(const Reference&
     }
 
     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 @@ bool ReferenceLinker::linkReference(Reference* reference, IAaptContext* context,
 
     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;
index 6f11d58..a0eb00c 100644 (file)
@@ -80,6 +80,13 @@ struct ReferenceLinker : public IResourceTableConsumer {
                                                          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.
index bd42b67..5cc7aa7 100644 (file)
@@ -575,11 +575,10 @@ bool BinaryResourceParser::parseType(const ResourceTablePackage* package,
 
         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 @@ std::unique_ptr<Style> BinaryResourceParser::parseStyle(const ResourceNameRef& n
     }
 
     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 @@ std::unique_ptr<Attribute> BinaryResourceParser::parseAttr(const ResourceNameRef
     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 @@ std::unique_ptr<Styleable> BinaryResourceParser::parseStyleable(const ResourceNa
                                                                 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 @@ std::unique_ptr<Plural> BinaryResourceParser::parsePlural(const ResourceNameRef&
                                                           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);
index 73fcf52..0745a59 100644 (file)
@@ -91,6 +91,13 @@ private:
                                               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;
 
index a898619..0dacbd7 100644 (file)
@@ -158,6 +158,15 @@ inline StringPiece16 getString(const android::ResStringPool& pool, size_t idx) {
     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