From e3ef431fcff53a948d3f566cc42df8ae1a155614 Mon Sep 17 00:00:00 2001 From: Bas Leijdekkers Date: Sat, 13 Oct 2018 12:58:03 +0200 Subject: [PATCH] IG: use CommentTracker instead of fragile custom comment handling (IDEA-199489) --- .../com/siyeh/ig/LightInspectionTestCase.java | 7 + ...intlessArithmeticExpressionInspection.java | 127 +++++++++--------- .../Cast.after.java | 4 + .../pointless_arithmetic_expression/Cast.java | 3 + .../Comments.after.java | 5 + .../Comments.java | 5 + .../PointlessArithmeticExpression.java | 8 +- ...essArithmeticExpressionInspectionTest.java | 9 +- 8 files changed, 96 insertions(+), 72 deletions(-) create mode 100644 plugins/InspectionGadgets/test/com/siyeh/igtest/numeric/pointless_arithmetic_expression/Cast.after.java create mode 100644 plugins/InspectionGadgets/test/com/siyeh/igtest/numeric/pointless_arithmetic_expression/Cast.java create mode 100644 plugins/InspectionGadgets/test/com/siyeh/igtest/numeric/pointless_arithmetic_expression/Comments.after.java create mode 100644 plugins/InspectionGadgets/test/com/siyeh/igtest/numeric/pointless_arithmetic_expression/Comments.java diff --git a/java/testFramework/src/com/siyeh/ig/LightInspectionTestCase.java b/java/testFramework/src/com/siyeh/ig/LightInspectionTestCase.java index 2fabe307171f..a4d97734ce41 100644 --- a/java/testFramework/src/com/siyeh/ig/LightInspectionTestCase.java +++ b/java/testFramework/src/com/siyeh/ig/LightInspectionTestCase.java @@ -94,6 +94,13 @@ public abstract class LightInspectionTestCase extends LightCodeInsightFixtureTes myFixture.checkResult(result); } + protected final void checkQuickFix(String intentionName) { + final IntentionAction intention = myFixture.getAvailableIntention(intentionName); + assertNotNull(intention); + myFixture.launchAction(intention); + myFixture.checkResultByFile(getTestName(false) + ".after.java"); + } + protected final void doTest(@Language("JAVA") @NotNull String classText, String fileName) { final StringBuilder newText = new StringBuilder(); int start = 0; diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/numeric/PointlessArithmeticExpressionInspection.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/numeric/PointlessArithmeticExpressionInspection.java index 2c9ef97fde9b..bedf8364ed54 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/numeric/PointlessArithmeticExpressionInspection.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/numeric/PointlessArithmeticExpressionInspection.java @@ -23,11 +23,13 @@ import com.intellij.psi.tree.IElementType; import com.intellij.psi.util.PsiUtil; import com.intellij.psi.util.PsiUtilCore; import com.intellij.psi.util.TypeConversionUtil; +import com.intellij.util.SmartList; +import com.intellij.util.containers.ContainerUtil; import com.siyeh.InspectionGadgetsBundle; import com.siyeh.ig.BaseInspection; import com.siyeh.ig.BaseInspectionVisitor; import com.siyeh.ig.InspectionGadgetsFix; -import com.siyeh.ig.PsiReplacementUtil; +import com.siyeh.ig.psiutils.CommentTracker; import com.siyeh.ig.psiutils.EquivalenceChecker; import com.siyeh.ig.psiutils.ExpressionUtils; import com.siyeh.ig.psiutils.SideEffectChecker; @@ -36,7 +38,9 @@ import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; import javax.swing.*; +import java.util.List; import java.util.Set; +import java.util.stream.Collectors; public class PointlessArithmeticExpressionInspection extends BaseInspection { @@ -82,80 +86,58 @@ public class PointlessArithmeticExpressionInspection extends BaseInspection { @Override @NotNull public String buildErrorString(Object... infos) { - return InspectionGadgetsBundle.message( - "expression.can.be.replaced.problem.descriptor", - calculateReplacementExpression((PsiExpression)infos[0])); + return InspectionGadgetsBundle.message("expression.can.be.replaced.problem.descriptor", + calculateReplacementExpression((PsiPolyadicExpression)infos[0])); } @NonNls - String calculateReplacementExpression(PsiExpression expression) { - final PsiPolyadicExpression polyadicExpression = (PsiPolyadicExpression)expression; - final PsiExpression[] operands = polyadicExpression.getOperands(); - final IElementType tokenType = polyadicExpression.getOperationTokenType(); - PsiElement fromTarget = null; - PsiElement untilTarget = null; - PsiExpression previousOperand = null; - @NonNls String replacement = ""; + String calculateReplacementExpression(PsiPolyadicExpression expression) { + final PsiExpression[] operands = expression.getOperands(); + final IElementType tokenType = expression.getOperationTokenType(); + final List expressions = collectSalientOperands(operands, tokenType, expression.getType()); + final PsiJavaToken token = expression.getTokenBeforeOperand(operands[1]); + assert token != null; + final String delimiter = " " + token.getText() + " "; + return expressions.stream().map(PsiElement::getText).collect(Collectors.joining(delimiter)); + } + + @NotNull + List collectSalientOperands(PsiExpression[] operands, IElementType tokenType, PsiType type) { + final PsiElementFactory factory = JavaPsiFacade.getElementFactory(operands[0].getProject()); + final List expressions = new SmartList<>(); for (int i = 0, length = operands.length; i < length; i++) { final PsiExpression operand = operands[i]; if (tokenType.equals(JavaTokenType.PLUS) && isZero(operand) || - tokenType.equals(JavaTokenType.MINUS) && isZero(operand) && i > 0 || + tokenType.equals(JavaTokenType.MINUS) && isZero(operand) && !expressions.isEmpty() || tokenType.equals(JavaTokenType.ASTERISK) && isOne(operand) || - tokenType.equals(JavaTokenType.DIV) && isOne(operand) && i > 0) { - fromTarget = (i == length - 1) ? polyadicExpression.getTokenBeforeOperand(operand) : operand; - break; + tokenType.equals(JavaTokenType.DIV) && isOne(operand) && !expressions.isEmpty()) { + continue; } - else if ((tokenType.equals(JavaTokenType.MINUS) && i == 1 || tokenType.equals(JavaTokenType.DIV)) && - EquivalenceChecker.getCanonicalPsiEquivalence().expressionsAreEquivalent(previousOperand, operand)) { - fromTarget = previousOperand; - untilTarget = operand; - replacement = PsiType.LONG.equals(polyadicExpression.getType()) - ? tokenType.equals(JavaTokenType.DIV) ? "1L" : "0L" - : tokenType.equals(JavaTokenType.DIV) ? "1" : "0"; - break; + else if (tokenType.equals(JavaTokenType.MINUS) && i == 1 && + EquivalenceChecker.getCanonicalPsiEquivalence().expressionsAreEquivalent(ContainerUtil.getLastItem(expressions), operand)) { + expressions.remove(expressions.size() - 1); + expressions.add(factory.createExpressionFromText(PsiType.LONG.equals(type) ? "0L" : "0", operand)); + continue; + } + else if (tokenType.equals(JavaTokenType.DIV) && + EquivalenceChecker.getCanonicalPsiEquivalence().expressionsAreEquivalent(ContainerUtil.getLastItem(expressions), operand)) { + expressions.remove(expressions.size() - 1); + expressions.add(factory.createExpressionFromText(PsiType.LONG.equals(type) ? "1L" : "1", operand)); + continue; } else if (tokenType.equals(JavaTokenType.ASTERISK) && isZero(operand) || - tokenType.equals(JavaTokenType.PERC) && (isOne(operand) || EquivalenceChecker.getCanonicalPsiEquivalence() - .expressionsAreEquivalent(previousOperand, operand))) { - fromTarget = operands[0]; - untilTarget = operands[length - 1]; - replacement = PsiType.LONG.equals(polyadicExpression.getType()) ? "0L" : "0"; - break; + tokenType.equals(JavaTokenType.PERC) && + (isOne(operand) || EquivalenceChecker.getCanonicalPsiEquivalence().expressionsAreEquivalent(ContainerUtil.getLastItem(expressions), operand))) { + expressions.clear(); + expressions.add(factory.createExpressionFromText(PsiType.LONG.equals(type) ? "0L" : "0", operand)); + return expressions; } - - previousOperand = operand; + expressions.add(operand); } - return getText(polyadicExpression, fromTarget, untilTarget, replacement).trim(); - } - - public static String getText(PsiPolyadicExpression expression, PsiElement fromTarget, PsiElement untilTarget, - @NotNull @NonNls String replacement) { - final StringBuilder result = new StringBuilder(); - boolean stop = false; - boolean longTypeSeen = false; - for (PsiElement child : expression.getChildren()) { - if (child == fromTarget) { - stop = true; - result.append(replacement); - } - else if (child == untilTarget) { - stop = false; - } - else if (child instanceof PsiComment || !stop) { - if (child instanceof PsiExpression) { - final PsiExpression childExpression = (PsiExpression)child; - longTypeSeen |= TypeConversionUtil.isLongType(childExpression.getType()); - } - result.append(child.getText()); - } - else if (child instanceof PsiJavaToken && untilTarget == null) { - stop = false; - } + if (expressions.isEmpty()) { + expressions.add(factory.createExpressionFromText(tokenType.equals(JavaTokenType.ASTERISK) ? "1" : "0", operands[0])); } - if (!longTypeSeen && TypeConversionUtil.isLongType(expression.getType()) && replacement.isEmpty()) { - result.insert(0, "(long)"); - } - return result.toString(); + return expressions; } @Override @@ -174,11 +156,22 @@ public class PointlessArithmeticExpressionInspection extends BaseInspection { @Override public void doFix(Project project, ProblemDescriptor descriptor) { - final PsiExpression expression = - (PsiExpression)descriptor.getPsiElement(); - final String newExpression = - calculateReplacementExpression(expression); - PsiReplacementUtil.replaceExpression(expression, newExpression); + final PsiElement element = descriptor.getPsiElement(); + if (!(element instanceof PsiPolyadicExpression)) { + return; + } + final PsiPolyadicExpression expression = (PsiPolyadicExpression)element; + final PsiExpression[] operands = expression.getOperands(); + final PsiType type = expression.getType(); + final List expressions = collectSalientOperands(operands, expression.getOperationTokenType(), type); + final CommentTracker tracker = new CommentTracker(); + final PsiJavaToken token = expression.getTokenBeforeOperand(operands[1]); + assert token != null; + final String delimiter = " " + token.getText() + " "; + final String replacement = expressions.stream().map(x -> tracker.textWithComments(x)).collect(Collectors.joining(delimiter)); + final boolean castToLongNeeded = TypeConversionUtil.isLongType(type) && + expressions.stream().noneMatch(x -> TypeConversionUtil.isLongType(x.getType())); + tracker.replaceAndRestoreComments(element, castToLongNeeded ? "(long)" + replacement : replacement); } } diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/numeric/pointless_arithmetic_expression/Cast.after.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/numeric/pointless_arithmetic_expression/Cast.after.java new file mode 100644 index 000000000000..eb17cfc35497 --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/numeric/pointless_arithmetic_expression/Cast.after.java @@ -0,0 +1,4 @@ +class X { + /*!*/ + long typePromotion = (long) Integer.MAX_VALUE * Integer.MAX_VALUE; +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/numeric/pointless_arithmetic_expression/Cast.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/numeric/pointless_arithmetic_expression/Cast.java new file mode 100644 index 000000000000..c6b6679f2cb5 --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/numeric/pointless_arithmetic_expression/Cast.java @@ -0,0 +1,3 @@ +class X { + long typePromotion = 1L /*!*/ * Integer.MAX_VALUE * Integer.MAX_VALUE; +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/numeric/pointless_arithmetic_expression/Comments.after.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/numeric/pointless_arithmetic_expression/Comments.after.java new file mode 100644 index 000000000000..f0d8418f4523 --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/numeric/pointless_arithmetic_expression/Comments.after.java @@ -0,0 +1,5 @@ +class A { + static int foo() { + return 4 * /* comment*/ 5; + } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/numeric/pointless_arithmetic_expression/Comments.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/numeric/pointless_arithmetic_expression/Comments.java new file mode 100644 index 000000000000..1c6dc3768949 --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/numeric/pointless_arithmetic_expression/Comments.java @@ -0,0 +1,5 @@ +class A { + static int foo() { + return 4 * 1 */* comment*/ 1 * 5; + } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/numeric/pointless_arithmetic_expression/PointlessArithmeticExpression.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/numeric/pointless_arithmetic_expression/PointlessArithmeticExpression.java index 49253d9740ed..0431eb321867 100644 --- a/plugins/InspectionGadgets/test/com/siyeh/igtest/numeric/pointless_arithmetic_expression/PointlessArithmeticExpression.java +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/numeric/pointless_arithmetic_expression/PointlessArithmeticExpression.java @@ -117,12 +117,12 @@ class Main { int one = 5/5; } class Expanded {{ - int m = 1/**/ - (byte)0 - 9; // warn + int m = 1/**/ - (byte)0 - 9; // warn int j = 8 * 0 * 8; - int k = 1 + /*a*/0 +/**/ 9; + int k = 1 + /*a*/0 +/**/ 9; byte l = (byte) (1L - 1L); byte u = 1; - int z = 2 / 1 / 1; + int z = 2 / 1 / 1; System.out.println(u * 1); long g = 8L / 8L; long h = 9L * 0L; @@ -131,7 +131,7 @@ class Expanded {{ int div = 3 / 2 / 2; int mod = 3 % 2 % 2; - long typePromotion = 1L * Integer.MAX_VALUE * Integer.MAX_VALUE; + long typePromotion = 1L * Integer.MAX_VALUE * Integer.MAX_VALUE; }} class SideEffects { public static void main( String args[] ){ diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/numeric/PointlessArithmeticExpressionInspectionTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/numeric/PointlessArithmeticExpressionInspectionTest.java index 58bbf452bd9c..dfabddd6e46e 100644 --- a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/numeric/PointlessArithmeticExpressionInspectionTest.java +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/numeric/PointlessArithmeticExpressionInspectionTest.java @@ -1,13 +1,20 @@ +// Copyright 2000-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. package com.siyeh.ig.numeric; import com.intellij.codeInspection.InspectionProfileEntry; +import com.siyeh.InspectionGadgetsBundle; import com.siyeh.ig.LightInspectionTestCase; import org.jetbrains.annotations.Nullable; public class PointlessArithmeticExpressionInspectionTest extends LightInspectionTestCase { - public void testPointlessArithmeticExpression() { + public void testPointlessArithmeticExpression() { doTest(); } + public void testComments() { doQuickFixTest(); } + public void testCast() { doQuickFixTest(); } + + private void doQuickFixTest() { doTest(); + checkQuickFix(InspectionGadgetsBundle.message("constant.conditional.expression.simplify.quickfix")); } @Nullable