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
diff --git a/tools/aapt2/Diagnostics.h b/tools/aapt2/Diagnostics.h
index 50e8b33..30deb55 100644
--- a/tools/aapt2/Diagnostics.h
+++ b/tools/aapt2/Diagnostics.h
@@ -133,11 +133,19 @@
   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);
 };
diff --git a/tools/aapt2/compile/XmlIdCollector.cpp b/tools/aapt2/compile/XmlIdCollector.cpp
index d61a15a..2199d00 100644
--- a/tools/aapt2/compile/XmlIdCollector.cpp
+++ b/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 @@
  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 @@
       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 @@
 
  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
diff --git a/tools/aapt2/compile/XmlIdCollector_test.cpp b/tools/aapt2/compile/XmlIdCollector_test.cpp
index 98da56d..d49af3b 100644
--- a/tools/aapt2/compile/XmlIdCollector_test.cpp
+++ b/tools/aapt2/compile/XmlIdCollector_test.cpp
@@ -64,4 +64,14 @@
   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