IDEA-170032 Replace with foreach is not proposed in alt-enter

Now both options (when unchecked) only disable warning and batch-mode replacement, but the action is still available as an intention.

java/java-impl/src/com/intellij/codeInspection/streamMigration/BaseStreamApiMigration.java

@@ -28,9 +28,11 @@ import org.jetbrains.annotations.NotNull;
  * @author Tagir Valeev
  */
 abstract class BaseStreamApiMigration {
+  private final boolean myShouldWarn;
   private final String myReplacement;
 
-  protected BaseStreamApiMigration(String replacement) {
+  protected BaseStreamApiMigration(boolean shouldWarn, String replacement) {
+    myShouldWarn = shouldWarn;
     myReplacement = replacement;
   }
 
@@ -40,6 +42,10 @@ abstract class BaseStreamApiMigration {
 
   abstract PsiElement migrate(@NotNull Project project, @NotNull PsiStatement body, @NotNull TerminalBlock tb);
 
+  public boolean isShouldWarn() {
+    return myShouldWarn;
+  }
+
   static PsiElement replaceWithNumericAddition(PsiLoopStatement loopStatement,
                                                PsiVariable var,
                                                String streamText,

java/java-impl/src/com/intellij/codeInspection/streamMigration/CollectMigration.java

@@ -63,8 +63,8 @@ class CollectMigration extends BaseStreamApiMigration {
     "java.util.TreeSet", ".distinct().sorted()"
   ).toMap();
 
-  protected CollectMigration(String methodName) {
-    super(methodName);
+  protected CollectMigration(boolean shouldWarn, String methodName) {
+    super(shouldWarn, methodName);
   }
 
   @Nullable

java/java-impl/src/com/intellij/codeInspection/streamMigration/CountMigration.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2000-2016 JetBrains s.r.o.
+ * Copyright 2000-2017 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.
@@ -24,7 +24,9 @@ import org.jetbrains.annotations.NotNull;
  */
 class CountMigration extends BaseStreamApiMigration {
 
-  CountMigration() {super("count()");}
+  CountMigration(boolean shouldWarn) {
+    super(shouldWarn, "count()");
+  }
 
   @Override
   PsiElement migrate(@NotNull Project project, @NotNull PsiStatement body, @NotNull TerminalBlock tb) {

java/java-impl/src/com/intellij/codeInspection/streamMigration/FindFirstMigration.java

@@ -31,7 +31,7 @@ import static com.intellij.util.ObjectUtils.tryCast;
  * @author Tagir Valeev
  */
 class FindFirstMigration extends BaseStreamApiMigration {
-  FindFirstMigration() {super("findFirst()");}
+  FindFirstMigration(boolean shouldWarn) {super(shouldWarn, "findFirst()");}
 
   @Override
   PsiElement migrate(@NotNull Project project, @NotNull PsiStatement body, @NotNull TerminalBlock tb) {

java/java-impl/src/com/intellij/codeInspection/streamMigration/ForEachMigration.java

@@ -36,8 +36,8 @@ import static com.intellij.codeInspection.streamMigration.CollectMigration.getAd
 class ForEachMigration extends BaseStreamApiMigration {
   private static final Logger LOG = Logger.getInstance(ForEachMigration.class);
 
-  protected ForEachMigration(String forEachMethodName) {
-    super(forEachMethodName);
+  protected ForEachMigration(boolean shouldWarn, String forEachMethodName) {
+    super(shouldWarn, forEachMethodName);
   }
 
   @Nullable

java/java-impl/src/com/intellij/codeInspection/streamMigration/MatchMigration.java

@@ -31,8 +31,8 @@ import org.jetbrains.annotations.NotNull;
 class MatchMigration extends BaseStreamApiMigration {
   private static final Logger LOG = Logger.getInstance("#" + MatchMigration.class.getName());
 
-  public MatchMigration(String methodName) {
-    super(methodName+"()");
+  public MatchMigration(boolean shouldWarn, String methodName) {
+    super(shouldWarn, methodName+"()");
   }
 
   @Override

java/java-impl/src/com/intellij/codeInspection/streamMigration/StreamApiMigrationInspection.java

@@ -21,6 +21,7 @@ import com.intellij.codeInsight.daemon.GroupNames;
 import com.intellij.codeInsight.daemon.impl.analysis.HighlightControlFlowUtil;
 import com.intellij.codeInspection.BaseJavaBatchLocalInspectionTool;
 import com.intellij.codeInspection.LambdaCanBeMethodReferenceInspection;
+import com.intellij.codeInspection.ProblemHighlightType;
 import com.intellij.codeInspection.ProblemsHolder;
 import com.intellij.codeInspection.ui.MultipleCheckboxOptionsPanel;
 import com.intellij.openapi.diagnostic.Logger;
@@ -64,8 +65,8 @@ public class StreamApiMigrationInspection extends BaseJavaBatchLocalInspectionTo
   @Override
   public JComponent createOptionsPanel() {
     MultipleCheckboxOptionsPanel panel = new MultipleCheckboxOptionsPanel(this);
-    panel.addCheckbox("Suggest to replace with forEach or forEachOrdered", "SUGGEST_FOREACH");
-    panel.addCheckbox("Replace trivial foreach statements", "REPLACE_TRIVIAL_FOREACH");
+    panel.addCheckbox("Warn if only 'forEach' replacement is available", "SUGGEST_FOREACH");
+    panel.addCheckbox("Warn if the loop is trivial", "REPLACE_TRIVIAL_FOREACH");
     return panel;
   }
 
@@ -407,13 +408,16 @@ public class StreamApiMigrationInspection extends BaseJavaBatchLocalInspectionTo
       TerminalBlock tb = TerminalBlock.from(source, body);
 
       BaseStreamApiMigration migration = findMigration(statement, body, tb);
-      if(migration != null) {
+      if (migration != null && (myIsOnTheFly || migration.isShouldWarn())) {
         MigrateToStreamFix[] fixes = {new MigrateToStreamFix(migration)};
-        if (migration instanceof ForEachMigration && tb.hasOperations()) { //for .stream()
-          fixes = ArrayUtil.append(fixes, new MigrateToStreamFix(new ForEachMigration("forEachOrdered")));
+        if (migration instanceof ForEachMigration && !(tb.getLastOperation() instanceof CollectionStream)) { //for .stream()
+          fixes = ArrayUtil.append(fixes, new MigrateToStreamFix(new ForEachMigration(migration.isShouldWarn(), "forEachOrdered")));
         }
-        myHolder.registerProblem(statement, getRange(statement).shiftRight(-statement.getTextOffset()),
-                                 "Can be replaced with '" + migration.getReplacement() + "' call", fixes);
+        ProblemHighlightType highlightType =
+          migration.isShouldWarn() ? ProblemHighlightType.GENERIC_ERROR_OR_WARNING : ProblemHighlightType.INFORMATION;
+        myHolder.registerProblem(statement, "Can be replaced with '" + migration.getReplacement() + "' call",
+                                 highlightType, getRange(migration.isShouldWarn(), statement).shiftRight(-statement.getTextOffset()),
+                                 fixes);
       }
     }
 
@@ -437,7 +441,7 @@ public class StreamApiMigrationInspection extends BaseJavaBatchLocalInspectionTo
         .remove(variable -> isVariableSuitableForStream(variable, loop, tb)).toList();
 
       if (isCountOperation(nonFinalVariables, tb)) {
-        return new CountMigration();
+        return new CountMigration(true);
       }
       if (nonFinalVariables.isEmpty()) {
         CollectMigration.CollectTerminal terminal = CollectMigration.extractCollectTerminal(tb);
@@ -446,31 +450,30 @@ public class StreamApiMigrationInspection extends BaseJavaBatchLocalInspectionTo
           // Don't suggest to convert the loop which can be trivially replaced via addAll:
           // this is covered by UseBulkOperationInspection and ManualArrayToCollectionCopyInspection
           if(addAll) return null;
-          if (!REPLACE_TRIVIAL_FOREACH &&
-              !tb.hasOperations() &&
-              !(tb.getLastOperation() instanceof BufferedReaderLines) &&
-              terminal.isTrivial()) {
-            return null;
-          }
-          return new CollectMigration(terminal.getMethodName());
+          boolean shouldWarn = REPLACE_TRIVIAL_FOREACH ||
+                               tb.hasOperations() ||
+                               tb.getLastOperation() instanceof BufferedReaderLines ||
+                               !terminal.isTrivial();
+          return new CollectMigration(shouldWarn, terminal.getMethodName());
         }
       }
       if (tb.getCountExpression() != null || tb.isEmpty()) return null;
       if (nonFinalVariables.isEmpty() && extractArray(tb) != null) {
-        return new ToArrayMigration();
+        return new ToArrayMigration(true);
       }
       if (getAccumulatedVariable(tb, nonFinalVariables) != null) {
-        return new SumMigration();
+        return new SumMigration(true);
       }
       Collection<PsiStatement> exitPoints = tb.findExitPoints(controlFlow);
       if (exitPoints == null) return null;
-      if (SUGGEST_FOREACH && exitPoints.isEmpty() && nonFinalVariables.isEmpty()) {
-        boolean nonTrivial = tb.hasOperations() || ForEachMigration.tryExtractMapExpression(tb) != null || !isTrivial(tb);
-        // do not replace for(T e : arr) {} with Arrays.stream(arr).forEach(e -> {}) even if REPLACE_TRIVIAL_FOREACH is set
-        if (!nonTrivial && (!REPLACE_TRIVIAL_FOREACH || tb.getLastOperation() instanceof ArrayStream)) return null;
-        return new ForEachMigration("forEach");
+      if (exitPoints.isEmpty() && nonFinalVariables.isEmpty()) {
+        boolean shouldWarn = SUGGEST_FOREACH &&
+                             (REPLACE_TRIVIAL_FOREACH ||
+                              tb.hasOperations() ||
+                              ForEachMigration.tryExtractMapExpression(tb) != null ||
+                              !isTrivial(tb));
+        return new ForEachMigration(shouldWarn, "forEach");
       }
-      if (!tb.hasOperations() && !REPLACE_TRIVIAL_FOREACH) return null;
       if (nonFinalVariables.isEmpty() && tb.getSingleStatement() instanceof PsiReturnStatement) {
         return findMigrationForReturn(loop, tb);
       }
@@ -494,11 +497,12 @@ public class StreamApiMigrationInspection extends BaseJavaBatchLocalInspectionTo
 
     @Nullable
     private BaseStreamApiMigration findMigrationForBreak(TerminalBlock tb, List<PsiVariable> nonFinalVariables, PsiStatement statement) {
+      boolean shouldWarn = REPLACE_TRIVIAL_FOREACH || tb.hasOperations();
       if (ReferencesSearch.search(tb.getVariable(), new LocalSearchScope(statement)).findFirst() == null) {
-        return new MatchMigration("anyMatch");
+        return new MatchMigration(shouldWarn, "anyMatch");
       }
       if (nonFinalVariables.isEmpty() && statement instanceof PsiExpressionStatement) {
-        return new FindFirstMigration();
+        return new FindFirstMigration(shouldWarn);
       }
       if (nonFinalVariables.size() == 1) {
         PsiAssignmentExpression assignment = ExpressionUtils.getAssignment(statement);
@@ -510,13 +514,14 @@ public class StreamApiMigrationInspection extends BaseJavaBatchLocalInspectionTo
         PsiExpression rValue = assignment.getRExpression();
         if(rValue == null || VariableAccessUtils.variableIsUsed(var, rValue)) return null;
         if(tb.getVariable().getType() instanceof PsiPrimitiveType && !ExpressionUtils.isReferenceTo(rValue, tb.getVariable())) return null;
-        return new FindFirstMigration();
+        return new FindFirstMigration(shouldWarn);
       }
       return null;
     }
 
     @Nullable
     private BaseStreamApiMigration findMigrationForReturn(PsiLoopStatement statement, TerminalBlock tb) {
+      boolean shouldWarn = REPLACE_TRIVIAL_FOREACH || tb.hasOperations();
       PsiReturnStatement returnStatement = (PsiReturnStatement)tb.getSingleStatement();
       PsiExpression value = returnStatement.getReturnValue();
       PsiReturnStatement nextReturnStatement = getNextReturnStatement(statement);
@@ -536,7 +541,7 @@ public class StreamApiMigrationInspection extends BaseJavaBatchLocalInspectionTo
         }
         if(nextReturnStatement.getParent() == statement.getParent() ||
            ExpressionUtils.isLiteral(nextReturnStatement.getReturnValue(), !foundResult)) {
-          return new MatchMigration(methodName);
+          return new MatchMigration(shouldWarn, methodName);
         }
       }
       if (!VariableAccessUtils.variableIsUsed(tb.getVariable(), value)) {
@@ -544,18 +549,19 @@ public class StreamApiMigrationInspection extends BaseJavaBatchLocalInspectionTo
             (tb.getLastOperation() instanceof FilterOp && tb.operations().count() == 2)) {
           return null;
         }
-        return new MatchMigration("anyMatch");
+        return new MatchMigration(shouldWarn, "anyMatch");
       }
       if(nextReturnStatement != null && ExpressionUtils.isSimpleExpression(nextReturnStatement.getReturnValue())
          && (!(tb.getVariable().getType() instanceof PsiPrimitiveType) || ExpressionUtils.isReferenceTo(value, tb.getVariable()))) {
-        return new FindFirstMigration();
+        return new FindFirstMigration(shouldWarn);
       }
       return null;
     }
 
     @NotNull
-    private TextRange getRange(PsiLoopStatement statement) {
-      boolean wholeStatement = myIsOnTheFly && InspectionProjectProfileManager.isInformationLevel(getShortName(), statement);
+    private TextRange getRange(boolean shouldWarn, PsiLoopStatement statement) {
+      boolean wholeStatement =
+        myIsOnTheFly && (!shouldWarn || InspectionProjectProfileManager.isInformationLevel(getShortName(), statement));
       if(statement instanceof PsiForeachStatement) {
         PsiJavaToken rParenth = ((PsiForeachStatement)statement).getRParenth();
         if (wholeStatement && rParenth != null) {

java/java-impl/src/com/intellij/codeInspection/streamMigration/SumMigration.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2000-2016 JetBrains s.r.o.
+ * Copyright 2000-2017 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.
@@ -27,7 +27,7 @@ import org.jetbrains.annotations.NotNull;
  */
 class SumMigration extends BaseStreamApiMigration {
 
-  SumMigration() {super("sum()");}
+  SumMigration(boolean shouldWarn) {super(shouldWarn, "sum()");}
 
   @Override
   PsiElement migrate(@NotNull Project project, @NotNull PsiStatement body, @NotNull TerminalBlock tb) {

java/java-impl/src/com/intellij/codeInspection/streamMigration/ToArrayMigration.java

@@ -30,8 +30,8 @@ import static com.intellij.util.ObjectUtils.tryCast;
  * @author Tagir Valeev
  */
 public class ToArrayMigration extends BaseStreamApiMigration {
-  protected ToArrayMigration() {
-    super("toArray");
+  protected ToArrayMigration(boolean shouldWarn) {
+    super(shouldWarn, "toArray");
   }
 
   @Override

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterDisableForArrays.java

@@ -0,0 +1,11 @@
+// "Replace with forEach" "INFORMATION"
+import java.util.*;
+
+class A {
+  void fun(List<String>... lists) {
+      Arrays.stream(lists).forEach(list -> list.add(""));
+  }
+
+
+
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterIncompleteNewCall.java

@@ -0,0 +1,13 @@
+// "Replace with forEach" "INFORMATION"
+
+import java.io.FileInputStream;
+import java.io.FileNotFoundException;
+import java.util.*;
+
+class A {
+  public void foo(final Set<String> strings) throws FileNotFoundException {
+      strings.forEach(s -> new FileInputStream());
+
+  }
+
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterTooLazyCalculation.java

@@ -0,0 +1,20 @@
+// "Replace with forEach" "INFORMATION"
+import java.util.*;
+
+class A {
+  public static void main(String[] args) {
+    List<String> names = Arrays.asList("Bob", "Alice", "Bob", "Carol");
+    Set<String> uniqNames = new HashSet<>(names.size());
+      names.forEach(name -> uniqNames.add(makeNameUnique(name, uniqNames)));
+    uniqNames.forEach(System.out::println);
+  }
+
+  private static String makeNameUnique(final String name, final Set<String> uniqNames) {
+    if (uniqNames.contains(name)) {
+      return name + "1";
+    }
+    return name;
+  }
+
+
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeDisableForArrays.java

@@ -1,4 +1,4 @@
-// "Replace with forEach" "false"
+// "Replace with forEach" "INFORMATION"
 import java.util.*;
 
 class A {

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeIncompleteNewCall.java

@@ -1,4 +1,4 @@
-// "Replace with forEach" "false"
+// "Replace with forEach" "INFORMATION"
 
 import java.io.FileInputStream;
 import java.io.FileNotFoundException;

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeTooLazyCalculation.java

@@ -1,4 +1,4 @@
-// "Replace with forEach" "false"
+// "Replace with forEach" "INFORMATION"
 import java.util.*;
 
 class A {