[FIR] Remove implicit coercion of ifs and whens to Unit
The rule of thumb is the following: If the `if` and `when` can be successfully replaced with `while`, then it is used as a statement, otherwise, it is used as an expression. #KT-59883
This commit is contained in:
committed by
Space Team
parent
2e66954d01
commit
931f2eab58
+6
@@ -8062,6 +8062,12 @@ public class DiagnosticCompilerTestFE10TestdataTestGenerated extends AbstractDia
|
|||||||
runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/throwInLambda.kt");
|
runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/throwInLambda.kt");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
@TestMetadata("tryCatchFinallyIfs.kt")
|
||||||
|
public void testTryCatchFinallyIfs() {
|
||||||
|
runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/tryCatchFinallyIfs.kt");
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
@TestMetadata("tryWithAssignmentUsedInCatch.kt")
|
@TestMetadata("tryWithAssignmentUsedInCatch.kt")
|
||||||
public void testTryWithAssignmentUsedInCatch() {
|
public void testTryWithAssignmentUsedInCatch() {
|
||||||
|
|||||||
+6
@@ -8062,6 +8062,12 @@ public class LLFirPreresolvedReversedDiagnosticCompilerFE10TestDataTestGenerated
|
|||||||
runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/throwInLambda.kt");
|
runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/throwInLambda.kt");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
@TestMetadata("tryCatchFinallyIfs.kt")
|
||||||
|
public void testTryCatchFinallyIfs() {
|
||||||
|
runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/tryCatchFinallyIfs.kt");
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
@TestMetadata("tryWithAssignmentUsedInCatch.kt")
|
@TestMetadata("tryWithAssignmentUsedInCatch.kt")
|
||||||
public void testTryWithAssignmentUsedInCatch() {
|
public void testTryWithAssignmentUsedInCatch() {
|
||||||
|
|||||||
+6
@@ -8056,6 +8056,12 @@ public class FirLightTreeOldFrontendDiagnosticsTestGenerated extends AbstractFir
|
|||||||
runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/throwInLambda.kt");
|
runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/throwInLambda.kt");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
@TestMetadata("tryCatchFinallyIfs.kt")
|
||||||
|
public void testTryCatchFinallyIfs() {
|
||||||
|
runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/tryCatchFinallyIfs.kt");
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
@TestMetadata("tryWithAssignmentUsedInCatch.kt")
|
@TestMetadata("tryWithAssignmentUsedInCatch.kt")
|
||||||
public void testTryWithAssignmentUsedInCatch() {
|
public void testTryWithAssignmentUsedInCatch() {
|
||||||
|
|||||||
+6
@@ -8062,6 +8062,12 @@ public class FirPsiOldFrontendDiagnosticsTestGenerated extends AbstractFirPsiDia
|
|||||||
runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/throwInLambda.kt");
|
runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/throwInLambda.kt");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
@TestMetadata("tryCatchFinallyIfs.kt")
|
||||||
|
public void testTryCatchFinallyIfs() {
|
||||||
|
runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/tryCatchFinallyIfs.kt");
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
@TestMetadata("tryWithAssignmentUsedInCatch.kt")
|
@TestMetadata("tryWithAssignmentUsedInCatch.kt")
|
||||||
public void testTryWithAssignmentUsedInCatch() {
|
public void testTryWithAssignmentUsedInCatch() {
|
||||||
|
|||||||
+1
-1
@@ -90,7 +90,7 @@ abstract class AbstractLightTreeRawFirBuilder(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private fun LighterASTNode.getLastChildExpression(): LighterASTNode? {
|
fun LighterASTNode.getLastChildExpression(): LighterASTNode? {
|
||||||
var result: LighterASTNode? = null
|
var result: LighterASTNode? = null
|
||||||
forEachChildren {
|
forEachChildren {
|
||||||
if (it.isExpression()) {
|
if (it.isExpression()) {
|
||||||
|
|||||||
+12
-6
@@ -1402,13 +1402,19 @@ class LightTreeRawFirExpressionBuilder(
|
|||||||
parent = parent.getParent() ?: return true
|
parent = parent.getParent() ?: return true
|
||||||
}
|
}
|
||||||
val parentTokenType = parent.tokenType
|
val parentTokenType = parent.tokenType
|
||||||
if (parentTokenType == BLOCK) return false
|
return when (parentTokenType) {
|
||||||
if (parentTokenType == THEN || parentTokenType == ELSE || parentTokenType == WHEN_ENTRY) {
|
BLOCK -> parent.getLastChildExpression() == this && parent.usedAsExpression
|
||||||
return parent.getParent()?.usedAsExpression ?: true
|
TRY, CATCH -> parent.usedAsExpression
|
||||||
|
THEN, ELSE, WHEN_ENTRY -> parent.getParent()?.usedAsExpression ?: true
|
||||||
|
CLASS_INITIALIZER, SCRIPT_INITIALIZER, SECONDARY_CONSTRUCTOR, FUNCTION_LITERAL, FINALLY -> false
|
||||||
|
FUN, PROPERTY_ACCESSOR -> parent.getChildrenAsArray().any { it?.tokenType == EQ }
|
||||||
|
DOT_QUALIFIED_EXPRESSION -> parent.getFirstChild() == this
|
||||||
|
BODY -> when (parent.getParent()?.tokenType) {
|
||||||
|
FOR, WHILE, DO_WHILE -> false
|
||||||
|
else -> true
|
||||||
|
}
|
||||||
|
else -> true
|
||||||
}
|
}
|
||||||
if (parentTokenType != BODY) return true
|
|
||||||
val type = parent.getParent()?.tokenType ?: return true
|
|
||||||
return !(type == FOR || type == WHILE || type == DO_WHILE)
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
+22
-6
@@ -2690,17 +2690,33 @@ open class PsiRawFirBuilder(
|
|||||||
) {
|
) {
|
||||||
parent = parent.parent
|
parent = parent.parent
|
||||||
}
|
}
|
||||||
if (parent is KtBlockExpression) return false
|
|
||||||
when (parent.elementType) {
|
when (parent.elementType) {
|
||||||
KtNodeTypes.THEN, KtNodeTypes.ELSE, KtNodeTypes.WHEN_ENTRY -> {
|
KtNodeTypes.THEN, KtNodeTypes.ELSE, KtNodeTypes.WHEN_ENTRY -> {
|
||||||
return (parent.parent as? KtExpression)?.usedAsExpression != false
|
return (parent.parent as? KtExpression)?.usedAsExpression != false
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
if (parent is KtScriptInitializer) return false
|
|
||||||
// Here we check that when used is a single statement of a loop
|
fun PsiElement.getLastChildExpression() = children.asList().asReversed().firstIsInstanceOrNull<KtExpression>()
|
||||||
if (parent !is KtContainerNodeForControlStructureBody) return true
|
|
||||||
val type = parent.parent.elementType
|
return when (parent) {
|
||||||
return !(type == KtNodeTypes.FOR || type == KtNodeTypes.WHILE || type == KtNodeTypes.DO_WHILE)
|
// todo KT-62472 Replace with the following when K2 is used in TeamCity
|
||||||
|
// is KtBlockExpression, is KtTryExpression -> parent.getLastChildExpression() == this && parent.usedAsExpression
|
||||||
|
is KtBlockExpression -> parent.getLastChildExpression() == this && parent.usedAsExpression
|
||||||
|
is KtTryExpression -> parent.getLastChildExpression() == this && parent.usedAsExpression
|
||||||
|
is KtCatchClause -> (parent.parent as? KtTryExpression)?.usedAsExpression == true
|
||||||
|
is KtClassInitializer, is KtScriptInitializer, is KtSecondaryConstructor, is KtFunctionLiteral, is KtFinallySection -> false
|
||||||
|
is KtDotQualifiedExpression -> parent.firstChild == this
|
||||||
|
// todo KT-62472 Replace with the following when K2 is used in TeamCity
|
||||||
|
// is KtFunction, is KtPropertyAccessor -> parent.hasBody() && !parent.hasBlockBody()
|
||||||
|
is KtFunction -> parent.hasBody() && !parent.hasBlockBody()
|
||||||
|
is KtPropertyAccessor -> parent.hasBody() && !parent.hasBlockBody()
|
||||||
|
is KtContainerNodeForControlStructureBody -> when (parent.parent.elementType) {
|
||||||
|
KtNodeTypes.FOR, KtNodeTypes.WHILE, KtNodeTypes.DO_WHILE -> false
|
||||||
|
else -> true
|
||||||
|
}
|
||||||
|
else -> true
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
override fun visitDoWhileExpression(expression: KtDoWhileExpression, data: FirElement?): FirElement {
|
override fun visitDoWhileExpression(expression: KtDoWhileExpression, data: FirElement?): FirElement {
|
||||||
|
|||||||
@@ -71,7 +71,7 @@ fun main1() {
|
|||||||
|
|
||||||
1.<!ILLEGAL_SELECTOR!>"sdf"<!>
|
1.<!ILLEGAL_SELECTOR!>"sdf"<!>
|
||||||
1.<!ILLEGAL_SELECTOR!>{}<!>
|
1.<!ILLEGAL_SELECTOR!>{}<!>
|
||||||
1.<!ILLEGAL_SELECTOR!><!INVALID_IF_AS_EXPRESSION!>if<!> (true) {}<!>
|
1.<!ILLEGAL_SELECTOR!>if (true) {}<!>
|
||||||
}
|
}
|
||||||
|
|
||||||
fun test() {
|
fun test() {
|
||||||
|
|||||||
+184
@@ -0,0 +1,184 @@
|
|||||||
|
// FIR_IDENTICAL
|
||||||
|
// DIAGNOSTICS: -UNUSED_EXPRESSION
|
||||||
|
|
||||||
|
fun ifExpr() = try {
|
||||||
|
if (true) 2
|
||||||
|
<!INVALID_IF_AS_EXPRESSION!>if<!> (true) 2
|
||||||
|
} catch (e: Exception) {
|
||||||
|
if (true) 3
|
||||||
|
<!INVALID_IF_AS_EXPRESSION!>if<!> (true) 3
|
||||||
|
} catch (e: Throwable) {
|
||||||
|
if (true) 4
|
||||||
|
<!INVALID_IF_AS_EXPRESSION!>if<!> (true) 4
|
||||||
|
} finally {
|
||||||
|
if (true) 5
|
||||||
|
if (true) 5
|
||||||
|
}
|
||||||
|
|
||||||
|
fun ifBlock() = try {
|
||||||
|
if (true) {
|
||||||
|
2
|
||||||
|
}
|
||||||
|
<!INVALID_IF_AS_EXPRESSION!>if<!> (true) {
|
||||||
|
2
|
||||||
|
}
|
||||||
|
} catch (e: Exception) {
|
||||||
|
if (true) {
|
||||||
|
3
|
||||||
|
}
|
||||||
|
<!INVALID_IF_AS_EXPRESSION!>if<!> (true) {
|
||||||
|
3
|
||||||
|
}
|
||||||
|
} catch (e: Throwable) {
|
||||||
|
if (true) {
|
||||||
|
4
|
||||||
|
}
|
||||||
|
<!INVALID_IF_AS_EXPRESSION!>if<!> (true) {
|
||||||
|
4
|
||||||
|
}
|
||||||
|
} finally {
|
||||||
|
if (true) {
|
||||||
|
5
|
||||||
|
}
|
||||||
|
if (true) {
|
||||||
|
5
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
fun whenExpr() = try {
|
||||||
|
when {
|
||||||
|
true -> 2
|
||||||
|
}
|
||||||
|
<!NO_ELSE_IN_WHEN!>when<!> {
|
||||||
|
true -> 2
|
||||||
|
}
|
||||||
|
} catch (e: Exception) {
|
||||||
|
when {
|
||||||
|
true -> 3
|
||||||
|
}
|
||||||
|
<!NO_ELSE_IN_WHEN!>when<!> {
|
||||||
|
true -> 3
|
||||||
|
}
|
||||||
|
} catch (e: Throwable) {
|
||||||
|
when {
|
||||||
|
true -> 4
|
||||||
|
}
|
||||||
|
<!NO_ELSE_IN_WHEN!>when<!> {
|
||||||
|
true -> 4
|
||||||
|
}
|
||||||
|
} finally {
|
||||||
|
when {
|
||||||
|
true -> 5
|
||||||
|
}
|
||||||
|
when {
|
||||||
|
true -> 5
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
fun whenBlock() = try {
|
||||||
|
when {
|
||||||
|
true -> {
|
||||||
|
2
|
||||||
|
}
|
||||||
|
}
|
||||||
|
<!NO_ELSE_IN_WHEN!>when<!> {
|
||||||
|
true -> {
|
||||||
|
2
|
||||||
|
}
|
||||||
|
}
|
||||||
|
} catch (e: Exception) {
|
||||||
|
when {
|
||||||
|
true -> {
|
||||||
|
3
|
||||||
|
}
|
||||||
|
}
|
||||||
|
<!NO_ELSE_IN_WHEN!>when<!> {
|
||||||
|
true -> {
|
||||||
|
3
|
||||||
|
}
|
||||||
|
}
|
||||||
|
} catch (e: Throwable) {
|
||||||
|
when {
|
||||||
|
true -> {
|
||||||
|
4
|
||||||
|
}
|
||||||
|
}
|
||||||
|
<!NO_ELSE_IN_WHEN!>when<!> {
|
||||||
|
true -> {
|
||||||
|
4
|
||||||
|
}
|
||||||
|
}
|
||||||
|
} finally {
|
||||||
|
when {
|
||||||
|
true -> {
|
||||||
|
5
|
||||||
|
}
|
||||||
|
}
|
||||||
|
when {
|
||||||
|
true -> {
|
||||||
|
5
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
fun ifExpr2(): Any {
|
||||||
|
try {
|
||||||
|
if (true) 2
|
||||||
|
if (true) 2
|
||||||
|
} catch (e: Exception) {
|
||||||
|
if (true) 3
|
||||||
|
if (true) 3
|
||||||
|
} catch (e: Throwable) {
|
||||||
|
if (true) 4
|
||||||
|
if (true) 4
|
||||||
|
} finally {
|
||||||
|
if (true) 5
|
||||||
|
if (true) 5
|
||||||
|
}
|
||||||
|
|
||||||
|
return try {
|
||||||
|
if (true) 2
|
||||||
|
<!INVALID_IF_AS_EXPRESSION!>if<!> (true) 2
|
||||||
|
} catch (e: Exception) {
|
||||||
|
if (true) 3
|
||||||
|
<!INVALID_IF_AS_EXPRESSION!>if<!> (true) 3
|
||||||
|
} catch (e: Throwable) {
|
||||||
|
if (true) 4
|
||||||
|
<!INVALID_IF_AS_EXPRESSION!>if<!> (true) 4
|
||||||
|
} finally {
|
||||||
|
if (true) 5
|
||||||
|
if (true) 5
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
fun ifExpr3() {
|
||||||
|
try {
|
||||||
|
if (true) 2
|
||||||
|
if (true) 2
|
||||||
|
} catch (e: Exception) {
|
||||||
|
if (true) 3
|
||||||
|
if (true) 3
|
||||||
|
} catch (e: Throwable) {
|
||||||
|
if (true) 4
|
||||||
|
if (true) 4
|
||||||
|
} finally {
|
||||||
|
if (true) 5
|
||||||
|
if (true) 5
|
||||||
|
}
|
||||||
|
|
||||||
|
try {
|
||||||
|
if (true) 2
|
||||||
|
if (true) 2
|
||||||
|
} catch (e: Exception) {
|
||||||
|
if (true) 3
|
||||||
|
if (true) 3
|
||||||
|
} catch (e: Throwable) {
|
||||||
|
if (true) 4
|
||||||
|
if (true) 4
|
||||||
|
} finally {
|
||||||
|
if (true) 5
|
||||||
|
if (true) 5
|
||||||
|
}
|
||||||
|
}
|
||||||
+1
-1
@@ -18,7 +18,7 @@ fun testMixedIfAndWhen() =
|
|||||||
fun testWrappedExpressions() =
|
fun testWrappedExpressions() =
|
||||||
if (true) {
|
if (true) {
|
||||||
println()
|
println()
|
||||||
if (true) {
|
<!INVALID_IF_AS_EXPRESSION!>if<!> (true) {
|
||||||
println()
|
println()
|
||||||
if (true) {
|
if (true) {
|
||||||
println()
|
println()
|
||||||
|
|||||||
@@ -1,11 +1,11 @@
|
|||||||
package f
|
package f
|
||||||
|
|
||||||
fun test(a: Boolean, b: Boolean): Int {
|
fun test(a: Boolean, b: Boolean): Int {
|
||||||
return <!RETURN_TYPE_MISMATCH!>if(a) {
|
return if(a) {
|
||||||
1
|
1
|
||||||
} else {
|
} else {
|
||||||
if (b) {
|
<!INVALID_IF_AS_EXPRESSION!>if<!> (b) {
|
||||||
3
|
3
|
||||||
}
|
}
|
||||||
}<!>
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,37 +0,0 @@
|
|||||||
/*
|
|
||||||
* KOTLIN DIAGNOSTICS SPEC TEST (POSITIVE)
|
|
||||||
*
|
|
||||||
* SPEC VERSION: 0.1-152
|
|
||||||
* PRIMARY LINKS: expressions, when-expression -> paragraph 5 -> sentence 1
|
|
||||||
* expressions, when-expression, exhaustive-when-expressions -> paragraph 1 -> sentence 1
|
|
||||||
* expressions, when-expression, exhaustive-when-expressions -> paragraph 2 -> sentence 1
|
|
||||||
* expressions, when-expression -> paragraph 6 -> sentence 5
|
|
||||||
*/
|
|
||||||
|
|
||||||
// KT-4434 Missed diagnostic about else branch in when
|
|
||||||
|
|
||||||
package test
|
|
||||||
|
|
||||||
fun foo(): Int {
|
|
||||||
val a = "a"
|
|
||||||
return <!RETURN_TYPE_MISMATCH!>if (a.length > 0) {
|
|
||||||
when (a) {
|
|
||||||
"a" -> 1
|
|
||||||
}
|
|
||||||
}
|
|
||||||
else {
|
|
||||||
3
|
|
||||||
}<!>
|
|
||||||
}
|
|
||||||
|
|
||||||
fun bar(): Int {
|
|
||||||
val a = "a"
|
|
||||||
if (a.length > 0) {
|
|
||||||
return <!NO_ELSE_IN_WHEN!>when<!> (a) {
|
|
||||||
"a" -> 1
|
|
||||||
}
|
|
||||||
}
|
|
||||||
else {
|
|
||||||
return 3
|
|
||||||
}
|
|
||||||
}
|
|
||||||
@@ -1,3 +1,4 @@
|
|||||||
|
// FIR_IDENTICAL
|
||||||
/*
|
/*
|
||||||
* KOTLIN DIAGNOSTICS SPEC TEST (POSITIVE)
|
* KOTLIN DIAGNOSTICS SPEC TEST (POSITIVE)
|
||||||
*
|
*
|
||||||
|
|||||||
-13
@@ -1,13 +0,0 @@
|
|||||||
// SKIP_TXT
|
|
||||||
fun bar(a: String): String {
|
|
||||||
return <!RETURN_TYPE_MISMATCH!>when {
|
|
||||||
a.length == 1 -> {
|
|
||||||
when { // Error in K1, no error in K2
|
|
||||||
a == "a" -> ""
|
|
||||||
a == "b" -> ""
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
else -> ""
|
|
||||||
}<!>
|
|
||||||
}
|
|
||||||
@@ -1,3 +1,4 @@
|
|||||||
|
// FIR_IDENTICAL
|
||||||
// SKIP_TXT
|
// SKIP_TXT
|
||||||
fun bar(a: String): String {
|
fun bar(a: String): String {
|
||||||
return when {
|
return when {
|
||||||
|
|||||||
Generated
+6
@@ -8062,6 +8062,12 @@ public class DiagnosticTestGenerated extends AbstractDiagnosticTest {
|
|||||||
runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/throwInLambda.kt");
|
runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/throwInLambda.kt");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
@TestMetadata("tryCatchFinallyIfs.kt")
|
||||||
|
public void testTryCatchFinallyIfs() {
|
||||||
|
runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/tryCatchFinallyIfs.kt");
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
@TestMetadata("tryWithAssignmentUsedInCatch.kt")
|
@TestMetadata("tryWithAssignmentUsedInCatch.kt")
|
||||||
public void testTryWithAssignmentUsedInCatch() {
|
public void testTryWithAssignmentUsedInCatch() {
|
||||||
|
|||||||
Reference in New Issue
Block a user