Fix type checking of local return inside return expression

 #KT-16426 Fixed

Author: zarechenskiy

compiler/frontend/src/org/jetbrains/kotlin/types/expressions/FunctionsTypingVisitor.kt

@@ -268,8 +268,8 @@ internal class FunctionsTypingVisitor(facade: ExpressionTypingInternals) : Expre
         val result = Lists.newArrayList<KtReturnExpression>()
         val bodyExpression = functionLiteral.bodyExpression
         bodyExpression?.accept(object : KtTreeVisitor<MutableList<KtReturnExpression>>() {
-            override fun visitReturnExpression(expression: KtReturnExpression, data: MutableList<KtReturnExpression>): Void? {
-                data.add(expression)
+            override fun visitReturnExpression(expression: KtReturnExpression, insideActualFunction: MutableList<KtReturnExpression>): Void? {
+                insideActualFunction.add(expression)
                 return null
             }
         }, result)
@@ -285,39 +285,49 @@ internal class FunctionsTypingVisitor(facade: ExpressionTypingInternals) : Expre
         if ((function !is KtNamedFunction || function.typeReference != null)
             && (function !is KtPropertyAccessor || function.returnTypeReference == null)) return
 
-        val bodyExpression = function.bodyExpression ?: return
+        for (returnForCheck in collectReturns(function, trace)) {
+            val expression = returnForCheck.returnedExpression
+            if (expression == null) {
+                if (!actualReturnType.isUnit()) {
+                    trace.report(Errors.RETURN_TYPE_MISMATCH.on(returnForCheck, actualReturnType))
+                }
+                continue
+            }
+
+            val expressionType = trace.getType(expression) ?: continue
+            if (!KotlinTypeChecker.DEFAULT.isSubtypeOf(expressionType, actualReturnType)) {
+                trace.report(Errors.TYPE_MISMATCH.on(expression, expressionType, actualReturnType))
+            }
+        }
+    }
+
+    private fun collectReturns(function: KtDeclarationWithBody, trace: BindingTrace): List<KtReturnExpression> {
+        val bodyExpression = function.bodyExpression ?: return emptyList()
         val returns = ArrayList<KtReturnExpression>()
 
-        // data == false means, that we inside other function, so ours return should be with label
         bodyExpression.accept(object : KtTreeVisitor<Boolean>() {
-            override fun visitReturnExpression(expression: KtReturnExpression, data: Boolean): Void? {
-                val label = expression.getTargetLabel()
-                if ((label != null && trace[BindingContext.LABEL_TARGET, label] == function)
-                    || (label == null && data)
-                ) {
+            override fun visitReturnExpression(expression: KtReturnExpression, insideActualFunction: Boolean): Void? {
+                val labelTarget = expression.getTargetLabel()?.let { trace[BindingContext.LABEL_TARGET, it] }
+                if (labelTarget == function || (labelTarget == null && insideActualFunction)) {
                     returns.add(expression)
                 }
-                return super.visitReturnExpression(expression, data)
+
+                return super.visitReturnExpression(expression, insideActualFunction)
             }
 
             override fun visitNamedFunction(function: KtNamedFunction, data: Boolean): Void? {
                 return super.visitNamedFunction(function, false)
             }
-        }, true)
 
-        for (returnForCheck in returns) {
-            val expression = returnForCheck.returnedExpression
-            if (expression == null) {
-                if (!actualReturnType.isUnit()) {
-                    trace.report(Errors.RETURN_TYPE_MISMATCH.on(returnForCheck, actualReturnType))
-                }
-                continue
+            override fun visitPropertyAccessor(accessor: KtPropertyAccessor, data: Boolean): Void? {
+                return super.visitPropertyAccessor(accessor, false)
             }
 
-            val expressionType = trace.getType(expression) ?: continue
-            if (!KotlinTypeChecker.DEFAULT.isSubtypeOf(expressionType, actualReturnType)) {
-                trace.report(Errors.TYPE_MISMATCH.on(expression, expressionType, actualReturnType))
+            override fun visitAnonymousInitializer(initializer: KtAnonymousInitializer, data: Boolean): Void? {
+                return super.visitAnonymousInitializer(initializer, false)
             }
-        }
+        }, true)
+
+        return returns
     }
 }

compiler/testData/codegen/box/nonLocalReturns/localReturnInsideProperty.kt

@@ -0,0 +1,13 @@
+interface ClassData
+
+fun f() = object : ClassData {
+    val someInt: Int
+        get() {
+            return 5
+        }
+}
+
+fun box(): String{
+    f()
+    return "OK"
+}

compiler/testData/diagnostics/tests/controlStructures/localReturnInsidePropertyAccessor.kt

@@ -0,0 +1,20 @@
+interface ClassData
+
+fun f() = object : ClassData {
+    val someInt: Int
+        get() {
+            return 5
+        }
+}
+
+fun g() = object : ClassData {
+    init {
+        if (true) {
+            <!RETURN_NOT_ALLOWED, RETURN_IN_FUNCTION_WITH_EXPRESSION_BODY!>return<!> 0
+        }
+    }
+
+    fun some(): Int {
+        return 6
+    }
+}

compiler/testData/diagnostics/tests/controlStructures/localReturnInsidePropertyAccessor.txt

@@ -0,0 +1,10 @@
+package
+
+public fun f(): ClassData
+public fun g(): ClassData
+
+public interface ClassData {
+    public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
+    public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
+    public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
+}

compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java

@@ -11093,6 +11093,12 @@ public void testKt9644let() throws Exception {
             doTest(fileName);
         }
 
+        @TestMetadata("localReturnInsideProperty.kt")
+        public void testLocalReturnInsideProperty() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/nonLocalReturns/localReturnInsideProperty.kt");
+            doTest(fileName);
+        }
+
         @TestMetadata("use.kt")
         public void testUse() throws Exception {
             String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/nonLocalReturns/use.kt");

compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java

@@ -4534,6 +4534,12 @@ public void testLambdasInExclExclAndElvis() throws Exception {
                 doTest(fileName);
             }
 
+            @TestMetadata("localReturnInsidePropertyAccessor.kt")
+            public void testLocalReturnInsidePropertyAccessor() throws Exception {
+                String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/controlStructures/localReturnInsidePropertyAccessor.kt");
+                doTest(fileName);
+            }
+
             @TestMetadata("specialConstructsAndPlatformTypes.kt")
             public void testSpecialConstructsAndPlatformTypes() throws Exception {
                 String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/controlStructures/specialConstructsAndPlatformTypes.kt");

compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java

@@ -11093,6 +11093,12 @@ public void testKt9644let() throws Exception {
             doTest(fileName);
         }
 
+        @TestMetadata("localReturnInsideProperty.kt")
+        public void testLocalReturnInsideProperty() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/nonLocalReturns/localReturnInsideProperty.kt");
+            doTest(fileName);
+        }
+
         @TestMetadata("use.kt")
         public void testUse() throws Exception {
             String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/nonLocalReturns/use.kt");

compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java

@@ -11093,6 +11093,12 @@ public void testKt9644let() throws Exception {
             doTest(fileName);
         }
 
+        @TestMetadata("localReturnInsideProperty.kt")
+        public void testLocalReturnInsideProperty() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/nonLocalReturns/localReturnInsideProperty.kt");
+            doTest(fileName);
+        }
+
         @TestMetadata("use.kt")
         public void testUse() throws Exception {
             String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/nonLocalReturns/use.kt");

js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java

@@ -12505,6 +12505,12 @@ public void testKt9644let() throws Exception {
             doTest(fileName);
         }
 
+        @TestMetadata("localReturnInsideProperty.kt")
+        public void testLocalReturnInsideProperty() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/nonLocalReturns/localReturnInsideProperty.kt");
+            doTest(fileName);
+        }
+
         @TestMetadata("use.kt")
         public void testUse() throws Exception {
             String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/nonLocalReturns/use.kt");