diff --git a/compiler/fir/java/src/org/jetbrains/kotlin/fir/java/JavaScopeProvider.kt b/compiler/fir/java/src/org/jetbrains/kotlin/fir/java/JavaScopeProvider.kt index 53415d1054c..223d44f379f 100644 --- a/compiler/fir/java/src/org/jetbrains/kotlin/fir/java/JavaScopeProvider.kt +++ b/compiler/fir/java/src/org/jetbrains/kotlin/fir/java/JavaScopeProvider.kt @@ -102,7 +102,9 @@ object JavaScopeProvider : FirScopeProvider() { useSiteSession, JavaOverrideChecker( useSiteSession, - regularClass.javaTypeParameterStack + regularClass.javaTypeParameterStack, + baseScope = null, + considerReturnTypeKinds = false, ), superTypeScopes, regularClass.defaultType(), diff --git a/compiler/fir/java/src/org/jetbrains/kotlin/fir/java/scopes/JavaClassStaticUseSiteScope.kt b/compiler/fir/java/src/org/jetbrains/kotlin/fir/java/scopes/JavaClassStaticUseSiteScope.kt index 23380cb5065..ffdd5ed8ca3 100644 --- a/compiler/fir/java/src/org/jetbrains/kotlin/fir/java/scopes/JavaClassStaticUseSiteScope.kt +++ b/compiler/fir/java/src/org/jetbrains/kotlin/fir/java/scopes/JavaClassStaticUseSiteScope.kt @@ -23,7 +23,7 @@ class JavaClassStaticUseSiteScope internal constructor( ) : FirContainingNamesAwareScope() { private val functions = hashMapOf>() private val properties = hashMapOf>>() - private val overrideChecker = JavaOverrideChecker(session, javaTypeParameterStack) + private val overrideChecker = JavaOverrideChecker(session, javaTypeParameterStack, baseScope = null, considerReturnTypeKinds = false) override fun processFunctionsByName(name: Name, processor: (FirNamedFunctionSymbol) -> Unit) { functions.getOrPut(name) { diff --git a/compiler/fir/java/src/org/jetbrains/kotlin/fir/java/scopes/JavaClassUseSiteMemberScope.kt b/compiler/fir/java/src/org/jetbrains/kotlin/fir/java/scopes/JavaClassUseSiteMemberScope.kt index ac5ceee2ab2..23970279de6 100644 --- a/compiler/fir/java/src/org/jetbrains/kotlin/fir/java/scopes/JavaClassUseSiteMemberScope.kt +++ b/compiler/fir/java/src/org/jetbrains/kotlin/fir/java/scopes/JavaClassUseSiteMemberScope.kt @@ -45,7 +45,7 @@ class JavaClassUseSiteMemberScope( declaredMemberScope: FirContainingNamesAwareScope ) : AbstractFirUseSiteMemberScope( session, - JavaOverrideChecker(session, klass.javaTypeParameterStack), + JavaOverrideChecker(session, klass.javaTypeParameterStack, superTypesScope, considerReturnTypeKinds = true), superTypesScope, declaredMemberScope ) { diff --git a/compiler/fir/java/src/org/jetbrains/kotlin/fir/java/scopes/JavaOverrideChecker.kt b/compiler/fir/java/src/org/jetbrains/kotlin/fir/java/scopes/JavaOverrideChecker.kt index 95597725e04..15c73a5ed8e 100644 --- a/compiler/fir/java/src/org/jetbrains/kotlin/fir/java/scopes/JavaOverrideChecker.kt +++ b/compiler/fir/java/src/org/jetbrains/kotlin/fir/java/scopes/JavaOverrideChecker.kt @@ -16,15 +16,20 @@ import org.jetbrains.kotlin.fir.java.toConeKotlinTypeProbablyFlexible import org.jetbrains.kotlin.fir.resolve.fullyExpandedType import org.jetbrains.kotlin.fir.resolve.substitution.ConeSubstitutor import org.jetbrains.kotlin.fir.resolve.substitution.substitutorByMap +import org.jetbrains.kotlin.fir.scopes.FirTypeScope +import org.jetbrains.kotlin.fir.scopes.ProcessorAction import org.jetbrains.kotlin.fir.scopes.impl.FirAbstractOverrideChecker import org.jetbrains.kotlin.fir.scopes.jvm.computeJvmDescriptorRepresentation +import org.jetbrains.kotlin.fir.scopes.processOverriddenFunctions import org.jetbrains.kotlin.fir.symbols.ensureResolved import org.jetbrains.kotlin.fir.types.* import org.jetbrains.kotlin.name.StandardClassIds class JavaOverrideChecker internal constructor( private val session: FirSession, - private val javaTypeParameterStack: JavaTypeParameterStack + private val javaTypeParameterStack: JavaTypeParameterStack, + private val baseScope: FirTypeScope?, + private val considerReturnTypeKinds: Boolean, ) : FirAbstractOverrideChecker() { private val context: ConeTypeContext = session.typeContext @@ -74,7 +79,7 @@ class JavaOverrideChecker internal constructor( val candidateType = candidateTypeRef.toConeKotlinTypeProbablyFlexible(session, javaTypeParameterStack) val baseType = baseTypeRef.toConeKotlinTypeProbablyFlexible(session, javaTypeParameterStack) - if (candidateType.isPrimitiveInJava() != baseType.isPrimitiveInJava()) return false + if (candidateType.isPrimitiveInJava(isReturnType = false) != baseType.isPrimitiveInJava(isReturnType = false)) return false return isEqualTypes( candidateType, @@ -83,8 +88,50 @@ class JavaOverrideChecker internal constructor( ) } - private fun ConeKotlinType.isPrimitiveInJava(): Boolean = with(context) { - !isNullableType() && CompilerConeAttributes.EnhancedNullability !in attributes && isPrimitiveOrNullablePrimitive + // In most cases checking erasure of value parameters should be enough, but in some cases there might be semi-valid Java hierarchies + // with same value parameters, but different return type kinds, so it's worth distinguishing them as different non-overridable members + private fun doesReturnTypesHaveSameKind( + candidate: FirSimpleFunction, + base: FirSimpleFunction, + ): Boolean { + val candidateTypeRef = candidate.returnTypeRef + val baseTypeRef = base.returnTypeRef + + val candidateType = candidateTypeRef.toConeKotlinTypeProbablyFlexible(session, javaTypeParameterStack) + val baseType = baseTypeRef.toConeKotlinTypeProbablyFlexible(session, javaTypeParameterStack) + + val candidateHasPrimitiveReturnType = candidate.hasPrimitiveReturnTypeInJvm(candidateType) + if (candidateHasPrimitiveReturnType != base.hasPrimitiveReturnTypeInJvm(baseType)) return false + + // Both candidate and base are not primitive + if (!candidateHasPrimitiveReturnType) return true + + return (candidateType as? ConeClassLikeType)?.lookupTag == (baseType as? ConeClassLikeType)?.lookupTag + } + + private fun ConeKotlinType.isPrimitiveInJava(isReturnType: Boolean): Boolean = with(context) { + if (isNullableType() || CompilerConeAttributes.EnhancedNullability in attributes) return false + + val isVoid = isReturnType && isUnit + return isPrimitiveOrNullablePrimitive || isVoid + } + + private fun FirSimpleFunction.hasPrimitiveReturnTypeInJvm(returnType: ConeKotlinType): Boolean { + if (!returnType.isPrimitiveInJava(isReturnType = true)) return false + + var foundNonPrimitiveOverridden = false + + baseScope?.processOverriddenFunctions(symbol) { + val type = it.fir.returnTypeRef.toConeKotlinTypeProbablyFlexible(session, javaTypeParameterStack) + if (!type.isPrimitiveInJava(isReturnType = true)) { + foundNonPrimitiveOverridden = true + ProcessorAction.STOP + } else { + ProcessorAction.NEXT + } + } + + return !foundNonPrimitiveOverridden } private fun isEqualArrayElementTypeProjections( @@ -176,9 +223,16 @@ class JavaOverrideChecker internal constructor( if (overrideCandidate.valueParameters.size != baseParameterTypes.size) return false val substitutor = buildTypeParametersSubstitutorIfCompatible(overrideCandidate, baseDeclaration) - return overrideCandidate.valueParameters.zip(baseParameterTypes).all { (paramFromJava, baseType) -> - isEqualTypes(paramFromJava.returnTypeRef, baseType, substitutor) + + if (!overrideCandidate.valueParameters.zip(baseParameterTypes).all { (paramFromJava, baseType) -> + isEqualTypes(paramFromJava.returnTypeRef, baseType, substitutor) + }) { + return false } + + if (considerReturnTypeKinds && !doesReturnTypesHaveSameKind(overrideCandidate, baseDeclaration)) return false + + return true } override fun isOverriddenProperty(overrideCandidate: FirCallableDeclaration, baseDeclaration: FirProperty): Boolean { diff --git a/compiler/testData/compileKotlinAgainstCustomBinaries/firIncorrectJavaSignature/A.java b/compiler/testData/compileKotlinAgainstCustomBinaries/firIncorrectJavaSignature/A.java new file mode 100644 index 00000000000..81226999ec0 --- /dev/null +++ b/compiler/testData/compileKotlinAgainstCustomBinaries/firIncorrectJavaSignature/A.java @@ -0,0 +1,6 @@ +package test; +import java.util.List; +public class A { + public T foo(List t) { return null; } + public boolean foo(List t) { return false; } +} diff --git a/compiler/testData/compileKotlinAgainstCustomBinaries/firIncorrectJavaSignature/B.java b/compiler/testData/compileKotlinAgainstCustomBinaries/firIncorrectJavaSignature/B.java new file mode 100644 index 00000000000..d948c9b2687 --- /dev/null +++ b/compiler/testData/compileKotlinAgainstCustomBinaries/firIncorrectJavaSignature/B.java @@ -0,0 +1,6 @@ +package test; +import java.util.List; +public class B extends A { + public T foo(List t) { return null; } + public boolean foo(List t) { return false; } +} diff --git a/compiler/testData/compileKotlinAgainstCustomBinaries/firIncorrectJavaSignature/output.txt b/compiler/testData/compileKotlinAgainstCustomBinaries/firIncorrectJavaSignature/output.txt new file mode 100644 index 00000000000..d86bac9de59 --- /dev/null +++ b/compiler/testData/compileKotlinAgainstCustomBinaries/firIncorrectJavaSignature/output.txt @@ -0,0 +1 @@ +OK diff --git a/compiler/testData/compileKotlinAgainstCustomBinaries/firIncorrectJavaSignature/source.kt b/compiler/testData/compileKotlinAgainstCustomBinaries/firIncorrectJavaSignature/source.kt new file mode 100644 index 00000000000..cc63ce63e03 --- /dev/null +++ b/compiler/testData/compileKotlinAgainstCustomBinaries/firIncorrectJavaSignature/source.kt @@ -0,0 +1,4 @@ +package test +fun main() { + object : B() {} +} diff --git a/compiler/tests-common/tests/org/jetbrains/kotlin/jvm/compiler/AbstractKotlinCompilerIntegrationTest.kt b/compiler/tests-common/tests/org/jetbrains/kotlin/jvm/compiler/AbstractKotlinCompilerIntegrationTest.kt index 7e85c2c0932..3994f33d244 100644 --- a/compiler/tests-common/tests/org/jetbrains/kotlin/jvm/compiler/AbstractKotlinCompilerIntegrationTest.kt +++ b/compiler/tests-common/tests/org/jetbrains/kotlin/jvm/compiler/AbstractKotlinCompilerIntegrationTest.kt @@ -150,13 +150,16 @@ abstract class AbstractKotlinCompilerIntegrationTest : TestCaseWithTmpdir() { classpath: List = emptyList(), compiler: CLICompiler<*> = K2JVMCompiler(), additionalOptions: List = emptyList(), - expectedFileName: String? = "output.txt" + expectedFileName: String? = "output.txt", + additionalSources: List = emptyList(), ): Pair { val args = mutableListOf() val sourceFile = File(testDataDirectory, fileName) assert(sourceFile.exists()) { "Source file does not exist: ${sourceFile.absolutePath}" } args.add(sourceFile.path) + additionalSources.mapTo(args) { File(testDataDirectory, it).path } + if (compiler is K2JSCompiler) { if (classpath.isNotEmpty()) { args.add("-libraries") diff --git a/compiler/tests/org/jetbrains/kotlin/jvm/compiler/CompileKotlinAgainstCustomBinariesTest.kt b/compiler/tests/org/jetbrains/kotlin/jvm/compiler/CompileKotlinAgainstCustomBinariesTest.kt index b442f3453b6..e432a90c32b 100644 --- a/compiler/tests/org/jetbrains/kotlin/jvm/compiler/CompileKotlinAgainstCustomBinariesTest.kt +++ b/compiler/tests/org/jetbrains/kotlin/jvm/compiler/CompileKotlinAgainstCustomBinariesTest.kt @@ -612,6 +612,15 @@ class CompileKotlinAgainstCustomBinariesTest : AbstractKotlinCompilerIntegration compileKotlin("source.kt", tmpdir, listOf(library), additionalOptions = listOf("-Xuse-fir")) } + fun testFirIncorrectJavaSignature() { + compileKotlin( + "source.kt", tmpdir, + listOf(), + additionalOptions = listOf("-Xuse-fir"), + additionalSources = listOf("A.java", "B.java"), + ) + } + fun testOldJvmAgainstJvmIr() { val library = compileLibrary("library", additionalOptions = listOf("-Xuse-ir")) compileKotlin("source.kt", tmpdir, listOf(library))