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

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

AAPT2: Better error messages for ManifestFixer 

AAPT2 will now print the XML hierarchy where it found an unexpected
element.

Test: make aapt2_tests
Change-Id: Iac7918b2f344fab874f0a3e7aa9c6936ecde8913
parent 4f340a4f

tools/aapt2/xml/XmlActionExecutor.cpp

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


#include "xml/XmlActionExecutor.h"



using ::android::StringPiece;



namespace aapt {

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

}



bool XmlNodeAction::Execute(XmlActionExecutorPolicy policy, SourcePathDiagnostics* diag,

                            Element* el) const {

bool XmlNodeAction::Execute(XmlActionExecutorPolicy policy, std::vector<StringPiece>* bread_crumb,

                            SourcePathDiagnostics* diag, Element* el) const {

  bool error = false;

  for (const ActionFuncWithDiag& action : actions_) {

    error |= !action(el, diag);
@@ -57,15 +59,21 @@ bool XmlNodeAction::Execute(XmlActionExecutorPolicy policy, SourcePathDiagnostic 
    if (child_el->namespace_uri.empty()) {

      std::map<std::string, XmlNodeAction>::const_iterator iter = map_.find(child_el->name);

      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;

      }



      if (policy == XmlActionExecutorPolicy::kWhitelist) {

        DiagMessage error_msg(child_el->line_number);

        error_msg << "unknown element ";

        error_msg << "unexpected element ";

        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);

        error = true;

      }
@@ -90,14 +98,15 @@ bool XmlActionExecutor::Execute(XmlActionExecutorPolicy policy, IDiagnostics* di 
  if (el->namespace_uri.empty()) {

    std::map<std::string, XmlNodeAction>::const_iterator iter = map_.find(el->name);

    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) {

      DiagMessage error_msg(el->line_number);

      error_msg << "unknown element ";

      error_msg << "unexpected root element ";

      PrintElementToDiagMessage(el, &error_msg);

      error_msg << " found";

      source_diag.Error(error_msg);

      return false;

    }

tools/aapt2/xml/XmlActionExecutor.h

+18 −28
Original line number Diff line number Diff line
@@ -40,56 +40,46 @@ enum class XmlActionExecutorPolicy { 
  kWhitelist,

};



/**

 * Contains the actions to perform at this XML node. This is a recursive data

 * structure that

 * holds XmlNodeActions for child XML nodes.

 */

// Contains the actions to perform at this XML node. This is a recursive data structure that

// holds XmlNodeActions for child XML nodes.

class XmlNodeAction {

 public:

  using ActionFuncWithDiag = std::function<bool(Element*, SourcePathDiagnostics*)>;

  using ActionFunc = std::function<bool(Element*)>;



  /**

   * Find or create a child XmlNodeAction that will be performed for the child

   * element with the name `name`.

   */

  XmlNodeAction& operator[](const std::string& name) { return map_[name]; }

  // Find or create a child XmlNodeAction that will be performed for the child element with the

  // name `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(ActionFuncWithDiag);



 private:

  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::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 {

 public:

  XmlActionExecutor() = default;



  /**

   * Find or create a root XmlNodeAction that will be performed for the root XML

   * element with the name `name`.

   */

  XmlNodeAction& operator[](const std::string& name) { return map_[name]; }

  // Find or create a root XmlNodeAction that will be performed for the root XML element with the

  // name `name`.

  XmlNodeAction& operator[](const std::string& name) {

    return map_[name];

  }



  /**

   * Execute the defined actions for this XmlResource.

   * Returns true if all actions return true, otherwise returns false.

   */

  // Execute the defined actions for this XmlResource.

  // Returns true if all actions return true, otherwise returns false.

  bool Execute(XmlActionExecutorPolicy policy, IDiagnostics* diag, XmlResource* doc) const;



 private:

tools/aapt2/xml/XmlActionExecutor_test.cpp

+6 −2
Original line number Diff line number Diff line
@@ -56,9 +56,13 @@ TEST(XmlActionExecutorTest, FailsWhenUndefinedHierarchyExists) { 
  XmlActionExecutor executor;

  executor["manifest"]["application"];



  std::unique_ptr<XmlResource> doc =

      test::BuildXmlDom("<manifest><application /><activity /></manifest>");

  std::unique_ptr<XmlResource> doc;

  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()));

}