KT-456 No check for obligatory return in getters

compiler/frontend/src/org/jetbrains/jet/lang/cfg/JetControlFlowProcessor.java

@@ -548,7 +548,7 @@ public class JetControlFlowProcessor {
             if (subroutine instanceof JetFunctionLiteralExpression) {
                 subroutine = ((JetFunctionLiteralExpression) subroutine).getFunctionLiteral();
             }
-            if (subroutine instanceof JetFunction) {
+            if (subroutine instanceof JetFunction || subroutine instanceof JetPropertyAccessor || subroutine instanceof JetConstructor) {
                 if (returnedExpression == null) {
                     builder.returnNoValue(expression, subroutine);
                 }
@@ -560,6 +560,9 @@ public class JetControlFlowProcessor {
                 if (labelElement != null) {
                     trace.report(NOT_A_RETURN_LABEL.on(expression, expression.getLabelName()));
                 }
+                else {
+                    trace.report(RETURN_NOT_ALLOWED.on(expression));
+                }
             }
         }
 

compiler/frontend/src/org/jetbrains/jet/lang/cfg/JetFlowInformationProvider.java

@@ -16,10 +16,14 @@ import org.jetbrains.jet.lang.resolve.BindingContext;
 import org.jetbrains.jet.lang.resolve.BindingContextUtils;
 import org.jetbrains.jet.lang.resolve.BindingTrace;
 import org.jetbrains.jet.lang.types.JetStandardClasses;
+import org.jetbrains.jet.lang.types.JetType;
 import org.jetbrains.jet.lexer.JetTokens;
 
 import java.util.*;
 
+import static org.jetbrains.jet.lang.diagnostics.Errors.*;
+import static org.jetbrains.jet.lang.types.TypeUtils.NO_EXPECTED_TYPE;
+
 /**
  * @author svtk
  */
@@ -67,7 +71,7 @@ public class JetFlowInformationProvider {
         wrappedTrace.close();
     }
 
-    public void collectReturnExpressions(@NotNull JetElement subroutine, @NotNull final Collection<JetExpression> returnedExpressions) {
+    private void collectReturnExpressions(@NotNull JetElement subroutine, @NotNull final Collection<JetExpression> returnedExpressions) {
         Pseudocode pseudocode = pseudocodeMap.get(subroutine);
         assert pseudocode != null;
 
@@ -120,19 +124,64 @@ public class JetFlowInformationProvider {
         }
     }
     
-    public void collectUnreachableExpressions(@NotNull JetElement subroutine, @NotNull Collection<JetElement> unreachableElements) {
+    public void checkDefiniteReturn(@NotNull JetDeclarationWithBody function, final @NotNull JetType expectedReturnType) {
+        assert function instanceof JetDeclaration;
+
+        JetExpression bodyExpression = function.getBodyExpression();
+        if (bodyExpression == null) return;
+        
+        List<JetExpression> returnedExpressions = Lists.newArrayList();
+        collectReturnExpressions(function.asElement(), returnedExpressions);
+
+        boolean nothingReturned = returnedExpressions.isEmpty();
+
+        returnedExpressions.remove(function); // This will be the only "expression" if the body is empty
+
+        if (expectedReturnType != NO_EXPECTED_TYPE && !JetStandardClasses.isUnit(expectedReturnType) && returnedExpressions.isEmpty() && !nothingReturned) {
+            trace.report(RETURN_TYPE_MISMATCH.on(bodyExpression, expectedReturnType));
+        }
+        final boolean blockBody = function.hasBlockBody();
+        
+        final Set<JetElement> rootUnreachableElements = collectUnreachableCode(function.asElement());
+        for (JetElement element : rootUnreachableElements) {
+            trace.report(UNREACHABLE_CODE.on(element));
+        }
+
+        for (JetExpression returnedExpression : returnedExpressions) {
+            returnedExpression.accept(new JetVisitorVoid() {
+                @Override
+                public void visitReturnExpression(JetReturnExpression expression) {
+                    if (!blockBody) {
+                        trace.report(RETURN_IN_FUNCTION_WITH_EXPRESSION_BODY.on(expression));
+                    }
+                }
+
+                @Override
+                public void visitExpression(JetExpression expression) {
+                    if (blockBody && expectedReturnType != NO_EXPECTED_TYPE && !JetStandardClasses.isUnit(expectedReturnType) && !rootUnreachableElements.contains(expression)) {
+                        trace.report(NO_RETURN_IN_FUNCTION_WITH_BLOCK_BODY.on(expression));
+                    }
+                }
+            });
+        }
+    }
+
+    private Set<JetElement> collectUnreachableCode(@NotNull JetElement subroutine) {
         Pseudocode pseudocode = pseudocodeMap.get(subroutine);
         assert pseudocode != null;
 
+        Collection<JetElement> unreachableElements = Lists.newArrayList();
         for (Instruction deadInstruction : pseudocode.getDeadInstructions()) {
             if (deadInstruction instanceof JetElementInstruction &&
                 !(deadInstruction instanceof ReadUnitValueInstruction)) {
                 unreachableElements.add(((JetElementInstruction) deadInstruction).getElement());
             }
         }
+        // This is needed in order to highlight only '1 < 2' and not '1', '<' and '2' as well
+        return JetPsiUtil.findRootExpressions(unreachableElements);
     }
 
-    public void markUninitializedVariables(@NotNull JetElement subroutine, final boolean inAnonymousInitializers, final boolean analyzeLocalDeclaration) {
+    public void markUninitializedVariables(@NotNull JetElement subroutine, final boolean inAnonymousInitializers, final boolean inLocalDeclaration) {
         final Pseudocode pseudocode = pseudocodeMap.get(subroutine);
         assert pseudocode != null;
 
@@ -202,7 +251,7 @@ public class JetFlowInformationProvider {
                                 isInitialized = true;
                             }
                         }
-                        if (!analyzeLocalDeclaration && !isInitialized && !varWithUninitializedErrorGenerated.contains(variableDescriptor)) {
+                        if (!inLocalDeclaration && !isInitialized && !varWithUninitializedErrorGenerated.contains(variableDescriptor)) {
                             varWithUninitializedErrorGenerated.add(variableDescriptor);
                             trace.report(Errors.UNINITIALIZED_VARIABLE.on((JetSimpleNameExpression) element, variableDescriptor));
                         }
@@ -225,7 +274,7 @@ public class JetFlowInformationProvider {
                             }
                         }
                         JetExpression expression = (JetExpression) element;
-                        if (!analyzeLocalDeclaration && hasInitializer && !variableDescriptor.isVar() && !varWithValReassignErrorGenerated.contains(variableDescriptor)) {
+                        if (!inLocalDeclaration && hasInitializer && !variableDescriptor.isVar() && !varWithValReassignErrorGenerated.contains(variableDescriptor)) {
                             PsiElement psiElement = trace.get(BindingContext.DESCRIPTOR_TO_DECLARATION, variableDescriptor);
                             JetProperty property = psiElement instanceof JetProperty ? (JetProperty) psiElement : null;
                             varWithValReassignErrorGenerated.add(variableDescriptor);
@@ -254,7 +303,7 @@ public class JetFlowInformationProvider {
         for (Instruction instruction : pseudocode.getInstructions()) {
             if (instruction instanceof LocalDeclarationInstruction) {
                 JetElement element = ((LocalDeclarationInstruction) instruction).getElement();
-                markUninitializedVariables(element, false, analyzeLocalDeclaration);
+                markUninitializedVariables(element, false, inLocalDeclaration);
             }
         }
     }

compiler/frontend/src/org/jetbrains/jet/lang/cfg/pseudocode/JetControlFlowInstructionsGenerator.java

@@ -45,12 +45,7 @@ public class JetControlFlowInstructionsGenerator extends JetControlFlowBuilderAd
 
     @Override
     public void enterSubroutine(@NotNull JetDeclaration subroutine) {
-        if (builder != null) {
+        pushBuilder(subroutine, subroutine);
-            pushBuilder(subroutine, builder.getCurrentSubroutine());
-        }
-        else {
-            pushBuilder(subroutine, subroutine);
-        }
         assert builder != null;
         builder.enterSubroutine(subroutine);
     }

compiler/frontend/src/org/jetbrains/jet/lang/resolve/BodyResolver.java

@@ -281,11 +281,11 @@ public class BodyResolver {
     }
 
     private void resolveSecondaryConstructorBodies() {
-        for (Map.Entry<JetDeclaration, ConstructorDescriptor> entry : this.context.getConstructors().entrySet()) {
+        for (Map.Entry<JetConstructor, ConstructorDescriptor> entry : this.context.getConstructors().entrySet()) {
-            JetDeclaration declaration = entry.getKey();
+            JetConstructor constructor = entry.getKey();
             ConstructorDescriptor descriptor = entry.getValue();
 
-            resolveSecondaryConstructorBody((JetConstructor) declaration, descriptor, ((MutableClassDescriptor) descriptor.getContainingDeclaration()).getScopeForMemberResolution());
+            resolveSecondaryConstructorBody(constructor, descriptor, ((MutableClassDescriptor) descriptor.getContainingDeclaration()).getScopeForMemberResolution());
 
             assert descriptor.getReturnType() != null;
         }

compiler/frontend/src/org/jetbrains/jet/lang/resolve/ControlFlowAnalyzer.java

@@ -1,22 +1,16 @@
 package org.jetbrains.jet.lang.resolve;
 
-import com.google.common.collect.Lists;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.jet.lang.cfg.JetFlowInformationProvider;
 import org.jetbrains.jet.lang.cfg.pseudocode.JetControlFlowDataTraceFactory;
 import org.jetbrains.jet.lang.descriptors.*;
 import org.jetbrains.jet.lang.psi.*;
-import org.jetbrains.jet.lang.resolve.scopes.JetScope;
 import org.jetbrains.jet.lang.types.JetStandardClasses;
 import org.jetbrains.jet.lang.types.JetType;
-import org.jetbrains.jet.lang.types.expressions.ExpressionTypingServices;
-import org.jetbrains.jet.lexer.JetTokens;
 
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
 
-import static org.jetbrains.jet.lang.diagnostics.Errors.*;
 import static org.jetbrains.jet.lang.types.TypeUtils.NO_EXPECTED_TYPE;
 
 /**
@@ -24,130 +18,70 @@ import static org.jetbrains.jet.lang.types.TypeUtils.NO_EXPECTED_TYPE;
  */
 public class ControlFlowAnalyzer {
     private TopDownAnalysisContext context;
-    private ExpressionTypingServices typeInferrerServices;
     private final JetControlFlowDataTraceFactory flowDataTraceFactory;
-    private final boolean analyzeLocalDeclaration;
+    private final boolean inLocalDeclaration;
 
-    public ControlFlowAnalyzer(TopDownAnalysisContext context, JetControlFlowDataTraceFactory flowDataTraceFactory, boolean analyzeLocalDeclaration) {
+    public ControlFlowAnalyzer(TopDownAnalysisContext context, JetControlFlowDataTraceFactory flowDataTraceFactory, boolean inLocalDeclaration) {
         this.context = context;
         this.flowDataTraceFactory = flowDataTraceFactory;
-        this.analyzeLocalDeclaration = analyzeLocalDeclaration;
+        this.inLocalDeclaration = inLocalDeclaration;
-        this.typeInferrerServices = context.getSemanticServices().getTypeInferrerServices(context.getTrace());
     }
 
     public void process() {
-        for (Map.Entry<JetClass, MutableClassDescriptor> entry : context.getClasses().entrySet()) {
+        for (JetClass aClass : context.getClasses().keySet()) {
-            JetClass aClass = entry.getKey();
-            MutableClassDescriptor classDescriptor = entry.getValue();
-
             if (!context.completeAnalysisNeeded(aClass)) continue;
-
+            checkClassOrObject(aClass);
-            checkClassOrObject(aClass, classDescriptor);
         }
-
+        for (JetObjectDeclaration objectDeclaration : context.getObjects().keySet()) {
-        for (Map.Entry<JetObjectDeclaration, MutableClassDescriptor> entry : context.getObjects().entrySet()) {
-            JetObjectDeclaration objectDeclaration = entry.getKey();
-            MutableClassDescriptor objectDescriptor = entry.getValue();
-
             if (!context.completeAnalysisNeeded(objectDeclaration)) continue;
-
+            checkClassOrObject(objectDeclaration);
-            checkClassOrObject(objectDeclaration, objectDescriptor);
         }
-
         for (Map.Entry<JetNamedFunction, FunctionDescriptorImpl> entry : context.getFunctions().entrySet()) {
             JetNamedFunction function = entry.getKey();
             FunctionDescriptorImpl functionDescriptor = entry.getValue();
-
             if (!context.completeAnalysisNeeded(function)) continue;
-
             final JetType expectedReturnType = !function.hasBlockBody() && !function.hasDeclaredReturnType()
                                                ? NO_EXPECTED_TYPE
                                                : functionDescriptor.getReturnType();
-            checkFunction(function, functionDescriptor, expectedReturnType);
+            checkFunction(function, expectedReturnType);
         }
-
+        for (JetConstructor constructor : this.context.getConstructors().keySet()) {
-        for (Map.Entry<JetDeclaration, ConstructorDescriptor> entry : this.context.getConstructors().entrySet()) {
-            JetDeclaration declaration = entry.getKey();
-            assert declaration instanceof JetConstructor;
-            JetConstructor constructor = (JetConstructor) declaration;
-            ConstructorDescriptor descriptor = entry.getValue();
-
             if (!context.completeAnalysisNeeded(constructor)) continue;
-            checkFunction(constructor, descriptor, JetStandardClasses.getUnitType());
+            checkFunction(constructor, JetStandardClasses.getUnitType());
-        }
-
-        for (JetProperty property : context.getProperties().keySet()) {
-            if (!context.completeAnalysisNeeded(property)) continue;
-            checkProperty(property);
         }
     }
     
-    private void checkClassOrObject(JetClassOrObject klass, MutableClassDescriptor classDescriptor) {
+    private void checkClassOrObject(JetClassOrObject klass) {
         JetFlowInformationProvider flowInformationProvider = new JetFlowInformationProvider((JetDeclaration) klass, (JetExpression) klass, flowDataTraceFactory, context.getTrace());
-        flowInformationProvider.markUninitializedVariables((JetElement) klass, true, analyzeLocalDeclaration);
+        flowInformationProvider.markUninitializedVariables((JetElement) klass, true, inLocalDeclaration);
+
+        List<JetDeclaration> declarations = klass.getDeclarations();
+        for (JetDeclaration declaration : declarations) {
+            if (declaration instanceof JetProperty) {
+                JetProperty property = (JetProperty) declaration;
+                DeclarationDescriptor descriptor = context.getTrace().get(BindingContext.DECLARATION_TO_DESCRIPTOR, property);
+                assert descriptor instanceof PropertyDescriptor;
+                PropertyDescriptor propertyDescriptor = (PropertyDescriptor) descriptor;
+                for (JetPropertyAccessor accessor : property.getAccessors()) {
+                    PropertyAccessorDescriptor accessorDescriptor = accessor.isGetter()
+                                                                    ? propertyDescriptor.getGetter()
+                                                                    : propertyDescriptor.getSetter();
+                    assert accessorDescriptor != null;
+                    flowInformationProvider.checkDefiniteReturn(accessor, accessorDescriptor.getReturnType());
+                }
+            }
+        }
     }
 
-    private void checkFunction(JetDeclarationWithBody function, FunctionDescriptor functionDescriptor, final @NotNull JetType expectedReturnType) {
+    private void checkFunction(JetDeclarationWithBody function, final @NotNull JetType expectedReturnType) {
         assert function instanceof JetDeclaration;
 
         JetExpression bodyExpression = function.getBodyExpression();
         if (bodyExpression == null) return;
         JetFlowInformationProvider flowInformationProvider = new JetFlowInformationProvider((JetDeclaration) function, bodyExpression, flowDataTraceFactory, context.getTrace());
 
-        final boolean blockBody = function.hasBlockBody();
+        flowInformationProvider.checkDefiniteReturn(function, expectedReturnType);
-        List<JetElement> unreachableElements = Lists.newArrayList();
-        flowInformationProvider.collectUnreachableExpressions(function.asElement(), unreachableElements);
 
-        // This is needed in order to highlight only '1 < 2' and not '1', '<' and '2' as well
+        flowInformationProvider.markUninitializedVariables(function.asElement(), false, inLocalDeclaration);
-        final Set<JetElement> rootUnreachableElements = JetPsiUtil.findRootExpressions(unreachableElements);
-
-        for (JetElement element : rootUnreachableElements) {
-            context.getTrace().report(UNREACHABLE_CODE.on(element));
-        }
-
-        List<JetExpression> returnedExpressions = Lists.newArrayList();
-        flowInformationProvider.collectReturnExpressions(function.asElement(), returnedExpressions);
-
-        boolean nothingReturned = returnedExpressions.isEmpty();
-
-        returnedExpressions.remove(function); // This will be the only "expression" if the body is empty
-
-        final JetScope scope = this.context.getDeclaringScopes().get(function);
-
-        if (expectedReturnType != NO_EXPECTED_TYPE && !JetStandardClasses.isUnit(expectedReturnType) && returnedExpressions.isEmpty() && !nothingReturned) {
-            context.getTrace().report(RETURN_TYPE_MISMATCH.on(bodyExpression, expectedReturnType));
-        }
-
-        for (JetExpression returnedExpression : returnedExpressions) {
-            returnedExpression.accept(new JetVisitorVoid() {
-                @Override
-                public void visitReturnExpression(JetReturnExpression expression) {
-                    if (!blockBody) {
-                        context.getTrace().report(RETURN_IN_FUNCTION_WITH_EXPRESSION_BODY.on(expression));
-                    }
-                }
-
-                @Override
-                public void visitExpression(JetExpression expression) {
-                    if (blockBody && expectedReturnType != NO_EXPECTED_TYPE && !JetStandardClasses.isUnit(expectedReturnType) && !rootUnreachableElements.contains(expression)) {
-                        JetType type = typeInferrerServices.getType(scope, expression, expectedReturnType);
-                        if (type == null || !JetStandardClasses.isNothing(type)) {
-                            context.getTrace().report(NO_RETURN_IN_FUNCTION_WITH_BLOCK_BODY.on(expression));
-                        }
-                    }
-                }
-            });
-        }
-        flowInformationProvider.markUninitializedVariables(function.asElement(), false, analyzeLocalDeclaration);
-        if (((JetDeclaration) function).hasModifier(JetTokens.INLINE_KEYWORD)) {
-            //inline functions after M1
-//                flowInformationProvider.markNotOnlyInvokedFunctionVariables(function.asElement(), functionDescriptor.getValueParameters());
-        }
-    }
-
-    private void checkProperty(JetProperty property) {
-        JetExpression initializer = property.getInitializer();
-        if (initializer == null) return;
-        new JetFlowInformationProvider(property, initializer, flowDataTraceFactory, context.getTrace());
     }
 }

compiler/frontend/src/org/jetbrains/jet/lang/resolve/TopDownAnalysisContext.java

@@ -33,7 +33,7 @@ import java.util.Set;
     private final Map<JetDeclaration, JetScope> declaringScopes = Maps.newHashMap();
 
     private final Map<JetNamedFunction, FunctionDescriptorImpl> functions = Maps.newLinkedHashMap();
-    private final Map<JetDeclaration, ConstructorDescriptor> constructors = Maps.newLinkedHashMap();
+    private final Map<JetConstructor, ConstructorDescriptor> constructors = Maps.newLinkedHashMap();
     private final Map<JetProperty, PropertyDescriptor> properties = Maps.newLinkedHashMap();
     private final Set<PropertyDescriptor> primaryConstructorParameterProperties = Sets.newHashSet();
 
@@ -107,7 +107,7 @@ import java.util.Set;
         return primaryConstructorParameterProperties;
     }
 
-    public Map<JetDeclaration, ConstructorDescriptor> getConstructors() {
+    public Map<JetConstructor, ConstructorDescriptor> getConstructors() {
         return constructors;
     }
 

compiler/frontend/src/org/jetbrains/jet/lang/types/expressions/ClosureExpressionsTypingVisitor.java

@@ -143,7 +143,7 @@ public class ClosureExpressionsTypingVisitor extends ExpressionTypingVisitor {
             if (functionTypeExpected) {
                 returnType = JetStandardClasses.getReturnType(expectedType);
             }
-            returnType = context.getServices().getBlockReturnedType(functionInnerScope, functionLiteral.getBodyExpression(), CoercionStrategy.COERCION_TO_UNIT, context.replaceExpectedType(returnType));
+            returnType = context.getServices().getBlockReturnedType(functionInnerScope, functionLiteral.getBodyExpression(), CoercionStrategy.COERCION_TO_UNIT, context.replaceExpectedType(returnType).replaceExpectedReturnType(returnType));
         }
         JetType safeReturnType = returnType == null ? ErrorUtils.createErrorType("<return type>") : returnType;
         functionDescriptor.setReturnType(safeReturnType);

compiler/testData/cfg/AnonymousInitializers.instructions

@@ -55,16 +55,3 @@ error:
 sink:
     <SINK>     NEXT:[]          PREV:[<END>]
 =====================
-== k ==
-val k = 34
----------------------
-l0:
-    <START>    NEXT:[r(34)]     PREV:[]
-    r(34)      NEXT:[<END>]     PREV:[<START>]
-l1:
-    <END>      NEXT:[<SINK>]    PREV:[r(34)]
-error:
-    <ERROR>    NEXT:[]          PREV:[]
-sink:
-    <SINK>     NEXT:[]          PREV:[<END>]
-=====================

compiler/testData/cfg/LocalDeclarations.instructions

@@ -387,16 +387,3 @@ error:
 sink:
     <SINK>          NEXT:[]                PREV:[<END>]
 =====================
-== a ==
-val a: Int = 1
----------------------
-l0:
-    <START>    NEXT:[r(1)]      PREV:[]
-    r(1)       NEXT:[<END>]     PREV:[<START>]
-l1:
-    <END>      NEXT:[<SINK>]    PREV:[r(1)]
-error:
-    <ERROR>    NEXT:[]          PREV:[]
-sink:
-    <SINK>     NEXT:[]          PREV:[<END>]
-=====================

compiler/testData/checkerWithErrorTypes/full/FunctionReturnTypes.jet

@@ -5,7 +5,7 @@ fun unitEmpty() : Unit {}
 fun unitEmptyReturn() : Unit {return}
 fun unitIntReturn() : Unit {return <!TYPE_MISMATCH!>1<!>}
 fun unitUnitReturn() : Unit {return ()}
-fun test1() : Any = {<!RETURN_NOT_ALLOWED, RETURN_IN_FUNCTION_WITH_EXPRESSION_BODY!>return<!>}
+fun test1() : Any = {return}
 fun test2() : Any = @a {return@a 1}
 fun test3() : Any { <!RETURN_TYPE_MISMATCH!>return<!> }
 

compiler/testData/checkerWithErrorTypes/quick/regressions/kt456.jet

@@ -0,0 +1,30 @@
+//KT-456 No check for obligatory return in getters
+
+namespace kt456
+
+class A() {
+    val i: Int
+    get() : Int <!NO_RETURN_IN_FUNCTION_WITH_BLOCK_BODY!>{  //no error
+    }<!>
+}
+
+//more tests
+class B() {
+    val i: Int
+    get() <!NO_RETURN_IN_FUNCTION_WITH_BLOCK_BODY!>{  //no error
+    }<!>
+}
+
+class C() {
+    val i : Int
+    get() : Int {
+        try {
+            doSmth()
+        }
+        finally {
+            <!NO_RETURN_IN_FUNCTION_WITH_BLOCK_BODY!>doSmth()<!>
+        }
+    }
+}
+
+fun doSmth() {}

idea/testData/checker/FunctionReturnTypes.jet

@@ -5,7 +5,7 @@ fun unitEmpty() : Unit {}
 fun unitEmptyReturn() : Unit {return}
 fun unitIntReturn() : Unit {return <error>1</error>}
 fun unitUnitReturn() : Unit {return ()}
-fun test1() : Any = {<error>return</error>}
+fun test1() : Any = {return}
 fun test2() : Any = @a {return@a 1}
 fun test3() : Any { <error>return</error> }