RedundantStringOperationInspection: warn about append(s.substring(x, y))

Replaceable with append(s, x, y)
Fixes IDEA-185652 Improve StringBulder.append(s.substring(1,2)) intention
Also optimization of new expression resolution
This commit is contained in:
Tagir Valeev
2018-01-29 13:34:55 +07:00
parent c741500753
commit 2f547ca38f
4 changed files with 90 additions and 29 deletions
@@ -0,0 +1,7 @@
// "Remove redundant 'substring()' call" "true"
class Foo {
public static void main(String[] args) {
StringBuilder sb = new StringBuilder();
sb.append(args[0], 3, 4);
}
}
@@ -0,0 +1,7 @@
// "Remove redundant 'substring()' call" "true"
class Foo {
public static void main(String[] args) {
StringBuilder sb = new StringBuilder();
sb.append(args[0].subs<caret>tring(3, 4));
}
}
@@ -15,6 +15,7 @@
*/
package com.siyeh.ig.psiutils;
import com.intellij.openapi.util.text.StringUtil;
import com.intellij.psi.*;
import com.intellij.psi.util.PsiUtil;
import com.intellij.util.containers.ContainerUtil;
@@ -59,13 +60,7 @@ public class ConstructionUtils {
if (!(construction instanceof PsiNewExpression)) return null;
final PsiNewExpression newExpression = (PsiNewExpression)construction;
final PsiJavaCodeReferenceElement classReference = newExpression.getClassReference();
if (classReference == null) return null;
final PsiElement target = classReference.resolve();
if (!(target instanceof PsiClass)) return null;
final PsiClass aClass = (PsiClass)target;
final String qualifiedName = aClass.getQualifiedName();
if (!CommonClassNames.JAVA_LANG_STRING_BUILDER.equals(qualifiedName) &&
!CommonClassNames.JAVA_LANG_STRING_BUFFER.equals(qualifiedName)) {
if (!isReferenceTo(classReference, CommonClassNames.JAVA_LANG_STRING_BUILDER, CommonClassNames.JAVA_LANG_STRING_BUFFER)) {
return null;
}
final PsiExpressionList argumentList = newExpression.getArgumentList();
@@ -184,4 +179,21 @@ public class ConstructionUtils {
}
return true;
}
public static boolean isReferenceTo(PsiJavaCodeReferenceElement ref, String... classNames) {
if(ref == null) return false;
String name = ref.getReferenceName();
if (name == null) return false;
String qualifiedName = null;
for (String className : classNames) {
if(StringUtil.getShortName(className).equals(name)) {
if (qualifiedName == null) {
// Defer resolution if possible
qualifiedName = ref.getQualifiedName();
}
if (className.equals(qualifiedName)) return true;
}
}
return false;
}
}
@@ -8,7 +8,6 @@ import com.intellij.openapi.util.TextRange;
import com.intellij.psi.*;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.psi.util.PsiUtil;
import com.intellij.util.ObjectUtils;
import com.siyeh.InspectionGadgetsBundle;
import com.siyeh.ig.callMatcher.CallMatcher;
import com.siyeh.ig.psiutils.*;
@@ -21,9 +20,15 @@ import java.util.List;
import java.util.Objects;
import static com.intellij.psi.CommonClassNames.JAVA_LANG_STRING;
import static com.intellij.util.ObjectUtils.tryCast;
import static com.siyeh.InspectionGadgetsBundle.BUNDLE;
public class RedundantStringOperationInspection extends AbstractBaseJavaLocalInspectionTool implements CleanupLocalInspectionTool {
enum FixType {
REPLACE_WITH_QUALIFIER,
REPLACE_WITH_ARGUMENTS
}
private static final CallMatcher STRING_TO_STRING = CallMatcher.instanceCall(JAVA_LANG_STRING, "toString").parameterCount(0);
private static final CallMatcher STRING_INTERN = CallMatcher.instanceCall(JAVA_LANG_STRING, "intern").parameterCount(0);
private static final CallMatcher STRING_LENGTH = CallMatcher.instanceCall(JAVA_LANG_STRING, "length").parameterCount(0);
@@ -67,9 +72,7 @@ public class RedundantStringOperationInspection extends AbstractBaseJavaLocalIns
@Override
public void visitNewExpression(PsiNewExpression expression) {
PsiJavaCodeReferenceElement classRef = expression.getClassReference();
if (classRef == null) return;
String className = classRef.getQualifiedName();
if (CommonClassNames.JAVA_LANG_STRING_BUILDER.equals(className) || CommonClassNames.JAVA_LANG_STRING_BUFFER.equals(className)) {
if (ConstructionUtils.isReferenceTo(classRef, CommonClassNames.JAVA_LANG_STRING_BUILDER, CommonClassNames.JAVA_LANG_STRING_BUFFER)) {
checkUnnecessaryEmptyStringArgument(expression);
}
}
@@ -94,24 +97,41 @@ public class RedundantStringOperationInspection extends AbstractBaseJavaLocalIns
private void processSubstring(PsiMethodCallExpression call) {
PsiExpression[] args = call.getArgumentList().getExpressions();
if (!ExpressionUtils.isZero(args[0])) return;
if (args.length == 2) {
PsiMethodCallExpression argCall =
ObjectUtils.tryCast(PsiUtil.skipParenthesizedExprDown(args[1]), PsiMethodCallExpression.class);
if (!STRING_LENGTH.test(argCall) ||
!EquivalenceChecker.getCanonicalPsiEquivalence()
.expressionsAreEquivalent(call.getMethodExpression().getQualifierExpression(),
argCall.getMethodExpression().getQualifierExpression())) {
return;
if (isRedundantSubstring(call, args)) {
registerProblem(call, "inspection.redundant.string.call.message");
}
else if (args.length == 2) {
PsiElement parent = PsiUtil.skipParenthesizedExprUp(call.getParent());
if (parent instanceof PsiExpressionList && ((PsiExpressionList)parent).getExpressionCount() == 1) {
PsiMethodCallExpression parentCall = tryCast(parent.getParent(), PsiMethodCallExpression.class);
if (STRING_BUILDER_APPEND.test(parentCall)) {
PsiElement nameElement = Objects.requireNonNull(call.getMethodExpression().getReferenceNameElement());
holder.registerProblem(nameElement, InspectionGadgetsBundle.message("inspection.redundant.string.call.message"),
ProblemHighlightType.LIKE_UNUSED_SYMBOL,
new RemoveRedundantStringCallFix(nameElement.getText(), FixType.REPLACE_WITH_ARGUMENTS));
}
}
}
registerProblem(call, "inspection.redundant.string.call.message");
}
private boolean isRedundantSubstring(PsiMethodCallExpression call, PsiExpression[] args) {
if (!ExpressionUtils.isZero(args[0])) return false;
if (args.length == 2) {
PsiMethodCallExpression argCall = tryCast(PsiUtil.skipParenthesizedExprDown(args[1]), PsiMethodCallExpression.class);
if (!STRING_LENGTH.test(argCall) ||
!EquivalenceChecker.getCanonicalPsiEquivalence()
.expressionsAreEquivalent(call.getMethodExpression().getQualifierExpression(),
argCall.getMethodExpression().getQualifierExpression())) {
return false;
}
}
return true;
}
private void registerProblem(PsiMethodCallExpression call, @NotNull @PropertyKey(resourceBundle = BUNDLE) String key) {
String name = call.getMethodExpression().getReferenceName();
holder.registerProblem(call, InspectionGadgetsBundle.message(key),
ProblemHighlightType.LIKE_UNUSED_SYMBOL, getRange(call), new ReplaceWithQualifierFix(name));
holder.registerProblem(call, InspectionGadgetsBundle.message(key), ProblemHighlightType.LIKE_UNUSED_SYMBOL, getRange(call),
new RemoveRedundantStringCallFix(name, FixType.REPLACE_WITH_QUALIFIER));
}
};
}
@@ -127,11 +147,13 @@ public class RedundantStringOperationInspection extends AbstractBaseJavaLocalIns
return call.getTextRange();
}
private static class ReplaceWithQualifierFix implements LocalQuickFix {
private String myToRemove;
private static class RemoveRedundantStringCallFix implements LocalQuickFix {
private final FixType myFixType;
private final String myToRemove;
public ReplaceWithQualifierFix(String toRemove) {
public RemoveRedundantStringCallFix(String toRemove, FixType fixType) {
myToRemove = toRemove;
myFixType = fixType;
}
@Nls
@@ -154,9 +176,22 @@ public class RedundantStringOperationInspection extends AbstractBaseJavaLocalIns
if (call == null) return;
PsiExpression qualifier = ExpressionUtils.getQualifierOrThis(call.getMethodExpression());
CommentTracker ct = new CommentTracker();
PsiExpression result = (PsiExpression)ct.replaceAndRestoreComments(call, ct.markUnchanged(qualifier));
if (result.getParent() instanceof PsiExpressionStatement) {
extractSideEffects(result, (PsiExpressionStatement)result.getParent());
switch (myFixType) {
case REPLACE_WITH_QUALIFIER: {
PsiExpression result = (PsiExpression)ct.replaceAndRestoreComments(call, ct.markUnchanged(qualifier));
if (result.getParent() instanceof PsiExpressionStatement) {
extractSideEffects(result, (PsiExpressionStatement)result.getParent());
}
break;
}
case REPLACE_WITH_ARGUMENTS:
PsiExpressionList list = tryCast(PsiUtil.skipParenthesizedExprUp(call.getParent()), PsiExpressionList.class);
if (list == null) return;
for (PsiExpression arg : call.getArgumentList().getExpressions()) {
list.add(ct.markUnchanged(arg));
}
ct.replaceAndRestoreComments(call, ct.markUnchanged(qualifier));
break;
}
}