diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/calls/autocasts/DataFlowInfo.java b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/calls/autocasts/DataFlowInfo.java index e0c5a2f3b30..87c6ec23856 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/calls/autocasts/DataFlowInfo.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/calls/autocasts/DataFlowInfo.java @@ -64,7 +64,7 @@ public class DataFlowInfo { } @NotNull - private Nullability getNullability(@NotNull DataFlowValue a) { + public Nullability getNullability(@NotNull DataFlowValue a) { if (!a.isStableIdentifier()) return a.getImmanentNullability(); Nullability nullability = nullabilityInfo.get(a); if (nullability == null) { diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/types/expressions/BasicExpressionTypingVisitor.java b/compiler/frontend/src/org/jetbrains/jet/lang/types/expressions/BasicExpressionTypingVisitor.java index ec3ac80f27a..a13db2f5ec2 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/types/expressions/BasicExpressionTypingVisitor.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/types/expressions/BasicExpressionTypingVisitor.java @@ -35,6 +35,7 @@ import org.jetbrains.jet.lang.resolve.calls.OverloadResolutionResultsUtil; import org.jetbrains.jet.lang.resolve.calls.autocasts.DataFlowInfo; import org.jetbrains.jet.lang.resolve.calls.autocasts.DataFlowValue; import org.jetbrains.jet.lang.resolve.calls.autocasts.DataFlowValueFactory; +import org.jetbrains.jet.lang.resolve.calls.autocasts.Nullability; import org.jetbrains.jet.lang.resolve.constants.*; import org.jetbrains.jet.lang.resolve.constants.StringValue; import org.jetbrains.jet.lang.resolve.name.LabelName; @@ -1036,29 +1037,50 @@ public class BasicExpressionTypingVisitor extends ExpressionTypingVisitor { } private void ensureNonemptyIntersectionOfOperandTypes(JetBinaryExpression expression, ExpressionTypingContext context) { - JetSimpleNameExpression operationSign = expression.getOperationReference(); JetExpression left = expression.getLeft(); JetExpression right = expression.getRight(); // TODO : duplicated effort for == and != - JetType leftType = facade.getTypeInfo(left, context.replaceScope(context.scope)).getType(); + JetType leftType = facade.getTypeInfo(left, context).getType(); if (leftType != null && right != null) { - JetType rightType = facade.getTypeInfo(right, context.replaceScope(context.scope)).getType(); + JetType rightType = facade.getTypeInfo(right, context).getType(); if (rightType != null) { if (TypeUtils.isIntersectionEmpty(leftType, rightType)) { - context.trace.report(EQUALITY_NOT_APPLICABLE.on(expression, operationSign, leftType, rightType)); + context.trace.report(EQUALITY_NOT_APPLICABLE.on(expression, expression.getOperationReference(), leftType, rightType)); } - } - if (isSenselessComparisonWithNull(leftType, right) || isSenselessComparisonWithNull(rightType, left)) { - context.trace.report(SENSELESS_COMPARISON.on(expression, expression, operationSign.getReferencedNameElementType() == JetTokens.EXCLEQ)); + checkSenselessComparisonWithNull(expression, left, right, context); } } } - private boolean isSenselessComparisonWithNull(@Nullable JetType firstType, @NotNull JetExpression secondExpression) { - if (firstType == null) return false; - return !firstType.isNullable() && secondExpression instanceof JetConstantExpression && secondExpression.getNode().getElementType() == JetNodeTypes.NULL; + private void checkSenselessComparisonWithNull(@NotNull JetBinaryExpression expression, @NotNull JetExpression left, @NotNull JetExpression right, @NotNull ExpressionTypingContext context) { + JetExpression expr; + if (left instanceof JetConstantExpression && left.getNode().getElementType() == JetNodeTypes.NULL) { + expr = right; + } + else if (right instanceof JetConstantExpression && right.getNode().getElementType() == JetNodeTypes.NULL) { + expr = left; + } + else return; + + JetSimpleNameExpression operationSign = expression.getOperationReference(); + JetType type = facade.getTypeInfo(expr, context).getType(); + DataFlowValue value = DataFlowValueFactory.INSTANCE.createDataFlowValue(expr, type, context.trace.getBindingContext()); + Nullability nullability = context.dataFlowInfo.getNullability(value); + + boolean expressionIsAlways; + boolean equality = operationSign.getReferencedNameElementType() == JetTokens.EQEQ || operationSign.getReferencedNameElementType() == JetTokens.EQEQEQ; + + if (nullability == Nullability.NULL) { + expressionIsAlways = equality; + } + else if (nullability == Nullability.NOT_NULL) { + expressionIsAlways = !equality; + } + else return; + + context.trace.report(SENSELESS_COMPARISON.on(expression, expression, expressionIsAlways)); } protected JetType visitAssignmentOperation(JetBinaryExpression expression, ExpressionTypingContext context) { diff --git a/compiler/testData/diagnostics/tests/Nullability.jet b/compiler/testData/diagnostics/tests/Nullability.jet index b4a7c8e5786..07ec8334475 100644 --- a/compiler/testData/diagnostics/tests/Nullability.jet +++ b/compiler/testData/diagnostics/tests/Nullability.jet @@ -1,4 +1,3 @@ - fun test() { val a : Int? = 0 if (a != null) { @@ -53,7 +52,7 @@ fun test() { out?.println(); } - if (out == null || out != null && out.println() == #()) { + if (out == null || out.println() == #()) { out?.println(); } else { @@ -109,7 +108,7 @@ fun test() { out?.println(); } - if (out == null || out != null && out.println() == #()) { + if (out == null || out.println() == #()) { out?.println(); } else { @@ -134,10 +133,12 @@ fun test() { } out?.println(); - while (out == null) { - out?.println(); + val out2 : java.io.PrintStream? = null + + while (out2 == null) { + out2?.println(); } - out.println() + out2.println() } @@ -247,7 +248,7 @@ fun f7(s : String?, t : String?) { } s?.get(0) t?.get(0) - if (!(s == null && s == null)) { + if (!(s == null)) { s.get(0) t?.get(0) } diff --git a/compiler/testData/diagnostics/tests/dataFlowInfoTraversal/DeepIf.jet b/compiler/testData/diagnostics/tests/dataFlowInfoTraversal/DeepIf.jet index 5373ed1f4d4..28681d401db 100644 --- a/compiler/testData/diagnostics/tests/dataFlowInfoTraversal/DeepIf.jet +++ b/compiler/testData/diagnostics/tests/dataFlowInfoTraversal/DeepIf.jet @@ -5,24 +5,24 @@ fun foo() { if (x != null) { bar(x) - if (x != null) { + if (x != null) { bar(x) if (1 < 2) bar(x) if (1 > 2) bar(x) } - if (x == null) { + if (x == null) { bar(x) } - if (x == null) bar(x) else bar(x) + if (x == null) bar(x) else bar(x) bar(bar(x)) - } else if (x == null) { + } else if (x == null) { bar(x) - if (x != null) { + if (x != null) { bar(x) if (x == null) bar(x) if (x == null) bar(x) else bar(x) bar(bar(x) + bar(x)) - } else if (x == null) { + } else if (x == null) { bar(x) } } diff --git a/compiler/testData/diagnostics/tests/dataFlowInfoTraversal/ExclExcl.jet b/compiler/testData/diagnostics/tests/dataFlowInfoTraversal/ExclExcl.jet index 9464d13025c..3cd4605a65c 100644 --- a/compiler/testData/diagnostics/tests/dataFlowInfoTraversal/ExclExcl.jet +++ b/compiler/testData/diagnostics/tests/dataFlowInfoTraversal/ExclExcl.jet @@ -1,13 +1,23 @@ fun bar(x: Int) = x + 1 -fun foo() { - val x: Int? = null - +fun f1(x: Int?) { bar(x) if (x != null) bar(x!!) if (x == null) bar(x!!) - if (x != null) else bar(x!!) - if (x == null) else bar(x!!) - if (x != null) bar(x!!) else bar(x!!) - if (x == null) bar(x!!) else bar(x!!) +} + +fun f2(x: Int?) { + if (x != null) else bar(x!!) +} + +fun f3(x: Int?) { + if (x != null) bar(x!!) else bar(x!!) +} + +fun f4(x: Int?) { + if (x == null) bar(x!!) else bar(x!!) +} + +fun f5(x: Int?) { + if (x == null) else bar(x!!) } diff --git a/compiler/testData/diagnostics/tests/dataFlowInfoTraversal/For.jet b/compiler/testData/diagnostics/tests/dataFlowInfoTraversal/For.jet index aa1ef31839c..136384a32bd 100644 --- a/compiler/testData/diagnostics/tests/dataFlowInfoTraversal/For.jet +++ b/compiler/testData/diagnostics/tests/dataFlowInfoTraversal/For.jet @@ -12,7 +12,7 @@ fun foo() { bar(x) for (q in a) { bar(x) - if (x == null) bar(x) + if (x == null) bar(x) } } diff --git a/compiler/testData/diagnostics/tests/dataFlowInfoTraversal/IfThenElse.jet b/compiler/testData/diagnostics/tests/dataFlowInfoTraversal/IfThenElse.jet index bc1e14b7dab..6d14c43e8ca 100644 --- a/compiler/testData/diagnostics/tests/dataFlowInfoTraversal/IfThenElse.jet +++ b/compiler/testData/diagnostics/tests/dataFlowInfoTraversal/IfThenElse.jet @@ -26,5 +26,5 @@ fun foo() { if (z != null) bar(z) bar(z) bar(z!!) - if (z != null) bar(z!!) + if (z != null) bar(z!!) } diff --git a/compiler/testData/diagnostics/tests/dataFlowInfoTraversal/IfThenElseBothInvalid.jet b/compiler/testData/diagnostics/tests/dataFlowInfoTraversal/IfThenElseBothInvalid.jet index 37f20e95089..8b40195586a 100644 --- a/compiler/testData/diagnostics/tests/dataFlowInfoTraversal/IfThenElseBothInvalid.jet +++ b/compiler/testData/diagnostics/tests/dataFlowInfoTraversal/IfThenElseBothInvalid.jet @@ -9,7 +9,7 @@ fun foo() { 2+ } else { - if (x == null) return + if (x == null) return 2+ } bar(x) diff --git a/compiler/testData/diagnostics/tests/dataFlowInfoTraversal/Return.jet b/compiler/testData/diagnostics/tests/dataFlowInfoTraversal/Return.jet index 23f0379d1a7..99ab397035b 100644 --- a/compiler/testData/diagnostics/tests/dataFlowInfoTraversal/Return.jet +++ b/compiler/testData/diagnostics/tests/dataFlowInfoTraversal/Return.jet @@ -5,8 +5,12 @@ fun foo(): Int { bar(x) if (x != null) return x - if (x == null) return if (x != null) x else x - if (x == null) return x - if (x != null) return if (x == null) x else x - return x + + val y: Int? = null + if (y == null) return if (y != null) y else y + + val z: Int? = null + if (z != null) return if (z == null) z else z + + return z } diff --git a/compiler/testData/diagnostics/tests/dataFlowInfoTraversal/Throw.jet b/compiler/testData/diagnostics/tests/dataFlowInfoTraversal/Throw.jet index e1e2ce8fd57..82e21b339ff 100644 --- a/compiler/testData/diagnostics/tests/dataFlowInfoTraversal/Throw.jet +++ b/compiler/testData/diagnostics/tests/dataFlowInfoTraversal/Throw.jet @@ -3,8 +3,7 @@ fun bar(x: Int): RuntimeException = RuntimeException(x.toString()) fun foo() { val x: Int? = null - if (x == null || 1 < 2) throw bar(x) - if (x == null) return + if (x == null) throw bar(x) throw bar(x) throw bar(x) } diff --git a/compiler/testData/diagnostics/tests/nullabilityAndAutoCasts/kt2223.jet b/compiler/testData/diagnostics/tests/nullabilityAndAutoCasts/kt2223.jet new file mode 100644 index 00000000000..a3cfda37e59 --- /dev/null +++ b/compiler/testData/diagnostics/tests/nullabilityAndAutoCasts/kt2223.jet @@ -0,0 +1,8 @@ +//KT-2223 Comparing non-null value with null might produce helpful warning +package kt2223 + +fun foo() { + val x: Int? = null + if (x == null) return + if (x == null) return +} diff --git a/idea/testData/checker/Nullability.jet b/idea/testData/checker/Nullability.jet index 51250eb9b6b..07ec8334475 100644 --- a/idea/testData/checker/Nullability.jet +++ b/idea/testData/checker/Nullability.jet @@ -52,7 +52,7 @@ fun test() { out?.println(); } - if (out == null || out != null && out.println() == #()) { + if (out == null || out.println() == #()) { out?.println(); } else { @@ -108,7 +108,7 @@ fun test() { out?.println(); } - if (out == null || out != null && out.println() == #()) { + if (out == null || out.println() == #()) { out?.println(); } else { @@ -133,10 +133,12 @@ fun test() { } out?.println(); - while (out == null) { - out?.println(); + val out2 : java.io.PrintStream? = null + + while (out2 == null) { + out2?.println(); } - out.println() + out2.println() } @@ -246,7 +248,7 @@ fun f7(s : String?, t : String?) { } s?.get(0) t?.get(0) - if (!(s == null && s == null)) { + if (!(s == null)) { s.get(0) t?.get(0) }