SimplifyForEachInspection: compilation fix added for non-final var

Adds another alternative to IDEA-177104
2026-04-19 21:11:28 +07:00 · 2017-09-07 15:45:35 +07:00
parent bead9e9440
commit a2dfaa56dc
8 changed files with 221 additions and 101 deletions
--- a/java/java-impl/src/com/intellij/codeInspection/streamMigration/MigrateToStreamFix.java
+++ b/java/java-impl/src/com/intellij/codeInspection/streamMigration/MigrateToStreamFix.java
@@ -21,7 +21,9 @@ import com.intellij.codeInspection.ProblemDescriptor;
 import com.intellij.codeInspection.SimplifyStreamApiCallChainsInspection;
 import com.intellij.codeInspection.streamMigration.StreamApiMigrationInspection.StreamSource;
 import com.intellij.openapi.project.Project;
-import com.intellij.psi.*;
+import com.intellij.psi.PsiElement;
+import com.intellij.psi.PsiLoopStatement;
+import com.intellij.psi.PsiStatement;
 import com.intellij.psi.codeStyle.CodeStyleManager;
 import com.intellij.psi.codeStyle.JavaCodeStyleManager;
 import com.intellij.psi.impl.PsiDiamondTypeUtil;
@@ -59,37 +61,16 @@ class MigrateToStreamFix implements LocalQuickFix {

  @Override
  public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) {
-    PsiElement element = descriptor.getPsiElement();
-    if (element instanceof PsiLoopStatement) {
-      PsiLoopStatement loopStatement = (PsiLoopStatement)element;
-      StreamSource source = StreamSource.tryCreate(loopStatement);
-      PsiStatement body = loopStatement.getBody();
-      if(body == null || source == null) return;
-      TerminalBlock tb = TerminalBlock.from(source, body);
-      migrate(project, body, tb);
-    } else if(element instanceof PsiExpressionStatement) {
-      PsiMethodCallExpression call = tryCast(((PsiExpressionStatement)element).getExpression(), PsiMethodCallExpression.class);
-      if(call == null) return;
-
-      PsiLambdaExpression lambda = SimplifyForEachInspection.extractLambdaFromForEach(call);
-      if (lambda == null) return;
-      PsiElement lambdaBody = lambda.getBody();
-      SimplifyForEachInspection.ExistingStreamSource
-        source = SimplifyForEachInspection.ExistingStreamSource.extractSource(call, lambda);
-      if(source == null) return;
-      TerminalBlock terminalBlock = SimplifyForEachInspection.extractTerminalBlock(lambdaBody, source);
-      if (terminalBlock == null) return;
-
-      migrate(project, lambdaBody, terminalBlock);
-    }
-  }
-
-  private void migrate(@NotNull Project project, PsiElement block, TerminalBlock tb) {
-    PsiElement result = myMigration.migrate(project, block, tb);
-    if(result != null) {
-      tb.operations().forEach(StreamApiMigrationInspection.Operation::cleanUp);
-      simplifyAndFormat(project, result);
-    }
+    PsiLoopStatement loopStatement = tryCast(descriptor.getPsiElement(), PsiLoopStatement.class);
+    if (loopStatement == null) return;
+    StreamSource source = StreamSource.tryCreate(loopStatement);
+    PsiStatement body = loopStatement.getBody();
+    if(body == null || source == null) return;
+    TerminalBlock tb = TerminalBlock.from(source, body);
+    PsiElement result = myMigration.migrate(project, body, tb);
+    if (result == null) return;
+    tb.operations().forEach(StreamApiMigrationInspection.Operation::cleanUp);
+    simplifyAndFormat(project, result);
  }

  static void simplifyAndFormat(@NotNull Project project, PsiElement result) {
--- a/java/java-impl/src/com/intellij/codeInspection/streamMigration/SimplifyForEachInspection.java
+++ b/java/java-impl/src/com/intellij/codeInspection/streamMigration/SimplifyForEachInspection.java
@@ -16,16 +16,21 @@
 package com.intellij.codeInspection.streamMigration;

 import com.intellij.codeInsight.daemon.GroupNames;
-import com.intellij.codeInspection.BaseJavaBatchLocalInspectionTool;
-import com.intellij.codeInspection.ProblemsHolder;
+import com.intellij.codeInsight.intention.HighPriorityAction;
+import com.intellij.codeInsight.intention.PsiElementBaseIntentionAction;
+import com.intellij.codeInspection.*;
 import com.intellij.lang.java.JavaLanguage;
+import com.intellij.openapi.editor.Editor;
+import com.intellij.openapi.project.Project;
 import com.intellij.openapi.roots.FileIndexFacade;
 import com.intellij.openapi.util.TextRange;
 import com.intellij.openapi.vfs.VirtualFile;
 import com.intellij.psi.*;
 import com.intellij.psi.impl.light.LightElement;
 import com.intellij.psi.util.InheritanceUtil;
+import com.intellij.psi.util.PsiTreeUtil;
 import com.intellij.psi.util.PsiUtil;
+import com.intellij.util.IncorrectOperationException;
 import com.siyeh.ig.callMatcher.CallMatcher;
 import org.jetbrains.annotations.Nls;
 import org.jetbrains.annotations.NotNull;
@@ -56,13 +61,6 @@ public class SimplifyForEachInspection extends BaseJavaBatchLocalInspectionTool
    return "forEach call can be simplified";
  }

