diff options
Diffstat (limited to 'tools')
59 files changed, 1515 insertions, 133 deletions
diff --git a/tools/aapt2/Android.bp b/tools/aapt2/Android.bp index 48cfc4453d8f..7a33de0262e2 100644 --- a/tools/aapt2/Android.bp +++ b/tools/aapt2/Android.bp @@ -111,6 +111,7 @@ cc_library_host_static { "link/XmlReferenceLinker.cpp", "optimize/MultiApkGenerator.cpp", "optimize/ResourceDeduper.cpp", + "optimize/ResourceFilter.cpp", "optimize/VersionCollapser.cpp", "process/SymbolTable.cpp", "split/TableSplitter.cpp", diff --git a/tools/aapt2/ResourceParser.cpp b/tools/aapt2/ResourceParser.cpp index 7f48544c0ae4..8719a233d774 100644 --- a/tools/aapt2/ResourceParser.cpp +++ b/tools/aapt2/ResourceParser.cpp @@ -208,6 +208,15 @@ class SegmentNode : public Node { } }; +// A chunk of text in the XML string within a CDATA tags. +class CdataSegmentNode : public SegmentNode { + public: + + void Build(StringBuilder* builder) const override { + builder->AppendText(data, /* preserve_spaces */ true); + } +}; + // A tag that will be encoded into the final flattened string. Tags like <b> or <i>. class SpanNode : public Node { public: @@ -244,6 +253,7 @@ bool ResourceParser::FlattenXmlSubtree( std::vector<Node*> node_stack; node_stack.push_back(&root); + bool cdata_block = false; bool saw_span_node = false; SegmentNode* first_segment = nullptr; SegmentNode* last_segment = nullptr; @@ -253,11 +263,15 @@ bool ResourceParser::FlattenXmlSubtree( const xml::XmlPullParser::Event event = parser->event(); // First take care of any SegmentNodes that should be created. - if (event == xml::XmlPullParser::Event::kStartElement || - event == xml::XmlPullParser::Event::kEndElement) { + if (event == xml::XmlPullParser::Event::kStartElement + || event == xml::XmlPullParser::Event::kEndElement + || event == xml::XmlPullParser::Event::kCdataStart + || event == xml::XmlPullParser::Event::kCdataEnd) { if (!current_text.empty()) { - std::unique_ptr<SegmentNode> segment_node = util::make_unique<SegmentNode>(); + std::unique_ptr<SegmentNode> segment_node = (cdata_block) + ? util::make_unique<CdataSegmentNode>() : util::make_unique<SegmentNode>(); segment_node->data = std::move(current_text); + last_segment = node_stack.back()->AddChild(std::move(segment_node)); if (first_segment == nullptr) { first_segment = last_segment; @@ -333,6 +347,16 @@ bool ResourceParser::FlattenXmlSubtree( } } break; + case xml::XmlPullParser::Event::kCdataStart: { + cdata_block = true; + break; + } + + case xml::XmlPullParser::Event::kCdataEnd: { + cdata_block = false; + break; + } + default: // ignore. break; @@ -447,6 +471,9 @@ bool ResourceParser::ParseResources(xml::XmlPullParser* parser) { parsed_resource.config = config_; parsed_resource.source = source_.WithLine(parser->line_number()); parsed_resource.comment = std::move(comment); + if (options_.visibility) { + parsed_resource.visibility_level = options_.visibility.value(); + } // Extract the product name if it exists. if (Maybe<StringPiece> maybe_product = xml::FindNonEmptyAttribute(parser, "product")) { @@ -774,7 +801,8 @@ std::unique_ptr<Item> ResourceParser::ParseXml(xml::XmlPullParser* parser, if (allow_raw_value) { // We can't parse this so return a RawString if we are allowed. return util::make_unique<RawString>( - table_->string_pool.MakeRef(raw_value, StringPool::Context(config_))); + table_->string_pool.MakeRef(util::TrimWhitespace(raw_value), + StringPool::Context(config_))); } return {}; } @@ -836,6 +864,12 @@ bool ResourceParser::ParseString(xml::XmlPullParser* parser, } bool ResourceParser::ParsePublic(xml::XmlPullParser* parser, ParsedResource* out_resource) { + if (options_.visibility) { + diag_->Error(DiagMessage(out_resource->source) + << "<public> tag not allowed with --visibility flag"); + return false; + } + if (out_resource->config != ConfigDescription::DefaultConfig()) { diag_->Warn(DiagMessage(out_resource->source) << "ignoring configuration '" << out_resource->config << "' for <public> tag"); @@ -878,6 +912,12 @@ bool ResourceParser::ParsePublic(xml::XmlPullParser* parser, ParsedResource* out } bool ResourceParser::ParsePublicGroup(xml::XmlPullParser* parser, ParsedResource* out_resource) { + if (options_.visibility) { + diag_->Error(DiagMessage(out_resource->source) + << "<public-group> tag not allowed with --visibility flag"); + return false; + } + if (out_resource->config != ConfigDescription::DefaultConfig()) { diag_->Warn(DiagMessage(out_resource->source) << "ignoring configuration '" << out_resource->config @@ -999,6 +1039,11 @@ bool ResourceParser::ParseSymbolImpl(xml::XmlPullParser* parser, } bool ResourceParser::ParseSymbol(xml::XmlPullParser* parser, ParsedResource* out_resource) { + if (options_.visibility) { + diag_->Error(DiagMessage(out_resource->source) + << "<java-symbol> and <symbol> tags not allowed with --visibility flag"); + return false; + } if (out_resource->config != ConfigDescription::DefaultConfig()) { diag_->Warn(DiagMessage(out_resource->source) << "ignoring configuration '" << out_resource->config << "' for <" @@ -1070,6 +1115,9 @@ bool ResourceParser::ParseOverlayable(xml::XmlPullParser* parser, ParsedResource child_resource.name.entry = maybe_name.value().to_string(); child_resource.source = item_source; child_resource.overlayable = true; + if (options_.visibility) { + child_resource.visibility_level = options_.visibility.value(); + } out_resource->child_resources.push_back(std::move(child_resource)); xml::XmlPullParser::SkipCurrentElement(parser); @@ -1212,6 +1260,9 @@ bool ResourceParser::ParseAttrImpl(xml::XmlPullParser* parser, child_resource.name = symbol.symbol.name.value(); child_resource.source = item_source; child_resource.value = util::make_unique<Id>(); + if (options_.visibility) { + child_resource.visibility_level = options_.visibility.value(); + } out_resource->child_resources.push_back(std::move(child_resource)); symbol.symbol.SetComment(std::move(comment)); @@ -1589,6 +1640,9 @@ bool ResourceParser::ParseDeclareStyleable(xml::XmlPullParser* parser, child_resource.name = child_ref.name.value(); child_resource.source = item_source; child_resource.comment = std::move(comment); + if (options_.visibility) { + child_resource.visibility_level = options_.visibility.value(); + } if (!ParseAttrImpl(parser, &child_resource, true)) { error = true; diff --git a/tools/aapt2/ResourceParser.h b/tools/aapt2/ResourceParser.h index fb9dbd0cd0fd..68130c2512d3 100644 --- a/tools/aapt2/ResourceParser.h +++ b/tools/aapt2/ResourceParser.h @@ -45,6 +45,10 @@ struct ResourceParserOptions { * warnings. */ bool error_on_positional_arguments = true; + + // If visibility was forced, we need to use it when creating a new resource and also error if we + // try to parse the <public>, <public-group>, <java-symbol> or <symbol> tags. + Maybe<Visibility::Level> visibility; }; /* diff --git a/tools/aapt2/ResourceParser_test.cpp b/tools/aapt2/ResourceParser_test.cpp index 41b4041efb7a..ee496d501113 100644 --- a/tools/aapt2/ResourceParser_test.cpp +++ b/tools/aapt2/ResourceParser_test.cpp @@ -497,6 +497,24 @@ TEST_F(ResourceParserTest, ParseStyleWithPackageAliasedItems) { EXPECT_THAT(style->entries[0].key.name, Eq(make_value(test::ParseNameOrDie("android:attr/bar")))); } +TEST_F(ResourceParserTest, ParseStyleWithRawStringItem) { + std::string input = R"( + <style name="foo"> + <item name="bar"> + com.helloworld.AppClass + </item> + </style>)"; + ASSERT_TRUE(TestParse(input)); + + Style* style = test::GetValue<Style>(&table_, "style/foo"); + ASSERT_THAT(style, NotNull()); + EXPECT_THAT(style->entries[0].value, NotNull()); + RawString* value = ValueCast<RawString>(style->entries[0].value.get()); + EXPECT_THAT(value, NotNull()); + EXPECT_THAT(*value->value, StrEq(R"(com.helloworld.AppClass)")); +} + + TEST_F(ResourceParserTest, ParseStyleWithInferredParent) { ASSERT_TRUE(TestParse(R"(<style name="foo.bar"/>)")); @@ -971,4 +989,40 @@ TEST_F(ResourceParserTest, ParseIdItem) { ASSERT_FALSE(TestParse(input)); } +TEST_F(ResourceParserTest, ParseCData) { + std::string input = R"( + <string name="foo"><![CDATA[some text and ' apostrophe]]></string>)"; + + ASSERT_TRUE(TestParse(input)); + String* output = test::GetValue<String>(&table_, "string/foo"); + ASSERT_THAT(output, NotNull()); + EXPECT_THAT(*output, StrValueEq("some text and ' apostrophe")); + + // Double quotes should not change the state of whitespace processing + input = R"(<string name="foo2">Hello<![CDATA[ "</string>' ]]> World</string>)"; + ASSERT_TRUE(TestParse(input)); + output = test::GetValue<String>(&table_, "string/foo2"); + ASSERT_THAT(output, NotNull()); + EXPECT_THAT(*output, StrValueEq(std::string("Hello \"</string>' World").data())); + + // Cdata blocks should not have their whitespace trimmed + input = R"(<string name="foo3"> <![CDATA[ text ]]> </string>)"; + ASSERT_TRUE(TestParse(input)); + output = test::GetValue<String>(&table_, "string/foo3"); + ASSERT_THAT(output, NotNull()); + EXPECT_THAT(*output, StrValueEq(std::string(" text ").data())); + + input = R"(<string name="foo4"> <![CDATA[]]> </string>)"; + ASSERT_TRUE(TestParse(input)); + output = test::GetValue<String>(&table_, "string/foo4"); + ASSERT_THAT(output, NotNull()); + EXPECT_THAT(*output, StrValueEq(std::string("").data())); + + input = R"(<string name="foo5"> <![CDATA[ ]]> </string>)"; + ASSERT_TRUE(TestParse(input)); + output = test::GetValue<String>(&table_, "string/foo5"); + ASSERT_THAT(output, NotNull()); + EXPECT_THAT(*output, StrValueEq(std::string(" ").data())); +} + } // namespace aapt diff --git a/tools/aapt2/ResourceUtils.cpp b/tools/aapt2/ResourceUtils.cpp index 560077cc322c..c48765b7b947 100644 --- a/tools/aapt2/ResourceUtils.cpp +++ b/tools/aapt2/ResourceUtils.cpp @@ -797,16 +797,20 @@ StringBuilder::StringBuilder(bool preserve_spaces) : preserve_spaces_(preserve_spaces), quote_(preserve_spaces) { } -StringBuilder& StringBuilder::AppendText(const std::string& text) { +StringBuilder& StringBuilder::AppendText(const std::string& text, bool preserve_spaces) { if (!error_.empty()) { return *this; } + // Enable preserving spaces if it is enabled for this append or the StringBuilder was constructed + // to preserve spaces + preserve_spaces = (preserve_spaces) ? preserve_spaces : preserve_spaces_; + const size_t previous_len = xml_string_.text.size(); Utf8Iterator iter(text); while (iter.HasNext()) { char32_t codepoint = iter.Next(); - if (!quote_ && iswspace(codepoint)) { + if (!preserve_spaces && !quote_ && iswspace(codepoint)) { if (!last_codepoint_was_space_) { // Emit a space if it's the first. xml_string_.text += ' '; @@ -827,7 +831,6 @@ StringBuilder& StringBuilder::AppendText(const std::string& text) { case U't': xml_string_.text += '\t'; break; - case U'n': xml_string_.text += '\n'; break; @@ -855,12 +858,12 @@ StringBuilder& StringBuilder::AppendText(const std::string& text) { break; } } - } else if (!preserve_spaces_ && codepoint == U'"') { + } else if (!preserve_spaces && codepoint == U'"') { // Only toggle the quote state when we are not preserving spaces. quote_ = !quote_; - } else if (!quote_ && codepoint == U'\'') { - // This should be escaped. + } else if (!preserve_spaces && !quote_ && codepoint == U'\'') { + // This should be escaped when we are not preserving spaces error_ = StringPrintf("unescaped apostrophe in string\n\"%s\"", text.c_str()); return *this; diff --git a/tools/aapt2/ResourceUtils.h b/tools/aapt2/ResourceUtils.h index 7af2fe06b908..410ef28ce78a 100644 --- a/tools/aapt2/ResourceUtils.h +++ b/tools/aapt2/ResourceUtils.h @@ -267,8 +267,10 @@ class StringBuilder { // single quotations can be used without escaping them. explicit StringBuilder(bool preserve_spaces = false); - // Appends a chunk of text. - StringBuilder& AppendText(const std::string& text); + // Appends a chunk of text. If preserve_spaces is true, whitespace removal is not performed, and + // single quotations can be used without escaping them for this append. Otherwise, the + // StringBuilder will behave as it was constructed. + StringBuilder& AppendText(const std::string& text, bool preserve_spaces = false); // Starts a Span (tag) with the given name. The name is expected to be of the form: // "tag_name;attr1=value;attr2=value;" diff --git a/tools/aapt2/ResourceUtils_test.cpp b/tools/aapt2/ResourceUtils_test.cpp index 11f3fa3bc6cd..5ce464074335 100644 --- a/tools/aapt2/ResourceUtils_test.cpp +++ b/tools/aapt2/ResourceUtils_test.cpp @@ -254,6 +254,29 @@ TEST(ResourceUtilsTest, StringBuilderUnicodeCodes) { TEST(ResourceUtilsTest, StringBuilderPreserveSpaces) { EXPECT_THAT(ResourceUtils::StringBuilder(true /*preserve_spaces*/).AppendText("\"").to_string(), Eq("\"")); + + // Single quotes should be able to be used without escaping them when preserving spaces and the + // spaces should not be trimmed + EXPECT_THAT(ResourceUtils::StringBuilder() + .AppendText(" hey guys ") + .AppendText(" 'this is so cool' ", /* preserve_spaces */ true) + .AppendText(" wow ") + .to_string(), + Eq(" hey guys 'this is so cool' wow ")); + + // Reading a double quote while preserving spaces should not change the quote state + EXPECT_THAT(ResourceUtils::StringBuilder() + .AppendText(" hey guys ") + .AppendText(" \"this is so cool' ", /* preserve_spaces */ true) + .AppendText(" wow ") + .to_string(), + Eq(" hey guys \"this is so cool' wow ")); + EXPECT_THAT(ResourceUtils::StringBuilder() + .AppendText(" hey guys\" ") + .AppendText(" \"this is so cool' ", /* preserve_spaces */ true) + .AppendText(" wow \" ") + .to_string(), + Eq(" hey guys \"this is so cool' wow ")); } } // namespace aapt diff --git a/tools/aapt2/StringPool.cpp b/tools/aapt2/StringPool.cpp index b37e1fbd9693..8eabd3225d87 100644 --- a/tools/aapt2/StringPool.cpp +++ b/tools/aapt2/StringPool.cpp @@ -367,7 +367,7 @@ const std::string kStringTooLarge = "STRING_TOO_LARGE"; static bool EncodeString(const std::string& str, const bool utf8, BigBuffer* out, IDiagnostics* diag) { if (utf8) { - const std::string& encoded = str; + const std::string& encoded = util::Utf8ToModifiedUtf8(str); const ssize_t utf16_length = utf8_to_utf16_length( reinterpret_cast<const uint8_t*>(encoded.data()), encoded.size()); CHECK(utf16_length >= 0); diff --git a/tools/aapt2/StringPool_test.cpp b/tools/aapt2/StringPool_test.cpp index 4b3afe2bb962..0778564ee079 100644 --- a/tools/aapt2/StringPool_test.cpp +++ b/tools/aapt2/StringPool_test.cpp @@ -303,6 +303,25 @@ TEST(StringPoolTest, Flatten) { } } +TEST(StringPoolTest, FlattenModifiedUTF8) { + using namespace android; // For NO_ERROR on Windows. + StdErrDiagnostics diag; + StringPool pool; + StringPool::Ref ref_a = pool.MakeRef("\xF0\x90\x90\x80"); // 𐐀 (U+10400) + StringPool::Ref ref_b = pool.MakeRef("foo \xF0\x90\x90\xB7 bar"); // 𐐷 (U+10437) + StringPool::Ref ref_c = pool.MakeRef("\xF0\x90\x90\x80\xF0\x90\x90\xB7"); + + BigBuffer buffer(1024); + StringPool::FlattenUtf8(&buffer, pool, &diag); + std::unique_ptr<uint8_t[]> data = util::Copy(buffer); + + // Check that the 4 byte utf-8 codepoint is encoded using 2 3 byte surrogate pairs + ResStringPool test; + ASSERT_EQ(test.setTo(data.get(), buffer.size()), NO_ERROR); + EXPECT_THAT(util::GetString(test, 0), Eq("\xED\xA0\x81\xED\xB0\x80")); + EXPECT_THAT(util::GetString(test, 1), Eq("foo \xED\xA0\x81\xED\xB0\xB7 bar")); + EXPECT_THAT(util::GetString(test, 2), Eq("\xED\xA0\x81\xED\xB0\x80\xED\xA0\x81\xED\xB0\xB7")); +} TEST(StringPoolTest, MaxEncodingLength) { StdErrDiagnostics diag; diff --git a/tools/aapt2/cmd/Compile.cpp b/tools/aapt2/cmd/Compile.cpp index ab8a4b77a89d..a17a0d3e7299 100644 --- a/tools/aapt2/cmd/Compile.cpp +++ b/tools/aapt2/cmd/Compile.cpp @@ -30,6 +30,7 @@ #include "Flags.h" #include "ResourceParser.h" #include "ResourceTable.h" +#include "cmd/Util.h" #include "compile/IdAssigner.h" #include "compile/InlineXmlFormatParser.h" #include "compile/Png.h" @@ -124,6 +125,7 @@ struct CompileOptions { std::string output_path; Maybe<std::string> res_dir; Maybe<std::string> generate_text_symbols_path; + Maybe<Visibility::Level> visibility; bool pseudolocalize = false; bool no_png_crunch = false; bool legacy_mode = false; @@ -226,6 +228,10 @@ static bool CompileTable(IAaptContext* context, const CompileOptions& options, // If the filename includes donottranslate, then the default translatable is false. parser_options.translatable = path_data.name.find("donottranslate") == std::string::npos; + // If visibility was forced, we need to use it when creating a new resource and also error if + // we try to parse the <public>, <public-group>, <java-symbol> or <symbol> tags. + parser_options.visibility = options.visibility; + ResourceParser res_parser(context->GetDiagnostics(), &table, path_data.source, path_data.config, parser_options); if (!res_parser.Parse(&xml_parser)) { @@ -319,8 +325,16 @@ static bool CompileTable(IAaptContext* context, const CompileOptions& options, if (!entry->values.empty()) { auto styleable = static_cast<const Styleable*>(entry->values.front()->value.get()); for (const auto& attr : styleable->entries) { + // The visibility of the children under the styleable does not matter as they are + // nested under their parent and use its visibility. r_txt_printer.Print("default int styleable "); r_txt_printer.Print(entry->name); + // If the package name is present, also include it in the mangled name (e.g. + // "android") + if (!attr.name.value().package.empty()) { + r_txt_printer.Print("_"); + r_txt_printer.Print(MakePackageSafeName(attr.name.value().package)); + } r_txt_printer.Print("_"); r_txt_printer.Println(attr.name.value().entry); } @@ -687,6 +701,10 @@ class CompileContext : public IAaptContext { return 0; } + bool IsAutoNamespace() override { + return false; + } + private: DISALLOW_COPY_AND_ASSIGN(CompileContext); @@ -700,6 +718,7 @@ int Compile(const std::vector<StringPiece>& args, IDiagnostics* diagnostics) { CompileOptions options; bool verbose = false; + Maybe<std::string> visibility; Flags flags = Flags() .RequiredFlag("-o", "Output path", &options.output_path) @@ -715,13 +734,32 @@ int Compile(const std::vector<StringPiece>& args, IDiagnostics* diagnostics) { .OptionalSwitch("--no-crunch", "Disables PNG processing", &options.no_png_crunch) .OptionalSwitch("--legacy", "Treat errors that used to be valid in AAPT as warnings", &options.legacy_mode) - .OptionalSwitch("-v", "Enables verbose logging", &verbose); + .OptionalSwitch("-v", "Enables verbose logging", &verbose) + .OptionalFlag("--visibility", + "Sets the visibility of the compiled resources to the specified\n" + "level. Accepted levels: public, private, default", + &visibility); if (!flags.Parse("aapt2 compile", args, &std::cerr)) { return 1; } context.SetVerbose(verbose); + if (visibility) { + if (visibility.value() == "public") { + options.visibility = Visibility::Level::kPublic; + } else if (visibility.value() == "private") { + options.visibility = Visibility::Level::kPrivate; + } else if (visibility.value() == "default") { + options.visibility = Visibility::Level::kUndefined; + } else { + context.GetDiagnostics()->Error( + DiagMessage() << "Unrecognized visibility level passes to --visibility: '" + << visibility.value() << "'. Accepted levels: public, private, default"); + return 1; + } + } + std::unique_ptr<IArchiveWriter> archive_writer; std::vector<ResourcePathData> input_data; diff --git a/tools/aapt2/cmd/Convert.cpp b/tools/aapt2/cmd/Convert.cpp index eb307fb1ddce..3c8beaa19008 100644 --- a/tools/aapt2/cmd/Convert.cpp +++ b/tools/aapt2/cmd/Convert.cpp @@ -310,6 +310,10 @@ class Context : public IAaptContext { return 0u; } + bool IsAutoNamespace() override { + return false; + } + bool verbose_ = false; std::string package_; diff --git a/tools/aapt2/cmd/Diff.cpp b/tools/aapt2/cmd/Diff.cpp index 12113ed8a48a..16c7bba33e10 100644 --- a/tools/aapt2/cmd/Diff.cpp +++ b/tools/aapt2/cmd/Diff.cpp @@ -64,6 +64,10 @@ class DiffContext : public IAaptContext { return 0; } + bool IsAutoNamespace() override { + return false; + } + private: std::string empty_; StdErrDiagnostics diagnostics_; diff --git a/tools/aapt2/cmd/Dump.cpp b/tools/aapt2/cmd/Dump.cpp index 8e7e5e59bc31..fd133f3da175 100644 --- a/tools/aapt2/cmd/Dump.cpp +++ b/tools/aapt2/cmd/Dump.cpp @@ -298,6 +298,10 @@ class DumpContext : public IAaptContext { return 0; } + bool IsAutoNamespace() override { + return false; + } + private: StdErrDiagnostics diagnostics_; bool verbose_ = false; diff --git a/tools/aapt2/cmd/Link.cpp b/tools/aapt2/cmd/Link.cpp index db42e7cb3e02..1a2da7fbd000 100644 --- a/tools/aapt2/cmd/Link.cpp +++ b/tools/aapt2/cmd/Link.cpp @@ -17,6 +17,7 @@ #include <sys/stat.h> #include <cinttypes> +#include <algorithm> #include <queue> #include <unordered_map> #include <vector> @@ -111,6 +112,7 @@ struct LinkOptions { // Static lib options. bool no_static_lib_packages = false; + bool auto_namespace_static_lib = false; // AndroidManifest.xml massaging options. ManifestFixerOptions manifest_fixer_options; @@ -135,6 +137,9 @@ struct LinkOptions { // In order to work around this limitation, we allow the use of traditionally reserved // resource IDs [those between 0x02 and 0x7E]. bool allow_reserved_package_id = false; + + // Whether we should fail on definitions of a resource with conflicting visibility. + bool strict_visibility = false; }; class LinkContext : public IAaptContext { @@ -199,6 +204,14 @@ class LinkContext : public IAaptContext { min_sdk_version_ = minSdk; } + bool IsAutoNamespace() override { + return auto_namespace_; + } + + void SetAutoNamespace(bool val) { + auto_namespace_ = val; + } + private: DISALLOW_COPY_AND_ASSIGN(LinkContext); @@ -210,6 +223,7 @@ class LinkContext : public IAaptContext { SymbolTable symbols_; bool verbose_ = false; int min_sdk_version_ = 0; + bool auto_namespace_ = false; }; // A custom delegate that generates compatible pre-O IDs for use with feature splits. @@ -486,7 +500,7 @@ std::vector<std::unique_ptr<xml::XmlResource>> ResourceFileFlattener::LinkAndVer return {}; } - if (options_.update_proguard_spec && !proguard::CollectProguardRules(doc, keep_set_)) { + if (options_.update_proguard_spec && !proguard::CollectProguardRules(context_, doc, keep_set_)) { return {}; } @@ -1702,6 +1716,7 @@ class LinkCommand { TableMergerOptions table_merger_options; table_merger_options.auto_add_overlay = options_.auto_add_overlay; + table_merger_options.strict_visibility = options_.strict_visibility; table_merger_ = util::make_unique<TableMerger>(context_, &final_table_, table_merger_options); if (context_->IsVerbose()) { @@ -2143,6 +2158,10 @@ int Link(const std::vector<StringPiece>& args, IDiagnostics* diagnostics) { .OptionalSwitch("--no-static-lib-packages", "Merge all library resources under the app's package.", &options.no_static_lib_packages) + .OptionalSwitch("--auto-namespace-static-lib", + "Automatically namespace resource references when building a static\n" + "library.", + &options.auto_namespace_static_lib) .OptionalSwitch("--non-final-ids", "Generates R.java without the final modifier. This is implied when\n" "--static-lib is specified.", @@ -2186,6 +2205,8 @@ int Link(const std::vector<StringPiece>& args, IDiagnostics* diagnostics) { &options.manifest_fixer_options.rename_instrumentation_target_package) .OptionalFlagList("-0", "File extensions not to compress.", &options.extensions_to_not_compress) + .OptionalSwitch("--no-compress", "Do not compress any resources.", + &options.do_not_compress_anything) .OptionalSwitch("--warn-manifest-validation", "Treat manifest validation errors as warnings.", &options.manifest_fixer_options.warn_validation) @@ -2198,7 +2219,10 @@ int Link(const std::vector<StringPiece>& args, IDiagnostics* diagnostics) { .OptionalSwitch("--debug-mode", "Inserts android:debuggable=\"true\" in to the application node of the\n" "manifest, making the application debuggable even on production devices.", - &options.manifest_fixer_options.debug_mode); + &options.manifest_fixer_options.debug_mode) + .OptionalSwitch("--strict-visibility", + "Do not allow overlays with different visibility levels.", + &options.strict_visibility); if (!flags.Parse("aapt2 link", args, &std::cerr)) { return 1; @@ -2258,6 +2282,15 @@ int Link(const std::vector<StringPiece>& args, IDiagnostics* diagnostics) { options.output_format = OutputFormat::kProto; } + if (options.auto_namespace_static_lib) { + if (!static_lib) { + context.GetDiagnostics()->Error( + DiagMessage() << "--auto-namespace-static-lib can only be used with --static-lib"); + return 1; + } + context.SetAutoNamespace(true); + } + if (package_id) { if (context.GetPackageType() != PackageType::kApp) { context.GetDiagnostics()->Error( diff --git a/tools/aapt2/cmd/Optimize.cpp b/tools/aapt2/cmd/Optimize.cpp index 9c76119f9504..4afa8f0a907d 100644 --- a/tools/aapt2/cmd/Optimize.cpp +++ b/tools/aapt2/cmd/Optimize.cpp @@ -38,6 +38,7 @@ #include "io/Util.h" #include "optimize/MultiApkGenerator.h" #include "optimize/ResourceDeduper.h" +#include "optimize/ResourceFilter.h" #include "optimize/VersionCollapser.h" #include "split/TableSplitter.h" #include "util/Files.h" @@ -62,6 +63,9 @@ struct OptimizeOptions { // Details of the app extracted from the AndroidManifest.xml AppInfo app_info; + // Blacklist of unused resources that should be removed from the apk. + std::unordered_set<ResourceName> resources_blacklist; + // Split APK options. TableSplitterOptions table_splitter_options; @@ -129,6 +133,10 @@ class OptimizeContext : public IAaptContext { return sdk_version_; } + bool IsAutoNamespace() override { + return false; + } + private: DISALLOW_COPY_AND_ASSIGN(OptimizeContext); @@ -147,6 +155,13 @@ class OptimizeCommand { if (context_->IsVerbose()) { context_->GetDiagnostics()->Note(DiagMessage() << "Optimizing APK..."); } + if (!options_.resources_blacklist.empty()) { + ResourceFilter filter(options_.resources_blacklist); + if (!filter.Consume(context_, apk->GetResourceTable())) { + context_->GetDiagnostics()->Error(DiagMessage() << "failed filtering resources"); + return 1; + } + } VersionCollapser collapser; if (!collapser.Consume(context_, apk->GetResourceTable())) { @@ -284,16 +299,62 @@ class OptimizeCommand { OptimizeContext* context_; }; -bool ExtractWhitelistFromConfig(const std::string& path, OptimizeContext* context, - OptimizeOptions* options) { +bool ExtractObfuscationWhitelistFromConfig(const std::string& path, OptimizeContext* context, + OptimizeOptions* options) { std::string contents; if (!ReadFileToString(path, &contents, true)) { context->GetDiagnostics()->Error(DiagMessage() << "failed to parse whitelist from config file: " << path); return false; } - for (const StringPiece& resource_name : util::Tokenize(contents, ',')) { - options->table_flattener_options.whitelisted_resources.insert(resource_name.to_string()); + for (StringPiece resource_name : util::Tokenize(contents, ',')) { + options->table_flattener_options.whitelisted_resources.insert( + resource_name.to_string()); + } + return true; +} + +bool ExtractConfig(const std::string& path, OptimizeContext* context, + OptimizeOptions* options) { + std::string content; + if (!android::base::ReadFileToString(path, &content, true /*follow_symlinks*/)) { + context->GetDiagnostics()->Error(DiagMessage(path) << "failed reading whitelist"); + return false; + } + + size_t line_no = 0; + for (StringPiece line : util::Tokenize(content, '\n')) { + line_no++; + line = util::TrimWhitespace(line); + if (line.empty()) { + continue; + } + + auto split_line = util::Split(line, '#'); + if (split_line.size() < 2) { + context->GetDiagnostics()->Error(DiagMessage(line) << "No # found in line"); + return false; + } + StringPiece resource_string = split_line[0]; + StringPiece directives = split_line[1]; + ResourceNameRef resource_name; + if (!ResourceUtils::ParseResourceName(resource_string, &resource_name)) { + context->GetDiagnostics()->Error(DiagMessage(line) << "Malformed resource name"); + return false; + } + if (!resource_name.package.empty()) { + context->GetDiagnostics()->Error(DiagMessage(line) + << "Package set for resource. Only use type/name"); + return false; + } + for (StringPiece directive : util::Tokenize(directives, ',')) { + if (directive == "remove") { + options->resources_blacklist.insert(resource_name.ToResourceName()); + } else if (directive == "no_obfuscate") { + options->table_flattener_options.whitelisted_resources.insert( + resource_name.entry.to_string()); + } + } } return true; } @@ -322,6 +383,7 @@ int Optimize(const std::vector<StringPiece>& args) { OptimizeOptions options; Maybe<std::string> config_path; Maybe<std::string> whitelist_path; + Maybe<std::string> resources_config_path; Maybe<std::string> target_densities; std::vector<std::string> configs; std::vector<std::string> split_args; @@ -340,10 +402,15 @@ int Optimize(const std::vector<StringPiece>& args) { "All the resources that would be unused on devices of the given densities will be \n" "removed from the APK.", &target_densities) - .OptionalFlag("--whitelist-config-path", + .OptionalFlag("--whitelist-path", "Path to the whitelist.cfg file containing whitelisted resources \n" "whose names should not be altered in final resource tables.", &whitelist_path) + .OptionalFlag("--resources-config-path", + "Path to the resources.cfg file containing the list of resources and \n" + "directives to each resource. \n" + "Format: type/resource_name#[directive][,directive]", + &resources_config_path) .OptionalFlagList("-c", "Comma separated list of configurations to include. The default\n" "is all configurations.", @@ -460,12 +527,19 @@ int Optimize(const std::vector<StringPiece>& args) { if (options.table_flattener_options.collapse_key_stringpool) { if (whitelist_path) { std::string& path = whitelist_path.value(); - if (!ExtractWhitelistFromConfig(path, &context, &options)) { + if (!ExtractObfuscationWhitelistFromConfig(path, &context, &options)) { return 1; } } } + if (resources_config_path) { + std::string& path = resources_config_path.value(); + if (!ExtractConfig(path, &context, &options)) { + return 1; + } + } + if (!ExtractAppDataFromManifest(&context, apk.get(), &options)) { return 1; } diff --git a/tools/aapt2/cmd/Util.cpp b/tools/aapt2/cmd/Util.cpp index 8b3a6701b409..4e77e9ae9a45 100644 --- a/tools/aapt2/cmd/Util.cpp +++ b/tools/aapt2/cmd/Util.cpp @@ -145,7 +145,7 @@ static xml::NamespaceDecl CreateAndroidNamespaceDecl() { // // See frameworks/base/core/java/android/content/pm/PackageParser.java which // checks this at runtime. -static std::string MakePackageSafeName(const std::string &name) { +std::string MakePackageSafeName(const std::string &name) { std::string result(name); bool first = true; for (char &c : result) { diff --git a/tools/aapt2/cmd/Util.h b/tools/aapt2/cmd/Util.h index 7611c1526104..fb8753ed2286 100644 --- a/tools/aapt2/cmd/Util.h +++ b/tools/aapt2/cmd/Util.h @@ -60,6 +60,13 @@ std::unique_ptr<xml::XmlResource> GenerateSplitManifest(const AppInfo& app_info, Maybe<AppInfo> ExtractAppInfoFromBinaryManifest(const xml::XmlResource& xml_res, IDiagnostics* diag); +// Returns a copy of 'name' which conforms to the regex '[a-zA-Z]+[a-zA-Z0-9_]*' by +// replacing nonconforming characters with underscores. +// +// See frameworks/base/core/java/android/content/pm/PackageParser.java which +// checks this at runtime. +std::string MakePackageSafeName(const std::string &name); + } // namespace aapt #endif /* AAPT_SPLIT_UTIL_H */ diff --git a/tools/aapt2/integration-tests/AutoNamespaceTest/Android.mk b/tools/aapt2/integration-tests/AutoNamespaceTest/Android.mk new file mode 100644 index 000000000000..5d7a6f7bedab --- /dev/null +++ b/tools/aapt2/integration-tests/AutoNamespaceTest/Android.mk @@ -0,0 +1,18 @@ +# +# Copyright (C) 2018 The Android Open Source Project +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +LOCAL_PATH := $(call my-dir) +include $(call all-makefiles-under,$(LOCAL_PATH)) diff --git a/tools/aapt2/integration-tests/AutoNamespaceTest/LibOne/Android.mk b/tools/aapt2/integration-tests/AutoNamespaceTest/LibOne/Android.mk new file mode 100644 index 000000000000..91716b9870ea --- /dev/null +++ b/tools/aapt2/integration-tests/AutoNamespaceTest/LibOne/Android.mk @@ -0,0 +1,29 @@ +# +# Copyright (C) 2018 The Android Open Source Project +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +LOCAL_PATH := $(call my-dir) + +include $(CLEAR_VARS) +LOCAL_USE_AAPT2 := true +LOCAL_MODULE := AaptTestAutoNamespace_LibOne +LOCAL_MODULE_TAGS := tests +LOCAL_SRC_FILES := $(call all-java-files-under,src) +LOCAL_RESOURCE_DIR := $(LOCAL_PATH)/res +LOCAL_AAPT_NAMESPACES := true +LOCAL_AAPT_FLAGS := --auto-namespace-static-lib +# We need this to compile the Java sources of AaptTestStaticLib_LibTwo using javac. +LOCAL_JAR_EXCLUDE_FILES := none +include $(BUILD_STATIC_JAVA_LIBRARY) diff --git a/tools/aapt2/integration-tests/AutoNamespaceTest/LibOne/AndroidManifest.xml b/tools/aapt2/integration-tests/AutoNamespaceTest/LibOne/AndroidManifest.xml new file mode 100644 index 000000000000..f585840cdb12 --- /dev/null +++ b/tools/aapt2/integration-tests/AutoNamespaceTest/LibOne/AndroidManifest.xml @@ -0,0 +1,17 @@ +<?xml version="1.0" encoding="utf-8"?> +<!-- Copyright (C) 2018 The Android Open Source Project + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +--> + +<manifest package="com.example.android.aapt2.autonamespace.staticlib.one" /> diff --git a/tools/aapt2/integration-tests/AutoNamespaceTest/LibOne/res/values/values.xml b/tools/aapt2/integration-tests/AutoNamespaceTest/LibOne/res/values/values.xml new file mode 100644 index 000000000000..3e57b0f20739 --- /dev/null +++ b/tools/aapt2/integration-tests/AutoNamespaceTest/LibOne/res/values/values.xml @@ -0,0 +1,27 @@ +<?xml version="1.0" encoding="utf-8"?> +<!-- Copyright (C) 2018 The Android Open Source Project + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +--> + +<resources> + <!-- An attribute from StaticLibOne --> + <attr name="StaticLibOne_attr" format="string" /> + + <string name="Foo">Foo</string> + + <declare-styleable name="Widget"> + <attr name="StaticLibOne_attr" /> + <attr name="android:text" /> + </declare-styleable> +</resources> diff --git a/tools/aapt2/integration-tests/AutoNamespaceTest/LibOne/src/com/example/android/aapt2/autonamespace/staticlib/one/StaticLibOne.java b/tools/aapt2/integration-tests/AutoNamespaceTest/LibOne/src/com/example/android/aapt2/autonamespace/staticlib/one/StaticLibOne.java new file mode 100644 index 000000000000..886d48c7597c --- /dev/null +++ b/tools/aapt2/integration-tests/AutoNamespaceTest/LibOne/src/com/example/android/aapt2/autonamespace/staticlib/one/StaticLibOne.java @@ -0,0 +1,18 @@ +/* + * Copyright (C) 2018 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.example.android.aapt2.autonamespace.staticlib.one; + +public class StaticLibOne { public static int FooId = R.string.Foo; } diff --git a/tools/aapt2/integration-tests/AutoNamespaceTest/LibTwo/Android.mk b/tools/aapt2/integration-tests/AutoNamespaceTest/LibTwo/Android.mk new file mode 100644 index 000000000000..c85496d38fda --- /dev/null +++ b/tools/aapt2/integration-tests/AutoNamespaceTest/LibTwo/Android.mk @@ -0,0 +1,29 @@ +# +# Copyright (C) 2018 The Android Open Source Project +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +LOCAL_PATH := $(call my-dir) + +include $(CLEAR_VARS) +LOCAL_USE_AAPT2 := true +LOCAL_MODULE := AaptTestAutoNamespace_LibTwo +LOCAL_MODULE_TAGS := tests +LOCAL_SRC_FILES := $(call all-java-files-under,src) +LOCAL_RESOURCE_DIR := $(LOCAL_PATH)/res +LOCAL_SHARED_ANDROID_LIBRARIES := AaptTestAutoNamespace_LibOne +LOCAL_AAPT_NAMESPACES := true +LOCAL_AAPT_FLAGS := --auto-namespace-static-lib +include $(BUILD_STATIC_JAVA_LIBRARY) + diff --git a/tools/aapt2/integration-tests/AutoNamespaceTest/LibTwo/AndroidManifest.xml b/tools/aapt2/integration-tests/AutoNamespaceTest/LibTwo/AndroidManifest.xml new file mode 100644 index 000000000000..8d3c506c5ac3 --- /dev/null +++ b/tools/aapt2/integration-tests/AutoNamespaceTest/LibTwo/AndroidManifest.xml @@ -0,0 +1,17 @@ +<?xml version="1.0" encoding="utf-8"?> +<!-- Copyright (C) 2018 The Android Open Source Project + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +--> + +<manifest package="com.example.android.aapt2.autonamespace.staticlib.two" /> diff --git a/tools/aapt2/integration-tests/AutoNamespaceTest/LibTwo/res/layout/layout_two.xml b/tools/aapt2/integration-tests/AutoNamespaceTest/LibTwo/res/layout/layout_two.xml new file mode 100644 index 000000000000..fb202201a03a --- /dev/null +++ b/tools/aapt2/integration-tests/AutoNamespaceTest/LibTwo/res/layout/layout_two.xml @@ -0,0 +1,18 @@ +<?xml version="1.0" encoding="utf-8"?> +<!-- Copyright (C) 2018 The Android Open Source Project + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +--> + +<View xmlns:custom="http://schemas.android.com/apk/res-auto" + custom:StaticLibOne_attr="@string/FooBar" /> diff --git a/tools/aapt2/integration-tests/AutoNamespaceTest/LibTwo/res/values/values.xml b/tools/aapt2/integration-tests/AutoNamespaceTest/LibTwo/res/values/values.xml new file mode 100644 index 000000000000..c532387ee961 --- /dev/null +++ b/tools/aapt2/integration-tests/AutoNamespaceTest/LibTwo/res/values/values.xml @@ -0,0 +1,19 @@ +<?xml version="1.0" encoding="utf-8"?> +<!-- Copyright (C) 2018 The Android Open Source Project + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +--> + +<resources> + <string name="FooBar">@string/Foo</string> +</resources> diff --git a/tools/aapt2/integration-tests/AutoNamespaceTest/LibTwo/src/com/example/android/aapt2/autonamespace/staticlib/two/StaticLibTwo.java b/tools/aapt2/integration-tests/AutoNamespaceTest/LibTwo/src/com/example/android/aapt2/autonamespace/staticlib/two/StaticLibTwo.java new file mode 100644 index 000000000000..323f53ae907b --- /dev/null +++ b/tools/aapt2/integration-tests/AutoNamespaceTest/LibTwo/src/com/example/android/aapt2/autonamespace/staticlib/two/StaticLibTwo.java @@ -0,0 +1,25 @@ +/* + * Copyright (C) 2018 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.example.android.aapt2.autonamespace.staticlib.two; + +public class StaticLibTwo { + // IDs from StaticLibOne + public static int FooId = com.example.android.aapt2.autonamespace.staticlib.one.R.string.Foo; + + // IDs from StaticLibTwo + public static int FooBarId = R.string.FooBar; + public static int LayoutId = R.layout.layout_two; +} diff --git a/tools/aapt2/java/JavaClassGenerator.cpp b/tools/aapt2/java/JavaClassGenerator.cpp index 6b07b1e96261..db1561e17f16 100644 --- a/tools/aapt2/java/JavaClassGenerator.cpp +++ b/tools/aapt2/java/JavaClassGenerator.cpp @@ -347,7 +347,9 @@ void JavaClassGenerator::ProcessStyleable(const ResourceNameRef& name, const Res } // Add the Styleable array to the Styleable class. - out_class_def->AddMember(std::move(array_def)); + if (out_class_def != nullptr) { + out_class_def->AddMember(std::move(array_def)); + } // Now we emit the indices into the array. for (size_t i = 0; i < attr_count; i++) { @@ -578,7 +580,6 @@ bool JavaClassGenerator::Generate(const StringPiece& package_name_to_generate, if (out_r_txt != nullptr) { r_txt_printer = util::make_unique<Printer>(out_r_txt); } - // Generate an onResourcesLoaded() callback if requested. if (out != nullptr && options_.rewrite_callback_options) { rewrite_method = diff --git a/tools/aapt2/java/JavaClassGenerator_test.cpp b/tools/aapt2/java/JavaClassGenerator_test.cpp index e449546f9399..10a97d84f59d 100644 --- a/tools/aapt2/java/JavaClassGenerator_test.cpp +++ b/tools/aapt2/java/JavaClassGenerator_test.cpp @@ -438,4 +438,22 @@ TEST(JavaClassGeneratorTest, GenerateOnResourcesLoadedCallbackForSharedLibrary) EXPECT_THAT(output, HasSubstr("com.boo.R.onResourcesLoaded")); } +TEST(JavaClassGeneratorTest, OnlyGenerateRText) { + std::unique_ptr<ResourceTable> table = + test::ResourceTableBuilder() + .SetPackageId("android", 0x01) + .AddValue("android:attr/foo", ResourceId(0x01010000), util::make_unique<Attribute>()) + .AddValue("android:styleable/hey.dude", ResourceId(0x01020000), + test::StyleableBuilder() + .AddItem("android:attr/foo", ResourceId(0x01010000)) + .Build()) + .Build(); + + std::unique_ptr<IAaptContext> context = + test::ContextBuilder().SetPackageId(0x01).SetCompilationPackage("android").Build(); + JavaClassGenerator generator(context.get(), table.get(), {}); + + ASSERT_TRUE(generator.Generate("android", nullptr)); +} + } // namespace aapt diff --git a/tools/aapt2/java/ProguardRules.cpp b/tools/aapt2/java/ProguardRules.cpp index ffcef8966654..d03cdb3d8518 100644 --- a/tools/aapt2/java/ProguardRules.cpp +++ b/tools/aapt2/java/ProguardRules.cpp @@ -79,8 +79,10 @@ class BaseVisitor : public xml::Visitor { keep_set_->AddConditionalClass({file_.name, file_.source.WithLine(line_number)}, class_name); } - void AddMethod(size_t line_number, const std::string& method_name) { - keep_set_->AddMethod({file_.name, file_.source.WithLine(line_number)}, method_name); + void AddMethod(size_t line_number, const std::string& method_name, + const std::string& method_signature) { + keep_set_->AddMethod({file_.name, file_.source.WithLine(line_number)}, + {method_name, method_signature}); } void AddReference(size_t line_number, Reference* ref) { @@ -125,7 +127,7 @@ class LayoutVisitor : public BaseVisitor { AddClass(node->line_number, attr.value); } else if (attr.namespace_uri == xml::kSchemaAndroid && attr.name == "onClick") { - AddMethod(node->line_number, attr.value); + AddMethod(node->line_number, attr.value, "android.view.View"); } } @@ -149,7 +151,7 @@ class MenuVisitor : public BaseVisitor { util::IsJavaClassName(attr.value)) { AddClass(node->line_number, attr.value); } else if (attr.name == "onClick") { - AddMethod(node->line_number, attr.value); + AddMethod(node->line_number, attr.value, "android.view.MenuItem"); } } } @@ -189,6 +191,29 @@ class XmlResourceVisitor : public BaseVisitor { DISALLOW_COPY_AND_ASSIGN(XmlResourceVisitor); }; +class NavigationVisitor : public BaseVisitor { + public: + NavigationVisitor(const ResourceFile& file, KeepSet* keep_set, const std::string& package) + : BaseVisitor(file, keep_set), package_(package) { + } + + void Visit(xml::Element* node) override { + const auto& attr = node->FindAttribute(xml::kSchemaAndroid, "name"); + if (attr != nullptr && !attr->value.empty()) { + std::string name = (attr->value[0] == '.') ? package_ + attr->value : attr->value; + if (util::IsJavaClassName(name)) { + AddClass(node->line_number, name); + } + } + + BaseVisitor::Visit(node); + } + + private: + DISALLOW_COPY_AND_ASSIGN(NavigationVisitor); + const std::string package_; +}; + class TransitionVisitor : public BaseVisitor { public: TransitionVisitor(const ResourceFile& file, KeepSet* keep_set) : BaseVisitor(file, keep_set) { @@ -291,7 +316,7 @@ bool CollectProguardRulesForManifest(xml::XmlResource* res, KeepSet* keep_set, b return false; } -bool CollectProguardRules(xml::XmlResource* res, KeepSet* keep_set) { +bool CollectProguardRules(IAaptContext* context_, xml::XmlResource* res, KeepSet* keep_set) { if (!res->root) { return false; } @@ -309,6 +334,12 @@ bool CollectProguardRules(xml::XmlResource* res, KeepSet* keep_set) { break; } + case ResourceType::kNavigation: { + NavigationVisitor visitor(res->file, keep_set, context_->GetCompilationPackage()); + res->root->Accept(&visitor); + break; + } + case ResourceType::kTransition: { TransitionVisitor visitor(res->file, keep_set); res->root->Accept(&visitor); @@ -336,7 +367,7 @@ void WriteKeepSet(const KeepSet& keep_set, OutputStream* out) { for (const UsageLocation& location : entry.second) { printer.Print("# Referenced at ").Println(location.source.to_string()); } - printer.Print("-keep class ").Print(entry.first).Println(" { <init>(...); }"); + printer.Print("-keep class ").Print(entry.first).Println(" { <init>(); }"); } for (const auto& entry : keep_set.conditional_class_set_) { @@ -367,7 +398,8 @@ void WriteKeepSet(const KeepSet& keep_set, OutputStream* out) { for (const UsageLocation& location : entry.second) { printer.Print("# Referenced at ").Println(location.source.to_string()); } - printer.Print("-keepclassmembers class * { *** ").Print(entry.first).Println("(...); }"); + printer.Print("-keepclassmembers class * { *** ").Print(entry.first.name) + .Print("(").Print(entry.first.signature).Println("); }"); printer.Println(); } } diff --git a/tools/aapt2/java/ProguardRules.h b/tools/aapt2/java/ProguardRules.h index 46827ee7cf93..acaceac41237 100644 --- a/tools/aapt2/java/ProguardRules.h +++ b/tools/aapt2/java/ProguardRules.h @@ -40,6 +40,11 @@ struct UsageLocation { Source source; }; +struct NameAndSignature { + std::string name; + std::string signature; +}; + class KeepSet { public: KeepSet() = default; @@ -55,8 +60,8 @@ class KeepSet { conditional_class_set_[class_name].insert(file); } - inline void AddMethod(const UsageLocation& file, const std::string& method_name) { - method_set_[method_name].insert(file); + inline void AddMethod(const UsageLocation& file, const NameAndSignature& name_and_signature) { + method_set_[name_and_signature].insert(file); } inline void AddReference(const UsageLocation& file, const ResourceName& resource_name) { @@ -71,7 +76,7 @@ class KeepSet { bool conditional_keep_rules_ = false; std::map<std::string, std::set<UsageLocation>> manifest_class_set_; - std::map<std::string, std::set<UsageLocation>> method_set_; + std::map<NameAndSignature, std::set<UsageLocation>> method_set_; std::map<std::string, std::set<UsageLocation>> conditional_class_set_; std::map<ResourceName, std::set<UsageLocation>> reference_set_; }; @@ -79,7 +84,7 @@ class KeepSet { bool CollectProguardRulesForManifest(xml::XmlResource* res, KeepSet* keep_set, bool main_dex_only = false); -bool CollectProguardRules(xml::XmlResource* res, KeepSet* keep_set); +bool CollectProguardRules(IAaptContext* context, xml::XmlResource* res, KeepSet* keep_set); bool CollectResourceReferences(IAaptContext* context, ResourceTable* table, KeepSet* keep_set); @@ -100,6 +105,20 @@ inline int operator<(const UsageLocation& lhs, const UsageLocation& rhs) { return lhs.name.compare(rhs.name); } +// +// NameAndSignature implementation. +// + +inline bool operator<(const NameAndSignature& lhs, const NameAndSignature& rhs) { + if (lhs.name < rhs.name) { + return true; + } + if (lhs.name == rhs.name) { + return lhs.signature < rhs.signature; + } + return false; +} + } // namespace proguard } // namespace aapt diff --git a/tools/aapt2/java/ProguardRules_test.cpp b/tools/aapt2/java/ProguardRules_test.cpp index 37d1a5fbaeb8..b5e27e0cb952 100644 --- a/tools/aapt2/java/ProguardRules_test.cpp +++ b/tools/aapt2/java/ProguardRules_test.cpp @@ -34,6 +34,31 @@ std::string GetKeepSetString(const proguard::KeepSet& set) { return out; } +TEST(ProguardRulesTest, ManifestRuleDefaultConstructorOnly) { + std::unique_ptr<xml::XmlResource> manifest = test::BuildXmlDom(R"( + <manifest xmlns:android="http://schemas.android.com/apk/res/android"> + <application android:backupAgent="com.foo.BarBackupAgent"> + <activity android:name="com.foo.BarActivity"/> + <service android:name="com.foo.BarService"/> + <receiver android:name="com.foo.BarReceiver"/> + <provider android:name="com.foo.BarProvider"/> + </application> + <instrumentation android:name="com.foo.BarInstrumentation"/> + </manifest>)"); + + proguard::KeepSet set; + ASSERT_TRUE(proguard::CollectProguardRulesForManifest(manifest.get(), &set, false)); + + std::string actual = GetKeepSetString(set); + + EXPECT_THAT(actual, HasSubstr("-keep class com.foo.BarBackupAgent { <init>(); }")); + EXPECT_THAT(actual, HasSubstr("-keep class com.foo.BarActivity { <init>(); }")); + EXPECT_THAT(actual, HasSubstr("-keep class com.foo.BarService { <init>(); }")); + EXPECT_THAT(actual, HasSubstr("-keep class com.foo.BarReceiver { <init>(); }")); + EXPECT_THAT(actual, HasSubstr("-keep class com.foo.BarProvider { <init>(); }")); + EXPECT_THAT(actual, HasSubstr("-keep class com.foo.BarInstrumentation { <init>(); }")); +} + TEST(ProguardRulesTest, FragmentNameRuleIsEmitted) { std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build(); std::unique_ptr<xml::XmlResource> layout = test::BuildXmlDom(R"( @@ -42,11 +67,11 @@ TEST(ProguardRulesTest, FragmentNameRuleIsEmitted) { layout->file.name = test::ParseNameOrDie("layout/foo"); proguard::KeepSet set; - ASSERT_TRUE(proguard::CollectProguardRules(layout.get(), &set)); + ASSERT_TRUE(proguard::CollectProguardRules(context.get(), layout.get(), &set)); std::string actual = GetKeepSetString(set); - EXPECT_THAT(actual, HasSubstr("com.foo.Bar")); + EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Bar { <init>(...); }")); } TEST(ProguardRulesTest, FragmentClassRuleIsEmitted) { @@ -56,11 +81,11 @@ TEST(ProguardRulesTest, FragmentClassRuleIsEmitted) { layout->file.name = test::ParseNameOrDie("layout/foo"); proguard::KeepSet set; - ASSERT_TRUE(proguard::CollectProguardRules(layout.get(), &set)); + ASSERT_TRUE(proguard::CollectProguardRules(context.get(), layout.get(), &set)); std::string actual = GetKeepSetString(set); - EXPECT_THAT(actual, HasSubstr("com.foo.Bar")); + EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Bar { <init>(...); }")); } TEST(ProguardRulesTest, FragmentNameAndClassRulesAreEmitted) { @@ -72,12 +97,40 @@ TEST(ProguardRulesTest, FragmentNameAndClassRulesAreEmitted) { layout->file.name = test::ParseNameOrDie("layout/foo"); proguard::KeepSet set; - ASSERT_TRUE(proguard::CollectProguardRules(layout.get(), &set)); + ASSERT_TRUE(proguard::CollectProguardRules(context.get(), layout.get(), &set)); std::string actual = GetKeepSetString(set); - EXPECT_THAT(actual, HasSubstr("com.foo.Bar")); - EXPECT_THAT(actual, HasSubstr("com.foo.Baz")); + EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Bar { <init>(...); }")); + EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Baz { <init>(...); }")); +} + +TEST(ProguardRulesTest, NavigationFragmentNameAndClassRulesAreEmitted) { + std::unique_ptr<IAaptContext> context = test::ContextBuilder() + .SetCompilationPackage("com.base").Build(); + std::unique_ptr<xml::XmlResource> navigation = test::BuildXmlDom(R"( + <navigation + xmlns:android="http://schemas.android.com/apk/res/android" + xmlns:app="http://schemas.android.com/apk/res-auto"> + <custom android:id="@id/foo" + android:name="com.package.Foo"/> + <fragment android:id="@id/bar" + android:name="com.package.Bar"> + <nested android:id="@id/nested" + android:name=".Nested"/> + </fragment> + </navigation> + )"); + + navigation->file.name = test::ParseNameOrDie("navigation/graph.xml"); + + proguard::KeepSet set; + ASSERT_TRUE(proguard::CollectProguardRules(context.get(), navigation.get(), &set)); + + std::string actual = GetKeepSetString(set); + EXPECT_THAT(actual, HasSubstr("-keep class com.package.Foo { <init>(...); }")); + EXPECT_THAT(actual, HasSubstr("-keep class com.package.Bar { <init>(...); }")); + EXPECT_THAT(actual, HasSubstr("-keep class com.base.Nested { <init>(...); }")); } TEST(ProguardRulesTest, CustomViewRulesAreEmitted) { @@ -89,11 +142,11 @@ TEST(ProguardRulesTest, CustomViewRulesAreEmitted) { layout->file.name = test::ParseNameOrDie("layout/foo"); proguard::KeepSet set; - ASSERT_TRUE(proguard::CollectProguardRules(layout.get(), &set)); + ASSERT_TRUE(proguard::CollectProguardRules(context.get(), layout.get(), &set)); std::string actual = GetKeepSetString(set); - EXPECT_THAT(actual, HasSubstr("com.foo.Bar")); + EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Bar { <init>(...); }")); } TEST(ProguardRulesTest, IncludedLayoutRulesAreConditional) { @@ -125,8 +178,8 @@ TEST(ProguardRulesTest, IncludedLayoutRulesAreConditional) { ASSERT_TRUE(xml_linker.Consume(context.get(), foo_layout.get())); proguard::KeepSet set = proguard::KeepSet(true); - ASSERT_TRUE(proguard::CollectProguardRules(bar_layout.get(), &set)); - ASSERT_TRUE(proguard::CollectProguardRules(foo_layout.get(), &set)); + ASSERT_TRUE(proguard::CollectProguardRules(context.get(), bar_layout.get(), &set)); + ASSERT_TRUE(proguard::CollectProguardRules(context.get(), foo_layout.get(), &set)); std::string actual = GetKeepSetString(set); @@ -134,7 +187,6 @@ TEST(ProguardRulesTest, IncludedLayoutRulesAreConditional) { EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Bar { <init>(...); }")); EXPECT_THAT(actual, HasSubstr("int foo")); EXPECT_THAT(actual, HasSubstr("int bar")); - EXPECT_THAT(actual, HasSubstr("com.foo.Bar")); } TEST(ProguardRulesTest, AliasedLayoutRulesAreConditional) { @@ -147,7 +199,7 @@ TEST(ProguardRulesTest, AliasedLayoutRulesAreConditional) { proguard::KeepSet set = proguard::KeepSet(true); set.AddReference({test::ParseNameOrDie("layout/bar"), {}}, layout->file.name); - ASSERT_TRUE(proguard::CollectProguardRules(layout.get(), &set)); + ASSERT_TRUE(proguard::CollectProguardRules(context.get(), layout.get(), &set)); std::string actual = GetKeepSetString(set); @@ -155,7 +207,6 @@ TEST(ProguardRulesTest, AliasedLayoutRulesAreConditional) { EXPECT_THAT(actual, HasSubstr("-if class **.R$layout")); EXPECT_THAT(actual, HasSubstr("int foo")); EXPECT_THAT(actual, HasSubstr("int bar")); - EXPECT_THAT(actual, HasSubstr("com.foo.Bar")); } TEST(ProguardRulesTest, NonLayoutReferencesAreUnconditional) { @@ -168,7 +219,7 @@ TEST(ProguardRulesTest, NonLayoutReferencesAreUnconditional) { proguard::KeepSet set = proguard::KeepSet(true); set.AddReference({test::ParseNameOrDie("style/MyStyle"), {}}, layout->file.name); - ASSERT_TRUE(proguard::CollectProguardRules(layout.get(), &set)); + ASSERT_TRUE(proguard::CollectProguardRules(context.get(), layout.get(), &set)); std::string actual = GetKeepSetString(set); @@ -184,11 +235,12 @@ TEST(ProguardRulesTest, ViewOnClickRuleIsEmitted) { layout->file.name = test::ParseNameOrDie("layout/foo"); proguard::KeepSet set; - ASSERT_TRUE(proguard::CollectProguardRules(layout.get(), &set)); + ASSERT_TRUE(proguard::CollectProguardRules(context.get(), layout.get(), &set)); std::string actual = GetKeepSetString(set); - EXPECT_THAT(actual, HasSubstr("bar_method")); + EXPECT_THAT(actual, HasSubstr( + "-keepclassmembers class * { *** bar_method(android.view.View); }")); } TEST(ProguardRulesTest, MenuRulesAreEmitted) { @@ -203,14 +255,47 @@ TEST(ProguardRulesTest, MenuRulesAreEmitted) { menu->file.name = test::ParseNameOrDie("menu/foo"); proguard::KeepSet set; - ASSERT_TRUE(proguard::CollectProguardRules(menu.get(), &set)); + ASSERT_TRUE(proguard::CollectProguardRules(context.get(), menu.get(), &set)); std::string actual = GetKeepSetString(set); - EXPECT_THAT(actual, HasSubstr("on_click")); - EXPECT_THAT(actual, HasSubstr("com.foo.Bar")); - EXPECT_THAT(actual, HasSubstr("com.foo.Baz")); + EXPECT_THAT(actual, HasSubstr( + "-keepclassmembers class * { *** on_click(android.view.MenuItem); }")); + EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Bar { <init>(...); }")); + EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Baz { <init>(...); }")); EXPECT_THAT(actual, Not(HasSubstr("com.foo.Bat"))); } +TEST(ProguardRulesTest, TransitionPathMotionRulesAreEmitted) { + std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build(); + std::unique_ptr<xml::XmlResource> transition = test::BuildXmlDom(R"( + <changeBounds> + <pathMotion class="com.foo.Bar"/> + </changeBounds>)"); + transition->file.name = test::ParseNameOrDie("transition/foo"); + + proguard::KeepSet set; + ASSERT_TRUE(proguard::CollectProguardRules(context.get(), transition.get(), &set)); + + std::string actual = GetKeepSetString(set); + + EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Bar { <init>(...); }")); +} + +TEST(ProguardRulesTest, TransitionRulesAreEmitted) { + std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build(); + std::unique_ptr<xml::XmlResource> transitionSet = test::BuildXmlDom(R"( + <transitionSet> + <transition class="com.foo.Bar"/> + </transitionSet>)"); + transitionSet->file.name = test::ParseNameOrDie("transition/foo"); + + proguard::KeepSet set; + ASSERT_TRUE(proguard::CollectProguardRules(context.get(), transitionSet.get(), &set)); + + std::string actual = GetKeepSetString(set); + + EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Bar { <init>(...); }")); +} + } // namespace aapt diff --git a/tools/aapt2/link/NoDefaultResourceRemover.cpp b/tools/aapt2/link/NoDefaultResourceRemover.cpp index cfb4b26aba02..5173b8541943 100644 --- a/tools/aapt2/link/NoDefaultResourceRemover.cpp +++ b/tools/aapt2/link/NoDefaultResourceRemover.cpp @@ -22,17 +22,7 @@ namespace aapt { -static bool IsDefaultConfigRequired(const ConfigDescription& config) { - // We don't want to be overzealous with resource removal, so have strict requirements. - // If a resource defines a value for a locale-only configuration, the default configuration is - // required. - if (ConfigDescription::DefaultConfig().diff(config) == ConfigDescription::CONFIG_LOCALE) { - return true; - } - return false; -} - -static bool KeepResource(const std::unique_ptr<ResourceEntry>& entry) { +static bool KeepResource(const std::unique_ptr<ResourceEntry>& entry, int minSdk) { if (entry->visibility.level == Visibility::Level::kPublic) { // Removing a public API without the developer knowing is bad, so just leave this here for now. return true; @@ -44,22 +34,44 @@ static bool KeepResource(const std::unique_ptr<ResourceEntry>& entry) { } // There is no default value defined, check if removal is required. + bool defaultRequired = false; for (const auto& config_value : entry->values) { - if (IsDefaultConfigRequired(config_value->config)) { - return false; + const int config = ConfigDescription::DefaultConfig().diff(config_value->config); + // If a resource defines a value for a locale-only configuration, the default configuration is + // required. + if (config == ConfigDescription::CONFIG_LOCALE) { + defaultRequired = true; + } + // If a resource defines a version-only config, the config value can be used as a default if + // the version is at most the minimum sdk version + else if (config == ConfigDescription::CONFIG_VERSION + && config_value->config.sdkVersion <= minSdk) { + return true; + } + // If a resource defines a value for a density only configuration, then that value could be used + // as a default and the entry should not be removed + else if (config == ConfigDescription::CONFIG_DENSITY + || (config == (ConfigDescription::CONFIG_DENSITY | ConfigDescription::CONFIG_VERSION) + && config_value->config.sdkVersion <= minSdk)) { + return true; } } - return true; + + return !defaultRequired; } bool NoDefaultResourceRemover::Consume(IAaptContext* context, ResourceTable* table) { - const ConfigDescription default_config = ConfigDescription::DefaultConfig(); for (auto& pkg : table->packages) { for (auto& type : pkg->types) { + // Gather the entries without defaults that must be removed + const int minSdk = context->GetMinSdkVersion(); const auto end_iter = type->entries.end(); - const auto new_end_iter = - std::stable_partition(type->entries.begin(), end_iter, KeepResource); - for (auto iter = new_end_iter; iter != end_iter; ++iter) { + const auto remove_iter = std::stable_partition(type->entries.begin(), end_iter, + [&minSdk](const std::unique_ptr<ResourceEntry>& entry) -> bool { + return KeepResource(entry, minSdk); + }); + + for (auto iter = remove_iter; iter != end_iter; ++iter) { const ResourceName name(pkg->name, type->type, (*iter)->name); IDiagnostics* diag = context->GetDiagnostics(); diag->Warn(DiagMessage() << "removing resource " << name @@ -74,7 +86,7 @@ bool NoDefaultResourceRemover::Consume(IAaptContext* context, ResourceTable* tab } } - type->entries.erase(new_end_iter, type->entries.end()); + type->entries.erase(remove_iter, end_iter); } } return true; diff --git a/tools/aapt2/link/NoDefaultResourceRemover_test.cpp b/tools/aapt2/link/NoDefaultResourceRemover_test.cpp index 943709a2af12..d129c9ac8db7 100644 --- a/tools/aapt2/link/NoDefaultResourceRemover_test.cpp +++ b/tools/aapt2/link/NoDefaultResourceRemover_test.cpp @@ -46,4 +46,64 @@ TEST(NoDefaultResourceRemoverTest, RemoveEntryWithNoDefaultAndOnlyLocales) { EXPECT_TRUE(table->FindResource(test::ParseNameOrDie("android:string/baz"))); } +TEST(NoDefaultResourceRemoverTest, KeepEntryWithLocalesAndDensities) { + std::unique_ptr<IAaptContext> context = test::ContextBuilder().SetMinSdkVersion(26).Build(); + std::unique_ptr<ResourceTable> table = + test::ResourceTableBuilder() + .SetPackageId("android", 0x01) + .AddSimple("android:drawable/keep1", test::ParseConfigOrDie("mdpi")) // v4 + .AddSimple("android:drawable/keep1", test::ParseConfigOrDie("en-rGB")) + .AddSimple("android:drawable/keep1", test::ParseConfigOrDie("fr-rFR")) + .AddSimple("android:drawable/keep2", test::ParseConfigOrDie("fr-rFR")) + .AddSimple("android:drawable/keep2", test::ParseConfigOrDie("en-rGB")) + .AddSimple("android:drawable/keep2", test::ParseConfigOrDie("xxxhdpi")) // v4 + .AddSimple("android:drawable/remove1", test::ParseConfigOrDie("fr-rFR")) + .AddSimple("android:drawable/remove1", test::ParseConfigOrDie("en-rGB")) + .AddSimple("android:drawable/remove1", test::ParseConfigOrDie("w600dp-xhdpi")) // v13 + .Build(); + + NoDefaultResourceRemover remover; + ASSERT_TRUE(remover.Consume(context.get(), table.get())); + + EXPECT_TRUE(table->FindResource(test::ParseNameOrDie("android:drawable/keep1"))); + EXPECT_TRUE(table->FindResource(test::ParseNameOrDie("android:drawable/keep2"))); + EXPECT_FALSE(table->FindResource(test::ParseNameOrDie("android:drawable/remove1"))); +} + +TEST(NoDefaultResourceRemoverTest, RemoveEntryWithLocalesAndDensitiesLowVersion) { + std::unique_ptr<IAaptContext> context = test::ContextBuilder().SetMinSdkVersion(3).Build(); + std::unique_ptr<ResourceTable> table = + test::ResourceTableBuilder() + .SetPackageId("android", 0x01) + .AddSimple("android:drawable/remove1", test::ParseConfigOrDie("mdpi")) // v4 + .AddSimple("android:drawable/remove1", test::ParseConfigOrDie("en-rGB")) + .AddSimple("android:drawable/remove1", test::ParseConfigOrDie("fr-rFR")) + .Build(); + + NoDefaultResourceRemover remover; + ASSERT_TRUE(remover.Consume(context.get(), table.get())); + + EXPECT_FALSE(table->FindResource(test::ParseNameOrDie("android:drawable/remove1"))); +} + +TEST(NoDefaultResourceRemoverTest, KeepEntryWithVersion) { + std::unique_ptr<IAaptContext> context = test::ContextBuilder().SetMinSdkVersion(8).Build(); + std::unique_ptr<ResourceTable> table = + test::ResourceTableBuilder() + .SetPackageId("android", 0x01) + .AddSimple("android:drawable/keep1", test::ParseConfigOrDie("v8")) + .AddSimple("android:drawable/keep1", test::ParseConfigOrDie("en-rGB")) + .AddSimple("android:drawable/keep1", test::ParseConfigOrDie("fr-rFR")) + .AddSimple("android:drawable/remove1", test::ParseConfigOrDie("v9")) + .AddSimple("android:drawable/remove1", test::ParseConfigOrDie("en-rGB")) + .AddSimple("android:drawable/remove1", test::ParseConfigOrDie("fr-rFR")) + .Build(); + + NoDefaultResourceRemover remover; + ASSERT_TRUE(remover.Consume(context.get(), table.get())); + + EXPECT_TRUE(table->FindResource(test::ParseNameOrDie("android:drawable/keep1"))); + EXPECT_FALSE(table->FindResource(test::ParseNameOrDie("android:drawable/remove1"))); +} + } // namespace aapt diff --git a/tools/aapt2/link/ReferenceLinker.cpp b/tools/aapt2/link/ReferenceLinker.cpp index 3a5d5858254d..28e71cc24f79 100644 --- a/tools/aapt2/link/ReferenceLinker.cpp +++ b/tools/aapt2/link/ReferenceLinker.cpp @@ -80,7 +80,7 @@ class ReferenceLinkerVisitor : public DescendingValueVisitor { // Find the attribute in the symbol table and check if it is visible from this callsite. const SymbolTable::Symbol* symbol = ReferenceLinker::ResolveAttributeCheckVisibility( - transformed_reference, callsite_, symbols_, &err_str); + transformed_reference, callsite_, symbols_, context_->IsAutoNamespace(), &err_str); if (symbol) { // Assign our style key the correct ID. The ID may not exist. entry.key.id = symbol->id; @@ -202,12 +202,18 @@ bool IsSymbolVisible(const SymbolTable::Symbol& symbol, const Reference& ref, const SymbolTable::Symbol* ReferenceLinker::ResolveSymbol(const Reference& reference, const CallSite& callsite, - SymbolTable* symbols) { + SymbolTable* symbols, + bool auto_namespace) { if (reference.name) { const ResourceName& name = reference.name.value(); if (name.package.empty()) { // Use the callsite's package name if no package name was defined. - return symbols->FindByName(ResourceName(callsite.package, name.type, name.entry)); + const SymbolTable::Symbol* local_symbol = + symbols->FindByName(ResourceName(callsite.package, name.type, name.entry)); + if (!auto_namespace || local_symbol) { + return local_symbol; + } + return symbols->FindByNameInAnyPackage(name); } return symbols->FindByName(name); } else if (reference.id) { @@ -220,8 +226,9 @@ const SymbolTable::Symbol* ReferenceLinker::ResolveSymbol(const Reference& refer const SymbolTable::Symbol* ReferenceLinker::ResolveSymbolCheckVisibility(const Reference& reference, const CallSite& callsite, SymbolTable* symbols, + bool auto_namespace, std::string* out_error) { - const SymbolTable::Symbol* symbol = ResolveSymbol(reference, callsite, symbols); + const SymbolTable::Symbol* symbol = ResolveSymbol(reference, callsite, symbols, auto_namespace); if (!symbol) { if (out_error) *out_error = "not found"; return nullptr; @@ -235,10 +242,10 @@ const SymbolTable::Symbol* ReferenceLinker::ResolveSymbolCheckVisibility(const R } const SymbolTable::Symbol* ReferenceLinker::ResolveAttributeCheckVisibility( - const Reference& reference, const CallSite& callsite, SymbolTable* symbols, + const Reference& reference, const CallSite& callsite, SymbolTable* symbols, bool auto_namespace, std::string* out_error) { const SymbolTable::Symbol* symbol = - ResolveSymbolCheckVisibility(reference, callsite, symbols, out_error); + ResolveSymbolCheckVisibility(reference, callsite, symbols, auto_namespace, out_error); if (!symbol) { return nullptr; } @@ -253,9 +260,10 @@ const SymbolTable::Symbol* ReferenceLinker::ResolveAttributeCheckVisibility( Maybe<xml::AaptAttribute> ReferenceLinker::CompileXmlAttribute(const Reference& reference, const CallSite& callsite, SymbolTable* symbols, + bool auto_namespace, std::string* out_error) { const SymbolTable::Symbol* symbol = - ResolveAttributeCheckVisibility(reference, callsite, symbols, out_error); + ResolveAttributeCheckVisibility(reference, callsite, symbols, auto_namespace, out_error); if (!symbol) { return {}; } @@ -333,8 +341,8 @@ bool ReferenceLinker::LinkReference(const CallSite& callsite, Reference* referen xml::ResolvePackage(decls, &transformed_reference); std::string err_str; - const SymbolTable::Symbol* s = - ResolveSymbolCheckVisibility(transformed_reference, callsite, symbols, &err_str); + const SymbolTable::Symbol* s = ResolveSymbolCheckVisibility( + transformed_reference, callsite, symbols, context->IsAutoNamespace(), &err_str); if (s) { // The ID may not exist. This is fine because of the possibility of building // against libraries without assigned IDs. diff --git a/tools/aapt2/link/ReferenceLinker.h b/tools/aapt2/link/ReferenceLinker.h index b0b49457e5dd..7887915a7bb1 100644 --- a/tools/aapt2/link/ReferenceLinker.h +++ b/tools/aapt2/link/ReferenceLinker.h @@ -36,10 +36,12 @@ class ReferenceLinker : public IResourceTableConsumer { ReferenceLinker() = default; // Performs name mangling and looks up the resource in the symbol table. Uses the callsite's - // package if the reference has no package name defined (implicit). + // package if the reference has no package name defined (implicit), or if auto_namespace is + // set try looking in all avaliable packages for a symbol of that name. // Returns nullptr if the symbol was not found. static const SymbolTable::Symbol* ResolveSymbol(const Reference& reference, - const CallSite& callsite, SymbolTable* symbols); + const CallSite& callsite, SymbolTable* symbols, + bool auto_namespace); // Performs name mangling and looks up the resource in the symbol table. If the symbol is not // visible by the reference at the callsite, nullptr is returned. @@ -47,6 +49,7 @@ class ReferenceLinker : public IResourceTableConsumer { static const SymbolTable::Symbol* ResolveSymbolCheckVisibility(const Reference& reference, const CallSite& callsite, SymbolTable* symbols, + bool auto_namespace, std::string* out_error); // Same as ResolveSymbolCheckVisibility(), but also makes sure the symbol is an attribute. @@ -54,13 +57,14 @@ class ReferenceLinker : public IResourceTableConsumer { static const SymbolTable::Symbol* ResolveAttributeCheckVisibility(const Reference& reference, const CallSite& callsite, SymbolTable* symbols, + bool auto_namespace, std::string* out_error); // Resolves the attribute reference and returns an xml::AaptAttribute if successful. // If resolution fails, outError holds the error message. static Maybe<xml::AaptAttribute> CompileXmlAttribute(const Reference& reference, const CallSite& callsite, - SymbolTable* symbols, + SymbolTable* symbols, bool auto_namespace, std::string* out_error); // Writes the resource name to the DiagMessage, using the diff --git a/tools/aapt2/link/ReferenceLinker_test.cpp b/tools/aapt2/link/ReferenceLinker_test.cpp index be38b967c986..0b7b1ce03a52 100644 --- a/tools/aapt2/link/ReferenceLinker_test.cpp +++ b/tools/aapt2/link/ReferenceLinker_test.cpp @@ -267,7 +267,7 @@ TEST(ReferenceLinkerTest, AppsWithSamePackageButDifferentIdAreVisibleNonPublic) std::string error; const CallSite call_site{"com.app.test"}; const SymbolTable::Symbol* symbol = ReferenceLinker::ResolveSymbolCheckVisibility( - *test::BuildReference("com.app.test:string/foo"), call_site, &table, &error); + *test::BuildReference("com.app.test:string/foo"), call_site, &table, false, &error); ASSERT_THAT(symbol, NotNull()); EXPECT_TRUE(error.empty()); } @@ -285,13 +285,13 @@ TEST(ReferenceLinkerTest, AppsWithDifferentPackageCanNotUseEachOthersAttribute) std::string error; const CallSite call_site{"com.app.ext"}; - EXPECT_FALSE(ReferenceLinker::CompileXmlAttribute( - *test::BuildReference("com.app.test:attr/foo"), call_site, &table, &error)); + EXPECT_FALSE(ReferenceLinker::CompileXmlAttribute(*test::BuildReference("com.app.test:attr/foo"), + call_site, &table, false, &error)); EXPECT_FALSE(error.empty()); error = ""; ASSERT_TRUE(ReferenceLinker::CompileXmlAttribute( - *test::BuildReference("com.app.test:attr/public_foo"), call_site, &table, &error)); + *test::BuildReference("com.app.test:attr/public_foo"), call_site, &table, false, &error)); EXPECT_TRUE(error.empty()); } @@ -303,19 +303,74 @@ TEST(ReferenceLinkerTest, ReferenceWithNoPackageUsesCallSitePackage) { .AddSymbol("com.app.lib:string/foo", ResourceId(0x7f010001)) .Build()); - const SymbolTable::Symbol* s = ReferenceLinker::ResolveSymbol(*test::BuildReference("string/foo"), - CallSite{"com.app.test"}, &table); + const SymbolTable::Symbol* s = ReferenceLinker::ResolveSymbol( + *test::BuildReference("string/foo"), CallSite{"com.app.test"}, &table, false); ASSERT_THAT(s, NotNull()); EXPECT_THAT(s->id, Eq(make_value<ResourceId>(0x7f010000))); s = ReferenceLinker::ResolveSymbol(*test::BuildReference("string/foo"), CallSite{"com.app.lib"}, - &table); + &table, false); ASSERT_THAT(s, NotNull()); EXPECT_THAT(s->id, Eq(make_value<ResourceId>(0x7f010001))); EXPECT_THAT(ReferenceLinker::ResolveSymbol(*test::BuildReference("string/foo"), - CallSite{"com.app.bad"}, &table), + CallSite{"com.app.bad"}, &table, false), IsNull()); } +TEST(ReferenceLinkerTest, AutomaticNamespace) { + NameMangler mangler(NameManglerPolicy{"com.example.thislib"}); + SymbolTable table(&mangler); + table.AppendSource( + test::StaticSymbolSourceBuilder() + .AddSymbol("com.example.thislib:string/thislib_string", ResourceId(0x7f010006)) + .AddSymbol("com.example.thislib:string/explicit_override_string", ResourceId(0x7f010007)) + .Build()); + // Lib2 is higher priority than lib1 + table.AppendSource( + test::StaticSymbolSourceBuilder() + .AddSymbol("com.example.lib2:string/lib2_string", ResourceId(0x7f010003)) + .AddSymbol("com.example.lib2:string/explicit_override_string", ResourceId(0x7f010004)) + .AddSymbol("com.example.lib2:string/implicit_override_string", ResourceId(0x7f010005)) + .Build()); + table.AppendSource( + test::StaticSymbolSourceBuilder() + .AddSymbol("com.example.lib1:string/explicit_override_string", ResourceId(0x7f010001)) + .AddSymbol("com.example.lib1:string/implicit_override_string", ResourceId(0x7f010002)) + .Build()); + + // Sanity test: Local references are still fine. + const SymbolTable::Symbol* s = + ReferenceLinker::ResolveSymbol(*test::BuildReference("string/thislib_string"), + CallSite{"com.example.thislib"}, &table, true); + ASSERT_THAT(s, NotNull()); + EXPECT_THAT(s->id, Eq(make_value<ResourceId>(0x7f010006))); + + // Local references are fine, even if clash with remote ones. + s = ReferenceLinker::ResolveSymbol(*test::BuildReference("string/explicit_override_string"), + CallSite{"com.example.thislib"}, &table, true); + ASSERT_THAT(s, NotNull()); + EXPECT_THAT(s->id, Eq(make_value<ResourceId>(0x7f010007))); + + // An unqualified reference to lib2 is rewritten + s = ReferenceLinker::ResolveSymbol(*test::BuildReference("string/lib2_string"), + CallSite{"com.example.thislib"}, &table, true); + ASSERT_THAT(s, NotNull()); + EXPECT_THAT(s->id, Eq(make_value<ResourceId>(0x7f010003))); + + // Qualified references are left alone. + s = ReferenceLinker::ResolveSymbol( + *test::BuildReference("com.example.lib2:string/explicit_override_string"), + CallSite{"com.example.thislib"}, &table, true); + ASSERT_THAT(s, NotNull()); + EXPECT_THAT(s->id, Eq(make_value<ResourceId>(0x7f010004))); + + // Implicit overrides respect priority ordering. + s = ReferenceLinker::ResolveSymbol(*test::BuildReference("string/implicit_override_string"), + CallSite{"com.example.thislib"}, &table, true); + ASSERT_THAT(s, NotNull()); + EXPECT_THAT(s->id, Eq(make_value<ResourceId>(0x7f010005))); + + // +} } // namespace aapt diff --git a/tools/aapt2/link/TableMerger.cpp b/tools/aapt2/link/TableMerger.cpp index e819f51a5634..91a55b337071 100644 --- a/tools/aapt2/link/TableMerger.cpp +++ b/tools/aapt2/link/TableMerger.cpp @@ -102,7 +102,17 @@ static bool MergeType(IAaptContext* context, const Source& src, ResourceTableTyp } static bool MergeEntry(IAaptContext* context, const Source& src, bool overlay, - ResourceEntry* dst_entry, ResourceEntry* src_entry) { + ResourceEntry* dst_entry, ResourceEntry* src_entry, + bool strict_visibility) { + if (strict_visibility + && dst_entry->visibility.level != Visibility::Level::kUndefined + && src_entry->visibility.level != dst_entry->visibility.level) { + context->GetDiagnostics()->Error( + DiagMessage(src) << "cannot merge resource '" << dst_entry->name << "' with conflicting visibilities: " + << "public and private"); + return false; + } + // Copy over the strongest visibility. if (src_entry->visibility.level > dst_entry->visibility.level) { // Only copy the ID if the source is public, or else the ID is meaningless. @@ -234,7 +244,7 @@ bool TableMerger::DoMerge(const Source& src, ResourceTable* src_table, continue; } - if (!MergeEntry(context_, src, overlay, dst_entry, src_entry.get())) { + if (!MergeEntry(context_, src, overlay, dst_entry, src_entry.get(), options_.strict_visibility)) { error = true; continue; } diff --git a/tools/aapt2/link/TableMerger.h b/tools/aapt2/link/TableMerger.h index 47e23dded26f..24c5e1329244 100644 --- a/tools/aapt2/link/TableMerger.h +++ b/tools/aapt2/link/TableMerger.h @@ -35,6 +35,8 @@ namespace aapt { struct TableMergerOptions { // If true, resources in overlays can be added without previously having existed. bool auto_add_overlay = false; + // If true, resource overlays with conflicting visibility are not allowed. + bool strict_visibility = false; }; // TableMerger takes resource tables and merges all packages within the tables that have the same diff --git a/tools/aapt2/link/TableMerger_test.cpp b/tools/aapt2/link/TableMerger_test.cpp index 34461c6b467d..cf504c4f4ce5 100644 --- a/tools/aapt2/link/TableMerger_test.cpp +++ b/tools/aapt2/link/TableMerger_test.cpp @@ -241,6 +241,37 @@ TEST_F(TableMergerTest, FailToOverrideConflictingEntryIdsWithOverlay) { ASSERT_FALSE(merger.Merge({}, overlay.get(), true /*overlay*/)); } +TEST_F(TableMergerTest, FailConflictingVisibility) { + std::unique_ptr<ResourceTable> base = + test::ResourceTableBuilder() + .SetPackageId("", 0x7f) + .SetSymbolState("bool/foo", ResourceId(0x7f, 0x01, 0x0001), Visibility::Level::kPublic) + .Build(); + std::unique_ptr<ResourceTable> overlay = + test::ResourceTableBuilder() + .SetPackageId("", 0x7f) + .SetSymbolState("bool/foo", ResourceId(0x7f, 0x01, 0x0001), Visibility::Level::kPrivate) + .Build(); + + // It should fail if the "--strict-visibility" flag is set. + ResourceTable final_table; + TableMergerOptions options; + options.auto_add_overlay = false; + options.strict_visibility = true; + TableMerger merger(context_.get(), &final_table, options); + + ASSERT_TRUE(merger.Merge({}, base.get(), false /*overlay*/)); + ASSERT_FALSE(merger.Merge({}, overlay.get(), true /*overlay*/)); + + // But it should still pass if the flag is not set. + ResourceTable final_table2; + options.strict_visibility = false; + TableMerger merger2(context_.get(), &final_table2, options); + + ASSERT_TRUE(merger2.Merge({}, base.get(), false /*overlay*/)); + ASSERT_TRUE(merger2.Merge({}, overlay.get(), true /*overlay*/)); +} + TEST_F(TableMergerTest, MergeAddResourceFromOverlay) { std::unique_ptr<ResourceTable> table_a = test::ResourceTableBuilder().SetPackageId("", 0x7f).Build(); diff --git a/tools/aapt2/link/XmlReferenceLinker.cpp b/tools/aapt2/link/XmlReferenceLinker.cpp index 160ff925f6cc..420a1271479d 100644 --- a/tools/aapt2/link/XmlReferenceLinker.cpp +++ b/tools/aapt2/link/XmlReferenceLinker.cpp @@ -97,8 +97,8 @@ class XmlVisitor : public xml::PackageAwareVisitor { attr_ref.private_reference = maybe_package.value().private_namespace; std::string err_str; - attr.compiled_attribute = - ReferenceLinker::CompileXmlAttribute(attr_ref, callsite_, symbols_, &err_str); + attr.compiled_attribute = ReferenceLinker::CompileXmlAttribute( + attr_ref, callsite_, symbols_, context_->IsAutoNamespace(), &err_str); if (!attr.compiled_attribute) { DiagMessage error_msg(source); diff --git a/tools/aapt2/link/XmlReferenceLinker_test.cpp b/tools/aapt2/link/XmlReferenceLinker_test.cpp index ef99355e5b5f..d321f8f76d87 100644 --- a/tools/aapt2/link/XmlReferenceLinker_test.cpp +++ b/tools/aapt2/link/XmlReferenceLinker_test.cpp @@ -18,6 +18,7 @@ #include "test/Test.h" +using ::testing::Eq; using ::testing::IsNull; using ::testing::NotNull; @@ -70,12 +71,29 @@ class XmlReferenceLinkerTest : public ::testing::Test { .Build()) .AddPublicSymbol("com.app.test:attr/attr", ResourceId(0x7f010002), test::AttributeBuilder().Build()) + .AddPublicSymbol("com.app.lib:string/lib_string", ResourceId(0x7f020003)) .Build()) .Build(); + + auto_namespace_context_ = + test::ContextBuilder() + .SetCompilationPackage("com.app.test") + .SetNameManglerPolicy(NameManglerPolicy{"com.app.test", {"com.android.support"}}) + .SetAutoNamespace(true) + .AddSymbolSource( + test::StaticSymbolSourceBuilder() + .AddPublicSymbol("android:attr/text", ResourceId(0x01010003), + test::AttributeBuilder() + .SetTypeMask(android::ResTable_map::TYPE_STRING) + .Build()) + .AddPublicSymbol("com.app.lib:string/lib_string", ResourceId(0x7f020003)) + .Build()) + .Build(); } protected: std::unique_ptr<IAaptContext> context_; + std::unique_ptr<IAaptContext> auto_namespace_context_; }; TEST_F(XmlReferenceLinkerTest, LinkBasicAttributes) { @@ -195,6 +213,31 @@ TEST_F(XmlReferenceLinkerTest, LinkAutoResReference) { EXPECT_EQ(make_value(ResourceId(0x7f020001)), ref->id); } +TEST_F(XmlReferenceLinkerTest, LinkAutoNamespaceResReference) { + std::unique_ptr<xml::XmlResource> doc = test::BuildXmlDomForPackageName(context_.get(), R"( + <View + xmlns:android="http://schemas.android.com/apk/res/android" + android:text="@string/lib_string" />)"); + + XmlReferenceLinker linker; + // Should not link with auto-namespace support disabled. + ASSERT_FALSE(linker.Consume(context_.get(), doc.get())); + // Should link with auto-namespace enabled. + ASSERT_TRUE(linker.Consume(auto_namespace_context_.get(), doc.get())); + + xml::Element* view_el = doc->root.get(); + ASSERT_THAT(view_el, NotNull()); + + xml::Attribute* xml_attr = view_el->FindAttribute(xml::kSchemaAndroid, "text"); + ASSERT_THAT(xml_attr, NotNull()); + ASSERT_TRUE(xml_attr->compiled_attribute); + EXPECT_EQ(make_value(ResourceId(0x01010003)), xml_attr->compiled_attribute.value().id); + Reference* ref = ValueCast<Reference>(xml_attr->compiled_value.get()); + ASSERT_THAT(ref, NotNull()); + ASSERT_TRUE(ref->name); + EXPECT_EQ(make_value(ResourceId(0x7f020003)), ref->id); +} + TEST_F(XmlReferenceLinkerTest, LinkViewWithShadowedPackageAlias) { std::unique_ptr<xml::XmlResource> doc = test::BuildXmlDomForPackageName(context_.get(), R"( <View xmlns:app="http://schemas.android.com/apk/res/android" app:attr="@app:id/id"> diff --git a/tools/aapt2/optimize/MultiApkGenerator.cpp b/tools/aapt2/optimize/MultiApkGenerator.cpp index 588b3316e6fa..9cfd7300df3d 100644 --- a/tools/aapt2/optimize/MultiApkGenerator.cpp +++ b/tools/aapt2/optimize/MultiApkGenerator.cpp @@ -98,6 +98,10 @@ class ContextWrapper : public IAaptContext { util::make_unique<SourcePathDiagnostics>(Source{source}, context_->GetDiagnostics()); } + bool IsAutoNamespace() override { + return context_->IsAutoNamespace(); + } + private: IAaptContext* context_; std::unique_ptr<SourcePathDiagnostics> source_diag_; diff --git a/tools/aapt2/optimize/ResourceFilter.cpp b/tools/aapt2/optimize/ResourceFilter.cpp new file mode 100644 index 000000000000..250b65197a7d --- /dev/null +++ b/tools/aapt2/optimize/ResourceFilter.cpp @@ -0,0 +1,43 @@ +/* + * Copyright (C) 2018 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "optimize/ResourceFilter.h" + +#include "ResourceTable.h" + +namespace aapt { + +ResourceFilter::ResourceFilter(const std::unordered_set<ResourceName>& blacklist) + : blacklist_(blacklist) { +} + +bool ResourceFilter::Consume(IAaptContext* context, ResourceTable* table) { + for (auto& package : table->packages) { + for (auto& type : package->types) { + for (auto it = type->entries.begin(); it != type->entries.end(); ) { + ResourceName resource = ResourceName({}, type->type, (*it)->name); + if (blacklist_.find(resource) != blacklist_.end()) { + it = type->entries.erase(it); + } else { + ++it; + } + } + } + } + return true; +} + +} // namespace aapt diff --git a/tools/aapt2/optimize/ResourceFilter.h b/tools/aapt2/optimize/ResourceFilter.h new file mode 100644 index 000000000000..d4baf654b0ff --- /dev/null +++ b/tools/aapt2/optimize/ResourceFilter.h @@ -0,0 +1,42 @@ +/* + * Copyright (C) 2018 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef AAPT_OPTIMIZE_RESOURCEFILTER_H +#define AAPT_OPTIMIZE_RESOURCEFILTER_H + +#include "android-base/macros.h" + +#include "process/IResourceTableConsumer.h" + +#include <unordered_set> + +namespace aapt { + +// Removes non-whitelisted entries from resource table. +class ResourceFilter : public IResourceTableConsumer { + public: + explicit ResourceFilter(const std::unordered_set<ResourceName>& blacklist); + + bool Consume(IAaptContext* context, ResourceTable* table) override; + + private: + DISALLOW_COPY_AND_ASSIGN(ResourceFilter); + std::unordered_set<ResourceName> blacklist_; +}; + +} // namespace aapt + +#endif // AAPT_OPTIMIZE_RESOURCEFILTER_H diff --git a/tools/aapt2/optimize/ResourceFilter_test.cpp b/tools/aapt2/optimize/ResourceFilter_test.cpp new file mode 100644 index 000000000000..800b2bfd0403 --- /dev/null +++ b/tools/aapt2/optimize/ResourceFilter_test.cpp @@ -0,0 +1,74 @@ +/* + * Copyright (C) 2018 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "optimize/ResourceFilter.h" + +#include "ResourceTable.h" +#include "test/Test.h" + +using ::aapt::test::HasValue; +using ::testing::Not; + +namespace aapt { + +TEST(ResourceFilterTest, SomeValuesAreFilteredOut) { + std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build(); + const ConfigDescription default_config = {}; + + std::unique_ptr<ResourceTable> table = + test::ResourceTableBuilder() + .AddString("android:string/notblacklisted", ResourceId{}, default_config, "value") + .AddString("android:string/blacklisted", ResourceId{}, default_config, "value") + .AddString("android:string/notblacklisted2", ResourceId{}, default_config, "value") + .AddString("android:string/blacklisted2", ResourceId{}, default_config, "value") + .Build(); + + std::unordered_set<ResourceName> blacklist = { + ResourceName({}, ResourceType::kString, "blacklisted"), + ResourceName({}, ResourceType::kString, "blacklisted2"), + }; + + ASSERT_TRUE(ResourceFilter(blacklist).Consume(context.get(), table.get())); + EXPECT_THAT(table, HasValue("android:string/notblacklisted", default_config)); + EXPECT_THAT(table, HasValue("android:string/notblacklisted2", default_config)); + EXPECT_THAT(table, Not(HasValue("android:string/blacklisted", default_config))); + EXPECT_THAT(table, Not(HasValue("android:string/blacklisted2", default_config))); +} + +TEST(ResourceFilterTest, TypeIsCheckedBeforeFiltering) { + std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build(); + const ConfigDescription default_config = {}; + + std::unique_ptr<ResourceTable> table = + test::ResourceTableBuilder() + .AddString("android:string/notblacklisted", ResourceId{}, default_config, "value") + .AddString("android:string/blacklisted", ResourceId{}, default_config, "value") + .AddString("android:drawable/notblacklisted", ResourceId{}, default_config, "value") + .AddString("android:drawable/blacklisted", ResourceId{}, default_config, "value") + .Build(); + + std::unordered_set<ResourceName> blacklist = { + ResourceName({}, ResourceType::kString, "blacklisted"), + }; + + ASSERT_TRUE(ResourceFilter(blacklist).Consume(context.get(), table.get())); + EXPECT_THAT(table, HasValue("android:string/notblacklisted", default_config)); + EXPECT_THAT(table, HasValue("android:drawable/blacklisted", default_config)); + EXPECT_THAT(table, HasValue("android:drawable/notblacklisted", default_config)); + EXPECT_THAT(table, Not(HasValue("android:string/blacklisted", default_config))); +} + +} // namespace aapt diff --git a/tools/aapt2/process/IResourceTableConsumer.h b/tools/aapt2/process/IResourceTableConsumer.h index 30dad8025900..a3a7719bfe16 100644 --- a/tools/aapt2/process/IResourceTableConsumer.h +++ b/tools/aapt2/process/IResourceTableConsumer.h @@ -50,6 +50,7 @@ struct IAaptContext { virtual NameMangler* GetNameMangler() = 0; virtual bool IsVerbose() = 0; virtual int GetMinSdkVersion() = 0; + virtual bool IsAutoNamespace() = 0; }; struct IResourceTableConsumer { diff --git a/tools/aapt2/process/SymbolTable.cpp b/tools/aapt2/process/SymbolTable.cpp index fc4c9b537e73..ef2e448b68b8 100644 --- a/tools/aapt2/process/SymbolTable.cpp +++ b/tools/aapt2/process/SymbolTable.cpp @@ -114,6 +114,16 @@ const SymbolTable::Symbol* SymbolTable::FindByName(const ResourceName& name) { return shared_symbol.get(); } +const SymbolTable::Symbol* SymbolTable::FindByNameInAnyPackage(const ResourceName& name) { + for (auto& source : sources_) { + std::string package = source->GetPackageForSymbol(name); + if (!package.empty()) { + return FindByName(ResourceName(package, name.type, name.entry)); + } + } + return {}; +} + const SymbolTable::Symbol* SymbolTable::FindById(const ResourceId& id) { if (const std::shared_ptr<Symbol>& s = id_cache_.get(id)) { return s.get(); @@ -211,6 +221,25 @@ std::unique_ptr<SymbolTable::Symbol> ResourceTableSymbolSource::FindByName( return symbol; } +std::string ResourceTableSymbolSource::GetPackageForSymbol(const ResourceName& name) { + for (auto& package : table_->packages) { + ResourceTableType* type = package->FindType(name.type); + if (type == nullptr) { + continue; + } + ResourceEntry* entry = type->FindEntry(name.entry); + if (entry == nullptr) { + continue; + } + return package->name; + } + if (name.type == ResourceType::kAttr) { + // Recurse and try looking up a private attribute. + return GetPackageForSymbol(ResourceName(name.package, ResourceType::kAttrPrivate, name.entry)); + } + return {}; +} + bool AssetManagerSymbolSource::AddAssetPath(const StringPiece& path) { int32_t cookie = 0; return assets_.addAssetPath(android::String8(path.data(), path.size()), &cookie); diff --git a/tools/aapt2/process/SymbolTable.h b/tools/aapt2/process/SymbolTable.h index 51a2e373596a..c798cbb8d8a2 100644 --- a/tools/aapt2/process/SymbolTable.h +++ b/tools/aapt2/process/SymbolTable.h @@ -89,6 +89,13 @@ class SymbolTable { // results are stored in a cache which may evict entries on subsequent calls. const Symbol* FindByName(const ResourceName& name); + // Finds the symbol from any package, for use as part of automatic conversion to namespaces. + // This returns the symbol from the highest priority package, + // which mimics the behavior of the resource merger and overlays. + // NOTE: Never hold on to the result between calls to FindByXXX. The + // results are stored in a cache which may evict entries on subsequent calls. + const Symbol* FindByNameInAnyPackage(const ResourceName& name); + // NOTE: Never hold on to the result between calls to FindByXXX. The // results are stored in a cache which may evict entries on subsequent calls. const Symbol* FindById(const ResourceId& id); @@ -153,6 +160,11 @@ class ISymbolSource { virtual std::unique_ptr<SymbolTable::Symbol> FindByName( const ResourceName& name) = 0; + // Finds the name of a symbol from any package, + // for use as part of automatic conversion to namespaces. + // This returns the symbol from the highest priority package, + // which mimics the behavior of the resource merger and overlays. + virtual std::string GetPackageForSymbol(const ResourceName& name) = 0; virtual std::unique_ptr<SymbolTable::Symbol> FindById(ResourceId id) = 0; // Default implementation tries the name if it exists, else the ID. @@ -177,6 +189,7 @@ class ResourceTableSymbolSource : public ISymbolSource { std::unique_ptr<SymbolTable::Symbol> FindByName( const ResourceName& name) override; + std::string GetPackageForSymbol(const ResourceName& name) override; std::unique_ptr<SymbolTable::Symbol> FindById(ResourceId id) override { return {}; } @@ -197,6 +210,9 @@ class AssetManagerSymbolSource : public ISymbolSource { std::unique_ptr<SymbolTable::Symbol> FindByName( const ResourceName& name) override; + std::string GetPackageForSymbol(const ResourceName& name) override { + return {}; + } std::unique_ptr<SymbolTable::Symbol> FindById(ResourceId id) override; std::unique_ptr<SymbolTable::Symbol> FindByReference( const Reference& ref) override; diff --git a/tools/aapt2/process/SymbolTable_test.cpp b/tools/aapt2/process/SymbolTable_test.cpp index 1f59d7034300..df40b26a64f4 100644 --- a/tools/aapt2/process/SymbolTable_test.cpp +++ b/tools/aapt2/process/SymbolTable_test.cpp @@ -120,4 +120,39 @@ TEST(SymbolTableTest, FindByNameWhenSymbolIsMangledInResTable) { EXPECT_THAT(symbol_table.FindByName(test::ParseNameOrDie("com.android.other:id/foo")), IsNull()); } +TEST(SymbolTableTest, FindByNameInAnyPackage) { + // This represents lib3 --depends-on--> lib2 --depends-on--> lib1 + + NameMangler mangler(NameManglerPolicy{"com.example.lib3"}); + SymbolTable symbol_table(&mangler); + // Lib2 has higher precedence than lib1, as it is closer to the current library (lib3) + // in the dependency graph. + + symbol_table.AppendSource(test::StaticSymbolSourceBuilder() + .AddPublicSymbol("com.example.lib1:string/foo", ResourceId()) + .AddSymbol("com.example.lib1:attr/foo", ResourceId(), + test::AttributeBuilder() + .SetTypeMask(android::ResTable_map::TYPE_FLAGS) + .AddItem("one", 0x01) + .AddItem("two", 0x02) + .Build()) + .Build()); + symbol_table.PrependSource(test::StaticSymbolSourceBuilder() + .AddPublicSymbol("com.example.lib2:string/foo", ResourceId()) + .Build()); + + // Sanity test + EXPECT_THAT(symbol_table.FindByName(test::ParseNameOrDie("string/foo")), IsNull()); + + // Test public symbol resolution + const SymbolTable::Symbol* const found_string = + symbol_table.FindByNameInAnyPackage(test::ParseNameOrDie("string/foo")); + ASSERT_THAT(found_string, NotNull()); + + // Test attr resolution + const SymbolTable::Symbol* const found_attr = + symbol_table.FindByNameInAnyPackage(test::ParseNameOrDie("attr/foo")); + ASSERT_THAT(found_attr, NotNull()); +} + } // namespace aapt diff --git a/tools/aapt2/test/Context.h b/tools/aapt2/test/Context.h index 0564db063b9a..a07d79f01dfe 100644 --- a/tools/aapt2/test/Context.h +++ b/tools/aapt2/test/Context.h @@ -81,6 +81,10 @@ class Context : public IAaptContext { return min_sdk_version_; } + bool IsAutoNamespace() override { + return auto_namespace_; + } + private: DISALLOW_COPY_AND_ASSIGN(Context); @@ -93,6 +97,7 @@ class Context : public IAaptContext { NameMangler name_mangler_; SymbolTable symbols_; int min_sdk_version_; + bool auto_namespace_; }; class ContextBuilder { @@ -127,6 +132,11 @@ class ContextBuilder { return *this; } + ContextBuilder& SetAutoNamespace(bool auto_namespace) { + context_->auto_namespace_ = auto_namespace; + return *this; + } + std::unique_ptr<Context> Build() { return std::move(context_); } private: @@ -172,6 +182,15 @@ class StaticSymbolSourceBuilder { return nullptr; } + std::string GetPackageForSymbol(const ResourceName& name) override { + for (auto const& imap : name_map_) { + if (imap.first.type == name.type && imap.first.entry == name.entry) { + return imap.first.package; + } + } + return ""; + } + std::unique_ptr<SymbolTable::Symbol> FindById(ResourceId id) override { auto iter = id_map_.find(id); if (iter != id_map_.end()) { diff --git a/tools/aapt2/util/Util.cpp b/tools/aapt2/util/Util.cpp index d1c9ca1644d9..9bef54e590c9 100644 --- a/tools/aapt2/util/Util.cpp +++ b/tools/aapt2/util/Util.cpp @@ -297,6 +297,53 @@ bool VerifyJavaStringFormat(const StringPiece& str) { return true; } +std::string Utf8ToModifiedUtf8(const std::string& utf8) { + // Java uses Modified UTF-8 which only supports the 1, 2, and 3 byte formats of UTF-8. To encode + // 4 byte UTF-8 codepoints, Modified UTF-8 allows the use of surrogate pairs in the same format + // of CESU-8 surrogate pairs. Calculate the size of the utf8 string with all 4 byte UTF-8 + // codepoints replaced with 2 3 byte surrogate pairs + size_t modified_size = 0; + const size_t size = utf8.size(); + for (size_t i = 0; i < size; i++) { + if (((uint8_t) utf8[i] >> 4) == 0xF) { + modified_size += 6; + i += 3; + } else { + modified_size++; + } + } + + // Early out if no 4 byte codepoints are found + if (size == modified_size) { + return utf8; + } + + std::string output; + output.reserve(modified_size); + for (size_t i = 0; i < size; i++) { + if (((uint8_t) utf8[i] >> 4) == 0xF) { + auto codepoint = (char32_t) utf32_from_utf8_at(utf8.data(), size, i, nullptr); + + // Calculate the high and low surrogates as UTF-16 would + char32_t high = ((codepoint - 0x10000) / 0x400) + 0xD800; + char32_t low = ((codepoint - 0x10000) % 0x400) + 0xDC00; + + // Encode each surrogate in UTF-8 + output.push_back((char) (0xE4 | ((high >> 12) & 0xF))); + output.push_back((char) (0x80 | ((high >> 6) & 0x3F))); + output.push_back((char) (0x80 | (high & 0x3F))); + output.push_back((char) (0xE4 | ((low >> 12) & 0xF))); + output.push_back((char) (0x80 | ((low >> 6) & 0x3F))); + output.push_back((char) (0x80 | (low & 0x3F))); + i += 3; + } else { + output.push_back(utf8[i]); + } + } + + return output; +} + std::u16string Utf8ToUtf16(const StringPiece& utf8) { ssize_t utf16_length = utf8_to_utf16_length( reinterpret_cast<const uint8_t*>(utf8.data()), utf8.length()); diff --git a/tools/aapt2/util/Util.h b/tools/aapt2/util/Util.h index 0eb35d18c06e..36b733376e6f 100644 --- a/tools/aapt2/util/Util.h +++ b/tools/aapt2/util/Util.h @@ -197,6 +197,9 @@ inline StringBuilder::operator bool() const { return error_.empty(); } +// Converts a UTF8 string into Modified UTF8 +std::string Utf8ToModifiedUtf8(const std::string& utf8); + // Converts a UTF8 string to a UTF16 string. std::u16string Utf8ToUtf16(const android::StringPiece& utf8); std::string Utf16ToUtf8(const android::StringPiece16& utf16); diff --git a/tools/aapt2/xml/XmlPullParser.cpp b/tools/aapt2/xml/XmlPullParser.cpp index 402e5a459f4e..a023494ad8f7 100644 --- a/tools/aapt2/xml/XmlPullParser.cpp +++ b/tools/aapt2/xml/XmlPullParser.cpp @@ -38,6 +38,7 @@ XmlPullParser::XmlPullParser(InputStream* in) : in_(in), empty_(), depth_(0) { EndNamespaceHandler); XML_SetCharacterDataHandler(parser_, CharacterDataHandler); XML_SetCommentHandler(parser_, CommentDataHandler); + XML_SetCdataSectionHandler(parser_, StartCdataSectionHandler, EndCdataSectionHandler); event_queue_.push(EventData{Event::kStartDocument, 0, depth_++}); } @@ -287,6 +288,22 @@ void XMLCALL XmlPullParser::CommentDataHandler(void* user_data, parser->depth_, comment}); } +void XMLCALL XmlPullParser::StartCdataSectionHandler(void* user_data) { + XmlPullParser* parser = reinterpret_cast<XmlPullParser*>(user_data); + + parser->event_queue_.push(EventData{Event::kCdataStart, + XML_GetCurrentLineNumber(parser->parser_), + parser->depth_ }); +} + +void XMLCALL XmlPullParser::EndCdataSectionHandler(void* user_data) { + XmlPullParser* parser = reinterpret_cast<XmlPullParser*>(user_data); + + parser->event_queue_.push(EventData{Event::kCdataEnd, + XML_GetCurrentLineNumber(parser->parser_), + parser->depth_ }); +} + Maybe<StringPiece> FindAttribute(const XmlPullParser* parser, const StringPiece& name) { auto iter = parser->FindAttribute("", name); diff --git a/tools/aapt2/xml/XmlPullParser.h b/tools/aapt2/xml/XmlPullParser.h index 63db66f0b2b7..6ebaa285745b 100644 --- a/tools/aapt2/xml/XmlPullParser.h +++ b/tools/aapt2/xml/XmlPullParser.h @@ -52,6 +52,8 @@ class XmlPullParser : public IPackageDeclStack { kEndElement, kText, kComment, + kCdataStart, + kCdataEnd, }; /** @@ -159,6 +161,8 @@ class XmlPullParser : public IPackageDeclStack { static void XMLCALL EndElementHandler(void* user_data, const char* name); static void XMLCALL EndNamespaceHandler(void* user_data, const char* prefix); static void XMLCALL CommentDataHandler(void* user_data, const char* comment); + static void XMLCALL StartCdataSectionHandler(void* user_data); + static void XMLCALL EndCdataSectionHandler(void* user_data); struct EventData { Event event; @@ -223,6 +227,10 @@ inline ::std::ostream& operator<<(::std::ostream& out, return out << "Text"; case XmlPullParser::Event::kComment: return out << "Comment"; + case XmlPullParser::Event::kCdataStart: + return out << "CdataStart"; + case XmlPullParser::Event::kCdataEnd: + return out << "CdataEnd"; } return out; } @@ -240,6 +248,8 @@ inline bool XmlPullParser::NextChildNode(XmlPullParser* parser, size_t start_dep case Event::kText: case Event::kComment: case Event::kStartElement: + case Event::kCdataStart: + case Event::kCdataEnd: return true; default: break; diff --git a/tools/apilint/apilint.py b/tools/apilint/apilint.py index 70a47cf42755..9db3f0271598 100644 --- a/tools/apilint/apilint.py +++ b/tools/apilint/apilint.py @@ -1334,18 +1334,25 @@ def verify_clone(clazz): error(clazz, m, None, "Provide an explicit copy constructor instead of implementing clone()") +def is_interesting(clazz): + """Test if given class is interesting from an Android PoV.""" + + if clazz.pkg.name.startswith("java"): return False + if clazz.pkg.name.startswith("junit"): return False + if clazz.pkg.name.startswith("org.apache"): return False + if clazz.pkg.name.startswith("org.xml"): return False + if clazz.pkg.name.startswith("org.json"): return False + if clazz.pkg.name.startswith("org.w3c"): return False + if clazz.pkg.name.startswith("android.icu."): return False + return True + + def examine_clazz(clazz): """Find all style issues in the given class.""" notice(clazz) - if clazz.pkg.name.startswith("java"): return - if clazz.pkg.name.startswith("junit"): return - if clazz.pkg.name.startswith("org.apache"): return - if clazz.pkg.name.startswith("org.xml"): return - if clazz.pkg.name.startswith("org.json"): return - if clazz.pkg.name.startswith("org.w3c"): return - if clazz.pkg.name.startswith("android.icu."): return + if not is_interesting(clazz): return verify_constants(clazz) verify_enums(clazz) @@ -1479,6 +1486,7 @@ def show_deprecations_at_birth(cur, prev): # Remove all existing things so we're left with new for prev_clazz in prev.values(): cur_clazz = cur[prev_clazz.fullname] + if not is_interesting(cur_clazz): continue sigs = { i.ident: i for i in prev_clazz.ctors } cur_clazz.ctors = [ i for i in cur_clazz.ctors if i.ident not in sigs ] @@ -1506,6 +1514,38 @@ def show_deprecations_at_birth(cur, prev): print +def show_stats(cur, prev): + """Show API stats.""" + + stats = collections.defaultdict(int) + for cur_clazz in cur.values(): + if not is_interesting(cur_clazz): continue + + if cur_clazz.fullname not in prev: + stats['new_classes'] += 1 + stats['new_ctors'] += len(cur_clazz.ctors) + stats['new_methods'] += len(cur_clazz.methods) + stats['new_fields'] += len(cur_clazz.fields) + else: + prev_clazz = prev[cur_clazz.fullname] + + sigs = { i.ident: i for i in prev_clazz.ctors } + ctors = len([ i for i in cur_clazz.ctors if i.ident not in sigs ]) + sigs = { i.ident: i for i in prev_clazz.methods } + methods = len([ i for i in cur_clazz.methods if i.ident not in sigs ]) + sigs = { i.ident: i for i in prev_clazz.fields } + fields = len([ i for i in cur_clazz.fields if i.ident not in sigs ]) + + if ctors + methods + fields > 0: + stats['extend_classes'] += 1 + stats['extend_ctors'] += ctors + stats['extend_methods'] += methods + stats['extend_fields'] += fields + + print "#", "".join([ k.ljust(20) for k in sorted(stats.keys()) ]) + print " ", "".join([ str(stats[k]).ljust(20) for k in sorted(stats.keys()) ]) + + if __name__ == "__main__": parser = argparse.ArgumentParser(description="Enforces common Android public API design \ patterns. It ignores lint messages from a previous API level, if provided.") @@ -1520,6 +1560,8 @@ if __name__ == "__main__": help="Show API changes noticed") parser.add_argument("--show-deprecations-at-birth", action='store_const', const=True, help="Show API deprecations at birth") + parser.add_argument("--show-stats", action='store_const', const=True, + help="Show API stats") args = vars(parser.parse_args()) if args['no_color']: @@ -1539,6 +1581,14 @@ if __name__ == "__main__": show_deprecations_at_birth(cur, prev) sys.exit() + if args['show_stats']: + with current_file as f: + cur = _parse_stream(f) + with previous_file as f: + prev = _parse_stream(f) + show_stats(cur, prev) + sys.exit() + with current_file as f: cur_fail, cur_noticed = examine_stream(f) if not previous_file is None: diff --git a/tools/apilint/apilint_stats.sh b/tools/apilint/apilint_stats.sh new file mode 100755 index 000000000000..052d9a5265fe --- /dev/null +++ b/tools/apilint/apilint_stats.sh @@ -0,0 +1,7 @@ +#!/bin/bash +API=28 +while [ $API -gt 14 ]; do + echo "# Changes in API $((API))" + python tools/apilint/apilint.py --show-stats ../../prebuilts/sdk/$((API))/public/api/android.txt ../../prebuilts/sdk/$((API-1))/public/api/android.txt + let API=API-1 +done diff --git a/tools/incident_section_gen/main.cpp b/tools/incident_section_gen/main.cpp index 3689a8f2981e..639f98062b71 100644 --- a/tools/incident_section_gen/main.cpp +++ b/tools/incident_section_gen/main.cpp @@ -110,9 +110,7 @@ static bool generateIncidentSectionsCpp(Descriptor const* descriptor) N = descriptor->field_count(); for (int i=0; i<N; i++) { const FieldDescriptor* field = descriptor->field(i); - if (field->type() == FieldDescriptor::TYPE_MESSAGE) { - sections[field->name()] = field; - } + sections[field->name()] = field; } printf("IncidentSection const INCIDENT_SECTIONS[] = {\n"); @@ -404,7 +402,7 @@ static bool generateSectionListCpp(Descriptor const* descriptor) { for (int i=0; i<descriptor->field_count(); i++) { const FieldDescriptor* field = descriptor->field(i); - if (field->type() != FieldDescriptor::TYPE_MESSAGE) { + if (field->type() != FieldDescriptor::TYPE_MESSAGE && field->type() != FieldDescriptor::TYPE_STRING) { continue; } const SectionFlags s = getSectionFlags(field); @@ -412,8 +410,7 @@ static bool generateSectionListCpp(Descriptor const* descriptor) { case SECTION_NONE: continue; case SECTION_FILE: - printf(" new FileSection(%d, \"%s\", %s),\n", field->number(), s.args().c_str(), - s.device_specific() ? "true" : "false"); + printf(" new FileSection(%d, \"%s\"),\n", field->number(), s.args().c_str()); break; case SECTION_COMMAND: printf(" new CommandSection(%d,", field->number()); @@ -457,13 +454,13 @@ static bool generateSectionListCpp(Descriptor const* descriptor) { const FieldDescriptor* field = fieldsInOrder[i]; const string fieldName = getFieldName(field); const Destination fieldDest = getFieldDest(field); - const string fieldMessageName = getMessageName(field->message_type(), fieldDest); - - skip[i] = true; - if (field->type() != FieldDescriptor::TYPE_MESSAGE) { + printPrivacy(fieldName, field, "NULL", fieldDest, "NULL"); continue; } + + skip[i] = true; + const string fieldMessageName = getMessageName(field->message_type(), fieldDest); // generate privacy flags for each section. if (generatePrivacyFlags(field->message_type(), fieldDest, variableNames, &parents)) { printPrivacy(fieldName, field, fieldMessageName, fieldDest, "NULL"); diff --git a/tools/stringslint/stringslint.py b/tools/stringslint/stringslint.py index d637ff346c82..03c0b9af66a0 100644 --- a/tools/stringslint/stringslint.py +++ b/tools/stringslint/stringslint.py @@ -20,11 +20,22 @@ a previous strings file, if provided. Usage: stringslint.py strings.xml Usage: stringslint.py strings.xml old_strings.xml + +In general: +* Errors signal issues that must be fixed before submitting, and are only + used when there are no false-positives. +* Warnings signal issues that might need to be fixed, but need manual + inspection due to risk of false-positives. +* Info signal issues that should be fixed to match best-practices, such + as providing comments to aid translation. """ -import re, sys +import re, sys, codecs import lxml.etree as ET +reload(sys) +sys.setdefaultencoding('utf8') + BLACK, RED, GREEN, YELLOW, BLUE, MAGENTA, CYAN, WHITE = range(8) def format(fg=None, bg=None, bright=False, bold=False, dim=False, reset=False): @@ -43,10 +54,10 @@ def format(fg=None, bg=None, bright=False, bold=False, dim=False, reset=False): warnings = None -def warn(tag, msg, actual, expected): +def warn(tag, msg, actual, expected, color=YELLOW): global warnings key = "%s:%d" % (tag.attrib["name"], hash(msg)) - value = "%sLine %d: '%s':%s %s" % (format(fg=YELLOW, bold=True), + value = "%sLine %d: '%s':%s %s" % (format(fg=color, bold=True), tag.sourceline, tag.attrib["name"], format(reset=True), @@ -59,6 +70,46 @@ def warn(tag, msg, actual, expected): format(reset=True)) warnings[key] = value + +def error(tag, msg, actual, expected): + warn(tag, msg, actual, expected, RED) + +def info(tag, msg, actual, expected): + warn(tag, msg, actual, expected, CYAN) + +# Escaping logic borrowed from https://stackoverflow.com/a/24519338 +ESCAPE_SEQUENCE_RE = re.compile(r''' + ( \\U........ # 8-digit hex escapes + | \\u.... # 4-digit hex escapes + | \\x.. # 2-digit hex escapes + | \\[0-7]{1,3} # Octal escapes + | \\N\{[^}]+\} # Unicode characters by name + | \\[\\'"abfnrtv] # Single-character escapes + )''', re.UNICODE | re.VERBOSE) + +def decode_escapes(s): + def decode_match(match): + return codecs.decode(match.group(0), 'unicode-escape') + + s = re.sub(r"\n\s*", " ", s) + s = ESCAPE_SEQUENCE_RE.sub(decode_match, s) + s = re.sub(r"%(\d+\$)?[a-z]", "____", s) + s = re.sub(r"\^\d+", "____", s) + s = re.sub(r"<br/?>", "\n", s) + s = re.sub(r"</?[a-z]+>", "", s) + return s + +def sample_iter(tag): + if not isinstance(tag, ET._Comment) and re.match("{.*xliff.*}g", tag.tag) and "example" in tag.attrib: + yield tag.attrib["example"] + elif tag.text: + yield decode_escapes(tag.text) + for e in tag: + for v in sample_iter(e): + yield v + if e.tail: + yield decode_escapes(e.tail) + def lint(path): global warnings warnings = {} @@ -80,35 +131,45 @@ def lint(path): comment = last_comment last_comment = None + # Prepare string for analysis + text = "".join(child.itertext()) + sample = "".join(sample_iter(child)).strip().strip("'\"") + # Validate comment if comment is None: - warn(child, "Missing string comment to aid translation", + info(child, "Missing string comment to aid translation", None, None) continue if "do not translate" in comment.text.lower(): continue if "translatable" in child.attrib and child.attrib["translatable"].lower() == "false": continue - if re.search("CHAR[ _-]LIMIT=(\d+|NONE|none)", comment.text) is None: - warn(child, "Missing CHAR LIMIT to aid translation", + + limit = re.search("CHAR[ _-]LIMIT=(\d+|NONE|none)", comment.text) + if limit is None: + info(child, "Missing CHAR LIMIT to aid translation", repr(comment), "<!-- Description of string [CHAR LIMIT=32] -->") + elif re.match("\d+", limit.group(1)): + limit = int(limit.group(1)) + if len(sample) > limit: + warn(child, "Expanded string length is larger than CHAR LIMIT", + sample, None) # Look for common mistakes/substitutions - text = "".join(child.itertext()).strip() if "'" in text: - warn(child, "Turned quotation mark glyphs are more polished", + error(child, "Turned quotation mark glyphs are more polished", text, "This doesn\u2019t need to \u2018happen\u2019 today") if '"' in text and not text.startswith('"') and text.endswith('"'): - warn(child, "Turned quotation mark glyphs are more polished", + error(child, "Turned quotation mark glyphs are more polished", text, "This needs to \u201chappen\u201d today") if "..." in text: - warn(child, "Ellipsis glyph is more polished", + error(child, "Ellipsis glyph is more polished", text, "Loading\u2026") if "wi-fi" in text.lower(): - warn(child, "Non-breaking glyph is more polished", + error(child, "Non-breaking glyph is more polished", text, "Wi\u2011Fi") if "wifi" in text.lower(): - warn(child, "Using non-standard spelling", + error(child, "Using non-standard spelling", text, "Wi\u2011Fi") if re.search("\d-\d", text): warn(child, "Ranges should use en dash glyph", @@ -119,11 +180,17 @@ def lint(path): if ". " in text: warn(child, "Only use single space between sentences", text, "First idea. Second idea.") + if re.match(r"^[A-Z\s]{5,}$", text): + warn(child, "Actions should use android:textAllCaps in layout; ignore if acronym", + text, "Refresh data") + if " phone " in text and "product" not in child.attrib: + warn(child, "Strings mentioning phones should have variants for tablets", + text, None) # When more than one substitution, require indexes if len(re.findall("%[^%]", text)) > 1: if len(re.findall("%[^\d]", text)) > 0: - warn(child, "Substitutions must be indexed", + error(child, "Substitutions must be indexed", text, "Add %1$s to %2$s") # Require xliff substitutions @@ -132,15 +199,15 @@ def lint(path): if gc.tail and re.search("%[^%]", gc.tail): badsub = True if re.match("{.*xliff.*}g", gc.tag): if "id" not in gc.attrib: - warn(child, "Substitutions must define id attribute", + error(child, "Substitutions must define id attribute", None, "<xliff:g id=\"domain\" example=\"example.com\">%1$s</xliff:g>") if "example" not in gc.attrib: - warn(child, "Substitutions must define example attribute", + error(child, "Substitutions must define example attribute", None, "<xliff:g id=\"domain\" example=\"example.com\">%1$s</xliff:g>") else: if gc.text and re.search("%[^%]", gc.text): badsub = True if badsub: - warn(child, "Substitutions must be inside xliff tags", + error(child, "Substitutions must be inside xliff tags", text, "<xliff:g id=\"domain\" example=\"example.com\">%1$s</xliff:g>") return warnings |