OptionalIsPresentInspection: simplify isRaw() and isOptionalLambdaCandidate(); fix getComments(); add more tests (IDEA-CR-13980)

java/java-analysis-impl/src/com/intellij/codeInspection/OptionalIsPresentInspection.java

@@ -37,6 +37,8 @@ import org.jetbrains.annotations.Nls;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
+import java.util.Collection;
+
 /**
  * @author Tagir Valeev
  */
@@ -84,14 +86,7 @@ public class OptionalIsPresentInspection extends BaseJavaBatchLocalInspectionToo
 
   private static boolean isRaw(PsiVariable variable) {
     PsiType type = variable.getType();
-    if(type instanceof PsiClassType) {
-      PsiClassType.ClassResolveResult resolveResult = ((PsiClassType)type).resolveGenerics();
-      PsiClass element = resolveResult.getElement();
-      if(element != null) {
-        return PsiUtil.isRawSubstitutor(element, resolveResult.getSubstitutor());
-      }
-    }
-    return false;
+    return type instanceof PsiClassType && ((PsiClassType)type).isRaw();
   }
 
   @Nullable
@@ -148,23 +143,13 @@ public class OptionalIsPresentInspection extends BaseJavaBatchLocalInspectionToo
     if(lambdaCandidate == null) return false;
     if(!ExceptionUtil.getThrownCheckedExceptions(new PsiElement[] {lambdaCandidate}).isEmpty()) return false;
     return PsiTreeUtil.processElements(lambdaCandidate, e -> {
-      if(e instanceof PsiReferenceExpression) {
-        PsiElement element = ((PsiReferenceExpression)e).resolve();
-        if(element == optionalVariable) {
-          return e.getParent() instanceof PsiReferenceExpression && e.getParent().getParent() instanceof PsiMethodCallExpression;
-        }
-        return !(element instanceof PsiVariable) ||
-               HighlightControlFlowUtil.isEffectivelyFinal((PsiVariable)element, lambdaCandidate, null);
-      }
-      if(e instanceof PsiMethodCallExpression) {
-        PsiMethodCallExpression methodCall = (PsiMethodCallExpression)e;
-        PsiExpression qualifier = methodCall.getMethodExpression().getQualifierExpression();
-        if(qualifier instanceof PsiReferenceExpression && ((PsiReferenceExpression)qualifier).resolve() == optionalVariable) {
-          return methodCall.getArgumentList().getExpressions().length == 0 &&
-                 "get".equals(methodCall.getMethodExpression().getReferenceName());
-        }
-      }
-      return true;
+      if (!(e instanceof PsiReferenceExpression)) return true;
+      PsiElement element = ((PsiReferenceExpression)e).resolve();
+      if(!(element instanceof PsiVariable)) return true;
+      // Check that Optional variable is referenced only in context of get() call and other variables are effectively final
+      return element == optionalVariable
+             ? isOptionalGetCall(e.getParent().getParent(), optionalVariable)
+             : HighlightControlFlowUtil.isEffectivelyFinal((PsiVariable)element, lambdaCandidate, null);
     });
   }
 
@@ -181,7 +166,14 @@ public class OptionalIsPresentInspection extends BaseJavaBatchLocalInspectionToo
   }
 
   static String getComments(PsiStatement statement) {
-    return StreamEx.of(statement.getChildren()).select(PsiComment.class).map(PsiElement::getText).joining(" ");
+    Collection<PsiComment> comments = PsiTreeUtil.collectElementsOfType(statement, PsiComment.class);
+    return StreamEx.of(comments)
+      .filter(c -> c.getParent() == statement ||
+                   (statement instanceof PsiExpressionStatement && c.getParent() == ((PsiExpressionStatement)statement).getExpression()))
+      .flatMap(c -> StreamEx.of(c.getPrevSibling(), c, c.getNextSibling())) // add both siblings for every comment
+      .filter(e -> e instanceof PsiComment || e instanceof PsiWhiteSpace) // select only comments and whitespace
+      .distinct()
+      .map(PsiElement::getText).joining("");
   }
 
   @NotNull

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/afterAssignmentMap.java

@@ -5,7 +5,9 @@ import java.util.*;
 public class Main {
   public void testOptional(Optional<String> str) {
     String val;
-      val = str.map(String::trim).orElse("");
+      val = str.map( // line comment
+              // another line comment
+         /* block comment *//*block comment*/String::trim).orElse("");
     System.out.println(val);
   }
 }

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/afterReturnComments.java

@@ -12,6 +12,6 @@ public class Main {
 
   public Number testOptionalComments(Optional<MyList> strList) {
     /* optional is present *//* optional is absent */
-      return strList.map(/*return something */myList -> myList.size() > /*too big*/ 1 ? myList.get(1) : 1.0).orElse(/* return null*/null);
+      return strList.map( /*return something */ myList -> myList.size() > /*too big*/ 1 ? myList.get(1) : 1.0).orElse( /* return null*/ null);
   }
 }

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/beforeAssignmentMap.java

@@ -6,7 +6,9 @@ public class Main {
   public void testOptional(Optional<String> str) {
     String val;
     if (str.isPrese<caret>nt()) {
-      val = str.get().trim();
+      val = // line comment
+      // another line comment
+        str.get().trim() /* block comment *//*block comment*/;
     } else {
       val = "";
     }

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/beforeReturnNonFinalUsed.java

@@ -0,0 +1,14 @@
+// "Replace Optional.isPresent() condition with map().orElse()" "false"
+
+import java.util.*;
+
+public class Main {
+  public String testOptional(Optional<String> str) {
+    int i = 5;
+    if (Math.random() > 0.5) i = 6;
+    if (str.isPre<caret>sent()) {
+      return str.get().substring(i);
+    }
+    return "";
+  }
+}