AAPT2: Separate out the various steps

An early refactor. Some ideas became clearer as
development continued. Now the various phases are much
clearer and more easily reusable.

Also added a ton of tests!

Change-Id: Ic8f0a70c8222370352e63533b329c40457c0903e

1 files changed, 221 insertions, 246 deletions

diff --git a/tools/aapt2/ResourceTable.cpp b/tools/aapt2/ResourceTable.cpp
index c93ecc768022..a1e7d36d91d9 100644
--- a/tools/aapt2/ResourceTable.cpp
+++ b/tools/aapt2/ResourceTable.cpp
@@ -15,11 +15,11 @@
  */
 
 #include "ConfigDescription.h"
-#include "Logger.h"
 #include "NameMangler.h"
 #include "ResourceTable.h"
 #include "ResourceValues.h"
-#include "Util.h"
+#include "ValueVisitor.h"
+#include "util/Util.h"
 
 #include <algorithm>
 #include <androidfw/ResourceTypes.h>
@@ -37,65 +37,109 @@ static bool lessThanType(const std::unique_ptr<ResourceTableType>& lhs, Resource
     return lhs->type < rhs;
 }
 
-static bool lessThanEntry(const std::unique_ptr<ResourceEntry>& lhs, const StringPiece16& rhs) {
+template <typename T>
+static bool lessThanStructWithName(const std::unique_ptr<T>& lhs,
+                                   const StringPiece16& rhs) {
     return lhs->name.compare(0, lhs->name.size(), rhs.data(), rhs.size()) < 0;
 }
 
-ResourceTable::ResourceTable() : mPackageId(kUnsetPackageId) {
-    // Make sure attrs always have type ID 1.
-    findOrCreateType(ResourceType::kAttr)->typeId = 1;
+ResourceTablePackage* ResourceTable::findPackage(const StringPiece16& name) {
+    const auto last = packages.end();
+    auto iter = std::lower_bound(packages.begin(), last, name,
+                                 lessThanStructWithName<ResourceTablePackage>);
+    if (iter != last && name == (*iter)->name) {
+        return iter->get();
+    }
+    return nullptr;
 }
 
-std::unique_ptr<ResourceTableType>& ResourceTable::findOrCreateType(ResourceType type) {
-    auto last = mTypes.end();
-    auto iter = std::lower_bound(mTypes.begin(), last, type, lessThanType);
-    if (iter != last) {
-        if ((*iter)->type == type) {
-            return *iter;
+ResourceTablePackage* ResourceTable::findPackageById(uint8_t id) {
+    for (auto& package : packages) {
+        if (package->id && package->id.value() == id) {
+            return package.get();
         }
     }
-    return *mTypes.emplace(iter, new ResourceTableType{ type });
+    return nullptr;
 }
 
-std::unique_ptr<ResourceEntry>& ResourceTable::findOrCreateEntry(
-        std::unique_ptr<ResourceTableType>& type, const StringPiece16& name) {
-    auto last = type->entries.end();
-    auto iter = std::lower_bound(type->entries.begin(), last, name, lessThanEntry);
-    if (iter != last) {
-        if (name == (*iter)->name) {
-            return *iter;
-        }
+ResourceTablePackage* ResourceTable::createPackage(const StringPiece16& name, uint8_t id) {
+    ResourceTablePackage* package = findOrCreatePackage(name);
+    if (!package->id) {
+        package->id = id;
+        return package;
     }
-    return *type->entries.emplace(iter, new ResourceEntry{ name });
+
+    if (package->id.value() == id) {
+        return package;
+    }
+    return nullptr;
 }
 
-struct IsAttributeVisitor : ConstValueVisitor {
-    bool isAttribute = false;
+ResourceTablePackage* ResourceTable::findOrCreatePackage(const StringPiece16& name) {
+    const auto last = packages.end();
+    auto iter = std::lower_bound(packages.begin(), last, name,
+                                 lessThanStructWithName<ResourceTablePackage>);
+    if (iter != last && name == (*iter)->name) {
+        return iter->get();
+    }
 
-    void visit(const Attribute&, ValueVisitorArgs&) override {
-        isAttribute = true;
+    std::unique_ptr<ResourceTablePackage> newPackage = util::make_unique<ResourceTablePackage>();
+    newPackage->name = name.toString();
+    return packages.emplace(iter, std::move(newPackage))->get();
+}
+
+ResourceTableType* ResourceTablePackage::findType(ResourceType type) {
+    const auto last = types.end();
+    auto iter = std::lower_bound(types.begin(), last, type, lessThanType);
+    if (iter != last && (*iter)->type == type) {
+        return iter->get();
     }
+    return nullptr;
+}
 
-    operator bool() {
-        return isAttribute;
+ResourceTableType* ResourceTablePackage::findOrCreateType(ResourceType type) {
+    const auto last = types.end();
+    auto iter = std::lower_bound(types.begin(), last, type, lessThanType);
+    if (iter != last && (*iter)->type == type) {
+        return iter->get();
     }
-};
+    return types.emplace(iter, new ResourceTableType{ type })->get();
+}
+
+ResourceEntry* ResourceTableType::findEntry(const StringPiece16& name) {
+    const auto last = entries.end();
+    auto iter = std::lower_bound(entries.begin(), last, name,
+                                 lessThanStructWithName<ResourceEntry>);
+    if (iter != last && name == (*iter)->name) {
+        return iter->get();
+    }
+    return nullptr;
+}
+
+ResourceEntry* ResourceTableType::findOrCreateEntry(const StringPiece16& name) {
+    auto last = entries.end();
+    auto iter = std::lower_bound(entries.begin(), last, name,
+                                 lessThanStructWithName<ResourceEntry>);
+    if (iter != last && name == (*iter)->name) {
+        return iter->get();
+    }
+    return entries.emplace(iter, new ResourceEntry{ name })->get();
+}
 
 /**
  * The default handler for collisions. A return value of -1 means keep the
  * existing value, 0 means fail, and +1 means take the incoming value.
  */
-static int defaultCollisionHandler(const Value& existing, const Value& incoming) {
-    IsAttributeVisitor existingIsAttr, incomingIsAttr;
-    existing.accept(existingIsAttr, {});
-    incoming.accept(incomingIsAttr, {});
+int ResourceTable::resolveValueCollision(Value* existing, Value* incoming) {
+    Attribute* existingAttr = valueCast<Attribute>(existing);
+    Attribute* incomingAttr = valueCast<Attribute>(incoming);
 
-    if (!incomingIsAttr) {
-        if (incoming.isWeak()) {
+    if (!incomingAttr) {
+        if (incoming->isWeak()) {
             // We're trying to add a weak resource but a resource
             // already exists. Keep the existing.
             return -1;
-        } else if (existing.isWeak()) {
+        } else if (existing->isWeak()) {
             // Override the weak resource with the new strong resource.
             return 1;
         }
@@ -104,8 +148,8 @@ static int defaultCollisionHandler(const Value& existing, const Value& incoming)
         return 0;
     }
 
-    if (!existingIsAttr) {
-        if (existing.isWeak()) {
+    if (!existingAttr) {
+        if (existing->isWeak()) {
             // The existing value is not an attribute and it is weak,
             // so take the incoming attribute value.
             return 1;
@@ -115,27 +159,27 @@ static int defaultCollisionHandler(const Value& existing, const Value& incoming)
         return 0;
     }
 
+    assert(incomingAttr && existingAttr);
+
     //
     // Attribute specific handling. At this point we know both
     // values are attributes. Since we can declare and define
     // attributes all-over, we do special handling to see
     // which definition sticks.
     //
-    const Attribute& existingAttr = static_cast<const Attribute&>(existing);
-    const Attribute& incomingAttr = static_cast<const Attribute&>(incoming);
-    if (existingAttr.typeMask == incomingAttr.typeMask) {
+    if (existingAttr->typeMask == incomingAttr->typeMask) {
         // The two attributes are both DECLs, but they are plain attributes
         // with the same formats.
         // Keep the strongest one.
-        return existingAttr.isWeak() ? 1 : -1;
+        return existingAttr->isWeak() ? 1 : -1;
     }
 
-    if (existingAttr.isWeak() && existingAttr.typeMask == android::ResTable_map::TYPE_ANY) {
+    if (existingAttr->isWeak() && existingAttr->typeMask == android::ResTable_map::TYPE_ANY) {
         // Any incoming attribute is better than this.
         return 1;
     }
 
-    if (incomingAttr.isWeak() && incomingAttr.typeMask == android::ResTable_map::TYPE_ANY) {
+    if (incomingAttr->isWeak() && incomingAttr->typeMask == android::ResTable_map::TYPE_ANY) {
         // The incoming attribute may be a USE instead of a DECL.
         // Keep the existing attribute.
         return -1;
@@ -147,180 +191,183 @@ static constexpr const char16_t* kValidNameChars = u"._-";
 static constexpr const char16_t* kValidNameMangledChars = u"._-$";
 
 bool ResourceTable::addResource(const ResourceNameRef& name, const ConfigDescription& config,
-                                const SourceLine& source, std::unique_ptr<Value> value) {
-    return addResourceImpl(name, ResourceId{}, config, source, std::move(value), kValidNameChars);
+                                const Source& source, std::unique_ptr<Value> value,
+                                IDiagnostics* diag) {
+    return addResourceImpl(name, ResourceId{}, config, source, std::move(value), kValidNameChars,
+                           diag);
 }
 
 bool ResourceTable::addResource(const ResourceNameRef& name, const ResourceId resId,
-                                const ConfigDescription& config, const SourceLine& source,
-                                std::unique_ptr<Value> value) {
-    return addResourceImpl(name, resId, config, source, std::move(value), kValidNameChars);
+                                const ConfigDescription& config, const Source& source,
+                                std::unique_ptr<Value> value, IDiagnostics* diag) {
+    return addResourceImpl(name, resId, config, source, std::move(value), kValidNameChars, diag);
+}
+
+bool ResourceTable::addFileReference(const ResourceNameRef& name, const ConfigDescription& config,
+                                     const Source& source, const StringPiece16& path,
+                                     IDiagnostics* diag) {
+    return addResourceImpl(name, ResourceId{}, config, source,
+                           util::make_unique<FileReference>(stringPool.makeRef(path)),
+                           kValidNameChars, diag);
 }
 
 bool ResourceTable::addResourceAllowMangled(const ResourceNameRef& name,
                                             const ConfigDescription& config,
-                                            const SourceLine& source,
-                                            std::unique_ptr<Value> value) {
+                                            const Source& source,
+                                            std::unique_ptr<Value> value,
+                                            IDiagnostics* diag) {
     return addResourceImpl(name, ResourceId{}, config, source, std::move(value),
-                           kValidNameMangledChars);
+                           kValidNameMangledChars, diag);
 }
 
 bool ResourceTable::addResourceImpl(const ResourceNameRef& name, const ResourceId resId,
-                                    const ConfigDescription& config, const SourceLine& source,
-                                    std::unique_ptr<Value> value, const char16_t* validChars) {
-    if (!name.package.empty() && name.package != mPackage) {
-        Logger::error(source)
-                << "resource '"
-                << name
-                << "' has incompatible package. Must be '"
-                << mPackage
-                << "'."
-                << std::endl;
+                                    const ConfigDescription& config, const Source& source,
+                                    std::unique_ptr<Value> value, const char16_t* validChars,
+                                    IDiagnostics* diag) {
+    auto badCharIter = util::findNonAlphaNumericAndNotInSet(name.entry, validChars);
+    if (badCharIter != name.entry.end()) {
+        diag->error(DiagMessage(source)
+                    << "resource '"
+                    << name
+                    << "' has invalid entry name '"
+                    << name.entry
+                    << "'. Invalid character '"
+                    << StringPiece16(badCharIter, 1)
+                    << "'");
         return false;
     }
 
-    auto badCharIter = util::findNonAlphaNumericAndNotInSet(name.entry, validChars);
-    if (badCharIter != name.entry.end()) {
-        Logger::error(source)
-                << "resource '"
-                << name
-                << "' has invalid entry name '"
-                << name.entry
-                << "'. Invalid character '"
-                << StringPiece16(badCharIter, 1)
-                << "'."
-                << std::endl;
+    ResourceTablePackage* package = findOrCreatePackage(name.package);
+    if (resId.isValid() && package->id && package->id.value() != resId.packageId()) {
+        diag->error(DiagMessage(source)
+                    << "trying to add resource '"
+                    << name
+                    << "' with ID "
+                    << resId
+                    << " but package '"
+                    << package->name
+                    << "' already has ID "
+                    << std::hex << (int) package->id.value() << std::dec);
         return false;
     }
 
-    std::unique_ptr<ResourceTableType>& type = findOrCreateType(name.type);
-    if (resId.isValid() && type->typeId != ResourceTableType::kUnsetTypeId &&
-            type->typeId != resId.typeId()) {
-        Logger::error(source)
-                << "trying to add resource '"
-                << name
-                << "' with ID "
-                << resId
-                << " but type '"
-                << type->type
-                << "' already has ID "
-                << std::hex << type->typeId << std::dec
-                << "."
-                << std::endl;
+    ResourceTableType* type = package->findOrCreateType(name.type);
+    if (resId.isValid() && type->id && type->id.value() != resId.typeId()) {
+        diag->error(DiagMessage(source)
+                    << "trying to add resource '"
+                    << name
+                    << "' with ID "
+                    << resId
+                    << " but type '"
+                    << type->type
+                    << "' already has ID "
+                    << std::hex << (int) type->id.value() << std::dec);
         return false;
     }
 
-    std::unique_ptr<ResourceEntry>& entry = findOrCreateEntry(type, name.entry);
-    if (resId.isValid() && entry->entryId != ResourceEntry::kUnsetEntryId &&
-            entry->entryId != resId.entryId()) {
-        Logger::error(source)
-                << "trying to add resource '"
-                << name
-                << "' with ID "
-                << resId
-                << " but resource already has ID "
-                << ResourceId(mPackageId, type->typeId, entry->entryId)
-                << "."
-                << std::endl;
+    ResourceEntry* entry = type->findOrCreateEntry(name.entry);
+    if (resId.isValid() && entry->id && entry->id.value() != resId.entryId()) {
+        diag->error(DiagMessage(source)
+                    << "trying to add resource '"
+                    << name
+                    << "' with ID "
+                    << resId
+                    << " but resource already has ID "
+                    << ResourceId(package->id.value(), type->id.value(), entry->id.value()));
         return false;
     }
 
-    const auto endIter = std::end(entry->values);
-    auto iter = std::lower_bound(std::begin(entry->values), endIter, config, compareConfigs);
+    const auto endIter = entry->values.end();
+    auto iter = std::lower_bound(entry->values.begin(), endIter, config, compareConfigs);
     if (iter == endIter || iter->config != config) {
         // This resource did not exist before, add it.
         entry->values.insert(iter, ResourceConfigValue{ config, source, {}, std::move(value) });
     } else {
-        int collisionResult = defaultCollisionHandler(*iter->value, *value);
+        int collisionResult = resolveValueCollision(iter->value.get(), value.get());
         if (collisionResult > 0) {
             // Take the incoming value.
             *iter = ResourceConfigValue{ config, source, {}, std::move(value) };
         } else if (collisionResult == 0) {
-            Logger::error(source)
-                    << "duplicate value for resource '" << name << "' "
-                    << "with config '" << iter->config << "'."
-                    << std::endl;
-
-            Logger::error(iter->source)
-                    << "resource previously defined here."
-                    << std::endl;
+            diag->error(DiagMessage(source)
+                        << "duplicate value for resource '" << name << "' "
+                        << "with config '" << iter->config << "'");
+            diag->error(DiagMessage(iter->source)
+                        << "resource previously defined here");
             return false;
         }
     }
 
     if (resId.isValid()) {
-        type->typeId = resId.typeId();
-        entry->entryId = resId.entryId();
+        package->id = resId.packageId();
+        type->id = resId.typeId();
+        entry->id = resId.entryId();
     }
     return true;
 }
 
 bool ResourceTable::markPublic(const ResourceNameRef& name, const ResourceId resId,
-                               const SourceLine& source) {
-    return markPublicImpl(name, resId, source, kValidNameChars);
+                               const Source& source, IDiagnostics* diag) {
+    return markPublicImpl(name, resId, source, kValidNameChars, diag);
 }
 
 bool ResourceTable::markPublicAllowMangled(const ResourceNameRef& name, const ResourceId resId,
-                                           const SourceLine& source) {
-    return markPublicImpl(name, resId, source, kValidNameMangledChars);
+                                           const Source& source, IDiagnostics* diag) {
+    return markPublicImpl(name, resId, source, kValidNameMangledChars, diag);
 }
 
 bool ResourceTable::markPublicImpl(const ResourceNameRef& name, const ResourceId resId,
-                                   const SourceLine& source, const char16_t* validChars) {
-    if (!name.package.empty() && name.package != mPackage) {
-        Logger::error(source)
-                << "resource '"
-                << name
-                << "' has incompatible package. Must be '"
-                << mPackage
-                << "'."
-            << std::endl;
+                                   const Source& source, const char16_t* validChars,
+                                   IDiagnostics* diag) {
+    auto badCharIter = util::findNonAlphaNumericAndNotInSet(name.entry, validChars);
+    if (badCharIter != name.entry.end()) {
+        diag->error(DiagMessage(source)
+                    << "resource '"
+                    << name
+                    << "' has invalid entry name '"
+                    << name.entry
+                    << "'. Invalid character '"
+                    << StringPiece16(badCharIter, 1)
+                    << "'");
         return false;
     }
 
-    auto badCharIter = util::findNonAlphaNumericAndNotInSet(name.entry, validChars);
-    if (badCharIter != name.entry.end()) {
-        Logger::error(source)
-                << "resource '"
-                << name
-                << "' has invalid entry name '"
-                << name.entry
-                << "'. Invalid character '"
-                << StringPiece16(badCharIter, 1)
-                << "'."
-                << std::endl;
+    ResourceTablePackage* package = findOrCreatePackage(name.package);
+    if (resId.isValid() && package->id && package->id.value() != resId.packageId()) {
+        diag->error(DiagMessage(source)
+                    << "trying to add resource '"
+                    << name
+                    << "' with ID "
+                    << resId
+                    << " but package '"
+                    << package->name
+                    << "' already has ID "
+                    << std::hex << (int) package->id.value() << std::dec);
         return false;
     }
 
-    std::unique_ptr<ResourceTableType>& type = findOrCreateType(name.type);
-    if (resId.isValid() && type->typeId != ResourceTableType::kUnsetTypeId &&
-            type->typeId != resId.typeId()) {
-        Logger::error(source)
-                << "trying to make resource '"
-                << name
-                << "' public with ID "
-                << resId
-                << " but type '"
-                << type->type
-                << "' already has ID "
-                << std::hex << type->typeId << std::dec
-                << "."
-                << std::endl;
+    ResourceTableType* type = package->findOrCreateType(name.type);
+    if (resId.isValid() && type->id && type->id.value() != resId.typeId()) {
+        diag->error(DiagMessage(source)
+                    << "trying to add resource '"
+                    << name
+                    << "' with ID "
+                    << resId
+                    << " but type '"
+                    << type->type
+                    << "' already has ID "
+                    << std::hex << (int) type->id.value() << std::dec);
         return false;
     }
 
-    std::unique_ptr<ResourceEntry>& entry = findOrCreateEntry(type, name.entry);
-    if (resId.isValid() && entry->entryId != ResourceEntry::kUnsetEntryId &&
-            entry->entryId != resId.entryId()) {
-        Logger::error(source)
-                << "trying to make resource '"
-                << name
-                << "' public with ID "
-                << resId
-                << " but resource already has ID "
-                << ResourceId(mPackageId, type->typeId, entry->entryId)
-                << "."
-                << std::endl;
+    ResourceEntry* entry = type->findOrCreateEntry(name.entry);
+    if (resId.isValid() && entry->id && entry->id.value() != resId.entryId()) {
+        diag->error(DiagMessage(source)
+                    << "trying to add resource '"
+                    << name
+                    << "' with ID "
+                    << resId
+                    << " but resource already has ID "
+                    << ResourceId(package->id.value(), type->id.value(), entry->id.value()));
         return false;
     }
 
@@ -329,102 +376,30 @@ bool ResourceTable::markPublicImpl(const ResourceNameRef& name, const ResourceId
     entry->publicStatus.source = source;
 
     if (resId.isValid()) {
-        type->typeId = resId.typeId();
-        entry->entryId = resId.entryId();
-    }
-    return true;
-}
-
-bool ResourceTable::merge(ResourceTable&& other) {
-    const bool mangleNames = mPackage != other.getPackage();
-    std::u16string mangledName;
-
-    for (auto& otherType : other) {
-        std::unique_ptr<ResourceTableType>& type = findOrCreateType(otherType->type);
-        if (otherType->publicStatus.isPublic) {
-            if (type->publicStatus.isPublic && type->typeId != otherType->typeId) {
-                Logger::error() << "can not merge type '" << type->type
-                                << "': conflicting public IDs "
-                                << "(" << type->typeId << " vs " << otherType->typeId << ")."
-                                << std::endl;
-                return false;
-            }
-            type->publicStatus = std::move(otherType->publicStatus);
-            type->typeId = otherType->typeId;
-        }
-
-        for (auto& otherEntry : otherType->entries) {
-            const std::u16string* nameToAdd = &otherEntry->name;
-            if (mangleNames) {
-                mangledName = otherEntry->name;
-                NameMangler::mangle(other.getPackage(), &mangledName);
-                nameToAdd = &mangledName;
-            }
-
-            std::unique_ptr<ResourceEntry>& entry = findOrCreateEntry(type, *nameToAdd);
-            if (otherEntry->publicStatus.isPublic) {
-                if (entry->publicStatus.isPublic && entry->entryId != otherEntry->entryId) {
-                    Logger::error() << "can not merge entry '" << type->type << "/" << entry->name
-                                    << "': conflicting public IDs "
-                                    << "(" << entry->entryId << " vs " << entry->entryId << ")."
-                                    << std::endl;
-                    return false;
-                }
-                entry->publicStatus = std::move(otherEntry->publicStatus);
-                entry->entryId = otherEntry->entryId;
-            }
-
-            for (ResourceConfigValue& otherValue : otherEntry->values) {
-                auto iter = std::lower_bound(entry->values.begin(), entry->values.end(),
-                                             otherValue.config, compareConfigs);
-                if (iter != entry->values.end() && iter->config == otherValue.config) {
-                    int collisionResult = defaultCollisionHandler(*iter->value, *otherValue.value);
-                    if (collisionResult > 0) {
-                        // Take the incoming value.
-                        iter->source = std::move(otherValue.source);
-                        iter->comment = std::move(otherValue.comment);
-                        iter->value = std::unique_ptr<Value>(otherValue.value->clone(&mValuePool));
-                    } else if (collisionResult == 0) {
-                        ResourceNameRef resourceName = { mPackage, type->type, entry->name };
-                        Logger::error(otherValue.source)
-                                << "resource '" << resourceName << "' has a conflicting value for "
-                                << "configuration (" << otherValue.config << ")."
-                                << std::endl;
-                        Logger::note(iter->source) << "originally defined here." << std::endl;
-                        return false;
-                    }
-                } else {
-                    entry->values.insert(iter, ResourceConfigValue{
-                            otherValue.config,
-                            std::move(otherValue.source),
-                            std::move(otherValue.comment),
-                            std::unique_ptr<Value>(otherValue.value->clone(&mValuePool)),
-                    });
-                }
-            }
-        }
+        package->id = resId.packageId();
+        type->id = resId.typeId();
+        entry->id = resId.entryId();
     }
     return true;
 }
 
-std::tuple<const ResourceTableType*, const ResourceEntry*>
-ResourceTable::findResource(const ResourceNameRef& name) const {
-    if (name.package != mPackage) {
+Maybe<ResourceTable::SearchResult>
+ResourceTable::findResource(const ResourceNameRef& name) {
+    ResourceTablePackage* package = findPackage(name.package);
+    if (!package) {
         return {};
     }
 
-    auto iter = std::lower_bound(mTypes.begin(), mTypes.end(), name.type, lessThanType);
-    if (iter == mTypes.end() || (*iter)->type != name.type) {
+    ResourceTableType* type = package->findType(name.type);
+    if (!type) {
         return {};
     }
 
-    const std::unique_ptr<ResourceTableType>& type = *iter;
-    auto iter2 = std::lower_bound(type->entries.begin(), type->entries.end(), name.entry,
-                                  lessThanEntry);
-    if (iter2 == type->entries.end() || name.entry != (*iter2)->name) {
+    ResourceEntry* entry = type->findEntry(name.entry);
+    if (!entry) {
         return {};
     }
-    return std::make_tuple(iter->get(), iter2->get());
+    return SearchResult{ package, type, entry };
 }
 
 } // namespace aapt