From 43b83e761c645e6961bedea1c0f886bae9373df0 Mon Sep 17 00:00:00 2001 From: Nikolai Manzhos Date: Fri, 29 May 2026 22:08:31 +0200 Subject: [PATCH] Fix hashCode/equals contract violation in XType.hash() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `else` branch of `hash()` was using identity-based `hashCode()`, while `isEquivalent()` (used by `equals()`) performed value-based comparison via `typeName`. This caused equivalent XType instances — particularly those originating from mixed Java and Kotlin sources in KSP — to produce different hash codes, breaking HashMap deduplication in the scope graph. Replace the identity `hashCode()` with `typeName.removeWildcardTypeIfContains().toString().hashCode()` to match the equivalence logic, and remove the now-redundant `hasCollectionType` branch which did the same thing. --- .../kotlin/com/uber/xprocessing/ext/XType.kt | 8 +- .../ast/compiler/CompilerTypeHashTest.kt | 91 +++++++++++++++++++ 2 files changed, 92 insertions(+), 7 deletions(-) create mode 100644 compiler/ast/src/test/kotlin/motif/ast/compiler/CompilerTypeHashTest.kt diff --git a/compiler/ast/src/main/kotlin/com/uber/xprocessing/ext/XType.kt b/compiler/ast/src/main/kotlin/com/uber/xprocessing/ext/XType.kt index 86a7044c..1dd5a69c 100644 --- a/compiler/ast/src/main/kotlin/com/uber/xprocessing/ext/XType.kt +++ b/compiler/ast/src/main/kotlin/com/uber/xprocessing/ext/XType.kt @@ -172,10 +172,8 @@ fun XType.hash(): Int = } catch (t: Throwable) { if (typeArguments.any { it.typeName is WildcardTypeName && it.typeName.toString() != "?" }) { extendsBoundOrSelf().typeName.toString().hashCode() - } else if (this.hasCollectionType()) { - typeName.removeWildcardTypeIfContains().toString().hashCode() } else { - hashCode() + typeName.removeWildcardTypeIfContains().toString().hashCode() } } @@ -299,9 +297,5 @@ fun XType.isEnum(): Boolean = typeElement?.isEnum() ?: false fun XType.isPrimitive(): Boolean = typeName.isPrimitive -private fun XType.hasCollectionType(): Boolean = - this.typeElement?.name.orEmpty() in collectionTypes || - typeArguments.any { it.hasCollectionType() } - private val collectionTypes = setOf("MutableList", "MutableSet", "MutableCollection", "List", "Set", "Collection") diff --git a/compiler/ast/src/test/kotlin/motif/ast/compiler/CompilerTypeHashTest.kt b/compiler/ast/src/test/kotlin/motif/ast/compiler/CompilerTypeHashTest.kt new file mode 100644 index 00000000..312d65ed --- /dev/null +++ b/compiler/ast/src/test/kotlin/motif/ast/compiler/CompilerTypeHashTest.kt @@ -0,0 +1,91 @@ +/* + * Copyright (c) 2024 Uber Technologies, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package motif.ast.compiler + +import androidx.room.compiler.processing.ExperimentalProcessingApi +import androidx.room.compiler.processing.util.Source +import androidx.room.compiler.processing.util.runKspTest +import com.google.common.truth.Truth.assertThat +import com.uber.xprocessing.ext.hash +import com.uber.xprocessing.ext.isEquivalent +import org.junit.Test + +@OptIn(ExperimentalProcessingApi::class) +class CompilerTypeHashTest { + + @Test + fun hashEqualsContractForParameterizedType() { + val sources = + listOf( + Source.kotlin( + "test/Wrapper.kt", + """ + package test + class Wrapper(val value: T) + """ + .trimIndent(), + ), + Source.java( + "test.JavaUser", + """ + package test; + public class JavaUser { + public Wrapper provide() { return null; } + } + """ + .trimIndent(), + ), + Source.kotlin( + "test/KotlinUser.kt", + """ + package test + class KotlinUser { + fun provide(): Wrapper = Wrapper("s") + } + """ + .trimIndent(), + ), + ) + + runKspTest(sources) { invocation -> + val env = invocation.processingEnv + + val javaClass = env.findTypeElement("test.JavaUser")!! + val kotlinClass = env.findTypeElement("test.KotlinUser")!! + + val javaReturnType = javaClass.getDeclaredMethods().first { it.name == "provide" }.returnType + val kotlinReturnType = + kotlinClass.getDeclaredMethods().first { it.name == "provide" }.returnType + + assertThat(javaReturnType.isEquivalent(kotlinReturnType, env)).isTrue() + assertThat(javaReturnType.hash()).isEqualTo(kotlinReturnType.hash()) + + val type1 = CompilerType(env, javaReturnType) + val type2 = CompilerType(env, kotlinReturnType) + assertThat(type1).isEqualTo(type2) + assertThat(type1.hashCode()).isEqualTo(type2.hashCode()) + + val map = HashMap() + map[type1] = "first" + map.putIfAbsent(type2, "should-not-appear") + assertThat(map).hasSize(1) + assertThat(map[type1]).isEqualTo("first") + assertThat(map[type2]).isEqualTo("first") + + invocation.assertCompilationResult { hasErrorCount(0) } + } + } +}