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")

500dc11514 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

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<KtElement>) {
+    private fun collectReturnExpressions(): ReturnedExpressionsInfo {
         val instructions = pseudocode.instructions.toHashSet()
         val exitInstruction = pseudocode.exitInstruction
+
+        val returnedExpressions = arrayListOf<KtElement>()
+        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<KtElement>()
-        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<KtElement>, val hasReturnsInInlinedLambda: Boolean)
+
     private fun reportUnreachableCode(unreachableCode: UnreachableCode) {
         for (element in unreachableCode.elements) {
             trace.report(Errors.UNREACHABLE_CODE.on(element, unreachableCode.getUnreachableTextRanges(element)))

compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java

@@ -1018,6 +1018,8 @@ public interface Errors {
     DiagnosticFactory0<KtReturnExpression> RETURN_IN_FUNCTION_WITH_EXPRESSION_BODY = DiagnosticFactory0.create(ERROR, PositioningStrategies.RETURN_WITH_LABEL);
     DiagnosticFactory0<KtDeclarationWithBody>
             NO_RETURN_IN_FUNCTION_WITH_BLOCK_BODY = DiagnosticFactory0.create(ERROR, DECLARATION_WITH_BODY);
+    DiagnosticFactory0<KtDeclarationWithBody>
+            NO_RETURN_IN_FUNCTION_WITH_BLOCK_BODY_MIGRATION = DiagnosticFactory0.create(ERROR, DECLARATION_WITH_BODY);
 
     DiagnosticFactory0<KtAnonymousInitializer> ANONYMOUS_INITIALIZER_IN_INTERFACE = DiagnosticFactory0.create(ERROR, DECLARATION_SIGNATURE);
 

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,

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 <T> 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_RETURN_IN_FUNCTION_WITH_BLOCK_BODY!>}<!>
+<!NO_RETURN_IN_FUNCTION_WITH_BLOCK_BODY_MIGRATION!>}<!>
+
+fun ok(): String {
+    val x: String? = null
+
+    x?.run { return "non-null" } ?: return "null"
+}

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 </*0*/ T> kotlin.Any?.myRun(/*0*/ block: () -> T): T
     CallsInPlace(block, EXACTLY_ONCE)
 

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)