IDEA-114997 (IDEA 13: inspection to replace StringBuilder with String is incorrect)

This commit is contained in:
Bas Leijdekkers
2013-10-17 11:33:40 +02:00
parent 538082257d
commit 3710d76b01
3 changed files with 121 additions and 8 deletions
@@ -21,6 +21,7 @@ import com.intellij.openapi.util.text.StringUtil;
import com.intellij.psi.*;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.util.IncorrectOperationException;
import com.intellij.util.containers.ContainerUtil;
import com.siyeh.InspectionGadgetsBundle;
import com.siyeh.ig.BaseInspection;
import com.siyeh.ig.BaseInspectionVisitor;
@@ -31,7 +32,6 @@ import org.jetbrains.annotations.NonNls;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import java.util.ArrayList;
import java.util.List;
public class StringBufferReplaceableByStringInspection extends BaseInspection {
@@ -257,7 +257,7 @@ public class StringBufferReplaceableByStringInspection extends BaseInspection {
private final PsiVariable myVariable;
private final StringBuilder myBuilder;
private final List<PsiMethodCallExpression> expressions = new ArrayList();
private final List<PsiMethodCallExpression> expressions = ContainerUtil.newArrayList();
private boolean myProblem = false;
public StringBuildingVisitor(@NotNull PsiVariable variable, StringBuilder builder) {
@@ -425,8 +425,9 @@ public class StringBufferReplaceableByStringInspection extends BaseInspection {
private static class ReplaceableByStringVisitor extends JavaRecursiveElementVisitor {
private final PsiElement myParent;
private PsiVariable myVariable;
private final PsiVariable myVariable;
private boolean myReplaceable = true;
private boolean myPossibleSideEffect = false;
private boolean myToStringFound = false;
public ReplaceableByStringVisitor(@NotNull PsiVariable variable) {
@@ -450,7 +451,7 @@ public class StringBufferReplaceableByStringInspection extends BaseInspection {
public void visitAssignmentExpression(PsiAssignmentExpression expression) {
super.visitAssignmentExpression(expression);
if (expression.getTextOffset() > myVariable.getTextOffset() && !myToStringFound) {
myReplaceable = false;
myPossibleSideEffect = true;
}
}
@@ -458,7 +459,7 @@ public class StringBufferReplaceableByStringInspection extends BaseInspection {
public void visitPostfixExpression(PsiPostfixExpression expression) {
super.visitPostfixExpression(expression);
if (expression.getTextOffset() > myVariable.getTextOffset() && !myToStringFound) {
myReplaceable = false;
myPossibleSideEffect = true;
}
}
@@ -466,10 +467,77 @@ public class StringBufferReplaceableByStringInspection extends BaseInspection {
public void visitPrefixExpression(PsiPrefixExpression expression) {
super.visitPrefixExpression(expression);
if (expression.getTextOffset() > myVariable.getTextOffset() && !myToStringFound) {
myReplaceable = false;
myPossibleSideEffect = true;
}
}
@Override
public void visitMethodCallExpression(PsiMethodCallExpression expression) {
super.visitMethodCallExpression(expression);
if (expression.getTextOffset() < myVariable.getTextOffset() || myToStringFound) {
return;
}
final PsiMethod method = expression.resolveMethod();
if (method == null) {
myPossibleSideEffect = true;
return;
}
final PsiClass aClass = method.getContainingClass();
if (aClass == null) {
myPossibleSideEffect = true;
return;
}
final String name = aClass.getQualifiedName();
if (CommonClassNames.JAVA_LANG_STRING_BUFFER.equals(name) ||
CommonClassNames.JAVA_LANG_STRING_BUILDER.equals(name)) {
return;
}
if (isArgumentOfStringBuilderMethod(expression)) {
return;
}
myPossibleSideEffect = true;
}
private boolean isArgumentOfStringBuilderMethod(PsiMethodCallExpression expression) {
final PsiElement parent = expression.getParent();
if (!(parent instanceof PsiExpressionList)) {
return false;
}
final PsiElement grandParent = parent.getParent();
if (!(grandParent instanceof PsiMethodCallExpression)) {
return false;
}
final PsiMethodCallExpression methodCallExpression = (PsiMethodCallExpression)grandParent;
final PsiReferenceExpression methodExpression = methodCallExpression.getMethodExpression();
PsiExpression qualifier = methodExpression.getQualifierExpression();
while (qualifier instanceof PsiMethodCallExpression) {
final PsiMethodCallExpression callExpression = (PsiMethodCallExpression)qualifier;
final PsiReferenceExpression methodExpression1 = callExpression.getMethodExpression();
qualifier = methodExpression1.getQualifierExpression();
}
if (qualifier instanceof PsiReferenceExpression) {
final PsiReferenceExpression referenceExpression = (PsiReferenceExpression)qualifier;
final PsiElement target = referenceExpression.resolve();
if (!myVariable.equals(target)) {
return false;
}
}
final PsiMethod method = methodCallExpression.resolveMethod();
if (method == null) {
return false;
}
final PsiClass aClass = method.getContainingClass();
if (aClass == null) {
return false;
}
final String name1 = aClass.getQualifiedName();
if (CommonClassNames.JAVA_LANG_STRING_BUFFER.equals(name1) ||
CommonClassNames.JAVA_LANG_STRING_BUILDER.equals(name1)) {
return true;
}
return false;
}
@Override
public void visitReferenceExpression(PsiReferenceExpression expression) {
if (!myReplaceable || expression.getTextOffset() < myVariable.getTextOffset()) {
@@ -508,6 +576,10 @@ public class StringBufferReplaceableByStringInspection extends BaseInspection {
myToStringFound = true;
return;
}
if (myPossibleSideEffect) {
myReplaceable = false;
return;
}
parent = grandParent.getParent();
if (parent instanceof PsiExpressionStatement) {
return;
@@ -39,12 +39,12 @@ public class StringBufferReplaceableByString {
public void assignment(int p) {
StringBuilder b = new StringBuilder();
b.append(p);
p++;
b.append(p);
System.out.println(b.toString());
StringBuilder c = new StringBuilder();
c.append(p);
p = 2;
c.append(p);
System.out.println(c.toString());
StringBuilder d = new StringBuilder();
@@ -79,4 +79,38 @@ public class StringBufferReplaceableByString {
data.append(String.format("%02d:%02d", Math.abs(hours), min));
return data.toString();
}
class HighlightStaticImport {
void example1() {
System.out.println();
final StringBuilder builder = new StringBuilder();
builder.append(foo1());
b(); // side effect
builder.append(foo1());
bar(builder.toString());
System.out.println();
}
void example2() {
final StringBuilder builder = new StringBuilder();
builder.append(foo1());
b(); // side effect, but has no effect on builder anymore
bar(builder.toString());
}
String s;
void b() {
s = "asdf";
}
private void bar(String s) {
}
private String foo1() {
return null;
}
}
}
@@ -36,4 +36,11 @@
<description>&lt;code&gt;StringBuilder d&lt;/code&gt; can be replaced with 'String' #loc</description>
</problem>
<problem>
<file>StringBufferReplaceableByString.java</file>
<line>96</line>
<problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">'StringBuffer' can be replaced with 'String'</problem_class>
<description>&lt;code&gt;StringBuilder builder&lt;/code&gt; can be replaced with 'String' #loc</description>
</problem>
</problems>