diff --git a/tools/aapt2/Debug.cpp b/tools/aapt2/Debug.cpp index b872ebbeb1598..49ed7780f9501 100644 --- a/tools/aapt2/Debug.cpp +++ b/tools/aapt2/Debug.cpp @@ -33,6 +33,8 @@ namespace aapt { +namespace { + class PrintVisitor : public ValueVisitor { public: using ValueVisitor::Visit; @@ -88,9 +90,13 @@ class PrintVisitor : public ValueVisitor { } } - void Visit(Array* array) override { array->Print(&std::cout); } + void Visit(Array* array) override { + array->Print(&std::cout); + } - void Visit(Plural* plural) override { plural->Print(&std::cout); } + void Visit(Plural* plural) override { + plural->Print(&std::cout); + } void Visit(Styleable* styleable) override { std::cout << "(styleable)"; @@ -110,11 +116,14 @@ class PrintVisitor : public ValueVisitor { } } - void VisitItem(Item* item) override { item->Print(&std::cout); } + void VisitItem(Item* item) override { + item->Print(&std::cout); + } }; -void Debug::PrintTable(ResourceTable* table, - const DebugPrintTableOptions& options) { +} // namespace + +void Debug::PrintTable(ResourceTable* table, const DebugPrintTableOptions& options) { PrintVisitor visitor; for (auto& package : table->packages) { @@ -148,10 +157,9 @@ void Debug::PrintTable(ResourceTable* table, } for (const ResourceEntry* entry : sorted_entries) { - ResourceId id(package->id ? package->id.value() : uint8_t(0), - type->id ? type->id.value() : uint8_t(0), - entry->id ? entry->id.value() : uint16_t(0)); - ResourceName name(package->name, type->type, entry->name); + const ResourceId id(package->id.value_or_default(0), type->id.value_or_default(0), + entry->id.value_or_default(0)); + const ResourceName name(package->name, type->type, entry->name); std::cout << " spec resource " << id << " " << name; switch (entry->symbol_status.state) { @@ -180,16 +188,14 @@ void Debug::PrintTable(ResourceTable* table, } } -static size_t GetNodeIndex(const std::vector& names, - const ResourceName& name) { +static size_t GetNodeIndex(const std::vector& names, const ResourceName& name) { auto iter = std::lower_bound(names.begin(), names.end(), name); CHECK(iter != names.end()); CHECK(*iter == name); return std::distance(names.begin(), iter); } -void Debug::PrintStyleGraph(ResourceTable* table, - const ResourceName& target_style) { +void Debug::PrintStyleGraph(ResourceTable* table, const ResourceName& target_style) { std::map> graph; std::queue styles_to_visit; @@ -223,8 +229,7 @@ void Debug::PrintStyleGraph(ResourceTable* table, std::cout << "digraph styles {\n"; for (const auto& name : names) { - std::cout << " node_" << GetNodeIndex(names, name) << " [label=\"" << name - << "\"];\n"; + std::cout << " node_" << GetNodeIndex(names, name) << " [label=\"" << name << "\"];\n"; } for (const auto& entry : graph) { @@ -243,8 +248,7 @@ void Debug::PrintStyleGraph(ResourceTable* table, void Debug::DumpHex(const void* data, size_t len) { const uint8_t* d = (const uint8_t*)data; for (size_t i = 0; i < len; i++) { - std::cerr << std::hex << std::setfill('0') << std::setw(2) << (uint32_t)d[i] - << " "; + std::cerr << std::hex << std::setfill('0') << std::setw(2) << (uint32_t)d[i] << " "; if (i % 8 == 7) { std::cerr << "\n"; } @@ -262,8 +266,15 @@ class XmlPrinter : public xml::Visitor { using xml::Visitor::Visit; void Visit(xml::Element* el) override { - std::cerr << prefix_; - std::cerr << "E: "; + const size_t previous_size = prefix_.size(); + + for (const xml::NamespaceDecl& decl : el->namespace_decls) { + std::cerr << prefix_ << "N: " << decl.prefix << "=" << decl.uri + << " (line=" << decl.line_number << ")\n"; + prefix_ += " "; + } + + std::cerr << prefix_ << "E: "; if (!el->namespace_uri.empty()) { std::cerr << el->namespace_uri << ":"; } @@ -283,26 +294,13 @@ class XmlPrinter : public xml::Visitor { std::cerr << "=" << attr.value << "\n"; } - const size_t previous_size = prefix_.size(); prefix_ += " "; xml::Visitor::Visit(el); prefix_.resize(previous_size); } - void Visit(xml::Namespace* ns) override { - std::cerr << prefix_; - std::cerr << "N: " << ns->namespace_prefix << "=" << ns->namespace_uri - << " (line=" << ns->line_number << ")\n"; - - const size_t previous_size = prefix_.size(); - prefix_ += " "; - xml::Visitor::Visit(ns); - prefix_.resize(previous_size); - } - void Visit(xml::Text* text) override { - std::cerr << prefix_; - std::cerr << "T: '" << text->text << "'\n"; + std::cerr << prefix_ << "T: '" << text->text << "'\n"; } private: diff --git a/tools/aapt2/cmd/Link.cpp b/tools/aapt2/cmd/Link.cpp index c25c4f63d4be7..35ab3c6515d18 100644 --- a/tools/aapt2/cmd/Link.cpp +++ b/tools/aapt2/cmd/Link.cpp @@ -482,7 +482,7 @@ std::vector> ResourceFileFlattener::LinkAndVer if (options_.no_version_vectors || options_.no_version_transitions) { // Skip this if it is a vector or animated-vector. - xml::Element* el = xml::FindRootElement(doc); + xml::Element* el = doc->root.get(); if (el && el->namespace_uri.empty()) { if ((options_.no_version_vectors && IsVectorElement(el->name)) || (options_.no_version_transitions && IsTransitionElement(el->name))) { diff --git a/tools/aapt2/cmd/Util.cpp b/tools/aapt2/cmd/Util.cpp index e1c45d68f6115..d17858d45d08a 100644 --- a/tools/aapt2/cmd/Util.cpp +++ b/tools/aapt2/cmd/Util.cpp @@ -28,7 +28,7 @@ #include "util/Maybe.h" #include "util/Util.h" -using android::StringPiece; +using ::android::StringPiece; namespace aapt { @@ -134,19 +134,21 @@ static xml::AaptAttribute CreateAttributeWithId(const ResourceId& id) { return xml::AaptAttribute(Attribute(), id); } +static xml::NamespaceDecl CreateAndroidNamespaceDecl() { + xml::NamespaceDecl decl; + decl.prefix = "android"; + decl.uri = xml::kSchemaAndroid; + return decl; +} + std::unique_ptr GenerateSplitManifest(const AppInfo& app_info, const SplitConstraints& constraints) { const ResourceId kVersionCode(0x0101021b); const ResourceId kRevisionCode(0x010104d5); const ResourceId kHasCode(0x0101000c); - std::unique_ptr doc = util::make_unique(); - - std::unique_ptr namespace_android = util::make_unique(); - namespace_android->namespace_uri = xml::kSchemaAndroid; - namespace_android->namespace_prefix = "android"; - std::unique_ptr manifest_el = util::make_unique(); + manifest_el->namespace_decls.push_back(CreateAndroidNamespaceDecl()); manifest_el->name = "manifest"; manifest_el->attributes.push_back(xml::Attribute{"", "package", app_info.package}); @@ -179,8 +181,8 @@ std::unique_ptr GenerateSplitManifest(const AppInfo& app_info, xml::Attribute{"", "configForSplit", app_info.split_name.value()}); } - // Splits may contain more configurations than originally desired (fallback densities, etc.). - // This makes programmatic discovery of split targetting difficult. Encode the original + // Splits may contain more configurations than originally desired (fall-back densities, etc.). + // This makes programmatic discovery of split targeting difficult. Encode the original // split constraints intended for this split. std::stringstream target_config_str; target_config_str << util::Joiner(constraints.configs, ","); @@ -193,8 +195,9 @@ std::unique_ptr GenerateSplitManifest(const AppInfo& app_info, util::make_unique(android::Res_value::TYPE_INT_BOOLEAN, 0u)}); manifest_el->AppendChild(std::move(application_el)); - namespace_android->AppendChild(std::move(manifest_el)); - doc->root = std::move(namespace_android); + + std::unique_ptr doc = util::make_unique(); + doc->root = std::move(manifest_el); return doc; } @@ -284,7 +287,7 @@ static Maybe ExtractSdkVersion(xml::Attribute* attr, std::string* out_error Maybe ExtractAppInfoFromBinaryManifest(xml::XmlResource* xml_res, IDiagnostics* diag) { // Make sure the first element is with package attribute. - xml::Element* manifest_el = xml::FindRootElement(xml_res->root.get()); + xml::Element* manifest_el = xml_res->root.get(); if (manifest_el == nullptr) { return {}; } diff --git a/tools/aapt2/compile/InlineXmlFormatParser.cpp b/tools/aapt2/compile/InlineXmlFormatParser.cpp index 786494b6ad1c3..857cdd5365a0c 100644 --- a/tools/aapt2/compile/InlineXmlFormatParser.cpp +++ b/tools/aapt2/compile/InlineXmlFormatParser.cpp @@ -16,12 +16,8 @@ #include "compile/InlineXmlFormatParser.h" -#include #include -#include "android-base/macros.h" - -#include "Debug.h" #include "ResourceUtils.h" #include "util/Util.h" #include "xml/XmlDom.h" @@ -31,19 +27,17 @@ namespace aapt { namespace { -/** - * XML Visitor that will find all elements for extraction. - */ +struct InlineDeclaration { + xml::Element* el; + std::string attr_namespace_uri; + std::string attr_name; +}; + +// XML Visitor that will find all elements for extraction. class Visitor : public xml::PackageAwareVisitor { public: using xml::PackageAwareVisitor::Visit; - struct InlineDeclaration { - xml::Element* el; - std::string attr_namespace_uri; - std::string attr_name; - }; - explicit Visitor(IAaptContext* context, xml::XmlResource* xml_resource) : context_(context), xml_resource_(xml_resource) {} @@ -53,51 +47,44 @@ class Visitor : public xml::PackageAwareVisitor { return; } - const Source& src = xml_resource_->file.source.WithLine(el->line_number); + const Source src = xml_resource_->file.source.WithLine(el->line_number); xml::Attribute* attr = el->FindAttribute({}, "name"); if (!attr) { - context_->GetDiagnostics()->Error(DiagMessage(src) - << "missing 'name' attribute"); + context_->GetDiagnostics()->Error(DiagMessage(src) << "missing 'name' attribute"); error_ = true; return; } Maybe ref = ResourceUtils::ParseXmlAttributeName(attr->value); if (!ref) { - context_->GetDiagnostics()->Error( - DiagMessage(src) << "invalid XML attribute '" << attr->value << "'"); + context_->GetDiagnostics()->Error(DiagMessage(src) << "invalid XML attribute '" << attr->value + << "'"); error_ = true; return; } const ResourceName& name = ref.value().name.value(); - // Use an empty string for the compilation package because we don't want to - // default to - // the local package if the user specified name="style" or something. This - // should just + // Use an empty string for the compilation package because we don't want to default to + // the local package if the user specified name="style" or something. This should just // be the default namespace. - Maybe maybe_pkg = - TransformPackageAlias(name.package, {}); + Maybe maybe_pkg = TransformPackageAlias(name.package, {}); if (!maybe_pkg) { - context_->GetDiagnostics()->Error(DiagMessage(src) - << "invalid namespace prefix '" - << name.package << "'"); + context_->GetDiagnostics()->Error(DiagMessage(src) << "invalid namespace prefix '" + << name.package << "'"); error_ = true; return; } const xml::ExtractedPackage& pkg = maybe_pkg.value(); - const bool private_namespace = - pkg.private_namespace || ref.value().private_reference; + const bool private_namespace = pkg.private_namespace || ref.value().private_reference; InlineDeclaration decl; decl.el = el; decl.attr_name = name.entry; if (!pkg.package.empty()) { - decl.attr_namespace_uri = - xml::BuildPackageNamespace(pkg.package, private_namespace); + decl.attr_namespace_uri = xml::BuildPackageNamespace(pkg.package, private_namespace); } inline_declarations_.push_back(std::move(decl)); @@ -107,7 +94,9 @@ class Visitor : public xml::PackageAwareVisitor { return inline_declarations_; } - bool HasError() const { return error_; } + bool HasError() const { + return error_; + } private: DISALLOW_COPY_AND_ASSIGN(Visitor); @@ -120,8 +109,7 @@ class Visitor : public xml::PackageAwareVisitor { } // namespace -bool InlineXmlFormatParser::Consume(IAaptContext* context, - xml::XmlResource* doc) { +bool InlineXmlFormatParser::Consume(IAaptContext* context, xml::XmlResource* doc) { Visitor visitor(context, doc); doc->root->Accept(&visitor); if (visitor.HasError()) { @@ -129,69 +117,53 @@ bool InlineXmlFormatParser::Consume(IAaptContext* context, } size_t name_suffix_counter = 0; - for (const Visitor::InlineDeclaration& decl : - visitor.GetInlineDeclarations()) { + for (const InlineDeclaration& decl : visitor.GetInlineDeclarations()) { auto new_doc = util::make_unique(); new_doc->file.config = doc->file.config; new_doc->file.source = doc->file.source.WithLine(decl.el->line_number); new_doc->file.name = doc->file.name; // Modify the new entry name. We need to suffix the entry with a number to - // avoid - // local collisions, then mangle it with the empty package, such that it - // won't show up + // avoid local collisions, then mangle it with the empty package, such that it won't show up // in R.java. - - new_doc->file.name.entry = - NameMangler::MangleEntry({}, new_doc->file.name.entry + "__" + - std::to_string(name_suffix_counter)); + new_doc->file.name.entry = NameMangler::MangleEntry( + {}, new_doc->file.name.entry + "__" + std::to_string(name_suffix_counter)); // Extracted elements must be the only child of . // Make sure there is one root node in the children (ignore empty text). - for (auto& child : decl.el->children) { + for (std::unique_ptr& child : decl.el->children) { const Source child_source = doc->file.source.WithLine(child->line_number); if (xml::Text* t = xml::NodeCast(child.get())) { if (!util::TrimWhitespace(t->text).empty()) { - context->GetDiagnostics()->Error( - DiagMessage(child_source) - << "can't extract text into its own resource"); + context->GetDiagnostics()->Error(DiagMessage(child_source) + << "can't extract text into its own resource"); return false; } } else if (new_doc->root) { - context->GetDiagnostics()->Error( - DiagMessage(child_source) - << "inline XML resources must have a single root"); + context->GetDiagnostics()->Error(DiagMessage(child_source) + << "inline XML resources must have a single root"); return false; } else { - new_doc->root = std::move(child); + new_doc->root.reset(static_cast(child.release())); new_doc->root->parent = nullptr; } } - // Walk up and find the parent element. - xml::Node* node = decl.el; - xml::Element* parent_el = nullptr; - while (node->parent && - (parent_el = xml::NodeCast(node->parent)) == nullptr) { - node = node->parent; - } - + // Get the parent element of + xml::Element* parent_el = decl.el->parent; if (!parent_el) { - context->GetDiagnostics()->Error( - DiagMessage(new_doc->file.source) - << "no suitable parent for inheriting attribute"); + context->GetDiagnostics()->Error(DiagMessage(new_doc->file.source) + << "no suitable parent for inheriting attribute"); return false; } // Add the inline attribute to the parent. - parent_el->attributes.push_back( - xml::Attribute{decl.attr_namespace_uri, decl.attr_name, - "@" + new_doc->file.name.ToString()}); + parent_el->attributes.push_back(xml::Attribute{decl.attr_namespace_uri, decl.attr_name, + "@" + new_doc->file.name.ToString()}); // Delete the subtree. - for (auto iter = parent_el->children.begin(); - iter != parent_el->children.end(); ++iter) { - if (iter->get() == node) { + for (auto iter = parent_el->children.begin(); iter != parent_el->children.end(); ++iter) { + if (iter->get() == decl.el) { parent_el->children.erase(iter); break; } diff --git a/tools/aapt2/compile/InlineXmlFormatParser.h b/tools/aapt2/compile/InlineXmlFormatParser.h index 1a658fd6a1808..4300023e77267 100644 --- a/tools/aapt2/compile/InlineXmlFormatParser.h +++ b/tools/aapt2/compile/InlineXmlFormatParser.h @@ -26,35 +26,30 @@ namespace aapt { -/** - * Extracts Inline XML definitions into their own xml::XmlResource objects. - * - * Inline XML looks like: - * - * - * - * - * - * - * - * The will be extracted into its own XML file and - * will - * gain an attribute 'android:drawable' set to a reference to the extracted - * resource. - */ +// Extracts Inline XML definitions into their own xml::XmlResource objects. +// +// Inline XML looks like: +// +// +// +// +// +// +// +// The will be extracted into its own XML file and will +// gain an attribute 'android:drawable' set to a reference to the extracted resource. class InlineXmlFormatParser : public IXmlResourceConsumer { public: explicit InlineXmlFormatParser() = default; bool Consume(IAaptContext* context, xml::XmlResource* doc) override; - std::vector>& - GetExtractedInlineXmlDocuments() { + std::vector>& GetExtractedInlineXmlDocuments() { return queue_; } diff --git a/tools/aapt2/compile/InlineXmlFormatParser_test.cpp b/tools/aapt2/compile/InlineXmlFormatParser_test.cpp index 348796c98c227..de7739ada407a 100644 --- a/tools/aapt2/compile/InlineXmlFormatParser_test.cpp +++ b/tools/aapt2/compile/InlineXmlFormatParser_test.cpp @@ -18,25 +18,32 @@ #include "test/Test.h" +using ::testing::Eq; +using ::testing::IsNull; +using ::testing::Not; +using ::testing::NotNull; +using ::testing::SizeIs; +using ::testing::StrEq; + namespace aapt { TEST(InlineXmlFormatParserTest, PassThrough) { std::unique_ptr context = test::ContextBuilder().Build(); - std::unique_ptr doc = test::BuildXmlDom(R"EOF( + std::unique_ptr doc = test::BuildXmlDom(R"( - )EOF"); + )"); InlineXmlFormatParser parser; ASSERT_TRUE(parser.Consume(context.get(), doc.get())); - EXPECT_EQ(0u, parser.GetExtractedInlineXmlDocuments().size()); + EXPECT_THAT(parser.GetExtractedInlineXmlDocuments(), SizeIs(0u)); } TEST(InlineXmlFormatParserTest, ExtractOneXmlResource) { std::unique_ptr context = test::ContextBuilder().Build(); - std::unique_ptr doc = test::BuildXmlDom(R"EOF( + std::unique_ptr doc = test::BuildXmlDom(R"( @@ -44,7 +51,7 @@ TEST(InlineXmlFormatParserTest, ExtractOneXmlResource) { - )EOF"); + )"); doc->file.name = test::ParseNameOrDie("layout/main"); @@ -52,42 +59,38 @@ TEST(InlineXmlFormatParserTest, ExtractOneXmlResource) { ASSERT_TRUE(parser.Consume(context.get(), doc.get())); // One XML resource should have been extracted. - EXPECT_EQ(1u, parser.GetExtractedInlineXmlDocuments().size()); + EXPECT_THAT(parser.GetExtractedInlineXmlDocuments(), SizeIs(1u)); - xml::Element* el = xml::FindRootElement(doc.get()); - ASSERT_NE(nullptr, el); - - EXPECT_EQ("View1", el->name); + xml::Element* el = doc->root.get(); + ASSERT_THAT(el, NotNull()); + EXPECT_THAT(el->name, StrEq("View1")); // The tag should be extracted. - EXPECT_EQ(nullptr, el->FindChild(xml::kSchemaAapt, "attr")); + EXPECT_THAT(el->FindChild(xml::kSchemaAapt, "attr"), IsNull()); // The 'android:text' attribute should be set with a reference. xml::Attribute* attr = el->FindAttribute(xml::kSchemaAndroid, "text"); - ASSERT_NE(nullptr, attr); + ASSERT_THAT(attr, NotNull()); ResourceNameRef name_ref; ASSERT_TRUE(ResourceUtils::ParseReference(attr->value, &name_ref)); - xml::XmlResource* extracted_doc = - parser.GetExtractedInlineXmlDocuments()[0].get(); - ASSERT_NE(nullptr, extracted_doc); + xml::XmlResource* extracted_doc = parser.GetExtractedInlineXmlDocuments()[0].get(); + ASSERT_THAT(extracted_doc, NotNull()); // Make sure the generated reference is correct. - EXPECT_EQ(name_ref.package, extracted_doc->file.name.package); - EXPECT_EQ(name_ref.type, extracted_doc->file.name.type); - EXPECT_EQ(name_ref.entry, extracted_doc->file.name.entry); + EXPECT_THAT(extracted_doc->file.name, Eq(name_ref)); // Verify the structure of the extracted XML. - el = xml::FindRootElement(extracted_doc); - ASSERT_NE(nullptr, el); - EXPECT_EQ("View2", el->name); - EXPECT_NE(nullptr, el->FindChild({}, "View3")); + el = extracted_doc->root.get(); + ASSERT_THAT(el, NotNull()); + EXPECT_THAT(el->name, StrEq("View2")); + EXPECT_THAT(el->FindChild({}, "View3"), NotNull()); } TEST(InlineXmlFormatParserTest, ExtractTwoXmlResources) { std::unique_ptr context = test::ContextBuilder().Build(); - std::unique_ptr doc = test::BuildXmlDom(R"EOF( + std::unique_ptr doc = test::BuildXmlDom(R"( @@ -99,45 +102,39 @@ TEST(InlineXmlFormatParserTest, ExtractTwoXmlResources) { - )EOF"); + )"); doc->file.name = test::ParseNameOrDie("layout/main"); InlineXmlFormatParser parser; ASSERT_TRUE(parser.Consume(context.get(), doc.get())); - ASSERT_EQ(2u, parser.GetExtractedInlineXmlDocuments().size()); + ASSERT_THAT(parser.GetExtractedInlineXmlDocuments(), SizeIs(2u)); - xml::Element* el = xml::FindRootElement(doc.get()); - ASSERT_NE(nullptr, el); - - EXPECT_EQ("View1", el->name); + xml::Element* el = doc->root.get(); + ASSERT_THAT(el, NotNull()); + EXPECT_THAT(el->name, StrEq("View1")); xml::Attribute* attr_text = el->FindAttribute(xml::kSchemaAndroid, "text"); - ASSERT_NE(nullptr, attr_text); + ASSERT_THAT(attr_text, NotNull()); - xml::Attribute* attr_drawable = - el->FindAttribute(xml::kSchemaAndroid, "drawable"); - ASSERT_NE(nullptr, attr_drawable); + xml::Attribute* attr_drawable = el->FindAttribute(xml::kSchemaAndroid, "drawable"); + ASSERT_THAT(attr_drawable, NotNull()); // The two extracted resources should have different names. - EXPECT_NE(attr_text->value, attr_drawable->value); + EXPECT_THAT(attr_text->value, Not(Eq(attr_drawable->value))); // The child elements should be gone. - EXPECT_EQ(nullptr, el->FindChild(xml::kSchemaAapt, "attr")); + EXPECT_THAT(el->FindChild(xml::kSchemaAapt, "attr"), IsNull()); - xml::XmlResource* extracted_doc_text = - parser.GetExtractedInlineXmlDocuments()[0].get(); - ASSERT_NE(nullptr, extracted_doc_text); - el = xml::FindRootElement(extracted_doc_text); - ASSERT_NE(nullptr, el); - EXPECT_EQ("View2", el->name); + xml::XmlResource* extracted_doc_text = parser.GetExtractedInlineXmlDocuments()[0].get(); + ASSERT_THAT(extracted_doc_text, NotNull()); + ASSERT_THAT(extracted_doc_text->root, NotNull()); + EXPECT_THAT(extracted_doc_text->root->name, StrEq("View2")); - xml::XmlResource* extracted_doc_drawable = - parser.GetExtractedInlineXmlDocuments()[1].get(); - ASSERT_NE(nullptr, extracted_doc_drawable); - el = xml::FindRootElement(extracted_doc_drawable); - ASSERT_NE(nullptr, el); - EXPECT_EQ("vector", el->name); + xml::XmlResource* extracted_doc_drawable = parser.GetExtractedInlineXmlDocuments()[1].get(); + ASSERT_THAT(extracted_doc_drawable, NotNull()); + ASSERT_THAT(extracted_doc_drawable->root, NotNull()); + EXPECT_THAT(extracted_doc_drawable->root->name, StrEq("vector")); } } // namespace aapt diff --git a/tools/aapt2/configuration/ConfigurationParser.cpp b/tools/aapt2/configuration/ConfigurationParser.cpp index a9d6da0258cbf..424e9be3ef097 100644 --- a/tools/aapt2/configuration/ConfigurationParser.cpp +++ b/tools/aapt2/configuration/ConfigurationParser.cpp @@ -53,7 +53,6 @@ using ::aapt::io::RegularFile; using ::aapt::io::StringInputStream; using ::aapt::util::TrimWhitespace; using ::aapt::xml::Element; -using ::aapt::xml::FindRootElement; using ::aapt::xml::NodeCast; using ::aapt::xml::XmlActionExecutor; using ::aapt::xml::XmlActionExecutorPolicy; @@ -238,13 +237,13 @@ ConfigurationParser::ConfigurationParser(std::string contents) Maybe ConfigurationParser::Parse() { StringInputStream in(contents_); - auto doc = xml::Inflate(&in, diag_, Source("config.xml")); + std::unique_ptr doc = xml::Inflate(&in, diag_, Source("config.xml")); if (!doc) { return {}; } // Strip any namespaces from the XML as the XmlActionExecutor ignores anything with a namespace. - auto* root = FindRootElement(doc.get()); + Element* root = doc->root.get(); if (root == nullptr) { diag_->Error(DiagMessage() << "Could not find the root element in the XML document"); return {}; diff --git a/tools/aapt2/configuration/ConfigurationParser_test.cpp b/tools/aapt2/configuration/ConfigurationParser_test.cpp index 5bd083120c4d4..ece70a9e42ba3 100644 --- a/tools/aapt2/configuration/ConfigurationParser_test.cpp +++ b/tools/aapt2/configuration/ConfigurationParser_test.cpp @@ -18,9 +18,6 @@ #include -#include -#include - #include "androidfw/ResourceTypes.h" #include "test/Test.h" @@ -29,7 +26,7 @@ namespace aapt { namespace { -using android::ResTable_config; +using ::android::ResTable_config; using configuration::Abi; using configuration::AndroidSdk; using configuration::Artifact; @@ -38,7 +35,7 @@ using configuration::DeviceFeature; using configuration::GlTexture; using configuration::Locale; using configuration::AndroidManifest; -using testing::ElementsAre; +using ::testing::ElementsAre; using xml::Element; using xml::NodeCast; @@ -188,7 +185,7 @@ TEST_F(ConfigurationParserTest, ArtifactAction) { auto doc = test::BuildXmlDom(xml); PostProcessingConfiguration config; - bool ok = artifact_handler_(&config, NodeCast(doc.get()->root.get()), &diag_); + bool ok = artifact_handler_(&config, NodeCast(doc->root.get()), &diag_); ASSERT_TRUE(ok); EXPECT_EQ(1ul, config.artifacts.size()); diff --git a/tools/aapt2/flatten/XmlFlattener.cpp b/tools/aapt2/flatten/XmlFlattener.cpp index 331ef784a7da0..b3b308a29fc50 100644 --- a/tools/aapt2/flatten/XmlFlattener.cpp +++ b/tools/aapt2/flatten/XmlFlattener.cpp @@ -38,12 +38,10 @@ namespace { constexpr uint32_t kLowPriority = 0xffffffffu; -static bool cmp_xml_attribute_by_id(const xml::Attribute* a, - const xml::Attribute* b) { +static bool cmp_xml_attribute_by_id(const xml::Attribute* a, const xml::Attribute* b) { if (a->compiled_attribute && a->compiled_attribute.value().id) { if (b->compiled_attribute && b->compiled_attribute.value().id) { - return a->compiled_attribute.value().id.value() < - b->compiled_attribute.value().id.value(); + return a->compiled_attribute.value().id.value() < b->compiled_attribute.value().id.value(); } return true; } else if (!b->compiled_attribute) { @@ -75,17 +73,6 @@ class XmlFlattenerVisitor : public xml::Visitor { XmlFlattenerVisitor(BigBuffer* buffer, XmlFlattenerOptions options) : buffer_(buffer), options_(options) {} - void Visit(xml::Namespace* node) override { - if (node->namespace_uri == xml::kSchemaTools) { - // Skip dedicated tools namespace. - xml::Visitor::Visit(node); - } else { - WriteNamespace(node, android::RES_XML_START_NAMESPACE_TYPE); - xml::Visitor::Visit(node); - WriteNamespace(node, android::RES_XML_END_NAMESPACE_TYPE); - } - } - void Visit(xml::Text* node) override { if (util::TrimWhitespace(node->text).empty()) { // Skip whitespace only text nodes. @@ -93,8 +80,7 @@ class XmlFlattenerVisitor : public xml::Visitor { } ChunkWriter writer(buffer_); - ResXMLTree_node* flat_node = - writer.StartChunk(RES_XML_CDATA_TYPE); + ResXMLTree_node* flat_node = writer.StartChunk(RES_XML_CDATA_TYPE); flat_node->lineNumber = util::HostToDevice32(node->line_number); flat_node->comment.index = util::HostToDevice32(-1); @@ -109,6 +95,13 @@ class XmlFlattenerVisitor : public xml::Visitor { } void Visit(xml::Element* node) override { + for (const xml::NamespaceDecl& decl : node->namespace_decls) { + // Skip dedicated tools namespace. + if (decl.uri != xml::kSchemaTools) { + WriteNamespace(decl, android::RES_XML_START_NAMESPACE_TYPE); + } + } + { ChunkWriter start_writer(buffer_); ResXMLTree_node* flat_node = @@ -116,19 +109,15 @@ class XmlFlattenerVisitor : public xml::Visitor { flat_node->lineNumber = util::HostToDevice32(node->line_number); flat_node->comment.index = util::HostToDevice32(-1); - ResXMLTree_attrExt* flat_elem = - start_writer.NextBlock(); + ResXMLTree_attrExt* flat_elem = start_writer.NextBlock(); - // A missing namespace must be null, not an empty string. Otherwise the - // runtime complains. + // A missing namespace must be null, not an empty string. Otherwise the runtime complains. AddString(node->namespace_uri, kLowPriority, &flat_elem->ns, true /* treat_empty_string_as_null */); - AddString(node->name, kLowPriority, &flat_elem->name, - true /* treat_empty_string_as_null */); + AddString(node->name, kLowPriority, &flat_elem->name, true /* treat_empty_string_as_null */); flat_elem->attributeStart = util::HostToDevice16(sizeof(*flat_elem)); - flat_elem->attributeSize = - util::HostToDevice16(sizeof(ResXMLTree_attribute)); + flat_elem->attributeSize = util::HostToDevice16(sizeof(ResXMLTree_attribute)); WriteAttributes(node, flat_elem, &start_writer); @@ -144,14 +133,20 @@ class XmlFlattenerVisitor : public xml::Visitor { flat_end_node->lineNumber = util::HostToDevice32(node->line_number); flat_end_node->comment.index = util::HostToDevice32(-1); - ResXMLTree_endElementExt* flat_end_elem = - end_writer.NextBlock(); + ResXMLTree_endElementExt* flat_end_elem = end_writer.NextBlock(); AddString(node->namespace_uri, kLowPriority, &flat_end_elem->ns, true /* treat_empty_string_as_null */); AddString(node->name, kLowPriority, &flat_end_elem->name); end_writer.Finish(); } + + for (auto iter = node->namespace_decls.rbegin(); iter != node->namespace_decls.rend(); ++iter) { + // Skip dedicated tools namespace. + if (iter->uri != xml::kSchemaTools) { + WriteNamespace(*iter, android::RES_XML_END_NAMESPACE_TYPE); + } + } } private: @@ -173,16 +168,16 @@ class XmlFlattenerVisitor : public xml::Visitor { string_refs.push_back(StringFlattenDest{ref, dest}); } - void WriteNamespace(xml::Namespace* node, uint16_t type) { + void WriteNamespace(const xml::NamespaceDecl& decl, uint16_t type) { ChunkWriter writer(buffer_); ResXMLTree_node* flatNode = writer.StartChunk(type); - flatNode->lineNumber = util::HostToDevice32(node->line_number); + flatNode->lineNumber = util::HostToDevice32(decl.line_number); flatNode->comment.index = util::HostToDevice32(-1); ResXMLTree_namespaceExt* flat_ns = writer.NextBlock(); - AddString(node->namespace_prefix, kLowPriority, &flat_ns->prefix); - AddString(node->namespace_uri, kLowPriority, &flat_ns->uri); + AddString(decl.prefix, kLowPriority, &flat_ns->prefix); + AddString(decl.uri, kLowPriority, &flat_ns->uri); writer.Finish(); } diff --git a/tools/aapt2/java/ProguardRules.cpp b/tools/aapt2/java/ProguardRules.cpp index 5f61faeeebe75..10c46101123cc 100644 --- a/tools/aapt2/java/ProguardRules.cpp +++ b/tools/aapt2/java/ProguardRules.cpp @@ -29,18 +29,12 @@ namespace proguard { class BaseVisitor : public xml::Visitor { public: - BaseVisitor(const Source& source, KeepSet* keep_set) - : source_(source), keep_set_(keep_set) {} + using xml::Visitor::Visit; - virtual void Visit(xml::Text*) override{}; - - virtual void Visit(xml::Namespace* node) override { - for (const auto& child : node->children) { - child->Accept(this); - } + BaseVisitor(const Source& source, KeepSet* keep_set) : source_(source), keep_set_(keep_set) { } - virtual void Visit(xml::Element* node) override { + void Visit(xml::Element* node) override { if (!node->namespace_uri.empty()) { Maybe maybe_package = xml::ExtractPackageFromNamespace(node->namespace_uri); @@ -78,10 +72,10 @@ class BaseVisitor : public xml::Visitor { class LayoutVisitor : public BaseVisitor { public: - LayoutVisitor(const Source& source, KeepSet* keep_set) - : BaseVisitor(source, keep_set) {} + LayoutVisitor(const Source& source, KeepSet* keep_set) : BaseVisitor(source, keep_set) { + } - virtual void Visit(xml::Element* node) override { + void Visit(xml::Element* node) override { bool check_class = false; bool check_name = false; if (node->namespace_uri.empty()) { @@ -119,7 +113,7 @@ class MenuVisitor : public BaseVisitor { MenuVisitor(const Source& source, KeepSet* keep_set) : BaseVisitor(source, keep_set) { } - virtual void Visit(xml::Element* node) override { + void Visit(xml::Element* node) override { if (node->namespace_uri.empty() && node->name == "item") { for (const auto& attr : node->attributes) { if (attr.namespace_uri == xml::kSchemaAndroid) { @@ -142,10 +136,10 @@ class MenuVisitor : public BaseVisitor { class XmlResourceVisitor : public BaseVisitor { public: - XmlResourceVisitor(const Source& source, KeepSet* keep_set) - : BaseVisitor(source, keep_set) {} + XmlResourceVisitor(const Source& source, KeepSet* keep_set) : BaseVisitor(source, keep_set) { + } - virtual void Visit(xml::Element* node) override { + void Visit(xml::Element* node) override { bool check_fragment = false; if (node->namespace_uri.empty()) { check_fragment = @@ -169,13 +163,12 @@ class XmlResourceVisitor : public BaseVisitor { class TransitionVisitor : public BaseVisitor { public: - TransitionVisitor(const Source& source, KeepSet* keep_set) - : BaseVisitor(source, keep_set) {} + TransitionVisitor(const Source& source, KeepSet* keep_set) : BaseVisitor(source, keep_set) { + } - virtual void Visit(xml::Element* node) override { + void Visit(xml::Element* node) override { bool check_class = - node->namespace_uri.empty() && - (node->name == "transition" || node->name == "pathMotion"); + node->namespace_uri.empty() && (node->name == "transition" || node->name == "pathMotion"); if (check_class) { xml::Attribute* attr = node->FindAttribute({}, "class"); if (attr && util::IsJavaClassName(attr->value)) { @@ -195,7 +188,7 @@ class ManifestVisitor : public BaseVisitor { ManifestVisitor(const Source& source, KeepSet* keep_set, bool main_dex_only) : BaseVisitor(source, keep_set), main_dex_only_(main_dex_only) {} - virtual void Visit(xml::Element* node) override { + void Visit(xml::Element* node) override { if (node->namespace_uri.empty()) { bool get_name = false; if (node->name == "manifest") { @@ -205,18 +198,15 @@ class ManifestVisitor : public BaseVisitor { } } else if (node->name == "application") { get_name = true; - xml::Attribute* attr = - node->FindAttribute(xml::kSchemaAndroid, "backupAgent"); + xml::Attribute* attr = node->FindAttribute(xml::kSchemaAndroid, "backupAgent"); if (attr) { - Maybe result = - util::GetFullyQualifiedClassName(package_, attr->value); + Maybe result = util::GetFullyQualifiedClassName(package_, attr->value); if (result) { AddClass(node->line_number, result.value()); } } if (main_dex_only_) { - xml::Attribute* default_process = - node->FindAttribute(xml::kSchemaAndroid, "process"); + xml::Attribute* default_process = node->FindAttribute(xml::kSchemaAndroid, "process"); if (default_process) { default_process_ = default_process->value; } @@ -226,8 +216,7 @@ class ManifestVisitor : public BaseVisitor { get_name = true; if (main_dex_only_) { - xml::Attribute* component_process = - node->FindAttribute(xml::kSchemaAndroid, "process"); + xml::Attribute* component_process = node->FindAttribute(xml::kSchemaAndroid, "process"); const std::string& process = component_process ? component_process->value : default_process_; @@ -242,8 +231,7 @@ class ManifestVisitor : public BaseVisitor { get_name = attr != nullptr; if (get_name) { - Maybe result = - util::GetFullyQualifiedClassName(package_, attr->value); + Maybe result = util::GetFullyQualifiedClassName(package_, attr->value); if (result) { AddClass(node->line_number, result.value()); } @@ -261,8 +249,7 @@ class ManifestVisitor : public BaseVisitor { std::string default_process_; }; -bool CollectProguardRulesForManifest(const Source& source, - xml::XmlResource* res, KeepSet* keep_set, +bool CollectProguardRulesForManifest(const Source& source, xml::XmlResource* res, KeepSet* keep_set, bool main_dex_only) { ManifestVisitor visitor(source, keep_set, main_dex_only); if (res->root) { @@ -272,8 +259,7 @@ bool CollectProguardRulesForManifest(const Source& source, return false; } -bool CollectProguardRules(const Source& source, xml::XmlResource* res, - KeepSet* keep_set) { +bool CollectProguardRules(const Source& source, xml::XmlResource* res, KeepSet* keep_set) { if (!res->root) { return false; } @@ -321,8 +307,7 @@ bool WriteKeepSet(std::ostream* out, const KeepSet& keep_set) { for (const Source& source : entry.second) { *out << "# Referenced at " << source << "\n"; } - *out << "-keepclassmembers class * { *** " << entry.first << "(...); }\n" - << std::endl; + *out << "-keepclassmembers class * { *** " << entry.first << "(...); }\n" << std::endl; } return true; } diff --git a/tools/aapt2/link/ManifestFixer_test.cpp b/tools/aapt2/link/ManifestFixer_test.cpp index 80edb352f42c2..da7f410b8b086 100644 --- a/tools/aapt2/link/ManifestFixer_test.cpp +++ b/tools/aapt2/link/ManifestFixer_test.cpp @@ -122,7 +122,7 @@ TEST_F(ManifestFixerTest, UseDefaultSdkVersionsIfNonePresent) { xml::Element* el; xml::Attribute* attr; - el = xml::FindRootElement(doc.get()); + el = doc->root.get(); ASSERT_NE(nullptr, el); el = el->FindChild({}, "uses-sdk"); ASSERT_NE(nullptr, el); @@ -141,7 +141,7 @@ TEST_F(ManifestFixerTest, UseDefaultSdkVersionsIfNonePresent) { options); ASSERT_NE(nullptr, doc); - el = xml::FindRootElement(doc.get()); + el = doc->root.get(); ASSERT_NE(nullptr, el); el = el->FindChild({}, "uses-sdk"); ASSERT_NE(nullptr, el); @@ -160,7 +160,7 @@ TEST_F(ManifestFixerTest, UseDefaultSdkVersionsIfNonePresent) { options); ASSERT_NE(nullptr, doc); - el = xml::FindRootElement(doc.get()); + el = doc->root.get(); ASSERT_NE(nullptr, el); el = el->FindChild({}, "uses-sdk"); ASSERT_NE(nullptr, el); @@ -177,7 +177,7 @@ TEST_F(ManifestFixerTest, UseDefaultSdkVersionsIfNonePresent) { options); ASSERT_NE(nullptr, doc); - el = xml::FindRootElement(doc.get()); + el = doc->root.get(); ASSERT_NE(nullptr, el); el = el->FindChild({}, "uses-sdk"); ASSERT_NE(nullptr, el); @@ -199,7 +199,7 @@ TEST_F(ManifestFixerTest, UsesSdkMustComeBeforeApplication) { options); ASSERT_NE(nullptr, doc); - xml::Element* manifest_el = xml::FindRootElement(doc.get()); + xml::Element* manifest_el = doc->root.get(); ASSERT_NE(nullptr, manifest_el); ASSERT_EQ("manifest", manifest_el->name); @@ -248,7 +248,7 @@ TEST_F(ManifestFixerTest, RenameManifestPackageAndFullyQualifyClasses) { options); ASSERT_NE(nullptr, doc); - xml::Element* manifestEl = xml::FindRootElement(doc.get()); + xml::Element* manifestEl = doc->root.get(); ASSERT_NE(nullptr, manifestEl); xml::Attribute* attr = nullptr; @@ -297,7 +297,7 @@ TEST_F(ManifestFixerTest, options); ASSERT_NE(nullptr, doc); - xml::Element* manifest_el = xml::FindRootElement(doc.get()); + xml::Element* manifest_el = doc->root.get(); ASSERT_NE(nullptr, manifest_el); xml::Element* instrumentation_el = @@ -321,7 +321,7 @@ TEST_F(ManifestFixerTest, UseDefaultVersionNameAndCode) { options); ASSERT_NE(nullptr, doc); - xml::Element* manifest_el = xml::FindRootElement(doc.get()); + xml::Element* manifest_el = doc->root.get(); ASSERT_NE(nullptr, manifest_el); xml::Attribute* attr = @@ -344,7 +344,7 @@ TEST_F(ManifestFixerTest, EnsureManifestAttributesAreTyped) { Verify(""); ASSERT_NE(nullptr, doc); - xml::Element* el = xml::FindRootElement(doc.get()); + xml::Element* el = doc->root.get(); ASSERT_NE(nullptr, el); EXPECT_EQ("manifest", el->name); diff --git a/tools/aapt2/link/XmlCompatVersioner.cpp b/tools/aapt2/link/XmlCompatVersioner.cpp index f1f4e3b7f05ff..20ebdc696814d 100644 --- a/tools/aapt2/link/XmlCompatVersioner.cpp +++ b/tools/aapt2/link/XmlCompatVersioner.cpp @@ -107,7 +107,7 @@ std::unique_ptr XmlCompatVersioner::ProcessDoc( std::unique_ptr cloned_doc = util::make_unique(doc->file); cloned_doc->file.config.sdkVersion = static_cast(target_api); - cloned_doc->root = doc->root->Clone([&](const xml::Element& el, xml::Element* out_el) { + cloned_doc->root = doc->root->CloneElement([&](const xml::Element& el, xml::Element* out_el) { for (const auto& attr : el.attributes) { if (!attr.compiled_attribute) { // Just copy if this isn't a compiled attribute. diff --git a/tools/aapt2/link/XmlCompatVersioner_test.cpp b/tools/aapt2/link/XmlCompatVersioner_test.cpp index ce6605c6ad97d..29ad25f79ab98 100644 --- a/tools/aapt2/link/XmlCompatVersioner_test.cpp +++ b/tools/aapt2/link/XmlCompatVersioner_test.cpp @@ -19,6 +19,12 @@ #include "Linkers.h" #include "test/Test.h" +using ::aapt::test::ValueEq; +using ::testing::Eq; +using ::testing::IsNull; +using ::testing::NotNull; +using ::testing::SizeIs; + namespace aapt { constexpr auto TYPE_DIMENSION = android::ResTable_map::TYPE_DIMENSION; @@ -68,12 +74,12 @@ class XmlCompatVersionerTest : public ::testing::Test { }; TEST_F(XmlCompatVersionerTest, NoRulesOnlyStripsAndCopies) { - auto doc = test::BuildXmlDomForPackageName(context_.get(), R"EOF( + auto doc = test::BuildXmlDomForPackageName(context_.get(), R"( )EOF"); + foo="bar"/>)"); XmlReferenceLinker linker; ASSERT_TRUE(linker.Consume(context_.get(), doc.get())); @@ -84,35 +90,35 @@ TEST_F(XmlCompatVersionerTest, NoRulesOnlyStripsAndCopies) { XmlCompatVersioner versioner(&rules); std::vector> versioned_docs = versioner.Process(context_.get(), doc.get(), api_range); - ASSERT_EQ(2u, versioned_docs.size()); + ASSERT_THAT(versioned_docs, SizeIs(2u)); xml::Element* el; // Source XML file's sdkVersion == 0, so the first one must also have the same sdkVersion. - EXPECT_EQ(static_cast(0), versioned_docs[0]->file.config.sdkVersion); - el = xml::FindRootElement(versioned_docs[0].get()); - ASSERT_NE(nullptr, el); - EXPECT_EQ(2u, el->attributes.size()); - EXPECT_EQ(nullptr, el->FindAttribute(xml::kSchemaAndroid, "paddingHorizontal")); - EXPECT_NE(nullptr, el->FindAttribute(xml::kSchemaAuto, "foo")); - EXPECT_NE(nullptr, el->FindAttribute({}, "foo")); + EXPECT_THAT(versioned_docs[0]->file.config.sdkVersion, Eq(0u)); + el = versioned_docs[0]->root.get(); + ASSERT_THAT(el, NotNull()); + EXPECT_THAT(el->attributes, SizeIs(2u)); + EXPECT_THAT(el->FindAttribute(xml::kSchemaAndroid, "paddingHorizontal"), IsNull()); + EXPECT_THAT(el->FindAttribute(xml::kSchemaAuto, "foo"), NotNull()); + EXPECT_THAT(el->FindAttribute({}, "foo"), NotNull()); - EXPECT_EQ(static_cast(SDK_LOLLIPOP_MR1), versioned_docs[1]->file.config.sdkVersion); - el = xml::FindRootElement(versioned_docs[1].get()); - ASSERT_NE(nullptr, el); - EXPECT_EQ(3u, el->attributes.size()); - EXPECT_NE(nullptr, el->FindAttribute(xml::kSchemaAndroid, "paddingHorizontal")); - EXPECT_NE(nullptr, el->FindAttribute(xml::kSchemaAuto, "foo")); - EXPECT_NE(nullptr, el->FindAttribute({}, "foo")); + EXPECT_THAT(versioned_docs[1]->file.config.sdkVersion, Eq(SDK_LOLLIPOP_MR1)); + el = versioned_docs[1]->root.get(); + ASSERT_THAT(el, NotNull()); + EXPECT_THAT(el->attributes, SizeIs(3u)); + EXPECT_THAT(el->FindAttribute(xml::kSchemaAndroid, "paddingHorizontal"), NotNull()); + EXPECT_THAT(el->FindAttribute(xml::kSchemaAuto, "foo"), NotNull()); + EXPECT_THAT(el->FindAttribute({}, "foo"), NotNull()); } TEST_F(XmlCompatVersionerTest, SingleRule) { - auto doc = test::BuildXmlDomForPackageName(context_.get(), R"EOF( + auto doc = test::BuildXmlDomForPackageName(context_.get(), R"( )EOF"); + foo="bar"/>)"); XmlReferenceLinker linker; ASSERT_TRUE(linker.Consume(context_.get(), doc.get())); @@ -129,51 +135,51 @@ TEST_F(XmlCompatVersionerTest, SingleRule) { XmlCompatVersioner versioner(&rules); std::vector> versioned_docs = versioner.Process(context_.get(), doc.get(), api_range); - ASSERT_EQ(2u, versioned_docs.size()); + ASSERT_THAT(versioned_docs, SizeIs(2u)); xml::Element* el; - EXPECT_EQ(static_cast(0), versioned_docs[0]->file.config.sdkVersion); - el = xml::FindRootElement(versioned_docs[0].get()); - ASSERT_NE(nullptr, el); - EXPECT_EQ(4u, el->attributes.size()); - EXPECT_EQ(nullptr, el->FindAttribute(xml::kSchemaAndroid, "paddingHorizontal")); - EXPECT_NE(nullptr, el->FindAttribute(xml::kSchemaAuto, "foo")); - EXPECT_NE(nullptr, el->FindAttribute({}, "foo")); + EXPECT_THAT(versioned_docs[0]->file.config.sdkVersion, Eq(0u)); + el = versioned_docs[0]->root.get(); + ASSERT_THAT(el, NotNull()); + EXPECT_THAT(el->attributes, SizeIs(4u)); + EXPECT_THAT(el->FindAttribute(xml::kSchemaAndroid, "paddingHorizontal"), IsNull()); + EXPECT_THAT(el->FindAttribute(xml::kSchemaAuto, "foo"), NotNull()); + EXPECT_THAT(el->FindAttribute({}, "foo"), NotNull()); xml::Attribute* attr = el->FindAttribute(xml::kSchemaAndroid, "paddingLeft"); - ASSERT_NE(nullptr, attr); - ASSERT_NE(nullptr, attr->compiled_value); + ASSERT_THAT(attr, NotNull()); + ASSERT_THAT(attr->compiled_value, NotNull()); ASSERT_TRUE(attr->compiled_attribute); attr = el->FindAttribute(xml::kSchemaAndroid, "paddingRight"); - ASSERT_NE(nullptr, attr); - ASSERT_NE(nullptr, attr->compiled_value); + ASSERT_THAT(attr, NotNull()); + ASSERT_THAT(attr->compiled_value, NotNull()); ASSERT_TRUE(attr->compiled_attribute); - EXPECT_EQ(static_cast(SDK_LOLLIPOP_MR1), versioned_docs[1]->file.config.sdkVersion); - el = xml::FindRootElement(versioned_docs[1].get()); - ASSERT_NE(nullptr, el); - EXPECT_EQ(5u, el->attributes.size()); - EXPECT_NE(nullptr, el->FindAttribute(xml::kSchemaAndroid, "paddingHorizontal")); - EXPECT_NE(nullptr, el->FindAttribute(xml::kSchemaAuto, "foo")); - EXPECT_NE(nullptr, el->FindAttribute({}, "foo")); + EXPECT_THAT(versioned_docs[1]->file.config.sdkVersion, Eq(SDK_LOLLIPOP_MR1)); + el = versioned_docs[1]->root.get(); + ASSERT_THAT(el, NotNull()); + EXPECT_THAT(el->attributes, SizeIs(5u)); + EXPECT_THAT(el->FindAttribute(xml::kSchemaAndroid, "paddingHorizontal"), NotNull()); + EXPECT_THAT(el->FindAttribute(xml::kSchemaAuto, "foo"), NotNull()); + EXPECT_THAT(el->FindAttribute({}, "foo"), NotNull()); attr = el->FindAttribute(xml::kSchemaAndroid, "paddingLeft"); - ASSERT_NE(nullptr, attr); - ASSERT_NE(nullptr, attr->compiled_value); + ASSERT_THAT(attr, NotNull()); + ASSERT_THAT(attr->compiled_value, NotNull()); ASSERT_TRUE(attr->compiled_attribute); attr = el->FindAttribute(xml::kSchemaAndroid, "paddingRight"); - ASSERT_NE(nullptr, attr); - ASSERT_NE(nullptr, attr->compiled_value); + ASSERT_THAT(attr, NotNull()); + ASSERT_THAT(attr->compiled_value, NotNull()); ASSERT_TRUE(attr->compiled_attribute); } TEST_F(XmlCompatVersionerTest, ChainedRule) { - auto doc = test::BuildXmlDomForPackageName(context_.get(), R"EOF( + auto doc = test::BuildXmlDomForPackageName(context_.get(), R"( )EOF"); + android:paddingHorizontal="24dp" />)"); XmlReferenceLinker linker; ASSERT_TRUE(linker.Consume(context_.get(), doc.get())); @@ -193,71 +199,70 @@ TEST_F(XmlCompatVersionerTest, ChainedRule) { XmlCompatVersioner versioner(&rules); std::vector> versioned_docs = versioner.Process(context_.get(), doc.get(), api_range); - ASSERT_EQ(3u, versioned_docs.size()); + ASSERT_THAT(versioned_docs, SizeIs(3u)); xml::Element* el; - EXPECT_EQ(static_cast(0), versioned_docs[0]->file.config.sdkVersion); - el = xml::FindRootElement(versioned_docs[0].get()); - ASSERT_NE(nullptr, el); - EXPECT_EQ(2u, el->attributes.size()); - EXPECT_EQ(nullptr, el->FindAttribute(xml::kSchemaAndroid, "paddingHorizontal")); + EXPECT_THAT(versioned_docs[0]->file.config.sdkVersion, Eq(0u)); + el = versioned_docs[0]->root.get(); + ASSERT_THAT(el, NotNull()); + EXPECT_THAT(el->attributes, SizeIs(2u)); + EXPECT_THAT(el->FindAttribute(xml::kSchemaAndroid, "paddingHorizontal"), IsNull()); xml::Attribute* attr = el->FindAttribute(xml::kSchemaAndroid, "paddingLeft"); - ASSERT_NE(nullptr, attr); - ASSERT_NE(nullptr, attr->compiled_value); + ASSERT_THAT(attr, NotNull()); + ASSERT_THAT(attr->compiled_value, NotNull()); ASSERT_TRUE(attr->compiled_attribute); attr = el->FindAttribute(xml::kSchemaAndroid, "paddingRight"); - ASSERT_NE(nullptr, attr); - ASSERT_NE(nullptr, attr->compiled_value); + ASSERT_THAT(attr, NotNull()); + ASSERT_THAT(attr->compiled_value, NotNull()); ASSERT_TRUE(attr->compiled_attribute); - EXPECT_EQ(static_cast(SDK_HONEYCOMB), versioned_docs[1]->file.config.sdkVersion); - el = xml::FindRootElement(versioned_docs[1].get()); - ASSERT_NE(nullptr, el); - EXPECT_EQ(1u, el->attributes.size()); - EXPECT_EQ(nullptr, el->FindAttribute(xml::kSchemaAndroid, "paddingHorizontal")); - EXPECT_EQ(nullptr, el->FindAttribute(xml::kSchemaAndroid, "paddingLeft")); - EXPECT_EQ(nullptr, el->FindAttribute(xml::kSchemaAndroid, "paddingRight")); + EXPECT_THAT(versioned_docs[1]->file.config.sdkVersion, Eq(SDK_HONEYCOMB)); + el = versioned_docs[1]->root.get(); + ASSERT_THAT(el, NotNull()); + EXPECT_THAT(el->attributes, SizeIs(1u)); + EXPECT_THAT(el->FindAttribute(xml::kSchemaAndroid, "paddingHorizontal"), IsNull()); + EXPECT_THAT(el->FindAttribute(xml::kSchemaAndroid, "paddingLeft"), IsNull()); + EXPECT_THAT(el->FindAttribute(xml::kSchemaAndroid, "paddingRight"), IsNull()); attr = el->FindAttribute(xml::kSchemaAndroid, "progressBarPadding"); - ASSERT_NE(nullptr, attr); - ASSERT_NE(nullptr, attr->compiled_value); + ASSERT_THAT(attr, NotNull()); + ASSERT_THAT(attr->compiled_value, NotNull()); ASSERT_TRUE(attr->compiled_attribute); - EXPECT_EQ(static_cast(SDK_LOLLIPOP_MR1), versioned_docs[2]->file.config.sdkVersion); - el = xml::FindRootElement(versioned_docs[2].get()); - ASSERT_NE(nullptr, el); - EXPECT_EQ(2u, el->attributes.size()); - EXPECT_EQ(nullptr, el->FindAttribute(xml::kSchemaAndroid, "paddingLeft")); - EXPECT_EQ(nullptr, el->FindAttribute(xml::kSchemaAndroid, "paddingRight")); + EXPECT_THAT(versioned_docs[2]->file.config.sdkVersion, Eq(SDK_LOLLIPOP_MR1)); + el = versioned_docs[2]->root.get(); + ASSERT_THAT(el, NotNull()); + EXPECT_THAT(el->attributes, SizeIs(2u)); + EXPECT_THAT(el->FindAttribute(xml::kSchemaAndroid, "paddingLeft"), IsNull()); + EXPECT_THAT(el->FindAttribute(xml::kSchemaAndroid, "paddingRight"), IsNull()); attr = el->FindAttribute(xml::kSchemaAndroid, "paddingHorizontal"); - ASSERT_NE(nullptr, attr); - ASSERT_NE(nullptr, attr->compiled_value); + ASSERT_THAT(attr, NotNull()); + ASSERT_THAT(attr->compiled_value, NotNull()); ASSERT_TRUE(attr->compiled_attribute); attr = el->FindAttribute(xml::kSchemaAndroid, "progressBarPadding"); - ASSERT_NE(nullptr, attr); - ASSERT_NE(nullptr, attr->compiled_value); + ASSERT_THAT(attr, NotNull()); + ASSERT_THAT(attr->compiled_value, NotNull()); ASSERT_TRUE(attr->compiled_attribute); } TEST_F(XmlCompatVersionerTest, DegradeRuleOverridesExistingAttribute) { - auto doc = test::BuildXmlDomForPackageName(context_.get(), R"EOF( + auto doc = test::BuildXmlDomForPackageName(context_.get(), R"( )EOF"); + android:paddingRight="16dp"/>)"); XmlReferenceLinker linker; ASSERT_TRUE(linker.Consume(context_.get(), doc.get())); - Item* padding_horizontal_value = xml::FindRootElement(doc.get()) - ->FindAttribute(xml::kSchemaAndroid, "paddingHorizontal") - ->compiled_value.get(); - ASSERT_NE(nullptr, padding_horizontal_value); + Item* padding_horizontal_value = + doc->root->FindAttribute(xml::kSchemaAndroid, "paddingHorizontal")->compiled_value.get(); + ASSERT_THAT(padding_horizontal_value, NotNull()); XmlCompatVersioner::Rules rules; rules[R::attr::paddingHorizontal] = @@ -271,50 +276,50 @@ TEST_F(XmlCompatVersionerTest, DegradeRuleOverridesExistingAttribute) { XmlCompatVersioner versioner(&rules); std::vector> versioned_docs = versioner.Process(context_.get(), doc.get(), api_range); - ASSERT_EQ(2u, versioned_docs.size()); + ASSERT_THAT(versioned_docs, SizeIs(2u)); xml::Element* el; - EXPECT_EQ(static_cast(0), versioned_docs[0]->file.config.sdkVersion); - el = xml::FindRootElement(versioned_docs[0].get()); - ASSERT_NE(nullptr, el); - EXPECT_EQ(2u, el->attributes.size()); - EXPECT_EQ(nullptr, el->FindAttribute(xml::kSchemaAndroid, "paddingHorizontal")); + EXPECT_THAT(versioned_docs[0]->file.config.sdkVersion, Eq(0u)); + el = versioned_docs[0]->root.get(); + ASSERT_THAT(el, NotNull()); + EXPECT_THAT(el->attributes, SizeIs(2u)); + EXPECT_THAT(el->FindAttribute(xml::kSchemaAndroid, "paddingHorizontal"), IsNull()); xml::Attribute* attr = el->FindAttribute(xml::kSchemaAndroid, "paddingLeft"); - ASSERT_NE(nullptr, attr); - ASSERT_NE(nullptr, attr->compiled_value); - ASSERT_TRUE(padding_horizontal_value->Equals(attr->compiled_value.get())); + ASSERT_THAT(attr, NotNull()); + ASSERT_THAT(attr->compiled_value, NotNull()); ASSERT_TRUE(attr->compiled_attribute); + ASSERT_THAT(*attr->compiled_value, ValueEq(padding_horizontal_value)); attr = el->FindAttribute(xml::kSchemaAndroid, "paddingRight"); - ASSERT_NE(nullptr, attr); - ASSERT_NE(nullptr, attr->compiled_value); - ASSERT_TRUE(padding_horizontal_value->Equals(attr->compiled_value.get())); + ASSERT_THAT(attr, NotNull()); + ASSERT_THAT(attr->compiled_value, NotNull()); ASSERT_TRUE(attr->compiled_attribute); + ASSERT_THAT(*attr->compiled_value, ValueEq(padding_horizontal_value)); - EXPECT_EQ(static_cast(SDK_LOLLIPOP_MR1), versioned_docs[1]->file.config.sdkVersion); - el = xml::FindRootElement(versioned_docs[1].get()); - ASSERT_NE(nullptr, el); - EXPECT_EQ(3u, el->attributes.size()); + EXPECT_THAT(versioned_docs[1]->file.config.sdkVersion, Eq(SDK_LOLLIPOP_MR1)); + el = versioned_docs[1]->root.get(); + ASSERT_THAT(el, NotNull()); + EXPECT_THAT(el->attributes, SizeIs(3u)); attr = el->FindAttribute(xml::kSchemaAndroid, "paddingHorizontal"); - ASSERT_NE(nullptr, attr); - ASSERT_NE(nullptr, attr->compiled_value); - ASSERT_TRUE(padding_horizontal_value->Equals(attr->compiled_value.get())); + ASSERT_THAT(attr, NotNull()); + ASSERT_THAT(attr->compiled_value, NotNull()); ASSERT_TRUE(attr->compiled_attribute); + ASSERT_THAT(*attr->compiled_value, ValueEq(padding_horizontal_value)); attr = el->FindAttribute(xml::kSchemaAndroid, "paddingLeft"); - ASSERT_NE(nullptr, attr); - ASSERT_NE(nullptr, attr->compiled_value); - ASSERT_TRUE(padding_horizontal_value->Equals(attr->compiled_value.get())); + ASSERT_THAT(attr, NotNull()); + ASSERT_THAT(attr->compiled_value, NotNull()); ASSERT_TRUE(attr->compiled_attribute); + ASSERT_THAT(*attr->compiled_value, ValueEq(padding_horizontal_value)); attr = el->FindAttribute(xml::kSchemaAndroid, "paddingRight"); - ASSERT_NE(nullptr, attr); - ASSERT_NE(nullptr, attr->compiled_value); - ASSERT_TRUE(padding_horizontal_value->Equals(attr->compiled_value.get())); + ASSERT_THAT(attr, NotNull()); + ASSERT_THAT(attr->compiled_value, NotNull()); ASSERT_TRUE(attr->compiled_attribute); + ASSERT_THAT(*attr->compiled_value, ValueEq(padding_horizontal_value)); } } // namespace aapt diff --git a/tools/aapt2/link/XmlNamespaceRemover.cpp b/tools/aapt2/link/XmlNamespaceRemover.cpp index 24aa5660ae309..b5e2423d58dc1 100644 --- a/tools/aapt2/link/XmlNamespaceRemover.cpp +++ b/tools/aapt2/link/XmlNamespaceRemover.cpp @@ -24,37 +24,19 @@ namespace aapt { namespace { -/** - * Visits each xml Node, removing URI references and nested namespaces. - */ +// Visits each xml Node, removing URI references and nested namespaces. class XmlVisitor : public xml::Visitor { public: explicit XmlVisitor(bool keep_uris) : keep_uris_(keep_uris) {} void Visit(xml::Element* el) override { - // Strip namespaces - for (auto& child : el->children) { - while (child && xml::NodeCast(child.get())) { - if (child->children.empty()) { - child = {}; - } else { - child = std::move(child->children.front()); - child->parent = el; - } - } - } - el->children.erase( - std::remove_if(el->children.begin(), el->children.end(), - [](const std::unique_ptr& child) -> bool { - return child == nullptr; - }), - el->children.end()); + el->namespace_decls.clear(); if (!keep_uris_) { for (xml::Attribute& attr : el->attributes) { - attr.namespace_uri = std::string(); + attr.namespace_uri.clear(); } - el->namespace_uri = std::string(); + el->namespace_uri.clear(); } xml::Visitor::Visit(el); } @@ -67,19 +49,11 @@ class XmlVisitor : public xml::Visitor { } // namespace -bool XmlNamespaceRemover::Consume(IAaptContext* context, - xml::XmlResource* resource) { +bool XmlNamespaceRemover::Consume(IAaptContext* context, xml::XmlResource* resource) { if (!resource->root) { return false; } - // Replace any root namespaces until the root is a non-namespace node - while (xml::NodeCast(resource->root.get())) { - if (resource->root->children.empty()) { - break; - } - resource->root = std::move(resource->root->children.front()); - resource->root->parent = nullptr; - } + XmlVisitor visitor(keep_uris_); resource->root->Accept(&visitor); return true; diff --git a/tools/aapt2/link/XmlNamespaceRemover_test.cpp b/tools/aapt2/link/XmlNamespaceRemover_test.cpp index a176c03a64325..df4920fde37f2 100644 --- a/tools/aapt2/link/XmlNamespaceRemover_test.cpp +++ b/tools/aapt2/link/XmlNamespaceRemover_test.cpp @@ -18,6 +18,10 @@ #include "test/Test.h" +using ::testing::NotNull; +using ::testing::SizeIs; +using ::testing::StrEq; + namespace aapt { class XmlUriTestVisitor : public xml::Visitor { @@ -25,16 +29,14 @@ class XmlUriTestVisitor : public xml::Visitor { XmlUriTestVisitor() = default; void Visit(xml::Element* el) override { - for (const auto& attr : el->attributes) { - EXPECT_EQ(std::string(), attr.namespace_uri); - } - EXPECT_EQ(std::string(), el->namespace_uri); - xml::Visitor::Visit(el); - } + EXPECT_THAT(el->namespace_decls, SizeIs(0u)); - void Visit(xml::Namespace* ns) override { - EXPECT_EQ(std::string(), ns->namespace_uri); - xml::Visitor::Visit(ns); + for (const auto& attr : el->attributes) { + EXPECT_THAT(attr.namespace_uri, StrEq("")); + } + EXPECT_THAT(el->namespace_uri, StrEq("")); + + xml::Visitor::Visit(el); } private: @@ -45,10 +47,9 @@ class XmlNamespaceTestVisitor : public xml::Visitor { public: XmlNamespaceTestVisitor() = default; - void Visit(xml::Namespace* ns) override { - ADD_FAILURE() << "Detected namespace: " << ns->namespace_prefix << "=\"" - << ns->namespace_uri << "\""; - xml::Visitor::Visit(ns); + void Visit(xml::Element* el) override { + EXPECT_THAT(el->namespace_decls, SizeIs(0u)); + xml::Visitor::Visit(el); } private: @@ -58,8 +59,7 @@ class XmlNamespaceTestVisitor : public xml::Visitor { class XmlNamespaceRemoverTest : public ::testing::Test { public: void SetUp() override { - context_ = - test::ContextBuilder().SetCompilationPackage("com.app.test").Build(); + context_ = test::ContextBuilder().SetCompilationPackage("com.app.test").Build(); } protected: @@ -75,8 +75,8 @@ TEST_F(XmlNamespaceRemoverTest, RemoveUris) { XmlNamespaceRemover remover; ASSERT_TRUE(remover.Consume(context_.get(), doc.get())); - xml::Node* root = doc.get()->root.get(); - ASSERT_NE(root, nullptr); + xml::Node* root = doc->root.get(); + ASSERT_THAT(root, NotNull()); XmlUriTestVisitor visitor; root->Accept(&visitor); @@ -93,8 +93,8 @@ TEST_F(XmlNamespaceRemoverTest, RemoveNamespaces) { XmlNamespaceRemover remover; ASSERT_TRUE(remover.Consume(context_.get(), doc.get())); - xml::Node* root = doc.get()->root.get(); - ASSERT_NE(root, nullptr); + xml::Node* root = doc->root.get(); + ASSERT_THAT(root, NotNull()); XmlNamespaceTestVisitor visitor; root->Accept(&visitor); @@ -112,8 +112,8 @@ TEST_F(XmlNamespaceRemoverTest, RemoveNestedNamespaces) { XmlNamespaceRemover remover; ASSERT_TRUE(remover.Consume(context_.get(), doc.get())); - xml::Node* root = doc.get()->root.get(); - ASSERT_NE(root, nullptr); + xml::Node* root = doc->root.get(); + ASSERT_THAT(root, NotNull()); XmlNamespaceTestVisitor visitor; root->Accept(&visitor); diff --git a/tools/aapt2/link/XmlReferenceLinker.cpp b/tools/aapt2/link/XmlReferenceLinker.cpp index 721fc26399bcc..bcecd20038468 100644 --- a/tools/aapt2/link/XmlReferenceLinker.cpp +++ b/tools/aapt2/link/XmlReferenceLinker.cpp @@ -144,7 +144,9 @@ class XmlVisitor : public xml::PackageAwareVisitor { xml::PackageAwareVisitor::Visit(el); } - bool HasError() { return error_ || reference_visitor_.HasError(); } + bool HasError() { + return error_ || reference_visitor_.HasError(); + } private: DISALLOW_COPY_AND_ASSIGN(XmlVisitor); diff --git a/tools/aapt2/link/XmlReferenceLinker_test.cpp b/tools/aapt2/link/XmlReferenceLinker_test.cpp index 228cfb9e3d663..ef99355e5b5fb 100644 --- a/tools/aapt2/link/XmlReferenceLinker_test.cpp +++ b/tools/aapt2/link/XmlReferenceLinker_test.cpp @@ -79,20 +79,20 @@ class XmlReferenceLinkerTest : public ::testing::Test { }; TEST_F(XmlReferenceLinkerTest, LinkBasicAttributes) { - std::unique_ptr doc = test::BuildXmlDomForPackageName(context_.get(), R"EOF( - )EOF"); + std::unique_ptr doc = test::BuildXmlDomForPackageName(context_.get(), R"( + )"); XmlReferenceLinker linker; ASSERT_TRUE(linker.Consume(context_.get(), doc.get())); - xml::Element* view_el = xml::FindRootElement(doc.get()); + xml::Element* view_el = doc->root.get(); ASSERT_THAT(view_el, NotNull()); xml::Attribute* xml_attr = view_el->FindAttribute(xml::kSchemaAndroid, "layout_width"); @@ -138,32 +138,32 @@ TEST_F(XmlReferenceLinkerTest, LinkBasicAttributes) { } TEST_F(XmlReferenceLinkerTest, PrivateSymbolsAreNotLinked) { - std::unique_ptr doc = test::BuildXmlDomForPackageName(context_.get(), R"EOF( - )EOF"); + std::unique_ptr doc = test::BuildXmlDomForPackageName(context_.get(), R"( + )"); XmlReferenceLinker linker; ASSERT_FALSE(linker.Consume(context_.get(), doc.get())); } TEST_F(XmlReferenceLinkerTest, PrivateSymbolsAreLinkedWhenReferenceHasStarPrefix) { - std::unique_ptr doc = test::BuildXmlDomForPackageName(context_.get(), R"EOF( + std::unique_ptr doc = test::BuildXmlDomForPackageName(context_.get(), R"( )EOF"); + android:colorAccent="@*android:color/hidden" />)"); XmlReferenceLinker linker; ASSERT_TRUE(linker.Consume(context_.get(), doc.get())); } TEST_F(XmlReferenceLinkerTest, LinkMangledAttributes) { - std::unique_ptr doc = test::BuildXmlDomForPackageName(context_.get(), R"EOF( - )EOF"); + std::unique_ptr doc = test::BuildXmlDomForPackageName(context_.get(), R"( + )"); XmlReferenceLinker linker; ASSERT_TRUE(linker.Consume(context_.get(), doc.get())); - xml::Element* view_el = xml::FindRootElement(doc.get()); + xml::Element* view_el = doc->root.get(); ASSERT_THAT(view_el, NotNull()); xml::Attribute* xml_attr = @@ -175,14 +175,14 @@ TEST_F(XmlReferenceLinkerTest, LinkMangledAttributes) { } TEST_F(XmlReferenceLinkerTest, LinkAutoResReference) { - std::unique_ptr doc = test::BuildXmlDomForPackageName(context_.get(), R"EOF( - )EOF"); + std::unique_ptr doc = test::BuildXmlDomForPackageName(context_.get(), R"( + )"); XmlReferenceLinker linker; ASSERT_TRUE(linker.Consume(context_.get(), doc.get())); - xml::Element* view_el = xml::FindRootElement(doc.get()); + xml::Element* view_el = doc->root.get(); ASSERT_THAT(view_el, NotNull()); xml::Attribute* xml_attr = view_el->FindAttribute(xml::kSchemaAuto, "colorAccent"); @@ -196,17 +196,15 @@ TEST_F(XmlReferenceLinkerTest, LinkAutoResReference) { } TEST_F(XmlReferenceLinkerTest, LinkViewWithShadowedPackageAlias) { - std::unique_ptr doc = test::BuildXmlDomForPackageName(context_.get(), R"EOF( - - - )EOF"); + std::unique_ptr doc = test::BuildXmlDomForPackageName(context_.get(), R"( + + + )"); XmlReferenceLinker linker; ASSERT_TRUE(linker.Consume(context_.get(), doc.get())); - xml::Element* view_el = xml::FindRootElement(doc.get()); + xml::Element* view_el = doc->root.get(); ASSERT_THAT(view_el, NotNull()); // All attributes and references in this element should be referring to @@ -235,14 +233,14 @@ TEST_F(XmlReferenceLinkerTest, LinkViewWithShadowedPackageAlias) { } TEST_F(XmlReferenceLinkerTest, LinkViewWithLocalPackageAndAliasOfTheSameName) { - std::unique_ptr doc = test::BuildXmlDomForPackageName(context_.get(), R"EOF( - )EOF"); + std::unique_ptr doc = test::BuildXmlDomForPackageName(context_.get(), R"( + )"); XmlReferenceLinker linker; ASSERT_TRUE(linker.Consume(context_.get(), doc.get())); - xml::Element* view_el = xml::FindRootElement(doc.get()); + xml::Element* view_el = doc->root.get(); ASSERT_THAT(view_el, NotNull()); // All attributes and references in this element should be referring to diff --git a/tools/aapt2/proto/ProtoHelpers.h b/tools/aapt2/proto/ProtoHelpers.h index 344e9477ea714..cecec2d06c030 100644 --- a/tools/aapt2/proto/ProtoHelpers.h +++ b/tools/aapt2/proto/ProtoHelpers.h @@ -27,23 +27,18 @@ namespace aapt { -void SerializeStringPoolToPb(const StringPool& pool, - pb::StringPool* out_pb_pool); +void SerializeStringPoolToPb(const StringPool& pool, pb::StringPool* out_pb_pool); -void SerializeSourceToPb(const Source& source, StringPool* src_pool, - pb::Source* out_pb_source); +void SerializeSourceToPb(const Source& source, StringPool* src_pool, pb::Source* out_pb_source); -void DeserializeSourceFromPb(const pb::Source& pb_source, - const android::ResStringPool& src_pool, +void DeserializeSourceFromPb(const pb::Source& pb_source, const android::ResStringPool& src_pool, Source* out_source); pb::SymbolStatus_Visibility SerializeVisibilityToPb(SymbolState state); -SymbolState DeserializeVisibilityFromPb( - pb::SymbolStatus_Visibility pb_visibility); +SymbolState DeserializeVisibilityFromPb(pb::SymbolStatus_Visibility pb_visibility); -void SerializeConfig(const ConfigDescription& config, - pb::ConfigDescription* out_pb_config); +void SerializeConfig(const ConfigDescription& config, pb::ConfigDescription* out_pb_config); bool DeserializeConfigDescriptionFromPb(const pb::ConfigDescription& pb_config, ConfigDescription* out_config); diff --git a/tools/aapt2/test/Builders.cpp b/tools/aapt2/test/Builders.cpp index 80e6adf134e6f..57e3df442e3c3 100644 --- a/tools/aapt2/test/Builders.cpp +++ b/tools/aapt2/test/Builders.cpp @@ -201,7 +201,7 @@ std::unique_ptr BuildXmlDom(const StringPiece& str) { StringInputStream in(input); StdErrDiagnostics diag; std::unique_ptr doc = xml::Inflate(&in, &diag, Source("test.xml")); - CHECK(doc != nullptr) << "failed to parse inline XML string"; + CHECK(doc != nullptr && doc->root != nullptr) << "failed to parse inline XML string"; return doc; } diff --git a/tools/aapt2/test/Common.h b/tools/aapt2/test/Common.h index d53c92f6a7b9d..7482b8b627d30 100644 --- a/tools/aapt2/test/Common.h +++ b/tools/aapt2/test/Common.h @@ -142,10 +142,97 @@ MATCHER_P(StrEq, a, return android::StringPiece16(arg) == a; } -MATCHER_P(ValueEq, a, - std::string(negation ? "isn't" : "is") + " equal to " + ::testing::PrintToString(a)) { - return arg.Equals(&a); -} +class ValueEq { + public: + template + class BaseImpl : public ::testing::MatcherInterface { + BaseImpl(const BaseImpl&) = default; + + void DescribeTo(::std::ostream* os) const override { + *os << "is equal to " << *expected_; + } + + void DescribeNegationTo(::std::ostream* os) const override { + *os << "is not equal to " << *expected_; + } + + protected: + BaseImpl(const Value* expected) : expected_(expected) { + } + + const Value* expected_; + }; + + template + class Impl {}; + + template + class Impl : public ::testing::MatcherInterface { + public: + explicit Impl(const Value* expected) : expected_(expected) { + } + + bool MatchAndExplain(T x, ::testing::MatchResultListener* listener) const override { + return expected_->Equals(&x); + } + + void DescribeTo(::std::ostream* os) const override { + *os << "is equal to " << *expected_; + } + + void DescribeNegationTo(::std::ostream* os) const override { + *os << "is not equal to " << *expected_; + } + + private: + DISALLOW_COPY_AND_ASSIGN(Impl); + + const Value* expected_; + }; + + template + class Impl : public ::testing::MatcherInterface { + public: + explicit Impl(const Value* expected) : expected_(expected) { + } + + bool MatchAndExplain(T x, ::testing::MatchResultListener* listener) const override { + return expected_->Equals(x); + } + + void DescribeTo(::std::ostream* os) const override { + *os << "is equal to " << *expected_; + } + + void DescribeNegationTo(::std::ostream* os) const override { + *os << "is not equal to " << *expected_; + } + + private: + DISALLOW_COPY_AND_ASSIGN(Impl); + + const Value* expected_; + }; + + ValueEq(const Value& expected) : expected_(&expected) { + } + ValueEq(const Value* expected) : expected_(expected) { + } + ValueEq(const ValueEq&) = default; + + template + operator ::testing::Matcher() const { + return ::testing::Matcher(new Impl::value>(expected_)); + } + + private: + const Value* expected_; +}; + +// MATCHER_P(ValueEq, a, +// std::string(negation ? "isn't" : "is") + " equal to " + ::testing::PrintToString(a)) { +// return arg.Equals(&a); +//} MATCHER_P(StrValueEq, a, std::string(negation ? "isn't" : "is") + " equal to " + ::testing::PrintToString(a)) { diff --git a/tools/aapt2/xml/XmlActionExecutor.cpp b/tools/aapt2/xml/XmlActionExecutor.cpp index 352a933616333..cc664a5de7224 100644 --- a/tools/aapt2/xml/XmlActionExecutor.cpp +++ b/tools/aapt2/xml/XmlActionExecutor.cpp @@ -78,7 +78,7 @@ bool XmlActionExecutor::Execute(XmlActionExecutorPolicy policy, IDiagnostics* di XmlResource* doc) const { SourcePathDiagnostics source_diag(doc->file.source, diag); - Element* el = FindRootElement(doc); + Element* el = doc->root.get(); if (!el) { if (policy == XmlActionExecutorPolicy::kWhitelist) { source_diag.Error(DiagMessage() << "no root XML tag found"); diff --git a/tools/aapt2/xml/XmlDom.cpp b/tools/aapt2/xml/XmlDom.cpp index d6df7150214bf..cbb652ed9a1a2 100644 --- a/tools/aapt2/xml/XmlDom.cpp +++ b/tools/aapt2/xml/XmlDom.cpp @@ -39,17 +39,15 @@ namespace xml { constexpr char kXmlNamespaceSep = 1; struct Stack { - std::unique_ptr root; - std::stack node_stack; + std::unique_ptr root; + std::stack node_stack; + std::unique_ptr pending_element; std::string pending_comment; std::unique_ptr last_text_node; }; -/** - * Extracts the namespace and name of an expanded element or attribute name. - */ -static void SplitName(const char* name, std::string* out_ns, - std::string* out_name) { +// Extracts the namespace and name of an expanded element or attribute name. +static void SplitName(const char* name, std::string* out_ns, std::string* out_name) { const char* p = name; while (*p != 0 && *p != kXmlNamespaceSep) { p++; @@ -67,6 +65,7 @@ static void SplitName(const char* name, std::string* out_ns, static void FinishPendingText(Stack* stack) { if (stack->last_text_node != nullptr) { if (!stack->last_text_node->text.empty()) { + CHECK(!stack->node_stack.empty()); stack->node_stack.top()->AppendChild(std::move(stack->last_text_node)); } else { // Drop an empty text node. @@ -75,48 +74,27 @@ static void FinishPendingText(Stack* stack) { } } -static void AddToStack(Stack* stack, XML_Parser parser, - std::unique_ptr node) { - node->line_number = XML_GetCurrentLineNumber(parser); - node->column_number = XML_GetCurrentColumnNumber(parser); - - Node* this_node = node.get(); - if (!stack->node_stack.empty()) { - stack->node_stack.top()->AppendChild(std::move(node)); - } else { - stack->root = std::move(node); - } - - if (!NodeCast(this_node)) { - stack->node_stack.push(this_node); - } -} - -static void XMLCALL StartNamespaceHandler(void* user_data, const char* prefix, - const char* uri) { +static void XMLCALL StartNamespaceHandler(void* user_data, const char* prefix, const char* uri) { XML_Parser parser = reinterpret_cast(user_data); Stack* stack = reinterpret_cast(XML_GetUserData(parser)); FinishPendingText(stack); - std::unique_ptr ns = util::make_unique(); - if (prefix) { - ns->namespace_prefix = prefix; - } + NamespaceDecl decl; + decl.line_number = XML_GetCurrentLineNumber(parser); + decl.column_number = XML_GetCurrentColumnNumber(parser); + decl.prefix = prefix ? prefix : ""; + decl.uri = uri ? uri : ""; - if (uri) { - ns->namespace_uri = uri; + if (stack->pending_element == nullptr) { + stack->pending_element = util::make_unique(); } - - AddToStack(stack, parser, std::move(ns)); + stack->pending_element->namespace_decls.push_back(std::move(decl)); } -static void XMLCALL EndNamespaceHandler(void* user_data, const char* prefix) { +static void XMLCALL EndNamespaceHandler(void* user_data, const char* /*prefix*/) { XML_Parser parser = reinterpret_cast(user_data); Stack* stack = reinterpret_cast(XML_GetUserData(parser)); FinishPendingText(stack); - - CHECK(!stack->node_stack.empty()); - stack->node_stack.pop(); } static bool less_attribute(const Attribute& lhs, const Attribute& rhs) { @@ -124,28 +102,42 @@ static bool less_attribute(const Attribute& lhs, const Attribute& rhs) { std::tie(rhs.namespace_uri, rhs.name, rhs.value); } -static void XMLCALL StartElementHandler(void* user_data, const char* name, - const char** attrs) { +static void XMLCALL StartElementHandler(void* user_data, const char* name, const char** attrs) { XML_Parser parser = reinterpret_cast(user_data); Stack* stack = reinterpret_cast(XML_GetUserData(parser)); FinishPendingText(stack); - std::unique_ptr el = util::make_unique(); + std::unique_ptr el; + if (stack->pending_element != nullptr) { + el = std::move(stack->pending_element); + } else { + el = util::make_unique(); + } + + el->line_number = XML_GetCurrentLineNumber(parser); + el->column_number = XML_GetCurrentColumnNumber(parser); + el->comment = std::move(stack->pending_comment); + SplitName(name, &el->namespace_uri, &el->name); while (*attrs) { Attribute attribute; SplitName(*attrs++, &attribute.namespace_uri, &attribute.name); attribute.value = *attrs++; - - // Insert in sorted order. - auto iter = std::lower_bound(el->attributes.begin(), el->attributes.end(), attribute, - less_attribute); - el->attributes.insert(iter, std::move(attribute)); + el->attributes.push_back(std::move(attribute)); } - el->comment = std::move(stack->pending_comment); - AddToStack(stack, parser, std::move(el)); + // Sort the attributes. + std::sort(el->attributes.begin(), el->attributes.end(), less_attribute); + + // Add to the stack. + Element* this_el = el.get(); + if (!stack->node_stack.empty()) { + stack->node_stack.top()->AppendChild(std::move(el)); + } else { + stack->root = std::move(el); + } + stack->node_stack.push(this_el); } static void XMLCALL EndElementHandler(void* user_data, const char* name) { @@ -263,13 +255,13 @@ static void CopyAttributes(Element* el, android::ResXMLParser* parser, StringPoo std::unique_ptr Inflate(const void* data, size_t data_len, IDiagnostics* diag, const Source& source) { // We import the android namespace because on Windows NO_ERROR is a macro, not - // an enum, which - // causes errors when qualifying it with android:: + // an enum, which causes errors when qualifying it with android:: using namespace android; StringPool string_pool; - std::unique_ptr root; - std::stack node_stack; + std::unique_ptr root; + std::stack node_stack; + std::unique_ptr pending_element; ResXMLTree tree; if (tree.setTo(data, data_len) != NO_ERROR) { @@ -277,57 +269,76 @@ std::unique_ptr Inflate(const void* data, size_t data_len, IDiagnos } ResXMLParser::event_code_t code; - while ((code = tree.next()) != ResXMLParser::BAD_DOCUMENT && - code != ResXMLParser::END_DOCUMENT) { + while ((code = tree.next()) != ResXMLParser::BAD_DOCUMENT && code != ResXMLParser::END_DOCUMENT) { std::unique_ptr new_node; switch (code) { case ResXMLParser::START_NAMESPACE: { - std::unique_ptr node = util::make_unique(); + NamespaceDecl decl; size_t len; const char16_t* str16 = tree.getNamespacePrefix(&len); if (str16) { - node->namespace_prefix = util::Utf16ToUtf8(StringPiece16(str16, len)); + decl.prefix = util::Utf16ToUtf8(StringPiece16(str16, len)); } str16 = tree.getNamespaceUri(&len); if (str16) { - node->namespace_uri = util::Utf16ToUtf8(StringPiece16(str16, len)); + decl.uri = util::Utf16ToUtf8(StringPiece16(str16, len)); + } + + if (pending_element == nullptr) { + pending_element = util::make_unique(); } - new_node = std::move(node); break; } case ResXMLParser::START_TAG: { - std::unique_ptr node = util::make_unique(); + std::unique_ptr el; + if (pending_element != nullptr) { + el = std::move(pending_element); + } else { + el = util::make_unique(); + ; + } + size_t len; const char16_t* str16 = tree.getElementNamespace(&len); if (str16) { - node->namespace_uri = util::Utf16ToUtf8(StringPiece16(str16, len)); + el->namespace_uri = util::Utf16ToUtf8(StringPiece16(str16, len)); } str16 = tree.getElementName(&len); if (str16) { - node->name = util::Utf16ToUtf8(StringPiece16(str16, len)); + el->name = util::Utf16ToUtf8(StringPiece16(str16, len)); } - CopyAttributes(node.get(), &tree, &string_pool); + Element* this_el = el.get(); + CopyAttributes(el.get(), &tree, &string_pool); - new_node = std::move(node); + if (!node_stack.empty()) { + node_stack.top()->AppendChild(std::move(el)); + } else { + root = std::move(el); + } + node_stack.push(this_el); break; } case ResXMLParser::TEXT: { - std::unique_ptr node = util::make_unique(); + std::unique_ptr text = util::make_unique(); + text->line_number = tree.getLineNumber(); size_t len; const char16_t* str16 = tree.getText(&len); if (str16) { - node->text = util::Utf16ToUtf8(StringPiece16(str16, len)); + text->text = util::Utf16ToUtf8(StringPiece16(str16, len)); } - new_node = std::move(node); + CHECK(!node_stack.empty()); + node_stack.top()->AppendChild(std::move(text)); break; } case ResXMLParser::END_NAMESPACE: + break; + case ResXMLParser::END_TAG: CHECK(!node_stack.empty()); node_stack.pop(); @@ -337,74 +348,32 @@ std::unique_ptr Inflate(const void* data, size_t data_len, IDiagnos LOG(FATAL) << "unhandled XML chunk type"; break; } - - if (new_node) { - new_node->line_number = tree.getLineNumber(); - - Node* this_node = new_node.get(); - if (!root) { - CHECK(node_stack.empty()) << "node stack should be empty"; - root = std::move(new_node); - } else { - CHECK(!node_stack.empty()) << "node stack should not be empty"; - node_stack.top()->AppendChild(std::move(new_node)); - } - - if (!NodeCast(this_node)) { - node_stack.push(this_node); - } - } } return util::make_unique(ResourceFile{}, std::move(string_pool), std::move(root)); } -std::unique_ptr Namespace::Clone(const ElementCloneFunc& el_cloner) { - auto ns = util::make_unique(); - ns->comment = comment; - ns->line_number = line_number; - ns->column_number = column_number; - ns->namespace_prefix = namespace_prefix; - ns->namespace_uri = namespace_uri; - ns->children.reserve(children.size()); - for (const std::unique_ptr& child : children) { - ns->AppendChild(child->Clone(el_cloner)); - } - return std::move(ns); -} - -Element* FindRootElement(XmlResource* doc) { - return FindRootElement(doc->root.get()); -} - Element* FindRootElement(Node* node) { - if (!node) { + if (node == nullptr) { return nullptr; } - Element* el = nullptr; - while ((el = NodeCast(node)) == nullptr) { - if (node->children.empty()) { - return nullptr; - } - // We are looking for the first element, and namespaces can only have one - // child. - node = node->children.front().get(); + while (node->parent != nullptr) { + node = node->parent; } - return el; + return NodeCast(node); } -void Node::AppendChild(std::unique_ptr child) { +void Element::AppendChild(std::unique_ptr child) { child->parent = this; children.push_back(std::move(child)); } -void Node::InsertChild(size_t index, std::unique_ptr child) { +void Element::InsertChild(size_t index, std::unique_ptr child) { child->parent = this; children.insert(children.begin() + index, std::move(child)); } -Attribute* Element::FindAttribute(const StringPiece& ns, - const StringPiece& name) { +Attribute* Element::FindAttribute(const StringPiece& ns, const StringPiece& name) { for (auto& attr : attributes) { if (ns == attr.namespace_uri && name == attr.name) { return &attr; @@ -426,21 +395,11 @@ Element* Element::FindChild(const StringPiece& ns, const StringPiece& name) { return FindChildWithAttribute(ns, name, {}, {}, {}); } -Element* Element::FindChildWithAttribute(const StringPiece& ns, - const StringPiece& name, - const StringPiece& attr_ns, - const StringPiece& attr_name, +Element* Element::FindChildWithAttribute(const StringPiece& ns, const StringPiece& name, + const StringPiece& attr_ns, const StringPiece& attr_name, const StringPiece& attr_value) { - for (auto& child_node : children) { - Node* child = child_node.get(); - while (NodeCast(child)) { - if (child->children.empty()) { - break; - } - child = child->children[0].get(); - } - - if (Element* el = NodeCast(child)) { + for (auto& child : children) { + if (Element* el = NodeCast(child.get())) { if (ns == el->namespace_uri && name == el->name) { if (attr_ns.empty() && attr_name.empty()) { return el; @@ -459,23 +418,16 @@ Element* Element::FindChildWithAttribute(const StringPiece& ns, std::vector Element::GetChildElements() { std::vector elements; for (auto& child_node : children) { - Node* child = child_node.get(); - while (NodeCast(child)) { - if (child->children.empty()) { - break; - } - child = child->children[0].get(); - } - - if (Element* el = NodeCast(child)) { - elements.push_back(el); + if (Element* child = NodeCast(child_node.get())) { + elements.push_back(child); } } return elements; } -std::unique_ptr Element::Clone(const ElementCloneFunc& el_cloner) { +std::unique_ptr Element::Clone(const ElementCloneFunc& el_cloner) const { auto el = util::make_unique(); + el->namespace_decls = namespace_decls; el->comment = comment; el->line_number = line_number; el->column_number = column_number; @@ -490,7 +442,17 @@ std::unique_ptr Element::Clone(const ElementCloneFunc& el_cloner) { return std::move(el); } -std::unique_ptr Text::Clone(const ElementCloneFunc&) { +std::unique_ptr Element::CloneElement(const ElementCloneFunc& el_cloner) const { + return std::unique_ptr(static_cast(Clone(el_cloner).release())); +} + +void Element::Accept(Visitor* visitor) { + visitor->BeforeVisitElement(this); + visitor->Visit(this); + visitor->AfterVisitElement(this); +} + +std::unique_ptr Text::Clone(const ElementCloneFunc&) const { auto t = util::make_unique(); t->comment = comment; t->line_number = line_number; @@ -499,21 +461,22 @@ std::unique_ptr Text::Clone(const ElementCloneFunc&) { return std::move(t); } -void PackageAwareVisitor::Visit(Namespace* ns) { - bool added = false; - if (Maybe maybe_package = - ExtractPackageFromNamespace(ns->namespace_uri)) { - ExtractedPackage& package = maybe_package.value(); - package_decls_.push_back( - PackageDecl{ns->namespace_prefix, std::move(package)}); - added = true; - } +void Text::Accept(Visitor* visitor) { + visitor->Visit(this); +} - Visitor::Visit(ns); - - if (added) { - package_decls_.pop_back(); +void PackageAwareVisitor::BeforeVisitElement(Element* el) { + std::vector decls; + for (const NamespaceDecl& decl : el->namespace_decls) { + if (Maybe maybe_package = ExtractPackageFromNamespace(decl.uri)) { + decls.push_back(PackageDecl{decl.prefix, std::move(maybe_package.value())}); + } } + package_decls_.push_back(std::move(decls)); +} + +void PackageAwareVisitor::AfterVisitElement(Element* el) { + package_decls_.pop_back(); } Maybe PackageAwareVisitor::TransformPackageAlias( @@ -524,11 +487,16 @@ Maybe PackageAwareVisitor::TransformPackageAlias( const auto rend = package_decls_.rend(); for (auto iter = package_decls_.rbegin(); iter != rend; ++iter) { - if (alias == iter->prefix) { - if (iter->package.package.empty()) { - return ExtractedPackage{local_package.to_string(), iter->package.private_namespace}; + const std::vector& decls = *iter; + const auto rend2 = decls.rend(); + for (auto iter2 = decls.rbegin(); iter2 != rend2; ++iter2) { + const PackageDecl& decl = *iter2; + if (alias == decl.prefix) { + if (decl.package.package.empty()) { + return ExtractedPackage{local_package.to_string(), decl.package.private_namespace}; + } + return decl.package; } - return iter->package; } } return {}; diff --git a/tools/aapt2/xml/XmlDom.h b/tools/aapt2/xml/XmlDom.h index 54a70333fad26..1542243816266 100644 --- a/tools/aapt2/xml/XmlDom.h +++ b/tools/aapt2/xml/XmlDom.h @@ -33,45 +33,33 @@ namespace aapt { namespace xml { -class RawVisitor; - class Element; +class Visitor; // Base class for all XML nodes. class Node { public: - Node* parent = nullptr; - size_t line_number = 0; - size_t column_number = 0; - std::string comment; - std::vector> children; - virtual ~Node() = default; - void AppendChild(std::unique_ptr child); - void InsertChild(size_t index, std::unique_ptr child); - virtual void Accept(RawVisitor* visitor) = 0; + Element* parent = nullptr; + size_t line_number = 0u; + size_t column_number = 0u; + std::string comment; + + virtual void Accept(Visitor* visitor) = 0; using ElementCloneFunc = std::function; // Clones the Node subtree, using the given function to decide how to clone an Element. - virtual std::unique_ptr Clone(const ElementCloneFunc& el_cloner) = 0; + virtual std::unique_ptr Clone(const ElementCloneFunc& el_cloner) const = 0; }; -// Base class that implements the visitor methods for a subclass of Node. -template -class BaseNode : public Node { - public: - virtual void Accept(RawVisitor* visitor) override; -}; - -// A Namespace XML node. Can only have one child. -class Namespace : public BaseNode { - public: - std::string namespace_prefix; - std::string namespace_uri; - - std::unique_ptr Clone(const ElementCloneFunc& el_cloner) override; +// A namespace declaration (xmlns:prefix="uri"). +struct NamespaceDecl { + std::string prefix; + std::string uri; + size_t line_number = 0u; + size_t column_number = 0u; }; struct AaptAttribute { @@ -94,31 +82,46 @@ struct Attribute { }; // An Element XML node. -class Element : public BaseNode { +class Element : public Node { public: + // Ordered namespace prefix declarations. + std::vector namespace_decls; + std::string namespace_uri; std::string name; std::vector attributes; + std::vector> children; + + void AppendChild(std::unique_ptr child); + void InsertChild(size_t index, std::unique_ptr child); Attribute* FindAttribute(const android::StringPiece& ns, const android::StringPiece& name); const Attribute* FindAttribute(const android::StringPiece& ns, const android::StringPiece& name) const; - xml::Element* FindChild(const android::StringPiece& ns, const android::StringPiece& name); - xml::Element* FindChildWithAttribute(const android::StringPiece& ns, - const android::StringPiece& name, - const android::StringPiece& attr_ns, - const android::StringPiece& attr_name, - const android::StringPiece& attr_value); - std::vector GetChildElements(); - std::unique_ptr Clone(const ElementCloneFunc& el_cloner) override; + Element* FindChild(const android::StringPiece& ns, const android::StringPiece& name); + Element* FindChildWithAttribute(const android::StringPiece& ns, const android::StringPiece& name, + const android::StringPiece& attr_ns, + const android::StringPiece& attr_name, + const android::StringPiece& attr_value); + std::vector GetChildElements(); + + // Due to overriding of subtypes not working with unique_ptr, define a convenience Clone method + // that knows cloning an element returns an element. + std::unique_ptr CloneElement(const ElementCloneFunc& el_cloner) const; + + std::unique_ptr Clone(const ElementCloneFunc& el_cloner) const override; + + void Accept(Visitor* visitor) override; }; // A Text (CDATA) XML node. Can not have any children. -class Text : public BaseNode { +class Text : public Node { public: std::string text; - std::unique_ptr Clone(const ElementCloneFunc& el_cloner) override; + std::unique_ptr Clone(const ElementCloneFunc& el_cloner) const override; + + void Accept(Visitor* visitor) override; }; // An XML resource with a source, name, and XML tree. @@ -131,7 +134,7 @@ class XmlResource { // is destroyed. StringPool string_pool; - std::unique_ptr root; + std::unique_ptr root; }; // Inflates an XML DOM from an InputStream, logging errors to the logger. @@ -143,42 +146,38 @@ std::unique_ptr Inflate(io::InputStream* in, IDiagnostics* diag, co std::unique_ptr Inflate(const void* data, size_t data_len, IDiagnostics* diag, const Source& source); -Element* FindRootElement(XmlResource* doc); Element* FindRootElement(Node* node); -// A visitor interface for the different XML Node subtypes. This will not traverse into children. -// Use Visitor for that. -class RawVisitor { - public: - virtual ~RawVisitor() = default; - - virtual void Visit(Namespace* node) {} - virtual void Visit(Element* node) {} - virtual void Visit(Text* text) {} -}; - // Visitor whose default implementation visits the children nodes of any node. -class Visitor : public RawVisitor { +class Visitor { public: - using RawVisitor::Visit; + virtual ~Visitor() = default; - void Visit(Namespace* node) override { - VisitChildren(node); + virtual void Visit(Element* el) { + VisitChildren(el); } - void Visit(Element* node) override { - VisitChildren(node); + virtual void Visit(Text* text) { } - void Visit(Text* text) override { - VisitChildren(text); - } + protected: + Visitor() = default; - void VisitChildren(Node* node) { - for (auto& child : node->children) { + void VisitChildren(Element* el) { + for (auto& child : el->children) { child->Accept(this); } } + + virtual void BeforeVisitElement(Element* el) { + } + virtual void AfterVisitElement(Element* el) { + } + + private: + DISALLOW_COPY_AND_ASSIGN(Visitor); + + friend class Element; }; // An XML DOM visitor that will record the package name for a namespace prefix. @@ -186,41 +185,70 @@ class PackageAwareVisitor : public Visitor, public IPackageDeclStack { public: using Visitor::Visit; - void Visit(Namespace* ns) override; Maybe TransformPackageAlias( const android::StringPiece& alias, const android::StringPiece& local_package) const override; + protected: + PackageAwareVisitor() = default; + + void BeforeVisitElement(Element* el) override; + void AfterVisitElement(Element* el) override; + private: + DISALLOW_COPY_AND_ASSIGN(PackageAwareVisitor); + struct PackageDecl { std::string prefix; ExtractedPackage package; }; - std::vector package_decls_; + std::vector> package_decls_; }; -// Implementations +namespace internal { -template -void BaseNode::Accept(RawVisitor* visitor) { - visitor->Visit(static_cast(this)); -} +// Base class that overrides the default behaviour and does not descend into child nodes. +class NodeCastBase : public Visitor { + public: + void Visit(Element* el) override { + } + void Visit(Text* el) override { + } + + protected: + NodeCastBase() = default; + + void BeforeVisitElement(Element* el) override { + } + void AfterVisitElement(Element* el) override { + } + + private: + DISALLOW_COPY_AND_ASSIGN(NodeCastBase); +}; template -class NodeCastImpl : public RawVisitor { +class NodeCastImpl : public NodeCastBase { public: - using RawVisitor::Visit; + using NodeCastBase::Visit; + + NodeCastImpl() = default; T* value = nullptr; void Visit(T* v) override { value = v; } + + private: + DISALLOW_COPY_AND_ASSIGN(NodeCastImpl); }; +} // namespace internal + template T* NodeCast(Node* node) { - NodeCastImpl visitor; + internal::NodeCastImpl visitor; node->Accept(&visitor); return visitor.value; } diff --git a/tools/aapt2/xml/XmlDom_test.cpp b/tools/aapt2/xml/XmlDom_test.cpp index 1340ada6d9538..6ed2d616f7828 100644 --- a/tools/aapt2/xml/XmlDom_test.cpp +++ b/tools/aapt2/xml/XmlDom_test.cpp @@ -25,8 +25,10 @@ using ::aapt::io::StringInputStream; using ::testing::Eq; using ::testing::NotNull; using ::testing::SizeIs; +using ::testing::StrEq; namespace aapt { +namespace xml { TEST(XmlDomTest, Inflate) { std::string input = R"( @@ -40,24 +42,23 @@ TEST(XmlDomTest, Inflate) { StdErrDiagnostics diag; StringInputStream in(input); - std::unique_ptr doc = xml::Inflate(&in, &diag, Source("test.xml")); + std::unique_ptr doc = Inflate(&in, &diag, Source("test.xml")); ASSERT_THAT(doc, NotNull()); - xml::Namespace* ns = xml::NodeCast(doc->root.get()); - ASSERT_THAT(ns, NotNull()); - EXPECT_THAT(ns->namespace_uri, Eq(xml::kSchemaAndroid)); - EXPECT_THAT(ns->namespace_prefix, Eq("android")); + Element* el = doc->root.get(); + EXPECT_THAT(el->namespace_decls, SizeIs(1u)); + EXPECT_THAT(el->namespace_decls[0].uri, StrEq(xml::kSchemaAndroid)); + EXPECT_THAT(el->namespace_decls[0].prefix, StrEq("android")); } // Escaping is handled after parsing of the values for resource-specific values. TEST(XmlDomTest, ForwardEscapes) { - std::unique_ptr doc = test::BuildXmlDom(R"( + std::unique_ptr doc = test::BuildXmlDom(R"( \\d{5})"); - xml::Element* el = xml::FindRootElement(doc.get()); - ASSERT_THAT(el, NotNull()); + Element* el = doc->root.get(); - xml::Attribute* attr = el->FindAttribute({}, "pattern"); + Attribute* attr = el->FindAttribute({}, "pattern"); ASSERT_THAT(attr, NotNull()); EXPECT_THAT(attr->value, Eq("\\\\d{5}")); @@ -67,21 +68,54 @@ TEST(XmlDomTest, ForwardEscapes) { ASSERT_THAT(el->children, SizeIs(1u)); - xml::Text* text = xml::NodeCast(el->children[0].get()); + Text* text = xml::NodeCast(el->children[0].get()); ASSERT_THAT(text, NotNull()); EXPECT_THAT(text->text, Eq("\\\\d{5}")); } TEST(XmlDomTest, XmlEscapeSequencesAreParsed) { - std::unique_ptr doc = test::BuildXmlDom(R"()"); - - xml::Element* el = xml::FindRootElement(doc.get()); - ASSERT_THAT(el, NotNull()); - - xml::Attribute* attr = el->FindAttribute({}, "value"); + std::unique_ptr doc = test::BuildXmlDom(R"()"); + Attribute* attr = doc->root->FindAttribute({}, "value"); ASSERT_THAT(attr, NotNull()); - EXPECT_THAT(attr->value, Eq("\"")); } +class TestVisitor : public PackageAwareVisitor { + public: + using PackageAwareVisitor::Visit; + + void Visit(Element* el) override { + if (el->name == "View1") { + EXPECT_THAT(TransformPackageAlias("one", "local"), + Eq(make_value(ExtractedPackage{"com.one", false}))); + } else if (el->name == "View2") { + EXPECT_THAT(TransformPackageAlias("one", "local"), + Eq(make_value(ExtractedPackage{"com.one", false}))); + EXPECT_THAT(TransformPackageAlias("two", "local"), + Eq(make_value(ExtractedPackage{"com.two", false}))); + } else if (el->name == "View3") { + EXPECT_THAT(TransformPackageAlias("one", "local"), + Eq(make_value(ExtractedPackage{"com.one", false}))); + EXPECT_THAT(TransformPackageAlias("two", "local"), + Eq(make_value(ExtractedPackage{"com.two", false}))); + EXPECT_THAT(TransformPackageAlias("three", "local"), + Eq(make_value(ExtractedPackage{"com.three", false}))); + } + } +}; + +TEST(XmlDomTest, PackageAwareXmlVisitor) { + std::unique_ptr doc = test::BuildXmlDom(R"( + + + + + )"); + + Debug::DumpXml(doc.get()); + TestVisitor visitor; + doc->root->Accept(&visitor); +} + +} // namespace xml } // namespace aapt diff --git a/tools/aapt2/xml/XmlUtil.h b/tools/aapt2/xml/XmlUtil.h index 1650ac2124ac4..866b6dcd7a880 100644 --- a/tools/aapt2/xml/XmlUtil.h +++ b/tools/aapt2/xml/XmlUtil.h @@ -26,79 +26,56 @@ namespace aapt { namespace xml { constexpr const char* kSchemaAuto = "http://schemas.android.com/apk/res-auto"; -constexpr const char* kSchemaPublicPrefix = - "http://schemas.android.com/apk/res/"; -constexpr const char* kSchemaPrivatePrefix = - "http://schemas.android.com/apk/prv/res/"; -constexpr const char* kSchemaAndroid = - "http://schemas.android.com/apk/res/android"; +constexpr const char* kSchemaPublicPrefix = "http://schemas.android.com/apk/res/"; +constexpr const char* kSchemaPrivatePrefix = "http://schemas.android.com/apk/prv/res/"; +constexpr const char* kSchemaAndroid = "http://schemas.android.com/apk/res/android"; constexpr const char* kSchemaTools = "http://schemas.android.com/tools"; constexpr const char* kSchemaAapt = "http://schemas.android.com/aapt"; -/** - * Result of extracting a package name from a namespace URI declaration. - */ +// Result of extracting a package name from a namespace URI declaration. struct ExtractedPackage { - /** - * The name of the package. This can be the empty string, which means that the - * package - * should be assumed to be the package being compiled. - */ + // The name of the package. This can be the empty string, which means that the package + // should be assumed to be the package being compiled. std::string package; - /** - * True if the package's private namespace was declared. This means that - * private resources - * are made visible. - */ + // True if the package's private namespace was declared. This means that private resources + // are made visible. bool private_namespace; + + friend inline bool operator==(const ExtractedPackage& a, const ExtractedPackage& b) { + return a.package == b.package && a.private_namespace == b.private_namespace; + } }; -/** - * Returns an ExtractedPackage struct if the namespace URI is of the form: - * http://schemas.android.com/apk/res/ or - * http://schemas.android.com/apk/prv/res/ - * - * Special case: if namespaceUri is http://schemas.android.com/apk/res-auto, - * returns an empty package name. - */ -Maybe ExtractPackageFromNamespace( - const std::string& namespace_uri); +// Returns an ExtractedPackage struct if the namespace URI is of the form: +// http://schemas.android.com/apk/res/ or +// http://schemas.android.com/apk/prv/res/ +// +// Special case: if namespaceUri is http://schemas.android.com/apk/res-auto, +// returns an empty package name. +Maybe ExtractPackageFromNamespace(const std::string& namespace_uri); -/** - * Returns an XML Android namespace for the given package of the form: - * - * http://schemas.android.com/apk/res/ - * - * If privateReference == true, the package will be of the form: - * - * http://schemas.android.com/apk/prv/res/ - */ +// Returns an XML Android namespace for the given package of the form: +// http://schemas.android.com/apk/res/ +// +// If privateReference == true, the package will be of the form: +// http://schemas.android.com/apk/prv/res/ std::string BuildPackageNamespace(const android::StringPiece& package, bool private_reference = false); -/** - * Interface representing a stack of XML namespace declarations. When looking up - * the package - * for a namespace prefix, the stack is checked from top to bottom. - */ +// Interface representing a stack of XML namespace declarations. When looking up the package +// for a namespace prefix, the stack is checked from top to bottom. struct IPackageDeclStack { virtual ~IPackageDeclStack() = default; - /** - * Returns an ExtractedPackage struct if the alias given corresponds with a - * package declaration. - */ + // Returns an ExtractedPackage struct if the alias given corresponds with a package declaration. virtual Maybe TransformPackageAlias( const android::StringPiece& alias, const android::StringPiece& local_package) const = 0; }; -/** - * Helper function for transforming the original Reference inRef to a fully - * qualified reference - * via the IPackageDeclStack. This will also mark the Reference as private if - * the namespace of the package declaration was private. - */ +// Helper function for transforming the original Reference inRef to a fully qualified reference +// via the IPackageDeclStack. This will also mark the Reference as private if the namespace of the +// package declaration was private. void TransformReferenceFromNamespace(IPackageDeclStack* decl_stack, const android::StringPiece& local_package, Reference* in_ref);