From d33b1e45dbeeed47e382e37ea9bae020849e1814 Mon Sep 17 00:00:00 2001 From: Tor Norbye <tnorbye@google.com> Date: Wed, 23 Jan 2019 07:19:55 -0800 Subject: [PATCH] Fix type comparison to ignore whitespace When comparing type signatures, spaces should not be significant, eg. Map<X,Y> == Map<X, Y>. This was already mostly the case, except for when comparing types from one signature file with types from another, as is the case when doing compatibility checks with show annotations (such as the system API). Test: Unit tests included Change-Id: Icec1a679cfb9245905581bc0fe9074745f835038 --- .../android/tools/metalava/model/TypeItem.kt | 41 +++++++++++++++++++ .../tools/metalava/model/psi/PsiTypeItem.kt | 2 +- .../tools/metalava/model/text/TextTypeItem.kt | 13 ++---- .../tools/metalava/CompatibilityCheckTest.kt | 24 +++++++++++ .../tools/metalava/model/TypeItemTest.kt | 18 ++++++++ 5 files changed, 88 insertions(+), 10 deletions(-) diff --git a/src/main/java/com/android/tools/metalava/model/TypeItem.kt b/src/main/java/com/android/tools/metalava/model/TypeItem.kt index 7ca5ff9..c39e072 100644 --- a/src/main/java/com/android/tools/metalava/model/TypeItem.kt +++ b/src/main/java/com/android/tools/metalava/model/TypeItem.kt @@ -297,5 +297,46 @@ interface TypeItem { return dimension + base } + + /** Compares two strings, ignoring space diffs (spaces, not whitespace in general) */ + fun equalsWithoutSpace(s1: String, s2: String): Boolean { + if (s1 == s2) { + return true + } + val sp1 = s1.indexOf(' ') // first space + val sp2 = s2.indexOf(' ') + if (sp1 == -1 && sp2 == -1) { + // no spaces in strings and aren't equal + return false + } + + val l1 = s1.length + val l2 = s2.length + var i1 = 0 + var i2 = 0 + + while (i1 < l1 && i2 < l2) { + var c1 = s1[i1++] + var c2 = s2[i2++] + + while (c1 == ' ' && i1 < l1) { + c1 = s1[i1++] + } + while (c2 == ' ' && i2 < l2) { + c2 = s2[i2++] + } + if (c1 != c2) { + return false + } + } + // Skip trailing spaces + while (i1 < l1 && s1[i1] == ' ') { + i1++ + } + while (i2 < l2 && s2[i2] == ' ') { + i2++ + } + return i1 == l1 && i2 == l2 + } } } diff --git a/src/main/java/com/android/tools/metalava/model/psi/PsiTypeItem.kt b/src/main/java/com/android/tools/metalava/model/psi/PsiTypeItem.kt index ef758ff..306770a 100644 --- a/src/main/java/com/android/tools/metalava/model/psi/PsiTypeItem.kt +++ b/src/main/java/com/android/tools/metalava/model/psi/PsiTypeItem.kt @@ -149,7 +149,7 @@ class PsiTypeItem private constructor(private val codebase: PsiBasedCodebase, pr if (this === other) return true return when (other) { - is TypeItem -> toTypeString().replace(" ", "") == other.toTypeString().replace(" ", "") + is TypeItem -> TypeItem.equalsWithoutSpace(toTypeString(), other.toTypeString()) else -> false } } diff --git a/src/main/java/com/android/tools/metalava/model/text/TextTypeItem.kt b/src/main/java/com/android/tools/metalava/model/text/TextTypeItem.kt index d74415b..9dae73f 100644 --- a/src/main/java/com/android/tools/metalava/model/text/TextTypeItem.kt +++ b/src/main/java/com/android/tools/metalava/model/text/TextTypeItem.kt @@ -73,20 +73,15 @@ class TextTypeItem( if (this === other) return true return when (other) { - is TextTypeItem -> toString() == other.toString() + // Note: when we support type-use annotations, this is not safe: there could be a string + // literal inside which is significant + is TextTypeItem -> TypeItem.equalsWithoutSpace(toString(), other.toString()) is TypeItem -> { val thisString = toTypeString() val otherString = other.toTypeString() - if (thisString == otherString) { + if (TypeItem.equalsWithoutSpace(thisString, otherString)) { return true } - if (thisString[0] == otherString[0]) { - val thisCondensed = thisString.replace(" ", "") - val otherCondensed = otherString.replace(" ", "") - if (thisCondensed == otherCondensed) { - return true - } - } if (thisString.startsWith(JAVA_LANG_PREFIX) && thisString.endsWith(otherString) && thisString.length == otherString.length + JAVA_LANG_PREFIX.length ) { diff --git a/src/test/java/com/android/tools/metalava/CompatibilityCheckTest.kt b/src/test/java/com/android/tools/metalava/CompatibilityCheckTest.kt index 819bd2b..864429d 100644 --- a/src/test/java/com/android/tools/metalava/CompatibilityCheckTest.kt +++ b/src/test/java/com/android/tools/metalava/CompatibilityCheckTest.kt @@ -2275,6 +2275,30 @@ CompatibilityCheckTest : DriverTest() { ) } + @Test + fun `Insignificant type formatting differences`() { + check( + checkCompatibilityApi = """ + package test.pkg { + public final class UsageStatsManager { + method public java.util.Map<java.lang.String, java.lang.Integer> getAppStandbyBuckets(); + method public void setAppStandbyBuckets(java.util.Map<java.lang.String, java.lang.Integer>); + field public java.util.Map<java.lang.String, java.lang.Integer> map; + } + } + """, + signatureSource = """ + package test.pkg { + public final class UsageStatsManager { + method public java.util.Map<java.lang.String,java.lang.Integer> getAppStandbyBuckets(); + method public void setAppStandbyBuckets(java.util.Map<java.lang.String,java.lang.Integer>); + field public java.util.Map<java.lang.String,java.lang.Integer> map; + } + } + """ + ) + } + @Test fun `Adding and removing reified`() { check( diff --git a/src/test/java/com/android/tools/metalava/model/TypeItemTest.kt b/src/test/java/com/android/tools/metalava/model/TypeItemTest.kt index 112d513..908248d 100644 --- a/src/test/java/com/android/tools/metalava/model/TypeItemTest.kt +++ b/src/test/java/com/android/tools/metalava/model/TypeItemTest.kt @@ -33,4 +33,22 @@ class TypeItemTest { "java.util.List<@NonNull java.lang.String>" ) } + + @Test + fun testEqualsWithoutSpace() { + assertThat(TypeItem.equalsWithoutSpace("", "")).isTrue() + assertThat(TypeItem.equalsWithoutSpace(" ", "")).isTrue() + assertThat(TypeItem.equalsWithoutSpace("", " ")).isTrue() + assertThat(TypeItem.equalsWithoutSpace(" ", " ")).isTrue() + assertThat(TypeItem.equalsWithoutSpace("true", "tr ue")).isTrue() + assertThat(TypeItem.equalsWithoutSpace("tr ue", "true")).isTrue() + assertThat(TypeItem.equalsWithoutSpace("true", "true ")).isTrue() + assertThat(TypeItem.equalsWithoutSpace("true ", "true")).isTrue() + assertThat(TypeItem.equalsWithoutSpace("true ", "true")).isTrue() + assertThat(TypeItem.equalsWithoutSpace("true", " true")).isTrue() + + assertThat(TypeItem.equalsWithoutSpace("true", "false")).isFalse() + assertThat(TypeItem.equalsWithoutSpace("true", " true false")).isFalse() + assertThat(TypeItem.equalsWithoutSpace("false ", "falser")).isFalse() + } } \ No newline at end of file -- GitLab