dfa: don't suggest to assert/surround-with-if possible NPEs on volatile fields, suggest to extract a local variable instead (IDEA-145668)

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

@@ -225,7 +225,10 @@ public class DataFlowInspectionBase extends BaseJavaBatchLocalInspectionTool {
     try {
       final List<LocalQuickFix> fixes = new SmartList<LocalQuickFix>();
 
-      if (!(qualifier instanceof PsiLiteralExpression && ((PsiLiteralExpression)qualifier).getValue() == null))  {
+      if (isVolatileFieldReference(qualifier)) {
+        ContainerUtil.addIfNotNull(fixes, createIntroduceVariableFix(qualifier));
+      }
+      else 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();
@@ -250,6 +253,16 @@ public class DataFlowInspectionBase extends BaseJavaBatchLocalInspectionTool {
     }
   }
 
+  @Nullable
+  protected LocalQuickFixOnPsiElement createIntroduceVariableFix(PsiExpression expression) {
+    return null;
+  }
+
+  private static boolean isVolatileFieldReference(PsiExpression qualifier) {
+    PsiElement target = qualifier instanceof PsiReferenceExpression ? ((PsiReferenceExpression)qualifier).resolve() : null;
+    return target instanceof PsiField && ((PsiField)target).hasModifierProperty(PsiModifier.VOLATILE);
+  }
+
   protected LocalQuickFix createAssertFix(PsiBinaryExpression binary, PsiExpression expression) {
     return null;
   }

java/java-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspection.java

@@ -19,14 +19,18 @@ import com.intellij.codeInsight.NullableNotNullDialog;
 import com.intellij.codeInspection.*;
 import com.intellij.ide.DataManager;
 import com.intellij.openapi.actionSystem.CommonDataKeys;
+import com.intellij.openapi.actionSystem.DataContext;
 import com.intellij.openapi.project.Project;
 import com.intellij.openapi.project.ProjectManager;
-import com.intellij.psi.JavaTokenType;
-import com.intellij.psi.PsiAssignmentExpression;
-import com.intellij.psi.PsiBinaryExpression;
-import com.intellij.psi.PsiExpression;
+import com.intellij.openapi.util.AsyncResult;
+import com.intellij.psi.*;
 import com.intellij.psi.tree.IElementType;
+import com.intellij.refactoring.JavaRefactoringActionHandlerFactory;
+import com.intellij.refactoring.RefactoringActionHandler;
 import com.intellij.refactoring.util.RefactoringUtil;
+import com.intellij.util.Consumer;
+import org.jetbrains.annotations.Nls;
+import org.jetbrains.annotations.NotNull;
 
 import javax.swing.*;
 import javax.swing.event.ChangeEvent;
@@ -64,6 +68,40 @@ public class DataFlowInspection extends DataFlowInspectionBase {
     return RefactoringUtil.getParentStatement(expression, false) == null ? null : new AddAssertStatementFix(binary);
   }
 
+  @Override
+  protected LocalQuickFixOnPsiElement createIntroduceVariableFix(final PsiExpression expression) {
+    return new LocalQuickFixOnPsiElement(expression) {
+      @NotNull
+      @Override
+      public String getText() {
+        return "Introduce Local Variable";
+      }
+
+      @Override
+      public void invoke(@NotNull final Project project,
+                         @NotNull PsiFile file,
+                         @NotNull final PsiElement startElement,
+                         @NotNull PsiElement endElement) {
+        final RefactoringActionHandler handler = JavaRefactoringActionHandlerFactory.getInstance().createIntroduceVariableHandler();
+        final AsyncResult<DataContext> dataContextContainer = DataManager.getInstance().getDataContextFromFocus();
+        dataContextContainer.doWhenDone(new Consumer<DataContext>() {
+          @Override
+          public void consume(DataContext dataContext) {
+            handler.invoke(project, new PsiElement[]{startElement}, dataContext);
+          }
+        });
+
+      }
+
+      @Nls
+      @NotNull
+      @Override
+      public String getFamilyName() {
+        return getText();
+      }
+    };
+  }
+
   private class OptionsPanel extends JPanel {
     private final JCheckBox myIgnoreAssertions;
     private final JCheckBox myReportConstantReferences;

java/java-tests/testData/inspection/dataFlow/fixture/VolatileFieldNPEFixes.java

@@ -0,0 +1,10 @@
+import org.jetbrains.annotations.Nullable;
+class Test {
+  @Nullable volatile String x;
+
+  public void foo() {
+    if (x != null) {
+      System.out.println(<warning descr="Method invocation 'x.substring(1)' may produce 'java.lang.NullPointerException'">x.sub<caret>string(1)</warning>);
+    }
+  }
+}

java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java

@@ -376,6 +376,13 @@ public class DataFlowInspectionTest extends LightCodeInsightFixtureTestCase {
     assertEmpty(myFixture.filterAvailableIntentions("Simplify"));
   }
 
+  public void testVolatileFieldNPEFixes() {
+    doTest();
+    assertEmpty(myFixture.filterAvailableIntentions("Surround"));
+    assertEmpty(myFixture.filterAvailableIntentions("Assert"));
+    assertNotEmpty(myFixture.filterAvailableIntentions("Introduce Local Variable"));
+  }
+
   public void testAssertThat() {
     myFixture.addClass("package org.hamcrest; public class CoreMatchers { " +
                        "public static <T> Matcher<T> notNullValue() {}\n" +