AAPT2: Cleanup Visitors for XML and Values

Test: make aapt2_tests
Change-Id: Ib61f64c155a380115610edeaf2d65e60258a2426
diff --git a/tools/aapt2/ValueVisitor.h b/tools/aapt2/ValueVisitor.h
index eb4fa49..4e74ec3 100644
--- a/tools/aapt2/ValueVisitor.h
+++ b/tools/aapt2/ValueVisitor.h
@@ -22,12 +22,11 @@
 
 namespace aapt {
 
-/**
- * Visits a value and invokes the appropriate method based on its type. Does not
- * traverse into compound types. Use ValueVisitor for that.
- */
-struct RawValueVisitor {
-  virtual ~RawValueVisitor() = default;
+// Visits a value and invokes the appropriate method based on its type.
+// Does not traverse into compound types. Use ValueVisitor for that.
+class ValueVisitor {
+ public:
+  virtual ~ValueVisitor() = default;
 
   virtual void VisitAny(Value* value) {}
   virtual void VisitItem(Item* value) { VisitAny(value); }
@@ -46,21 +45,67 @@
   virtual void Visit(Styleable* value) { VisitAny(value); }
 };
 
+// Const version of ValueVisitor.
+class ConstValueVisitor {
+ public:
+  virtual ~ConstValueVisitor() = default;
+
+  virtual void VisitAny(const Value* value) {
+  }
+  virtual void VisitItem(const Item* value) {
+    VisitAny(value);
+  }
+  virtual void Visit(const Reference* value) {
+    VisitItem(value);
+  }
+  virtual void Visit(const RawString* value) {
+    VisitItem(value);
+  }
+  virtual void Visit(const String* value) {
+    VisitItem(value);
+  }
+  virtual void Visit(const StyledString* value) {
+    VisitItem(value);
+  }
+  virtual void Visit(const FileReference* value) {
+    VisitItem(value);
+  }
+  virtual void Visit(const Id* value) {
+    VisitItem(value);
+  }
+  virtual void Visit(const BinaryPrimitive* value) {
+    VisitItem(value);
+  }
+
+  virtual void Visit(const Attribute* value) {
+    VisitAny(value);
+  }
+  virtual void Visit(const Style* value) {
+    VisitAny(value);
+  }
+  virtual void Visit(const Array* value) {
+    VisitAny(value);
+  }
+  virtual void Visit(const Plural* value) {
+    VisitAny(value);
+  }
+  virtual void Visit(const Styleable* value) {
+    VisitAny(value);
+  }
+};
+
 // NOLINT, do not add parentheses around T.
 #define DECL_VISIT_COMPOUND_VALUE(T)                   \
   virtual void Visit(T* value) override { /* NOLINT */ \
     VisitSubValues(value);                             \
   }
 
-/**
- * Visits values, and if they are compound values, visits the components as
- * well.
- */
-struct ValueVisitor : public RawValueVisitor {
+// Visits values, and if they are compound values, descends into their components as well.
+struct DescendingValueVisitor : public ValueVisitor {
   // The compiler will think we're hiding an overload, when we actually intend
   // to call into RawValueVisitor. This will expose the visit methods in the
   // super class so the compiler knows we are trying to call them.
-  using RawValueVisitor::Visit;
+  using ValueVisitor::Visit;
 
   void VisitSubValues(Attribute* attribute) {
     for (Attribute::Symbol& symbol : attribute->symbols) {
@@ -106,37 +151,30 @@
   DECL_VISIT_COMPOUND_VALUE(Styleable);
 };
 
-/**
- * Do not use directly. Helper struct for dyn_cast.
- */
+// Do not use directly. Helper struct for dyn_cast.
 template <typename T>
-struct DynCastVisitor : public RawValueVisitor {
-  T* value = nullptr;
+struct DynCastVisitor : public ConstValueVisitor {
+  const T* value = nullptr;
 
-  void Visit(T* v) override { value = v; }
+  void Visit(const T* v) override {
+    value = v;
+  }
 };
 
-/**
- * Specialization that checks if the value is an Item.
- */
+// Specialization that checks if the value is an Item.
 template <>
-struct DynCastVisitor<Item> : public RawValueVisitor {
-  Item* value = nullptr;
+struct DynCastVisitor<Item> : public ConstValueVisitor {
+  const Item* value = nullptr;
 
-  void VisitItem(Item* item) override { value = item; }
+  void VisitItem(const Item* item) override {
+    value = item;
+  }
 };
 
+// Returns a valid pointer to T if the value is an instance of T. Returns nullptr if value is
+// nullptr of if value is not an instance of T.
 template <typename T>
 const T* ValueCast(const Value* value) {
-  return ValueCast<T>(const_cast<Value*>(value));
-}
-
-/**
- * Returns a valid pointer to T if the Value is of subtype T.
- * Otherwise, returns nullptr.
- */
-template <typename T>
-T* ValueCast(Value* value) {
   if (!value) {
     return nullptr;
   }
@@ -145,8 +183,13 @@
   return visitor.value;
 }
 
-inline void VisitAllValuesInPackage(ResourceTablePackage* pkg,
-                                    RawValueVisitor* visitor) {
+// Non-const version of ValueCast.
+template <typename T>
+T* ValueCast(Value* value) {
+  return const_cast<T*>(ValueCast<T>(static_cast<const Value*>(value)));
+}
+
+inline void VisitAllValuesInPackage(ResourceTablePackage* pkg, ValueVisitor* visitor) {
   for (auto& type : pkg->types) {
     for (auto& entry : type->entries) {
       for (auto& config_value : entry->values) {
@@ -156,8 +199,7 @@
   }
 }
 
-inline void VisitAllValuesInTable(ResourceTable* table,
-                                  RawValueVisitor* visitor) {
+inline void VisitAllValuesInTable(ResourceTable* table, ValueVisitor* visitor) {
   for (auto& pkg : table->packages) {
     VisitAllValuesInPackage(pkg.get(), visitor);
   }