diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/CommentTracker.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/CommentTracker.java index 5be9343932dc..e9d527cb208d 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/CommentTracker.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/CommentTracker.java @@ -23,7 +23,7 @@ import java.util.function.Predicate; * @author Tagir Valeev */ public class CommentTracker { - private Set ignoredParents = new HashSet<>(); + private final Set ignoredParents = new HashSet<>(); private List comments = new ArrayList<>(); /** @@ -174,6 +174,26 @@ public class CommentTracker { return result; } + public static @NotNull PsiElement replaceWithSubexpressionAndRestoreComments(@NotNull PsiExpression expression, + @NotNull PsiExpression replacement) { + if (!PsiTreeUtil.isAncestor(expression, replacement, true)) throw new IllegalArgumentException("replacement is not a subexpression"); + final CommentTracker tracker = new CommentTracker(); + tracker.markUnchanged(replacement); + tracker.grabComments(expression); + final PsiElement parent = expression.getParent(); + final int limit = replacement.getTextOffset(); + PsiElement anchor = expression; + for (PsiComment comment : tracker.comments) { + if (comment.getTextOffset() < limit) { + parent.addBefore(comment, anchor); + } + else { + anchor = parent.addAfter(comment, anchor); + } + } + return expression.replace(replacement); + } + /** * Creates a replacement element from the text and replaces given element, * collecting all the comments inside it and restores comments putting them @@ -198,26 +218,24 @@ public class CommentTracker { private static @NotNull PsiElement createElement(@NotNull PsiElement element, @NotNull String text) { PsiElementFactory factory = JavaPsiFacade.getElementFactory(element.getProject()); - PsiElement replacement; if (element instanceof PsiExpression) { - replacement = factory.createExpressionFromText(text, element); + return factory.createExpressionFromText(text, element); } else if (element instanceof PsiStatement) { - replacement = factory.createStatementFromText(text, element); + return factory.createStatementFromText(text, element); } else if (element instanceof PsiTypeElement) { - replacement = factory.createTypeElementFromText(text, element); + return factory.createTypeElementFromText(text, element); } else if (element instanceof PsiIdentifier) { - replacement = factory.createIdentifier(text); + return factory.createIdentifier(text); } else if (element instanceof PsiComment) { - replacement = factory.createCommentFromText(text, element); + return factory.createCommentFromText(text, element); } else { throw new IllegalArgumentException("Unsupported element type: " + element); } - return replacement; } /** diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/SideEffectChecker.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/SideEffectChecker.java index 5864af6433d9..138ae7824221 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/SideEffectChecker.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/SideEffectChecker.java @@ -21,6 +21,7 @@ import com.intellij.psi.*; import com.intellij.psi.tree.IElementType; import com.intellij.psi.util.PropertyUtil; import com.intellij.psi.util.PsiTreeUtil; +import com.intellij.util.SmartList; import gnu.trove.THashSet; import one.util.streamex.StreamEx; import org.jetbrains.annotations.NotNull; @@ -79,7 +80,7 @@ public class SideEffectChecker { } public static List extractSideEffectExpressions(@NotNull PsiExpression element) { - List list = new ArrayList<>(); + List list = new SmartList<>(); element.accept(new SideEffectsVisitor(list, element)); return StreamEx.of(list).select(PsiExpression.class).toList(); } diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/controlflow/PointlessBooleanExpressionInspection.java b/plugins/InspectionGadgets/src/com/siyeh/ig/controlflow/PointlessBooleanExpressionInspection.java index e73fc997e364..de54845e934a 100644 --- a/plugins/InspectionGadgets/src/com/siyeh/ig/controlflow/PointlessBooleanExpressionInspection.java +++ b/plugins/InspectionGadgets/src/com/siyeh/ig/controlflow/PointlessBooleanExpressionInspection.java @@ -36,6 +36,7 @@ import org.jetbrains.annotations.Nullable; import javax.swing.*; import java.util.*; +import java.util.function.Supplier; public class PointlessBooleanExpressionInspection extends BaseInspection { private enum BooleanExpressionKind { @@ -78,9 +79,9 @@ public class PointlessBooleanExpressionInspection extends BaseInspection { @NotNull public String buildErrorString(Object... infos) { final String replacement = (String)infos[1]; - if (replacement.isEmpty()) { - final PsiAssignmentExpression expression = (PsiAssignmentExpression)infos[0]; - return InspectionGadgetsBundle.message("boolean.expression.does.not.modify.problem.descriptor", expression); + final PsiExpression expression = (PsiExpression)infos[0]; + if (replacement.isEmpty() && expression instanceof PsiAssignmentExpression) { + return InspectionGadgetsBundle.message("boolean.expression.does.not.modify.problem.descriptor", expression.getText()); } return InspectionGadgetsBundle.message("boolean.expression.can.be.simplified.problem.descriptor", replacement); } @@ -305,7 +306,7 @@ public class PointlessBooleanExpressionInspection extends BaseInspection { @Override public InspectionGadgetsFix buildFix(Object... infos) { final String replacement = (String)infos[1]; - if (replacement.isEmpty()) { + if (replacement.isEmpty() && infos[0] instanceof PsiAssignmentExpression) { return new RemovePointlessBooleanExpressionFix(); } boolean hasSideEffect = (boolean)infos[2]; @@ -323,7 +324,19 @@ public class PointlessBooleanExpressionInspection extends BaseInspection { @Override protected void doFix(Project project, ProblemDescriptor descriptor) { - new CommentTracker().deleteAndRestoreComments(descriptor.getPsiElement().getParent()); + final PsiElement element = descriptor.getPsiElement(); + if (!(element instanceof PsiAssignmentExpression)) { + return; + } + final PsiAssignmentExpression assignmentExpression = (PsiAssignmentExpression)element; + final List sideEffects = SideEffectChecker.extractSideEffectExpressions(assignmentExpression.getLExpression()); + assert sideEffects.size() < 2; + if (!sideEffects.isEmpty()) { + CommentTracker.replaceWithSubexpressionAndRestoreComments(assignmentExpression, sideEffects.get(0)); + } + else { + new CommentTracker().deleteAndRestoreComments(element.getParent()); + } } } @@ -430,11 +443,10 @@ public class PointlessBooleanExpressionInspection extends BaseInspection { return; } - String replacement = buildSimplifiedExpression(expression, new StringBuilder(), new CommentTracker()).toString(); - - if (parent instanceof PsiLambdaExpression && - !LambdaUtil.isSafeLambdaBodyReplacement((PsiLambdaExpression)parent, - () -> JavaPsiFacade.getElementFactory(expression.getProject()).createExpressionFromText(replacement, expression))) { + final String replacement = buildSimplifiedExpression(expression, new StringBuilder(), new CommentTracker()).toString(); + final Supplier newBodySupplier = + () -> JavaPsiFacade.getElementFactory(expression.getProject()).createExpressionFromText(replacement, expression); + if (parent instanceof PsiLambdaExpression && !LambdaUtil.isSafeLambdaBodyReplacement((PsiLambdaExpression)parent, newBodySupplier)) { return; } diff --git a/plugins/InspectionGadgets/test/com/siyeh/igfixes/pointlessboolean/CompoundAssignmentSideEffect.after.java b/plugins/InspectionGadgets/test/com/siyeh/igfixes/pointlessboolean/CompoundAssignmentSideEffect.after.java new file mode 100644 index 000000000000..811ba9484c7d --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igfixes/pointlessboolean/CompoundAssignmentSideEffect.after.java @@ -0,0 +1,16 @@ +class CompoundAssignmentSideEffect { + + void m() { + createSomeObject(/*1*/)// 2 + /*3*//*4*/ + ; + } + + X createSomeObject() { + return new X(); + } + + class X { + boolean b = false; + } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igfixes/pointlessboolean/CompoundAssignmentSideEffect.java b/plugins/InspectionGadgets/test/com/siyeh/igfixes/pointlessboolean/CompoundAssignmentSideEffect.java new file mode 100644 index 000000000000..8d826755496d --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igfixes/pointlessboolean/CompoundAssignmentSideEffect.java @@ -0,0 +1,15 @@ +class CompoundAssignmentSideEffect { + + void m() { + createSomeObject(/*1*/).b // 2 + &= /*3*//*4*/ true; + } + + X createSomeObject() { + return new X(); + } + + class X { + boolean b = false; + } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/PointlessBooleanExpressionFixTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/PointlessBooleanExpressionFixTest.java index 50e68c3e131d..107092d8dbe6 100644 --- a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/PointlessBooleanExpressionFixTest.java +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/PointlessBooleanExpressionFixTest.java @@ -20,9 +20,10 @@ public class PointlessBooleanExpressionFixTest extends IGQuickFixesTestCase { public void testNegation() { doTest(); } public void testPolyadic() { doTest(); } public void testBoxed() { doTest(); } - public void testSideEffects() { doTest(InspectionGadgetsBundle.message("constant.conditional.expression.simplify.quickfix.sideEffect"));} - public void testSideEffectsField() { doTest(InspectionGadgetsBundle.message("constant.conditional.expression.simplify.quickfix.sideEffect"));} + public void testSideEffects() { doTest(InspectionGadgetsBundle.message("constant.conditional.expression.simplify.quickfix.sideEffect")); } + public void testSideEffectsField() { doTest(InspectionGadgetsBundle.message("constant.conditional.expression.simplify.quickfix.sideEffect")); } public void testCompoundAssignment1() { doTest(InspectionGadgetsBundle.message("boolean.expression.remove.compound.assignment.quickfix")); } public void testCompoundAssignment2() { doTest(); } public void testCompoundAssignment3() { doTest(); } + public void testCompoundAssignmentSideEffect() { doTest(InspectionGadgetsBundle.message("boolean.expression.remove.compound.assignment.quickfix")); } }