Merge "Add option to avoid clobbering visibility of <declare-styleable>" am: 7b56a5075d am: 002ba59e2b

Change-Id: I151857468d1a35bae8396e612c0a17cbcdf5dea1

tools/aapt2/ResourceParser.cpp

@@ -1643,8 +1643,14 @@ bool ResourceParser::ParseDeclareStyleable(xml::XmlPullParser* parser,
                                            ParsedResource* out_resource) {
   out_resource->name.type = ResourceType::kStyleable;
 
-  // Declare-styleable is kPrivate by default, because it technically only exists in R.java.
+  if (!options_.preserve_visibility_of_styleables) {
+    // This was added in change Idd21b5de4d20be06c6f8c8eb5a22ccd68afc4927 to mimic aapt1, but no one
+    // knows exactly what for.
+    //
+    // FWIW, styleables only appear in generated R classes.  For custom views these should always be
+    // package-private (to be used only by the view class); themes are a different story.
     out_resource->visibility_level = Visibility::Level::kPublic;
+  }
 
   // Declare-styleable only ends up in default config;
   if (out_resource->config != ConfigDescription::DefaultConfig()) {

tools/aapt2/ResourceParser.h

@@ -46,6 +46,12 @@ struct ResourceParserOptions {
    */
   bool error_on_positional_arguments = true;
 
+  /**
+   * If true, apply the same visibility rules for styleables as are used for
+   * all other resources.  Otherwise, all styleables will be made public.
+   */
+  bool preserve_visibility_of_styleables = false;
+
   // 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;

tools/aapt2/ResourceParser_test.cpp

@@ -611,6 +611,32 @@ TEST_F(ResourceParserTest, ParseAttributesDeclareStyleable) {
   EXPECT_THAT(styleable->entries[2].name, Eq(make_value(test::ParseNameOrDie("attr/baz"))));
 }
 
+TEST_F(ResourceParserTest, ParseDeclareStyleablePreservingVisibility) {
+  StringInputStream input(R"(
+      <resources>
+        <declare-styleable name="foo">
+          <attr name="myattr" />
+        </declare-styleable>
+        <declare-styleable name="bar">
+          <attr name="myattr" />
+        </declare-styleable>
+        <public type="styleable" name="bar" />
+      </resources>)");
+  ResourceParser parser(context_->GetDiagnostics(), &table_, Source{"test"},
+                        ConfigDescription::DefaultConfig(),
+                        ResourceParserOptions{.preserve_visibility_of_styleables = true});
+
+  xml::XmlPullParser xml_parser(&input);
+  ASSERT_TRUE(parser.Parse(&xml_parser));
+
+  EXPECT_EQ(
+      table_.FindResource(test::ParseNameOrDie("styleable/foo")).value().entry->visibility.level,
+      Visibility::Level::kUndefined);
+  EXPECT_EQ(
+      table_.FindResource(test::ParseNameOrDie("styleable/bar")).value().entry->visibility.level,
+      Visibility::Level::kPublic);
+}
+
 TEST_F(ResourceParserTest, ParsePrivateAttributesDeclareStyleable) {
   std::string input = R"(
       <declare-styleable xmlns:privAndroid="http://schemas.android.com/apk/prv/res/android"

tools/aapt2/cmd/Compile.cpp

@@ -159,6 +159,7 @@ static bool CompileTable(IAaptContext* context, const CompileOptions& options,
 
     ResourceParserOptions parser_options;
     parser_options.error_on_positional_arguments = !options.legacy_mode;
+    parser_options.preserve_visibility_of_styleables = options.preserve_visibility_of_styleables;
     parser_options.translatable = translatable_file;
 
     // If visibility was forced, we need to use it when creating a new resource and also error if

tools/aapt2/cmd/Compile.h

@@ -35,6 +35,8 @@ struct CompileOptions {
   bool pseudolocalize = false;
   bool no_png_crunch = false;
   bool legacy_mode = false;
+  // See comments on aapt::ResourceParserOptions.
+  bool preserve_visibility_of_styleables = false;
   bool verbose = false;
 };
 
@@ -56,6 +58,11 @@ class CompileCommand : public Command {
     AddOptionalSwitch("--no-crunch", "Disables PNG processing", &options_.no_png_crunch);
     AddOptionalSwitch("--legacy", "Treat errors that used to be valid in AAPT as warnings",
         &options_.legacy_mode);
+    AddOptionalSwitch("--preserve-visibility-of-styleables",
+                      "If specified, apply the same visibility rules for\n"
+                      "styleables as are used for all other resources.\n"
+                      "Otherwise, all stylesables will be made public.",
+                      &options_.preserve_visibility_of_styleables);
     AddOptionalFlag("--visibility",
         "Sets the visibility of the compiled resources to the specified\n"
             "level. Accepted levels: public, private, default", &visibility_);