Fixes the case where an optional attribute needs to be an empty string. This is necessary for <style parent="">.

The previous code omitted empty strings to avoid the <attr format=""> case which is an error for aapt.

--
MOS_MIGRATED_REVID=126305040

3 files changed, 37 insertions, 8 deletions

diff --git a/src/tools/android/java/com/google/devtools/build/android/AndroidDataWriter.java b/src/tools/android/java/com/google/devtools/build/android/AndroidDataWriter.java
index 9193649d31..ab43942abd 100644
--- a/src/tools/android/java/com/google/devtools/build/android/AndroidDataWriter.java
+++ b/src/tools/android/java/com/google/devtools/build/android/AndroidDataWriter.java
@@ -16,7 +16,6 @@ package com.google.devtools.build.android;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Joiner;
 import com.google.common.base.Preconditions;
-import com.google.common.base.Strings;
 import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.Multimap;
 import com.google.common.collect.Ordering;
@@ -44,6 +43,7 @@ import java.util.ArrayList;
 import java.util.Deque;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
@@ -135,7 +135,11 @@ public class AndroidDataWriter implements AndroidDataWritingVisitor {
   }
 
   public static final char[] START_RESOURCES =
-      ("<resources xmlns:xliff=\"" + XmlResourceValues.XLIFF_NAMESPACE + "\">").toCharArray();
+      ("<?xml version=\"1.0\" encoding='utf-8' standalone='no'?>\n"
+              + "<resources xmlns:xliff=\""
+              + XmlResourceValues.XLIFF_NAMESPACE
+              + "\">")
+          .toCharArray();
   public static final char[] END_RESOURCES = "</resources>".toCharArray();
   private static final char[] LINE_END = "\n".toCharArray();
   private static final PngCruncher NOOP_CRUNCHER =
@@ -323,6 +327,7 @@ public class AndroidDataWriter implements AndroidDataWritingVisitor {
                   StandardOpenOption.CREATE_NEW,
                   StandardOpenOption.WRITE)) {
             writer.write(START_RESOURCES);
+            writer.write(LINE_END);
             Path previousSource = null;
             for (FullyQualifiedName key :
                 Ordering.natural().immutableSortedCopy(segments.keySet())) {
@@ -392,11 +397,25 @@ public class AndroidDataWriter implements AndroidDataWritingVisitor {
 
     @Override
     public StartTag setTo(String value) {
-      if (!(optional && Strings.isNullOrEmpty(value))) {
+      if (!optional || value != null) {
         owner.attributes.add(" " + name + "=\"" + value + "\"");
       }
       return owner;
     }
+
+    @Override
+    public ValueJoiner setFrom(final Iterable<String> values) {
+      return new ValueJoiner() {
+        @Override
+        public StartTag joinedBy(String separator) {
+          Iterator<String> valuesIterator = values.iterator();
+          if (!optional || valuesIterator.hasNext()) {
+            setTo(Joiner.on(separator).join(valuesIterator));
+          }
+          return owner;
+        }
+      };
+    }
   }
 
   /** Intermediate class that collects information for writing an xml start tag string. */
@@ -449,7 +468,7 @@ public class AndroidDataWriter implements AndroidDataWritingVisitor {
 
     @Override
     public ValuesResourceDefinition closeUnaryTag() {
-      writer.mapper.add("<" + tagName + Joiner.on("").join(attributes) + "/>");
+      writer.mapper.add("\n<" + tagName + Joiner.on("").join(attributes) + "/>");
       return writer;
     }
 
@@ -521,6 +540,7 @@ public class AndroidDataWriter implements AndroidDataWritingVisitor {
     @Override
     public void save() {
       Preconditions.checkArgument(tagStack.isEmpty(), "Unfinished tags %s", tagStack);
+      mapper.add("\n"); // Safe to add a line break to separate from other definitions.
       mapper.finish();
     }
 
@@ -629,7 +649,6 @@ public class AndroidDataWriter implements AndroidDataWritingVisitor {
     public Path write(Path previousSource, Writer writer) throws IOException {
       for (String line : lines) {
         writer.write(line);
-        writer.write(LINE_END);
       }
       return previousSource;
     }
diff --git a/src/tools/android/java/com/google/devtools/build/android/AndroidDataWritingVisitor.java b/src/tools/android/java/com/google/devtools/build/android/AndroidDataWritingVisitor.java
index 2aa469dc8a..c55ee3629e 100644
--- a/src/tools/android/java/com/google/devtools/build/android/AndroidDataWritingVisitor.java
+++ b/src/tools/android/java/com/google/devtools/build/android/AndroidDataWritingVisitor.java
@@ -160,7 +160,15 @@ public interface AndroidDataWritingVisitor extends Flushable {
   interface Attribute {
     /** Sets the attribute value. */
     StartTag setTo(String value);
-    /** Sets the attributes values to {@linkplain FullyQualifiedName#name()}. */
+    /** Sets the attribute value to {@linkplain FullyQualifiedName#name()}. */
     StartTag setTo(FullyQualifiedName fqn);
+    /** Start the process of setting an attribute value from an iterable of strings. * */
+    ValueJoiner setFrom(Iterable<String> values);
+  }
+
+  /** Represents the concatenation step of turning an {@link Iterable} into a string. */
+  @CheckReturnValue
+  interface ValueJoiner {
+    StartTag joinedBy(String separator);
   }
 }
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 a4608ce6ed..636f536183 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
@@ -18,7 +18,6 @@ import static com.google.common.base.Predicates.not;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Function;
-import com.google.common.base.Joiner;
 import com.google.common.base.MoreObjects;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.FluentIterable;
@@ -313,7 +312,10 @@ public class AttrXmlResourceValue implements XmlResourceValue {
               .derivedFrom(source)
               .startTag("attr")
               .named(key)
-              .optional().attribute("format").setTo(Joiner.on("|").join(formatKeys))
+              .optional()
+              .attribute("format")
+              .setFrom(formatKeys)
+              .joinedBy("|")
               .closeTag();
       for (ResourceXmlAttrValue value : formats.values()) {
         definition = value.writeTo(definition);