-
-  @NotNull
-  @Override
-  public String getShortName() {
-    return "SimplifyForEach";
-  }
-
  @NotNull
  @Override
  public PsiElementVisitor buildVisitor(@NotNull ProblemsHolder holder, boolean isOnTheFly) {
@@ -75,37 +73,15 @@ public class SimplifyForEachInspection extends BaseJavaBatchLocalInspectionTool
    return new JavaElementVisitor() {
      @Override
      public void visitMethodCallExpression(PsiMethodCallExpression call) {
-        PsiLambdaExpression lambda = extractLambdaFromForEach(call);
-        if (lambda == null) return;
-        PsiElement lambdaBody = lambda.getBody();
-        ExistingStreamSource
-          source = ExistingStreamSource.extractSource(call, lambda);
-        if (source == null) return;
-        TerminalBlock terminalBlock = extractTerminalBlock(lambdaBody, source);
-        if (terminalBlock == null) return;
-
-        PsiStatement mainStatement = source.getMainStatement();
-        BaseStreamApiMigration migration =
-          StreamApiMigrationInspection.findMigration(mainStatement, lambdaBody, terminalBlock, holder, true, true);
-        boolean opCountChanged = terminalBlock.getOperationCount() > 1;
-        boolean lastOpChanged = !(migration instanceof ForEachMigration);
+        SimplifyForEachContext context = SimplifyForEachContext.from(call);
+        if (context == null) return;
+        boolean opCountChanged = context.myTerminalBlock.getOperationCount() > 1;
+        boolean lastOpChanged = !(context.myMigration instanceof ForEachMigration);
        if (opCountChanged || lastOpChanged) {
-          String customMessage;
-          if (opCountChanged && !lastOpChanged) {
-            customMessage = "Extract intermediate operations";
-            if (STREAM_FOREACH_ORDERED.test(call)) {
-              migration = new ForEachMigration(migration.isShouldWarn(), "forEachOrdered");
-            }
-          }
-          else {
-            customMessage = null;
-          }
-          if (migration != null) {
-            migration.setShouldWarn(true);
-          }
-
-          StreamApiMigrationInspection
-            .offerMigration(mainStatement, terminalBlock, migration, m -> getRange(call).shiftRight(-call.getTextOffset()), customMessage, false, holder);
+          String customMessage = lastOpChanged ? "Replace with " + context.myMigration.getReplacement() : "Extract intermediate operations";
+          ProblemHighlightType highlightType = ProblemHighlightType.GENERIC_ERROR_OR_WARNING;
+          holder.registerProblem(context.myMainStatement, customMessage, highlightType, getRange(call).shiftRight(-call.getTextOffset()),
+                                 new SimplifyForEachFix(customMessage));
        }
      }
    };
@@ -190,4 +166,137 @@ public class SimplifyForEachInspection extends BaseJavaBatchLocalInspectionTool
      return new ExistingStreamSource(parent, parameter, qualifier, isCollectionForEach);
    }
  }
