Merge "AAPT2: Move comments and source into Value" (030cfd3d) · Commits · e / os / android_frameworks_base

tools/aapt2/JavaClassGenerator_test.cpp

+2 −4

Original line number	Diff line number	Diff line
		@@ -105,11 +105,9 @@ TEST(JavaClassGeneratorTest, OnlyWritePublicResources) {
		.addSimple(u"@android:id/one", ResourceId(0x01020000))
		.addSimple(u"@android:id/two", ResourceId(0x01020001))
		.addSimple(u"@android:id/three", ResourceId(0x01020002))
		.setSymbolState(u"@android:id/one", ResourceId(0x01020000), SymbolState::kPublic)
		.setSymbolState(u"@android:id/two", ResourceId(0x01020001), SymbolState::kPrivate)
		.build();
		ASSERT_TRUE(table->setSymbolState(test::parseNameOrDie(u"@android:id/one"), {}, {},
		SymbolState::kPublic, &diag));
		ASSERT_TRUE(table->setSymbolState(test::parseNameOrDie(u"@android:id/two"), {}, {},
		SymbolState::kPrivate, &diag));

		JavaClassGeneratorOptions options;
		options.types = JavaClassGeneratorOptions::SymbolTypes::kPublic;

tools/aapt2/ResourceParser.cpp

+21 −9

Original line number	Diff line number	Diff line
		@@ -186,6 +186,7 @@ struct ParsedResource {
		Source source;
		ResourceId id;
		SymbolState symbolState = SymbolState::kUndefined;
		std::u16string comment;
		std::unique_ptr<Value> value;
		std::list<ParsedResource> childResources;
		};
		@@ -194,7 +195,11 @@ struct ParsedResource {
		static bool addResourcesToTable(ResourceTable* table, const ConfigDescription& config,
		IDiagnostics* diag, ParsedResource* res) {
		if (res->symbolState != SymbolState::kUndefined) {
		if (!table->setSymbolState(res->name, res->id, res->source, res->symbolState, diag)) {
		Symbol symbol;
		symbol.state = res->symbolState;
		symbol.source = res->source;
		symbol.comment = res->comment;
		if (!table->setSymbolState(res->name, res->id, symbol, diag)) {
		return false;
		}
		}
		@@ -203,7 +208,11 @@ static bool addResourcesToTable(ResourceTable* table, const ConfigDescription& c
		return true;
		}

		if (!table->addResource(res->name, res->id, config, res->source, std::move(res->value), diag)) {
		// Attach the comment, source and config to the value.
		res->value->setComment(std::move(res->comment));
		res->value->setSource(std::move(res->source));

		if (!table->addResource(res->name, res->id, config, std::move(res->value), diag)) {
		return false;
		}

		@@ -275,6 +284,7 @@ bool ResourceParser::parseResources(XmlPullParser* parser) {
		ParsedResource parsedResource;
		parsedResource.name.entry = maybeName.value().toString();
		parsedResource.source = mSource.withLine(parser->getLineNumber());
		parsedResource.comment = std::move(comment);

		bool result = true;
		if (elementName == u"id") {
		@@ -368,8 +378,8 @@ enum {
		* an Item. If allowRawValue is false, nullptr is returned in this
		* case.
		*/
		std::unique_ptr<Item> ResourceParser::parseXml(XmlPullParser* parser, uint32_t typeMask,
		bool allowRawValue) {
		std::unique_ptr<Item> ResourceParser::parseXml(XmlPullParser* parser, const uint32_t typeMask,
		const bool allowRawValue) {
		const size_t beginXmlLine = parser->getLineNumber();

		std::u16string rawValue;
		@@ -386,8 +396,9 @@ std::unique_ptr<Item> ResourceParser::parseXml(XmlPullParser* parser, uint32_t t

		auto onCreateReference = [&](const ResourceName& name) {
		// name.package can be empty here, as it will assume the package name of the table.
		mTable->addResource(name, {}, mSource.withLine(beginXmlLine), util::make_unique<Id>(),
		mDiag);
		std::unique_ptr<Id> id = util::make_unique<Id>();
		id->setSource(mSource.withLine(beginXmlLine));
		mTable->addResource(name, {}, std::move(id), mDiag);
		};

		// Process the raw value.
		@@ -411,11 +422,12 @@ std::unique_ptr<Item> ResourceParser::parseXml(XmlPullParser* parser, uint32_t t
		mTable->stringPool.makeRef(styleString.str, StringPool::Context{ 1, mConfig }));
		}

		// We can't parse this so return a RawString if we are allowed.
		if (allowRawValue) {
		// We can't parse this so return a RawString if we are allowed.
		return util::make_unique<RawString>(
		mTable->stringPool.makeRef(rawValue, StringPool::Context{ 1, mConfig }));
		}

		return {};
		}

		@@ -683,7 +695,7 @@ Maybe<Attribute::Symbol> ResourceParser::parseEnumOrFlagItem(XmlPullParser* pars
		}

		return Attribute::Symbol{
		Reference(ResourceName{ {}, ResourceType::kId, maybeName.value().toString() }),
		Reference(ResourceName({}, ResourceType::kId, maybeName.value().toString())),
		val.data };
		}

tools/aapt2/ResourceParser.h

+6 −5

Original line number	Diff line number	Diff line
		@@ -65,12 +65,13 @@ private:
		StyleString* outStyleString);

		/*
		* Parses the XML subtree and converts it to an Item. The type of Item that can be
		* parsed is denoted by the `typeMask`. If `allowRawValue` is true and the subtree
		* can not be parsed as a regular Item, then a RawString is returned. Otherwise
		* this returns nullptr.
		* Parses the XML subtree and returns an Item.
		* The type of Item that can be parsed is denoted by the `typeMask`.
		* If `allowRawValue` is true and the subtree can not be parsed as a regular Item, then a
		* RawString is returned. Otherwise this returns false;
		*/
		std::unique_ptr<Item> parseXml(XmlPullParser* parser, uint32_t typeMask, bool allowRawValue);
		std::unique_ptr<Item> parseXml(XmlPullParser* parser, const uint32_t typeMask,
		const bool allowRawValue);

		bool parseResources(XmlPullParser* parser);
		bool parseString(XmlPullParser* parser, ParsedResource* outResource);

tools/aapt2/ResourceParser_test.cpp

+29 −8

Original line number	Diff line number	Diff line
		@@ -383,14 +383,35 @@ TEST_F(ResourceParserTest, ParseCommentsWithResource) {
		"<string name=\"foo\">Hi</string>";
		ASSERT_TRUE(testParse(input));

		Maybe<ResourceTable::SearchResult> result = mTable.findResource(
		test::parseNameOrDie(u"@string/foo"));
		AAPT_ASSERT_TRUE(result);

		ResourceEntry* entry = result.value().entry;
		ASSERT_NE(entry, nullptr);
		ASSERT_FALSE(entry->values.empty());
		EXPECT_EQ(entry->values.front().comment, u"This is a comment");
		String* value = test::getValue<String>(&mTable, u"@string/foo");
		ASSERT_NE(nullptr, value);
		EXPECT_EQ(value->getComment(), u"This is a comment");
		}

		TEST_F(ResourceParserTest, DoNotCombineMultipleComments) {
		std::string input = "<!--One-->\n"
		"<!--Two-->\n"
		"<string name=\"foo\">Hi</string>";

		ASSERT_TRUE(testParse(input));

		String* value = test::getValue<String>(&mTable, u"@string/foo");
		ASSERT_NE(nullptr, value);
		EXPECT_EQ(value->getComment(), u"Two");
		}

		TEST_F(ResourceParserTest, IgnoreCommentBeforeEndTag) {
		std::string input = "<!--One-->\n"
		"<string name=\"foo\">\n"
		" Hi\n"
		"<!--Two-->\n"
		"</string>";

		ASSERT_TRUE(testParse(input));

		String* value = test::getValue<String>(&mTable, u"@string/foo");
		ASSERT_NE(nullptr, value);
		EXPECT_EQ(value->getComment(), u"One");
		}

		/*

tools/aapt2/ResourceTable.cpp

+47 −50

Original line number	Diff line number	Diff line
		@@ -19,6 +19,8 @@
		#include "ResourceTable.h"
		#include "ResourceValues.h"
		#include "ValueVisitor.h"

		#include "util/Comparators.h"
		#include "util/Util.h"

		#include <algorithm>
		@@ -29,10 +31,6 @@

		namespace aapt {

		static bool compareConfigs(const ResourceConfigValue& lhs, const ConfigDescription& rhs) {
		return lhs.config < rhs;
		}

		static bool lessThanType(const std::unique_ptr<ResourceTableType>& lhs, ResourceType rhs) {
		return lhs->type < rhs;
		}
		@@ -191,52 +189,50 @@ static constexpr const char16_t* kValidNameChars = u"._-";
		static constexpr const char16_t* kValidNameMangledChars = u"._-$";

		bool ResourceTable::addResource(const ResourceNameRef& name, const ConfigDescription& config,
		const Source& source, std::unique_ptr<Value> value,
		IDiagnostics* diag) {
		return addResourceImpl(name, ResourceId{}, config, source, std::move(value), kValidNameChars,
		diag);
		std::unique_ptr<Value> value, IDiagnostics* diag) {
		return addResourceImpl(name, {}, config, std::move(value), kValidNameChars, diag);
		}

		bool ResourceTable::addResource(const ResourceNameRef& name, const ResourceId resId,
		const ConfigDescription& config, const Source& source,
		std::unique_ptr<Value> value, IDiagnostics* diag) {
		return addResourceImpl(name, resId, config, source, std::move(value), kValidNameChars, diag);
		const ConfigDescription& config, std::unique_ptr<Value> value,
		IDiagnostics* diag) {
		return addResourceImpl(name, resId, config, std::move(value), kValidNameChars, diag);
		}

		bool ResourceTable::addFileReference(const ResourceNameRef& name, const ConfigDescription& config,
		const Source& source, const StringPiece16& path,
		IDiagnostics* diag) {
		return addResourceImpl(name, ResourceId{}, config, source,
		util::make_unique<FileReference>(stringPool.makeRef(path)),
		kValidNameChars, diag);
		std::unique_ptr<FileReference> fileRef = util::make_unique<FileReference>(
		stringPool.makeRef(path));
		fileRef->setSource(source);
		return addResourceImpl(name, ResourceId{}, config, std::move(fileRef), kValidNameChars, diag);
		}

		bool ResourceTable::addResourceAllowMangled(const ResourceNameRef& name,
		const ConfigDescription& config,
		const Source& source,
		std::unique_ptr<Value> value,
		IDiagnostics* diag) {
		return addResourceImpl(name, ResourceId{}, config, source, std::move(value),
		kValidNameMangledChars, diag);
		return addResourceImpl(name, ResourceId{}, config, std::move(value), kValidNameMangledChars,
		diag);
		}

		bool ResourceTable::addResourceAllowMangled(const ResourceNameRef& name,
		const ResourceId id,
		const ConfigDescription& config,
		const Source& source,
		std::unique_ptr<Value> value,
		IDiagnostics* diag) {
		return addResourceImpl(name, id, config, source, std::move(value),
		kValidNameMangledChars, diag);
		return addResourceImpl(name, id, config, std::move(value), kValidNameMangledChars, diag);
		}

		bool ResourceTable::addResourceImpl(const ResourceNameRef& name, const ResourceId resId,
		const ConfigDescription& config, const Source& source,
		std::unique_ptr<Value> value, const char16_t* validChars,
		IDiagnostics* diag) {
		const ConfigDescription& config, std::unique_ptr<Value> value,
		const char16_t* validChars, IDiagnostics* diag) {
		assert(value && "value can't be nullptr");
		assert(diag && "diagnostics can't be nullptr");

		auto badCharIter = util::findNonAlphaNumericAndNotInSet(name.entry, validChars);
		if (badCharIter != name.entry.end()) {
		diag->error(DiagMessage(source)
		diag->error(DiagMessage(value->getSource())
		<< "resource '"
		<< name
		<< "' has invalid entry name '"
		@@ -249,7 +245,7 @@ bool ResourceTable::addResourceImpl(const ResourceNameRef& name, const ResourceI

		ResourceTablePackage* package = findOrCreatePackage(name.package);
		if (resId.isValid() && package->id && package->id.value() != resId.packageId()) {
		diag->error(DiagMessage(source)
		diag->error(DiagMessage(value->getSource())
		<< "trying to add resource '"
		<< name
		<< "' with ID "
		@@ -263,7 +259,7 @@ bool ResourceTable::addResourceImpl(const ResourceNameRef& name, const ResourceI

		ResourceTableType* type = package->findOrCreateType(name.type);
		if (resId.isValid() && type->id && type->id.value() != resId.typeId()) {
		diag->error(DiagMessage(source)
		diag->error(DiagMessage(value->getSource())
		<< "trying to add resource '"
		<< name
		<< "' with ID "
		@@ -277,7 +273,7 @@ bool ResourceTable::addResourceImpl(const ResourceNameRef& name, const ResourceI

		ResourceEntry* entry = type->findOrCreateEntry(name.entry);
		if (resId.isValid() && entry->id && entry->id.value() != resId.entryId()) {
		diag->error(DiagMessage(source)
		diag->error(DiagMessage(value->getSource())
		<< "trying to add resource '"
		<< name
		<< "' with ID "
		@@ -288,20 +284,20 @@ bool ResourceTable::addResourceImpl(const ResourceNameRef& name, const ResourceI
		}

		const auto endIter = entry->values.end();
		auto iter = std::lower_bound(entry->values.begin(), endIter, config, compareConfigs);
		auto iter = std::lower_bound(entry->values.begin(), endIter, config, cmp::lessThan);
		if (iter == endIter \|\| iter->config != config) {
		// This resource did not exist before, add it.
		entry->values.insert(iter, ResourceConfigValue{ config, source, {}, std::move(value) });
		entry->values.insert(iter, ResourceConfigValue{ config, std::move(value) });
		} else {
		int collisionResult = resolveValueCollision(iter->value.get(), value.get());
		if (collisionResult > 0) {
		// Take the incoming value.
		*iter = ResourceConfigValue{ config, source, {}, std::move(value) };
		iter->value = std::move(value);
		} else if (collisionResult == 0) {
		diag->error(DiagMessage(source)
		diag->error(DiagMessage(value->getSource())
		<< "duplicate value for resource '" << name << "' "
		<< "with config '" << iter->config << "'");
		diag->error(DiagMessage(iter->source)
		<< "with config '" << config << "'");
		diag->error(DiagMessage(iter->value->getSource())
		<< "resource previously defined here");
		return false;
		}
		@@ -316,27 +312,29 @@ bool ResourceTable::addResourceImpl(const ResourceNameRef& name, const ResourceI
		}

		bool ResourceTable::setSymbolState(const ResourceNameRef& name, const ResourceId resId,
		const Source& source, SymbolState state, IDiagnostics* diag) {
		return setSymbolStateImpl(name, resId, source, state, kValidNameChars, diag);
		const Symbol& symbol, IDiagnostics* diag) {
		return setSymbolStateImpl(name, resId, symbol, kValidNameChars, diag);
		}

		bool ResourceTable::setSymbolStateAllowMangled(const ResourceNameRef& name, const ResourceId resId,
		const Source& source, SymbolState state,
		IDiagnostics* diag) {
		return setSymbolStateImpl(name, resId, source, state, kValidNameMangledChars, diag);
		bool ResourceTable::setSymbolStateAllowMangled(const ResourceNameRef& name,
		const ResourceId resId,
		const Symbol& symbol, IDiagnostics* diag) {
		return setSymbolStateImpl(name, resId, symbol, kValidNameMangledChars, diag);
		}

		bool ResourceTable::setSymbolStateImpl(const ResourceNameRef& name, const ResourceId resId,
		const Source& source, SymbolState state,
		const char16_t* validChars, IDiagnostics* diag) {
		if (state == SymbolState::kUndefined) {
		const Symbol& symbol, const char16_t* validChars,
		IDiagnostics* diag) {
		assert(diag && "diagnostics can't be nullptr");

		if (symbol.state == SymbolState::kUndefined) {
		// Nothing to do.
		return true;
		}

		auto badCharIter = util::findNonAlphaNumericAndNotInSet(name.entry, validChars);
		if (badCharIter != name.entry.end()) {
		diag->error(DiagMessage(source)
		diag->error(DiagMessage(symbol.source)
		<< "resource '"
		<< name
		<< "' has invalid entry name '"
		@@ -349,7 +347,7 @@ bool ResourceTable::setSymbolStateImpl(const ResourceNameRef& name, const Resour

		ResourceTablePackage* package = findOrCreatePackage(name.package);
		if (resId.isValid() && package->id && package->id.value() != resId.packageId()) {
		diag->error(DiagMessage(source)
		diag->error(DiagMessage(symbol.source)
		<< "trying to add resource '"
		<< name
		<< "' with ID "
		@@ -363,7 +361,7 @@ bool ResourceTable::setSymbolStateImpl(const ResourceNameRef& name, const Resour

		ResourceTableType* type = package->findOrCreateType(name.type);
		if (resId.isValid() && type->id && type->id.value() != resId.typeId()) {
		diag->error(DiagMessage(source)
		diag->error(DiagMessage(symbol.source)
		<< "trying to add resource '"
		<< name
		<< "' with ID "
		@@ -377,7 +375,7 @@ bool ResourceTable::setSymbolStateImpl(const ResourceNameRef& name, const Resour

		ResourceEntry* entry = type->findOrCreateEntry(name.entry);
		if (resId.isValid() && entry->id && entry->id.value() != resId.entryId()) {
		diag->error(DiagMessage(source)
		diag->error(DiagMessage(symbol.source)
		<< "trying to add resource '"
		<< name
		<< "' with ID "
		@@ -388,15 +386,14 @@ bool ResourceTable::setSymbolStateImpl(const ResourceNameRef& name, const Resour
		}

		// Only mark the type state as public, it doesn't care about being private.
		if (state == SymbolState::kPublic) {
		if (symbol.state == SymbolState::kPublic) {
		type->symbolStatus.state = SymbolState::kPublic;
		}

		// Downgrading to a private symbol from a public one is not allowed.
		if (entry->symbolStatus.state != SymbolState::kPublic) {
		if (entry->symbolStatus.state != state) {
		entry->symbolStatus.state = state;
		entry->symbolStatus.source = source;
		if (entry->symbolStatus.state != symbol.state) {
		entry->symbolStatus = std::move(symbol);
		}
		}