diff --git a/java/java-impl/src/com/intellij/codeInspection/MakeVoidQuickFix.java b/java/java-impl/src/com/intellij/codeInspection/MakeVoidQuickFix.java index 5797e5e5a903..95c2f9ab09b0 100644 --- a/java/java-impl/src/com/intellij/codeInspection/MakeVoidQuickFix.java +++ b/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 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); - } + }); } } } diff --git a/java/java-tests/testData/inspection/unusedReturnValue/quickFix/SideEffects.java b/java/java-tests/testData/inspection/unusedReturnValue/quickFix/SideEffects.java index 78c66f845672..dfea119eb21f 100644 --- a/java/java-tests/testData/inspection/unusedReturnValue/quickFix/SideEffects.java +++ b/java/java-tests/testData/inspection/unusedReturnValue/quickFix/SideEffects.java @@ -7,6 +7,6 @@ class SideEffects { } int f() { - return i = 1; + return (i = 1) + 2; } } \ No newline at end of file diff --git a/java/java-tests/testData/inspection/unusedReturnValue/quickFix/SideEffectsComplex.after.java b/java/java-tests/testData/inspection/unusedReturnValue/quickFix/SideEffectsComplex.after.java new file mode 100644 index 000000000000..afa976c2be18 --- /dev/null +++ b/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()); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/unusedReturnValue/quickFix/SideEffectsComplex.java b/java/java-tests/testData/inspection/unusedReturnValue/quickFix/SideEffectsComplex.java new file mode 100644 index 000000000000..1c13c9e266f9 --- /dev/null +++ b/java/java-tests/testData/inspection/unusedReturnValue/quickFix/SideEffectsComplex.java @@ -0,0 +1,11 @@ +import java.util.concurrent.atomic.AtomicInteger; + +public class Main { + private int incOrDec(boolean b, AtomicInteger x) { + return b ? x.incrementAndGet() : x.decrementAndGet(); + } + + public void test() { + incOrDec(true, new AtomicInteger()); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/unusedReturnValue/quickFix/SideEffectsComplex2.after.java b/java/java-tests/testData/inspection/unusedReturnValue/quickFix/SideEffectsComplex2.after.java new file mode 100644 index 000000000000..38c4a376d359 --- /dev/null +++ b/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()); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/unusedReturnValue/quickFix/SideEffectsComplex2.java b/java/java-tests/testData/inspection/unusedReturnValue/quickFix/SideEffectsComplex2.java new file mode 100644 index 000000000000..eb5ff5c5d410 --- /dev/null +++ b/java/java-tests/testData/inspection/unusedReturnValue/quickFix/SideEffectsComplex2.java @@ -0,0 +1,13 @@ +import java.util.concurrent.atomic.AtomicInteger; + +public class Main { + private int incOrDec(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()); + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/UnusedReturnValueQuickFixTest.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/UnusedReturnValueQuickFixTest.java index 7f127427d8c8..9bbe5e8c0478 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/UnusedReturnValueQuickFixTest.java +++ b/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);