AAPT2: Better error messages for ManifestFixer (6671fe5c) · Commits · e / os / android_frameworks_base

tools/aapt2/xml/XmlActionExecutor.cpp

+17 −8

Original line number	Original line	Diff line number	Diff line
	@@ -16,6 +16,8 @@

	#include "xml/XmlActionExecutor.h"		#include "xml/XmlActionExecutor.h"

			using ::android::StringPiece;

	namespace aapt {		namespace aapt {
	namespace xml {		namespace xml {

	@@ -46,8 +48,8 @@ static void PrintElementToDiagMessage(const Element* el, DiagMessage* msg) {
	*msg << el->name << ">";		*msg << el->name << ">";
	}		}

	bool XmlNodeAction::Execute(XmlActionExecutorPolicy policy, SourcePathDiagnostics* diag,		bool XmlNodeAction::Execute(XmlActionExecutorPolicy policy, std::vector<StringPiece>* bread_crumb,
	Element* el) const {		SourcePathDiagnostics* diag, Element* el) const {
	bool error = false;		bool error = false;
	for (const ActionFuncWithDiag& action : actions_) {		for (const ActionFuncWithDiag& action : actions_) {
	error \|= !action(el, diag);		error \|= !action(el, diag);
	@@ -57,15 +59,21 @@ bool XmlNodeAction::Execute(XmlActionExecutorPolicy policy, SourcePathDiagnostic
	if (child_el->namespace_uri.empty()) {		if (child_el->namespace_uri.empty()) {
	std::map<std::string, XmlNodeAction>::const_iterator iter = map_.find(child_el->name);		std::map<std::string, XmlNodeAction>::const_iterator iter = map_.find(child_el->name);
	if (iter != map_.end()) {		if (iter != map_.end()) {
	error \|= !iter->second.Execute(policy, diag, child_el);		// Use the iterator's copy of the element name, because the element may be modified.
			bread_crumb->push_back(iter->first);
			error \|= !iter->second.Execute(policy, bread_crumb, diag, child_el);
			bread_crumb->pop_back();
	continue;		continue;
	}		}

	if (policy == XmlActionExecutorPolicy::kWhitelist) {		if (policy == XmlActionExecutorPolicy::kWhitelist) {
	DiagMessage error_msg(child_el->line_number);		DiagMessage error_msg(child_el->line_number);
	error_msg << "unknown element ";		error_msg << "unexpected element ";
	PrintElementToDiagMessage(child_el, &error_msg);		PrintElementToDiagMessage(child_el, &error_msg);
	error_msg << " found";		error_msg << " found in ";
			for (const StringPiece& element : *bread_crumb) {
			error_msg << "<" << element << ">";
			}
	diag->Error(error_msg);		diag->Error(error_msg);
	error = true;		error = true;
	}		}
	@@ -90,14 +98,15 @@ bool XmlActionExecutor::Execute(XmlActionExecutorPolicy policy, IDiagnostics* di
	if (el->namespace_uri.empty()) {		if (el->namespace_uri.empty()) {
	std::map<std::string, XmlNodeAction>::const_iterator iter = map_.find(el->name);		std::map<std::string, XmlNodeAction>::const_iterator iter = map_.find(el->name);
	if (iter != map_.end()) {		if (iter != map_.end()) {
	return iter->second.Execute(policy, &source_diag, el);		std::vector<StringPiece> bread_crumb;
			bread_crumb.push_back(iter->first);
			return iter->second.Execute(policy, &bread_crumb, &source_diag, el);
	}		}

	if (policy == XmlActionExecutorPolicy::kWhitelist) {		if (policy == XmlActionExecutorPolicy::kWhitelist) {
	DiagMessage error_msg(el->line_number);		DiagMessage error_msg(el->line_number);
	error_msg << "unknown element ";		error_msg << "unexpected root element ";
	PrintElementToDiagMessage(el, &error_msg);		PrintElementToDiagMessage(el, &error_msg);
	error_msg << " found";
	source_diag.Error(error_msg);		source_diag.Error(error_msg);
	return false;		return false;
	}		}

tools/aapt2/xml/XmlActionExecutor.h

+18 −28

Original line number	Original line	Diff line number	Diff line
	@@ -40,56 +40,46 @@ enum class XmlActionExecutorPolicy {
	kWhitelist,		kWhitelist,
	};		};

	/**		// Contains the actions to perform at this XML node. This is a recursive data structure that
	* Contains the actions to perform at this XML node. This is a recursive data		// holds XmlNodeActions for child XML nodes.
	* structure that
	* holds XmlNodeActions for child XML nodes.
	*/
	class XmlNodeAction {		class XmlNodeAction {
	public:		public:
	using ActionFuncWithDiag = std::function<bool(Element, SourcePathDiagnostics)>;		using ActionFuncWithDiag = std::function<bool(Element, SourcePathDiagnostics)>;
	using ActionFunc = std::function<bool(Element*)>;		using ActionFunc = std::function<bool(Element*)>;

	/**		// Find or create a child XmlNodeAction that will be performed for the child element with the
	* Find or create a child XmlNodeAction that will be performed for the child		// name `name`.
	* element with the name `name`.		XmlNodeAction& operator[](const std::string& name) {
	*/		return map_[name];
	XmlNodeAction& operator[](const std::string& name) { return map_[name]; }		}

	/**		// Add an action to be performed at this XmlNodeAction.
	* Add an action to be performed at this XmlNodeAction.
	*/
	void Action(ActionFunc f);		void Action(ActionFunc f);
	void Action(ActionFuncWithDiag);		void Action(ActionFuncWithDiag);

	private:		private:
	friend class XmlActionExecutor;		friend class XmlActionExecutor;

	bool Execute(XmlActionExecutorPolicy policy, SourcePathDiagnostics* diag, Element* el) const;		bool Execute(XmlActionExecutorPolicy policy, std::vector<::android::StringPiece>* bread_crumb,
			SourcePathDiagnostics* diag, Element* el) const;

	std::map<std::string, XmlNodeAction> map_;		std::map<std::string, XmlNodeAction> map_;
	std::vector<ActionFuncWithDiag> actions_;		std::vector<ActionFuncWithDiag> actions_;
	};		};

	/**		// Allows the definition of actions to execute at specific XML elements defined by their hierarchy.
	* Allows the definition of actions to execute at specific XML elements defined
	* by their
	* hierarchy.
	*/
	class XmlActionExecutor {		class XmlActionExecutor {
	public:		public:
	XmlActionExecutor() = default;		XmlActionExecutor() = default;

	/**		// Find or create a root XmlNodeAction that will be performed for the root XML element with the
	* Find or create a root XmlNodeAction that will be performed for the root XML		// name `name`.
	* element with the name `name`.		XmlNodeAction& operator[](const std::string& name) {
	*/		return map_[name];
	XmlNodeAction& operator[](const std::string& name) { return map_[name]; }		}

	/**		// Execute the defined actions for this XmlResource.
	* Execute the defined actions for this XmlResource.		// Returns true if all actions return true, otherwise returns false.
	* Returns true if all actions return true, otherwise returns false.
	*/
	bool Execute(XmlActionExecutorPolicy policy, IDiagnostics* diag, XmlResource* doc) const;		bool Execute(XmlActionExecutorPolicy policy, IDiagnostics* diag, XmlResource* doc) const;

	private:		private:

tools/aapt2/xml/XmlActionExecutor_test.cpp

+6 −2

Original line number	Original line	Diff line number	Diff line
	@@ -56,9 +56,13 @@ TEST(XmlActionExecutorTest, FailsWhenUndefinedHierarchyExists) {
	XmlActionExecutor executor;		XmlActionExecutor executor;
	executor["manifest"]["application"];		executor["manifest"]["application"];

	std::unique_ptr<XmlResource> doc =		std::unique_ptr<XmlResource> doc;
	test::BuildXmlDom("<manifest><application /><activity /></manifest>");
	StdErrDiagnostics diag;		StdErrDiagnostics diag;

			doc = test::BuildXmlDom("<manifest><application /><activity /></manifest>");
			ASSERT_FALSE(executor.Execute(XmlActionExecutorPolicy::kWhitelist, &diag, doc.get()));

			doc = test::BuildXmlDom("<manifest><application><activity /></application></manifest>");
	ASSERT_FALSE(executor.Execute(XmlActionExecutorPolicy::kWhitelist, &diag, doc.get()));		ASSERT_FALSE(executor.Execute(XmlActionExecutorPolicy::kWhitelist, &diag, doc.get()));
	}		}