K2: add ACCIDENTAL_OVERRIDE_CLASH_BY_JVM_SIGNATURE for clashing functions

This diagnostic is reported on an explicitly-declared function which overrides a regular Java function with the same signature as a hidden one from the same scope. #KT-64846 Fixed
2024-01-22 18:41:45 +01:00
parent 8f9614c1fb
commit 7615d863d5
19 changed files with 138 additions and 4 deletions
@@ -5099,6 +5099,15 @@ internal val KT_DIAGNOSTIC_CONVERTER = KtDiagnosticConverterBuilder.buildConvert
            token,
        )
    }
+    add(FirJvmErrors.ACCIDENTAL_OVERRIDE_CLASH_BY_JVM_SIGNATURE) { firDiagnostic ->
+        AccidentalOverrideClashByJvmSignatureImpl(
+            firSymbolBuilder.functionLikeBuilder.buildFunctionSymbol(firDiagnostic.a),
+            firDiagnostic.b,
+            firSymbolBuilder.functionLikeBuilder.buildFunctionSymbol(firDiagnostic.c),
+            firDiagnostic as KtPsiDiagnostic,
+            token,
+        )
+    }
    add(FirJvmErrors.JAVA_TYPE_MISMATCH) { firDiagnostic ->
        JavaTypeMismatchImpl(
            firSymbolBuilder.typeBuilder.buildKtType(firDiagnostic.a),
@@ -3548,6 +3548,13 @@ sealed interface KtFirDiagnostic<PSI : PsiElement> : KtDiagnosticWithPsi<PSI> {
        val base: KtCallableSymbol
    }

+    interface AccidentalOverrideClashByJvmSignature : KtFirDiagnostic<KtNamedFunction> {
+        override val diagnosticClass get() = AccidentalOverrideClashByJvmSignature::class
+        val hidden: KtFunctionLikeSymbol
+        val overrideDescription: String
+        val regular: KtFunctionLikeSymbol
+    }
+
    interface JavaTypeMismatch : KtFirDiagnostic<KtExpression> {
        override val diagnosticClass get() = JavaTypeMismatch::class
        val expectedType: KtType
@@ -4281,6 +4281,14 @@ internal class WrongNullabilityForJavaOverrideImpl(
    token: KtLifetimeToken,
 ) : KtAbstractFirDiagnostic<PsiElement>(firDiagnostic, token), KtFirDiagnostic.WrongNullabilityForJavaOverride

+internal class AccidentalOverrideClashByJvmSignatureImpl(
+    override val hidden: KtFunctionLikeSymbol,
+    override val overrideDescription: String,
+    override val regular: KtFunctionLikeSymbol,
+    firDiagnostic: KtPsiDiagnostic,
+    token: KtLifetimeToken,
+) : KtAbstractFirDiagnostic<KtNamedFunction>(firDiagnostic, token), KtFirDiagnostic.AccidentalOverrideClashByJvmSignature
+
 internal class JavaTypeMismatchImpl(
    override val expectedType: KtType,
    override val actualType: KtType,
@@ -20,6 +20,7 @@ import org.jetbrains.kotlin.psi.KtAnnotationEntry
 import org.jetbrains.kotlin.psi.KtDeclaration
 import org.jetbrains.kotlin.psi.KtElement
 import org.jetbrains.kotlin.psi.KtExpression
+import org.jetbrains.kotlin.psi.KtNamedFunction
 import org.jetbrains.kotlin.util.PrivateForInline

@Suppress("ClassName", "unused")
@@ -45,6 +46,12 @@ object JVM_DIAGNOSTICS_LIST : DiagnosticList("FirJvmErrors") {
            parameter<FirCallableSymbol<*>>("override")
            parameter<FirCallableSymbol<*>>("base")
        }
+
+        val ACCIDENTAL_OVERRIDE_CLASH_BY_JVM_SIGNATURE by error<KtNamedFunction>(PositioningStrategy.DECLARATION_NAME) {
+            parameter<FirNamedFunctionSymbol>("hidden")
+            parameter<String>("overrideDescription")
+            parameter<FirNamedFunctionSymbol>("regular")
+        }
    }

    val TYPES by object : DiagnosticGroup("Types") {
@@ -32,6 +32,7 @@ import org.jetbrains.kotlin.psi.KtAnnotationEntry
 import org.jetbrains.kotlin.psi.KtDeclaration
 import org.jetbrains.kotlin.psi.KtElement
 import org.jetbrains.kotlin.psi.KtExpression
+import org.jetbrains.kotlin.psi.KtNamedFunction

 /**
 * Generated from: [org.jetbrains.kotlin.fir.checkers.generator.diagnostics.JVM_DIAGNOSTICS_LIST]
@@ -50,6 +51,7 @@ object FirJvmErrors {
    val VALUE_CLASS_WITHOUT_JVM_INLINE_ANNOTATION: KtDiagnosticFactory0 by error0<PsiElement>()
    val JVM_INLINE_WITHOUT_VALUE_CLASS: KtDiagnosticFactory0 by error0<PsiElement>()
    val WRONG_NULLABILITY_FOR_JAVA_OVERRIDE: KtDiagnosticFactory2<FirCallableSymbol<*>, FirCallableSymbol<*>> by warning2<PsiElement, FirCallableSymbol<*>, FirCallableSymbol<*>>(SourceElementPositioningStrategies.OVERRIDE_MODIFIER)
+    val ACCIDENTAL_OVERRIDE_CLASH_BY_JVM_SIGNATURE: KtDiagnosticFactory3<FirNamedFunctionSymbol, String, FirNamedFunctionSymbol> by error3<KtNamedFunction, FirNamedFunctionSymbol, String, FirNamedFunctionSymbol>(SourceElementPositioningStrategies.DECLARATION_NAME)

    // Types
    val JAVA_TYPE_MISMATCH: KtDiagnosticFactory2<ConeKotlinType, ConeKotlinType> by error2<KtExpression, ConeKotlinType, ConeKotlinType>()
@@ -12,9 +12,11 @@ import org.jetbrains.kotlin.diagnostics.rendering.BaseDiagnosticRendererFactory
 import org.jetbrains.kotlin.diagnostics.rendering.CommonRenderers.NAME
 import org.jetbrains.kotlin.diagnostics.rendering.CommonRenderers.STRING
 import org.jetbrains.kotlin.fir.analysis.diagnostics.FirDiagnosticRenderers.DECLARATION_NAME
+import org.jetbrains.kotlin.fir.analysis.diagnostics.FirDiagnosticRenderers.FQ_NAMES_IN_TYPES
 import org.jetbrains.kotlin.fir.analysis.diagnostics.FirDiagnosticRenderers.OPTIONAL_SENTENCE
 import org.jetbrains.kotlin.fir.analysis.diagnostics.FirDiagnosticRenderers.RENDER_TYPE
 import org.jetbrains.kotlin.fir.analysis.diagnostics.FirDiagnosticRenderers.SYMBOL
+import org.jetbrains.kotlin.fir.analysis.diagnostics.jvm.FirJvmErrors.ACCIDENTAL_OVERRIDE_CLASH_BY_JVM_SIGNATURE
 import org.jetbrains.kotlin.fir.analysis.diagnostics.jvm.FirJvmErrors.CONCURRENT_HASH_MAP_CONTAINS_OPERATOR
 import org.jetbrains.kotlin.fir.analysis.diagnostics.jvm.FirJvmErrors.DELEGATION_BY_IN_JVM_RECORD
 import org.jetbrains.kotlin.fir.analysis.diagnostics.jvm.FirJvmErrors.DEPRECATED_JAVA_ANNOTATION
@@ -115,6 +117,15 @@ object FirJvmErrorsDefaultMessages : BaseDiagnosticRendererFactory() {
            SYMBOL,
        )

+        map.put(
+            ACCIDENTAL_OVERRIDE_CLASH_BY_JVM_SIGNATURE,
+            "This function accidentally overrides both ''{0}'' and {1} ''{2}'' from JVM point of view because of mixed Java/Kotlin hierarchy.\n" +
+                    "This situation provokes a JVM clash and thus is forbidden. To fix it, you have to delete either this function or one of overridden functions.",
+            FQ_NAMES_IN_TYPES,
+            STRING,
+            FQ_NAMES_IN_TYPES,
+        )
+
        map.put(
            UPPER_BOUND_VIOLATED_BASED_ON_JAVA_ANNOTATIONS,
            "Type argument is not within its bounds: should be subtype of ''{0}''.",
@@ -58,6 +58,7 @@ object JvmDeclarationCheckers : DeclarationCheckers() {
    override val simpleFunctionCheckers: Set<FirSimpleFunctionChecker>
        get() = setOf(
            FirInlineBodySimpleFunctionChecker,
+            FirAccidentalOverrideClashChecker,
        )

    override val valueParameterCheckers: Set<FirValueParameterChecker>
@@ -0,0 +1,59 @@
+/*
+ * Copyright 2010-2024 JetBrains s.r.o. and Kotlin Programming Language contributors.
+ * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file.
+ */
+
+package org.jetbrains.kotlin.fir.analysis.jvm.checkers.declaration
+
+import org.jetbrains.kotlin.diagnostics.DiagnosticReporter
+import org.jetbrains.kotlin.diagnostics.reportOn
+import org.jetbrains.kotlin.fir.analysis.checkers.MppCheckerKind
+import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext
+import org.jetbrains.kotlin.fir.analysis.checkers.declaration.FirSimpleFunctionChecker
+import org.jetbrains.kotlin.fir.analysis.checkers.unsubstitutedScope
+import org.jetbrains.kotlin.fir.analysis.diagnostics.jvm.FirJvmErrors.ACCIDENTAL_OVERRIDE_CLASH_BY_JVM_SIGNATURE
+import org.jetbrains.kotlin.fir.declarations.FirSimpleFunction
+import org.jetbrains.kotlin.fir.declarations.isHiddenToOvercomeSignatureClash
+import org.jetbrains.kotlin.fir.declarations.utils.isOverride
+import org.jetbrains.kotlin.fir.initialSignatureAttr
+import org.jetbrains.kotlin.fir.resolve.getContainingClass
+import org.jetbrains.kotlin.fir.scopes.jvm.computeJvmDescriptor
+import org.jetbrains.kotlin.fir.symbols.SymbolInternals
+import org.jetbrains.kotlin.load.java.SpecialGenericSignatures.Companion.JVM_SHORT_NAME_TO_BUILTIN_SHORT_NAMES_MAP
+import org.jetbrains.kotlin.load.java.SpecialGenericSignatures.Companion.sameAsBuiltinMethodWithErasedValueParameters
+
+object FirAccidentalOverrideClashChecker : FirSimpleFunctionChecker(MppCheckerKind.Platform) {
+    override fun check(
+        declaration: FirSimpleFunction,
+        context: CheckerContext,
+        reporter: DiagnosticReporter,
+    ) {
+        if (!declaration.isOverride) return
+        val name = declaration.name
+        val mayBeRenamedBuiltIn = name in namesPossibleForRenamedBuiltin
+        val mayBeSameAsBuiltInWithErasedParameters = name.sameAsBuiltinMethodWithErasedValueParameters
+        if (!mayBeRenamedBuiltIn && !mayBeSameAsBuiltInWithErasedParameters) return
+        val containingClass = declaration.getContainingClass(context.session) ?: return
+
+        var reported = false
+        containingClass.unsubstitutedScope(context).processFunctionsByName(name) {
+            @OptIn(SymbolInternals::class)
+            val hiddenFir = it.fir
+            if (!reported && hiddenFir.isHiddenToOvercomeSignatureClash == true) {
+                if (declaration.computeJvmDescriptor() == hiddenFir.computeJvmDescriptor()) {
+                    val regularBase = hiddenFir.initialSignatureAttr as? FirSimpleFunction ?: return@processFunctionsByName
+                    val description = when {
+                        mayBeRenamedBuiltIn -> "a renamed function"
+                        else -> "a function with erased parameters"
+                    }
+                    reporter.reportOn(
+                        declaration.source, ACCIDENTAL_OVERRIDE_CLASH_BY_JVM_SIGNATURE, it, description, regularBase.symbol, context
+                    )
+                    reported = true
+                }
+            }
+        }
+    }
+
+    private val namesPossibleForRenamedBuiltin = JVM_SHORT_NAME_TO_BUILTIN_SHORT_NAMES_MAP.values.toSet()
+}
@@ -589,6 +589,7 @@ val FIR_NON_SUPPRESSIBLE_ERROR_NAMES: Set<String> = setOf(
    "FUNCTION_DELEGATE_MEMBER_NAME_CLASH",
    "VALUE_CLASS_WITHOUT_JVM_INLINE_ANNOTATION",
    "JVM_INLINE_WITHOUT_VALUE_CLASS",
+    "ACCIDENTAL_OVERRIDE_CLASH_BY_JVM_SIGNATURE",
    "JAVA_TYPE_MISMATCH",
    "UPPER_BOUND_CANNOT_BE_ARRAY",
    "STRICTFP_ON_CLASS",
@@ -565,7 +565,7 @@ class JavaClassUseSiteMemberScope(
                symbol = newSymbol
                dispatchReceiverType = klass.defaultType()
            }.apply {
-                initialSignatureAttr = original
+                initialSignatureAttr = explicitlyDeclaredFunctionWithErasedValueParameters.fir
                isHiddenToOvercomeSignatureClash = true
            }
            // Collect synthetic function which is a hidden copy of declared one with unerased parameters
@@ -0,0 +1 @@
+
@@ -0,0 +1,2 @@
+/main.kt:(78,81): error: This function accidentally overrides both 'public open operator fun /A.get(index: kotlin/Int): kotlin/Char' and a renamed function 'public open fun /A.charAt(index: kotlin/Int): kotlin/Char' from JVM point of view because of mixed Java/Kotlin hierarchy.
+This situation provokes a JVM clash and thus is forbidden. To fix it, you have to delete either this function or one of overridden functions.
@@ -1,4 +1,5 @@
 // SCOPE_DUMP: KA:get
+// RENDER_DIAGNOSTICS_FULL_TEXT

 // FILE: A.java
 abstract public class A implements CharSequence {
@@ -17,7 +18,7 @@ abstract public class A implements CharSequence {

 // FILE: main.kt
 abstract class KA : A() {
-    override fun get(index: Int) = 'O'
+    override fun <!ACCIDENTAL_OVERRIDE_CLASH_BY_JVM_SIGNATURE!>get<!>(index: Int) = 'O'
 }

 fun foo(a: A, ka: KA) {
@@ -1,4 +1,5 @@
 // SCOPE_DUMP: KA:get
+// RENDER_DIAGNOSTICS_FULL_TEXT

 // FILE: A.java
 abstract public class A implements CharSequence {
@@ -0,0 +1,12 @@
+/main.kt:18:16: error: the integer literal does not conform to the expected type String!
+    a.contains(1)
+               ^
+/main.kt:20:5: error: the integer literal does not conform to the expected type String!
+    1 in a
+    ^
+/main.kt:23:17: error: the integer literal does not conform to the expected type String
+    ka.contains(1)
+                ^
+/main.kt:25:5: error: the integer literal does not conform to the expected type String
+    1 in ka
+    ^
@@ -0,0 +1,10 @@
+/main.kt:(70,78): error: This function accidentally overrides both 'public open operator fun /A.contains(x: kotlin/String!): kotlin/Boolean' and a function with erased parameters 'public open operator fun /A.contains(x: kotlin/Any!): kotlin/Boolean' from JVM point of view because of mixed Java/Kotlin hierarchy.
+This situation provokes a JVM clash and thus is forbidden. To fix it, you have to delete either this function or one of overridden functions.
+
+/main.kt:(159,160): error: Argument type mismatch: actual type is 'kotlin.Int', but 'kotlin.String!' was expected.
+
+/main.kt:(178,179): error: Argument type mismatch: actual type is 'kotlin.Int', but 'kotlin.String!' was expected.
+
+/main.kt:(222,223): error: Argument type mismatch: actual type is 'kotlin.Int', but 'kotlin.String' was expected.
+
+/main.kt:(242,243): error: Argument type mismatch: actual type is 'kotlin.Int', but 'kotlin.String' was expected.
@@ -1,5 +1,6 @@
 // !DIAGNOSTICS: -UNUSED_PARAMETER -UNUSED_VARIABLE -PARAMETER_NAME_CHANGED_ON_OVERRIDE
 // SCOPE_DUMP: KA:contains
+// RENDER_DIAGNOSTICS_FULL_TEXT

 // FILE: A.java
 abstract public class A implements java.util.Collection<String> {
@@ -9,7 +10,7 @@ abstract public class A implements java.util.Collection<String> {

 // FILE: main.kt
 abstract class KA : A() {
-    override fun contains(x: String) = false
+    override fun <!ACCIDENTAL_OVERRIDE_CLASH_BY_JVM_SIGNATURE!>contains<!>(x: String) = false
 }

 fun foo(a: A, ka: KA) {
@@ -1,5 +1,6 @@
 // !DIAGNOSTICS: -UNUSED_PARAMETER -UNUSED_VARIABLE -PARAMETER_NAME_CHANGED_ON_OVERRIDE
 // SCOPE_DUMP: KA:contains
+// RENDER_DIAGNOSTICS_FULL_TEXT

 // FILE: A.java
 abstract public class A implements java.util.Collection<String> {
@@ -146,7 +146,7 @@ public class X implements java.util.List<String> {

 class Y : X()
 class Y2 : X() {
-    override fun removeAt(index: Int) = ""
+    override fun <!ACCIDENTAL_OVERRIDE_CLASH_BY_JVM_SIGNATURE!>removeAt<!>(index: Int) = ""
 }

 fun main() {