Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 90919978 authored by Adam Lesinski's avatar Adam Lesinski
Browse files

DO NOT MERGE: AAPT2: Allow undefined resources (placeholders) 

A resource defined like so:

<item type="drawable" name="foo" />

should be assigned the value @null.

The only exception is for <string> resources, which are given the
empty string value (since <string></string> is ambiguous). The decision
to use "" is based off the fact that old AAPT used to assign "" to all
undefined resources, even non-string ones.

Bug: 38425050
Test: make aapt2_tests
Change-Id: Ib3e0f6f83d16ddd8b279c9fd44a07a37867b85e9
parent 76e76a5c

tools/aapt2/Android.bp

+1 −1
Original line number Diff line number Diff line
@@ -159,7 +159,7 @@ cc_library_host_shared { 
// ==========================================================

cc_test_host {

    name: "aapt2_tests",

    srcs: ["**/*_test.cpp"],

    srcs: ["test/Common.cpp", "**/*_test.cpp"],

    static_libs: ["libaapt2", "libgmock"],

    defaults: ["aapt_defaults"],

}

tools/aapt2/ResourceParser.cpp

+5 −0
Original line number Diff line number Diff line
@@ -623,6 +623,11 @@ std::unique_ptr<Item> ResourceParser::ParseXml(xml::XmlPullParser* parser, 
    return std::move(string);

  }



  // If the text is empty, and the value is not allowed to be a string, encode it as a @null.

  if (util::TrimWhitespace(raw_value).empty()) {

    return ResourceUtils::MakeNull();

  }



  if (allow_raw_value) {

    // We can't parse this so return a RawString if we are allowed.

    return util::make_unique<RawString>(

tools/aapt2/ResourceParser.h

+10 −20
Original line number Diff line number Diff line
@@ -78,42 +78,32 @@ class ResourceParser { 
   * Item, then a

   * RawString is returned. Otherwise this returns false;

   */

  std::unique_ptr<Item> ParseXml(xml::XmlPullParser* parser,

                                 const uint32_t type_mask,

  std::unique_ptr<Item> ParseXml(xml::XmlPullParser* parser, const uint32_t type_mask,

                                 const bool allow_raw_value);



  bool ParseResources(xml::XmlPullParser* parser);

  bool ParseResource(xml::XmlPullParser* parser, ParsedResource* out_resource);



  bool ParseItem(xml::XmlPullParser* parser, ParsedResource* out_resource,

                 uint32_t format);

  bool ParseItem(xml::XmlPullParser* parser, ParsedResource* out_resource, uint32_t format);

  bool ParseString(xml::XmlPullParser* parser, ParsedResource* out_resource);



  bool ParsePublic(xml::XmlPullParser* parser, ParsedResource* out_resource);

  bool ParsePublicGroup(xml::XmlPullParser* parser,

                        ParsedResource* out_resource);

  bool ParseSymbolImpl(xml::XmlPullParser* parser,

                       ParsedResource* out_resource);

  bool ParsePublicGroup(xml::XmlPullParser* parser, ParsedResource* out_resource);

  bool ParseSymbolImpl(xml::XmlPullParser* parser, ParsedResource* out_resource);

  bool ParseSymbol(xml::XmlPullParser* parser, ParsedResource* out_resource);

  bool ParseAddResource(xml::XmlPullParser* parser,

                        ParsedResource* out_resource);

  bool ParseAddResource(xml::XmlPullParser* parser, ParsedResource* out_resource);

  bool ParseAttr(xml::XmlPullParser* parser, ParsedResource* out_resource);

  bool ParseAttrImpl(xml::XmlPullParser* parser, ParsedResource* out_resource,

                     bool weak);

  bool ParseAttrImpl(xml::XmlPullParser* parser, ParsedResource* out_resource, bool weak);

  Maybe<Attribute::Symbol> ParseEnumOrFlagItem(xml::XmlPullParser* parser,

                                               const android::StringPiece& tag);

  bool ParseStyle(const ResourceType type, xml::XmlPullParser* parser,

                  ParsedResource* out_resource);

  bool ParseStyleItem(xml::XmlPullParser* parser, Style* style);

  bool ParseDeclareStyleable(xml::XmlPullParser* parser,

                             ParsedResource* out_resource);

  bool ParseDeclareStyleable(xml::XmlPullParser* parser, ParsedResource* out_resource);

  bool ParseArray(xml::XmlPullParser* parser, ParsedResource* out_resource);

  bool ParseIntegerArray(xml::XmlPullParser* parser,

                         ParsedResource* out_resource);

  bool ParseStringArray(xml::XmlPullParser* parser,

                        ParsedResource* out_resource);

  bool ParseArrayImpl(xml::XmlPullParser* parser, ParsedResource* out_resource,

                      uint32_t typeMask);

  bool ParseIntegerArray(xml::XmlPullParser* parser, ParsedResource* out_resource);

  bool ParseStringArray(xml::XmlPullParser* parser, ParsedResource* out_resource);

  bool ParseArrayImpl(xml::XmlPullParser* parser, ParsedResource* out_resource, uint32_t typeMask);

  bool ParsePlural(xml::XmlPullParser* parser, ParsedResource* out_resource);



  IDiagnostics* diag_;

tools/aapt2/ResourceParser_test.cpp

+37 −27
Original line number Diff line number Diff line
@@ -25,19 +25,20 @@ 
#include "test/Test.h"

#include "xml/XmlPullParser.h"



using ::aapt::test::ValueEq;

using ::android::StringPiece;

using ::testing::Eq;

using ::testing::NotNull;

using ::testing::Pointee;



namespace aapt {



constexpr const char* kXmlPreamble =

    "<?xml version=\"1.0\" encoding=\"utf-8\"?>\n";

constexpr const char* kXmlPreamble = "<?xml version=\"1.0\" encoding=\"utf-8\"?>\n";



TEST(ResourceParserSingleTest, FailToParseWithNoRootResourcesElement) {

  std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build();

  std::stringstream input(kXmlPreamble);

  input << "<attr name=\"foo\"/>" << std::endl;

  input << R"(<attr name="foo"/>)" << std::endl;

  ResourceTable table;

  ResourceParser parser(context->GetDiagnostics(), &table, Source{"test"}, {});

  xml::XmlPullParser xml_parser(input);
@@ -46,19 +47,20 @@ TEST(ResourceParserSingleTest, FailToParseWithNoRootResourcesElement) { 


class ResourceParserTest : public ::testing::Test {

 public:

  void SetUp() override { context_ = test::ContextBuilder().Build(); }

  void SetUp() override {

    context_ = test::ContextBuilder().Build();

  }



  ::testing::AssertionResult TestParse(const StringPiece& str) {

    return TestParse(str, ConfigDescription{});

  }



  ::testing::AssertionResult TestParse(const StringPiece& str,

                                       const ConfigDescription& config) {

  ::testing::AssertionResult TestParse(const StringPiece& str, const ConfigDescription& config) {

    std::stringstream input(kXmlPreamble);

    input << "<resources>\n" << str << "\n</resources>" << std::endl;

    ResourceParserOptions parserOptions;

    ResourceParser parser(context_->GetDiagnostics(), &table_, Source{"test"},

                          config, parserOptions);

    ResourceParser parser(context_->GetDiagnostics(), &table_, Source{"test"}, config,

                          parserOptions);

    xml::XmlPullParser xmlParser(input);

    if (parser.Parse(&xmlParser)) {

      return ::testing::AssertionSuccess();
@@ -205,18 +207,18 @@ TEST_F(ResourceParserTest, ParseNull) { 
  // a non-existing value, and this causes problems in styles when trying to

  // resolve an attribute. Null values must be encoded as android::Res_value::TYPE_REFERENCE

  // with a data value of 0.

  BinaryPrimitive* integer = test::GetValue<BinaryPrimitive>(&table_, "integer/foo");

  ASSERT_NE(nullptr, integer);

  EXPECT_EQ(uint16_t(android::Res_value::TYPE_REFERENCE), integer->value.dataType);

  EXPECT_EQ(0u, integer->value.data);

  Reference* null_ref = test::GetValue<Reference>(&table_, "integer/foo");

  ASSERT_THAT(null_ref, NotNull());

  EXPECT_FALSE(null_ref->name);

  EXPECT_FALSE(null_ref->id);

  EXPECT_EQ(Reference::Type::kResource, null_ref->reference_type);

}



TEST_F(ResourceParserTest, ParseEmpty) {

  std::string input = "<integer name=\"foo\">@empty</integer>";

  ASSERT_TRUE(TestParse(input));



  BinaryPrimitive* integer =

      test::GetValue<BinaryPrimitive>(&table_, "integer/foo");

  BinaryPrimitive* integer = test::GetValue<BinaryPrimitive>(&table_, "integer/foo");

  ASSERT_NE(nullptr, integer);

  EXPECT_EQ(uint16_t(android::Res_value::TYPE_NULL), integer->value.dataType);

  EXPECT_EQ(uint32_t(android::Res_value::DATA_NULL_EMPTY), integer->value.data);
@@ -241,22 +243,18 @@ TEST_F(ResourceParserTest, ParseAttr) { 
// ultimately

// stored them with the default configuration. Check that we have the same

// behavior.

TEST_F(ResourceParserTest,

       ParseAttrAndDeclareStyleableUnderConfigButRecordAsNoConfig) {

TEST_F(ResourceParserTest, ParseAttrAndDeclareStyleableUnderConfigButRecordAsNoConfig) {

  const ConfigDescription watch_config = test::ParseConfigOrDie("watch");

  std::string input = R"EOF(

  std::string input = R"(

      <attr name="foo" />

      <declare-styleable name="bar">

        <attr name="baz" />

        </declare-styleable>)EOF";

      </declare-styleable>)";

  ASSERT_TRUE(TestParse(input, watch_config));



  EXPECT_EQ(nullptr, test::GetValueForConfig<Attribute>(&table_, "attr/foo",

                                                        watch_config));

  EXPECT_EQ(nullptr, test::GetValueForConfig<Attribute>(&table_, "attr/baz",

                                                        watch_config));

  EXPECT_EQ(nullptr, test::GetValueForConfig<Styleable>(

                         &table_, "styleable/bar", watch_config));

  EXPECT_EQ(nullptr, test::GetValueForConfig<Attribute>(&table_, "attr/foo", watch_config));

  EXPECT_EQ(nullptr, test::GetValueForConfig<Attribute>(&table_, "attr/baz", watch_config));

  EXPECT_EQ(nullptr, test::GetValueForConfig<Styleable>(&table_, "styleable/bar", watch_config));



  EXPECT_NE(nullptr, test::GetValue<Attribute>(&table_, "attr/foo"));

  EXPECT_NE(nullptr, test::GetValue<Attribute>(&table_, "attr/baz"));
@@ -833,4 +831,16 @@ TEST_F(ResourceParserTest, ParseBagElement) { 
  EXPECT_NE(nullptr, ValueCast<RawString>(val->entries[0].value.get()));

}



TEST_F(ResourceParserTest, ParseElementWithNoValue) {

  std::string input = R"(

      <item type="drawable" format="reference" name="foo" />

      <string name="foo" />)";

  ASSERT_TRUE(TestParse(input));

  ASSERT_THAT(test::GetValue(&table_, "drawable/foo"), Pointee(ValueEq(Reference())));



  String* str = test::GetValue<String>(&table_, "string/foo");

  ASSERT_THAT(str, NotNull());

  EXPECT_THAT(*str->value, Eq(""));

}



}  // namespace aapt

tools/aapt2/ResourceUtils.cpp

+41 −37
Original line number Diff line number Diff line
@@ -305,21 +305,25 @@ std::unique_ptr<Reference> TryParseReference(const StringPiece& str, 
  return {};

}



std::unique_ptr<BinaryPrimitive> TryParseNullOrEmpty(const StringPiece& str) {

  StringPiece trimmed_str(util::TrimWhitespace(str));

  android::Res_value value = {};

std::unique_ptr<Item> TryParseNullOrEmpty(const StringPiece& str) {

  const StringPiece trimmed_str(util::TrimWhitespace(str));

  if (trimmed_str == "@null") {

    // TYPE_NULL with data set to 0 is interpreted by the runtime as an error.

    // Instead we set the data type to TYPE_REFERENCE with a value of 0.

    value.dataType = android::Res_value::TYPE_REFERENCE;

    return MakeNull();

  } else if (trimmed_str == "@empty") {

    // TYPE_NULL with value of DATA_NULL_EMPTY is handled fine by the runtime.

    value.dataType = android::Res_value::TYPE_NULL;

    value.data = android::Res_value::DATA_NULL_EMPTY;

  } else {

    return MakeEmpty();

  }

  return {};

}

  return util::make_unique<BinaryPrimitive>(value);



std::unique_ptr<Reference> MakeNull() {

  // TYPE_NULL with data set to 0 is interpreted by the runtime as an error.

  // Instead we set the data type to TYPE_REFERENCE with a value of 0.

  return util::make_unique<Reference>();

}



std::unique_ptr<BinaryPrimitive> MakeEmpty() {

  return util::make_unique<BinaryPrimitive>(android::Res_value::TYPE_NULL,

                                            android::Res_value::DATA_NULL_EMPTY);

}



std::unique_ptr<BinaryPrimitive> TryParseEnumSymbol(const Attribute* enum_attr,
@@ -569,13 +573,15 @@ uint32_t AndroidTypeToAttributeTypeMask(uint16_t type) { 
std::unique_ptr<Item> TryParseItemForAttribute(

    const StringPiece& value, uint32_t type_mask,

    const std::function<void(const ResourceName&)>& on_create_reference) {

  std::unique_ptr<BinaryPrimitive> null_or_empty = TryParseNullOrEmpty(value);

  using android::ResTable_map;



  auto null_or_empty = TryParseNullOrEmpty(value);

  if (null_or_empty) {

    return std::move(null_or_empty);

    return null_or_empty;

  }



  bool create = false;

  std::unique_ptr<Reference> reference = TryParseReference(value, &create);

  auto reference = TryParseReference(value, &create);

  if (reference) {

    if (create && on_create_reference) {

      on_create_reference(reference->name.value());
@@ -583,39 +589,37 @@ std::unique_ptr<Item> TryParseItemForAttribute( 
    return std::move(reference);

  }



  if (type_mask & android::ResTable_map::TYPE_COLOR) {

  if (type_mask & ResTable_map::TYPE_COLOR) {

    // Try parsing this as a color.

    std::unique_ptr<BinaryPrimitive> color = TryParseColor(value);

    auto color = TryParseColor(value);

    if (color) {

      return std::move(color);

    }

  }



  if (type_mask & android::ResTable_map::TYPE_BOOLEAN) {

  if (type_mask & ResTable_map::TYPE_BOOLEAN) {

    // Try parsing this as a boolean.

    std::unique_ptr<BinaryPrimitive> boolean = TryParseBool(value);

    auto boolean = TryParseBool(value);

    if (boolean) {

      return std::move(boolean);

    }

  }



  if (type_mask & android::ResTable_map::TYPE_INTEGER) {

  if (type_mask & ResTable_map::TYPE_INTEGER) {

    // Try parsing this as an integer.

    std::unique_ptr<BinaryPrimitive> integer = TryParseInt(value);

    auto integer = TryParseInt(value);

    if (integer) {

      return std::move(integer);

    }

  }



  const uint32_t float_mask = android::ResTable_map::TYPE_FLOAT |

                              android::ResTable_map::TYPE_DIMENSION |

                              android::ResTable_map::TYPE_FRACTION;

  const uint32_t float_mask =

      ResTable_map::TYPE_FLOAT | ResTable_map::TYPE_DIMENSION | ResTable_map::TYPE_FRACTION;

  if (type_mask & float_mask) {

    // Try parsing this as a float.

    std::unique_ptr<BinaryPrimitive> floating_point = TryParseFloat(value);

    auto floating_point = TryParseFloat(value);

    if (floating_point) {

      if (type_mask &

          AndroidTypeToAttributeTypeMask(floating_point->value.dataType)) {

      if (type_mask & AndroidTypeToAttributeTypeMask(floating_point->value.dataType)) {

        return std::move(floating_point);

      }

    }
@@ -630,24 +634,25 @@ std::unique_ptr<Item> TryParseItemForAttribute( 
std::unique_ptr<Item> TryParseItemForAttribute(

    const StringPiece& str, const Attribute* attr,

    const std::function<void(const ResourceName&)>& on_create_reference) {

  using android::ResTable_map;



  const uint32_t type_mask = attr->type_mask;

  std::unique_ptr<Item> value =

      TryParseItemForAttribute(str, type_mask, on_create_reference);

  auto value = TryParseItemForAttribute(str, type_mask, on_create_reference);

  if (value) {

    return value;

  }



  if (type_mask & android::ResTable_map::TYPE_ENUM) {

  if (type_mask & ResTable_map::TYPE_ENUM) {

    // Try parsing this as an enum.

    std::unique_ptr<BinaryPrimitive> enum_value = TryParseEnumSymbol(attr, str);

    auto enum_value = TryParseEnumSymbol(attr, str);

    if (enum_value) {

      return std::move(enum_value);

    }

  }



  if (type_mask & android::ResTable_map::TYPE_FLAGS) {

  if (type_mask & ResTable_map::TYPE_FLAGS) {

    // Try parsing this as a flag.

    std::unique_ptr<BinaryPrimitive> flag_value = TryParseFlagSymbol(attr, str);

    auto flag_value = TryParseFlagSymbol(attr, str);

    if (flag_value) {

      return std::move(flag_value);

    }
@@ -655,8 +660,7 @@ std::unique_ptr<Item> TryParseItemForAttribute( 
  return {};

}



std::string BuildResourceFileName(const ResourceFile& res_file,

                                  const NameMangler* mangler) {

std::string BuildResourceFileName(const ResourceFile& res_file, const NameMangler* mangler) {

  std::stringstream out;

  out << "res/" << res_file.name.type;

  if (res_file.config != ConfigDescription{}) {
@@ -719,9 +723,9 @@ std::unique_ptr<Item> ParseBinaryResValue(const ResourceType& type, const Config 
        ref_type = Reference::Type::kAttribute;

      }



      if (data == 0) {

      if (data == 0u) {

        // A reference of 0, must be the magic @null reference.

        return util::make_unique<BinaryPrimitive>(android::Res_value::TYPE_REFERENCE, 0u);

        return util::make_unique<Reference>();

      }



      // This is a normal reference.