From a100e0d540e895bfa0fa3b356ef8b1afec27d1ea Mon Sep 17 00:00:00 2001
From: Tor Norbye <tnorbye@google.com>
Date: Mon, 21 Jan 2019 08:58:30 -0800
Subject: [PATCH] 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
---
 .../android/tools/metalava/JDiffXmlWriter.kt  | 28 +++---
 .../tools/metalava/doclava1/ApiFile.java      |  2 +
 .../android/tools/metalava/JDiffXmlTest.kt    | 90 ++++++++++++++++++-
 3 files changed, 103 insertions(+), 17 deletions(-)

diff --git a/src/main/java/com/android/tools/metalava/JDiffXmlWriter.kt b/src/main/java/com/android/tools/metalava/JDiffXmlWriter.kt
index d4dbe0b..8087c3f 100644
--- a/src/main/java/com/android/tools/metalava/JDiffXmlWriter.kt
+++ b/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) {
diff --git a/src/main/java/com/android/tools/metalava/doclava1/ApiFile.java b/src/main/java/com/android/tools/metalava/doclava1/ApiFile.java
index b006d3c..00f3eb3 100644
--- a/src/main/java/com/android/tools/metalava/doclava1/ApiFile.java
+++ b/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;
diff --git a/src/test/java/com/android/tools/metalava/JDiffXmlTest.kt b/src/test/java/com/android/tools/metalava/JDiffXmlTest.kt
index c92458d..1a190c2 100644
--- a/src/test/java/com/android/tools/metalava/JDiffXmlTest.kt
+++ b/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
-- 
GitLab