From 635ceeee71fac2437403574316569cbbbf407689 Mon Sep 17 00:00:00 2001
From: Tagir Valeev <tagir.valeev@jetbrains.com>
Date: Thu, 2 Aug 2018 02:58:39 +0700
Subject: [PATCH] =?UTF-8?q?IDEA-196563=20Wrong=20=E2=80=9CContract=20issue?=
 =?UTF-8?q?=E2=80=9D=20inspection=20stating=20a=20method=20always=20fails?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 .../dataFlow/ContractChecker.java              | 18 +++++++++---------
 .../contractCheck/WrongFailSuggestion.java     | 17 +++++++++++++++++
 .../java/codeInspection/ContractCheckTest.java |  1 +
 3 files changed, 27 insertions(+), 9 deletions(-)
 create mode 100644 java/java-tests/testData/inspection/dataFlow/contractCheck/WrongFailSuggestion.java

diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ContractChecker.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ContractChecker.java
index 0add795dda55..7f244611f770 100644
--- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ContractChecker.java
+++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ContractChecker.java
@@ -70,10 +70,18 @@ class ContractChecker extends DataFlowRunner {
   @Override
   protected DfaInstructionState[] acceptInstruction(@NotNull InstructionVisitor visitor, @NotNull DfaInstructionState instructionState) {
     DfaMemoryState memState = instructionState.getMemoryState();
+    Instruction instruction = instructionState.getInstruction();
+    if (instruction instanceof ReturnInstruction) {
+      if (((ReturnInstruction)instruction).isViaException()) {
+        ContainerUtil.addIfNotNull(myFailures, ((ReturnInstruction)instruction).getAnchor());
+      } else {
+        myMayReturnNormally = true;
+      }
+    }
+
     if (memState.isEphemeral()) {
       return super.acceptInstruction(visitor, instructionState);
     }
-    Instruction instruction = instructionState.getInstruction();
     if (instruction instanceof CheckReturnValueInstruction) {
       PsiElement anchor = ((CheckReturnValueInstruction)instruction).getReturn();
       DfaValue retValue = memState.pop();
@@ -86,14 +94,6 @@ class ContractChecker extends DataFlowRunner {
 
     }
 
-    if (instruction instanceof ReturnInstruction) {
-      if (((ReturnInstruction)instruction).isViaException()) {
-        ContainerUtil.addIfNotNull(myFailures, ((ReturnInstruction)instruction).getAnchor());
-      } else {
-        myMayReturnNormally = true;
-      }
-    }
-
     if (instruction instanceof MethodCallInstruction &&
         ((MethodCallInstruction)instruction).getMethodType() == MethodCallInstruction.MethodType.REGULAR_METHOD_CALL) {
       if (myContract.getReturnValue().isFail()) {
diff --git a/java/java-tests/testData/inspection/dataFlow/contractCheck/WrongFailSuggestion.java b/java/java-tests/testData/inspection/dataFlow/contractCheck/WrongFailSuggestion.java
new file mode 100644
index 000000000000..14f6c5049ba8
--- /dev/null
+++ b/java/java-tests/testData/inspection/dataFlow/contractCheck/WrongFailSuggestion.java
@@ -0,0 +1,17 @@
+import org.jetbrains.annotations.Contract;
+import org.jetbrains.annotations.Nullable;
+import org.jetbrains.annotations.NotNull;
+import java.util.Map;
+
+class Foo {
+  // IDEA-196563
+  @Nullable
+  @Contract(pure = true, value = "null, _, true -> fail; _, _, true -> !null")
+  private static Object getParam(@Nullable Map<String, Object> params, @NotNull String paramName, boolean required) {
+    final Object value = params == null ? null : params.get(paramName);
+    if (value == null && required) {
+      throw new IllegalArgumentException("Parameter not found");
+    }
+    return value;
+  }
+}
diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/ContractCheckTest.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/ContractCheckTest.java
index c1f2fc3b581d..0956391901d9 100644
--- a/java/java-tests/testSrc/com/intellij/java/codeInspection/ContractCheckTest.java
+++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/ContractCheckTest.java
@@ -54,4 +54,5 @@ public class ContractCheckTest extends LightCodeInsightFixtureTestCase {
   public void testMutationSignatureProblems() { doTest(); }
   public void testNewThisParam() { doTest(); }
   public void testConditionsConflict() { doTest(); }
+  public void testWrongFailSuggestion() { doTest(); }
 }