Xml processing fixes:

* Support for <item name="foo" type="id">7x0000</item>
* Adds <eat-comment/> after source attributions to ensure they are not sent to the final binary
* Store attributes for arrays to the string translatable=false case

--
MOS_MIGRATED_REVID=124684322

5 files changed, 95 insertions, 46 deletions

diff --git a/src/tools/android/java/com/google/devtools/build/android/DataResourceXml.java b/src/tools/android/java/com/google/devtools/build/android/DataResourceXml.java
index a29c589743..76f1c24acc 100644
--- a/src/tools/android/java/com/google/devtools/build/android/DataResourceXml.java
+++ b/src/tools/android/java/com/google/devtools/build/android/DataResourceXml.java
@@ -149,8 +149,8 @@ public class DataResourceXml implements DataResource {
   private static XmlResourceValue parseXmlElements(
       ResourceType resourceType, XMLEventReader eventReader, StartElement start)
       throws XMLStreamException {
-    // Handle ids first, as they are a special kind of item.
-    if (resourceType == ID) {
+    // Handle unary ids first, as they are a special kind of item.
+    if (resourceType == ID && XmlResourceValues.isEndTag(eventReader.peek(), start.getName())) {
       return XmlResourceValues.parseId();
     }
     // Handle item stubs.
diff --git a/src/tools/android/java/com/google/devtools/build/android/XmlResourceValues.java b/src/tools/android/java/com/google/devtools/build/android/XmlResourceValues.java
index 9267e97d93..8f4978e571 100644
--- a/src/tools/android/java/com/google/devtools/build/android/XmlResourceValues.java
+++ b/src/tools/android/java/com/google/devtools/build/android/XmlResourceValues.java
@@ -165,6 +165,22 @@ public class XmlResourceValues {
   static XmlResourceValue parseSimple(
       XMLEventReader eventReader, ResourceType resourceType, StartElement start)
       throws XMLStreamException {
+    String contents;
+    // Check that the element is unary. If it is, the contents is null
+    if (isEndTag(eventReader.peek(), start.getName())) {
+      contents = null;
+    } else {
+      contents = readContentsAsString(eventReader, start.getName());
+    }
+    return SimpleXmlResourceValue.of(
+        start.getName().equals(TAG_ITEM)
+            ? SimpleXmlResourceValue.Type.ITEM
+            : SimpleXmlResourceValue.Type.from(resourceType),
+        ImmutableMap.copyOf(parseTagAttributes(start)),
+        contents);
+  }
+
+  public static Map<String, String> parseTagAttributes(StartElement start) {
     // Using a map to deduplicate xmlns declarations on the attributes.
     Map<String, String> attributeMap = new LinkedHashMap<>();
     Iterator<Attribute> attributes = iterateAttributesFrom(start);
@@ -190,19 +206,7 @@ public class XmlResourceValues {
         attributeMap.put(attribute.getName().getLocalPart(), value);
       }
     }
-    String contents;
-    // Check and see if the element is unary. If it is, the contents is null
-    if (isEndTag(eventReader.peek(), start.getName())) {
-      contents = null;
-    } else {
-      contents = readContentsAsString(eventReader, start.getName());
-    }
-    return SimpleXmlResourceValue.of(
-        start.getName().equals(TAG_ITEM)
-            ? SimpleXmlResourceValue.Type.ITEM
-            : SimpleXmlResourceValue.Type.from(resourceType),
-        ImmutableMap.copyOf(attributeMap),
-        contents);
+    return attributeMap;
   }
 
   // TODO(corysmith): Replace this with real escaping system, preferably a performant high level xml
diff --git a/src/tools/android/java/com/google/devtools/build/android/xml/ArrayXmlResourceValue.java b/src/tools/android/java/com/google/devtools/build/android/xml/ArrayXmlResourceValue.java
index 0a07a5daad..8ccb55d1d9 100644
--- a/src/tools/android/java/com/google/devtools/build/android/xml/ArrayXmlResourceValue.java
+++ b/src/tools/android/java/com/google/devtools/build/android/xml/ArrayXmlResourceValue.java
@@ -18,6 +18,7 @@ import com.google.common.base.Function;
 import com.google.common.base.MoreObjects;
 import com.google.common.collect.FluentIterable;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.android.AndroidDataWritingVisitor;
 import com.google.devtools.build.android.FullyQualifiedName;
 import com.google.devtools.build.android.XmlResourceValue;
@@ -30,6 +31,7 @@ import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
+import java.util.Map.Entry;
 import java.util.Objects;
 
 import javax.annotation.Nullable;
@@ -94,8 +96,19 @@ public class ArrayXmlResourceValue implements XmlResourceValue {
           String.format("%s not found in %s", tagQName, Arrays.toString(values())));
     }
 
-    String openTag(FullyQualifiedName key) {
-      return String.format("<%s name='%s'>", tagName.getLocalPart(), key.name());
+    String openTag(FullyQualifiedName key, ImmutableMap<String, String> attributes) {
+      StringBuilder xmlString = new StringBuilder("<");
+      xmlString.append(tagName.getLocalPart());
+      xmlString.append(" name='").append(key.name()).append("'");
+      for (Entry<String, String> entry : attributes.entrySet()) {
+        xmlString
+            .append(" ")
+            .append(entry.getKey())
+            .append("='")
+            .append(entry.getValue())
+            .append("'");
+      }
+      return xmlString.append(">").toString();
     }
 
     String closeTag() {
@@ -105,10 +118,13 @@ public class ArrayXmlResourceValue implements XmlResourceValue {
 
   private final ImmutableList<String> values;
   private final ArrayType arrayType;
+  private final ImmutableMap<String, String> attributes;
 
-  private ArrayXmlResourceValue(ArrayType arrayType, ImmutableList<String> values) {
+  private ArrayXmlResourceValue(
+      ArrayType arrayType, ImmutableList<String> values, ImmutableMap<String, String> attributes) {
     this.arrayType = arrayType;
     this.values = values;
+    this.attributes = attributes;
   }
 
   @VisibleForTesting
@@ -117,11 +133,19 @@ public class ArrayXmlResourceValue implements XmlResourceValue {
   }
 
   public static XmlResourceValue of(ArrayType arrayType, List<String> values) {
-    return new ArrayXmlResourceValue(arrayType, ImmutableList.copyOf(values));
+    return of(arrayType, values, ImmutableMap.<String, String>of());
+  }
+
+  public static XmlResourceValue of(
+      ArrayType arrayType, List<String> values, ImmutableMap<String, String> attributes) {
+    return new ArrayXmlResourceValue(arrayType, ImmutableList.copyOf(values), attributes);
   }
 
   public static XmlResourceValue from(SerializeFormat.DataValueXml proto) {
-    return of(ArrayType.valueOf(proto.getValueType()), proto.getListValueList());
+    return of(
+        ArrayType.valueOf(proto.getValueType()),
+        proto.getListValueList(),
+        ImmutableMap.copyOf(proto.getMappedStringValueMap()));
   }
 
   @Override
@@ -130,7 +154,8 @@ public class ArrayXmlResourceValue implements XmlResourceValue {
     mergedDataWriter.writeToValuesXml(
         key,
         FluentIterable.from(
-                ImmutableList.of(String.format("<!-- %s -->", source), arrayType.openTag(key)))
+                ImmutableList.of(
+                    String.format("<!-- %s -->", source), arrayType.openTag(key, attributes)))
             .append(FluentIterable.from(values).transform(ITEM_TO_XML))
             .append(arrayType.closeTag()));
   }
@@ -144,12 +169,13 @@ public class ArrayXmlResourceValue implements XmlResourceValue {
                 SerializeFormat.DataValueXml.newBuilder()
                     .addAllListValue(values)
                     .setType(SerializeFormat.DataValueXml.XmlType.ARRAY)
+                    .putAllMappedStringValue(attributes)
                     .setValueType(arrayType.toString())));
   }
 
   @Override
   public int hashCode() {
-    return Objects.hash(arrayType, values);
+    return Objects.hash(arrayType, values, attributes);
   }
 
   @Override
@@ -158,7 +184,9 @@ public class ArrayXmlResourceValue implements XmlResourceValue {
       return false;
     }
     ArrayXmlResourceValue other = (ArrayXmlResourceValue) obj;
-    return Objects.equals(arrayType, other.arrayType) && Objects.equals(values, other.values);
+    return Objects.equals(arrayType, other.arrayType)
+        && Objects.equals(values, other.values)
+        && Objects.equals(attributes, other.attributes);
   }
 
   @Override
@@ -166,6 +194,7 @@ public class ArrayXmlResourceValue implements XmlResourceValue {
     return MoreObjects.toStringHelper(getClass())
         .add("arrayType", arrayType)
         .add("values", values)
+        .add("attributes", attributes)
         .toString();
   }
 
@@ -185,13 +214,16 @@ public class ArrayXmlResourceValue implements XmlResourceValue {
           throw new XMLStreamException(
               String.format("Expected start element %s", element), element.getLocation());
         }
-        String contents = XmlResourceValues.readContentsAsString(eventReader,
-            element.asStartElement().getName());
+        String contents =
+            XmlResourceValues.readContentsAsString(eventReader, element.asStartElement().getName());
         values.add(contents != null ? contents : "");
       }
     }
     try {
-      return of(ArrayType.fromTagName(start), values);
+      return of(
+          ArrayType.fromTagName(start),
+          values,
+          ImmutableMap.copyOf(XmlResourceValues.parseTagAttributes(start)));
     } catch (IllegalArgumentException e) {
       throw new XMLStreamException(e.getMessage(), start.getLocation());
     }