+
+  static class SimplifyForEachContext {
+    @NotNull private final TerminalBlock myTerminalBlock;
+    @NotNull private final PsiStatement myMainStatement;
+    @NotNull private final BaseStreamApiMigration myMigration;
+    @NotNull private final PsiElement myLambdaBody;
+
+    private SimplifyForEachContext(@NotNull TerminalBlock terminalBlock,
+                                   @NotNull PsiStatement mainStatement,
+                                   @NotNull PsiElement body,
+                                   @NotNull BaseStreamApiMigration migration) {
+      myTerminalBlock = terminalBlock;
+      myMainStatement = mainStatement;
+      myLambdaBody = body;
+      myMigration = migration;
+    }
+
+    public PsiElement migrate() {
+      PsiElement result = myMigration.migrate(myMainStatement.getProject(), myLambdaBody, myTerminalBlock);
+      if (result != null) {
+        myTerminalBlock.operations().forEach(StreamApiMigrationInspection.Operation::cleanUp);
+      }
+      return result;
+    }
+
+    static SimplifyForEachContext from(PsiMethodCallExpression call) {
+      PsiLambdaExpression lambda = extractLambdaFromForEach(call);
+      if (lambda == null) return null;
+      PsiElement lambdaBody = lambda.getBody();
+      ExistingStreamSource
+        source = ExistingStreamSource.extractSource(call, lambda);
+      if (source == null) return null;
+      TerminalBlock terminalBlock = extractTerminalBlock(lambdaBody, source);
+      if (terminalBlock == null) return null;
+
+      PsiStatement mainStatement = source.getMainStatement();
+      BaseStreamApiMigration migration =
+        StreamApiMigrationInspection.findMigration(mainStatement, lambdaBody, terminalBlock, true, true);
+      if (migration instanceof ForEachMigration && STREAM_FOREACH_ORDERED.test(call)) {
+        migration = new ForEachMigration(migration.isShouldWarn(), "forEachOrdered");
+      }
+      return migration == null ? null : new SimplifyForEachContext(terminalBlock, mainStatement, lambdaBody, migration);
+    }
+  }
+
+  public static class SimplifyForEachFix implements LocalQuickFix {
+    @NotNull private final String myCustomName;
+
+    protected SimplifyForEachFix(@NotNull String customName) {
+      myCustomName = customName;
+    }
+
+    @Nls
+    @NotNull
+    @Override
+    public String getName() {
+      return myCustomName;
+    }
+
+    @SuppressWarnings("DialogTitleCapitalization")
+    @NotNull
+    @Override
+    public String getFamilyName() {
+      return "Simplify forEach lambda";
+    }
+
+    @Override
+    public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) {
+      PsiExpressionStatement statement = tryCast(descriptor.getPsiElement(), PsiExpressionStatement.class);
+      if (statement == null) return;
+      PsiMethodCallExpression call = tryCast(statement.getExpression(), PsiMethodCallExpression.class);
+      if (call == null) return;
+      SimplifyForEachInspection.SimplifyForEachContext simplifyForEachContext = SimplifyForEachInspection.SimplifyForEachContext.from(call);
+      if (simplifyForEachContext == null) return;
+      PsiElement result = simplifyForEachContext.migrate();
+      if (result == null) return;
+      MigrateToStreamFix.simplifyAndFormat(project, result);
+    }
+  }
+
+  public static class ForEachNonFinalFix extends PsiElementBaseIntentionAction implements HighPriorityAction {
+    private final PsiElement myContext;
+
+    public ForEachNonFinalFix(PsiElement context) {
+      SimplifyForEachContext simplifyContext = findMigration(context);
+      if (simplifyContext == null) {
+        myContext = null;
+      }
+      else {
+        myContext = context;
+        setText("Avoid mutation using Stream API '" + simplifyContext.myMigration.getReplacement() + "' operation");
+      }
+    }
+
+    private static SimplifyForEachContext findMigration(PsiElement context) {
+      if (!(context instanceof PsiReferenceExpression) || !PsiUtil.isAccessedForWriting((PsiExpression)context)) return null;
+      PsiLambdaExpression lambda = PsiTreeUtil.getParentOfType(context, PsiLambdaExpression.class);
+      if (lambda == null) return null;
+      PsiElement lambdaBody = lambda.getBody();
+      if (lambdaBody == null) return null;
+      PsiExpressionList parameters = tryCast(PsiUtil.skipParenthesizedExprUp(lambda.getParent()), PsiExpressionList.class);
+      if (parameters == null || parameters.getExpressions().length != 1) return null;
+      PsiMethodCallExpression call = tryCast(parameters.getParent(), PsiMethodCallExpression.class);
+      SimplifyForEachContext simplifyForEachContext = SimplifyForEachContext.from(call);
+      if (simplifyForEachContext == null || simplifyForEachContext.myMigration instanceof ForEachMigration) return null;
+      return simplifyForEachContext;
+    }
+
+    @Override
+    public void invoke(@NotNull Project project, Editor editor, @NotNull PsiElement element) throws IncorrectOperationException {
+      PsiMethodCallExpression call =
+        PsiTreeUtil.getParentOfType(PsiTreeUtil.getParentOfType(element, PsiLambdaExpression.class), PsiMethodCallExpression.class);
+      SimplifyForEachContext simplifyForEachContext = SimplifyForEachContext.from(call);
+      if (simplifyForEachContext != null) {
+        PsiElement result = simplifyForEachContext.migrate();
+        if (result != null) {
+          MigrateToStreamFix.simplifyAndFormat(project, result);
+        }
+      }
+    }
+
+    @Override
+    public boolean isAvailable(@NotNull Project project, Editor editor, @NotNull PsiElement element) {
+      return myContext != null && myContext.isValid();
+    }
+
+    @Nls
+    @NotNull
+    @Override
+    public String getFamilyName() {
+      return "Avoid mutation using Stream API";
+    }
+  }
 }
