AAPT2: add flag for strict visibility

Will only detect whether a resource was defined as both 'public' and
'private' (but will allow overriding 'undefined' visiblity for now).

Test: TableMerger_test + manual
Bug: 72735798

Change-Id: If0749559c91c4d8820a6286fc9ddc80209c1e5e9

tools/aapt2/cmd/Link.cpp

@@ -137,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 {
@@ -1713,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()) {
@@ -2209,7 +2213,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;

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;
       }

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

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();