Don't remove variable assignment in "Manual min/max calculation" inspection fix (IDEA-291924)

GitOrigin-RevId: f30aacee58cb79e738e79327f586cd371338a6ed

java/java-impl-inspections/src/com/intellij/codeInspection/ManualMinMaxCalculationInspection.java

@@ -122,28 +122,31 @@ public class ManualMinMaxCalculationInspection extends AbstractBaseJavaLocalInsp
     @Override
     public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) {
       PsiElement element = descriptor.getPsiElement();
+      final CommentTracker ct = new CommentTracker();
       if (element instanceof PsiConditionalExpression) {
         ConditionalModel model = ConditionalModel.from((PsiConditionalExpression)element);
         if (model == null) return;
-        String replacement = createReplacement(model.getCondition());
+        String replacement = createReplacement(model.getCondition(), ct);
         if (replacement == null) return;
-        PsiReplacementUtil.replaceExpression((PsiExpression)element, replacement, new CommentTracker());
+        PsiReplacementUtil.replaceExpression((PsiExpression)element, replacement, ct);
         return;
       }
       PsiIfStatement ifStatement = PsiTreeUtil.getParentOfType(element, PsiIfStatement.class);
       if (ifStatement == null) return;
       IfConditionalModel model = IfConditionalModel.from(ifStatement, false);
       if (model == null) return;
-      String replacement = createReplacement(model.getCondition());
+      String replacement = createReplacement(model.getCondition(), ct);
       if (replacement == null) return;
       PsiStatement elseBranch = model.getElseBranch();
       final PsiElement result;
       if (elseBranch instanceof PsiDeclarationStatement) {
-        result = replace(ifStatement, elseBranch, model.getElseExpression(), replacement);
+        PsiReplacementUtil.replaceExpression(model.getElseExpression(), replacement, new CommentTracker());
+        result = PsiReplacementUtil.replaceStatement(ifStatement, ct.text(elseBranch), ct);
         elseBranch.delete();
       }
       else {
-        result = replace(ifStatement, model.getThenBranch(), model.getThenExpression(), replacement);
+        PsiReplacementUtil.replaceExpression(model.getThenExpression(), replacement, new CommentTracker());
+        result = PsiReplacementUtil.replaceStatement(ifStatement, ct.text(model.getThenBranch()), ct);
         if (!PsiTreeUtil.isAncestor(ifStatement, elseBranch, true)) {
           new CommentTracker().deleteAndRestoreComments(elseBranch);
         }
@@ -151,23 +154,14 @@ public class ManualMinMaxCalculationInspection extends AbstractBaseJavaLocalInsp
       SimplifiableIfStatementInspection.tryJoinDeclaration(result);
     }
 
-    private static @NotNull PsiElement replace(@NotNull PsiIfStatement ifStatement,
-                                               @NotNull PsiStatement branch, @NotNull PsiExpression toReplace,
-                                               @NotNull String replacement) {
-      PsiReplacementUtil.replaceExpression(toReplace, replacement, new CommentTracker());
-      CommentTracker tracker = new CommentTracker();
-      tracker.text(branch);
-      return PsiReplacementUtil.replaceStatement(ifStatement, branch.getText(), tracker);
-    }
-
     @Nullable
-    private String createReplacement(@NotNull PsiExpression expression) {
+    private String createReplacement(@NotNull PsiExpression expression, CommentTracker ct) {
       PsiBinaryExpression condition = getCondition(expression);
       if (condition == null) return null;
       PsiExpression left = condition.getLOperand();
       PsiExpression right = condition.getROperand();
       if (right == null) return null;
-      return CommonClassNames.JAVA_LANG_MATH + (myUseMathMin ? ".min" : ".max") + "(" + left.getText() + "," + right.getText() + ")";
+      return CommonClassNames.JAVA_LANG_MATH + (myUseMathMin ? ".min" : ".max") + "(" + ct.text(left) + "," + ct.text(right) + ")";
     }
   }
 }

java/java-tests/testData/inspection/manualMinMaxCalculation/afterVariableValueUsed.java

@@ -0,0 +1,23 @@
+// "Replace with 'Math.min()' call" "true"
+import java.util.Random;
+
+class VariableValueUsed {
+  private static final Random rnd = new Random();
+
+  private static int test() {
+    return rnd.nextInt();
+  }
+
+  public static void main(String[] args) {
+    int l = 2;
+    int myNumber = test();
+
+      /*3*/
+      /*1*/
+      /*2*/
+      /*5*/
+      myNumber = Math.min(l +/*6*/1, myNumber/*4*/ - 2);
+
+    System.out.println(myNumber);
+  }
+}

java/java-tests/testData/inspection/manualMinMaxCalculation/beforeVariableValueUsed.java

@@ -0,0 +1,23 @@
+// "Replace with 'Math.min()' call" "true"
+import java.util.Random;
+
+class VariableValueUsed {
+  private static final Random rnd = new Random();
+
+  private static int test() {
+    return rnd.nextInt();
+  }
+
+  public static void main(String[] args) {
+    int l = 2;
+    int myNumber = test();
+
+    <caret>if (l+/*6*/1 <=/*3*/ myNumber/*4*/ - 2) {
+      myNumber = l/*1*/+1;
+    } else {
+      myNumber = /*2*/ myNumber /*5*/- 2;
+    }
+
+    System.out.println(myNumber);
+  }
+}

plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/style/SimplifiableIfStatementInspection.java

@@ -1,4 +1,4 @@
-// Copyright 2000-2021 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
+// Copyright 2000-2022 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
 package com.siyeh.ig.style;
 
 import com.intellij.codeInspection.*;
@@ -6,6 +6,7 @@ import com.intellij.codeInspection.ui.MultipleCheckboxOptionsPanel;
 import com.intellij.openapi.project.Project;
 import com.intellij.psi.*;
 import com.intellij.psi.util.PsiTreeUtil;
+import com.intellij.psi.util.PsiUtil;
 import com.siyeh.InspectionGadgetsBundle;
 import com.siyeh.ig.psiutils.CommentTracker;
 import com.siyeh.ig.psiutils.ControlFlowUtils;
@@ -69,6 +70,11 @@ public class SimplifiableIfStatementInspection extends AbstractBaseJavaLocalInsp
     if (var == null || !ref.isReferenceTo(var)) return;
     final PsiExpression rhs = assignment.getRExpression();
     assert rhs != null;
+    boolean readBeforeWritten = SyntaxTraverser.psiTraverser(rhs)
+      .filter(PsiReferenceExpression.class)
+      .filter(r -> r.isReferenceTo(var) && PsiUtil.isAccessedForReading(r))
+      .isNotEmpty();
+    if (readBeforeWritten) return;
     CommentTracker ct = new CommentTracker();
     var.setInitializer(ct.markUnchanged(rhs));
     ct.deleteAndRestoreComments(result);