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

Commit 27afb9e8 authored by Adam Lesinski's avatar Adam Lesinski
Browse files

AAPT2: Add support for clearer in-progress public attributes 

Before, the ID assigned to a public resource without an explicitly set id
was more difficult to figure out. It would be the next available ID.

AAPT2 introduces a new way to specify public attributes in progress.

<public-group type="attr" first-id="0x0101047f">
  <public name="foo" />
  <public name="bar" />
  ...
</public-group>

The IDs assigned to each resource is auto-incremented starting from `first-id`.
This also keeps resource's with the same type grouped together so that
the auto-incrementing nature is evident.

Also, due to how AAPT2 was implemented, this is required :P

Change-Id: I95ea92ad0405e87ed0b1766879bb2f1d9d0b636e
parent b274e35a

tools/aapt2/ResourceParser.cpp

+116 −27
Original line number Diff line number Diff line
@@ -49,6 +49,13 @@ static Maybe<StringPiece16> findNonEmptyAttribute(XmlPullParser* parser, 
    return {};

}



/**

 * Returns true if the element is <skip> or <eat-comment> and can be safely ignored.

 */

static bool shouldIgnoreElement(const StringPiece16& ns, const StringPiece16& name) {

    return ns.empty() && (name == u"skip" || name == u"eat-comment");

}



ResourceParser::ResourceParser(IDiagnostics* diag, ResourceTable* table, const Source& source,

                               const ConfigDescription& config,

                               const ResourceParserOptions& options) :
