FIR checker: fix positions of type mismatch on overrides

To handle implicit return types, those should be reported on
declarations with return type positioning strategy, instead of
return type itself.

compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirTypeMismatchOnOverrideChecker.kt

@@ -5,7 +5,6 @@
 
 package org.jetbrains.kotlin.fir.analysis.checkers.declaration
 
-import org.jetbrains.kotlin.fir.FirSourceElement
 import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext
 import org.jetbrains.kotlin.fir.analysis.diagnostics.DiagnosticReporter
 import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors
@@ -18,10 +17,7 @@ import org.jetbrains.kotlin.fir.scopes.impl.toConeType
 import org.jetbrains.kotlin.fir.scopes.unsubstitutedScope
 import org.jetbrains.kotlin.fir.symbols.impl.*
 import org.jetbrains.kotlin.fir.typeContext
-import org.jetbrains.kotlin.fir.types.ConeKotlinType
+import org.jetbrains.kotlin.fir.types.*
-import org.jetbrains.kotlin.fir.types.FirResolvedTypeRef
-import org.jetbrains.kotlin.fir.types.coneType
-import org.jetbrains.kotlin.fir.types.upperBoundIfFlexible
 import org.jetbrains.kotlin.types.AbstractTypeChecker
 import org.jetbrains.kotlin.types.AbstractTypeCheckerContext
 import org.jetbrains.kotlin.utils.addToStdlib.min
