[FIR] Refactor CanBeValChecker

compiler/fir/analysis-tests/testData/extendedCheckers/CanBeValChecker.kt

@@ -0,0 +1,185 @@
+// WITH_RUNTIME
+
+import kotlin.reflect.KProperty
+import kotlin.properties.Delegates
+
+fun testDelegator() {
+    var <!UNUSED_VARIABLE!>x<!>: Boolean by LocalFreezableVar(true)
+    var <!UNUSED_VARIABLE!>y<!> by LocalFreezableVar("")
+}
+
+class LocalFreezableVar<T>(private var value: T)  {
+    operator fun getValue(thisRef: Nothing?, property: KProperty<*>): T  = value
+
+    operator fun setValue(thisRef: Nothing?, property: KProperty<*>, value: T) {
+        this.value = value
+    }
+}
+
+
+operator fun C.plus(a: Any): C = this
+operator fun C.plusAssign(a: Any) {}
+
+fun testOperatorAssignment() {
+    val c = C()
+    c += ""
+    <!CAN_BE_VAL!>var<!> c1 = C()
+    <!ASSIGN_OPERATOR_AMBIGUITY!>c1 += ""<!>
+
+    var a = 1
+    a += 12
+    <!ASSIGNED_VALUE_IS_NEVER_READ!>a<!> -= 10
+}
+
+
+fun destructuringDeclaration() {
+    <!CAN_BE_VAL!>var<!> (v1, <!UNUSED_VARIABLE!>v2<!>) = getPair()
+    print(v1)
+
+    var (v3, <!VARIABLE_NEVER_READ!>v4<!>) = getPair()
+    print(v3)
+    <!ASSIGNED_VALUE_IS_NEVER_READ!>v4<!> = ""
+
+    var (<!VARIABLE_NEVER_READ!>v5<!>, <!UNUSED_VARIABLE!>v6<!>) = getPair()
+    <!ASSIGNED_VALUE_IS_NEVER_READ!>v5<!> = 1
+
+    var (<!VARIABLE_NEVER_READ!>v7<!>, <!VARIABLE_NEVER_READ!>v8<!>) = getPair()
+    <!ASSIGNED_VALUE_IS_NEVER_READ!>v7<!> = 2
+    <!ASSIGNED_VALUE_IS_NEVER_READ!>v8<!> = "42"
+
+    val (<!UNUSED_VARIABLE!>a<!>, <!UNUSED_VARIABLE!>b<!>, <!UNUSED_VARIABLE!>c<!>) = Triple(1, 1, 1)
+
+    <!CAN_BE_VAL!>var<!> (<!UNUSED_VARIABLE!>x<!>, <!UNUSED_VARIABLE!>y<!>, <!UNUSED_VARIABLE!>z<!>) = Triple(1, 1, 1)
+}
+
+fun stackOverflowBug() {
+    <!CAN_BE_VAL!>var<!> <!VARIABLE_NEVER_READ!>a<!>: Int
+    <!ASSIGNED_VALUE_IS_NEVER_READ!>a<!> = 1
+    for (i in 1..10)
+        print(i)
+}
+
+
+fun smth(flag: Boolean) {
+    var a = 1
+
+    if (flag) {
+        while (a > 0) {
+            a--
+        }
+    }
+}
+
+fun withAnnotation(p: List<Any>) {
+    @Suppress("UNCHECKED_CAST")
+    <!CAN_BE_VAL!>var<!> v = p as List<String>
+    print(v)
+}
+
+fun withReadonlyDeligate() {
+    val s: String by lazy { "Hello!" }
+    s.hashCode()
+}
+
+fun getPair(): Pair<Int, String> = Pair(1, "1")
+
+fun listReceiver(p: List<String>) {}
+
+fun withInitializer() {
+    var <!VARIABLE_NEVER_READ!>v1<!> = 1
+    var v2 = 2
+    <!CAN_BE_VAL!>var<!> v3 = 3
+    <!ASSIGNED_VALUE_IS_NEVER_READ!>v1<!> = 1
+    <!ASSIGNED_VALUE_IS_NEVER_READ!>v2<!>++ // todo mark this UNUSED_CHANGED_VALUES
+    print(v3)
+}
+
+fun test() {
+    var a = 0
+    while (a>0) {
+        a++
+    }
+}
+
+fun foo() {
+    <!CAN_BE_VAL!>var<!> <!VARIABLE_NEVER_READ!>a<!>: Int
+    val bool = true
+    if (bool) <!ASSIGNED_VALUE_IS_NEVER_READ!>a<!> = 4 else <!ASSIGNED_VALUE_IS_NEVER_READ!>a<!> = 42
+    val <!UNUSED_VARIABLE!>b<!>: String
+
+    <!ASSIGNED_VALUE_IS_NEVER_READ!>bool<!> = false
+}
+
+fun cycles() {
+    var a = 10
+    while (a > 0) {
+        a--
+    }
+
+    var <!VARIABLE_NEVER_READ!>b<!>: Int
+    while (a < 10) {
+        a++
+        <!ASSIGNED_VALUE_IS_NEVER_READ!>b<!> = a
+    }
+}
+
+fun assignedTwice(p: Int) {
+    var <!VARIABLE_NEVER_READ!>v<!>: Int
+    <!ASSIGNED_VALUE_IS_NEVER_READ!>v<!> = 0
+    if (p > 0) <!ASSIGNED_VALUE_IS_NEVER_READ!>v<!> = 1
+}
+
+fun main(args: Array<String?>) {
+    <!CAN_BE_VAL!>var<!> a: String?
+    val <!UNUSED_VARIABLE!>unused<!> = 0
+
+    if (args.size == 1) {
+        <!ASSIGNED_VALUE_IS_NEVER_READ!>a<!> = args[0]
+    } else {
+        a  = args.toString()
+        if (a != null && a.equals("cde")) return
+    }
+}
+
+fun run(f: () -> Unit) = f()
+
+fun lambda() {
+    var <!VARIABLE_NEVER_READ!>a<!>: Int
+    <!ASSIGNED_VALUE_IS_NEVER_READ!>a<!> = 10
+
+    run {
+        <!ASSIGNED_VALUE_IS_NEVER_READ!>a<!> = 20
+    }
+}
+
+fun lambdaInitialization() {
+    <!CAN_BE_VAL!>var<!> <!VARIABLE_NEVER_READ!>a<!>: Int
+
+    run {
+        <!ASSIGNED_VALUE_IS_NEVER_READ!>a<!> = 20
+    }
+}
+
+fun notAssignedWhenNotUsed(p: Int) {
+    <!CAN_BE_VAL!>var<!> v: Int
+    if (p > 0) {
+        v = 1
+        print(v)
+    }
+}
+
+var global = 1
+
+class C {
+    var field = 2
+
+    fun foo() {
+        print(field)
+        print(global)
+    }
+}
+
+fun withDelegate() {
+    var <!VARIABLE_NEVER_READ!>s<!>: String by Delegates.notNull()
+    <!ASSIGNED_VALUE_IS_NEVER_READ!>s<!> = ""
+}

