MakeVoidQuickFix: handle side-effects correctly; add braces

java/java-impl/src/com/intellij/codeInspection/MakeVoidQuickFix.java

@@ -28,10 +28,14 @@ import com.intellij.refactoring.changeSignature.ChangeSignatureProcessor;
 import com.intellij.refactoring.changeSignature.ParameterInfoImpl;
 import com.intellij.util.IncorrectOperationException;
 import com.siyeh.ig.controlflow.UnnecessaryReturnInspection;
+import com.siyeh.ig.psiutils.BlockUtils;
 import com.siyeh.ig.psiutils.SideEffectChecker;
+import com.siyeh.ig.psiutils.StatementExtractor;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
+import java.util.List;
+
 public class MakeVoidQuickFix implements LocalQuickFix {
   private final ProblemDescriptionsProcessor myProcessor;
   private static final Logger LOG = Logger.getInstance(MakeVoidQuickFix.class);
@@ -97,29 +101,32 @@ public class MakeVoidQuickFix implements LocalQuickFix {
 
   private static void replaceReturnStatements(@NotNull final PsiMethod method) {
     final PsiReturnStatement[] statements = PsiUtil.findReturnStatements(method);
-    for (int i = statements.length - 1; i >= 0; i--) {
-      final PsiReturnStatement returnStatement = statements[i];
-      try {
-        final PsiExpression expression = returnStatement.getReturnValue();
-        if (expression != null) {
-          WriteAction.run(() -> {
-            final boolean mayHaveSideEffects = SideEffectChecker.mayHaveSideEffects(expression);
-            final PsiElementFactory factory = JavaPsiFacade.getElementFactory(method.getProject());
-            final PsiReturnStatement ret =
-              (PsiReturnStatement)returnStatement.replace(factory.createStatementFromText("return;", returnStatement));
-            if (mayHaveSideEffects) {
-              final PsiStatement statement = factory.createStatementFromText(expression.getText() + ";", method);
-              ret.getParent().addBefore(statement, ret);
+    if (statements.length > 0) {
+      WriteAction.run(() -> {
+        for (int i = statements.length - 1; i >= 0; i--) {
+          PsiReturnStatement returnStatement = statements[i];
+          try {
+            final PsiExpression expression = returnStatement.getReturnValue();
+            if (expression != null) {
+              List<PsiExpression> sideEffectExpressions = SideEffectChecker.extractSideEffectExpressions(expression);
+              PsiStatement[] sideEffectStatements = StatementExtractor.generateStatements(sideEffectExpressions, expression);
+              if (sideEffectStatements.length > 0) {
+                PsiStatement added = BlockUtils.addBefore(returnStatement, sideEffectStatements);
+                returnStatement = PsiTreeUtil.getNextSiblingOfType(added, PsiReturnStatement.class);
+              }
+              if (returnStatement != null && returnStatement.getReturnValue() != null) {
+                returnStatement.getReturnValue().delete();
+                if (UnnecessaryReturnInspection.isReturnRedundant(returnStatement, false, null)) {
+                  returnStatement.delete();
+                }
+              }
             }
-            if (UnnecessaryReturnInspection.isReturnRedundant(ret, false, null)) {
-              ret.delete();
-            }
-          });
+          }
+          catch (IncorrectOperationException e) {
+            LOG.error(e);
+          }
         }
-      }
-      catch (IncorrectOperationException e) {
-        LOG.error(e);
-      }
+      });
     }
   }
 }

java/java-tests/testData/inspection/unusedReturnValue/quickFix/SideEffects.java

@@ -7,6 +7,6 @@ class SideEffects {
   }
 
   int <caret>f() {
-    return i = 1;
+    return (i = 1) + 2;
   }
 }

java/java-tests/testData/inspection/unusedReturnValue/quickFix/SideEffectsComplex.after.java

@@ -0,0 +1,15 @@
+import java.util.concurrent.atomic.AtomicInteger;
+
+public class Main {
+  private void incOrDec(boolean b, AtomicInteger x) {
+      if (b) {
+          x.incrementAndGet();
+      } else {
+          x.decrementAndGet();
+      }
+  }
+
+  public void test() {
+    incOrDec(true, new AtomicInteger());
+  }
+}

java/java-tests/testData/inspection/unusedReturnValue/quickFix/SideEffectsComplex.java

@@ -0,0 +1,11 @@
+import java.util.concurrent.atomic.AtomicInteger;
+
+public class Main {
+  private int in<caret>cOrDec(boolean b, AtomicInteger x) {
+    return b ? x.incrementAndGet() : x.decrementAndGet();
+  }
+
+  public void test() {
+    incOrDec(true, new AtomicInteger());
+  }
+}

java/java-tests/testData/inspection/unusedReturnValue/quickFix/SideEffectsComplex2.after.java

@@ -0,0 +1,16 @@
+import java.util.concurrent.atomic.AtomicInteger;
+
+public class Main {
+    private void incOrDec(boolean b, AtomicInteger x) {
+        if (b) {
+            x.incrementAndGet();
+            return;
+        }
+        System.out.println("dec");
+        x.decrementAndGet();
+    }
+
+    public void test() {
+        incOrDec(true, new AtomicInteger());
+    }
+}

java/java-tests/testData/inspection/unusedReturnValue/quickFix/SideEffectsComplex2.java

@@ -0,0 +1,13 @@
+import java.util.concurrent.atomic.AtomicInteger;
+
+public class Main {
+    private int in<caret>cOrDec(boolean b, AtomicInteger x) {
+        if (b) return x.incrementAndGet()+1;
+        System.out.println("dec");
+        return x.decrementAndGet()+1;
+    }
+
+    public void test() {
+        incOrDec(true, new AtomicInteger());
+    }
+}

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

@@ -54,6 +54,8 @@ public class UnusedReturnValueQuickFixTest extends LightCodeInsightFixtureTestCa
   }
 
   public void testSideEffects() { doTest(); }
+  public void testSideEffectsComplex() { doTest(); }
+  public void testSideEffectsComplex2() { doTest(); }
   public void testRedundantReturn() { doTest(); }
   public void testNoChangeForImplicitRead() {
     final String name = getTestName(false);