@@ -92,6 +88,11 @@ object FirTypeMismatchOnOverrideChecker : FirRegularClassChecker() {
         val returnType = returnTypeRef.safeAs<FirResolvedTypeRef>()?.type
             ?: return null
 
+        // Don't report *_ON_OVERRIDE diagnostics according to an error return type. That should be reported separately.
+        if (returnType is ConeKotlinErrorType) {
+            return null
+        }
+
         val bounds = overriddenSymbols.map { context.returnTypeCalculator.tryCalculateReturnType(it.fir).coneType.upperBoundIfFlexible() }
 
         for (it in bounds.indices) {
@@ -130,11 +131,7 @@ object FirTypeMismatchOnOverrideChecker : FirRegularClassChecker() {
         )
 
         restriction?.let {
-            reporter.reportMismatchOnFunction(
+            reporter.reportReturnTypeMismatchOnFunction(function, it)
-                function.returnTypeRef.source,
-                function.returnTypeRef.coneType.toString(),
-                it
-            )
         }
     }
 
@@ -163,30 +160,31 @@ object FirTypeMismatchOnOverrideChecker : FirRegularClassChecker() {
 
         restriction?.let {
             if (property.isVar) {
-                reporter.reportMismatchOnVariable(
+                reporter.reportTypeMismatchOnVariable(property, it)
-                    property.returnTypeRef.source,
-                    property.returnTypeRef.coneType.toString(),
-                    it
-                )
             } else {
-                reporter.reportMismatchOnProperty(
+                reporter.reportTypeMismatchOnProperty(property, it)
-                    property.returnTypeRef.source,
-                    property.returnTypeRef.coneType.toString(),
-                    it
-                )
             }
         }
     }
 
-    private fun DiagnosticReporter.reportMismatchOnFunction(source: FirSourceElement?, type: String, declaration: FirMemberDeclaration) {
+    private fun DiagnosticReporter.reportReturnTypeMismatchOnFunction(
-        source?.let { report(FirErrors.RETURN_TYPE_MISMATCH_ON_OVERRIDE.on(it, type, declaration)) }
+        overriding: FirMemberDeclaration,
+        overridden: FirMemberDeclaration
+    ) {
+        overriding.source?.let { report(FirErrors.RETURN_TYPE_MISMATCH_ON_OVERRIDE.on(it, overriding, overridden)) }
     }
 
-    private fun DiagnosticReporter.reportMismatchOnProperty(source: FirSourceElement?, type: String, declaration: FirMemberDeclaration) {
+    private fun DiagnosticReporter.reportTypeMismatchOnProperty(
-        source?.let { report(FirErrors.PROPERTY_TYPE_MISMATCH_ON_OVERRIDE.on(it, type, declaration)) }
+        overriding: FirMemberDeclaration,
+        overridden: FirMemberDeclaration
+    ) {
+        overriding.source?.let { report(FirErrors.PROPERTY_TYPE_MISMATCH_ON_OVERRIDE.on(it, overriding, overridden)) }
     }
 
-    private fun DiagnosticReporter.reportMismatchOnVariable(source: FirSourceElement?, type: String, declaration: FirMemberDeclaration) {
+    private fun DiagnosticReporter.reportTypeMismatchOnVariable(
-        source?.let { report(FirErrors.VAR_TYPE_MISMATCH_ON_OVERRIDE.on(it, type, declaration)) }
+        overriding: FirMemberDeclaration,
+        overridden: FirMemberDeclaration
+    ) {
+        overriding.source?.let { report(FirErrors.VAR_TYPE_MISMATCH_ON_OVERRIDE.on(it, overriding, overridden)) }
     }
 }

compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/FirDefaultErrorMessages.kt

@@ -9,6 +9,7 @@ import org.jetbrains.kotlin.diagnostics.rendering.DefaultErrorMessages
 import org.jetbrains.kotlin.diagnostics.rendering.DiagnosticFactoryToRendererMap
 import org.jetbrains.kotlin.fir.analysis.diagnostics.FirDiagnosticRenderers.AMBIGUOUS_CALLS
 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.NULLABLE_STRING
 import org.jetbrains.kotlin.fir.analysis.diagnostics.FirDiagnosticRenderers.PROPERTY_NAME
 import org.jetbrains.kotlin.fir.analysis.diagnostics.FirDiagnosticRenderers.RENDER_TYPE
@@ -322,35 +323,28 @@ class FirDefaultErrorMessages : DefaultErrorMessages.Extension {
                 VARIANCE_ON_TYPE_PARAMETER_NOT_ALLOWED,
                 "Variance annotations are only allowed for type parameters of classes and interfaces"
             )
+            map.put(CATCH_PARAMETER_WITH_DEFAULT_VALUE, "Catch clause parameter may not have a default value")
+            map.put(REIFIED_TYPE_IN_CATCH_CLAUSE, "Reified type is forbidden for catch parameter")
+            map.put(TYPE_PARAMETER_IN_CATCH_CLAUSE, "Type parameter is forbidden for catch parameter")
+
+            // Overrides
             map.put(
                 RETURN_TYPE_MISMATCH_ON_OVERRIDE,
                 "Return type of ''{0}'' is not a subtype of the return type of the overridden member ''{1}''",
-                TO_STRING,
+                DECLARATION_NAME,
-                DECLARATION_NAME
+                FQ_NAMES_IN_TYPES
-            ) // #
+            )
             map.put(
                 PROPERTY_TYPE_MISMATCH_ON_OVERRIDE,
                 "Type of ''{0}'' is not a subtype of the overridden property ''{1}''",
-                TO_STRING,
+                DECLARATION_NAME,
-                DECLARATION_NAME
+                FQ_NAMES_IN_TYPES
-            ) // #
+            )
             map.put(
                 VAR_TYPE_MISMATCH_ON_OVERRIDE,
                 "Type of ''{0}'' doesn''t match the type of the overridden var-property ''{1}''",
-                TO_STRING,
+                DECLARATION_NAME,
-                DECLARATION_NAME
+                FQ_NAMES_IN_TYPES
-            ) // #
-            map.put(
-                CATCH_PARAMETER_WITH_DEFAULT_VALUE,
-                "Catch clause parameter may not have a default value"
-            )
-            map.put(
-                REIFIED_TYPE_IN_CATCH_CLAUSE,
-                "Reified type is forbidden for catch parameter"
-            )
-            map.put(
-                TYPE_PARAMETER_IN_CATCH_CLAUSE,
-                "Type parameter is forbidden for catch parameter"
             )
 
             // Redeclarations

compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/FirDiagnosticRenderers.kt

@@ -7,8 +7,10 @@ package org.jetbrains.kotlin.fir.analysis.diagnostics
 
 import org.jetbrains.kotlin.diagnostics.rendering.Renderer
 import org.jetbrains.kotlin.fir.FirElement
+import org.jetbrains.kotlin.fir.FirRenderer
 import org.jetbrains.kotlin.fir.declarations.*
 import org.jetbrains.kotlin.fir.render
+import org.jetbrains.kotlin.fir.renderWithType
 import org.jetbrains.kotlin.fir.symbols.AbstractFirBasedSymbol
 import org.jetbrains.kotlin.fir.symbols.impl.FirCallableSymbol
 import org.jetbrains.kotlin.fir.symbols.impl.FirClassLikeSymbol
@@ -64,6 +66,10 @@ object FirDiagnosticRenderers {
         t.render()
     }
 
+    val FQ_NAMES_IN_TYPES = Renderer { element: FirElement ->
+        element.renderWithType(mode = FirRenderer.RenderMode.WithFqNamesExceptAnnotation)
+    }
+
     val AMBIGUOUS_CALLS = Renderer { candidates: Collection<AbstractFirBasedSymbol<*>> ->
         candidates.joinToString(separator = "\n", prefix = "\n") { symbol ->
             SYMBOL.render(symbol)

compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrors.kt

@@ -132,13 +132,15 @@ object FirErrors {
     val TYPE_PARAMETERS_IN_ENUM by error0<FirSourceElement, PsiElement>()
     val CONFLICTING_PROJECTION by error1<FirSourceElement, PsiElement, String>()
     val VARIANCE_ON_TYPE_PARAMETER_NOT_ALLOWED by error0<FirSourceElement, PsiElement>()
-    val RETURN_TYPE_MISMATCH_ON_OVERRIDE by error2<FirSourceElement, PsiElement, String, FirMemberDeclaration>()
-    val PROPERTY_TYPE_MISMATCH_ON_OVERRIDE by error2<FirSourceElement, PsiElement, String, FirMemberDeclaration>()
-    val VAR_TYPE_MISMATCH_ON_OVERRIDE by error2<FirSourceElement, PsiElement, String, FirMemberDeclaration>()
     val CATCH_PARAMETER_WITH_DEFAULT_VALUE by error0<FirSourceElement, PsiElement>()
     val REIFIED_TYPE_IN_CATCH_CLAUSE by error0<FirSourceElement, PsiElement>()
     val TYPE_PARAMETER_IN_CATCH_CLAUSE by error0<FirSourceElement, PsiElement>()
 
+    // Overrides
+    val RETURN_TYPE_MISMATCH_ON_OVERRIDE by error2<FirSourceElement, KtNamedDeclaration, FirMemberDeclaration, FirMemberDeclaration>(SourceElementPositioningStrategies.DECLARATION_RETURN_TYPE)
+    val PROPERTY_TYPE_MISMATCH_ON_OVERRIDE by error2<FirSourceElement, KtNamedDeclaration, FirMemberDeclaration, FirMemberDeclaration>(SourceElementPositioningStrategies.DECLARATION_RETURN_TYPE)
+    val VAR_TYPE_MISMATCH_ON_OVERRIDE by error2<FirSourceElement, KtNamedDeclaration, FirMemberDeclaration, FirMemberDeclaration>(SourceElementPositioningStrategies.DECLARATION_RETURN_TYPE)
+
     // Redeclarations
     val MANY_COMPANION_OBJECTS by error0<FirSourceElement, PsiElement>()
     val CONFLICTING_OVERLOADS by error1<FirSourceElement, PsiElement, String>(SourceElementPositioningStrategies.DECLARATION_SIGNATURE_OR_DEFAULT)

compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/LightTreePositioningStrategies.kt

@@ -91,6 +91,31 @@ object LightTreePositioningStrategies {
         }
     }
 
+    val DECLARATION_RETURN_TYPE: LightTreePositioningStrategy = object : LightTreePositioningStrategy() {
+        override fun mark(
+            node: LighterASTNode,
+            startOffset: Int,
+            endOffset: Int,
+            tree: FlyweightCapableTreeStructure<LighterASTNode>
+        ): List<TextRange> {
+            val (returnTypeRef, nameIdentifierOrPlaceHolder) = when {
+                node.tokenType == KtNodeTypes.PROPERTY_ACCESSOR ->
+                    tree.typeReference(node) to tree.accessorNamePlaceholder(node)
+                node.isDeclaration ->
+                    tree.typeReference(node) to tree.nameIdentifier(node)
+                else ->
+                    null to null
+            }
+            if (returnTypeRef != null) {
+                return markElement(returnTypeRef, startOffset, endOffset, tree, node)
+            }
+            if (nameIdentifierOrPlaceHolder != null) {
+                return markElement(nameIdentifierOrPlaceHolder, startOffset, endOffset, tree, node)
+            }
+            return DEFAULT.mark(node, startOffset, endOffset, tree)
+        }
+    }
+
     val DECLARATION_NAME: LightTreePositioningStrategy = object : LightTreePositioningStrategy() {
         override fun mark(
             node: LighterASTNode,
@@ -188,23 +213,23 @@ object LightTreePositioningStrategies {
             } else {
                 DEFAULT.mark(node, startOffset, endOffset, tree)
             }
-
-        private val LighterASTNode.isDeclaration: Boolean
-            get() =
-                when (tokenType) {
-                    KtNodeTypes.PRIMARY_CONSTRUCTOR, KtNodeTypes.SECONDARY_CONSTRUCTOR,
-                    KtNodeTypes.FUN, KtNodeTypes.FUNCTION_LITERAL,
-                    KtNodeTypes.PROPERTY,
-                    KtNodeTypes.PROPERTY_ACCESSOR,
-                    KtNodeTypes.CLASS,
-                    KtNodeTypes.OBJECT_DECLARATION,
-                    KtNodeTypes.CLASS_INITIALIZER ->
-                        true
-                    else ->
-                        false
-                }
     }
 
+    private val LighterASTNode.isDeclaration: Boolean
+        get() =
+            when (tokenType) {
+                KtNodeTypes.PRIMARY_CONSTRUCTOR, KtNodeTypes.SECONDARY_CONSTRUCTOR,
+                KtNodeTypes.FUN, KtNodeTypes.FUNCTION_LITERAL,
+                KtNodeTypes.PROPERTY,
+                KtNodeTypes.PROPERTY_ACCESSOR,
+                KtNodeTypes.CLASS,
+                KtNodeTypes.OBJECT_DECLARATION,
+                KtNodeTypes.CLASS_INITIALIZER ->
+                    true
+                else ->
+                    false
+            }
+
     private class ModifierSetBasedLightTreePositioningStrategy(private val modifierSet: TokenSet) : LightTreePositioningStrategy() {
         override fun mark(
             node: LighterASTNode,

compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/SourceElementPositioningStrategies.kt

@@ -23,6 +23,11 @@ object SourceElementPositioningStrategies {
         PositioningStrategies.SECONDARY_CONSTRUCTOR_DELEGATION_CALL
     )
 
+    val DECLARATION_RETURN_TYPE = SourceElementPositioningStrategy(
+        LightTreePositioningStrategies.DECLARATION_RETURN_TYPE,
+        PositioningStrategies.DECLARATION_RETURN_TYPE
+    )
+
     val DECLARATION_NAME = SourceElementPositioningStrategy(
         LightTreePositioningStrategies.DECLARATION_NAME,
         PositioningStrategies.DECLARATION_NAME

compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/dfa/cfg/CFGNodeRenderer.kt

@@ -127,7 +127,8 @@ private object CfgRenderMode : FirRenderer.RenderMode(
     renderLambdaBodies = false,
     renderCallArguments = false,
     renderCallableFqNames = false,
-    renderDeclarationResolvePhase = false
+    renderDeclarationResolvePhase = false,
+    renderAnnotation = false,
 )
 
 private fun FirFunction<*>.name(): String = when (this) {

compiler/fir/tree/src/org/jetbrains/kotlin/fir/FirRenderer.kt

@@ -52,27 +52,39 @@ class FirRenderer(builder: StringBuilder, private val mode: RenderMode = RenderM
         val renderLambdaBodies: Boolean,
         val renderCallArguments: Boolean,
         val renderCallableFqNames: Boolean,
-        val renderDeclarationResolvePhase: Boolean
+        val renderDeclarationResolvePhase: Boolean,
+        val renderAnnotation: Boolean,
     ) {
         object Normal : RenderMode(
             renderLambdaBodies = true,
             renderCallArguments = true,
             renderCallableFqNames = false,
-            renderDeclarationResolvePhase = false
+            renderDeclarationResolvePhase = false,
+            renderAnnotation = true,
         )
 
         object WithFqNames : RenderMode(
             renderLambdaBodies = true,
             renderCallArguments = true,
             renderCallableFqNames = true,
-            renderDeclarationResolvePhase = false
+            renderDeclarationResolvePhase = false,
+            renderAnnotation = true,
+        )
+
+        object WithFqNamesExceptAnnotation : RenderMode(
+            renderLambdaBodies = true,
+            renderCallArguments = true,
+            renderCallableFqNames = true,
+            renderDeclarationResolvePhase = false,
+            renderAnnotation = false,
         )
 
         object WithResolvePhases : RenderMode(
             renderLambdaBodies = true,
             renderCallArguments = true,
             renderCallableFqNames = false,
-            renderDeclarationResolvePhase = true
+            renderDeclarationResolvePhase = true,
+            renderAnnotation = true,
         )
     }
 
@@ -154,6 +166,7 @@ class FirRenderer(builder: StringBuilder, private val mode: RenderMode = RenderM
     }
 
     private fun List<FirAnnotationCall>.renderAnnotations() {
+        if (!mode.renderAnnotation) return
         for (annotation in this) {
             visitAnnotationCall(annotation)
         }

compiler/testData/diagnostics/tests/CovariantOverrideType.fir.kt

@@ -1,26 +0,0 @@
-interface A<H> {
-    fun foo() : Int = 1
-    fun foo2() : Int = 1
-    fun foo1() : Int = 1
-    val a : Int
-    val a1 : Int
-    val g : Iterator<H>
-
-    fun <T> g() : T
-    fun <T> g1() : T
-}
-
-abstract class B<H>() : A<H> {
-    override fun foo() {
-    }
-    override fun foo2() : <!RETURN_TYPE_MISMATCH_ON_OVERRIDE!>Unit<!> {
-    }
-
-    override val a : <!PROPERTY_TYPE_MISMATCH_ON_OVERRIDE!>Double<!> = 1.toDouble()
-    override val a1 = 1.toDouble()
-
-    abstract override fun <X> g() : <!RETURN_TYPE_MISMATCH_ON_OVERRIDE!>Int<!>
-    abstract override fun <X> g1() : <!RETURN_TYPE_MISMATCH_ON_OVERRIDE!>List<X><!>
-
-    abstract override val g : <!PROPERTY_TYPE_MISMATCH_ON_OVERRIDE!>Iterator<Int><!>
-}

compiler/testData/diagnostics/tests/CovariantOverrideType.kt

@@ -1,3 +1,4 @@
+// FIR_IDENTICAL
 interface A<H> {
     fun foo() : Int = 1
     fun foo2() : Int = 1

compiler/testData/diagnostics/tests/delegatedProperty/delegatedPropertyOverridedInTraitTypeMismatch.fir.kt

@@ -1,21 +0,0 @@
-// !DIAGNOSTICS: -UNUSED_PARAMETER
-
-import kotlin.reflect.KProperty
-
-interface A {
-    val prop: Int
-}
-
-class AImpl: A  {
-    override val prop by Delegate()
-}
-
-fun foo() {
-    AImpl().prop
-}
-
-class Delegate {
-    operator fun getValue(t: Any?, p: KProperty<*>): <!PROPERTY_TYPE_MISMATCH_ON_OVERRIDE!>String<!> {
-        return ""
-    }
-}

compiler/testData/diagnostics/tests/delegatedProperty/delegatedPropertyOverridedInTraitTypeMismatch.kt

@@ -1,3 +1,4 @@
+// FIR_IDENTICAL
 // !DIAGNOSTICS: -UNUSED_PARAMETER
 
 import kotlin.reflect.KProperty

compiler/testData/diagnostics/testsWithStdLib/coroutines/suspendFunctionType/lambdaInOverriddenValInitializer.fir.kt

@@ -1,23 +0,0 @@
-interface Foo {
-    val foo: suspend () -> Unit
-}
-
-interface Bar<T> {
-    val bar: T
-}
-
-class Test1 : Foo {
-    override val foo = <!PROPERTY_TYPE_MISMATCH_ON_OVERRIDE!>{}<!>
-}
-
-class Test2 : Foo {
-    override val foo: suspend () -> Unit = {}
-}
-
-class Test3 : Bar<suspend () -> Unit> {
-    override val bar = <!PROPERTY_TYPE_MISMATCH_ON_OVERRIDE!>{}<!>
-}
-
-class Test4 : Bar<suspend () -> Unit> {
-    override val bar: suspend () -> Unit = {}
-}

compiler/testData/diagnostics/testsWithStdLib/coroutines/suspendFunctionType/lambdaInOverriddenValInitializer.kt

@@ -1,3 +1,4 @@
+// FIR_IDENTICAL
 interface Foo {
     val foo: suspend () -> Unit
 }

compiler/testData/diagnostics/testsWithStdLib/regression/ea66827_dataClassWrongToString.fir.kt

@@ -1,7 +0,0 @@
-data class A(val x: Int) {
-    fun toArray(): IntArray =
-            intArrayOf(x)
-
-    override fun toString() =
-            toArray().takeWhile { it != -1 } // .joinToString()
-}

compiler/testData/diagnostics/testsWithStdLib/regression/ea66827_dataClassWrongToString.kt

@@ -1,3 +1,4 @@
+// FIR_IDENTICAL
 data class A(val x: Int) {
     fun toArray(): IntArray =
             intArrayOf(x)

compiler/tests-spec/testData/diagnostics/linked/type-system/introduction-1/p-6/neg/2.2.fir.kt

@@ -70,7 +70,7 @@ open class Case13_1 {
 }
 
 class Case13: Case13_1() {
-    override val x = null
+    override val <!PROPERTY_TYPE_MISMATCH_ON_OVERRIDE!>x<!> = null
 }
 
 // TESTCASE NUMBER: 14
@@ -79,7 +79,7 @@ abstract class Case14_1 {
 }
 
 class Case14: Case14_1() {
-    override val x = null
+    override val <!PROPERTY_TYPE_MISMATCH_ON_OVERRIDE!>x<!> = null
 }
 
 // TESTCASE NUMBER: 15
@@ -88,5 +88,5 @@ interface Case15_1 {
 }
 
 class Case15(): Case15_1 {
-    override fun foo() = null
+    override fun <!RETURN_TYPE_MISMATCH_ON_OVERRIDE!>foo<!>() = null
 }