IG: use CommentTracker instead of fragile custom comment handling (IDEA-199489)

This commit is contained in:
Bas Leijdekkers
2018-10-15 11:28:21 +02:00
parent 3dd1521ed5
commit e3ef431fcf
8 changed files with 96 additions and 72 deletions
@@ -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;
@@ -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<PsiExpression> 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<PsiExpression> collectSalientOperands(PsiExpression[] operands, IElementType tokenType, PsiType type) {
final PsiElementFactory factory = JavaPsiFacade.getElementFactory(operands[0].getProject());
final List<PsiExpression> 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<PsiExpression> 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);
}
}
@@ -0,0 +1,4 @@
class X {
/*!*/
long typePromotion = (long) Integer.MAX_VALUE * Integer.MAX_VALUE;
}
@@ -0,0 +1,3 @@
class X {
long typePromotion = <warning descr="'1L /*!*/ * Integer.MAX_VALUE * Integer.MAX_VALUE' can be replaced with 'Integer.MAX_VALUE * Integer.MAX_VALUE'">1L<caret> /*!*/ * Integer.MAX_VALUE * Integer.MAX_VALUE</warning>;
}
@@ -0,0 +1,5 @@
class A {
static int foo() {
return 4 * /* comment*/ 5;
}
}
@@ -0,0 +1,5 @@
class A {
static int foo() {
return <warning descr="'4 * 1 */* comment*/ 1 * 5' can be replaced with '4 * 5'"><caret>4 * 1 */* comment*/ 1 * 5</warning>;
}
}
@@ -117,12 +117,12 @@ class Main {
int one = <warning descr="'5/5' can be replaced with '1'">5/5</warning>;
}
class Expanded {{
int m = <warning descr="'1/**/ - (byte)0 - 9' can be replaced with '1/**/ - 9'">1/**/ - (byte)0 - 9</warning>; // warn
int m = <warning descr="'1/**/ - (byte)0 - 9' can be replaced with '1 - 9'">1/**/ - (byte)0 - 9</warning>; // warn
int j = <warning descr="'8 * 0 * 8' can be replaced with '0'">8 * 0 * 8</warning>;
int k = <warning descr="'1 + /*a*/0 +/**/ 9' can be replaced with '1 + /*a*//**/ 9'">1 + /*a*/0 +/**/ 9</warning>;
int k = <warning descr="'1 + /*a*/0 +/**/ 9' can be replaced with '1 + 9'">1 + /*a*/0 +/**/ 9</warning>;
byte l = (byte) (<warning descr="'1L - 1L' can be replaced with '0L'">1L - 1L</warning>);
byte u = 1;
int z = <warning descr="'2 / 1 / 1' can be replaced with '2 / 1'">2 / 1 / 1</warning>;
int z = <warning descr="'2 / 1 / 1' can be replaced with '2'">2 / 1 / 1</warning>;
System.out.println(<warning descr="'u * 1' can be replaced with 'u'">u * 1</warning>);
long g = <warning descr="'8L / 8L' can be replaced with '1L'">8L / 8L</warning>;
long h = <warning descr="'9L * 0L' can be replaced with '0L'">9L * 0L</warning>;
@@ -131,7 +131,7 @@ class Expanded {{
int div = 3 / 2 / 2;
int mod = 3 % 2 % 2;
long typePromotion = <warning descr="'1L * Integer.MAX_VALUE * Integer.MAX_VALUE' can be replaced with '(long) Integer.MAX_VALUE * Integer.MAX_VALUE'">1L * Integer.MAX_VALUE * Integer.MAX_VALUE</warning>;
long typePromotion = <warning descr="'1L * Integer.MAX_VALUE * Integer.MAX_VALUE' can be replaced with 'Integer.MAX_VALUE * Integer.MAX_VALUE'">1L * Integer.MAX_VALUE * Integer.MAX_VALUE</warning>;
}}
class SideEffects {
public static void main( String args[] ){
@@ -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