@@ -205,10 +212,7 @@ static bool addResourcesToTable(ResourceTable* table, const ConfigDescription& c 
        }

    }



    if (!res->value) {

        return true;

    }



    if (res->value) {

        // Attach the comment, source and config to the value.

        res->value->setComment(std::move(res->comment));

        res->value->setSource(std::move(res->source));
@@ -216,6 +220,7 @@ static bool addResourcesToTable(ResourceTable* table, const ConfigDescription& c 
        if (!table->addResource(res->name, res->id, config, std::move(res->value), diag)) {

            return false;

        }

    }



    bool error = false;

    for (ParsedResource& child : res->childResources) {
@@ -259,17 +264,6 @@ bool ResourceParser::parseResources(XmlPullParser* parser) { 
            continue;

        }



        Maybe<StringPiece16> maybeName = findNonEmptyAttribute(parser, u"name");

        if (!maybeName) {

            mDiag->error(DiagMessage(mSource.withLine(parser->getLineNumber()))

                         << "<" << elementName << "> tag must have a 'name' attribute");

            error = true;

            continue;

        }



        // Check if we should skip this product.

        const bool stripResource = shouldStripResource(parser, mOptions.product);



        if (elementName == u"item") {

            // Items simply have their type encoded in the type attribute.

            if (Maybe<StringPiece16> maybeType = findNonEmptyAttribute(parser, u"type")) {
@@ -283,10 +277,22 @@ bool ResourceParser::parseResources(XmlPullParser* parser) { 
        }



        ParsedResource parsedResource;

        parsedResource.name.entry = maybeName.value().toString();

        parsedResource.source = mSource.withLine(parser->getLineNumber());

        parsedResource.comment = std::move(comment);



        if (Maybe<StringPiece16> maybeName = findNonEmptyAttribute(parser, u"name")) {

            parsedResource.name.entry = maybeName.value().toString();



        } else if (elementName != u"public-group") {

            mDiag->error(DiagMessage(mSource.withLine(parser->getLineNumber()))

                         << "<" << elementName << "> tag must have a 'name' attribute");

            error = true;

            continue;

        }



        // Check if we should skip this product.

        const bool stripResource = shouldStripResource(parser, mOptions.product);



        bool result = true;

        if (elementName == u"id") {

            parsedResource.name.type = ResourceType::kId;
@@ -337,6 +343,8 @@ bool ResourceParser::parseResources(XmlPullParser* parser) { 
            result = parsePublic(parser, &parsedResource);

        } else if (elementName == u"java-symbol" || elementName == u"symbol") {

            result = parseSymbol(parser, &parsedResource);

        } else if (elementName == u"public-group") {

            result = parsePublicGroup(parser, &parsedResource);

        } else {

            mDiag->warn(DiagMessage(mSource.withLine(parser->getLineNumber()))

                        << "unknown resource type '" << elementName << "'");
@@ -559,6 +567,92 @@ bool ResourceParser::parsePublic(XmlPullParser* parser, ParsedResource* outResou 
    return true;

}



bool ResourceParser::parsePublicGroup(XmlPullParser* parser, ParsedResource* outResource) {

    const Source source = mSource.withLine(parser->getLineNumber());



    Maybe<StringPiece16> maybeType = findNonEmptyAttribute(parser, u"type");

    if (!maybeType) {

        mDiag->error(DiagMessage(source) << "<public-group> must have a 'type' attribute");

        return false;

    }



    const ResourceType* parsedType = parseResourceType(maybeType.value());

    if (!parsedType) {

        mDiag->error(DiagMessage(source) << "invalid resource type '" << maybeType.value()

                     << "' in <public-group>");

        return false;

    }



    Maybe<StringPiece16> maybeId = findNonEmptyAttribute(parser, u"first-id");

    if (!maybeId) {

        mDiag->error(DiagMessage(source) << "<public-group> must have a 'first-id' attribute");

        return false;

    }



    android::Res_value val;

    bool result = android::ResTable::stringToInt(maybeId.value().data(),

                                                 maybeId.value().size(), &val);

    ResourceId nextId(val.data);

    if (!result || !nextId.isValid()) {

        mDiag->error(DiagMessage(source) << "invalid resource ID '" << maybeId.value()

                     << "' in <public-group>");

        return false;

    }



    std::u16string comment;

    bool error = false;

    const size_t depth = parser->getDepth();

    while (XmlPullParser::nextChildNode(parser, depth)) {

        if (parser->getEvent() == XmlPullParser::Event::kComment) {

            comment = util::trimWhitespace(parser->getComment()).toString();

            continue;

        } else if (parser->getEvent() != XmlPullParser::Event::kStartElement) {

            // Skip text.

            continue;

        }



        const Source itemSource = mSource.withLine(parser->getLineNumber());

        const std::u16string& elementNamespace = parser->getElementNamespace();

        const std::u16string& elementName = parser->getElementName();

        if (elementNamespace.empty() && elementName == u"public") {

            Maybe<StringPiece16> maybeName = findNonEmptyAttribute(parser, u"name");

            if (!maybeName) {

                mDiag->error(DiagMessage(itemSource) << "<public> must have a 'name' attribute");

                error = true;

                continue;

            }



            if (findNonEmptyAttribute(parser, u"id")) {

                mDiag->error(DiagMessage(itemSource) << "'id' is ignored within <public-group>");

                error = true;

                continue;

            }



            if (findNonEmptyAttribute(parser, u"type")) {

                mDiag->error(DiagMessage(itemSource) << "'type' is ignored within <public-group>");

                error = true;

                continue;

            }



            ParsedResource childResource;

            childResource.name.type = *parsedType;

            childResource.name.entry = maybeName.value().toString();

            childResource.id = nextId;

            childResource.comment = std::move(comment);

            childResource.source = itemSource;

            childResource.symbolState = SymbolState::kPublic;

            outResource->childResources.push_back(std::move(childResource));



            nextId.id += 1;



        } else if (!shouldIgnoreElement(elementNamespace, elementName)) {

            mDiag->error(DiagMessage(itemSource) << ":" << elementName << ">");

            error = true;

        }

    }

    return !error;

}



bool ResourceParser::parseSymbol(XmlPullParser* parser, ParsedResource* outResource) {

    const Source source = mSource.withLine(parser->getLineNumber());
@@ -608,12 +702,7 @@ static uint32_t parseFormatAttribute(const StringPiece16& str) { 
    return mask;

}



/**

 * Returns true if the element is <skip> or <eat-comment> and can be safely ignored.

 */

static bool shouldIgnoreElement(const StringPiece16& ns, const StringPiece16& name) {

    return ns.empty() && (name == u"skip" || name == u"eat-comment");

}





bool ResourceParser::parseAttr(XmlPullParser* parser, ParsedResource* outResource) {

    outResource->source = mSource.withLine(parser->getLineNumber());

tools/aapt2/ResourceParser.h

+1 −0
Original line number Diff line number Diff line
@@ -83,6 +83,7 @@ private: 
    bool parseColor(XmlPullParser* parser, ParsedResource* outResource);

    bool parsePrimitive(XmlPullParser* parser, ParsedResource* outResource);

    bool parsePublic(XmlPullParser* parser, ParsedResource* outResource);

    bool parsePublicGroup(XmlPullParser* parser, ParsedResource* outResource);

    bool parseSymbol(XmlPullParser* parser, ParsedResource* outResource);

    bool parseAttr(XmlPullParser* parser, ParsedResource* outResource);

    bool parseAttrImpl(XmlPullParser* parser, ParsedResource* outResource, bool weak);

tools/aapt2/ResourceParser_test.cpp

+32 −0
Original line number Diff line number Diff line
@@ -489,4 +489,36 @@ TEST_F(ResourceParserTest, FailWhenProductFilterStripsOutAllVersionsOfResource) 
    ASSERT_FALSE(testParse(input, std::u16string(u"phone")));

}



TEST_F(ResourceParserTest, AutoIncrementIdsInPublicGroup) {

    std::string input = R"EOF(

    <public-group type="attr" first-id="0x01010040">

      <public name="foo" />

      <public name="bar" />

    </public-group>)EOF";

    ASSERT_TRUE(testParse(input));



    Maybe<ResourceTable::SearchResult> result = mTable.findResource(

            test::parseNameOrDie(u"@attr/foo"));

    AAPT_ASSERT_TRUE(result);



    AAPT_ASSERT_TRUE(result.value().package->id);

    AAPT_ASSERT_TRUE(result.value().type->id);

    AAPT_ASSERT_TRUE(result.value().entry->id);

    ResourceId actualId(result.value().package->id.value(),

                        result.value().type->id.value(),

                        result.value().entry->id.value());

    EXPECT_EQ(ResourceId(0x01010040), actualId);



    result = mTable.findResource(test::parseNameOrDie(u"@attr/bar"));

    AAPT_ASSERT_TRUE(result);



    AAPT_ASSERT_TRUE(result.value().package->id);

    AAPT_ASSERT_TRUE(result.value().type->id);

    AAPT_ASSERT_TRUE(result.value().entry->id);

    actualId = ResourceId(result.value().package->id.value(),

                          result.value().type->id.value(),

                          result.value().entry->id.value());

    EXPECT_EQ(ResourceId(0x01010041), actualId);

}



} // namespace aapt