From 8a0057b387edfc401306dd1bb6fb3557c6ab4f6e Mon Sep 17 00:00:00 2001 From: Dmitry Savvinov Date: Wed, 9 Jan 2019 15:16:08 +0300 Subject: [PATCH] Introduce specific version of NO_RETURN_IN_FUNCTION_WITH_BLOCK_BODY for KT-28061 In 1.3.0 there was introduced KT-28061 bug, which caused some of the NO_RETURN_IN_FUNCTION_WITH_BLOCK_BODY diagnostics to be lost (turning "red" code into "green") 500dc11514939d19b7c86283f878dcdfe78bc7da fixes the bug and returns lost diagnostics back. This commit adds additional information for some of the cases where diagnostics were list and then brought back (for the user sanity). The heuristic used for detecting cases which need additional information is simple: if function has return in in-place called lambda, and NO_RETURN_IN_FUNCTION_WITH_BLOCK_BODY should be reported, then we enhance it with additional information. ^KT-28061 Fixed --- .../cfg/ControlFlowInformationProvider.kt | 33 ++++++++++++++++--- .../jetbrains/kotlin/diagnostics/Errors.java | 2 ++ .../rendering/DefaultErrorMessages.java | 3 ++ .../flowInlining/safeCallAndInPlaceReturn.kt | 12 +++++-- .../flowInlining/safeCallAndInPlaceReturn.txt | 5 +-- .../kotlin/idea/quickfix/QuickFixRegistrar.kt | 1 + 6 files changed, 46 insertions(+), 10 deletions(-) diff --git a/compiler/frontend/src/org/jetbrains/kotlin/cfg/ControlFlowInformationProvider.kt b/compiler/frontend/src/org/jetbrains/kotlin/cfg/ControlFlowInformationProvider.kt index ee3e0823daf..b7bc7d526ae 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/cfg/ControlFlowInformationProvider.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/cfg/ControlFlowInformationProvider.kt @@ -11,6 +11,7 @@ import org.jetbrains.kotlin.cfg.TailRecursionKind.* import org.jetbrains.kotlin.cfg.VariableUseState.* import org.jetbrains.kotlin.cfg.pseudocode.Pseudocode import org.jetbrains.kotlin.cfg.pseudocode.PseudocodeUtil +import org.jetbrains.kotlin.cfg.pseudocode.containingDeclarationForPseudocode import org.jetbrains.kotlin.cfg.pseudocode.instructions.Instruction import org.jetbrains.kotlin.cfg.pseudocode.instructions.InstructionVisitor import org.jetbrains.kotlin.cfg.pseudocode.instructions.KtElementInstruction @@ -30,6 +31,7 @@ import org.jetbrains.kotlin.diagnostics.Errors.* import org.jetbrains.kotlin.idea.MainFunctionDetector import org.jetbrains.kotlin.lexer.KtTokens import org.jetbrains.kotlin.psi.* +import org.jetbrains.kotlin.psi.psiUtil.getStrictParentOfType import org.jetbrains.kotlin.resolve.* import org.jetbrains.kotlin.resolve.BindingContext.* import org.jetbrains.kotlin.resolve.bindingContextUtil.isUsedAsExpression @@ -141,21 +143,33 @@ class ControlFlowInformationProvider private constructor( * // isn't actually returned * } */ - private fun collectReturnExpressions(returnedExpressions: MutableCollection) { + private fun collectReturnExpressions(): ReturnedExpressionsInfo { val instructions = pseudocode.instructions.toHashSet() val exitInstruction = pseudocode.exitInstruction + + val returnedExpressions = arrayListOf() + var hasReturnsInInlinedLambda = false + for (previousInstruction in exitInstruction.previousInstructions) { previousInstruction.accept(object : InstructionVisitor() { override fun visitReturnValue(instruction: ReturnValueInstruction) { if (instructions.contains(instruction)) { //exclude non-local return expressions returnedExpressions.add(instruction.element) } + + if (instruction.owner.isInlined) { + hasReturnsInInlinedLambda = true + } } override fun visitReturnNoValue(instruction: ReturnNoValueInstruction) { if (instructions.contains(instruction)) { returnedExpressions.add(instruction.element) } + + if (instruction.owner.isInlined) { + hasReturnsInInlinedLambda = true + } } override fun visitUnconditionalJump(instruction: UnconditionalJumpInstruction) { @@ -195,6 +209,8 @@ class ControlFlowInformationProvider private constructor( } }) } + + return ReturnedExpressionsInfo(returnedExpressions, hasReturnsInInlinedLambda) } private fun checkLocalFunctions() { @@ -220,8 +236,7 @@ class ControlFlowInformationProvider private constructor( if (!function.hasBody()) return - val returnedExpressions = arrayListOf() - collectReturnExpressions(returnedExpressions) + val (returnedExpressions, hasReturnsInInlinedLambdas) = collectReturnExpressions() val blockBody = function.hasBlockBody() @@ -239,17 +254,25 @@ class ControlFlowInformationProvider private constructor( if (blockBody && !noExpectedType(expectedReturnType) && !KotlinBuiltIns.isUnit(expectedReturnType) - && !unreachableCode.elements.contains(element)) { + && !unreachableCode.elements.contains(element) + ) { noReturnError = true } } }) } + if (noReturnError) { - trace.report(NO_RETURN_IN_FUNCTION_WITH_BLOCK_BODY.on(function)) + if (hasReturnsInInlinedLambdas) { + trace.report(NO_RETURN_IN_FUNCTION_WITH_BLOCK_BODY_MIGRATION.on(function)) + } else { + trace.report(NO_RETURN_IN_FUNCTION_WITH_BLOCK_BODY.on(function)) + } } } + private data class ReturnedExpressionsInfo(val returnedExpressions: Collection, val hasReturnsInInlinedLambda: Boolean) + private fun reportUnreachableCode(unreachableCode: UnreachableCode) { for (element in unreachableCode.elements) { trace.report(Errors.UNREACHABLE_CODE.on(element, unreachableCode.getUnreachableTextRanges(element))) diff --git a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java index 3da1965289a..14736e8073f 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java @@ -1018,6 +1018,8 @@ public interface Errors { DiagnosticFactory0 RETURN_IN_FUNCTION_WITH_EXPRESSION_BODY = DiagnosticFactory0.create(ERROR, PositioningStrategies.RETURN_WITH_LABEL); DiagnosticFactory0 NO_RETURN_IN_FUNCTION_WITH_BLOCK_BODY = DiagnosticFactory0.create(ERROR, DECLARATION_WITH_BODY); + DiagnosticFactory0 + NO_RETURN_IN_FUNCTION_WITH_BLOCK_BODY_MIGRATION = DiagnosticFactory0.create(ERROR, DECLARATION_WITH_BODY); DiagnosticFactory0 ANONYMOUS_INITIALIZER_IN_INTERFACE = DiagnosticFactory0.create(ERROR, DECLARATION_SIGNATURE); diff --git a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java index 8d981838323..f18392afa6c 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java @@ -499,6 +499,9 @@ public class DefaultErrorMessages { MAP.put(RETURN_IN_FUNCTION_WITH_EXPRESSION_BODY, "Returns are not allowed for functions with expression body. Use block body in '{...}'"); MAP.put(NO_RETURN_IN_FUNCTION_WITH_BLOCK_BODY, "A 'return' expression required in a function with a block body ('{...}')"); + MAP.put(NO_RETURN_IN_FUNCTION_WITH_BLOCK_BODY_MIGRATION, "A 'return' expression required in a function with a block body ('{...}'). " + + "If you got this error after the compiler update, then it's most likely due to a fix of a bug " + + "introduced in 1.3.0 (see KT-28061 for details)"); MAP.put(RETURN_TYPE_MISMATCH, "This function must return a value of type {0}", RENDER_TYPE); MAP.put(EXPECTED_TYPE_MISMATCH, "Expected a value of type {0}", RENDER_TYPE); MAP.put(ASSIGNMENT_TYPE_MISMATCH, diff --git a/compiler/testData/diagnostics/testsWithStdLib/contracts/controlflow/flowInlining/safeCallAndInPlaceReturn.kt b/compiler/testData/diagnostics/testsWithStdLib/contracts/controlflow/flowInlining/safeCallAndInPlaceReturn.kt index 08292b09281..9e6bc986bd2 100644 --- a/compiler/testData/diagnostics/testsWithStdLib/contracts/controlflow/flowInlining/safeCallAndInPlaceReturn.kt +++ b/compiler/testData/diagnostics/testsWithStdLib/contracts/controlflow/flowInlining/safeCallAndInPlaceReturn.kt @@ -3,15 +3,21 @@ import kotlin.contracts.* -inline fun Any?.myRun(block: () -> Unit): Unit { +inline fun Any?.myRun(block: () -> T): T { contract { callsInPlace(block, InvocationKind.EXACTLY_ONCE) } return block() } -fun test(): String { +fun bad(): String { val x: String? = null x?.myRun { return "" } -} \ No newline at end of file +} + +fun ok(): String { + val x: String? = null + + x?.run { return "non-null" } ?: return "null" +} \ No newline at end of file diff --git a/compiler/testData/diagnostics/testsWithStdLib/contracts/controlflow/flowInlining/safeCallAndInPlaceReturn.txt b/compiler/testData/diagnostics/testsWithStdLib/contracts/controlflow/flowInlining/safeCallAndInPlaceReturn.txt index 7efe90276dd..15229211dae 100644 --- a/compiler/testData/diagnostics/testsWithStdLib/contracts/controlflow/flowInlining/safeCallAndInPlaceReturn.txt +++ b/compiler/testData/diagnostics/testsWithStdLib/contracts/controlflow/flowInlining/safeCallAndInPlaceReturn.txt @@ -1,6 +1,7 @@ package -public fun test(): kotlin.String -public inline fun kotlin.Any?.myRun(/*0*/ block: () -> kotlin.Unit): kotlin.Unit +public fun bad(): kotlin.String +public fun ok(): kotlin.String +public inline fun kotlin.Any?.myRun(/*0*/ block: () -> T): T CallsInPlace(block, EXACTLY_ONCE) diff --git a/idea/src/org/jetbrains/kotlin/idea/quickfix/QuickFixRegistrar.kt b/idea/src/org/jetbrains/kotlin/idea/quickfix/QuickFixRegistrar.kt index da4ba0133e0..9dcba9aa2c3 100644 --- a/idea/src/org/jetbrains/kotlin/idea/quickfix/QuickFixRegistrar.kt +++ b/idea/src/org/jetbrains/kotlin/idea/quickfix/QuickFixRegistrar.kt @@ -303,6 +303,7 @@ class QuickFixRegistrar : QuickFixContributor { val changeFunctionReturnTypeFix = ChangeCallableReturnTypeFix.ChangingReturnTypeToUnitFactory RETURN_TYPE_MISMATCH.registerFactory(changeFunctionReturnTypeFix) NO_RETURN_IN_FUNCTION_WITH_BLOCK_BODY.registerFactory(changeFunctionReturnTypeFix) + NO_RETURN_IN_FUNCTION_WITH_BLOCK_BODY_MIGRATION.registerFactory(changeFunctionReturnTypeFix) RETURN_TYPE_MISMATCH_ON_OVERRIDE.registerFactory(ChangeCallableReturnTypeFix.ReturnTypeMismatchOnOverrideFactory) COMPONENT_FUNCTION_RETURN_TYPE_MISMATCH.registerFactory(ChangeCallableReturnTypeFix.ComponentFunctionReturnTypeMismatchFactory) HAS_NEXT_FUNCTION_TYPE_MISMATCH.registerFactory(ChangeCallableReturnTypeFix.HasNextFunctionTypeMismatchFactory)