From 8f7c550e20a6edbc9af7bb48675afaf8bcb3783f Mon Sep 17 00:00:00 2001 From: Adam Lesinski Date: Thu, 2 Mar 2017 17:45:01 -0800 Subject: [PATCH] AAPT2: Fix Plural::Equals() method Test: make aapt2_tests Bug: 35902437 Change-Id: I8797f89af58876f891f0b0c5cce85fd7781c4e24 --- tools/aapt2/ResourceParser_test.cpp | 10 ++ tools/aapt2/ResourceValues.cpp | 39 +++--- tools/aapt2/ResourceValues_test.cpp | 151 +++++++++++++++++++++++ tools/aapt2/optimize/ResourceDeduper.cpp | 2 + 4 files changed, 186 insertions(+), 16 deletions(-) create mode 100644 tools/aapt2/ResourceValues_test.cpp diff --git a/tools/aapt2/ResourceParser_test.cpp b/tools/aapt2/ResourceParser_test.cpp index 67ed476b7a990..eefa320a44184 100644 --- a/tools/aapt2/ResourceParser_test.cpp +++ b/tools/aapt2/ResourceParser_test.cpp @@ -582,6 +582,16 @@ TEST_F(ResourceParserTest, ParsePlural) { " apple\n" ""; ASSERT_TRUE(TestParse(input)); + + Plural* plural = test::GetValue(&table_, "plurals/foo"); + ASSERT_NE(nullptr, plural); + EXPECT_EQ(nullptr, plural->values[Plural::Zero]); + EXPECT_EQ(nullptr, plural->values[Plural::Two]); + EXPECT_EQ(nullptr, plural->values[Plural::Few]); + EXPECT_EQ(nullptr, plural->values[Plural::Many]); + + EXPECT_NE(nullptr, plural->values[Plural::One]); + EXPECT_NE(nullptr, plural->values[Plural::Other]); } TEST_F(ResourceParserTest, ParseCommentsWithResource) { diff --git a/tools/aapt2/ResourceValues.cpp b/tools/aapt2/ResourceValues.cpp index 2868b2acee2d5..0cb8c67705f9d 100644 --- a/tools/aapt2/ResourceValues.cpp +++ b/tools/aapt2/ResourceValues.cpp @@ -341,7 +341,7 @@ Attribute::Attribute(bool w, uint32_t t) } template -T* addPointer(T& val) { +constexpr T* add_pointer(T& val) { return &val; } @@ -362,7 +362,7 @@ bool Attribute::Equals(const Value* value) const { std::vector sorted_a; std::transform(symbols.begin(), symbols.end(), std::back_inserter(sorted_a), - addPointer); + add_pointer); std::sort(sorted_a.begin(), sorted_a.end(), [](const Symbol* a, const Symbol* b) -> bool { return a->symbol.name < b->symbol.name; @@ -370,7 +370,7 @@ bool Attribute::Equals(const Value* value) const { std::vector sorted_b; std::transform(other->symbols.begin(), other->symbols.end(), - std::back_inserter(sorted_b), addPointer); + std::back_inserter(sorted_b), add_pointer); std::sort(sorted_b.begin(), sorted_b.end(), [](const Symbol* a, const Symbol* b) -> bool { return a->symbol.name < b->symbol.name; @@ -599,7 +599,7 @@ bool Style::Equals(const Value* value) const { std::vector sorted_a; std::transform(entries.begin(), entries.end(), std::back_inserter(sorted_a), - addPointer); + add_pointer); std::sort(sorted_a.begin(), sorted_a.end(), [](const Entry* a, const Entry* b) -> bool { return a->key.name < b->key.name; @@ -607,7 +607,7 @@ bool Style::Equals(const Value* value) const { std::vector sorted_b; std::transform(other->entries.begin(), other->entries.end(), - std::back_inserter(sorted_b), addPointer); + std::back_inserter(sorted_b), add_pointer); std::sort(sorted_b.begin(), sorted_b.end(), [](const Entry* a, const Entry* b) -> bool { return a->key.name < b->key.name; @@ -695,18 +695,21 @@ bool Plural::Equals(const Value* value) const { return false; } - if (values.size() != other->values.size()) { - return false; + auto one_iter = values.begin(); + auto one_end_iter = values.end(); + auto two_iter = other->values.begin(); + for (; one_iter != one_end_iter; ++one_iter, ++two_iter) { + const std::unique_ptr& a = *one_iter; + const std::unique_ptr& b = *two_iter; + if (a != nullptr && b != nullptr) { + if (!a->Equals(b.get())) { + return false; + } + } else if (a != b) { + return false; + } } - - return std::equal(values.begin(), values.end(), other->values.begin(), - [](const std::unique_ptr& a, - const std::unique_ptr& b) -> bool { - if (bool(a) != bool(b)) { - return false; - } - return bool(a) == bool(b) || a->Equals(b.get()); - }); + return true; } Plural* Plural::Clone(StringPool* new_pool) const { @@ -743,6 +746,10 @@ void Plural::Print(std::ostream* out) const { if (values[Many]) { *out << " many=" << *values[Many]; } + + if (values[Other]) { + *out << " other=" << *values[Other]; + } } static ::std::ostream& operator<<(::std::ostream& out, diff --git a/tools/aapt2/ResourceValues_test.cpp b/tools/aapt2/ResourceValues_test.cpp new file mode 100644 index 0000000000000..692258003471b --- /dev/null +++ b/tools/aapt2/ResourceValues_test.cpp @@ -0,0 +1,151 @@ +/* + * Copyright (C) 2017 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "ResourceValues.h" + +#include "test/Test.h" + +namespace aapt { + +TEST(ResourceValuesTest, PluralEquals) { + StringPool pool; + + Plural a; + a.values[Plural::One] = util::make_unique(pool.MakeRef("one")); + a.values[Plural::Other] = util::make_unique(pool.MakeRef("other")); + + Plural b; + b.values[Plural::One] = util::make_unique(pool.MakeRef("une")); + b.values[Plural::Other] = util::make_unique(pool.MakeRef("autre")); + + Plural c; + c.values[Plural::One] = util::make_unique(pool.MakeRef("one")); + c.values[Plural::Other] = util::make_unique(pool.MakeRef("other")); + + EXPECT_FALSE(a.Equals(&b)); + EXPECT_TRUE(a.Equals(&c)); +} + +TEST(ResourceValuesTest, PluralClone) { + StringPool pool; + + Plural a; + a.values[Plural::One] = util::make_unique(pool.MakeRef("one")); + a.values[Plural::Other] = util::make_unique(pool.MakeRef("other")); + + std::unique_ptr b(a.Clone(&pool)); + EXPECT_TRUE(a.Equals(b.get())); +} + +TEST(ResourceValuesTest, ArrayEquals) { + StringPool pool; + + Array a; + a.items.push_back(util::make_unique(pool.MakeRef("one"))); + a.items.push_back(util::make_unique(pool.MakeRef("two"))); + + Array b; + b.items.push_back(util::make_unique(pool.MakeRef("une"))); + b.items.push_back(util::make_unique(pool.MakeRef("deux"))); + + Array c; + c.items.push_back(util::make_unique(pool.MakeRef("uno"))); + + Array d; + d.items.push_back(util::make_unique(pool.MakeRef("one"))); + d.items.push_back(util::make_unique(pool.MakeRef("two"))); + + EXPECT_FALSE(a.Equals(&b)); + EXPECT_FALSE(a.Equals(&c)); + EXPECT_FALSE(b.Equals(&c)); + EXPECT_TRUE(a.Equals(&d)); +} + +TEST(ResourceValuesTest, ArrayClone) { + StringPool pool; + + Array a; + a.items.push_back(util::make_unique(pool.MakeRef("one"))); + a.items.push_back(util::make_unique(pool.MakeRef("two"))); + + std::unique_ptr b(a.Clone(&pool)); + EXPECT_TRUE(a.Equals(b.get())); +} + +TEST(ResourceValuesTest, StyleEquals) { + StringPool pool; + + std::unique_ptr