--- a/java/java-impl/src/com/intellij/codeInspection/streamMigration/StreamApiMigrationInspection.java
+++ b/java/java-impl/src/com/intellij/codeInspection/streamMigration/StreamApiMigrationInspection.java
@@ -25,7 +25,6 @@ import com.intellij.codeInspection.LambdaCanBeMethodReferenceInspection;
 import com.intellij.codeInspection.ProblemHighlightType;
 import com.intellij.codeInspection.ProblemsHolder;
 import com.intellij.codeInspection.ui.MultipleCheckboxOptionsPanel;
-import com.intellij.lang.java.JavaLanguage;
 import com.intellij.openapi.diagnostic.Logger;
 import com.intellij.openapi.project.Project;
 import com.intellij.openapi.roots.FileIndexFacade;
@@ -34,7 +33,6 @@ import com.intellij.openapi.vfs.VirtualFile;
 import com.intellij.profile.codeInspection.InspectionProjectProfileManager;
 import com.intellij.psi.*;
 import com.intellij.psi.controlFlow.*;
-import com.intellij.psi.impl.light.LightElement;
 import com.intellij.psi.search.LocalSearchScope;
 import com.intellij.psi.search.searches.ReferencesSearch;
 import com.intellij.psi.tree.IElementType;
@@ -43,8 +41,6 @@ import com.intellij.psi.util.PsiTreeUtil;
 import com.intellij.psi.util.PsiUtil;
 import com.intellij.psi.util.TypeConversionUtil;
 import com.intellij.util.ArrayUtil;
-import com.intellij.util.containers.ContainerUtil;
-import com.siyeh.ig.callMatcher.CallMatcher;
 import com.siyeh.ig.psiutils.*;
 import one.util.streamex.StreamEx;
 import org.jetbrains.annotations.Contract;
@@ -56,7 +52,6 @@ import javax.swing.*;
 import java.util.Collection;
 import java.util.List;
 import java.util.Objects;
-import java.util.function.Function;

 import static com.intellij.codeInspection.streamMigration.OperationReductionMigration.SUM_OPERATION;
 import static com.intellij.util.ObjectUtils.tryCast;
@@ -444,43 +439,30 @@ public class StreamApiMigrationInspection extends BaseJavaBatchLocalInspectionTo
      if (!ExceptionUtil.getThrownCheckedExceptions(body).isEmpty()) return;
      TerminalBlock tb = TerminalBlock.from(source, body);

