AAPT2: Fail on invalid id names in compiled xml

AAPT2 was not erroring on invalid resource ids created in layouts with
creation syntax. This change causes this to error suring compile.

Bug: 71394154
Test: aapt2_tests
Change-Id: Idf16fb4bd011ed2d65e8c48f7cba0429ead5a055

tools/aapt2/Diagnostics.h

@@ -133,11 +133,19 @@ class SourcePathDiagnostics : public IDiagnostics {
   void Log(Level level, DiagMessageActual& actual_msg) override {
     actual_msg.source.path = source_.path;
     diag_->Log(level, actual_msg);
+    if (level == Level::Error) {
+      error = true;
+    }
+  }
+
+  bool HadError() {
+    return error;
   }
 
  private:
   Source source_;
   IDiagnostics* diag_;
+  bool error = false;
 
   DISALLOW_COPY_AND_ASSIGN(SourcePathDiagnostics);
 };

tools/aapt2/compile/XmlIdCollector.cpp

@@ -21,6 +21,7 @@
 
 #include "ResourceUtils.h"
 #include "ResourceValues.h"
+#include "text/Unicode.h"
 #include "xml/XmlDom.h"
 
 namespace aapt {
@@ -35,8 +36,9 @@ struct IdCollector : public xml::Visitor {
  public:
   using xml::Visitor::Visit;
 
-  explicit IdCollector(std::vector<SourcedResourceName>* out_symbols)
-      : out_symbols_(out_symbols) {}
+  explicit IdCollector(std::vector<SourcedResourceName>* out_symbols,
+                       SourcePathDiagnostics* source_diag) : out_symbols_(out_symbols),
+                                                             source_diag_(source_diag) {}
 
   void Visit(xml::Element* element) override {
     for (xml::Attribute& attr : element->attributes) {
@@ -44,12 +46,16 @@ struct IdCollector : public xml::Visitor {
       bool create = false;
       if (ResourceUtils::ParseReference(attr.value, &name, &create, nullptr)) {
         if (create && name.type == ResourceType::kId) {
-          auto iter = std::lower_bound(out_symbols_->begin(),
-                                       out_symbols_->end(), name, cmp_name);
-          if (iter == out_symbols_->end() || iter->name != name) {
-            out_symbols_->insert(iter,
-                                 SourcedResourceName{name.ToResourceName(),
-                                                     element->line_number});
+          if (!text::IsValidResourceEntryName(name.entry)) {
+            source_diag_->Error(DiagMessage(element->line_number)
+                                   << "id '" << name << "' has an invalid entry name");
+          } else {
+            auto iter = std::lower_bound(out_symbols_->begin(),
+                                         out_symbols_->end(), name, cmp_name);
+            if (iter == out_symbols_->end() || iter->name != name) {
+              out_symbols_->insert(iter, SourcedResourceName{name.ToResourceName(),
+                                                             element->line_number});
+            }
           }
         }
       }
@@ -60,15 +66,17 @@ struct IdCollector : public xml::Visitor {
 
  private:
   std::vector<SourcedResourceName>* out_symbols_;
+  SourcePathDiagnostics* source_diag_;
 };
 
 }  // namespace
 
 bool XmlIdCollector::Consume(IAaptContext* context, xml::XmlResource* xmlRes) {
   xmlRes->file.exported_symbols.clear();
-  IdCollector collector(&xmlRes->file.exported_symbols);
+  SourcePathDiagnostics source_diag(xmlRes->file.source, context->GetDiagnostics());
+  IdCollector collector(&xmlRes->file.exported_symbols, &source_diag);
   xmlRes->root->Accept(&collector);
-  return true;
+  return !source_diag.HadError();
 }
 
 }  // namespace aapt

tools/aapt2/compile/XmlIdCollector_test.cpp

@@ -64,4 +64,14 @@ TEST(XmlIdCollectorTest, DontCollectNonIds) {
   EXPECT_TRUE(doc->file.exported_symbols.empty());
 }
 
+TEST(XmlIdCollectorTest, ErrorOnInvalidIds) {
+  std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build();
+
+  std::unique_ptr<xml::XmlResource> doc =
+      test::BuildXmlDom("<View foo=\"@+id/foo$bar\"/>");
+
+  XmlIdCollector collector;
+  ASSERT_FALSE(collector.Consume(context.get(), doc.get()));
+}
+
 }  // namespace aapt