AAPT2: Verify min/max attr fields

Integers are now checked to see if they fall in the range
of min/max for the attribute they are assigned.

Change-Id: I42c435b15fd3f0bd23691c83efccce4ad5973276

14 files changed, 311 insertions, 95 deletions

diff --git a/tools/aapt2/Diagnostics.h b/tools/aapt2/Diagnostics.h
index 7ea26b3ed0df..ab4d284516e1 100644
--- a/tools/aapt2/Diagnostics.h
+++ b/tools/aapt2/Diagnostics.h
@@ -18,7 +18,6 @@
 #define AAPT_DIAGNOSTICS_H
 
 #include "Source.h"
-
 #include "util/StringPiece.h"
 #include "util/Util.h"
 
diff --git a/tools/aapt2/ResourceParser.cpp b/tools/aapt2/ResourceParser.cpp
index 02fe59c0a03b..f6a720a6f2e4 100644
--- a/tools/aapt2/ResourceParser.cpp
+++ b/tools/aapt2/ResourceParser.cpp
@@ -711,6 +711,46 @@ bool ResourceParser::parseAttrImpl(xml::XmlPullParser* parser, ParsedResource* o
         }
     }
 
+    Maybe<int32_t> maybeMin, maybeMax;
+
+    if (Maybe<StringPiece16> maybeMinStr = xml::findAttribute(parser, u"min")) {
+        StringPiece16 minStr = util::trimWhitespace(maybeMinStr.value());
+        if (!minStr.empty()) {
+            android::Res_value value;
+            if (android::ResTable::stringToInt(minStr.data(), minStr.size(), &value)) {
+                maybeMin = static_cast<int32_t>(value.data);
+            }
+        }
+
+        if (!maybeMin) {
+            mDiag->error(DiagMessage(mSource.withLine(parser->getLineNumber()))
+                         << "invalid 'min' value '" << minStr << "'");
+            return false;
+        }
+    }
+
+    if (Maybe<StringPiece16> maybeMaxStr = xml::findAttribute(parser, u"max")) {
+        StringPiece16 maxStr = util::trimWhitespace(maybeMaxStr.value());
+        if (!maxStr.empty()) {
+            android::Res_value value;
+            if (android::ResTable::stringToInt(maxStr.data(), maxStr.size(), &value)) {
+                maybeMax = static_cast<int32_t>(value.data);
+            }
+        }
+
+        if (!maybeMax) {
+            mDiag->error(DiagMessage(mSource.withLine(parser->getLineNumber()))
+                         << "invalid 'max' value '" << maxStr << "'");
+            return false;
+        }
+    }
+
+    if ((maybeMin || maybeMax) && (typeMask & android::ResTable_map::TYPE_INTEGER) == 0) {
+        mDiag->error(DiagMessage(mSource.withLine(parser->getLineNumber()))
+                     << "'min' and 'max' can only be used when format='integer'");
+        return false;
+    }
+
     struct SymbolComparator {
         bool operator()(const Attribute::Symbol& a, const Attribute::Symbol& b) {
             return a.symbol.name.value() < b.symbol.name.value();
@@ -794,6 +834,13 @@ bool ResourceParser::parseAttrImpl(xml::XmlPullParser* parser, ParsedResource* o
     std::unique_ptr<Attribute> attr = util::make_unique<Attribute>(weak);
     attr->symbols = std::vector<Attribute::Symbol>(items.begin(), items.end());
     attr->typeMask = typeMask ? typeMask : uint32_t(android::ResTable_map::TYPE_ANY);
+    if (maybeMin) {
+        attr->minInt = maybeMin.value();
+    }
+
+    if (maybeMax) {
+        attr->maxInt = maybeMax.value();
+    }
     outResource->value = std::move(attr);
     return true;
 }
diff --git a/tools/aapt2/ResourceParser_test.cpp b/tools/aapt2/ResourceParser_test.cpp
index ab16424db709..6e0812bf86e5 100644
--- a/tools/aapt2/ResourceParser_test.cpp
+++ b/tools/aapt2/ResourceParser_test.cpp
@@ -138,6 +138,22 @@ TEST_F(ResourceParserTest, ParseAttr) {
     EXPECT_EQ(uint32_t(android::ResTable_map::TYPE_ANY), attr->typeMask);
 }
 
+TEST_F(ResourceParserTest, ParseAttrWithMinMax) {
+    std::string input = "<attr name=\"foo\" min=\"10\" max=\"23\" format=\"integer\"/>";
+    ASSERT_TRUE(testParse(input));
+
+    Attribute* attr = test::getValue<Attribute>(&mTable, u"@attr/foo");
+    ASSERT_NE(nullptr, attr);
+    EXPECT_EQ(uint32_t(android::ResTable_map::TYPE_INTEGER), attr->typeMask);
+    EXPECT_EQ(10, attr->minInt);
+    EXPECT_EQ(23, attr->maxInt);
+}
+
+TEST_F(ResourceParserTest, FailParseAttrWithMinMaxButNotInteger) {
+    std::string input = "<attr name=\"foo\" min=\"10\" max=\"23\" format=\"string\"/>";
+    ASSERT_FALSE(testParse(input));
+}
+
 TEST_F(ResourceParserTest, ParseUseAndDeclOfAttr) {
     std::string input = "<declare-styleable name=\"Styleable\">\n"
                         "  <attr name=\"foo\" />\n"
diff --git a/tools/aapt2/ResourceValues.cpp b/tools/aapt2/ResourceValues.cpp
index 5550f192d51a..04c375f5f974 100644
--- a/tools/aapt2/ResourceValues.cpp
+++ b/tools/aapt2/ResourceValues.cpp
@@ -15,9 +15,9 @@
  */
 
 #include "Resource.h"
+#include "ResourceUtils.h"
 #include "ResourceValues.h"
 #include "ValueVisitor.h"
-
 #include "util/Util.h"
 #include "flatten/ResourceTypeExtensions.h"
 
@@ -216,7 +216,7 @@ void BinaryPrimitive::print(std::ostream* out) const {
             *out << "(null)";
             break;
         case android::Res_value::TYPE_INT_DEC:
-            *out << "(integer) " << value.data;
+            *out << "(integer) " << static_cast<int32_t>(value.data);
             break;
         case android::Res_value::TYPE_INT_HEX:
             *out << "(integer) " << std::hex << value.data << std::dec;
@@ -237,7 +237,10 @@ void BinaryPrimitive::print(std::ostream* out) const {
     }
 }
 
-Attribute::Attribute(bool w, uint32_t t) : weak(w), typeMask(t) {
+Attribute::Attribute(bool w, uint32_t t) :
+        weak(w), typeMask(t),
+        minInt(std::numeric_limits<int32_t>::min()),
+        maxInt(std::numeric_limits<int32_t>::max()) {
 }
 
 bool Attribute::isWeak() const {
@@ -361,6 +364,81 @@ void Attribute::print(std::ostream* out) const {
     }
 }
 
+static void buildAttributeMismatchMessage(DiagMessage* msg, const Attribute* attr,
+                                          const Item* value) {
+    *msg << "expected";
+    if (attr->typeMask & android::ResTable_map::TYPE_BOOLEAN) {
+        *msg << " boolean";
+    }
+
+    if (attr->typeMask & android::ResTable_map::TYPE_COLOR) {
+        *msg << " color";
+    }
+
+    if (attr->typeMask & android::ResTable_map::TYPE_DIMENSION) {
+        *msg << " dimension";
+    }
+
+    if (attr->typeMask & android::ResTable_map::TYPE_ENUM) {
+        *msg << " enum";
+    }
+
+    if (attr->typeMask & android::ResTable_map::TYPE_FLAGS) {
+        *msg << " flags";
+    }
+
+    if (attr->typeMask & android::ResTable_map::TYPE_FLOAT) {
+        *msg << " float";
+    }
+
+    if (attr->typeMask & android::ResTable_map::TYPE_FRACTION) {
+        *msg << " fraction";
+    }
+
+    if (attr->typeMask & android::ResTable_map::TYPE_INTEGER) {
+        *msg << " integer";
+    }
+
+    if (attr->typeMask & android::ResTable_map::TYPE_REFERENCE) {
+        *msg << " reference";
+    }
+
+    if (attr->typeMask & android::ResTable_map::TYPE_STRING) {
+        *msg << " string";
+    }
+
+    *msg << " but got " << *value;
+}
+
+bool Attribute::matches(const Item* item, DiagMessage* outMsg) const {
+    android::Res_value val = {};
+    item->flatten(&val);
+
+    // Always allow references.
+    const uint32_t mask = typeMask | android::ResTable_map::TYPE_REFERENCE;
+    if (!(mask & ResourceUtils::androidTypeToAttributeTypeMask(val.dataType))) {
+        if (outMsg) {
+            buildAttributeMismatchMessage(outMsg, this, item);
+        }
+        return false;
+
+    } else if (ResourceUtils::androidTypeToAttributeTypeMask(val.dataType) &
+            android::ResTable_map::TYPE_INTEGER) {
+        if (static_cast<int32_t>(util::deviceToHost32(val.data)) < minInt) {
+            if (outMsg) {
+                *outMsg << *item << " is less than minimum integer " << minInt;
+            }
+            return false;
+        } else if (static_cast<int32_t>(util::deviceToHost32(val.data)) > maxInt) {
+            if (outMsg) {
+                *outMsg << *item << " is greater than maximum integer " << maxInt;
+            }
+            return false;
+        }
+    }
+    return true;
+}
+
 Style* Style::clone(StringPool* newPool) const {
     Style* style = new Style();
     style->parent = parent;
diff --git a/tools/aapt2/ResourceValues.h b/tools/aapt2/ResourceValues.h
index 7ae346aab370..a03828206c91 100644
--- a/tools/aapt2/ResourceValues.h
+++ b/tools/aapt2/ResourceValues.h
@@ -17,9 +17,10 @@
 #ifndef AAPT_RESOURCE_VALUES_H
 #define AAPT_RESOURCE_VALUES_H
 
-#include "util/Maybe.h"
+#include "Diagnostics.h"
 #include "Resource.h"
 #include "StringPool.h"
+#include "util/Maybe.h"
 
 #include <array>
 #include <androidfw/ResourceTypes.h>
@@ -233,8 +234,8 @@ struct Attribute : public BaseValue<Attribute> {
 
 	bool weak;
     uint32_t typeMask;
-    uint32_t minInt;
-    uint32_t maxInt;
+    int32_t minInt;
+    int32_t maxInt;
     std::vector<Symbol> symbols;
 
     Attribute(bool w, uint32_t t = 0u);
@@ -243,6 +244,7 @@ struct Attribute : public BaseValue<Attribute> {
     Attribute* clone(StringPool* newPool) const override;
     void printMask(std::ostream* out) const;
     void print(std::ostream* out) const override;
+    bool matches(const Item* item, DiagMessage* outMsg) const;
 };
 
 struct Style : public BaseValue<Style> {
diff --git a/tools/aapt2/Source.h b/tools/aapt2/Source.h
index 8af203cdad0e..319528e0ea1b 100644
--- a/tools/aapt2/Source.h
+++ b/tools/aapt2/Source.h
@@ -58,6 +58,10 @@ inline ::std::ostream& operator<<(::std::ostream& out, const Source& source) {
     return out;
 }
 
+inline bool operator==(const Source& lhs, const Source& rhs) {
+    return lhs.path == rhs.path && lhs.line == rhs.line;
+}
+
 inline bool operator<(const Source& lhs, const Source& rhs) {
     int cmp = lhs.path.compare(rhs.path);
     if (cmp < 0) return true;
diff --git a/tools/aapt2/flatten/TableFlattener.cpp b/tools/aapt2/flatten/TableFlattener.cpp
index 636e9774156e..05906102528f 100644
--- a/tools/aapt2/flatten/TableFlattener.cpp
+++ b/tools/aapt2/flatten/TableFlattener.cpp
@@ -165,6 +165,18 @@ struct MapFlattenVisitor : public RawValueVisitor {
             flattenEntry(&key, &val);
         }
 
+        if (attr->minInt != std::numeric_limits<int32_t>::min()) {
+            Reference key(ResourceId{ ResTable_map::ATTR_MIN });
+            BinaryPrimitive val(Res_value::TYPE_INT_DEC, static_cast<uint32_t>(attr->minInt));
+            flattenEntry(&key, &val);
+        }
+
+        if (attr->maxInt != std::numeric_limits<int32_t>::max()) {
+            Reference key(ResourceId{ ResTable_map::ATTR_MAX });
+            BinaryPrimitive val(Res_value::TYPE_INT_DEC, static_cast<uint32_t>(attr->maxInt));
+            flattenEntry(&key, &val);
+        }
+
         for (Attribute::Symbol& s : attr->symbols) {
             BinaryPrimitive val(Res_value::TYPE_INT_DEC, s.value);
             flattenEntry(&s.symbol, &val);
diff --git a/tools/aapt2/flatten/TableFlattener_test.cpp b/tools/aapt2/flatten/TableFlattener_test.cpp
index 4ffb980b122d..7030603e5bbd 100644
--- a/tools/aapt2/flatten/TableFlattener_test.cpp
+++ b/tools/aapt2/flatten/TableFlattener_test.cpp
@@ -262,4 +262,55 @@ TEST_F(TableFlattenerTest, FlattenUnlinkedTable) {
     EXPECT_EQ(ref->name.value(), test::parseNameOrDie(u"@com.app.test:color/green"));
 }
 
+TEST_F(TableFlattenerTest, FlattenMinMaxAttributes) {
+    Attribute attr(false);
+    attr.typeMask = android::ResTable_map::TYPE_INTEGER;
+    attr.minInt = 10;
+    attr.maxInt = 23;
+    std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder()
+            .setPackageId(u"android", 0x01)
+            .addValue(u"@android:attr/foo", ResourceId(0x01010000),
+                      util::make_unique<Attribute>(attr))
+            .build();
+
+    ResourceTable result;
+    ASSERT_TRUE(flatten(table.get(), &result));
+
+    Attribute* actualAttr = test::getValue<Attribute>(&result, u"@android:attr/foo");
+    ASSERT_NE(nullptr, actualAttr);
+    EXPECT_EQ(attr.isWeak(), actualAttr->isWeak());
+    EXPECT_EQ(attr.typeMask, actualAttr->typeMask);
+    EXPECT_EQ(attr.minInt, actualAttr->minInt);
+    EXPECT_EQ(attr.maxInt, actualAttr->maxInt);
+}
+
+TEST_F(TableFlattenerTest, FlattenSourceAndCommentsForChildrenOfCompoundValues) {
+    Style style;
+    Reference key(test::parseNameOrDie(u"@android:attr/foo"));
+    key.id = ResourceId(0x01010000);
+    key.setSource(Source("test").withLine(2));
+    key.setComment(StringPiece16(u"comment"));
+    style.entries.push_back(Style::Entry{ key, util::make_unique<Id>() });
+
+    test::ResourceTableBuilder builder = test::ResourceTableBuilder();
+    std::unique_ptr<ResourceTable> table = builder
+            .setPackageId(u"android", 0x01)
+            .addValue(u"@android:attr/foo", ResourceId(0x01010000),
+                      test::AttributeBuilder().build())
+            .addValue(u"@android:style/foo", ResourceId(0x01020000),
+                      std::unique_ptr<Style>(style.clone(builder.getStringPool())))
+            .build();
+
+    ResourceTable result;
+    ASSERT_TRUE(flatten(table.get(), &result));
+
+    Style* actualStyle = test::getValue<Style>(&result, u"@android:style/foo");
+    ASSERT_NE(nullptr, actualStyle);
+    ASSERT_EQ(1u, actualStyle->entries.size());
+
+    Reference* actualKey = &actualStyle->entries[0].key;
+    EXPECT_EQ(key.getSource(), actualKey->getSource());
+    EXPECT_EQ(key.getComment(), actualKey->getComment());
+}
+
 } // namespace aapt
diff --git a/tools/aapt2/link/ReferenceLinker.cpp b/tools/aapt2/link/ReferenceLinker.cpp
index 4b8253752d96..3f64d7b8c911 100644
--- a/tools/aapt2/link/ReferenceLinker.cpp
+++ b/tools/aapt2/link/ReferenceLinker.cpp
@@ -78,52 +78,6 @@ private:
         return value;
     }
 
-    void buildAttributeMismatchMessage(DiagMessage* msg, const Attribute* attr,
-                                       const Item* value) {
-        *msg << "expected";
-        if (attr->typeMask & android::ResTable_map::TYPE_BOOLEAN) {
-            *msg << " boolean";
-        }
-
-        if (attr->typeMask & android::ResTable_map::TYPE_COLOR) {
-            *msg << " color";
-        }
-
-        if (attr->typeMask & android::ResTable_map::TYPE_DIMENSION) {
-            *msg << " dimension";
-        }
-
-        if (attr->typeMask & android::ResTable_map::TYPE_ENUM) {
-            *msg << " enum";
-        }
-
-        if (attr->typeMask & android::ResTable_map::TYPE_FLAGS) {
-            *msg << " flags";
-        }
-
-        if (attr->typeMask & android::ResTable_map::TYPE_FLOAT) {
-            *msg << " float";
-        }
-
-        if (attr->typeMask & android::ResTable_map::TYPE_FRACTION) {
-            *msg << " fraction";
-        }
-
-        if (attr->typeMask & android::ResTable_map::TYPE_INTEGER) {
-            *msg << " integer";
-        }
-
-        if (attr->typeMask & android::ResTable_map::TYPE_REFERENCE) {
-            *msg << " reference";
-        }
-
-        if (attr->typeMask & android::ResTable_map::TYPE_STRING) {
-            *msg << " string";
-        }
-
-        *msg << " but got " << *value;
-    }
-
 public:
     using ValueVisitor::visit;
 
@@ -175,21 +129,19 @@ public:
                 entry.value->accept(this);
 
                 // Now verify that the type of this item is compatible with the attribute it
-                // is defined for.
-                android::Res_value val = {};
-                entry.value->flatten(&val);
-
-                // Always allow references.
-                const uint32_t typeMask = symbol->attribute->typeMask |
-                        android::ResTable_map::TYPE_REFERENCE;
-
-                if (!(typeMask & ResourceUtils::androidTypeToAttributeTypeMask(val.dataType))) {
+                // is defined for. We pass `nullptr` as the DiagMessage so that this check is
+                // 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());
-                    buildAttributeMismatchMessage(&msg, symbol->attribute.get(), entry.value.get());
+
+                    // Call the matches method again, this time with a DiagMessage so we fill
+                    // in the actual error message.
+                    symbol->attribute->matches(entry.value.get(), &msg);
                     mContext->getDiagnostics()->error(msg);
                     mError = true;
                 }
+
             } else {
                 DiagMessage msg(style->getSource());
                 msg << "style attribute '";
diff --git a/tools/aapt2/process/SymbolTable.cpp b/tools/aapt2/process/SymbolTable.cpp
index d04181d583bc..6ad2f9c10d22 100644
--- a/tools/aapt2/process/SymbolTable.cpp
+++ b/tools/aapt2/process/SymbolTable.cpp
@@ -61,7 +61,7 @@ const ISymbolTable::Symbol* SymbolTableWrapper::findByName(const ResourceName& n
         if (iter != sr.entry->values.end() && iter->config == kDefaultConfig) {
             // This resource has an Attribute.
             if (Attribute* attr = valueCast<Attribute>(iter->value.get())) {
-                symbol->attribute = std::unique_ptr<Attribute>(attr->clone(nullptr));
+                symbol->attribute = util::make_unique<Attribute>(*attr);
             } else {
                 return {};
             }
@@ -77,7 +77,6 @@ const ISymbolTable::Symbol* SymbolTableWrapper::findByName(const ResourceName& n
     return symbol.get();
 }
 
-
 static std::shared_ptr<ISymbolTable::Symbol> lookupAttributeInTable(const android::ResTable& table,
                                                                     ResourceId id) {
     // Try as a bag.
@@ -103,29 +102,40 @@ static std::shared_ptr<ISymbolTable::Symbol> lookupAttributeInTable(const androi
 
     if (s->attribute) {
         for (size_t i = 0; i < (size_t) count; i++) {
-            if (!Res_INTERNALID(entry[i].map.name.ident)) {
-                android::ResTable::resource_name entryName;
-                if (!table.getResourceName(entry[i].map.name.ident, false, &entryName)) {
-                    table.unlockBag(entry);
-                    return nullptr;
+            const android::ResTable_map& mapEntry = entry[i].map;
+            if (Res_INTERNALID(mapEntry.name.ident)) {
+                switch (mapEntry.name.ident) {
+                case android::ResTable_map::ATTR_MIN:
+                    s->attribute->minInt = static_cast<int32_t>(mapEntry.value.data);
+                    break;
+                case android::ResTable_map::ATTR_MAX:
+                    s->attribute->maxInt = static_cast<int32_t>(mapEntry.value.data);
+                    break;
                 }
+                continue;
+            }
 
-                const ResourceType* parsedType = parseResourceType(
-                        StringPiece16(entryName.type, entryName.typeLen));
-                if (!parsedType) {
-                    table.unlockBag(entry);
-                    return nullptr;
-                }
+            android::ResTable::resource_name entryName;
+            if (!table.getResourceName(mapEntry.name.ident, false, &entryName)) {
+                table.unlockBag(entry);
+                return nullptr;
+            }
 
-                Attribute::Symbol symbol;
-                symbol.symbol.name = ResourceNameRef(
-                        StringPiece16(entryName.package, entryName.packageLen),
-                        *parsedType,
-                        StringPiece16(entryName.name, entryName.nameLen)).toResourceName();
-                symbol.symbol.id = ResourceId(entry[i].map.name.ident);
-                symbol.value = entry[i].map.value.data;
-                s->attribute->symbols.push_back(std::move(symbol));
+            const ResourceType* parsedType = parseResourceType(
+                    StringPiece16(entryName.type, entryName.typeLen));
+            if (!parsedType) {
+                table.unlockBag(entry);
+                return nullptr;
             }
+
+            Attribute::Symbol symbol;
+            symbol.symbol.name = ResourceName(
+                    StringPiece16(entryName.package, entryName.packageLen),
+                    *parsedType,
+                    StringPiece16(entryName.name, entryName.nameLen));
+            symbol.symbol.id = ResourceId(mapEntry.name.ident);
+            symbol.value = mapEntry.value.data;
+            s->attribute->symbols.push_back(std::move(symbol));
         }
     }
     table.unlockBag(entry);
diff --git a/tools/aapt2/test/Builders.h b/tools/aapt2/test/Builders.h
index 9ca694a7ef61..f8e3d031fb67 100644
--- a/tools/aapt2/test/Builders.h
+++ b/tools/aapt2/test/Builders.h
@@ -36,6 +36,10 @@ private:
 public:
     ResourceTableBuilder() = default;
 
+    StringPool* getStringPool() {
+        return &mTable->stringPool;
+    }
+
     ResourceTableBuilder& setPackageId(const StringPiece16& packageName, uint8_t id) {
         ResourceTablePackage* package = mTable->createPackage(packageName, id);
         assert(package);
diff --git a/tools/aapt2/unflatten/BinaryResourceParser.cpp b/tools/aapt2/unflatten/BinaryResourceParser.cpp
index 49625b56acbb..bd42b6719efe 100644
--- a/tools/aapt2/unflatten/BinaryResourceParser.cpp
+++ b/tools/aapt2/unflatten/BinaryResourceParser.cpp
@@ -771,12 +771,20 @@ std::unique_ptr<Attribute> BinaryResourceParser::parseAttr(const ResourceNameRef
         attr->typeMask = util::deviceToHost32(typeMaskIter->value.data);
     }
 
-    if (attr->typeMask & (ResTable_map::TYPE_ENUM | ResTable_map::TYPE_FLAGS)) {
-        for (const ResTable_map& mapEntry : map) {
-            if (Res_INTERNALID(util::deviceToHost32(mapEntry.name.ident))) {
-                continue;
+    for (const ResTable_map& mapEntry : map) {
+        if (Res_INTERNALID(util::deviceToHost32(mapEntry.name.ident))) {
+            switch (util::deviceToHost32(mapEntry.name.ident)) {
+            case ResTable_map::ATTR_MIN:
+                attr->minInt = static_cast<int32_t>(mapEntry.value.data);
+                break;
+            case ResTable_map::ATTR_MAX:
+                attr->maxInt = static_cast<int32_t>(mapEntry.value.data);
+                break;
             }
+            continue;
+        }
 
+        if (attr->typeMask & (ResTable_map::TYPE_ENUM | ResTable_map::TYPE_FLAGS)) {
             Attribute::Symbol symbol;
             symbol.value = util::deviceToHost32(mapEntry.value.data);
             if (util::deviceToHost32(mapEntry.name.ident) == 0) {
@@ -799,7 +807,7 @@ std::unique_ptr<Attribute> BinaryResourceParser::parseAttr(const ResourceNameRef
         }
     }
 
-    // TODO(adamlesinski): Find min, max, i80n, etc attributes.
+    // TODO(adamlesinski): Find i80n, attributes.
     return attr;
 }
 
@@ -848,22 +856,22 @@ std::unique_ptr<Plural> BinaryResourceParser::parsePlural(const ResourceNameRef&
         }
 
         switch (util::deviceToHost32(mapEntry.name.ident)) {
-            case android::ResTable_map::ATTR_ZERO:
+            case ResTable_map::ATTR_ZERO:
                 plural->values[Plural::Zero] = std::move(item);
                 break;
-            case android::ResTable_map::ATTR_ONE:
+            case ResTable_map::ATTR_ONE:
                 plural->values[Plural::One] = std::move(item);
                 break;
-            case android::ResTable_map::ATTR_TWO:
+            case ResTable_map::ATTR_TWO:
                 plural->values[Plural::Two] = std::move(item);
                 break;
-            case android::ResTable_map::ATTR_FEW:
+            case ResTable_map::ATTR_FEW:
                 plural->values[Plural::Few] = std::move(item);
                 break;
-            case android::ResTable_map::ATTR_MANY:
+            case ResTable_map::ATTR_MANY:
                 plural->values[Plural::Many] = std::move(item);
                 break;
-            case android::ResTable_map::ATTR_OTHER:
+            case ResTable_map::ATTR_OTHER:
                 plural->values[Plural::Other] = std::move(item);
                 break;
         }
diff --git a/tools/aapt2/util/Maybe.h b/tools/aapt2/util/Maybe.h
index 1f7d5ce901b4..aa409ea62ade 100644
--- a/tools/aapt2/util/Maybe.h
+++ b/tools/aapt2/util/Maybe.h
@@ -275,6 +275,29 @@ inline Maybe<T> make_nothing() {
     return Maybe<T>();
 }
 
+/**
+ * Define the == operator between Maybe<T> and Maybe<U> if the operator T == U is defined.
+ * Otherwise this won't be defined and the compiler will yell at the callsite instead of inside
+ * Maybe.h.
+ */
+template <typename T, typename U>
+auto operator==(const Maybe<T>& a, const Maybe<U>& b)
+-> decltype(std::declval<T> == std::declval<U>) {
+    if (a && b) {
+        return a.value() == b.value();
+    }
+    return false;
+}
+
+/**
+ * Same as operator== but negated.
+ */
+template <typename T, typename U>
+auto operator!=(const Maybe<T>& a, const Maybe<U>& b)
+-> decltype(std::declval<T> == std::declval<U>) {
+    return !(a == b);
+}
+
 } // namespace aapt
 
 #endif // AAPT_MAYBE_H
diff --git a/tools/aapt2/util/Maybe_test.cpp b/tools/aapt2/util/Maybe_test.cpp
index d2c33cac7aa4..9cca40ea631f 100644
--- a/tools/aapt2/util/Maybe_test.cpp
+++ b/tools/aapt2/util/Maybe_test.cpp
@@ -119,4 +119,14 @@ TEST(MaybeTest, MoveAssign) {
     }
 }
 
+TEST(MaybeTest, Equality) {
+    Maybe<int> a = 1;
+    Maybe<int> b = 1;
+    Maybe<int> c;
+
+    EXPECT_EQ(a, b);
+    EXPECT_EQ(b, a);
+    EXPECT_NE(a, c);
+}
+
 } // namespace aapt