diff options
-rw-r--r-- | libs/androidfw/include/androidfw/StringPiece.h | 28 | ||||
-rw-r--r-- | tools/aapt2/Main.cpp | 2 | ||||
-rw-r--r-- | tools/aapt2/ResourceParser.cpp | 183 | ||||
-rw-r--r-- | tools/aapt2/ResourceParser.h | 20 | ||||
-rw-r--r-- | tools/aapt2/ResourceParser_test.cpp | 72 | ||||
-rw-r--r-- | tools/aapt2/ResourceValues.cpp | 47 | ||||
-rw-r--r-- | tools/aapt2/ResourceValues.h | 138 | ||||
-rw-r--r-- | tools/aapt2/StringPool.cpp | 30 | ||||
-rw-r--r-- | tools/aapt2/StringPool.h | 4 | ||||
-rw-r--r-- | tools/aapt2/compile/PseudolocaleGenerator.cpp | 103 | ||||
-rw-r--r-- | tools/aapt2/compile/PseudolocaleGenerator_test.cpp | 45 | ||||
-rw-r--r-- | tools/aapt2/readme.md | 4 |
12 files changed, 460 insertions, 216 deletions
diff --git a/libs/androidfw/include/androidfw/StringPiece.h b/libs/androidfw/include/androidfw/StringPiece.h index c9effd1a5112..02cc5052af3b 100644 --- a/libs/androidfw/include/androidfw/StringPiece.h +++ b/libs/androidfw/include/androidfw/StringPiece.h @@ -261,8 +261,36 @@ inline ::std::ostream& operator<<(::std::ostream& out, const BasicStringPiece<ch return out.write(str.data(), str.size()); } +template <typename TChar> +inline ::std::basic_string<TChar>& operator+=(::std::basic_string<TChar>& lhs, + const BasicStringPiece<TChar>& rhs) { + return lhs.append(rhs.data(), rhs.size()); +} + +template <typename TChar> +inline bool operator==(const ::std::basic_string<TChar>& lhs, const BasicStringPiece<TChar>& rhs) { + return rhs == lhs; +} + +template <typename TChar> +inline bool operator!=(const ::std::basic_string<TChar>& lhs, const BasicStringPiece<TChar>& rhs) { + return rhs != lhs; +} + } // namespace android +inline ::std::ostream& operator<<(::std::ostream& out, const std::u16string& str) { + ssize_t utf8_len = utf16_to_utf8_length(str.data(), str.size()); + if (utf8_len < 0) { + return out << "???"; + } + + std::string utf8; + utf8.resize(static_cast<size_t>(utf8_len)); + utf16_to_utf8(str.data(), str.size(), &*utf8.begin(), utf8_len + 1); + return out << utf8; +} + namespace std { template <typename TChar> diff --git a/tools/aapt2/Main.cpp b/tools/aapt2/Main.cpp index 227ffa34af4c..ba378d716793 100644 --- a/tools/aapt2/Main.cpp +++ b/tools/aapt2/Main.cpp @@ -25,7 +25,7 @@ namespace aapt { static const char* sMajorVersion = "2"; // Update minor version whenever a feature or flag is added. -static const char* sMinorVersion = "6"; +static const char* sMinorVersion = "7"; int PrintVersion() { std::cerr << "Android Asset Packaging Tool (aapt) " << sMajorVersion << "." diff --git a/tools/aapt2/ResourceParser.cpp b/tools/aapt2/ResourceParser.cpp index 1c750c6748b9..47ca2660f15a 100644 --- a/tools/aapt2/ResourceParser.cpp +++ b/tools/aapt2/ResourceParser.cpp @@ -26,6 +26,7 @@ #include "ResourceValues.h" #include "ValueVisitor.h" #include "util/ImmutableMap.h" +#include "util/Maybe.h" #include "util/Util.h" #include "xml/XmlPullParser.h" @@ -150,82 +151,108 @@ ResourceParser::ResourceParser(IDiagnostics* diag, ResourceTable* table, /** * Build a string from XML that converts nested elements into Span objects. */ -bool ResourceParser::FlattenXmlSubtree(xml::XmlPullParser* parser, - std::string* out_raw_string, - StyleString* out_style_string) { +bool ResourceParser::FlattenXmlSubtree( + xml::XmlPullParser* parser, std::string* out_raw_string, StyleString* out_style_string, + std::vector<UntranslatableSection>* out_untranslatable_sections) { + // Keeps track of formatting tags (<b>, <i>) and the range of characters for which they apply. std::vector<Span> span_stack; - bool error = false; + // Clear the output variables. out_raw_string->clear(); out_style_string->spans.clear(); + out_untranslatable_sections->clear(); + + // The StringBuilder will concatenate the various segments of text which are initially + // separated by tags. It also handles unicode escape codes and quotations. util::StringBuilder builder; + + // The first occurrence of a <xliff:g> tag. Nested <xliff:g> tags are illegal. + Maybe<size_t> untranslatable_start_depth; + size_t depth = 1; while (xml::XmlPullParser::IsGoodEvent(parser->Next())) { const xml::XmlPullParser::Event event = parser->event(); - if (event == xml::XmlPullParser::Event::kEndElement) { - if (!parser->element_namespace().empty()) { - // We already warned and skipped the start element, so just skip here - // too - continue; - } - - depth--; - if (depth == 0) { - break; - } - span_stack.back().last_char = builder.Utf16Len() - 1; - out_style_string->spans.push_back(span_stack.back()); - span_stack.pop_back(); + if (event == xml::XmlPullParser::Event::kStartElement) { + if (parser->element_namespace().empty()) { + // This is an HTML tag which we encode as a span. Add it to the span stack. + std::string span_name = parser->element_name(); + const auto end_attr_iter = parser->end_attributes(); + for (auto attr_iter = parser->begin_attributes(); attr_iter != end_attr_iter; ++attr_iter) { + span_name += ";"; + span_name += attr_iter->name; + span_name += "="; + span_name += attr_iter->value; + } - } else if (event == xml::XmlPullParser::Event::kText) { - out_raw_string->append(parser->text()); - builder.Append(parser->text()); + // Make sure the string is representable in our binary format. + if (builder.Utf16Len() > std::numeric_limits<uint32_t>::max()) { + diag_->Error(DiagMessage(source_.WithLine(parser->line_number())) + << "style string '" << builder.ToString() << "' is too long"); + return false; + } - } else if (event == xml::XmlPullParser::Event::kStartElement) { - if (!parser->element_namespace().empty()) { - if (parser->element_namespace() != sXliffNamespaceUri) { - // Only warn if this isn't an xliff namespace. - diag_->Warn(DiagMessage(source_.WithLine(parser->line_number())) - << "skipping element '" << parser->element_name() - << "' with unknown namespace '" - << parser->element_namespace() << "'"); + span_stack.push_back(Span{std::move(span_name), static_cast<uint32_t>(builder.Utf16Len())}); + } else if (parser->element_namespace() == sXliffNamespaceUri) { + if (parser->element_name() == "g") { + if (untranslatable_start_depth) { + // We've already encountered an <xliff:g> tag, and nested <xliff:g> tags are illegal. + diag_->Error(DiagMessage(source_.WithLine(parser->line_number())) + << "illegal nested XLIFF 'g' tag"); + return false; + } else { + // Mark the start of an untranslatable section. Use UTF8 indices/lengths. + untranslatable_start_depth = depth; + const size_t current_idx = builder.ToString().size(); + out_untranslatable_sections->push_back(UntranslatableSection{current_idx, current_idx}); + } } - continue; - } - depth++; + // Ignore other xliff tags, they get handled by other tools. - // Build a span object out of the nested element. - std::string span_name = parser->element_name(); - const auto end_attr_iter = parser->end_attributes(); - for (auto attr_iter = parser->begin_attributes(); - attr_iter != end_attr_iter; ++attr_iter) { - span_name += ";"; - span_name += attr_iter->name; - span_name += "="; - span_name += attr_iter->value; + } else { + // Besides XLIFF, any other namespaced tag is unsupported and ignored. + diag_->Warn(DiagMessage(source_.WithLine(parser->line_number())) + << "ignoring element '" << parser->element_name() + << "' with unknown namespace '" << parser->element_namespace() << "'"); } - if (builder.Utf16Len() > std::numeric_limits<uint32_t>::max()) { - diag_->Error(DiagMessage(source_.WithLine(parser->line_number())) - << "style string '" << builder.ToString() - << "' is too long"); - error = true; - } else { - span_stack.push_back( - Span{span_name, static_cast<uint32_t>(builder.Utf16Len())}); + // Enter one level inside the element. + depth++; + } else if (event == xml::XmlPullParser::Event::kText) { + // Record both the raw text and append to the builder to deal with escape sequences + // and quotations. + out_raw_string->append(parser->text()); + builder.Append(parser->text()); + } else if (event == xml::XmlPullParser::Event::kEndElement) { + // Return one level from within the element. + depth--; + if (depth == 0) { + break; } + if (parser->element_namespace().empty()) { + // This is an HTML tag which we encode as a span. Update the span + // stack and pop the top entry. + Span& top_span = span_stack.back(); + top_span.last_char = builder.Utf16Len() - 1; + out_style_string->spans.push_back(std::move(top_span)); + span_stack.pop_back(); + } else if (untranslatable_start_depth == make_value(depth)) { + // This is the end of an untranslatable section. Use UTF8 indices/lengths. + UntranslatableSection& untranslatable_section = out_untranslatable_sections->back(); + untranslatable_section.end = builder.ToString().size(); + untranslatable_start_depth = {}; + } } else if (event == xml::XmlPullParser::Event::kComment) { - // Skip + // Ignore. } else { LOG(FATAL) << "unhandled XML event"; } } - CHECK(span_stack.empty()) << "spans haven't been fully processed"; + CHECK(span_stack.empty()) << "spans haven't been fully processed"; out_style_string->str = builder.ToString(); - return !error; + return true; } bool ResourceParser::Parse(xml::XmlPullParser* parser) { @@ -548,15 +575,18 @@ std::unique_ptr<Item> ResourceParser::ParseXml(xml::XmlPullParser* parser, std::string raw_value; StyleString style_string; - if (!FlattenXmlSubtree(parser, &raw_value, &style_string)) { + std::vector<UntranslatableSection> untranslatable_sections; + if (!FlattenXmlSubtree(parser, &raw_value, &style_string, &untranslatable_sections)) { return {}; } if (!style_string.spans.empty()) { // This can only be a StyledString. - return util::make_unique<StyledString>(table_->string_pool.MakeRef( - style_string, - StringPool::Context(StringPool::Context::kStylePriority, config_))); + std::unique_ptr<StyledString> styled_string = + util::make_unique<StyledString>(table_->string_pool.MakeRef( + style_string, StringPool::Context(StringPool::Context::kStylePriority, config_))); + styled_string->untranslatable_sections = std::move(untranslatable_sections); + return std::move(styled_string); } auto on_create_reference = [&](const ResourceName& name) { @@ -582,8 +612,10 @@ std::unique_ptr<Item> ResourceParser::ParseXml(xml::XmlPullParser* parser, // Try making a regular string. if (type_mask & android::ResTable_map::TYPE_STRING) { // Use the trimmed, escaped string. - return util::make_unique<String>(table_->string_pool.MakeRef( - style_string.str, StringPool::Context(config_))); + std::unique_ptr<String> string = util::make_unique<String>( + table_->string_pool.MakeRef(style_string.str, StringPool::Context(config_))); + string->untranslatable_sections = std::move(untranslatable_sections); + return std::move(string); } if (allow_raw_value) { @@ -609,17 +641,15 @@ bool ResourceParser::ParseString(xml::XmlPullParser* parser, formatted = maybe_formatted.value(); } - bool translateable = options_.translatable; - if (Maybe<StringPiece> translateable_attr = - xml::FindAttribute(parser, "translatable")) { - Maybe<bool> maybe_translateable = - ResourceUtils::ParseBool(translateable_attr.value()); - if (!maybe_translateable) { + bool translatable = options_.translatable; + if (Maybe<StringPiece> translatable_attr = xml::FindAttribute(parser, "translatable")) { + Maybe<bool> maybe_translatable = ResourceUtils::ParseBool(translatable_attr.value()); + if (!maybe_translatable) { diag_->Error(DiagMessage(out_resource->source) << "invalid value for 'translatable'. Must be a boolean"); return false; } - translateable = maybe_translateable.value(); + translatable = maybe_translatable.value(); } out_resource->value = @@ -630,9 +660,9 @@ bool ResourceParser::ParseString(xml::XmlPullParser* parser, } if (String* string_value = ValueCast<String>(out_resource->value.get())) { - string_value->SetTranslateable(translateable); + string_value->SetTranslatable(translatable); - if (formatted && translateable) { + if (formatted && translatable) { if (!util::VerifyJavaStringFormat(*string_value->value)) { DiagMessage msg(out_resource->source); msg << "multiple substitutions specified in non-positional format; " @@ -646,9 +676,8 @@ bool ResourceParser::ParseString(xml::XmlPullParser* parser, } } - } else if (StyledString* string_value = - ValueCast<StyledString>(out_resource->value.get())) { - string_value->SetTranslateable(translateable); + } else if (StyledString* string_value = ValueCast<StyledString>(out_resource->value.get())) { + string_value->SetTranslatable(translatable); } return true; } @@ -1151,19 +1180,17 @@ bool ResourceParser::ParseArrayImpl(xml::XmlPullParser* parser, std::unique_ptr<Array> array = util::make_unique<Array>(); - bool translateable = options_.translatable; - if (Maybe<StringPiece> translateable_attr = - xml::FindAttribute(parser, "translatable")) { - Maybe<bool> maybe_translateable = - ResourceUtils::ParseBool(translateable_attr.value()); - if (!maybe_translateable) { + bool translatable = options_.translatable; + if (Maybe<StringPiece> translatable_attr = xml::FindAttribute(parser, "translatable")) { + Maybe<bool> maybe_translatable = ResourceUtils::ParseBool(translatable_attr.value()); + if (!maybe_translatable) { diag_->Error(DiagMessage(out_resource->source) << "invalid value for 'translatable'. Must be a boolean"); return false; } - translateable = maybe_translateable.value(); + translatable = maybe_translatable.value(); } - array->SetTranslateable(translateable); + array->SetTranslatable(translatable); bool error = false; const size_t depth = parser->depth(); diff --git a/tools/aapt2/ResourceParser.h b/tools/aapt2/ResourceParser.h index cc0fa26f44d5..825801995862 100644 --- a/tools/aapt2/ResourceParser.h +++ b/tools/aapt2/ResourceParser.h @@ -60,16 +60,16 @@ class ResourceParser { private: DISALLOW_COPY_AND_ASSIGN(ResourceParser); - /* - * Parses the XML subtree as a StyleString (flattened XML representation for - * strings - * with formatting). If successful, `out_style_string` - * contains the escaped and whitespace trimmed text, while `out_raw_string` - * contains the unescaped text. Returns true on success. - */ - bool FlattenXmlSubtree(xml::XmlPullParser* parser, - std::string* out_raw_string, - StyleString* out_style_string); + // Parses the XML subtree as a StyleString (flattened XML representation for strings with + // formatting). If parsing fails, false is returned and the out parameters are left in an + // unspecified state. Otherwise, + // `out_style_string` contains the escaped and whitespace trimmed text. + // `out_raw_string` contains the un-escaped text. + // `out_untranslatable_sections` contains the sections of the string that should not be + // translated. + bool FlattenXmlSubtree(xml::XmlPullParser* parser, std::string* out_raw_string, + StyleString* out_style_string, + std::vector<UntranslatableSection>* out_untranslatable_sections); /* * Parses the XML subtree and returns an Item. diff --git a/tools/aapt2/ResourceParser_test.cpp b/tools/aapt2/ResourceParser_test.cpp index cf901dae11bb..67ed476b7a99 100644 --- a/tools/aapt2/ResourceParser_test.cpp +++ b/tools/aapt2/ResourceParser_test.cpp @@ -76,6 +76,7 @@ TEST_F(ResourceParserTest, ParseQuotedString) { String* str = test::GetValue<String>(&table_, "string/foo"); ASSERT_NE(nullptr, str); EXPECT_EQ(std::string(" hey there "), *str->value); + EXPECT_TRUE(str->untranslatable_sections.empty()); } TEST_F(ResourceParserTest, ParseEscapedString) { @@ -85,6 +86,7 @@ TEST_F(ResourceParserTest, ParseEscapedString) { String* str = test::GetValue<String>(&table_, "string/foo"); ASSERT_NE(nullptr, str); EXPECT_EQ(std::string("?123"), *str->value); + EXPECT_TRUE(str->untranslatable_sections.empty()); } TEST_F(ResourceParserTest, ParseFormattedString) { @@ -97,8 +99,7 @@ TEST_F(ResourceParserTest, ParseFormattedString) { TEST_F(ResourceParserTest, ParseStyledString) { // Use a surrogate pair unicode point so that we can verify that the span - // indices - // use UTF-16 length and not UTF-18 length. + // indices use UTF-16 length and not UTF-8 length. std::string input = "<string name=\"foo\">This is my aunt\u2019s <b>string</b></string>"; ASSERT_TRUE(TestParse(input)); @@ -109,6 +110,7 @@ TEST_F(ResourceParserTest, ParseStyledString) { const std::string expected_str = "This is my aunt\u2019s string"; EXPECT_EQ(expected_str, *str->value->str); EXPECT_EQ(1u, str->value->spans.size()); + EXPECT_TRUE(str->untranslatable_sections.empty()); EXPECT_EQ(std::string("b"), *str->value->spans[0].name); EXPECT_EQ(17u, str->value->spans[0].first_char); @@ -122,6 +124,7 @@ TEST_F(ResourceParserTest, ParseStringWithWhitespace) { String* str = test::GetValue<String>(&table_, "string/foo"); ASSERT_NE(nullptr, str); EXPECT_EQ(std::string("This is what I think"), *str->value); + EXPECT_TRUE(str->untranslatable_sections.empty()); input = "<string name=\"foo2\">\" This is what I think \"</string>"; ASSERT_TRUE(TestParse(input)); @@ -131,16 +134,61 @@ TEST_F(ResourceParserTest, ParseStringWithWhitespace) { EXPECT_EQ(std::string(" This is what I think "), *str->value); } -TEST_F(ResourceParserTest, IgnoreXliffTags) { - std::string input = - "<string name=\"foo\" \n" - " xmlns:xliff=\"urn:oasis:names:tc:xliff:document:1.2\">\n" - " There are <xliff:g id=\"count\">%1$d</xliff:g> apples</string>"; +TEST_F(ResourceParserTest, IgnoreXliffTagsOtherThanG) { + std::string input = R"EOF( + <string name="foo" xmlns:xliff="urn:oasis:names:tc:xliff:document:1.2"> + There are <xliff:source>no</xliff:source> apples</string>)EOF"; + ASSERT_TRUE(TestParse(input)); + + String* str = test::GetValue<String>(&table_, "string/foo"); + ASSERT_NE(nullptr, str); + EXPECT_EQ(StringPiece("There are no apples"), StringPiece(*str->value)); + EXPECT_TRUE(str->untranslatable_sections.empty()); +} + +TEST_F(ResourceParserTest, NestedXliffGTagsAreIllegal) { + std::string input = R"EOF( + <string name="foo" xmlns:xliff="urn:oasis:names:tc:xliff:document:1.2"> + Do not <xliff:g>translate <xliff:g>this</xliff:g></xliff:g></string>)EOF"; + EXPECT_FALSE(TestParse(input)); +} + +TEST_F(ResourceParserTest, RecordUntranslateableXliffSectionsInString) { + std::string input = R"EOF( + <string name="foo" xmlns:xliff="urn:oasis:names:tc:xliff:document:1.2"> + There are <xliff:g id="count">%1$d</xliff:g> apples</string>)EOF"; ASSERT_TRUE(TestParse(input)); String* str = test::GetValue<String>(&table_, "string/foo"); ASSERT_NE(nullptr, str); EXPECT_EQ(StringPiece("There are %1$d apples"), StringPiece(*str->value)); + + ASSERT_EQ(1u, str->untranslatable_sections.size()); + + // We expect indices and lengths that span to include the whitespace + // before %1$d. This is due to how the StringBuilder withholds whitespace unless + // needed (to deal with line breaks, etc.). + EXPECT_EQ(9u, str->untranslatable_sections[0].start); + EXPECT_EQ(14u, str->untranslatable_sections[0].end); +} + +TEST_F(ResourceParserTest, RecordUntranslateableXliffSectionsInStyledString) { + std::string input = R"EOF( + <string name="foo" xmlns:xliff="urn:oasis:names:tc:xliff:document:1.2"> + There are <b><xliff:g id="count">%1$d</xliff:g></b> apples</string>)EOF"; + ASSERT_TRUE(TestParse(input)); + + StyledString* str = test::GetValue<StyledString>(&table_, "string/foo"); + ASSERT_NE(nullptr, str); + EXPECT_EQ(StringPiece("There are %1$d apples"), StringPiece(*str->value->str)); + + ASSERT_EQ(1u, str->untranslatable_sections.size()); + + // We expect indices and lengths that span to include the whitespace + // before %1$d. This is due to how the StringBuilder withholds whitespace unless + // needed (to deal with line breaks, etc.). + EXPECT_EQ(9u, str->untranslatable_sections[0].start); + EXPECT_EQ(14u, str->untranslatable_sections[0].end); } TEST_F(ResourceParserTest, ParseNull) { @@ -149,15 +197,11 @@ TEST_F(ResourceParserTest, ParseNull) { // The Android runtime treats a value of android::Res_value::TYPE_NULL as // a non-existing value, and this causes problems in styles when trying to - // resolve - // an attribute. Null values must be encoded as - // android::Res_value::TYPE_REFERENCE + // resolve an attribute. Null values must be encoded as android::Res_value::TYPE_REFERENCE // with a data value of 0. - BinaryPrimitive* integer = - test::GetValue<BinaryPrimitive>(&table_, "integer/foo"); + BinaryPrimitive* integer = test::GetValue<BinaryPrimitive>(&table_, "integer/foo"); ASSERT_NE(nullptr, integer); - EXPECT_EQ(uint16_t(android::Res_value::TYPE_REFERENCE), - integer->value.dataType); + EXPECT_EQ(uint16_t(android::Res_value::TYPE_REFERENCE), integer->value.dataType); EXPECT_EQ(0u, integer->value.data); } diff --git a/tools/aapt2/ResourceValues.cpp b/tools/aapt2/ResourceValues.cpp index 7956ad826acd..f75ed7ad978a 100644 --- a/tools/aapt2/ResourceValues.cpp +++ b/tools/aapt2/ResourceValues.cpp @@ -140,7 +140,23 @@ bool String::Equals(const Value* value) const { if (!other) { return false; } - return *this->value == *other->value; + + if (this->value != other->value) { + return false; + } + + if (untranslatable_sections.size() != other->untranslatable_sections.size()) { + return false; + } + + auto other_iter = other->untranslatable_sections.begin(); + for (const UntranslatableSection& this_section : untranslatable_sections) { + if (this_section != *other_iter) { + return false; + } + ++other_iter; + } + return true; } bool String::Flatten(android::Res_value* out_value) const { @@ -158,6 +174,7 @@ String* String::Clone(StringPool* new_pool) const { String* str = new String(new_pool->MakeRef(*value)); str->comment_ = comment_; str->source_ = source_; + str->untranslatable_sections = untranslatable_sections; return str; } @@ -173,17 +190,22 @@ bool StyledString::Equals(const Value* value) const { return false; } - if (*this->value->str == *other->value->str) { - const std::vector<StringPool::Span>& spans_a = this->value->spans; - const std::vector<StringPool::Span>& spans_b = other->value->spans; - return std::equal( - spans_a.begin(), spans_a.end(), spans_b.begin(), - [](const StringPool::Span& a, const StringPool::Span& b) -> bool { - return *a.name == *b.name && a.first_char == b.first_char && - a.last_char == b.last_char; - }); + if (this->value != other->value) { + return false; + } + + if (untranslatable_sections.size() != other->untranslatable_sections.size()) { + return false; + } + + auto other_iter = other->untranslatable_sections.begin(); + for (const UntranslatableSection& this_section : untranslatable_sections) { + if (this_section != *other_iter) { + return false; + } + ++other_iter; } - return false; + return true; } bool StyledString::Flatten(android::Res_value* out_value) const { @@ -200,6 +222,7 @@ StyledString* StyledString::Clone(StringPool* new_pool) const { StyledString* str = new StyledString(new_pool->MakeRef(value)); str->comment_ = comment_; str->source_ = source_; + str->untranslatable_sections = untranslatable_sections; return str; } @@ -218,7 +241,7 @@ bool FileReference::Equals(const Value* value) const { if (!other) { return false; } - return *path == *other->path; + return path == other->path; } bool FileReference::Flatten(android::Res_value* out_value) const { diff --git a/tools/aapt2/ResourceValues.h b/tools/aapt2/ResourceValues.h index d380f8d0583d..c71c738892da 100644 --- a/tools/aapt2/ResourceValues.h +++ b/tools/aapt2/ResourceValues.h @@ -34,44 +34,35 @@ namespace aapt { struct RawValueVisitor; -/** - * A resource value. This is an all-encompassing representation - * of Item and Map and their subclasses. The way to do - * type specific operations is to check the Value's type() and - * cast it to the appropriate subclass. This isn't super clean, - * but it is the simplest strategy. - */ +// A resource value. This is an all-encompassing representation +// of Item and Map and their subclasses. The way to do +// type specific operations is to check the Value's type() and +// cast it to the appropriate subclass. This isn't super clean, +// but it is the simplest strategy. struct Value { virtual ~Value() = default; - /** - * Whether this value is weak and can be overridden without - * warning or error. Default is false. - */ + // Whether this value is weak and can be overridden without warning or error. Default is false. bool IsWeak() const { return weak_; } void SetWeak(bool val) { weak_ = val; } - // Whether the value is marked as translateable. + // Whether the value is marked as translatable. // This does not persist when flattened. // It is only used during compilation phase. - void SetTranslateable(bool val) { translateable_ = val; } + void SetTranslatable(bool val) { translatable_ = val; } // Default true. - bool IsTranslateable() const { return translateable_; } + bool IsTranslatable() const { return translatable_; } - /** - * Returns the source where this value was defined. - */ + // Returns the source where this value was defined. const Source& GetSource() const { return source_; } void SetSource(const Source& source) { source_ = source; } void SetSource(Source&& source) { source_ = std::move(source); } - /** - * Returns the comment that was associated with this resource. - */ + // Returns the comment that was associated with this resource. const std::string& GetComment() const { return comment_; } void SetComment(const android::StringPiece& str) { comment_ = str.to_string(); } @@ -80,70 +71,48 @@ struct Value { virtual bool Equals(const Value* value) const = 0; - /** - * Calls the appropriate overload of ValueVisitor. - */ + // Calls the appropriate overload of ValueVisitor. virtual void Accept(RawValueVisitor* visitor) = 0; - /** - * Clone the value. new_pool is the new StringPool that - * any resources with strings should use when copying their string. - */ + // Clone the value. `new_pool` is the new StringPool that + // any resources with strings should use when copying their string. virtual Value* Clone(StringPool* new_pool) const = 0; - /** - * Human readable printout of this value. - */ + // Human readable printout of this value. virtual void Print(std::ostream* out) const = 0; protected: Source source_; std::string comment_; bool weak_ = false; - bool translateable_ = true; + bool translatable_ = true; }; -/** - * Inherit from this to get visitor accepting implementations for free. - */ +// Inherit from this to get visitor accepting implementations for free. template <typename Derived> struct BaseValue : public Value { void Accept(RawValueVisitor* visitor) override; }; -/** - * A resource item with a single value. This maps to android::ResTable_entry. - */ +// A resource item with a single value. This maps to android::ResTable_entry. struct Item : public Value { - /** - * Clone the Item. - */ + // Clone the Item. virtual Item* Clone(StringPool* new_pool) const override = 0; - /** - * Fills in an android::Res_value structure with this Item's binary - * representation. - * Returns false if an error occurred. - */ + // Fills in an android::Res_value structure with this Item's binary representation. + // Returns false if an error occurred. virtual bool Flatten(android::Res_value* out_value) const = 0; }; -/** - * Inherit from this to get visitor accepting implementations for free. - */ +// Inherit from this to get visitor accepting implementations for free. template <typename Derived> struct BaseItem : public Item { void Accept(RawValueVisitor* visitor) override; }; -/** - * A reference to another resource. This maps to - * android::Res_value::TYPE_REFERENCE. - * - * A reference can be symbolic (with the name set to a valid resource name) or - * be - * numeric (the id is set to a valid resource ID). - */ +// A reference to another resource. This maps to android::Res_value::TYPE_REFERENCE. +// A reference can be symbolic (with the name set to a valid resource name) or be +// numeric (the id is set to a valid resource ID). struct Reference : public BaseItem<Reference> { enum class Type { kResource, @@ -169,9 +138,7 @@ struct Reference : public BaseItem<Reference> { bool operator<(const Reference&, const Reference&); bool operator==(const Reference&, const Reference&); -/** - * An ID resource. Has no real value, just a place holder. - */ +// An ID resource. Has no real value, just a place holder. struct Id : public BaseItem<Id> { Id() { weak_ = true; } bool Equals(const Value* value) const override; @@ -180,11 +147,8 @@ struct Id : public BaseItem<Id> { void Print(std::ostream* out) const override; }; -/** - * A raw, unprocessed string. This may contain quotations, - * escape sequences, and whitespace. This shall *NOT* - * end up in the final resource table. - */ +// A raw, unprocessed string. This may contain quotations, escape sequences, and whitespace. +// This shall *NOT* end up in the final resource table. struct RawString : public BaseItem<RawString> { StringPool::Ref value; @@ -196,9 +160,32 @@ struct RawString : public BaseItem<RawString> { void Print(std::ostream* out) const override; }; +// Identifies a range of characters in a string that are untranslatable. +// These should not be pseudolocalized. The start and end indices are measured in bytes. +struct UntranslatableSection { + // Start offset inclusive. + size_t start; + + // End offset exclusive. + size_t end; +}; + +inline bool operator==(const UntranslatableSection& a, const UntranslatableSection& b) { + return a.start == b.start && a.end == b.end; +} + +inline bool operator!=(const UntranslatableSection& a, const UntranslatableSection& b) { + return a.start != b.start || a.end != b.end; +} + struct String : public BaseItem<String> { StringPool::Ref value; + // Sections of the string to NOT translate. Mainly used + // for pseudolocalization. This data is NOT persisted + // in any format. + std::vector<UntranslatableSection> untranslatable_sections; + explicit String(const StringPool::Ref& ref); bool Equals(const Value* value) const override; @@ -210,6 +197,11 @@ struct String : public BaseItem<String> { struct StyledString : public BaseItem<StyledString> { StringPool::StyleRef value; + // Sections of the string to NOT translate. Mainly used + // for pseudolocalization. This data is NOT persisted + // in any format. + std::vector<UntranslatableSection> untranslatable_sections; + explicit StyledString(const StringPool::StyleRef& ref); bool Equals(const Value* value) const override; @@ -221,9 +213,8 @@ struct StyledString : public BaseItem<StyledString> { struct FileReference : public BaseItem<FileReference> { StringPool::Ref path; - /** - * A handle to the file object from which this file can be read. - */ + // A handle to the file object from which this file can be read. + // This field is NOT persisted in any format. It is transient. io::IFile* file = nullptr; FileReference() = default; @@ -235,9 +226,7 @@ struct FileReference : public BaseItem<FileReference> { void Print(std::ostream* out) const override; }; -/** - * Represents any other android::Res_value. - */ +// Represents any other android::Res_value. struct BinaryPrimitive : public BaseItem<BinaryPrimitive> { android::Res_value value; @@ -279,10 +268,7 @@ struct Style : public BaseValue<Style> { Maybe<Reference> parent; - /** - * If set to true, the parent was auto inferred from the - * style's name. - */ + // If set to true, the parent was auto inferred from the style's name. bool parent_inferred = false; std::vector<Entry> entries; @@ -319,9 +305,7 @@ struct Styleable : public BaseValue<Styleable> { void MergeWith(Styleable* styleable); }; -/** - * Stream operator for printing Value objects. - */ +// Stream operator for printing Value objects. inline ::std::ostream& operator<<(::std::ostream& out, const Value& value) { value.Print(&out); return out; diff --git a/tools/aapt2/StringPool.cpp b/tools/aapt2/StringPool.cpp index d968d73c44ee..57da5f01dcd1 100644 --- a/tools/aapt2/StringPool.cpp +++ b/tools/aapt2/StringPool.cpp @@ -63,6 +63,14 @@ StringPool::Ref& StringPool::Ref::operator=(const StringPool::Ref& rhs) { return *this; } +bool StringPool::Ref::operator==(const Ref& rhs) const { + return entry_->value == rhs.entry_->value; +} + +bool StringPool::Ref::operator!=(const Ref& rhs) const { + return entry_->value != rhs.entry_->value; +} + const std::string* StringPool::Ref::operator->() const { return &entry_->value; } @@ -109,6 +117,28 @@ StringPool::StyleRef& StringPool::StyleRef::operator=( return *this; } +bool StringPool::StyleRef::operator==(const StyleRef& rhs) const { + if (entry_->str != rhs.entry_->str) { + return false; + } + + if (entry_->spans.size() != rhs.entry_->spans.size()) { + return false; + } + + auto rhs_iter = rhs.entry_->spans.begin(); + for (const Span& span : entry_->spans) { + const Span& rhs_span = *rhs_iter; + if (span.first_char != rhs_span.first_char || span.last_char != rhs_span.last_char || + span.name != rhs_span.name) { + return false; + } + } + return true; +} + +bool StringPool::StyleRef::operator!=(const StyleRef& rhs) const { return !operator==(rhs); } + const StringPool::StyleEntry* StringPool::StyleRef::operator->() const { return entry_; } diff --git a/tools/aapt2/StringPool.h b/tools/aapt2/StringPool.h index d0ce489dae26..a626d375b625 100644 --- a/tools/aapt2/StringPool.h +++ b/tools/aapt2/StringPool.h @@ -70,6 +70,8 @@ class StringPool { ~Ref(); Ref& operator=(const Ref& rhs); + bool operator==(const Ref& rhs) const; + bool operator!=(const Ref& rhs) const; const std::string* operator->() const; const std::string& operator*() const; @@ -93,6 +95,8 @@ class StringPool { ~StyleRef(); StyleRef& operator=(const StyleRef& rhs); + bool operator==(const StyleRef& rhs) const; + bool operator!=(const StyleRef& rhs) const; const StyleEntry* operator->() const; const StyleEntry& operator*() const; diff --git a/tools/aapt2/compile/PseudolocaleGenerator.cpp b/tools/aapt2/compile/PseudolocaleGenerator.cpp index 5035f816f0fa..fad9edd04e4c 100644 --- a/tools/aapt2/compile/PseudolocaleGenerator.cpp +++ b/tools/aapt2/compile/PseudolocaleGenerator.cpp @@ -43,16 +43,16 @@ std::unique_ptr<StyledString> PseudolocalizeStyledString( } // The ranges are all represented with a single value. This is the start of - // one range and - // end of another. + // one range and end of another. struct Range { size_t start; - // Once the new string is localized, these are the pointers to the spans to - // adjust. + // If set to true, toggles the state of translatability. + bool toggle_translatability; + + // Once the new string is localized, these are the pointers to the spans to adjust. // Since this struct represents the start of one range and end of another, - // we have - // the two pointers respectively. + // we have the two pointers respectively. uint32_t* update_start; uint32_t* update_end; }; @@ -63,12 +63,11 @@ std::unique_ptr<StyledString> PseudolocalizeStyledString( // Construct the ranges. The ranges are represented like so: [0, 2, 5, 7] // The ranges are the spaces in between. In this example, with a total string - // length of 9, - // the vector represents: (0,1], (2,4], (5,6], (7,9] + // length of 9, the vector represents: (0,1], (2,4], (5,6], (7,9] // std::vector<Range> ranges; - ranges.push_back(Range{0}); - ranges.push_back(Range{original_text.size() - 1}); + ranges.push_back(Range{0, false, nullptr, nullptr}); + ranges.push_back(Range{original_text.size() - 1, false, nullptr, nullptr}); for (size_t i = 0; i < string->value->spans.size(); i++) { const StringPool::Span& span = string->value->spans[i]; @@ -78,8 +77,7 @@ std::unique_ptr<StyledString> PseudolocalizeStyledString( if (iter != ranges.end() && iter->start == span.first_char) { iter->update_start = &localized.spans[i].first_char; } else { - ranges.insert(iter, Range{span.first_char, &localized.spans[i].first_char, - nullptr}); + ranges.insert(iter, Range{span.first_char, false, &localized.spans[i].first_char, nullptr}); } // Insert or update the Range marker for the end of this span. @@ -87,14 +85,45 @@ std::unique_ptr<StyledString> PseudolocalizeStyledString( if (iter != ranges.end() && iter->start == span.last_char) { iter->update_end = &localized.spans[i].last_char; } else { - ranges.insert( - iter, Range{span.last_char, nullptr, &localized.spans[i].last_char}); + ranges.insert(iter, Range{span.last_char, false, nullptr, &localized.spans[i].last_char}); + } + } + + // Parts of the string may be untranslatable. Merge those ranges + // in as well, so that we have continuous sections of text to + // feed into the pseudolocalizer. + // We do this by marking the beginning of a range as either toggling + // the translatability state or not. + for (const UntranslatableSection& section : string->untranslatable_sections) { + auto iter = std::lower_bound(ranges.begin(), ranges.end(), section.start, cmp); + if (iter != ranges.end() && iter->start == section.start) { + // An existing span starts (or ends) here. We just need to mark that + // the translatability should toggle here. If translatability was + // already being toggled, then that means we have two adjacent ranges of untranslatable + // text, so remove the toggle and only toggle at the end of this range, + // effectively merging these ranges. + iter->toggle_translatability = !iter->toggle_translatability; + } else { + // Insert a new range that specifies to toggle the translatability. + iter = ranges.insert(iter, Range{section.start, true, nullptr, nullptr}); + } + + // Update/create an end to the untranslatable section. + iter = std::lower_bound(iter, ranges.end(), section.end, cmp); + if (iter != ranges.end() && iter->start == section.end) { + iter->toggle_translatability = true; + } else { + iter = ranges.insert(iter, Range{section.end, true, nullptr, nullptr}); } } localized.str += localizer.Start(); // Iterate over the ranges and localize each section. + // The text starts as translatable, and each time a range has toggle_translatability + // set to true, we toggle whether to translate or not. + // This assumes no untranslatable ranges overlap. + bool translatable = true; for (size_t i = 0; i < ranges.size(); i++) { const size_t start = ranges[i].start; size_t len = original_text.size() - start; @@ -110,15 +139,20 @@ std::unique_ptr<StyledString> PseudolocalizeStyledString( *ranges[i].update_end = localized.str.size(); } - localized.str += localizer.Text(original_text.substr(start, len)); + if (ranges[i].toggle_translatability) { + translatable = !translatable; + } + + if (translatable) { + localized.str += localizer.Text(original_text.substr(start, len)); + } else { + localized.str += original_text.substr(start, len); + } } localized.str += localizer.End(); - std::unique_ptr<StyledString> localized_string = - util::make_unique<StyledString>(pool->MakeRef(localized)); - localized_string->SetSource(string->GetSource()); - return localized_string; + return util::make_unique<StyledString>(pool->MakeRef(localized)); } namespace { @@ -152,8 +186,30 @@ class Visitor : public RawValueVisitor { } void Visit(String* string) override { - std::string result = - localizer_.Start() + localizer_.Text(*string->value) + localizer_.End(); + const StringPiece original_string = *string->value; + std::string result = localizer_.Start(); + + // Pseudolocalize only the translatable sections. + size_t start = 0u; + for (const UntranslatableSection& section : string->untranslatable_sections) { + // Pseudolocalize the content before the untranslatable section. + const size_t len = section.start - start; + if (len > 0u) { + result += localizer_.Text(original_string.substr(start, len)); + } + + // Copy the untranslatable content. + result += original_string.substr(section.start, section.end - section.start); + start = section.end; + } + + // Pseudolocalize the content after the last untranslatable section. + if (start != original_string.size()) { + const size_t len = original_string.size() - start; + result += localizer_.Text(original_string.substr(start, len)); + } + result += localizer_.End(); + std::unique_ptr<String> localized = util::make_unique<String>(pool_->MakeRef(result)); localized->SetSource(string->GetSource()); @@ -163,6 +219,7 @@ class Visitor : public RawValueVisitor { void Visit(StyledString* string) override { item = PseudolocalizeStyledString(string, method_, pool_); + item->SetSource(string->GetSource()); item->SetWeak(true); } @@ -228,7 +285,7 @@ void PseudolocalizeIfNeeded(const Pseudolocalizer::Method method, /** * A value is pseudolocalizable if it does not define a locale (or is the * default locale) - * and is translateable. + * and is translatable. */ static bool IsPseudolocalizable(ResourceConfigValue* config_value) { const int diff = @@ -236,7 +293,7 @@ static bool IsPseudolocalizable(ResourceConfigValue* config_value) { if (diff & ConfigDescription::CONFIG_LOCALE) { return false; } - return config_value->value->IsTranslateable(); + return config_value->value->IsTranslatable(); } } // namespace diff --git a/tools/aapt2/compile/PseudolocaleGenerator_test.cpp b/tools/aapt2/compile/PseudolocaleGenerator_test.cpp index 5a9884d34dd1..4db37db55eb7 100644 --- a/tools/aapt2/compile/PseudolocaleGenerator_test.cpp +++ b/tools/aapt2/compile/PseudolocaleGenerator_test.cpp @@ -89,7 +89,7 @@ TEST(PseudolocaleGeneratorTest, PseudolocalizeOnlyDefaultConfigs) { String* val = test::GetValue<String>(table.get(), "android:string/four"); ASSERT_NE(nullptr, val); - val->SetTranslateable(false); + val->SetTranslatable(false); std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build(); PseudolocaleGenerator generator; @@ -130,4 +130,47 @@ TEST(PseudolocaleGeneratorTest, PseudolocalizeOnlyDefaultConfigs) { test::ParseConfigOrDie("ar-rXB"))); } +TEST(PseudolocaleGeneratorTest, RespectUntranslateableSections) { + std::unique_ptr<IAaptContext> context = + test::ContextBuilder().SetCompilationPackage("android").Build(); + std::unique_ptr<ResourceTable> table = util::make_unique<ResourceTable>(); + + { + StyleString original_style; + original_style.str = "Hello world!"; + original_style.spans = {Span{"b", 2, 3}, Span{"b", 6, 7}, Span{"i", 1, 10}}; + + auto styled_string = + util::make_unique<StyledString>(table->string_pool.MakeRef(original_style)); + styled_string->untranslatable_sections.push_back(UntranslatableSection{6u, 8u}); + styled_string->untranslatable_sections.push_back(UntranslatableSection{8u, 11u}); + + auto string = util::make_unique<String>(table->string_pool.MakeRef(original_style.str)); + string->untranslatable_sections.push_back(UntranslatableSection{6u, 11u}); + + ASSERT_TRUE(table->AddResource(test::ParseNameOrDie("android:string/foo"), ConfigDescription{}, + {} /* product */, std::move(styled_string), + context->GetDiagnostics())); + ASSERT_TRUE(table->AddResource(test::ParseNameOrDie("android:string/bar"), ConfigDescription{}, + {} /* product */, std::move(string), context->GetDiagnostics())); + } + + PseudolocaleGenerator generator; + ASSERT_TRUE(generator.Consume(context.get(), table.get())); + + StyledString* new_styled_string = test::GetValueForConfig<StyledString>( + table.get(), "android:string/foo", test::ParseConfigOrDie("en-rXA")); + ASSERT_NE(nullptr, new_styled_string); + + // "world" should be untranslated. + EXPECT_NE(std::string::npos, new_styled_string->value->str->find("world")); + + String* new_string = test::GetValueForConfig<String>(table.get(), "android:string/bar", + test::ParseConfigOrDie("en-rXA")); + ASSERT_NE(nullptr, new_string); + + // "world" should be untranslated. + EXPECT_NE(std::string::npos, new_string->value->find("world")); +} + } // namespace aapt diff --git a/tools/aapt2/readme.md b/tools/aapt2/readme.md index 44d22c42468d..8bc4e8c65fac 100644 --- a/tools/aapt2/readme.md +++ b/tools/aapt2/readme.md @@ -1,5 +1,9 @@ # Android Asset Packaging Tool 2.0 (AAPT2) release notes +## Version 2.7 +### `aapt2 compile` +- Fixes bug where psuedolocalization auto-translated strings marked 'translateable="false"'. + ## Version 2.6 ### `aapt2` - Support legacy `configVarying` resource type. |