From 0d09c94ebad7cd8915dd11e202424b0525eee258 Mon Sep 17 00:00:00 2001
From: Tagir Valeev <Tagir.Valeev@jetbrains.com>
Date: Thu, 12 Apr 2018 16:46:10 +0700
Subject: [PATCH] Hardcoded contracts for Queue.peek/poll

Fixes IDEA-189790 Incorrect code analysis for peek() in Deque<E>
---
 .../dataFlow/HardcodedContracts.java          | 12 ++++++++++-
 .../dataFlow/fixture/QueuePeek.java           | 21 +++++++++++++++++++
 .../DataFlowInspection8Test.java              |  1 +
 java/jdkAnnotations/java/util/annotations.xml |  2 --
 4 files changed, 33 insertions(+), 3 deletions(-)
 create mode 100644 java/java-tests/testData/inspection/dataFlow/fixture/QueuePeek.java

diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/HardcodedContracts.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/HardcodedContracts.java
index 9da3be0f97b0..8f3d33898a58 100644
--- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/HardcodedContracts.java
+++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/HardcodedContracts.java
@@ -21,6 +21,7 @@ import com.intellij.psi.*;
 import com.intellij.psi.util.PsiUtil;
 import com.intellij.util.containers.ContainerUtil;
 import com.siyeh.ig.callMatcher.CallMapper;
+import com.siyeh.ig.callMatcher.CallMatcher;
 import com.siyeh.ig.psiutils.MethodUtils;
 import com.siyeh.ig.psiutils.TypeUtils;
 import org.jetbrains.annotations.NotNull;
@@ -52,6 +53,8 @@ public class HardcodedContracts {
                                            ContractValue.argument(2), THROW_EXCEPTION)
   );
 
+  private static final CallMatcher QUEUE_POLL = instanceCall("java.util.Queue", "poll").parameterCount(0);
+
   private static StandardMethodContract standardContract(MethodContract.ValueConstraint returnValue,
                                                          MethodContract.ValueConstraint... args) {
     return new StandardMethodContract(args, returnValue);
@@ -120,7 +123,11 @@ public class HardcodedContracts {
     .register(staticCall(JAVA_UTIL_ARRAYS, "binarySearch", "fill", "parallelPrefix", "parallelSort", "sort", "spliterator", "stream"),
               (call, cnt) -> cnt >= 3 ? ARRAY_RANGE_CONTRACTS : null)
     .register(staticCall("org.mockito.ArgumentMatchers", "argThat").parameterCount(1),
-              ContractProvider.single(() -> new StandardMethodContract(new MethodContract.ValueConstraint[] {ANY_VALUE}, ANY_VALUE)));
+              ContractProvider.single(() -> new StandardMethodContract(new MethodContract.ValueConstraint[]{ANY_VALUE}, ANY_VALUE)))
+    .register(instanceCall("java.util.Queue", "peek", "poll").parameterCount(0),
+              (call, paramCount) -> Arrays.asList(MethodContract.singleConditionContract(
+                ContractValue.qualifier().specialField(SpecialField.COLLECTION_SIZE), RelationType.EQ,
+                ContractValue.zero(), NULL_VALUE), MethodContract.trivialContract(ANY_VALUE)));
 
   public static List<MethodContract> getHardcodedContracts(@NotNull PsiMethod method, @Nullable PsiMethodCallExpression call) {
     PsiClass owner = method.getContainingClass();
@@ -362,6 +369,9 @@ public class HardcodedContracts {
     if (JAVA_UTIL_ARRAYS.equals(className)) {
       return name.equals("binarySearch") || name.equals("spliterator") || name.equals("stream");
     }
+    if (QUEUE_POLL.methodMatches(method)) {
+      return false;
+    }
     return true;
   }
 
diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/QueuePeek.java b/java/java-tests/testData/inspection/dataFlow/fixture/QueuePeek.java
new file mode 100644
index 000000000000..3ad7c97ff822
--- /dev/null
+++ b/java/java-tests/testData/inspection/dataFlow/fixture/QueuePeek.java
@@ -0,0 +1,21 @@
+import java.util.*;
+import java.util.concurrent.*;
+import org.jetbrains.annotations.*;
+
+class ThisAsVariable {
+  void check(Queue<String> queue) {
+    if(!queue.isEmpty()) {
+      System.out.println(queue.peek().length() + queue.peek().length());
+    }
+  }
+
+  void checkPoll(Queue<String> queue) {
+    if(!queue.isEmpty()) {
+      System.out.println(queue.poll().length() + queue.poll().<warning descr="Method invocation 'length' may produce 'java.lang.NullPointerException'">length</warning>());
+    }
+  }
+
+  void notCheck(Queue<String> queue) {
+    System.out.println(queue.peek().<warning descr="Method invocation 'trim' may produce 'java.lang.NullPointerException'">trim</warning>());
+  }
+}
diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection8Test.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection8Test.java
index e98a2e9a31df..3ea03492c309 100644
--- a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection8Test.java
+++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection8Test.java
@@ -225,4 +225,5 @@ public class DataFlowInspection8Test extends DataFlowInspectionTestCase {
   public void testMethodReferenceBoundToNullable() { doTestWithCustomAnnotations(); }
   public void testEscapeAnalysis() { doTest(); }
   public void testThisAsVariable() { doTest(); }
+  public void testQueuePeek() { doTest(); }
 }
\ No newline at end of file
diff --git a/java/jdkAnnotations/java/util/annotations.xml b/java/jdkAnnotations/java/util/annotations.xml
index 3f81818a6c62..8f6039cc7575 100644
--- a/java/jdkAnnotations/java/util/annotations.xml
+++ b/java/jdkAnnotations/java/util/annotations.xml
@@ -1826,13 +1826,11 @@
     <annotation name='org.jetbrains.annotations.Contract'>
       <val name="pure" val="true"/>
     </annotation>
-    <annotation name='org.jetbrains.annotations.Nullable'/>
   </item>
   <item name='java.util.Queue E poll()'>
     <annotation name='org.jetbrains.annotations.Contract'>
       <val name="mutates" val="&quot;this&quot;"/>
     </annotation>
-    <annotation name='org.jetbrains.annotations.Nullable'/>
   </item>
   <item name='java.util.Queue E remove()'>
     <annotation name='org.jetbrains.annotations.Contract'>