From 0c0e8867b17cf16875306a41015d6cd7e947987e Mon Sep 17 00:00:00 2001
From: peter <peter@jetbrains.com>
Date: Wed, 8 Jan 2014 14:14:20 +0100
Subject: [PATCH] dfa: don't walk equals() method argument twice (IDEA-118971)

---
 .../dataFlow/ControlFlowAnalyzer.java         | 37 +++++++++++++------
 .../fixture/EqualsImpliesNotNull.java         | 11 ++++++
 .../DataFlowInspectionTest.java               |  2 +-
 3 files changed, 38 insertions(+), 12 deletions(-)

diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java
index 98e75b3d8c7e..401ccc64c723 100644
--- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java
+++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java
@@ -1365,12 +1365,26 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
     PsiExpression[] expressions = expression.getArgumentList().getExpressions();
     PsiElement method = methodExpression.resolve();
     PsiParameter[] parameters = method instanceof PsiMethod ? ((PsiMethod)method).getParameterList().getParameters() : null;
+    boolean isEqualsCall = expressions.length == 1 && method instanceof PsiMethod &&
+                           "equals".equals(((PsiMethod)method).getName()) && parameters.length == 1 &&
+                           parameters[0].getType().equalsToText(JAVA_LANG_OBJECT) &&
+                           PsiType.BOOLEAN.equals(((PsiMethod)method).getReturnType());
+
     for (int i = 0; i < expressions.length; i++) {
       PsiExpression paramExpr = expressions[i];
       paramExpr.accept(this);
       if (parameters != null && i < parameters.length) {
         generateBoxingUnboxingInstructionFor(paramExpr, parameters[i].getType());
       }
+      if (i == 0 && isEqualsCall) {
+        // stack: .., qualifier, arg1
+        addInstruction(new SwapInstruction());
+        // stack: .., arg1, qualifier
+        addInstruction(new DupInstruction(2, 1));
+        // stack: .., arg1, qualifier, arg1, qualifier
+        addInstruction(new PopInstruction());
+        // stack: .., arg1, qualifier, arg1
+      }
     }
 
     addConditionalRuntimeThrow();
@@ -1380,19 +1394,20 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
       addMethodThrows(expression.resolveMethod());
     }
 
-    if (expressions.length == 1 && method instanceof PsiMethod &&
-        "equals".equals(((PsiMethod)method).getName()) && parameters.length == 1 &&
-        parameters[0].getType().equalsToText(JAVA_LANG_OBJECT) &&
-        PsiType.BOOLEAN.equals(((PsiMethod)method).getReturnType())) {
-      addInstruction(new PushInstruction(myFactory.getConstFactory().getFalse(), null));
-      addInstruction(new SwapInstruction());
-      addInstruction(new ConditionalGotoInstruction(getEndOffset(expression), true, null));
+    if (isEqualsCall) {
+      // assume equals argument must be not-null if the result is true
+      // don't assume the call result to be false if arg1==null
+      
+      // stack: .., arg1, call-result
+      ConditionalGotoInstruction ifFalse = addInstruction(new ConditionalGotoInstruction(null, true, null));
 
-      addInstruction(new PopInstruction());
-      addInstruction(new PushInstruction(myFactory.getConstFactory().getTrue(), null));
-
-      expressions[0].accept(this);
       addInstruction(new ApplyNotNullInstruction(expression));
+      addInstruction(new PushInstruction(myFactory.getConstFactory().getTrue(), null));
+      addInstruction(new GotoInstruction(getEndOffset(expression)));
+
+      ifFalse.setOffset(myCurrentFlow.getInstructionCount());
+      addInstruction(new PopInstruction());
+      addInstruction(new PushInstruction(myFactory.getConstFactory().getFalse(), null));
     }
     finishElement(expression);
   }
diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/EqualsImpliesNotNull.java b/java/java-tests/testData/inspection/dataFlow/fixture/EqualsImpliesNotNull.java
index 432764450e38..b2d6bf6b96b1 100644
--- a/java/java-tests/testData/inspection/dataFlow/fixture/EqualsImpliesNotNull.java
+++ b/java/java-tests/testData/inspection/dataFlow/fixture/EqualsImpliesNotNull.java
@@ -21,4 +21,15 @@ class Test {
     System.out.println(<warning descr="Method invocation 'parentNode.toString()' may produce 'java.lang.NullPointerException'">parentNode.toString()</warning>);
   }
 
+  public static int foo(String a, String b) {
+    if (a.equals(b.startsWith("a") ? b : "")) {
+      return 0;
+    }
+    return a.length();
+  }
+
+  static boolean isEmpty(String s) {
+    return s.length() == 0;
+  }
+
 }
\ No newline at end of file
diff --git a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java
index 8b109f4ef9dd..3abc49aaddba 100644
--- a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java
+++ b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java
@@ -125,7 +125,7 @@ public class DataFlowInspectionTest extends LightCodeInsightFixtureTestCase {
   public void testPrimitiveCastMayChangeValue() throws Throwable { doTest(); }
 
   public void testPassingNullableIntoVararg() throws Throwable { doTest(); }
-  public void testEqualsImpliesNotNull() throws Throwable { doTest(); }
+  public void testEqualsImpliesNotNull() throws Throwable { doTestReportConstantReferences(); }
   public void testEffectivelyUnqualified() throws Throwable { doTest(); }
 
   public void testSkipAssertions() {