nullable: move report null values passed to non annotated params under @Nullable suggestion

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

@@ -30,6 +30,7 @@ import com.intellij.codeInsight.NullableNotNullDialog;
 import com.intellij.codeInsight.NullableNotNullManager;
 import com.intellij.codeInsight.daemon.GroupNames;
 import com.intellij.codeInsight.daemon.impl.quickfix.SimplifyBooleanExpressionFix;
+import com.intellij.codeInsight.intention.impl.AddNullableAnnotationFix;
 import com.intellij.codeInspection.*;
 import com.intellij.codeInspection.dataFlow.instructions.*;
 import com.intellij.codeInspection.ex.BaseLocalInspectionTool;
@@ -182,6 +183,36 @@ public class DataFlowInspection extends BaseLocalInspectionTool {
     reportNullableAssignments(runner, holder);
     reportUnboxedNullables(runner, holder);
     reportNullableReturns(runner, holder);
+    reportNullableArgumentsPassedToNonAnnotated(runner, holder);
+  }
+
+  private static void reportNullableArgumentsPassedToNonAnnotated(StandardDataFlowRunner runner, ProblemsHolder holder) {
+    Set<PsiExpression> exprs = runner.getNullableArgumentsPassedToNonAnnotatedParam();
+    for (PsiExpression expr : exprs) {
+      final String text = isNullLiteralExpression(expr)
+                          ? "Passing <code>null</code> argument to non annotated parameter"
+                          : "Argument <code>#ref</code> #loc might be null but passed to non annotated parameter";
+      LocalQuickFix[] fixes = createNPEFixes(expr, expr);
+      final PsiElement parent = expr.getParent();
+      if (parent instanceof PsiExpressionList) {
+        final int idx = ArrayUtil.find(((PsiExpressionList)parent).getExpressions(), expr);
+        if (idx > -1) {
+          final PsiElement gParent = parent.getParent();
+          if (gParent instanceof PsiCallExpression) {
+            final PsiMethod psiMethod = ((PsiCallExpression)gParent).resolveMethod();
+            if (psiMethod != null && psiMethod.getManager().isInProject(psiMethod)) {
+              final PsiParameter[] parameters = psiMethod.getParameterList().getParameters();
+              if (idx < parameters.length) {
+                final AddNullableAnnotationFix addNullableAnnotationFix = new AddNullableAnnotationFix(parameters[idx]);
+                fixes = fixes == null ? new LocalQuickFix[]{addNullableAnnotationFix} : ArrayUtil.append(fixes, addNullableAnnotationFix);
+                holder.registerProblem(expr, text, fixes);
+              }
+            }
+          }
+        }
+      }
+      
+    }
   }
 
   private static void reportCallMayProduceNpe(ProblemsHolder holder, MethodCallInstruction mcInstruction) {
@@ -577,5 +608,10 @@ public class DataFlowInspection extends BaseLocalInspectionTool {
     protected void onPassingNullParameter(DataFlowRunner runner, PsiExpression arg) {
       ((StandardDataFlowRunner)runner).onPassingNullParameter(arg); // Parameters on stack are reverted.
     }
+
+    @Override
+    protected void onPassingNullParameterToNonAnnotated(DataFlowRunner runner, PsiExpression arg) {
+      ((StandardDataFlowRunner)runner).onPassingNullParameterToNonAnnotated(arg);
+    }
   }
 }

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

