FirPropertyAccessorChecker: add three new diagnostics

compiler/fir/checkers/checkers-component-generator/src/org/jetbrains/kotlin/fir/checkers/generator/diagnostics/FirDiagnosticsList.kt

@@ -578,7 +578,13 @@ object DIAGNOSTICS_LIST : DiagnosticList() {
             parameter<ConeKotlinType>("actual")
         }
         val GETTER_VISIBILITY_DIFFERS_FROM_PROPERTY_VISIBILITY by error<KtModifierListOwner>(PositioningStrategy.VISIBILITY_MODIFIER)
+        val SETTER_VISIBILITY_INCONSISTENT_WITH_PROPERTY_VISIBILITY by error<KtModifierListOwner>(PositioningStrategy.VISIBILITY_MODIFIER)
         val WRONG_SETTER_RETURN_TYPE by error<KtProperty>()
+        val WRONG_GETTER_RETURN_TYPE by error<KtProperty> {
+            parameter<ConeKotlinType>("expectedType")
+            parameter<ConeKotlinType>("actualType")
+        }
+        val ACCESSOR_FOR_DELEGATED_PROPERTY by error<KtPropertyAccessor>()
     }
 
     val MPP_PROJECTS by object : DiagnosticGroup("Multi-platform projects") {

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

@@ -360,7 +360,10 @@ object FirErrors {
     val WRONG_SETTER_PARAMETER_TYPE by error2<KtTypeReference, ConeKotlinType, ConeKotlinType>()
     val INITIALIZER_TYPE_MISMATCH by error2<KtProperty, ConeKotlinType, ConeKotlinType>(SourceElementPositioningStrategies.ASSIGNMENT_VALUE)
     val GETTER_VISIBILITY_DIFFERS_FROM_PROPERTY_VISIBILITY by error0<KtModifierListOwner>(SourceElementPositioningStrategies.VISIBILITY_MODIFIER)
+    val SETTER_VISIBILITY_INCONSISTENT_WITH_PROPERTY_VISIBILITY by error0<KtModifierListOwner>(SourceElementPositioningStrategies.VISIBILITY_MODIFIER)
     val WRONG_SETTER_RETURN_TYPE by error0<KtProperty>()
+    val WRONG_GETTER_RETURN_TYPE by error2<KtProperty, ConeKotlinType, ConeKotlinType>()
+    val ACCESSOR_FOR_DELEGATED_PROPERTY by error0<KtPropertyAccessor>()
 
     // Multi-platform projects
     val EXPECTED_DECLARATION_WITH_BODY by error0<KtDeclaration>(SourceElementPositioningStrategies.DECLARATION_SIGNATURE)

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

@@ -5,12 +5,14 @@
 
 package org.jetbrains.kotlin.fir.analysis.checkers.declaration
 
+import org.jetbrains.kotlin.fir.FirFakeSourceElementKind
 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
 import org.jetbrains.kotlin.fir.analysis.diagnostics.reportOn
 import org.jetbrains.kotlin.fir.analysis.diagnostics.withSuppressedDiagnostics
 import org.jetbrains.kotlin.fir.declarations.FirProperty
+import org.jetbrains.kotlin.fir.declarations.FirPropertyAccessor
 import org.jetbrains.kotlin.fir.declarations.visibility
 import org.jetbrains.kotlin.fir.types.ConeClassErrorType
 import org.jetbrains.kotlin.fir.types.coneType
@@ -24,20 +26,43 @@ object FirPropertyAccessorChecker : FirPropertyChecker() {
 
     private fun checkGetter(property: FirProperty, context: CheckerContext, reporter: DiagnosticReporter) {
         val getter = property.getter ?: return
+        val propertyType = property.returnTypeRef.coneType
+
         withSuppressedDiagnostics(getter, context) {
+            checkAccessorForDelegatedProperty(property, getter, context, reporter)
             if (getter.visibility != property.visibility) {
                 reporter.reportOn(getter.source, FirErrors.GETTER_VISIBILITY_DIFFERS_FROM_PROPERTY_VISIBILITY, context)
             }
+            val getterReturnTypeRef = getter.returnTypeRef
+            if (getterReturnTypeRef.source?.kind is FirFakeSourceElementKind) {
+                return
+            }
+            val getterReturnType = getterReturnTypeRef.coneType
+            if (propertyType is ConeClassErrorType || getterReturnType is ConeClassErrorType) {
+                return
+            }
+            if (getterReturnType != property.returnTypeRef.coneType) {
+                val getterReturnTypeSource = getterReturnTypeRef.source
+                withSuppressedDiagnostics(getterReturnTypeRef, context) {
+                    reporter.reportOn(getterReturnTypeSource, FirErrors.WRONG_GETTER_RETURN_TYPE, propertyType, getterReturnType, context)
+                }
+            }
         }
     }
 
     private fun checkSetter(property: FirProperty, context: CheckerContext, reporter: DiagnosticReporter) {
         val setter = property.setter ?: return
+        val propertyType = property.returnTypeRef.coneType
 
         withSuppressedDiagnostics(setter, context) {
             if (property.isVal) {
                 reporter.reportOn(setter.source, FirErrors.VAL_WITH_SETTER, context)
             }
+            checkAccessorForDelegatedProperty(property, setter, context, reporter)
+            val visibilityCompareResult = setter.visibility.compareTo(property.visibility)
+            if (visibilityCompareResult == null || visibilityCompareResult > 0) {
+                reporter.reportOn(setter.source, FirErrors.SETTER_VISIBILITY_INCONSISTENT_WITH_PROPERTY_VISIBILITY, context)
+            }
 
             val valueSetterParameter = setter.valueParameters.first()
             if (valueSetterParameter.isVararg) {
@@ -45,7 +70,6 @@ object FirPropertyAccessorChecker : FirPropertyChecker() {
             }
             val valueSetterType = valueSetterParameter.returnTypeRef.coneType
             val valueSetterTypeSource = valueSetterParameter.returnTypeRef.source
-            val propertyType = property.returnTypeRef.coneType
             if (propertyType is ConeClassErrorType || valueSetterType is ConeClassErrorType) {
                 return
             }
@@ -62,8 +86,23 @@ object FirPropertyAccessorChecker : FirPropertyChecker() {
             }
 
             if (!setterReturnType.isUnit) {
-                reporter.reportOn(setter.returnTypeRef.source, FirErrors.WRONG_SETTER_RETURN_TYPE, context)
+                withSuppressedDiagnostics(setter.returnTypeRef, context) {
+                    reporter.reportOn(setter.returnTypeRef.source, FirErrors.WRONG_SETTER_RETURN_TYPE, context)
+                }
             }
         }
     }
+
+    private fun checkAccessorForDelegatedProperty(
+        property: FirProperty,
+        accessor: FirPropertyAccessor,
+        context: CheckerContext,
+        reporter: DiagnosticReporter
+    ) {
+        if (property.delegateFieldSymbol != null && accessor.body != null &&
+            accessor.source?.kind != FirFakeSourceElementKind.DelegatedPropertyAccessor
+        ) {
+            reporter.reportOn(accessor.source, FirErrors.ACCESSOR_FOR_DELEGATED_PROPERTY, context)
+        }
+    }
 }

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

@@ -33,6 +33,7 @@ import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.ABSTRACT_PROPERTY
 import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.ABSTRACT_PROPERTY_WITH_INITIALIZER
 import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.ABSTRACT_PROPERTY_WITH_SETTER
 import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.ABSTRACT_SUPER_CALL
+import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.ACCESSOR_FOR_DELEGATED_PROPERTY
 import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.ANNOTATION_ARGUMENT_MUST_BE_CONST
 import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.ANNOTATION_CLASS_MEMBER
 import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.ANNOTATION_PARAMETER_DEFAULT_VALUE_MUST_BE_CONSTANT
@@ -252,6 +253,7 @@ import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.SEALED_CLASS_CONS
 import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.SEALED_SUPERTYPE
 import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.SEALED_SUPERTYPE_IN_LOCAL_CLASS
 import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.SECONDARY_CONSTRUCTOR_WITH_BODY_INSIDE_INLINE_CLASS
+import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.SETTER_VISIBILITY_INCONSISTENT_WITH_PROPERTY_VISIBILITY
 import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.SUPERCLASS_NOT_ACCESSIBLE_FROM_INTERFACE
 import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.SUPERTYPES_FOR_ANNOTATION_CLASS
 import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.SUPERTYPE_INITIALIZED_IN_INTERFACE
@@ -314,6 +316,7 @@ import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.VARIANCE_ON_TYPE_
 import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.VAR_ANNOTATION_PARAMETER
 import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.VAR_OVERRIDDEN_BY_VAL
 import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.VAR_TYPE_MISMATCH_ON_OVERRIDE
+import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.WRONG_GETTER_RETURN_TYPE
 import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.WRONG_INVOCATION_KIND
 import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.WRONG_MODIFIER_TARGET
 import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.WRONG_NUMBER_OF_TYPE_ARGUMENTS
@@ -819,7 +822,15 @@ class FirDefaultErrorMessages : DefaultErrorMessages.Extension {
             )
             map.put(INITIALIZER_TYPE_MISMATCH, "Initializer type mismatch: expected {0}, actual {1}", RENDER_TYPE, RENDER_TYPE)
             map.put(GETTER_VISIBILITY_DIFFERS_FROM_PROPERTY_VISIBILITY, "Getter visibility must be the same as property visibility")
+            map.put(SETTER_VISIBILITY_INCONSISTENT_WITH_PROPERTY_VISIBILITY, "Setter visibility must be the same or less permissive than property visibility")
             map.put(WRONG_SETTER_RETURN_TYPE, "Setter return type must be Unit")
+            map.put(
+                WRONG_GETTER_RETURN_TYPE,
+                "Getter return type must be equal to the type of the property, i.e. ''{0}''",
+                RENDER_TYPE,
+                RENDER_TYPE
+            )
+            map.put(ACCESSOR_FOR_DELEGATED_PROPERTY, "Delegated property cannot have accessors with non-default implementations")
 
             map.put(CONST_VAL_NOT_TOP_LEVEL_OR_OBJECT, "Const 'val' are only allowed on top level or in objects")
             map.put(CONST_VAL_WITH_GETTER, "Const 'val' should not have a getter")

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

@@ -17,7 +17,7 @@ class Test: ATest(), ITest {
 
     override var prop2 : Int
         get() = 14
-        internal set(value) {}
+        <!SETTER_VISIBILITY_INCONSISTENT_WITH_PROPERTY_VISIBILITY!>internal<!> set(value) {}
 }
 
 fun main() {

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

@@ -1,25 +0,0 @@
-class My {
-    var x: Int = 0
-        // Ok
-        private set
-    
-    private var y: Int = 1
-        // Error: better
-        public set
-
-    protected var z: Int = 2
-        // Ok
-        private set
-
-    protected var w: Int = 3
-        // Error: incompatible
-        internal set
-
-    internal var v: Int = 4
-        // Error: incompatible
-        protected set
-
-    internal var t: Int = 5
-        // Error: better
-        public set
-}

compiler/testData/diagnostics/tests/SetterVisibility.kt

@@ -1,3 +1,4 @@
+// FIR_IDENTICAL
 class My {
     var x: Int = 0
         // Ok

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

@@ -1,12 +0,0 @@
-// KT-11809 Assertion error when delegated property has getter
-
-class A {
-    val p1 by this
-        get
-
-    var p2 by this
-        get() = ""
-
-    operator fun getValue(a: Any?, p: Any?) = ""
-    operator fun setValue(a: Any?, p: Any?, v: Any?) {}
-}

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

@@ -1,3 +1,4 @@
+// FIR_IDENTICAL
 // KT-11809 Assertion error when delegated property has getter
 
 class A {

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

@@ -1,12 +0,0 @@
-// !DIAGNOSTICS: -UNUSED_PARAMETER
-
-import kotlin.reflect.KProperty
-
-val a: Int by Delegate()
-  get() = 1
-
-class Delegate {
-  operator fun getValue(t: Any?, p: KProperty<*>): Int {
-    return 1
-  }
-}

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

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

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

@@ -1,15 +0,0 @@
-// !DIAGNOSTICS: -UNUSED_PARAMETER
-
-import kotlin.reflect.KProperty
-
-var a: Int by Delegate()
-  get() = 1
-  set(i) {}
-
-class Delegate {
-  operator fun getValue(t: Any?, p: KProperty<*>): Int {
-    return 1
-  }
-
-  operator fun setValue(t: Any?, p: KProperty<*>, i: Int) {}
-}

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

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

compiler/testData/diagnostics/tests/lateinit/setter.fir.kt

@@ -1,22 +0,0 @@
-class My {
-
-    lateinit var x: String
-        private set
-
-    lateinit var y: String
-        internal set
-
-    lateinit protected var z: String
-        private set
-
-    lateinit private var w: String
-        // Ok, private var / private set
-        private set
-
-    lateinit protected var v: String
-        public set
-
-    lateinit public var u: String
-        // Ok, public var / public set
-        public set
-}

compiler/testData/diagnostics/tests/lateinit/setter.kt

@@ -1,3 +1,4 @@
+// FIR_IDENTICAL
 class My {
 
     lateinit var x: String

compiler/testData/diagnostics/tests/regressions/kt251.fir.kt

@@ -6,7 +6,7 @@ class A() {
             field = value
         }
     val y: Int
-        get(): String = "s"
+        get(): <!WRONG_GETTER_RETURN_TYPE("Getter return type must be equal to the type of the property, i.e. 'kotlin/Int'")!>String<!> = "s"
     val z: Int
         get() {
             return "s"
@@ -17,7 +17,7 @@ class A() {
             field = v
         }
     val b: Int
-        get(): Any = "s"
+        get(): <!WRONG_GETTER_RETURN_TYPE!>Any<!> = "s"
     val c: Int
         get() {
             return 1
@@ -27,7 +27,7 @@ class A() {
             return field
         }
     val e = 1
-        get(): String {
+        get(): <!WRONG_GETTER_RETURN_TYPE!>String<!> {
             return field
         }
 

compiler/testData/diagnostics/testsWithStdLib/PropertyDelegateWithPrivateSet.fir.kt

@@ -8,5 +8,5 @@ class My {
     val another: String = delegate
 
     var delegateWithBackingField: String by kotlin.properties.Delegates.notNull()
-        private set(arg) { <!UNRESOLVED_REFERENCE!>field<!> = arg }
+        <!ACCESSOR_FOR_DELEGATED_PROPERTY!>private set(arg) { <!UNRESOLVED_REFERENCE!>field<!> = arg }<!>
 }

compiler/testData/diagnostics/testsWithStdLib/annotations/jvmStatic/interfaceCompanion_LL12.fir.kt

@@ -45,7 +45,7 @@ interface B {
             @JvmStatic get
 
         private var foo8 = 1
-        @JvmStatic public set
+        @JvmStatic <!SETTER_VISIBILITY_INCONSISTENT_WITH_PROPERTY_VISIBILITY!>public<!> set
 
         public var foo9 = 1
         @JvmStatic private set

compiler/testData/diagnostics/testsWithStdLib/annotations/jvmStatic/interfaceCompanion_LL13_16.fir.kt

@@ -47,7 +47,7 @@ interface B {
         @JvmStatic get
 
         private var foo8 = 1
-        @JvmStatic public set
+        @JvmStatic <!SETTER_VISIBILITY_INCONSISTENT_WITH_PROPERTY_VISIBILITY!>public<!> set
 
         public var foo9 = 1
         @JvmStatic private set

compiler/testData/diagnostics/testsWithStdLib/annotations/jvmStatic/interfaceCompanion_LL13_18.fir.kt

@@ -46,7 +46,7 @@ interface B {
         @JvmStatic get
 
         private var foo8 = 1
-        @JvmStatic public set
+        @JvmStatic <!SETTER_VISIBILITY_INCONSISTENT_WITH_PROPERTY_VISIBILITY!>public<!> set
 
         public var foo9 = 1
         @JvmStatic private set

idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDataClassConverters.kt

@@ -1696,12 +1696,32 @@ internal val KT_DIAGNOSTIC_CONVERTER = KtDiagnosticConverterBuilder.buildConvert
             token,
         )
     }
+    add(FirErrors.SETTER_VISIBILITY_INCONSISTENT_WITH_PROPERTY_VISIBILITY) { firDiagnostic ->
+        SetterVisibilityInconsistentWithPropertyVisibilityImpl(
+            firDiagnostic as FirPsiDiagnostic<*>,
+            token,
+        )
+    }
     add(FirErrors.WRONG_SETTER_RETURN_TYPE) { firDiagnostic ->
         WrongSetterReturnTypeImpl(
             firDiagnostic as FirPsiDiagnostic<*>,
             token,
         )
     }
+    add(FirErrors.WRONG_GETTER_RETURN_TYPE) { firDiagnostic ->
+        WrongGetterReturnTypeImpl(
+            firSymbolBuilder.typeBuilder.buildKtType(firDiagnostic.a),
+            firSymbolBuilder.typeBuilder.buildKtType(firDiagnostic.b),
+            firDiagnostic as FirPsiDiagnostic<*>,
+            token,
+        )
+    }
+    add(FirErrors.ACCESSOR_FOR_DELEGATED_PROPERTY) { firDiagnostic ->
+        AccessorForDelegatedPropertyImpl(
+            firDiagnostic as FirPsiDiagnostic<*>,
+            token,
+        )
+    }
     add(FirErrors.EXPECTED_DECLARATION_WITH_BODY) { firDiagnostic ->
         ExpectedDeclarationWithBodyImpl(
             firDiagnostic as FirPsiDiagnostic<*>,

idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnostics.kt

@@ -1191,10 +1191,24 @@ sealed class KtFirDiagnostic<PSI: PsiElement> : KtDiagnosticWithPsi<PSI> {
         override val diagnosticClass get() = GetterVisibilityDiffersFromPropertyVisibility::class
     }
 
+    abstract class SetterVisibilityInconsistentWithPropertyVisibility : KtFirDiagnostic<KtModifierListOwner>() {
+        override val diagnosticClass get() = SetterVisibilityInconsistentWithPropertyVisibility::class
+    }
+
     abstract class WrongSetterReturnType : KtFirDiagnostic<KtProperty>() {
         override val diagnosticClass get() = WrongSetterReturnType::class
     }
 
+    abstract class WrongGetterReturnType : KtFirDiagnostic<KtProperty>() {
+        override val diagnosticClass get() = WrongGetterReturnType::class
+        abstract val expectedType: KtType
+        abstract val actualType: KtType
+    }
+
+    abstract class AccessorForDelegatedProperty : KtFirDiagnostic<KtPropertyAccessor>() {
+        override val diagnosticClass get() = AccessorForDelegatedProperty::class
+    }
+
     abstract class ExpectedDeclarationWithBody : KtFirDiagnostic<KtDeclaration>() {
         override val diagnosticClass get() = ExpectedDeclarationWithBody::class
     }

idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnosticsImpl.kt

@@ -1936,6 +1936,13 @@ internal class GetterVisibilityDiffersFromPropertyVisibilityImpl(
     override val firDiagnostic: FirPsiDiagnostic<*> by weakRef(firDiagnostic)
 }
 
+internal class SetterVisibilityInconsistentWithPropertyVisibilityImpl(
+    firDiagnostic: FirPsiDiagnostic<*>,
+    override val token: ValidityToken,
+) : KtFirDiagnostic.SetterVisibilityInconsistentWithPropertyVisibility(), KtAbstractFirDiagnostic<KtModifierListOwner> {
+    override val firDiagnostic: FirPsiDiagnostic<*> by weakRef(firDiagnostic)
+}
+
 internal class WrongSetterReturnTypeImpl(
     firDiagnostic: FirPsiDiagnostic<*>,
     override val token: ValidityToken,
@@ -1943,6 +1950,22 @@ internal class WrongSetterReturnTypeImpl(
     override val firDiagnostic: FirPsiDiagnostic<*> by weakRef(firDiagnostic)
 }
 
+internal class WrongGetterReturnTypeImpl(
+    override val expectedType: KtType,
+    override val actualType: KtType,
+    firDiagnostic: FirPsiDiagnostic<*>,
+    override val token: ValidityToken,
+) : KtFirDiagnostic.WrongGetterReturnType(), KtAbstractFirDiagnostic<KtProperty> {
+    override val firDiagnostic: FirPsiDiagnostic<*> by weakRef(firDiagnostic)
+}
+
+internal class AccessorForDelegatedPropertyImpl(
+    firDiagnostic: FirPsiDiagnostic<*>,
+    override val token: ValidityToken,
+) : KtFirDiagnostic.AccessorForDelegatedProperty(), KtAbstractFirDiagnostic<KtPropertyAccessor> {
+    override val firDiagnostic: FirPsiDiagnostic<*> by weakRef(firDiagnostic)
+}
+
 internal class ExpectedDeclarationWithBodyImpl(
     firDiagnostic: FirPsiDiagnostic<*>,
     override val token: ValidityToken,