From 4b9e15dfa8dec3988260d490baf02db1e1f95780 Mon Sep 17 00:00:00 2001 From: Ivan Kochurkin Date: Tue, 19 Sep 2023 13:58:31 +0200 Subject: [PATCH] [FIR] Fix missing RETURN_TYPE_MISMATCH for labeled return statements in lambda functions, ^KT-59907 Fixed (cherry picked from commit 8e72f60996d61ab5f26ce32f7a6470bc19ebd7e7) --- .../basic.kt | 2 +- .../FirFunctionReturnTypeMismatchChecker.kt | 33 +++++++++++++++---- .../return/LocalReturnNoCoercionToUnit.fir.kt | 2 +- .../return/LocalReturnSecondUnit.fir.kt | 6 ---- .../return/LocalReturnSecondUnit.kt | 3 +- .../LocalReturnUnitAndDontCareType.fir.kt | 4 +-- .../LocalReturnsWithExplicitReturnType.fir.kt | 2 +- .../typeMismatchInLambda.fir.kt | 2 +- .../returnAsLastStatementInLambda.fir.kt | 4 +-- 9 files changed, 37 insertions(+), 21 deletions(-) delete mode 100644 compiler/testData/diagnostics/tests/functionLiterals/return/LocalReturnSecondUnit.fir.kt diff --git a/compiler/fir/analysis-tests/testData/resolveWithStdlib/diagnostics/functionReturnTypeMismatchChecker/basic.kt b/compiler/fir/analysis-tests/testData/resolveWithStdlib/diagnostics/functionReturnTypeMismatchChecker/basic.kt index 9a37dcdcb6a..cb7460aad0c 100644 --- a/compiler/fir/analysis-tests/testData/resolveWithStdlib/diagnostics/functionReturnTypeMismatchChecker/basic.kt +++ b/compiler/fir/analysis-tests/testData/resolveWithStdlib/diagnostics/functionReturnTypeMismatchChecker/basic.kt @@ -37,7 +37,7 @@ fun errorWithLambda(): String { return@foo } foo { bar() - return@foo 10 + return@foo 10 } return "" diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/expression/FirFunctionReturnTypeMismatchChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/expression/FirFunctionReturnTypeMismatchChecker.kt index f7f97cbc7db..6c2acf0bbc7 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/expression/FirFunctionReturnTypeMismatchChecker.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/expression/FirFunctionReturnTypeMismatchChecker.kt @@ -9,20 +9,19 @@ import org.jetbrains.kotlin.KtFakeSourceElementKind import org.jetbrains.kotlin.KtRealSourceElementKind import org.jetbrains.kotlin.diagnostics.DiagnosticReporter import org.jetbrains.kotlin.diagnostics.reportOn +import org.jetbrains.kotlin.fir.analysis.cfa.util.previousCfgNodes import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext import org.jetbrains.kotlin.fir.analysis.checkers.declaration.hasExplicitReturnType import org.jetbrains.kotlin.fir.analysis.checkers.isSubtypeForTypeMismatch import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.NULL_FOR_NONNULL_TYPE import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.RETURN_TYPE_MISMATCH import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.SMARTCAST_IMPOSSIBLE -import org.jetbrains.kotlin.fir.declarations.FirAnonymousFunction -import org.jetbrains.kotlin.fir.declarations.FirConstructor -import org.jetbrains.kotlin.fir.declarations.FirErrorFunction -import org.jetbrains.kotlin.fir.declarations.FirPropertyAccessor +import org.jetbrains.kotlin.fir.declarations.* import org.jetbrains.kotlin.fir.expressions.FirReturnExpression import org.jetbrains.kotlin.fir.expressions.FirSmartCastExpression import org.jetbrains.kotlin.fir.expressions.FirWhenExpression import org.jetbrains.kotlin.fir.expressions.isExhaustive +import org.jetbrains.kotlin.fir.resolve.dfa.controlFlowGraph import org.jetbrains.kotlin.fir.types.* object FirFunctionReturnTypeMismatchChecker : FirReturnExpressionChecker() { @@ -31,7 +30,7 @@ object FirFunctionReturnTypeMismatchChecker : FirReturnExpressionChecker() { if (expression.source?.kind == KtFakeSourceElementKind.DelegatedPropertyAccessor) return val targetElement = expression.target.labeledElement - if (targetElement is FirErrorFunction || targetElement is FirAnonymousFunction && targetElement.isLambda) { + if (targetElement is FirErrorFunction) { return } @@ -91,7 +90,21 @@ object FirFunctionReturnTypeMismatchChecker : FirReturnExpressionChecker() { ) } } - } else if (resultExpression.source?.kind is KtFakeSourceElementKind.ImplicitUnit && !functionReturnType.isUnit) { + } else if (resultExpression.source?.kind is KtFakeSourceElementKind.ImplicitUnit && + !functionReturnType.isUnit && + shouldCheckMismatchForAnonymousFunction(targetElement, expression) + ) { + // Disallow cases like + // fun foo(): Any { return } + // Allow cases like + // fun foo(): Unit { return } + // fun foo() { return Unit } + // But ignore anonymous functions without explicit returns, the following code is valid (where type of parameter R is being inferenced): + // run { + // if (flag) return@run + // "str" + // } + // If type parameter is specified explicitly, checking is performed in the branch above and RETURN_TYPE_MISMATCH is reported reporter.reportOn( resultExpression.source, RETURN_TYPE_MISMATCH, @@ -103,4 +116,12 @@ object FirFunctionReturnTypeMismatchChecker : FirReturnExpressionChecker() { ) } } + + private fun shouldCheckMismatchForAnonymousFunction(targetElement: FirFunction, expression: FirReturnExpression): Boolean { + if (targetElement !is FirAnonymousFunction || !targetElement.isLambda) return true + val cfgNodes = targetElement.controlFlowGraphReference?.controlFlowGraph?.exitNode?.previousCfgNodes ?: return true + // Check if any return expression other than the current is explicit + return cfgNodes.any { if (it.fir === expression) false else it.fir is FirReturnExpression } + } } + diff --git a/compiler/testData/diagnostics/tests/functionLiterals/return/LocalReturnNoCoercionToUnit.fir.kt b/compiler/testData/diagnostics/tests/functionLiterals/return/LocalReturnNoCoercionToUnit.fir.kt index bd241153105..ed8325c002e 100644 --- a/compiler/testData/diagnostics/tests/functionLiterals/return/LocalReturnNoCoercionToUnit.fir.kt +++ b/compiler/testData/diagnostics/tests/functionLiterals/return/LocalReturnNoCoercionToUnit.fir.kt @@ -1,6 +1,6 @@ fun test(a: Int) { run f@{ - if (a > 0) return@f + if (a > 0) return@f else return@f 1 } } diff --git a/compiler/testData/diagnostics/tests/functionLiterals/return/LocalReturnSecondUnit.fir.kt b/compiler/testData/diagnostics/tests/functionLiterals/return/LocalReturnSecondUnit.fir.kt deleted file mode 100644 index d1fb85056b4..00000000000 --- a/compiler/testData/diagnostics/tests/functionLiterals/return/LocalReturnSecondUnit.fir.kt +++ /dev/null @@ -1,6 +0,0 @@ -val flag = true - -val a = b@ { - if (flag) return@b 4 - return@b -} diff --git a/compiler/testData/diagnostics/tests/functionLiterals/return/LocalReturnSecondUnit.kt b/compiler/testData/diagnostics/tests/functionLiterals/return/LocalReturnSecondUnit.kt index 859d60ede90..11d8ab569cd 100644 --- a/compiler/testData/diagnostics/tests/functionLiterals/return/LocalReturnSecondUnit.kt +++ b/compiler/testData/diagnostics/tests/functionLiterals/return/LocalReturnSecondUnit.kt @@ -1,6 +1,7 @@ +// FIR_IDENTICAL val flag = true val a = b@ { if (flag) return@b 4 return@b -} +} \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/functionLiterals/return/LocalReturnUnitAndDontCareType.fir.kt b/compiler/testData/diagnostics/tests/functionLiterals/return/LocalReturnUnitAndDontCareType.fir.kt index 0dcc17c600a..47b2f719cb8 100644 --- a/compiler/testData/diagnostics/tests/functionLiterals/return/LocalReturnUnitAndDontCareType.fir.kt +++ b/compiler/testData/diagnostics/tests/functionLiterals/return/LocalReturnUnitAndDontCareType.fir.kt @@ -14,7 +14,7 @@ val b = run { // Unit val c = run { - if (flag) return@run + if (flag) return@run return@run 4 -} \ No newline at end of file +} diff --git a/compiler/testData/diagnostics/tests/functionLiterals/return/LocalReturnsWithExplicitReturnType.fir.kt b/compiler/testData/diagnostics/tests/functionLiterals/return/LocalReturnsWithExplicitReturnType.fir.kt index e5f864f2baa..959a6fdc54d 100644 --- a/compiler/testData/diagnostics/tests/functionLiterals/return/LocalReturnsWithExplicitReturnType.fir.kt +++ b/compiler/testData/diagnostics/tests/functionLiterals/return/LocalReturnsWithExplicitReturnType.fir.kt @@ -1,6 +1,6 @@ fun test(a: Int) { runf@{ - if (a > 0) return@f "" + if (a > 0) return@f "" return@f 1 } diff --git a/compiler/testData/diagnostics/tests/generics/projectionsScope/typeMismatchInLambda.fir.kt b/compiler/testData/diagnostics/tests/generics/projectionsScope/typeMismatchInLambda.fir.kt index c71c7c06a0f..ba6cde6ace1 100644 --- a/compiler/testData/diagnostics/tests/generics/projectionsScope/typeMismatchInLambda.fir.kt +++ b/compiler/testData/diagnostics/tests/generics/projectionsScope/typeMismatchInLambda.fir.kt @@ -20,7 +20,7 @@ fun test(a: A, z: Out) { a.bar { z.id() } a.foo { - z.foobar(if (1 > 2) return@foo "" else "") + z.foobar(if (1 > 2) return@foo "" else "") "" } } diff --git a/compiler/testData/diagnostics/tests/inference/nothingType/returnAsLastStatementInLambda.fir.kt b/compiler/testData/diagnostics/tests/inference/nothingType/returnAsLastStatementInLambda.fir.kt index 680fdb4122e..dd9b4bd9ba8 100644 --- a/compiler/testData/diagnostics/tests/inference/nothingType/returnAsLastStatementInLambda.fir.kt +++ b/compiler/testData/diagnostics/tests/inference/nothingType/returnAsLastStatementInLambda.fir.kt @@ -32,9 +32,9 @@ fun foo() { // Doesn't work both in K1 and K2, but probably should (KT-58232 for tracking) val dates3 = myRun { when { - else -> return@myRun buildList { + else -> return@myRun buildList { add(4) - } + } } } }