MDS/openide
1
0
Fork 0
mirror of https://gitflic.ru/project/openide/openide.git synced 2026-09-27 10:03:11 +07:00
Code Issues Packages Projects Releases Wiki Activity

[codeInsight] IDEA-219640 Inspection to detect some pointless String.substring

Browse Source 
This patch fixes the notes from the review, it contains:

1. fixing the javadoc for ExpressionUtils#isConversionToStringNecessary
2. properly handling JavaTokenType.MINUS in ExpressionUtils#isDifference
3. properly handling complex PsiPolyadicExpression values in ExpressionUtils#isDifference
4. changing ProblemHighlightType to GENERIC_ERROR_OR_WARNING for
inspecitons like these
5. fixing the comments duplication problem when replacing the old string
with a new one for inspections like stringValue.substring(0, 1).equals("_")

Signed-off-by: Nikita Eshkeev <nikita.eshkeev@jetbrains.com>

GitOrigin-RevId: 907524c9026e2d1d7cd6f0bd7b988e478e73298d
This commit is contained in:
Nikita Eshkeev
2020-04-20 20:10:15 +00:00
committed by intellij-monorepo-bot
parent 0c70f74bee
commit 565443bca4
6 changed files with 195 additions and 56 deletions
Download Patch File Download Diff File Expand all files Collapse all files
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantStringOperation/afterSubstringOneCharInAppend.java
+21 -2
View File
@@ -3,16 +3,18 @@ class Foo {
public static void main(String[] args) {
StringBuilder sb = new StringBuilder();
int i = Integer.parseInt(args[4]);
int i = Integer.parseInt(args[3]);
int j = Integer.parseInt(args[4]);
sb.append(args[0].charAt(0));
sb.append(args[0].charAt(2));
sb.append(args[0].charAt(i));
sb.append(args[0].substring(i - 3, i - 2));
sb.append(args[0].charAt(i - 3));
sb.append(args[0].substring(i - 3, 2 - i));
sb.append(args[0].substring(3 - i, i - 2));
sb.append(args[0].substring(3 - i, 2 - i));
sb.append(args[0].charAt(2 - i));
sb.append(args[0].substring(2 - i, 4 - i));
sb.append(args[0].substring(i - 2, i - 4));
@@ -40,5 +42,22 @@ class Foo {
System.out.print(args[0].substring(2, 4));
System.out.println(args[0].substring(2, 4));
sb.append(args[0].charAt(i + j + 1));
sb.append(args[0].charAt(i + j + (j + 1)));
sb.append(args[0].substring(i + j + 2 * j, i + j + 3 * j));
sb.append(args[0].substring(i + j + 3 * j, i + j + 2 * j));
sb.append(args[0].charAt(i - j - 2));
sb.append(args[0].charAt(i - (j + 2)));
sb.append(args[0].charAt(2 - (j + 2)));
sb.append(args[0].charAt(2 - ((2 - i) - j)));
sb.append(args[0].charAt(i - (-2*j + 3)));
sb.append(args[0].substring(i + j * 2, i + j * 3));
sb.append(args[0].substring(i + j * 3, i + j * 2));
sb.append(args[0].charAt(i - ((j - 1) - (j + 1))));
}
}
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantStringOperation/afterSubstringOneCharInEquals.java
+14 -4
View File
@@ -1,12 +1,22 @@
// "Fix all 'Redundant String operation' problems in file" "true"
class Foo {
public static void main(String[] args) {
int i = Integer.parseInt(args[2]);
int j = Integer.parseInt(args[4]);
boolean value = args[0].charAt(4) == '_';
if(args[0].charAt(4) == '_') { }
if(args[0].charAt(4) != '_') { }
if(args[0].charAt(4) == '_') { }
if(args[0].charAt(4) != '_') { }
if (args[0].charAt(4) == '_') { }
if (args[0].charAt(4) != '_') { }
if (args[0].charAt(4) == '_') { }
if (args[0].charAt(4) != '_') { }
if (/* one */args/* two */[/* three */ 0 /* four */].charAt(i /* five */ + 1) == 'x') { }
/* one */
if (args/* two */[/* three */ 0 /* four */].charAt(i /* five */ + 1) != 'x') { }
/* one */
if (args/* two */[/* three */ 0 /* four */].charAt(i /* five */ + 1) != 'x') { }
if (args[0].charAt(i - ((j - 1) - (j + 1))) == '\'') {}
}
}
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantStringOperation/beforeSubstringOneCharInAppend.java
+20 -1
View File
@@ -3,7 +3,8 @@ class Foo {
public static void main(String[] args) {
StringBuilder sb = new StringBuilder();
int i = Integer.parseInt(args[4]);
int i = Integer.parseInt(args[3]);
int j = Integer.parseInt(args[4]);
sb.append(args[0].sub<caret>string(0, 1));
sb.append(args[0].substring(2, 3));
@@ -13,6 +14,7 @@ class Foo {
sb.append(args[0].substring(i - 3, 2 - i));
sb.append(args[0].substring(3 - i, i - 2));
sb.append(args[0].substring(3 - i, 2 - i));
sb.append(args[0].substring(2 - i, 3 - i));
sb.append(args[0].substring(2 - i, 4 - i));
sb.append(args[0].substring(i - 2, i - 4));
@@ -40,5 +42,22 @@ class Foo {
System.out.print(args[0].substring(2, 4));
System.out.println(args[0].substring(2, 4));
sb.append(args[0].substring(i + j + 1, i + j + 2));
sb.append(args[0].substring(i + j + (j + 1), i + j + (j + 2)));
sb.append(args[0].substring(i + j + 2 * j, i + j + 3 * j));
sb.append(args[0].substring(i + j + 3 * j, i + j + 2 * j));
sb.append(args[0].substring(i - j - 2, i - j - 1));
sb.append(args[0].substring(i - (j + 2), i - (j + 1)));
sb.append(args[0].substring(2 - (j + 2), 3 - (j + 2)));
sb.append(args[0].substring(2 - ((2 - i) - j), 3 - ((2 - i) - j)));
sb.append(args[0].substring(i - (-2*j + 3), i - (-2*j + 2) ));
sb.append(args[0].substring(i + j * 2, i + j * 3));
sb.append(args[0].substring(i + j * 3, i + j * 2));
sb.append(args[0].substring(i - ((j - 1) - (j + 1)), i - ((j - 2) - (j + 1))));
}
}
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantStringOperation/beforeSubstringOneCharInEquals.java
+12 -4
View File
@@ -1,12 +1,20 @@
// "Fix all 'Redundant String operation' problems in file" "true"
class Foo {
public static void main(String[] args) {
int i = Integer.parseInt(args[2]);
int j = Integer.parseInt(args[4]);
boolean value = args[0].substring(4, 5).equals("_");
if(args[0].sub<caret>string(4, 5).equals("_")) { }
if(!args[0].substring(4, 5).equals("_")) { }
if(!!args[0].substring(4, 5).equals("_")) { }
if(!!!!!args[0].substring(4, 5).equals("_")) { }
if (args[0].sub<caret>string(4, 5).equals("_")) { }
if (!args[0].substring(4, 5).equals("_")) { }
if (!!args[0].substring(4, 5).equals("_")) { }
if (!!!!!args[0].substring(4, 5).equals("_")) { }
if (/* one */args/* two */[/* three */ 0 /* four */].substring(i /* five */ + 1, i + 2).equals("x")) { }
if (!/* one */args/* two */[/* three */ 0 /* four */].substring(i /* five */ + 1, i + 2).equals("x")) { }
if (!/* one */(!!args/* two */[/* three */ 0 /* four */].substring(i /* five */ + 1, i + 2).equals("x"))) { }
if (args[0].substring(i - ((j - 1) - (j + 1)), i - ((j - 2) - (j + 1))).equals("\'")) {}
}
}
plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ExpressionUtils.java
+68 -8
View File
@@ -495,12 +495,15 @@ public class ExpressionUtils {
/**
* The method checks if the passed expression does not need to be converted to string explicitly,
* because the method it is passed to can do the string conversion itself.
* because the containing expression (e.g. a {@code PrintStream#println} call or string concatenation expression)
* will convert to the string automatically.
*
* This is the case for some StringBuilder/Buffer, PrintStream/Writer and some logging methods.
* Otherwise it considers the conversion necessary and returns true
* Otherwise it considers the conversion necessary and returns true.
*
* @param expression an expression to examine
* @param throwable is the first parameter a conversion to string on a throwable? either {@link Throwable#toString()} or {@link String#valueOf(Object)}
* @param throwable is the first parameter a conversion to string on a throwable? Either {@link Throwable#toString()}
* or {@link String#valueOf(Object)}
*
* @return true if the explicit conversion to string is not required, otherwise - false
*/
@@ -1158,14 +1161,16 @@ public class ExpressionUtils {
public static boolean isDifference(@NotNull PsiExpression from, @NotNull PsiExpression to, @NotNull PsiExpression diff) {
diff = PsiUtil.skipParenthesizedExprDown(diff);
if (diff == null) return false;
EquivalenceChecker eq = EquivalenceChecker.getCanonicalPsiEquivalence();
if (isZero(from) && eq.expressionsAreEquivalent(to, diff)) return true;
if (isZero(diff) && eq.expressionsAreEquivalent(to, from)) return true;
if (to instanceof PsiBinaryExpression && from instanceof PsiBinaryExpression) {
final Pair<@NotNull PsiExpression, @NotNull PsiExpression> binaryExpressionsDiff = getBinaryExpressionsDiff((PsiBinaryExpression)from,
(PsiBinaryExpression)to);
from = binaryExpressionsDiff.first;
to = binaryExpressionsDiff.second;
if (to instanceof PsiPolyadicExpression && from instanceof PsiPolyadicExpression) {
final Pair<@NotNull PsiExpression, @NotNull PsiExpression> polyadicDiff = getPolyadicDiff(((PsiPolyadicExpression)from),
((PsiPolyadicExpression)to));
from = polyadicDiff.first;
to = polyadicDiff.second;
}
if (diff instanceof PsiBinaryExpression && ((PsiBinaryExpression)diff).getOperationTokenType().equals(JavaTokenType.MINUS)) {
PsiExpression left = ((PsiBinaryExpression)diff).getLOperand();
@@ -1198,6 +1203,61 @@ public class ExpressionUtils {
return diffConstant == toConstant - fromConstant;
}
/**
* Get a diff from two {@link PsiPolyadicExpression} instances using {@link EquivalenceChecker}.
* @param from the first expression to examine
* @param to the second expression to examine
* @return a pair of expressions without common parts if the original expressions had any,
* or the original expressions if no common parts were found
*/
@NotNull
private static Pair<@NotNull PsiExpression, @NotNull PsiExpression> getPolyadicDiff(@NotNull final PsiPolyadicExpression from,
@NotNull final PsiPolyadicExpression to) {
final EquivalenceChecker eq = EquivalenceChecker.getCanonicalPsiEquivalence();
final EquivalenceChecker.Match match = eq.expressionsMatch(from, to);
if (match.isPartialMatch()) {
final PsiExpression leftDiff = PsiUtil.skipParenthesizedExprDown((PsiExpression)match.getLeftDiff());
final PsiExpression rightDiff = PsiUtil.skipParenthesizedExprDown((PsiExpression)match.getRightDiff());
if (leftDiff == null || rightDiff == null) return Pair.create(from, to);
final PsiPolyadicExpression leftParent = PsiTreeUtil.getParentOfType(leftDiff, PsiPolyadicExpression.class);
assert leftParent != null;
final IElementType op = leftParent.getOperationTokenType();
if (op == JavaTokenType.MINUS || op == JavaTokenType.PLUS) {
if (shouldBeInverted(leftDiff, from)) return Pair.create(rightDiff, leftDiff);
else return Pair.create(leftDiff, rightDiff);
}
}
return Pair.create(from, to);
}
/**
* The method checks all the algebraic rules for subtraction to decide
* if the operand comes into the expression with the negative or positive sign
* by traversing the PSI tree going up to the specified element.
*
* @param start the operand to check the sign for
* @param end the end element to stop traversal
* @return true if the operand comes into the expression with the positive sign, otherwise false
*/
private static boolean shouldBeInverted(@NotNull PsiElement start, @NotNull final PsiElement end) {
boolean result = false;
PsiElement parent = start;
while (parent != end) {
start = parent;
parent = parent.getParent();
if (!(parent instanceof PsiPolyadicExpression)) continue;
final IElementType op = ((PsiPolyadicExpression)parent).getOperationTokenType();
if (op == JavaTokenType.MINUS && parent.getFirstChild() != start) {
result = !result;
}
}
return result;
}
/**
* Eliminate the common part from two {@link PsiBinaryExpression} expressions.
* It only handles {@link PsiBinaryExpression} with {@link JavaTokenType#PLUS}
plugins/InspectionGadgets/src/com/siyeh/ig/redundancy/RedundantStringOperationInspection.java
+60 -37
View File
@@ -30,6 +30,7 @@ import javax.swing.*;
import java.util.Collections;
import java.util.List;
import java.util.Objects;
import java.util.function.Function;
import static com.intellij.psi.CommonClassNames.JAVA_LANG_OBJECT;
import static com.intellij.psi.CommonClassNames.JAVA_LANG_STRING;
@@ -221,49 +222,56 @@ public class RedundantStringOperationInspection extends AbstractBaseJavaLocalIns
*/
@NotNull
private ProblemDescriptor createSubstringToCharAtProblemDescriptor(@NotNull final PsiMethodCallExpression call) {
final PsiMethodCallExpression qualifierCall = MethodCallUtils.getQualifierMethodCall(call);
assert qualifierCall != null : "The method is meant to be called only from getRedundantSubstringEqualsProblem";
final PsiExpression receiver = qualifierCall.getMethodExpression().getQualifierExpression();
assert receiver != null : "The method is meant to be called only from getRedundantSubstringEqualsProblem";
final PsiExpression[] args = qualifierCall.getArgumentList().getExpressions();
assert args.length == 2 : "The method is meant to be called only from getRedundantSubstringEqualsProblem";
final PsiExpression equalTo = call.getArgumentList().getExpressions()[0];
final String equalToValue = PsiLiteralUtil.charLiteralForCharString(equalTo.getText());
final Pair<@NotNull PsiExpression, @NotNull Boolean> sign = getExpressionSign(call);
final PsiElement outermostEqualsExpr = sign.fst;
final String eqSign = sign.snd ? "==" : "!=";
final String converted = String.format("%s.charAt(%s) %s %s",
receiver.getText(),
args[0].getText(),
eqSign,
equalToValue
);
final String converted = getTargetString(call, null);
assert converted != null : "Message cannot be null";
final PsiElement outermostEqualsExpr = getOutermostEquals(call);
final SubstringEqualsToCharAtEqualsQuickFix fix = new SubstringEqualsToCharAtEqualsQuickFix(outermostEqualsExpr.getText(),
converted);
return myManager.createProblemDescriptor(outermostEqualsExpr,
InspectionGadgetsBundle.message("inspection.x.call.can.be.replaced.with.y", "substring()", "charAt()"),
fix,
ProblemHighlightType.LIKE_UNUSED_SYMBOL, myIsOnTheFly);
ProblemHighlightType.GENERIC_ERROR_OR_WARNING, myIsOnTheFly);
}
@NotNull
private static Pair<@NotNull PsiExpression, @NotNull Boolean> getExpressionSign(@NotNull PsiExpression start) {
boolean sign = true;
PsiExpression expr = start;
while (true) {
final PsiPrefixExpression negation = (PsiPrefixExpression) BoolUtils.findNegation(expr);
if (negation == null) break;
sign = !sign;
expr = negation;
}
return Pair.of(expr, sign);
@Nullable
private static String getTargetString(@NotNull final PsiMethodCallExpression call,
@Nullable Function<@NotNull PsiElement, @NotNull String> textExtractor) {
if (textExtractor == null) textExtractor = PsiElement::getText;
final PsiMethodCallExpression qualifierCall = MethodCallUtils.getQualifierMethodCall(call);
if (qualifierCall == null) return null;
final PsiExpression receiver = qualifierCall.getMethodExpression().getQualifierExpression();
if (receiver == null) return null;
final PsiExpression[] args = qualifierCall.getArgumentList().getExpressions();
if (args.length != 2) return null;
final PsiExpression equalTo = call.getArgumentList().getExpressions()[0];
final String eqSign = isNegated(call, false) ? "!=" : "==";
final String equalToValue = PsiLiteralUtil.charLiteralForCharString(textExtractor.apply(equalTo));
return String.format("%s.charAt(%s) %s %s",
textExtractor.apply(receiver),
textExtractor.apply(args[0]),
eqSign,
equalToValue
);
}
private static boolean isNegated(@NotNull final PsiExpression start, boolean negated) {
final PsiExpression negation = BoolUtils.findNegation(start);
if (negation == null) return negated;
else return isNegated(negation, !negated);
}
private static PsiExpression getOutermostEquals(@NotNull final PsiExpression start) {
final PsiExpression negation = BoolUtils.findNegation(start);
if (negation == null) return start;
else return getOutermostEquals(negation);
}
private boolean lengthMatches(PsiExpression equalTo, PsiExpression from, PsiExpression to) {
@@ -383,7 +391,7 @@ public class RedundantStringOperationInspection extends AbstractBaseJavaLocalIns
substring.getStartOffsetInParent() + substring.getTextLength());
return myManager.createProblemDescriptor(call, textRange,
CommonQuickFixBundle.message("fix.replace.x.with.y", call.getText(), converted),
ProblemHighlightType.LIKE_UNUSED_SYMBOL, myIsOnTheFly,
ProblemHighlightType.GENERIC_ERROR_OR_WARNING, myIsOnTheFly,
fix);
}
PsiElement parent = PsiUtil.skipParenthesizedExprUp(call.getParent());
@@ -529,8 +537,23 @@ public class RedundantStringOperationInspection extends AbstractBaseJavaLocalIns
final PsiElement element = descriptor.getPsiElement();
if (element == null) return;
final PsiMethodCallExpression call;
if (element instanceof PsiMethodCallExpression) {
call = (PsiMethodCallExpression)element;
}
else {
// Strip PsiPrefixExpression
call = PsiTreeUtil.findChildOfType(element, PsiMethodCallExpression.class);
}
if (call == null) return;
final CommentTracker ct = new CommentTracker();
ct.replaceAndRestoreComments(element, myConverted);
final String convertTo = getTargetString(call, ct::text);
if (convertTo == null) return;
ct.replaceAndRestoreComments(element, convertTo);
}
}