diff --git a/src/tools/android/java/com/google/devtools/build/android/xml/AttrXmlResourceValue.java b/src/tools/android/java/com/google/devtools/build/android/xml/AttrXmlResourceValue.java
index 07b8855f4b..cdf140e6d2 100644
--- a/src/tools/android/java/com/google/devtools/build/android/xml/AttrXmlResourceValue.java
+++ b/src/tools/android/java/com/google/devtools/build/android/xml/AttrXmlResourceValue.java
@@ -289,21 +289,39 @@ public class AttrXmlResourceValue implements XmlResourceValue {
   @Override
   public void write(
       FullyQualifiedName key, Path source, AndroidDataWritingVisitor mergedDataWriter) {
+
     ImmutableList<String> formatKeys = Ordering.natural().immutableSortedCopy(formats.keySet());
-    FluentIterable<String> iterable =
-        FluentIterable.from(
-            ImmutableList.of(
-                String.format("<!-- %s -->", source),
-                formatKeys.isEmpty()
-                    ? String.format("<attr name='%s'>", key.name())
-                    : String.format(
-                        "<attr name='%s' format='%s'>",
-                        key.name(),
-                        Joiner.on('|').join(formatKeys))));
-    for (String formatKey : formatKeys) {
-      iterable = formats.get(formatKey).appendTo(iterable);
-    }
-    mergedDataWriter.writeToValuesXml(key, iterable.append("</attr>"));
+    if (formatKeys.isEmpty()) {
+      mergedDataWriter.writeToValuesXml(key,
+          ImmutableList.of(
+              String.format("<!-- %s -->", source),
+              String.format(
+                  "<attr name='%s'/>",
+                  key.name())));
+    } else if (formats.containsKey(FLAGS) || formats.containsKey(ENUM)) {
+      FluentIterable<String> iterable =
+          FluentIterable.from(
+              ImmutableList.of(
+                  String.format("<!-- %s -->", source),
+                  formatKeys.isEmpty()
+                      ? String.format("<attr name='%s'>", key.name())
+                      : String.format(
+                          "<attr format='%s' name='%s' >",
+                          Joiner.on('|').join(formatKeys), key.name())));
+      for (String formatKey : formatKeys) {
+        iterable = formats.get(formatKey).appendTo(iterable);
+      }
+      mergedDataWriter.writeToValuesXml(key, iterable.append("</attr>"));
+    } else {
+      mergedDataWriter.writeToValuesXml(key,
+          ImmutableList.of(
+              String.format("<!-- %s -->", source),
+              String.format(
+                  "<attr format='%s' name='%s'/>",
+                  Joiner.on('|').join(formatKeys),
+                  key.name()
+                  )));
+    }
   }
 
   @Override
diff --git a/src/tools/android/java/com/google/devtools/build/android/xml/SimpleXmlResourceValue.java b/src/tools/android/java/com/google/devtools/build/android/xml/SimpleXmlResourceValue.java
index 9169401c21..987a25d16f 100644
--- a/src/tools/android/java/com/google/devtools/build/android/xml/SimpleXmlResourceValue.java
+++ b/src/tools/android/java/com/google/devtools/build/android/xml/SimpleXmlResourceValue.java
@@ -175,11 +175,6 @@ public class SimpleXmlResourceValue implements XmlResourceValue {
     return withAttributes(Type.ITEM, ImmutableMap.of("type", resourceType.getName()));
   }
 
-  @Deprecated
-  public static XmlResourceValue of(Type valueType, @Nullable String value) {
-    return of(valueType, ImmutableMap.<String, String>of(), value);
-  }
-
   public static XmlResourceValue of(
       Type valueType, ImmutableMap<String, String> attributes, @Nullable String value) {
     return new SimpleXmlResourceValue(valueType, attributes, value);
@@ -226,7 +221,7 @@ public class SimpleXmlResourceValue implements XmlResourceValue {
   public static XmlResourceValue from(SerializeFormat.DataValueXml proto) {
     return of(
         Type.valueOf(proto.getValueType()),
-        ImmutableMap.copyOf(proto.getMappedStringValue()),
+        ImmutableMap.copyOf(proto.getMappedStringValueMap()),
         proto.hasValue() ? proto.getValue() : null);
   }