diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/cfg/JetControlFlowProcessor.java b/compiler/frontend/src/org/jetbrains/jet/lang/cfg/JetControlFlowProcessor.java index bc30b50af1e..ac4eb24fd0f 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/cfg/JetControlFlowProcessor.java +++ b/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)); + } } } diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/cfg/JetFlowInformationProvider.java b/compiler/frontend/src/org/jetbrains/jet/lang/cfg/JetFlowInformationProvider.java index 98b49352263..91b653fa8f3 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/cfg/JetFlowInformationProvider.java +++ b/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 returnedExpressions) { + private void collectReturnExpressions(@NotNull JetElement subroutine, @NotNull final Collection returnedExpressions) { Pseudocode pseudocode = pseudocodeMap.get(subroutine); assert pseudocode != null; @@ -119,20 +123,65 @@ public class JetFlowInformationProvider { }); } } + + public void checkDefiniteReturn(@NotNull JetDeclarationWithBody function, final @NotNull JetType expectedReturnType) { + assert function instanceof JetDeclaration; - public void collectUnreachableExpressions(@NotNull JetElement subroutine, @NotNull Collection unreachableElements) { + JetExpression bodyExpression = function.getBodyExpression(); + if (bodyExpression == null) return; + + List 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 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 collectUnreachableCode(@NotNull JetElement subroutine) { Pseudocode pseudocode = pseudocodeMap.get(subroutine); assert pseudocode != null; + Collection 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); } } } diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/cfg/pseudocode/JetControlFlowInstructionsGenerator.java b/compiler/frontend/src/org/jetbrains/jet/lang/cfg/pseudocode/JetControlFlowInstructionsGenerator.java index 9c5e14b2c58..261a27a31b7 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/cfg/pseudocode/JetControlFlowInstructionsGenerator.java +++ b/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, builder.getCurrentSubroutine()); - } - else { - pushBuilder(subroutine, subroutine); - } + pushBuilder(subroutine, subroutine); assert builder != null; builder.enterSubroutine(subroutine); } diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/BodyResolver.java b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/BodyResolver.java index 093f93825d2..8f45d29fc0e 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/BodyResolver.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/BodyResolver.java @@ -281,11 +281,11 @@ public class BodyResolver { } private void resolveSecondaryConstructorBodies() { - for (Map.Entry entry : this.context.getConstructors().entrySet()) { - JetDeclaration declaration = entry.getKey(); + for (Map.Entry entry : this.context.getConstructors().entrySet()) { + 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; } diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/ControlFlowAnalyzer.java b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/ControlFlowAnalyzer.java index 63f48823be3..efd8b63c65c 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/ControlFlowAnalyzer.java +++ b/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.typeInferrerServices = context.getSemanticServices().getTypeInferrerServices(context.getTrace()); + this.inLocalDeclaration = inLocalDeclaration; } public void process() { - for (Map.Entry entry : context.getClasses().entrySet()) { - JetClass aClass = entry.getKey(); - MutableClassDescriptor classDescriptor = entry.getValue(); - + for (JetClass aClass : context.getClasses().keySet()) { if (!context.completeAnalysisNeeded(aClass)) continue; - - checkClassOrObject(aClass, classDescriptor); + checkClassOrObject(aClass); } - - for (Map.Entry entry : context.getObjects().entrySet()) { - JetObjectDeclaration objectDeclaration = entry.getKey(); - MutableClassDescriptor objectDescriptor = entry.getValue(); - + for (JetObjectDeclaration objectDeclaration : context.getObjects().keySet()) { if (!context.completeAnalysisNeeded(objectDeclaration)) continue; - - checkClassOrObject(objectDeclaration, objectDescriptor); + checkClassOrObject(objectDeclaration); } - for (Map.Entry 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 (Map.Entry entry : this.context.getConstructors().entrySet()) { - JetDeclaration declaration = entry.getKey(); - assert declaration instanceof JetConstructor; - JetConstructor constructor = (JetConstructor) declaration; - ConstructorDescriptor descriptor = entry.getValue(); - + for (JetConstructor constructor : this.context.getConstructors().keySet()) { if (!context.completeAnalysisNeeded(constructor)) continue; - checkFunction(constructor, descriptor, JetStandardClasses.getUnitType()); - } - - for (JetProperty property : context.getProperties().keySet()) { - if (!context.completeAnalysisNeeded(property)) continue; - checkProperty(property); + checkFunction(constructor, JetStandardClasses.getUnitType()); } } - 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 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(); - List unreachableElements = Lists.newArrayList(); - flowInformationProvider.collectUnreachableExpressions(function.asElement(), unreachableElements); + flowInformationProvider.checkDefiniteReturn(function, expectedReturnType); - // This is needed in order to highlight only '1 < 2' and not '1', '<' and '2' as well - final Set rootUnreachableElements = JetPsiUtil.findRootExpressions(unreachableElements); - - for (JetElement element : rootUnreachableElements) { - context.getTrace().report(UNREACHABLE_CODE.on(element)); - } - - List 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()); + flowInformationProvider.markUninitializedVariables(function.asElement(), false, inLocalDeclaration); } } diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/TopDownAnalysisContext.java b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/TopDownAnalysisContext.java index 1fd0bad84f5..db48bedaaed 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/TopDownAnalysisContext.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/TopDownAnalysisContext.java @@ -33,7 +33,7 @@ import java.util.Set; private final Map declaringScopes = Maps.newHashMap(); private final Map functions = Maps.newLinkedHashMap(); - private final Map constructors = Maps.newLinkedHashMap(); + private final Map constructors = Maps.newLinkedHashMap(); private final Map properties = Maps.newLinkedHashMap(); private final Set primaryConstructorParameterProperties = Sets.newHashSet(); @@ -107,7 +107,7 @@ import java.util.Set; return primaryConstructorParameterProperties; } - public Map getConstructors() { + public Map getConstructors() { return constructors; } diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/types/expressions/ClosureExpressionsTypingVisitor.java b/compiler/frontend/src/org/jetbrains/jet/lang/types/expressions/ClosureExpressionsTypingVisitor.java index a8c2fde4a6c..7abdb5cefe9 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/types/expressions/ClosureExpressionsTypingVisitor.java +++ b/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("") : returnType; functionDescriptor.setReturnType(safeReturnType); diff --git a/compiler/testData/cfg/AnonymousInitializers.instructions b/compiler/testData/cfg/AnonymousInitializers.instructions index 3f5a19173d7..6aa7daaa126 100644 --- a/compiler/testData/cfg/AnonymousInitializers.instructions +++ b/compiler/testData/cfg/AnonymousInitializers.instructions @@ -55,16 +55,3 @@ error: sink: NEXT:[] PREV:[] ===================== -== k == -val k = 34 ---------------------- -l0: - NEXT:[r(34)] PREV:[] - r(34) NEXT:[] PREV:[] -l1: - NEXT:[] PREV:[r(34)] -error: - NEXT:[] PREV:[] -sink: - NEXT:[] PREV:[] -===================== diff --git a/compiler/testData/cfg/LocalDeclarations.instructions b/compiler/testData/cfg/LocalDeclarations.instructions index 5c28d11a9fc..8f7b54c2a39 100644 --- a/compiler/testData/cfg/LocalDeclarations.instructions +++ b/compiler/testData/cfg/LocalDeclarations.instructions @@ -387,16 +387,3 @@ error: sink: NEXT:[] PREV:[] ===================== -== a == -val a: Int = 1 ---------------------- -l0: - NEXT:[r(1)] PREV:[] - r(1) NEXT:[] PREV:[] -l1: - NEXT:[] PREV:[r(1)] -error: - NEXT:[] PREV:[] -sink: - NEXT:[] PREV:[] -===================== diff --git a/compiler/testData/checkerWithErrorTypes/full/FunctionReturnTypes.jet b/compiler/testData/checkerWithErrorTypes/full/FunctionReturnTypes.jet index 68b1803449f..5a041264da5 100644 --- a/compiler/testData/checkerWithErrorTypes/full/FunctionReturnTypes.jet +++ b/compiler/testData/checkerWithErrorTypes/full/FunctionReturnTypes.jet @@ -5,7 +5,7 @@ fun unitEmpty() : Unit {} fun unitEmptyReturn() : Unit {return} fun unitIntReturn() : Unit {return 1} fun unitUnitReturn() : Unit {return ()} -fun test1() : Any = {return} +fun test1() : Any = {return} fun test2() : Any = @a {return@a 1} fun test3() : Any { return } diff --git a/compiler/testData/checkerWithErrorTypes/quick/regressions/kt456.jet b/compiler/testData/checkerWithErrorTypes/quick/regressions/kt456.jet new file mode 100644 index 00000000000..d27eb2d3faa --- /dev/null +++ b/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 error + } +} + +//more tests +class B() { + val i: Int + get() { //no error + } +} + +class C() { + val i : Int + get() : Int { + try { + doSmth() + } + finally { + doSmth() + } + } +} + +fun doSmth() {} \ No newline at end of file diff --git a/idea/testData/checker/FunctionReturnTypes.jet b/idea/testData/checker/FunctionReturnTypes.jet index 43a607f9714..5730b95e18b 100644 --- a/idea/testData/checker/FunctionReturnTypes.jet +++ b/idea/testData/checker/FunctionReturnTypes.jet @@ -5,7 +5,7 @@ fun unitEmpty() : Unit {} fun unitEmptyReturn() : Unit {return} fun unitIntReturn() : Unit {return 1} fun unitUnitReturn() : Unit {return ()} -fun test1() : Any = {return} +fun test1() : Any = {return} fun test2() : Any = @a {return@a 1} fun test3() : Any { return }