compiler/fir/analysis-tests/testData/extendedCheckers/CanBeValChecker.txt

@@ -1,4 +1,4 @@
-FILE: VariableAssignmentChecker.kt
+FILE: CanBeValChecker.kt
     public final fun testDelegator(): R|kotlin/Unit| {
         lvar x: R|kotlin/Boolean|by R|/LocalFreezableVar.LocalFreezableVar|<R|kotlin/Boolean|>(Boolean(true))
         lvar y: R|kotlin/String|by R|/LocalFreezableVar.LocalFreezableVar|<R|kotlin/String|>(String())
@@ -20,12 +20,6 @@ FILE: VariableAssignmentChecker.kt
             this@R|/LocalFreezableVar|.R|/LocalFreezableVar.value| = R|<local>/value|
         }
 
-    }
-    public final class C : R|kotlin/Any| {
-        public constructor(): R|C| {
-            super<R|kotlin/Any|>()
-        }
-
     }
     public final operator fun R|C|.plus(a: R|kotlin/Any|): R|C| {
         ^plus this@R|/plus|
@@ -131,7 +125,6 @@ FILE: VariableAssignmentChecker.kt
     public final fun foo(): R|kotlin/Unit| {
         lvar a: R|kotlin/Int|
         lval bool: R|kotlin/Boolean| = Boolean(true)
-        lval b: R|kotlin/String|
         when () {
             R|<local>/bool| ->  {
                 R|<local>/a| = Int(4)
@@ -141,6 +134,7 @@ FILE: VariableAssignmentChecker.kt
             }
         }
 
+        lval b: R|kotlin/String|
         R|<local>/bool| = Boolean(false)
     }
     public final fun cycles(): R|kotlin/Unit| {
@@ -151,7 +145,7 @@ FILE: VariableAssignmentChecker.kt
             R|<local>/<unary>|
         }
 
-        lval b: R|kotlin/Int|
+        lvar b: R|kotlin/Int|
         while(CMP(<, R|<local>/a|.R|kotlin/Int.compareTo|(Int(10)))) {
             lval <unary>: R|kotlin/Int| = R|<local>/a|
             R|<local>/a| = R|<local>/<unary>|.R|kotlin/Int.inc|()
@@ -172,18 +166,19 @@ FILE: VariableAssignmentChecker.kt
     }
     public final fun main(args: R|kotlin/Array<kotlin/String?>|): R|kotlin/Unit| {
         lvar a: R|kotlin/String?|
+        lval unused: R|kotlin/Int| = Int(0)
         when () {
             ==(R|<local>/args|.R|kotlin/Array.size|, Int(1)) ->  {
                 R|<local>/a| = R|<local>/args|.R|FakeOverride<kotlin/Array.get: R|kotlin/String?|>|(Int(0))
             }
             else ->  {
                 R|<local>/a| = R|<local>/args|.R|kotlin/Any.toString|()
-            }
+                when () {
-        }
+                    !=(R|<local>/a|, Null(null)) && R|<local>/a|.R|kotlin/Any.equals|(String(cde)) ->  {
+                        ^main Unit
+                    }
+                }
 
-        when () {
-            !=(R|<local>/a|, Null(null)) && R|<local>/a|.R|kotlin/Any.equals|(String(cde)) ->  {
-                ^main Unit
             }
         }
 

compiler/fir/analysis-tests/testData/extendedCheckers/VariableAssignmentChecker.kt

@@ -1,187 +0,0 @@
-// WITH_RUNTIME
-
-import kotlin.reflect.KProperty
-import kotlin.properties.Delegates
-
-fun testDelegator() {
-    var x: Boolean by LocalFreezableVar(true)
-    var y by LocalFreezableVar("")
-}
-
-class LocalFreezableVar<T>(private var value: T)  {
-    operator fun getValue(thisRef: Nothing?, property: KProperty<*>): T  = value
-
-    operator fun setValue(thisRef: Nothing?, property: KProperty<*>, value: T) {
-        this.value = value
-    }
-}
-
-
-<!REDECLARATION!>class C<!>
-operator fun C.plus(a: Any): C = this
-operator fun C.plusAssign(a: Any) {}
-
-fun testOperatorAssignment() {
-    val c = C()
-    c += ""
-    <!CAN_BE_VAL!>var<!> c1 = C()
-    <!ASSIGN_OPERATOR_AMBIGUITY!>c1 += ""<!>
-
-    var a = 1
-    a += 12
-    a -= 10
-}
-
-
-fun destructuringDeclaration() {
-    <!CAN_BE_VAL!>var<!> (v1, v2) = getPair()
-    print(v1)
-
-    var (v3, v4) = getPair()
-    print(v3)
-    v4 = ""
-
-    var (v5, v6) = getPair()
-    v5 = 1
-
-    var (v7, v8) = getPair()
-    v7 = 2
-    v8 = "42"
-
-    val (a, b, c) = Triple(1, 1, 1)
-
-    <!CAN_BE_VAL!>var<!> (x, y, z) = Triple(1, 1, 1)
-}
-
-fun stackOverflowBug() {
-    <!CAN_BE_VAL!>var<!> a: Int
-    a = 1
-    for (i in 1..10)
-        print(i)
-}
-
-fun smth(flag: Boolean) {
-    var a = 1
-
-    if (flag) {
-        while (a > 0) {
-            a--
-        }
-    }
-}
-
-fun withAnnotation(p: List<Any>) {
-    @Suppress("UNCHECKED_CAST")
-    <!CAN_BE_VAL!>var<!> v = p as List<String>
-    print(v)
-}
-
-fun withReadonlyDeligate() {
-    val s: String by lazy { "Hello!" }
-    s.hashCode()
-}
-
-fun getPair(): Pair<Int, String> = Pair(1, "1")
-
-fun listReceiver(p: List<String>) {}
-
-fun withInitializer() {
-    var v1 = 1
-    var v2 = 2
-    <!CAN_BE_VAL!>var<!> v3 = 3
-    v1 = 1
-    v2++
-    print(v3)
-}
-
-fun test() {
-    var a = 0
-    while (a>0) {
-        a++
-    }
-}
-
-fun foo() {
-    <!CAN_BE_VAL!>var<!> a: Int
-    val bool = true
-    val b: String
-
-    if (bool) a = 4 else a = 42
-
-    bool = false
-}
-
-fun cycles() {
-    var a = 10
-    while (a > 0) {
-        a--
-    }
-
-    val b: Int
-    while (a < 10) {
-        a++
-        b = a
-    }
-}
-
-fun assignedTwice(p: Int) {
-    var v: Int
-    v = 0
-    if (p > 0) v = 1
-}
-
-fun main(args: Array<String?>) {
-    <!CAN_BE_VAL!>var<!> a: String?
-
-    if (args.size == 1) {
-        a = args[0]
-    }
-    else {
-        a  = args.toString()
-    }
-
-    if (a != null && a.equals("cde")) return
-}
-
-fun run(f: () -> Unit) = f()
-
-fun lambda() {
-    var a: Int
-    a = 10
-
-    run {
-        a = 20
-    }
-}
-
-fun lambdaInitialization() {
-    <!CAN_BE_VAL!>var<!> a: Int
-
-    run {
-        a = 20
-    }
-}
-
-fun notAssignedWhenNotUsed(p: Int) {
-    <!CAN_BE_VAL!>var<!> v: Int
-    if (p > 0) {
-        v = 1
-        print(v)
-    }
-}
-
-var global = 1
-
-<!REDECLARATION!>class C {
-    var field = 2
-
-    fun foo() {
-        print(field)
-        print(global)
-    }
-}<!>
-
-fun withDelegate() {
-    var s: String by Delegates.notNull()
-    s = ""
-}

compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/extended/CanBeValChecker.kt

@@ -8,6 +8,7 @@ package org.jetbrains.kotlin.fir.analysis.checkers.extended
 
 import com.intellij.lang.LighterASTNode
 import com.intellij.openapi.util.Ref
+import org.jetbrains.kotlin.KtNodeTypes
 import org.jetbrains.kotlin.contracts.description.EventOccurrencesRange
 import org.jetbrains.kotlin.fir.*
 import org.jetbrains.kotlin.fir.analysis.cfa.AbstractFirPropertyInitializationChecker
@@ -16,14 +17,14 @@ import org.jetbrains.kotlin.fir.analysis.cfa.TraverseDirection
 import org.jetbrains.kotlin.fir.analysis.cfa.traverse
 import org.jetbrains.kotlin.fir.analysis.diagnostics.DiagnosticReporter
 import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors
+import org.jetbrains.kotlin.fir.analysis.getChildren
 import org.jetbrains.kotlin.fir.references.FirResolvedNamedReference
 import org.jetbrains.kotlin.fir.resolve.dfa.cfg.*
 import org.jetbrains.kotlin.fir.symbols.impl.FirPropertySymbol
 import org.jetbrains.kotlin.lexer.KtTokens
-import org.jetbrains.kotlin.psi.KtProperty
 
 
-object VariableAssignmentChecker : AbstractFirPropertyInitializationChecker() {
+object CanBeValChecker : AbstractFirPropertyInitializationChecker() {
     override fun analyze(
         graph: ControlFlowGraph,
         reporter: DiagnosticReporter,
@@ -38,7 +39,7 @@ object VariableAssignmentChecker : AbstractFirPropertyInitializationChecker() {
 
         for (property in unprocessedProperties) {
             if (property.fir.source is FirFakeSourceElement<*>) continue
-            if (property.callableId.callableName.asString() == "<destruct>") continue
+            if (property.isDestructuring) continue
             propertiesCharacteristics[property] = EventOccurrencesRange.ZERO
         }
 
@@ -47,9 +48,9 @@ object VariableAssignmentChecker : AbstractFirPropertyInitializationChecker() {
         var lastDestructuredVariables = 0
 
         for ((symbol, value) in propertiesCharacteristics) {
-            val source = symbol.getValOrVarSource
+            val source = symbol.fir.source?.getChildren(setOf(KtTokens.VAL_KEYWORD, KtTokens.VAR_KEYWORD), depth = 1)
-            if (symbol.callableId.callableName.asString() == "<destruct>") {
+            if (symbol.isDestructuring) {
-                lastDestructuringSource = symbol.getValOrVarSource
+                lastDestructuringSource = source
                 lastDestructuredVariables = symbol.getDestructuringChildrenCount() ?: continue
                 destructuringCanBeVal = true
                 continue
@@ -71,10 +72,7 @@ object VariableAssignmentChecker : AbstractFirPropertyInitializationChecker() {
     }
 
     private fun canBeVal(symbol: FirPropertySymbol, value: EventOccurrencesRange) =
-        (value == EventOccurrencesRange.EXACTLY_ONCE
+        value in canBeValOccurrenceRanges && symbol.fir.isVar
-                || value == EventOccurrencesRange.AT_MOST_ONCE
-                || value == EventOccurrencesRange.ZERO
-                ) && symbol.fir.isVar
 
     private class UninitializedPropertyReporter(
         val data: Map<CFGNode<*>, PropertyInitializationInfo>,
@@ -104,24 +102,6 @@ object VariableAssignmentChecker : AbstractFirPropertyInitializationChecker() {
         }
     }
 
-    private val FirPropertySymbol.getValOrVarSource
-        get() = when (fir.source) {
-            is FirSourceElement -> {
-                (fir.psi as? KtProperty)?.valOrVarKeyword?.toFirPsiSourceElement()
-                    ?: fir.psi?.firstChild?.toFirPsiSourceElement()
-                    ?: fir.source
-            }
-            is FirLightSourceElement -> {
-                val children = Ref<Array<LighterASTNode>>()
-                val tree = (fir.source as FirLightSourceElement).tree
-                tree.getChildren(tree.root, children)
-                children.get().first { it.tokenType == KtTokens.VAL_KEYWORD || it.tokenType == KtTokens.VAR_KEYWORD }.let {
-                    it.toFirLightSourceElement(it.startOffset, it.endOffset, tree)
-                }
-            }
-            else -> null
-        }
-
     private fun FirPropertySymbol.getDestructuringChildrenCount(): Int? = when (fir.source) {
         is FirPsiSourceElement<*> -> fir.psi?.children?.size?.minus(1) // -1 cuz we don't need expression node after equals operator
         is FirLightSourceElement -> {
@@ -129,8 +109,17 @@ object VariableAssignmentChecker : AbstractFirPropertyInitializationChecker() {
             val tree = (fir.source as FirLightSourceElement).tree
             val children = Ref<Array<LighterASTNode?>>()
             tree.getChildren(source.element, children)
-            children.get().count { it != null }
+            children.get().filterNotNull().filter { it.tokenType == KtNodeTypes.DESTRUCTURING_DECLARATION_ENTRY }.size
         }
         else -> null
     }
+
+    private val FirPropertySymbol.isDestructuring
+        get() = callableId.callableName.asString() == "<destruct>"
+
+    private val canBeValOccurrenceRanges = setOf(
+        EventOccurrencesRange.EXACTLY_ONCE,
+        EventOccurrencesRange.AT_MOST_ONCE,
+        EventOccurrencesRange.ZERO
+    )
 }

compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/collectors/AbstractDiagnosticCollector.kt

@@ -20,7 +20,6 @@ import org.jetbrains.kotlin.fir.resolve.SessionHolder
 import org.jetbrains.kotlin.fir.resolve.collectImplicitReceivers
 import org.jetbrains.kotlin.fir.resolve.defaultType
 import org.jetbrains.kotlin.fir.types.ConeKotlinType
-import org.jetbrains.kotlin.fir.types.FirErrorTypeRef
 import org.jetbrains.kotlin.fir.types.FirTypeRef
 import org.jetbrains.kotlin.fir.types.builder.buildResolvedTypeRef
 import org.jetbrains.kotlin.fir.types.coneTypeSafe