FIR checker: rework VAL_REASSIGNMENT_VIA_BACKING_FIELD(_ERROR)

compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/context/CheckerContext.kt

@@ -37,7 +37,7 @@ abstract class CheckerContext {
 
     /**
      * Returns the closest to the end of context.containingDeclarations
-     * T instance or null if no such item could be found.
+     * instance of type [T] or null if no such item could be found.
      */
     inline fun <reified T : FirDeclaration> findClosest(): T? {
         for (it in containingDeclarations.asReversed()) {
@@ -47,6 +47,18 @@ abstract class CheckerContext {
         return null
     }
 
+    /**
+     * Same as the one without specific [check]. For some cases, an instance of type [T] isn't good enough. E.g., property accessor is
+     * either getter or setter, but a type-based search could return, say, the closest setter, while we want to keep searching for a getter.
+     */
+    inline fun <reified T : FirDeclaration> findClosest(check: (T) -> Boolean): T? {
+        for (it in containingDeclarations.asReversed()) {
+            return (it as? T)?.takeIf(check) ?: continue
+        }
+
+        return null
+    }
+
     abstract fun addSuppressedDiagnostics(
         diagnosticNames: Collection<String>,
         allInfosSuppressed: Boolean,

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

@@ -5,11 +5,9 @@
 
 package org.jetbrains.kotlin.fir.analysis.checkers.declaration
 
-import org.jetbrains.kotlin.config.LanguageFeature
 import org.jetbrains.kotlin.descriptors.ClassKind
 import org.jetbrains.kotlin.descriptors.Modality
 import org.jetbrains.kotlin.descriptors.Visibilities
-import org.jetbrains.kotlin.fir.FirElement
 import org.jetbrains.kotlin.fir.FirSourceElement
 import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext
 import org.jetbrains.kotlin.fir.analysis.checkers.modality
@@ -18,11 +16,7 @@ import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors
 import org.jetbrains.kotlin.fir.analysis.diagnostics.reportOn
 import org.jetbrains.kotlin.fir.declarations.*
 import org.jetbrains.kotlin.fir.declarations.impl.FirDefaultPropertyAccessor
-import org.jetbrains.kotlin.fir.expressions.FirVariableAssignment
-import org.jetbrains.kotlin.fir.languageVersionSettings
-import org.jetbrains.kotlin.fir.references.FirBackingFieldReference
 import org.jetbrains.kotlin.fir.types.FirImplicitTypeRef
-import org.jetbrains.kotlin.fir.visitors.FirVisitorVoid
 import org.jetbrains.kotlin.lexer.KtTokens
 
 internal fun isInsideExpectClass(containingClass: FirRegularClass, context: CheckerContext): Boolean {
@@ -175,34 +169,6 @@ internal fun checkPropertyAccessors(
     context: CheckerContext
 ) {
     if (property.isVal) {
-        if (property.getter != null) {
-            var reassignment: FirBackingFieldReference? = null
-            // TODO: consider replacing this with normal VariableAssignmentChecker and reporting all 'field = ...' in getter (not just 1st)
-            // This way is a bit hacky and does not handle diagnostic suppression normally
-            val visitor = object : FirVisitorVoid() {
-                override fun visitElement(element: FirElement) {
-                    element.acceptChildren(this)
-                }
-
-                override fun visitVariableAssignment(variableAssignment: FirVariableAssignment) {
-                    // Report the first violation (to match FE 1.0 behavior)
-                    if (reassignment != null) return
-
-                    val backingFieldReference = variableAssignment.lValue as? FirBackingFieldReference
-                    if (backingFieldReference?.resolvedSymbol?.fir == property) {
-                        reassignment = backingFieldReference
-                    }
-                }
-            }
-            property.getter?.body?.accept(visitor, null)
-            reassignment?.source?.let {
-                if (context.session.languageVersionSettings.supportsFeature(LanguageFeature.RestrictionOfValReassignmentViaBackingField)) {
-                    reporter.reportOn(it, FirErrors.VAL_REASSIGNMENT_VIA_BACKING_FIELD_ERROR, property.symbol, context)
-                } else {
-                    reporter.reportOn(it, FirErrors.VAL_REASSIGNMENT_VIA_BACKING_FIELD, property.symbol, context)
-                }
-            }
-        }
         property.setter?.source?.let {
             reporter.reportOn(it, FirErrors.VAL_WITH_SETTER, context)
         }

compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/expression/FirValReassignmentViaBackingFieldChecker.kt

+/*
+ * Copyright 2010-2021 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.checkers.expression
+
+import org.jetbrains.kotlin.config.LanguageFeature
+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.declarations.FirPropertyAccessor
+import org.jetbrains.kotlin.fir.expressions.FirVariableAssignment
+import org.jetbrains.kotlin.fir.languageVersionSettings
+import org.jetbrains.kotlin.fir.references.FirBackingFieldReference
+
+object FirValReassignmentViaBackingFieldChecker : FirVariableAssignmentChecker() {
+    override fun check(expression: FirVariableAssignment, context: CheckerContext, reporter: DiagnosticReporter) {
+        val backingFieldReference = expression.lValue as? FirBackingFieldReference ?: return
+        val property = backingFieldReference.resolvedSymbol.fir
+        if (property.isVar) return
+        val closestGetter = context.findClosest<FirPropertyAccessor> { it.isGetter } ?: return
+        if (property.getter != closestGetter) return
+
+        backingFieldReference.source?.let {
+            if (context.session.languageVersionSettings.supportsFeature(LanguageFeature.RestrictionOfValReassignmentViaBackingField)) {
+                reporter.reportOn(it, FirErrors.VAL_REASSIGNMENT_VIA_BACKING_FIELD_ERROR, property.symbol, context)
+            } else {
+                reporter.reportOn(it, FirErrors.VAL_REASSIGNMENT_VIA_BACKING_FIELD, property.symbol, context)
+            }
+        }
+    }
+}

compiler/fir/entrypoint/src/org/jetbrains/kotlin/fir/checkers/CommonExpressionCheckers.kt

@@ -25,12 +25,17 @@ object CommonExpressionCheckers : ExpressionCheckers() {
         FirTypeParameterInQualifiedAccessChecker,
         FirSealedClassConstructorCallChecker,
     )
+
     override val functionCallCheckers: Set<FirFunctionCallChecker> = setOf()
 
     override val tryExpressionCheckers: Set<FirTryExpressionChecker> = setOf(
         FirCatchParameterChecker
     )
 
+    override val variableAssignmentCheckers: Set<FirVariableAssignmentChecker> = setOf(
+        FirValReassignmentViaBackingFieldChecker
+    )
+
     override val whenExpressionCheckers: Set<FirWhenExpressionChecker> = setOf(
         FirExhaustiveWhenChecker
     )

compiler/testData/diagnostics/tests/backingField/LocalDeclarations.fir.kt

+// !LANGUAGE: +RestrictionOfValReassignmentViaBackingField
+
+class Outer {
+    val i: Int = 1
+        get() {
+            class Inner {
+                var i: Int = 2
+                    get() {
+                        field++
+                        return field
+                    }
+                val j: Int = 3
+                    get() {
+                        <!VAL_REASSIGNMENT_VIA_BACKING_FIELD_ERROR!>field<!> = 42
+                        return field
+                    }
+
+                fun innerMember() {
+                    <!VAL_REASSIGNMENT_VIA_BACKING_FIELD_ERROR!>field<!>++
+                }
+            }
+            return field
+        }
+
+    val j: Int = 4
+        get() {
+            fun local() {
+                <!VAL_REASSIGNMENT_VIA_BACKING_FIELD_ERROR!>field<!>++
+                <!VAL_REASSIGNMENT_VIA_BACKING_FIELD_ERROR!>field<!>++
+            }
+            local()
+            return field
+        }
+}

compiler/testData/diagnostics/tests/backingField/LocalDeclarations.kt

-// FIR_IDENTICAL
 // !LANGUAGE: +RestrictionOfValReassignmentViaBackingField
 
 class Outer {