KT-2223 Comparing non-null value with null might produce helpful warning

#KT-2223 Fixed

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

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));
                 }
-            }
+                checkSenselessComparisonWithNull(expression, left, right, context);
-            if (isSenselessComparisonWithNull(leftType, right) || isSenselessComparisonWithNull(rightType, left)) {
-                context.trace.report(SENSELESS_COMPARISON.on(expression, expression, operationSign.getReferencedNameElementType() == JetTokens.EXCLEQ));
             }
         }
     }
 
-    private boolean isSenselessComparisonWithNull(@Nullable JetType firstType, @NotNull JetExpression secondExpression) {
+    private void checkSenselessComparisonWithNull(@NotNull JetBinaryExpression expression, @NotNull JetExpression left, @NotNull JetExpression right, @NotNull ExpressionTypingContext context) {
-        if (firstType == null) return false;
+        JetExpression expr;
-        return !firstType.isNullable() && secondExpression instanceof JetConstantExpression && secondExpression.getNode().getElementType() == JetNodeTypes.NULL;
+        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) {

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) {
+  val out2 : java.io.PrintStream? = null
-    out?.println();
+  
+  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)
   }

compiler/testData/diagnostics/tests/dataFlowInfoTraversal/DeepIf.jet

@@ -5,24 +5,24 @@ fun foo() {
 
     if (x != null) {
         bar(x)
-        if (x != null) {
+        if (<!SENSELESS_COMPARISON!>x != null<!>) {
             bar(x)
             if (1 < 2) bar(x)
             if (1 > 2) bar(x)
         }
-        if (x == null) {
+        if (<!SENSELESS_COMPARISON!>x == null<!>) {
             bar(x)
         }
-        if (x == null) bar(x) else bar(x)
+        if (<!SENSELESS_COMPARISON!>x == null<!>) bar(x) else bar(x)
         bar(bar(x))
-    } else if (x == null) {
+    } else if (<!SENSELESS_COMPARISON!>x == null<!>) {
         bar(<!TYPE_MISMATCH!>x<!>)
-        if (x != null) {
+        if (<!SENSELESS_COMPARISON!>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 (<!SENSELESS_COMPARISON!>x == null<!>) {
             bar(<!TYPE_MISMATCH!>x<!>)
         }
     }

compiler/testData/diagnostics/tests/dataFlowInfoTraversal/ExclExcl.jet

@@ -1,13 +1,23 @@
 fun bar(x: Int) = x + 1
 
-fun foo() {
+fun f1(x: Int?) {
-    val x: Int? = null
-
     bar(<!TYPE_MISMATCH!>x<!>)
     if (x != null) bar(x<!UNNECESSARY_NOT_NULL_ASSERTION!>!!<!>)
     if (x == null) bar(x!!)
-    if (x != null) else bar(x<!UNNECESSARY_NOT_NULL_ASSERTION!>!!<!>)
+}
-    if (x == null) else bar(x<!UNNECESSARY_NOT_NULL_ASSERTION!>!!<!>)
+
-    if (x != null) bar(x<!UNNECESSARY_NOT_NULL_ASSERTION!>!!<!>) else bar(x<!UNNECESSARY_NOT_NULL_ASSERTION!>!!<!>)
+fun f2(x: Int?) {    
-    if (x == null) bar(x<!UNNECESSARY_NOT_NULL_ASSERTION!>!!<!>) else bar(x<!UNNECESSARY_NOT_NULL_ASSERTION!>!!<!>)
+    if (x != null) else bar(x!!)
+}
+
+fun f3(x: Int?) {    
+    if (x != null) bar(x<!UNNECESSARY_NOT_NULL_ASSERTION!>!!<!>) else bar(x!!)
+}
+    
+fun f4(x: Int?) {    
+    if (x == null) bar(x!!) else bar(x<!UNNECESSARY_NOT_NULL_ASSERTION!>!!<!>)
+}
+
+fun f5(x: Int?) {    
+    if (x == null) else bar(x<!UNNECESSARY_NOT_NULL_ASSERTION!>!!<!>)
 }

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 (<!SENSELESS_COMPARISON!>x == null<!>) bar(x)
         }
     }
 

compiler/testData/diagnostics/tests/dataFlowInfoTraversal/IfThenElse.jet

@@ -26,5 +26,5 @@ fun foo() {
     if (z != null) bar(z)
     bar(<!TYPE_MISMATCH!>z<!>)
     bar(z!!)
-    if (z != null) bar(z<!UNNECESSARY_NOT_NULL_ASSERTION!>!!<!>)
+    if (<!SENSELESS_COMPARISON!>z != null<!>) bar(z<!UNNECESSARY_NOT_NULL_ASSERTION!>!!<!>)
 }

compiler/testData/diagnostics/tests/dataFlowInfoTraversal/IfThenElseBothInvalid.jet

@@ -9,7 +9,7 @@ fun foo() {
         2+<!SYNTAX!><!>
     }
     else {
-        if (x == null) return
+        if (<!SENSELESS_COMPARISON!>x == null<!>) return
         2+<!SYNTAX!><!>
     }
     bar(x)

compiler/testData/diagnostics/tests/dataFlowInfoTraversal/Return.jet

@@ -5,8 +5,12 @@ fun foo(): Int {
 
     bar(<!TYPE_MISMATCH!>x<!>)
     if (x != null) return x
-    if (x == null) return if (x != null) x else <!TYPE_MISMATCH!>x<!>
+    
-    if (x == null) return x
+    val y: Int? = null
-    if (x != null) return if (x == null) x else x
+    if (y == null) return if (<!SENSELESS_COMPARISON!>y != null<!>) y else <!TYPE_MISMATCH!>y<!>
-    return x
+    
+    val z: Int? = null
+    if (z != null) return if (<!SENSELESS_COMPARISON!>z == null<!>) z else z
+    
+    return <!TYPE_MISMATCH!>z<!>
 }

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(<!TYPE_MISMATCH!>x<!>)
+    if (x == null) throw bar(<!TYPE_MISMATCH!>x<!>)
-    if (x == null) return
     throw bar(x)
     throw <!UNREACHABLE_CODE!>bar(x)<!>
 }

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 (<!SENSELESS_COMPARISON!>x == null<!>) return
+}

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) {
+  val out2 : java.io.PrintStream? = null
-    out?.println();
+  
+  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)
   }