@@ -46,6 +46,7 @@ public class StandardDataFlowRunner extends AnnotationsAwareDataFlowRunner {
   private final Set<Instruction> myNPEInstructions = new HashSet<Instruction>();
   private final Set<Instruction> myCCEInstructions = new HashSet<Instruction>();
   private final Set<PsiExpression> myNullableArguments = new HashSet<PsiExpression>();
+  private final Set<PsiExpression> myNullableArgumentsPassedToNonAnnotatedParam = new HashSet<PsiExpression>();
   private final Set<PsiExpression> myNullableAssignments = new HashSet<PsiExpression>();
   private final Set<PsiReturnStatement> myNullableReturns = new HashSet<PsiReturnStatement>();
 
@@ -74,6 +75,7 @@ public class StandardDataFlowRunner extends AnnotationsAwareDataFlowRunner {
     myNPEInstructions.clear();
     myCCEInstructions.clear();
     myNullableArguments.clear();
+    myNullableArgumentsPassedToNonAnnotatedParam.clear();
     myNullableAssignments.clear();
     myNullableReturns.clear();
     myUnboxedNullables.clear();
@@ -109,6 +111,10 @@ public class StandardDataFlowRunner extends AnnotationsAwareDataFlowRunner {
     return myNullableArguments;
   }
 
+  public Set<PsiExpression> getNullableArgumentsPassedToNonAnnotatedParam() {
+    return myNullableArgumentsPassedToNonAnnotatedParam;
+  }
+
   @NotNull public Set<PsiExpression> getNullableAssignments() {
     return myNullableAssignments;
   }
@@ -128,6 +134,12 @@ public class StandardDataFlowRunner extends AnnotationsAwareDataFlowRunner {
     myNullableArguments.add(expr);
   }
 
+  public void onPassingNullParameterToNonAnnotated(PsiExpression expr) {
+    if (mySuggestNullableAnnotations) {
+      myNullableArgumentsPassedToNonAnnotatedParam.add(expr);
+    }
+  }
+
   public void onAssigningToNotNullableVariable(final PsiExpression expr) {
     myNullableAssignments.add(expr);
   }
@@ -147,6 +159,7 @@ public class StandardDataFlowRunner extends AnnotationsAwareDataFlowRunner {
            || !myCCEInstructions.isEmpty()
            || !getRedundantInstanceofs(this, visitor).isEmpty()
            || !myNullableArguments.isEmpty()
+           || !myNullableArgumentsPassedToNonAnnotatedParam.isEmpty()
            || !myNullableAssignments.isEmpty()
            || !myNullableReturns.isEmpty()
            || !myUnboxedNullables.isEmpty();

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

@@ -56,6 +56,25 @@ public class StandardInstructionVisitor extends InstructionVisitor {
       }
     }
   };
+  private final FactoryMap<MethodCallInstruction, boolean[]> myParametersNonAnnotated = new FactoryMap<MethodCallInstruction, boolean[]>() {
+    @Override
+    protected boolean[] create(MethodCallInstruction key) {
+      final PsiCallExpression callExpression = key.getCallExpression();
+      final PsiMethod callee = callExpression == null ? null : callExpression.resolveMethod();
+      if (callee != null) {
+        final PsiParameter[] params = callee.getParameterList().getParameters();
+        boolean[] result = new boolean[params.length];
+        final NullableNotNullManager notNullManager = NullableNotNullManager.getInstance(callee.getProject());
+        for (int i = 0; i < params.length; i++) {
+          result[i] = !notNullManager.isNotNull(params[i], false) && !notNullManager.isNullable(params[i], false);
+        }
+        return result;
+      }
+      else {
+        return ArrayUtil.EMPTY_BOOLEAN_ARRAY;
+      }
+    }
+  };
   private final FactoryMap<MethodCallInstruction, Boolean> myCalleeNullability = new FactoryMap<MethodCallInstruction, Boolean>() {
     @Override
     protected Boolean create(MethodCallInstruction key) {
@@ -161,14 +180,22 @@ public class StandardInstructionVisitor extends InstructionVisitor {
   public DfaInstructionState[] visitMethodCall(MethodCallInstruction instruction, DataFlowRunner runner, DfaMemoryState memState) {
     final PsiExpression[] args = instruction.getArgs();
     final boolean[] parametersNotNull = myParametersNotNull.get(instruction);
+    final boolean[] nonAnnotated = myParametersNonAnnotated.get(instruction);
     final DfaNotNullValue.Factory factory = runner.getFactory().getNotNullFactory();
     for (int i = 0; i < args.length; i++) {
       final DfaValue arg = memState.pop();
       final int revIdx = args.length - i - 1;
-      if (args.length <= parametersNotNull.length && revIdx < parametersNotNull.length && parametersNotNull[revIdx] && !memState.applyNotNull(arg)) {
-        onPassingNullParameter(runner, args[revIdx]);
-        if (arg instanceof DfaVariableValue) {
-          memState.setVarValue((DfaVariableValue)arg, factory.create(((DfaVariableValue)arg).getVariableType()));
+      if (args.length <= parametersNotNull.length && revIdx < parametersNotNull.length) {
+        if (parametersNotNull[revIdx]) {
+          if (!memState.applyNotNull(arg)) {
+            onPassingNullParameter(runner, args[revIdx]);
+            if (arg instanceof DfaVariableValue) {
+              memState.setVarValue((DfaVariableValue)arg, factory.create(((DfaVariableValue)arg).getVariableType()));
+            }
+          }
+        }
+        else if (nonAnnotated[revIdx] && !memState.checkNotNullable(arg)) {
+          onPassingNullParameterToNonAnnotated(runner, args[revIdx]);
         }
       }
     }
@@ -248,6 +275,7 @@ public class StandardInstructionVisitor extends InstructionVisitor {
   protected void onUnboxingNullable(MethodCallInstruction instruction, DataFlowRunner runner) {}
 
   protected void onPassingNullParameter(DataFlowRunner runner, PsiExpression arg) {}
+  protected void onPassingNullParameterToNonAnnotated(DataFlowRunner runner, PsiExpression arg) {}
 
   @Override
   public DfaInstructionState[] visitBinop(BinopInstruction instruction, DataFlowRunner runner, DfaMemoryState memState) {

java/java-impl/src/com/intellij/codeInspection/nullable/NullableStuffInspection.java

@@ -31,7 +31,6 @@ import com.intellij.openapi.project.Project;
 import com.intellij.openapi.project.ProjectManager;
 import com.intellij.openapi.util.text.StringUtil;
 import com.intellij.psi.*;
-import com.intellij.psi.codeStyle.CodeStyleSettingsManager;
 import com.intellij.psi.codeStyle.JavaCodeStyleManager;
 import com.intellij.psi.codeStyle.VariableKind;
 import com.intellij.psi.search.GlobalSearchScope;
@@ -73,42 +72,6 @@ public class NullableStuffInspection extends BaseLocalInspectionTool {
         checkNullableStuffForMethod(method, holder);
       }
 
-      @Override
-      public void visitMethodCallExpression(PsiMethodCallExpression expression) {
-        if (!PsiUtil.isLanguageLevel5OrHigher(expression) || !REPORT_NULLS_PASSED_TO_NON_ANNOTATED_METHOD) return;
-        final PsiMethod psiMethod = expression.resolveMethod();
-        if (psiMethod != null && (psiMethod.getManager().isInProject(psiMethod) || CodeStyleSettingsManager.getInstance().getCurrentSettings().USE_EXTERNAL_ANNOTATIONS)) {
-          final NullableNotNullManager nullableNotNullManager = NullableNotNullManager.getInstance(holder.getProject());
-          final PsiClass annotationsClass =
-            JavaPsiFacade.getInstance(holder.getProject()).findClass(nullableNotNullManager.getDefaultNullable(), psiMethod.getResolveScope());
-          if (annotationsClass == null) return;
-          final PsiParameterList parameterList = psiMethod.getParameterList();
-          final PsiParameter[] parameters = parameterList.getParameters();
-          final PsiExpression[] expressions = expression.getArgumentList().getExpressions();
-          for (int i = 0, expressionsLength = expressions.length; i < Math.min(expressionsLength, parameters.length); i++) {
-            PsiExpression psiExpression = expressions[i];
-            boolean nullablePassedAsParameter = false;
-            if (psiExpression instanceof PsiMethodCallExpression) {
-              final PsiMethod method = expression.resolveMethod();
-              nullablePassedAsParameter = nullableNotNullManager.isNullable(method, false);
-            } else if (psiExpression instanceof PsiReferenceExpression) {
-              final PsiElement resolve = ((PsiReferenceExpression)psiExpression).resolve();
-              if (resolve instanceof PsiModifierListOwner) {
-                nullablePassedAsParameter = nullableNotNullManager.isNullable((PsiModifierListOwner)resolve, false);
-              }
-            }
-            final PsiType exprType = psiExpression.getType();
-            if (exprType == PsiType.NULL || nullablePassedAsParameter) {
-              final PsiParameter parameter = parameters[i];
-              if (!NullableNotNullManager.isNullable(parameter) && !NullableNotNullManager.isNotNull(parameter)) {
-                holder.registerProblem(psiExpression, "Nullable value is passed to parameter which is not yet @Nullable", 
-                                       new MyAddNullableAnnotationFix(parameter));
-              }
-            }
-          }
-        }
-      }
-
       @Override public void visitField(PsiField field) {
         if (!PsiUtil.isLanguageLevel5OrHigher(field)) return;
         final PsiType type = field.getType();
@@ -450,7 +413,6 @@ public class NullableStuffInspection extends BaseLocalInspectionTool {
     private JCheckBox myNAMethodOverridesNN;
     private JPanel myPanel;
     private JCheckBox myReportNotAnnotatedGetter;
-    private JCheckBox myReportNullsPassedToNonAnnotatedParameter;
     private JButton myConfigureAnnotationsButton;
 
     private OptionsPanel() {
@@ -465,7 +427,6 @@ public class NullableStuffInspection extends BaseLocalInspectionTool {
       myNAMethodOverridesNN.addActionListener(actionListener);
       myNNParameterOverridesN.addActionListener(actionListener);
       myReportNotAnnotatedGetter.addActionListener(actionListener);
-      myReportNullsPassedToNonAnnotatedParameter.addActionListener(actionListener);
       myConfigureAnnotationsButton.addActionListener(new ActionListener() {
         @Override
         public void actionPerformed(ActionEvent e) {
@@ -482,7 +443,6 @@ public class NullableStuffInspection extends BaseLocalInspectionTool {
       myNNParameterOverridesN.setSelected(REPORT_NOTNULL_PARAMETER_OVERRIDES_NULLABLE);
       myNAMethodOverridesNN.setSelected(REPORT_NOT_ANNOTATED_METHOD_OVERRIDES_NOTNULL);
       myReportNotAnnotatedGetter.setSelected(REPORT_NOT_ANNOTATED_GETTER);
-      myReportNullsPassedToNonAnnotatedParameter.setSelected(REPORT_NULLS_PASSED_TO_NON_ANNOTATED_METHOD);
     }
 
     private void apply() {
@@ -490,7 +450,6 @@ public class NullableStuffInspection extends BaseLocalInspectionTool {
       REPORT_NOTNULL_PARAMETER_OVERRIDES_NULLABLE = myNNParameterOverridesN.isSelected();
       REPORT_NOT_ANNOTATED_GETTER = myReportNotAnnotatedGetter.isSelected();
       REPORT_ANNOTATION_NOT_PROPAGATED_TO_OVERRIDERS = REPORT_NOT_ANNOTATED_METHOD_OVERRIDES_NOTNULL;
-      REPORT_NULLS_PASSED_TO_NON_ANNOTATED_METHOD = myReportNullsPassedToNonAnnotatedParameter.isSelected();
     }
   }
 

java/java-impl/src/com/intellij/codeInspection/nullable/OptionsPanel.form

@@ -1,6 +1,6 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <form xmlns="http://www.intellij.com/uidesigner/form/" version="1" bind-to-class="com.intellij.codeInspection.nullable.NullableStuffInspection.OptionsPanel">
-  <grid id="cc1c9" binding="myPanel" layout-manager="GridLayoutManager" row-count="6" column-count="1" same-size-horizontally="false" same-size-vertically="false" hgap="-1" vgap="-1">
+  <grid id="cc1c9" binding="myPanel" layout-manager="GridLayoutManager" row-count="5" column-count="1" same-size-horizontally="false" same-size-vertically="false" hgap="-1" vgap="-1">
     <margin top="0" left="0" bottom="0" right="0"/>
     <constraints>
       <xy x="69" y="57" width="634" height="235"/>
@@ -10,7 +10,7 @@
     <children>
       <vspacer id="c3eef">
         <constraints>
-          <grid row="5" column="0" row-span="1" col-span="1" vsize-policy="6" hsize-policy="1" anchor="0" fill="2" indent="0" use-parent-layout="false"/>
+          <grid row="4" column="0" row-span="1" col-span="1" vsize-policy="6" hsize-policy="1" anchor="0" fill="2" indent="0" use-parent-layout="false"/>
         </constraints>
       </vspacer>
       <component id="2f304" class="javax.swing.JCheckBox" binding="myReportNotAnnotatedGetter">
@@ -21,14 +21,6 @@
           <text resource-bundle="messages/InspectionsBundle" key="inspection.nullable.problems.not.annotated.getters.for.annotated.fields"/>
         </properties>
       </component>
-      <component id="690c8" class="javax.swing.JCheckBox" binding="myReportNullsPassedToNonAnnotatedParameter" default-binding="true">
-        <constraints>
-          <grid row="3" column="0" row-span="1" col-span="1" vsize-policy="0" hsize-policy="3" anchor="8" fill="0" indent="0" use-parent-layout="false"/>
-        </constraints>
-        <properties>
-          <text resource-bundle="messages/InspectionsBundle" key="inspection.nullable.problems.non.annotated.passed.null"/>
-        </properties>
-      </component>
       <component id="f4e37" class="javax.swing.JCheckBox" binding="myNNParameterOverridesN">
         <constraints>
           <grid row="0" column="0" row-span="1" col-span="1" vsize-policy="0" hsize-policy="3" anchor="8" fill="0" indent="0" use-parent-layout="false"/>
@@ -47,7 +39,7 @@
       </component>
       <component id="ef852" class="javax.swing.JButton" binding="myConfigureAnnotationsButton" default-binding="true">
         <constraints>
-          <grid row="4" column="0" row-span="1" col-span="1" vsize-policy="0" hsize-policy="3" anchor="8" fill="0" indent="0" use-parent-layout="false"/>
+          <grid row="3" column="0" row-span="1" col-span="1" vsize-policy="0" hsize-policy="3" anchor="8" fill="0" indent="0" use-parent-layout="false"/>
         </constraints>
         <properties>
           <text resource-bundle="messages/InspectionsBundle" key="configure.annotations.option"/>

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

@@ -9,7 +9,7 @@ public class BrokenAlignment {
       System.out.println(data.getText().hashCode());
     }
 
-    data = new Data(null, null);
+    data = new Data(null, <warning descr="Passing 'null' argument to non annotated parameter">null</warning>);
     System.out.println(<warning descr="Method invocation 'data.getText().hashCode()' may produce 'java.lang.NullPointerException'">data.getText().hashCode()</warning>);
 
     if (data.inner() != null) {
@@ -19,7 +19,7 @@ public class BrokenAlignment {
         System.out.println(data.inner().hashCode());
       }
 
-      data = new Data(null, null);
+      data = new Data(null, <warning descr="Passing 'null' argument to non annotated parameter">null</warning>);
       System.out.println(<warning descr="Method invocation 'data.inner().hashCode()' may produce 'java.lang.NullPointerException'">data.inner().hashCode()</warning>);
     }
   }

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

@@ -6,7 +6,7 @@ public class BrokenAlignment {
       System.out.println(data.text.hashCode());
     }
 
-    data = new Data(null, null);
+    data = new Data(null, <warning descr="Passing 'null' argument to non annotated parameter">null</warning>);
     System.out.println(<warning descr="Method invocation 'data.text.hashCode()' may produce 'java.lang.NullPointerException'">data.text.hashCode()</warning>);
 
     if (data.inner != null) {
@@ -16,7 +16,7 @@ public class BrokenAlignment {
         System.out.println(data.inner.hashCode());
       }
 
-      data = new Data(null, null);
+      data = new Data(null, <warning descr="Passing 'null' argument to non annotated parameter">null</warning>);
       System.out.println(<warning descr="Method invocation 'data.inner.hashCode()' may produce 'java.lang.NullPointerException'">data.inner.hashCode()</warning>);
     }
   }

platform/platform-resources-en/src/messages/InspectionsBundle.properties

@@ -45,7 +45,7 @@ inspection.annotate.method.quickfix.name=Annotate method as ''@{0}''
 
 #dataflow
 inspection.data.flow.display.name=Constant conditions \\& exceptions
-inspection.data.flow.nullable.quickfix.option=<html><body>Suggest @Nullable annotation for methods that may possibly return null</body></html>
+inspection.data.flow.nullable.quickfix.option=<html><body>Suggest @Nullable annotation for methods that may possibly return null and <br>report nullable values passed to non-annotated parameters</body></html>
 inspection.data.flow.true.asserts.option=<html><body>Don't report assert statements with condition statically proven to be always <code>true</code></body></html>
 inspection.data.flow.redundant.instanceof.quickfix=Replace with != null
 inspection.data.flow.simplify.boolean.expression.quickfix=Simplify Boolean Expression
@@ -114,7 +114,6 @@ inspection.nullable.problems.display.name=@NotNull/@Nullable problems
 inspection.nullable.problems.method.overrides.notnull.option=<html>report @NotNull &parameter overrides @Nullable and <br>@Nullable method overrides @NotNull</html>
 inspection.nullable.problems.method.overrides.option=report non-&annotated parameter or method overrides @NotNull
 inspection.nullable.problems.not.annotated.getters.for.annotated.fields=report non-annotated &setter parameter or getter of annotated fields
-inspection.nullable.problems.non.annotated.passed.null=report &null passed to non-annotated parameter
 inspection.nullable.problems.annotation.not.propagated=report @NotNull not propagated to ove&rridden methods
 
 #problem descriptor messages