dataflow inspection: suggest to replace Optional.of with Optional.ofNullable if value could be null (IDEA-124359)

java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java

@@ -53,6 +53,7 @@ import com.intellij.util.SmartList;
 import com.intellij.util.containers.ContainerUtil;
 import com.intellij.util.containers.MultiMap;
 import org.jdom.Element;
+import org.jetbrains.annotations.Nls;
 import org.jetbrains.annotations.NonNls;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
@@ -190,25 +191,31 @@ public class DataFlowInspectionBase extends BaseJavaBatchLocalInspectionTool {
   private LocalQuickFix[] createNPEFixes(PsiExpression qualifier, PsiExpression expression, boolean onTheFly) {
     if (qualifier == null || expression == null) return null;
     if (qualifier instanceof PsiMethodCallExpression) return null;
-    if (qualifier instanceof PsiLiteralExpression && ((PsiLiteralExpression)qualifier).getValue() == null) return null;
 
     try {
       final List<LocalQuickFix> fixes = new SmartList<LocalQuickFix>();
 
-      if (PsiUtil.getLanguageLevel(qualifier).isAtLeast(LanguageLevel.JDK_1_4)) {
-        final Project project = qualifier.getProject();
-        final PsiElementFactory elementFactory = JavaPsiFacade.getInstance(project).getElementFactory();
-        final PsiBinaryExpression binary = (PsiBinaryExpression)elementFactory.createExpressionFromText("a != null", null);
-        binary.getLOperand().replace(qualifier);
-        ContainerUtil.addIfNotNull(fixes, createAssertFix(binary, expression));
+      if (!(qualifier instanceof PsiLiteralExpression && ((PsiLiteralExpression)qualifier).getValue() == null))  {
+        if (PsiUtil.getLanguageLevel(qualifier).isAtLeast(LanguageLevel.JDK_1_4)) {
+          final Project project = qualifier.getProject();
+          final PsiElementFactory elementFactory = JavaPsiFacade.getInstance(project).getElementFactory();
+          final PsiBinaryExpression binary = (PsiBinaryExpression)elementFactory.createExpressionFromText("a != null", null);
+          binary.getLOperand().replace(qualifier);
+          ContainerUtil.addIfNotNull(fixes, createAssertFix(binary, expression));
+        }
+
+        addSurroundWithIfFix(qualifier, fixes, onTheFly);
+
+        if (ReplaceWithTernaryOperatorFix.isAvailable(qualifier, expression)) {
+          fixes.add(new ReplaceWithTernaryOperatorFix(qualifier));
+        }
       }
 
-      addSurroundWithIfFix(qualifier, fixes, onTheFly);
-
-      if (ReplaceWithTernaryOperatorFix.isAvailable(qualifier, expression)) {
-        fixes.add(new ReplaceWithTernaryOperatorFix(qualifier));
+      if (PsiUtil.getLanguageLevel(qualifier).isAtLeast(LanguageLevel.JDK_1_8)) {
+        ContainerUtil.addIfNotNull(fixes, ReplaceOptionalOfWithOfNullableFix.registerReplaceOptionalOfWithOfNullableFix(qualifier));
       }
-      return fixes.toArray(new LocalQuickFix[fixes.size()]);
+      
+      return fixes.isEmpty() ? null : fixes.toArray(new LocalQuickFix[fixes.size()]);
     }
     catch (IncorrectOperationException e) {
       LOG.error(e);
@@ -760,4 +767,56 @@ public class DataFlowInspectionBase extends BaseJavaBatchLocalInspectionTool {
       boolean normalOk;
     }
   }
+
+  private static class ReplaceOptionalOfWithOfNullableFix implements LocalQuickFix {
+
+    private static LocalQuickFix registerReplaceOptionalOfWithOfNullableFix(PsiExpression qualifier) {
+      final PsiElement argList = PsiUtil.skipParenthesizedExprUp(qualifier).getParent();
+      if (argList instanceof PsiExpressionList) {
+        final PsiElement parent = argList.getParent();
+        if (parent instanceof PsiMethodCallExpression) {
+          final PsiMethod method = ((PsiMethodCallExpression)parent).resolveMethod();
+          if (method != null) {
+            final PsiClass containingClass = method.getContainingClass();
+            if ("of".equals(method.getName()) &&
+                containingClass != null &&
+                "java.util.Optional".equals(containingClass.getQualifiedName())) {
+              return new ReplaceOptionalOfWithOfNullableFix();
+            }
+          }
+        }
+      }
+      return null;
+    }
+    
+    @Nls
+    @NotNull
+    @Override
+    public String getName() {
+      return getFamilyName();
+    }
+
+    @NotNull
+    @Override
+    public String getFamilyName() {
+      return "Replace with '.ofNullable()'";
+    }
+
+    @Override
+    public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) {
+      final PsiMethodCallExpression
+        methodCallExpression = PsiTreeUtil.getParentOfType(descriptor.getPsiElement(), PsiMethodCallExpression.class);
+      if (methodCallExpression != null) {
+        final PsiElement ofNullableExprName =
+          ((PsiMethodCallExpression)JavaPsiFacade.getElementFactory(project)
+            .createExpressionFromText("Optional.ofNullable(null)", null)).getMethodExpression();
+        final PsiElement referenceNameElement = methodCallExpression.getMethodExpression().getReferenceNameElement();
+        if (referenceNameElement != null) {
+          final PsiElement ofNullableNameElement = ((PsiReferenceExpression)ofNullableExprName).getReferenceNameElement();
+          LOG.assertTrue(ofNullableNameElement != null);
+          referenceNameElement.replace(ofNullableNameElement);
+        }
+      }
+    }
+  }
 }

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/replaceWithOfNullable/afterNullable.java

@@ -0,0 +1,6 @@
+// "Replace with '.ofNullable()'" "true"
+class A{
+  void test(){
+    java.util.Optional.ofNullable(null);
+  }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/replaceWithOfNullable/beforeNotNull.java

@@ -0,0 +1,6 @@
+// "Replace with '.ofNullable()'" "false"
+class A{
+  void test(){
+    java.util.Optional.of(1<caret>1);
+  }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/replaceWithOfNullable/beforeNullable.java

@@ -0,0 +1,6 @@
+// "Replace with '.ofNullable()'" "true"
+class A{
+  void test(){
+    java.util.Optional.of(nu<caret>ll);
+  }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/replaceWithOfNullable/beforeUnresolved.java

@@ -0,0 +1,6 @@
+// "Replace with '.ofNullable()'" "false"
+class A{
+  void test(){
+    Optional.of(nu<caret>ll);
+  }
+}

java/java-tests/testSrc/com/intellij/codeInsight/daemon/quickFix/ReplaceWithOfNullableFixTest.java

@@ -0,0 +1,49 @@
+/*
+ * Copyright 2000-2015 JetBrains s.r.o.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/*
+ * User: anna
+ * Date: 21-Mar-2008
+ */
+package com.intellij.codeInsight.daemon.quickFix;
+
+import com.intellij.codeInspection.LocalInspectionTool;
+import com.intellij.codeInspection.dataFlow.DataFlowInspection;
+import com.intellij.openapi.projectRoots.Sdk;
+import com.intellij.testFramework.IdeaTestUtil;
+import org.jetbrains.annotations.NotNull;
+
+public class ReplaceWithOfNullableFixTest extends LightQuickFixParameterizedTestCase {
+  @NotNull
+  @Override
+  protected LocalInspectionTool[] configureLocalInspectionTools() {
+    return new LocalInspectionTool[]{new DataFlowInspection()};
+  }
+
+  public void test() throws Exception {
+     doAllTests();
+   }
+
+  @Override
+  protected String getBasePath() {
+    return "/codeInsight/daemonCodeAnalyzer/quickFix/replaceWithOfNullable";
+  }
+
+  @Override
+  protected Sdk getProjectJDK() {
+    return IdeaTestUtil.getMockJdk18();
+  }
+}