123140708: Always include spaces in type argument lists in jdiff output

This CL fixes CTS tests that depend on string equality between
the JDiff type descriptions and their own internal pretty-printer
based on reflection lookup of the API; the pretty printer always
emits a space after a comma in type lists whereas metalava's
signature file to JDiff output converter would preserve whatever
was in the signature file, which as of v2 omitted spaces.

This CL forces the output to always have spaces when converting
to JDiff output.

Test: Unit test included (plus of course the failing CTS test)
Change-Id: Icbcb3067423cb5fa259e18e028704c27c48953e4
Fixes: 123140708

src/main/java/com/android/tools/metalava/JDiffXmlWriter.kt

@@ -201,12 +201,10 @@ class JDiffXmlWriter(
             }
         } else null
 
-        val fullTypeName = escapeAttributeValue(field.type().toTypeString())
-
         writer.print("<field name=\"")
         writer.print(field.name())
         writer.print("\"\n type=\"")
-        writer.print(fullTypeName)
+        writer.print(escapeAttributeValue(formatType(field.type())))
         writer.print("\"\n transient=\"")
         writer.print(modifiers.isTransient())
         writer.print("\"\n volatile=\"")
@@ -295,9 +293,11 @@ class JDiffXmlWriter(
                         return
                     }
                     escapeAttributeValue(
-                        superClass.toTypeString(
-                            erased = compatibility.omitTypeParametersInInterfaces,
-                            context = superClass.asClass()
+                        formatType(
+                            superClass.toTypeString(
+                                erased = compatibility.omitTypeParametersInInterfaces,
+                                context = superClass.asClass()
+                            )
                         )
                     )
                 }
@@ -325,8 +325,7 @@ class JDiffXmlWriter(
             interfaces.sortedWith(TypeItem.comparator).forEach { item ->
                 writer.print("<implements name=\"")
                 val type = item.toTypeString(erased = compatibility.omitTypeParametersInInterfaces, context = cls)
-                val escapedType = escapeAttributeValue(type)
-                writer.print(escapedType)
+                writer.print(escapeAttributeValue(formatType(type)))
                 writer.println("\">\n</implements>")
             }
         }
@@ -343,13 +342,12 @@ class JDiffXmlWriter(
         }
     }
 
-    private fun formatType(type: TypeItem): String {
-        val typeString = type.toTypeString()
-        return if (compatibility.spaceAfterCommaInTypes) {
-            typeString.replace(",", ", ").replace(",  ", ", ")
-        } else {
-            typeString
-        }
+    private fun formatType(type: TypeItem): String = formatType(type.toTypeString())
+
+    private fun formatType(typeString: String): String {
+        // In JDiff we always want to include spaces after commas; the API signature tests depend
+        // on this.
+        return typeString.replace(",", ", ").replace(",  ", ", ")
     }
 
     private fun writeThrowsList(method: MethodItem) {

src/main/java/com/android/tools/metalava/doclava1/ApiFile.java

@@ -42,7 +42,9 @@ import org.jetbrains.annotations.Nullable;
 import java.io.File;
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 
 import static com.android.tools.metalava.ConstantsKt.ANDROIDX_NONNULL;
 import static com.android.tools.metalava.ConstantsKt.ANDROIDX_NULLABLE;

src/test/java/com/android/tools/metalava/JDiffXmlTest.kt

@@ -711,7 +711,7 @@ class JDiffXmlTest : DriverTest() {
             <package name="test.pkg"
             >
             <interface name="AbstractList"
-             extends="test.pkg.List&lt;A,B,C>"
+             extends="test.pkg.List&lt;A, B, C>"
              abstract="true"
              static="false"
              final="false"
@@ -720,7 +720,7 @@ class JDiffXmlTest : DriverTest() {
             >
             </interface>
             <interface name="ConcreteList"
-             extends="test.pkg.AbstractList&lt;D,E,F>"
+             extends="test.pkg.AbstractList&lt;D, E, F>"
              abstract="true"
              static="false"
              final="false"
@@ -1041,4 +1041,90 @@ class JDiffXmlTest : DriverTest() {
             """
         )
     }
+
+    @Test
+    fun `Spaces in type argument lists`() {
+        // JDiff expects spaces in type argument lists
+        // Regression test for 123140708
+        check(
+            compatibilityMode = false,
+            checkDoclava1 = true,
+            format = FileFormat.V2,
+            signatureSource = """
+            // Signature format: 2.0
+            package org.apache.http.impl.conn.tsccm {
+              @Deprecated public class ConnPoolByRoute extends org.apache.http.impl.conn.tsccm.AbstractConnPool {
+                field @Deprecated protected final java.util.Map<org.apache.http.conn.routing.HttpRoute,org.apache.http.impl.conn.tsccm.RouteSpecificPool> routeToPool;
+                field @Deprecated protected java.util.Queue<org.apache.http.impl.conn.tsccm.WaitingThread> waitingThreads;
+              }
+            }
+            package test.pkg {
+              public abstract class MyClass extends HashMap<String,String> implements Map<String,String>  {
+                field public Map<String,String> map;
+              }
+            }
+            """,
+            apiXml =
+            """
+            <api>
+            <package name="org.apache.http.impl.conn.tsccm"
+            >
+            <class name="ConnPoolByRoute"
+             extends="org.apache.http.impl.conn.tsccm.AbstractConnPool"
+             abstract="false"
+             static="false"
+             final="false"
+             deprecated="deprecated"
+             visibility="public"
+            >
+            <field name="routeToPool"
+             type="java.util.Map&lt;org.apache.http.conn.routing.HttpRoute, org.apache.http.impl.conn.tsccm.RouteSpecificPool>"
+             transient="false"
+             volatile="false"
+             static="false"
+             final="true"
+             deprecated="deprecated"
+             visibility="protected"
+            >
+            </field>
+            <field name="waitingThreads"
+             type="java.util.Queue&lt;org.apache.http.impl.conn.tsccm.WaitingThread>"
+             transient="false"
+             volatile="false"
+             static="false"
+             final="false"
+             deprecated="deprecated"
+             visibility="protected"
+            >
+            </field>
+            </class>
+            </package>
+            <package name="test.pkg"
+            >
+            <class name="MyClass"
+             extends="java.lang.HashMap&lt;String, String>"
+             abstract="true"
+             static="false"
+             final="false"
+             deprecated="not deprecated"
+             visibility="public"
+            >
+            <implements name="java.lang.Map&lt;String, String>">
+            </implements>
+            <field name="map"
+             type="java.lang.Map&lt;String, String>"
+             transient="false"
+             volatile="false"
+             static="false"
+             final="false"
+             deprecated="not deprecated"
+             visibility="public"
+            >
+            </field>
+            </class>
+            </package>
+            </api>
+            """
+        )
+    }
 }
\ No newline at end of file