-      BaseStreamApiMigration migration = findMigration(statement, body, tb, myHolder, SUGGEST_FOREACH, REPLACE_TRIVIAL_FOREACH);
-      offerMigration(statement, tb, migration, (streamApiMigration) -> getRange(streamApiMigration.isShouldWarn(), statement, myIsOnTheFly)
-        .shiftRight(-statement.getTextOffset()), null, myIsOnTheFly, myHolder);
-    }
-  }
-
-  static void offerMigration(PsiStatement statement,
-                                     TerminalBlock tb,
-                                     BaseStreamApiMigration migration,
-                                     Function<BaseStreamApiMigration, TextRange> rangeSupplier,
-                                     @Nullable String customMessage,
-                                     boolean isOnTheFly,
-                                     ProblemsHolder holder) {
-    if (migration != null && (isOnTheFly || migration.isShouldWarn())) {
-
-      MigrateToStreamFix[] fixes = {new MigrateToStreamFix(migration, customMessage)};
+      BaseStreamApiMigration migration = findMigration(statement, body, tb, SUGGEST_FOREACH, REPLACE_TRIVIAL_FOREACH);
+      if (migration == null || (!myIsOnTheFly && !migration.isShouldWarn())) return;
+      MigrateToStreamFix[] fixes = {new MigrateToStreamFix(migration, null)};
      if (migration instanceof ForEachMigration && !(tb.getLastOperation() instanceof CollectionStream)) { //for .stream()
-        fixes = ArrayUtil.append(fixes, new MigrateToStreamFix(new ForEachMigration(migration.isShouldWarn(), "forEachOrdered"), customMessage));
+        fixes = ArrayUtil.append(fixes, new MigrateToStreamFix(new ForEachMigration(migration.isShouldWarn(), "forEachOrdered"), null));
      }
      ProblemHighlightType highlightType =
        migration.isShouldWarn() ? ProblemHighlightType.GENERIC_ERROR_OR_WARNING : ProblemHighlightType.INFORMATION;
-      String message = customMessage != null ? customMessage : "Can be replaced with '" + migration.getReplacement() + "' call";
-      holder.registerProblem(statement, message, highlightType, rangeSupplier.apply(migration), fixes);
+      String message = "Can be replaced with '" + migration.getReplacement() + "' call";
+      TextRange range = getRange(migration.isShouldWarn(), statement, myIsOnTheFly);
+      myHolder.registerProblem(statement, message, highlightType, range.shiftRight(-statement.getTextOffset()), fixes);
    }
  }


  @Nullable
  static BaseStreamApiMigration findMigration(PsiStatement loop,
-                                                      PsiElement body,
-                                                      TerminalBlock tb,
-                                                      ProblemsHolder holder,
-                                                      boolean suggestForeach,
-                                                      boolean replaceTrivialForEach) {
+                                              PsiElement body,
+                                              TerminalBlock tb,
+                                              boolean suggestForeach,
+                                              boolean replaceTrivialForEach) {
    final ControlFlow controlFlow;
    try {
-      controlFlow = ControlFlowFactory.getInstance(holder.getProject())
+      controlFlow = ControlFlowFactory.getInstance(loop.getProject())
        .getControlFlow(body, LocalsOrMyInstanceFieldsControlFlowPolicy.getInstance());
    }
    catch (AnalysisCanceledException ignored) {
--- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/simplifyForEach/afterCompilationFixCount.java
+++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/simplifyForEach/afterCompilationFixCount.java
@@ -0,0 +1,10 @@
+// "Avoid mutation using Stream API 'count()' operation" "true"
+
+import java.util.*;
+
+public class Main {
+  void test(List<String> list) {
+      int count = (int) list.stream().filter(s -> !s.isEmpty()).count();
+      System.out.println(count);
+  }
+}
--- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/simplifyForEach/afterCompilationFixMax.java
+++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/simplifyForEach/afterCompilationFixMax.java
@@ -0,0 +1,10 @@
+// "Avoid mutation using Stream API 'max()' operation" "true"
+
+import java.util.*;
+
+public class Main {
+  void test(List<String> list) {
+      String longest = list.stream().max(Comparator.comparingInt(String::length)).orElse(null);
+      System.out.println(longest);
+  }
+}
--- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/simplifyForEach/beforeCompilationFixCount.java
+++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/simplifyForEach/beforeCompilationFixCount.java
@@ -0,0 +1,13 @@
+// "Avoid mutation using Stream API 'count()' operation" "true"
+
+import java.util.*;
+
+public class Main {
+  void test(List<String> list) {
+    int count = 0;
+    list.forEach(s -> {
+      if(!s.isEmpty()) c<caret>ount++;
+    });
+    System.out.println(count);
+  }
+}
--- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/simplifyForEach/beforeCompilationFixMax.java
+++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/simplifyForEach/beforeCompilationFixMax.java
@@ -0,0 +1,13 @@
+// "Avoid mutation using Stream API 'max()' operation" "true"
+
+import java.util.*;
+
+public class Main {
+  void test(List<String> list) {
+    String longest = null;
+    list.forEach(s -> {
+      if(longest == null || longest.length() < s.length()) longe<caret>st = s;
+    });
+    System.out.println(longest);
+  }
+}
--- a/resources/src/META-INF/IdeaPlugin.xml
+++ b/resources/src/META-INF/IdeaPlugin.xml
@@ -2027,6 +2027,8 @@
    <scratch.rootType implementation="com.intellij.execution.jshell.JShellRootType"/>

    <javaModuleSystem implementation="com.intellij.psi.impl.JavaPlatformModuleSystem"/>
+
+    <java.error.fix errorCode="lambda.variable.must.be.final" implementationClass="com.intellij.codeInspection.streamMigration.SimplifyForEachInspection$ForEachNonFinalFix"/>
  </extensions>

  <extensions defaultExtensionNs="org.jetbrains.uast">