From 5e0dc53295a7c8460c2c0a035b70f41a76c2e90a Mon Sep 17 00:00:00 2001 From: vldf Date: Thu, 27 Aug 2020 10:36:48 +0300 Subject: [PATCH] [FIR] Refactor CanBeValChecker --- .../extendedCheckers/CanBeValChecker.kt | 185 +++++++++++++++++ ...ignmentChecker.txt => CanBeValChecker.txt} | 23 +-- .../VariableAssignmentChecker.kt | 187 ------------------ ...ssignmentChecker.kt => CanBeValChecker.kt} | 47 ++--- .../collectors/AbstractDiagnosticCollector.kt | 1 - 5 files changed, 212 insertions(+), 231 deletions(-) create mode 100644 compiler/fir/analysis-tests/testData/extendedCheckers/CanBeValChecker.kt rename compiler/fir/analysis-tests/testData/extendedCheckers/{VariableAssignmentChecker.txt => CanBeValChecker.txt} (96%) delete mode 100644 compiler/fir/analysis-tests/testData/extendedCheckers/VariableAssignmentChecker.kt rename compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/extended/{VariableAssignmentChecker.kt => CanBeValChecker.kt} (77%) diff --git a/compiler/fir/analysis-tests/testData/extendedCheckers/CanBeValChecker.kt b/compiler/fir/analysis-tests/testData/extendedCheckers/CanBeValChecker.kt new file mode 100644 index 00000000000..69ad599adbc --- /dev/null +++ b/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 x: Boolean by LocalFreezableVar(true) + var y by LocalFreezableVar("") +} + +class LocalFreezableVar(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 += "" + var c1 = C() + c1 += "" + + var a = 1 + a += 12 + a -= 10 +} + + +fun destructuringDeclaration() { + 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) + + var (x, y, z) = Triple(1, 1, 1) +} + +fun stackOverflowBug() { + 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) { + @Suppress("UNCHECKED_CAST") + var v = p as List + print(v) +} + +fun withReadonlyDeligate() { + val s: String by lazy { "Hello!" } + s.hashCode() +} + +fun getPair(): Pair = Pair(1, "1") + +fun listReceiver(p: List) {} + +fun withInitializer() { + var v1 = 1 + var v2 = 2 + var v3 = 3 + v1 = 1 + v2++ // todo mark this UNUSED_CHANGED_VALUES + print(v3) +} + +fun test() { + var a = 0 + while (a>0) { + a++ + } +} + +fun foo() { + var a: Int + val bool = true + if (bool) a = 4 else a = 42 + val b: String + + bool = false +} + +fun cycles() { + var a = 10 + while (a > 0) { + a-- + } + + var 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) { + var a: String? + val unused = 0 + + 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() { + var a: Int + + run { + a = 20 + } +} + +fun notAssignedWhenNotUsed(p: Int) { + 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 s: String by Delegates.notNull() + s = "" +} diff --git a/compiler/fir/analysis-tests/testData/extendedCheckers/VariableAssignmentChecker.txt b/compiler/fir/analysis-tests/testData/extendedCheckers/CanBeValChecker.txt similarity index 96% rename from compiler/fir/analysis-tests/testData/extendedCheckers/VariableAssignmentChecker.txt rename to compiler/fir/analysis-tests/testData/extendedCheckers/CanBeValChecker.txt index 39d296da7a3..99c24ccda3c 100644 --- a/compiler/fir/analysis-tests/testData/extendedCheckers/VariableAssignmentChecker.txt +++ b/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|(Boolean(true)) lvar y: R|kotlin/String|by R|/LocalFreezableVar.LocalFreezableVar|(String()) @@ -20,12 +20,6 @@ FILE: VariableAssignmentChecker.kt this@R|/LocalFreezableVar|.R|/LocalFreezableVar.value| = R|/value| } - } - public final class C : R|kotlin/Any| { - public constructor(): R|C| { - super() - } - } 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|/bool| -> { R|/a| = Int(4) @@ -141,6 +134,7 @@ FILE: VariableAssignmentChecker.kt } } + lval b: R|kotlin/String| R|/bool| = Boolean(false) } public final fun cycles(): R|kotlin/Unit| { @@ -151,7 +145,7 @@ FILE: VariableAssignmentChecker.kt R|/| } - lval b: R|kotlin/Int| + lvar b: R|kotlin/Int| while(CMP(<, R|/a|.R|kotlin/Int.compareTo|(Int(10)))) { lval : R|kotlin/Int| = R|/a| R|/a| = R|/|.R|kotlin/Int.inc|() @@ -172,18 +166,19 @@ FILE: VariableAssignmentChecker.kt } public final fun main(args: R|kotlin/Array|): R|kotlin/Unit| { lvar a: R|kotlin/String?| + lval unused: R|kotlin/Int| = Int(0) when () { ==(R|/args|.R|kotlin/Array.size|, Int(1)) -> { R|/a| = R|/args|.R|FakeOverride|(Int(0)) } else -> { R|/a| = R|/args|.R|kotlin/Any.toString|() - } - } + when () { + !=(R|/a|, Null(null)) && R|/a|.R|kotlin/Any.equals|(String(cde)) -> { + ^main Unit + } + } - when () { - !=(R|/a|, Null(null)) && R|/a|.R|kotlin/Any.equals|(String(cde)) -> { - ^main Unit } } diff --git a/compiler/fir/analysis-tests/testData/extendedCheckers/VariableAssignmentChecker.kt b/compiler/fir/analysis-tests/testData/extendedCheckers/VariableAssignmentChecker.kt deleted file mode 100644 index d89bde6a57a..00000000000 --- a/compiler/fir/analysis-tests/testData/extendedCheckers/VariableAssignmentChecker.kt +++ /dev/null @@ -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(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 - } -} - - -class C -operator fun C.plus(a: Any): C = this -operator fun C.plusAssign(a: Any) {} - -fun testOperatorAssignment() { - val c = C() - c += "" - var c1 = C() - c1 += "" - - var a = 1 - a += 12 - a -= 10 -} - - -fun destructuringDeclaration() { - 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) - - var (x, y, z) = Triple(1, 1, 1) -} - -fun stackOverflowBug() { - 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) { - @Suppress("UNCHECKED_CAST") - var v = p as List - print(v) -} - -fun withReadonlyDeligate() { - val s: String by lazy { "Hello!" } - s.hashCode() -} - -fun getPair(): Pair = Pair(1, "1") - -fun listReceiver(p: List) {} - -fun withInitializer() { - var v1 = 1 - var v2 = 2 - var v3 = 3 - v1 = 1 - v2++ - print(v3) -} - -fun test() { - var a = 0 - while (a>0) { - a++ - } -} - -fun foo() { - 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) { - 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() { - var a: Int - - run { - a = 20 - } -} - -fun notAssignedWhenNotUsed(p: Int) { - 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 s: String by Delegates.notNull() - s = "" -} diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/extended/VariableAssignmentChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/extended/CanBeValChecker.kt similarity index 77% rename from compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/extended/VariableAssignmentChecker.kt rename to compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/extended/CanBeValChecker.kt index 743a6b3d249..ebd40c621b9 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/extended/VariableAssignmentChecker.kt +++ b/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() == "") 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 - if (symbol.callableId.callableName.asString() == "") { - lastDestructuringSource = symbol.getValOrVarSource + val source = symbol.fir.source?.getChildren(setOf(KtTokens.VAL_KEYWORD, KtTokens.VAR_KEYWORD), depth = 1) + if (symbol.isDestructuring) { + 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 == EventOccurrencesRange.AT_MOST_ONCE - || value == EventOccurrencesRange.ZERO - ) && symbol.fir.isVar + value in canBeValOccurrenceRanges && symbol.fir.isVar private class UninitializedPropertyReporter( val data: Map, 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>() - 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>() 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() == "" + + private val canBeValOccurrenceRanges = setOf( + EventOccurrencesRange.EXACTLY_ONCE, + EventOccurrencesRange.AT_MOST_ONCE, + EventOccurrencesRange.ZERO + ) } \ No newline at end of file diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/collectors/AbstractDiagnosticCollector.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/collectors/AbstractDiagnosticCollector.kt index c6f2f50c772..5cc1e393661 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/collectors/AbstractDiagnosticCollector.kt +++ b/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