diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/style/StringBufferReplaceableByStringInspection.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/style/StringBufferReplaceableByStringInspection.java index 7e096d987ffa..d3edd0956d4e 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/style/StringBufferReplaceableByStringInspection.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/style/StringBufferReplaceableByStringInspection.java @@ -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 expressions = new ArrayList(); + private final List 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; diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/performance/string_buffer_replaceable_by_string/StringBufferReplaceableByString.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/performance/string_buffer_replaceable_by_string/StringBufferReplaceableByString.java index 10a389fa9032..266c094e5383 100644 --- a/plugins/InspectionGadgets/test/com/siyeh/igtest/performance/string_buffer_replaceable_by_string/StringBufferReplaceableByString.java +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/performance/string_buffer_replaceable_by_string/StringBufferReplaceableByString.java @@ -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; + } + } } diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/performance/string_buffer_replaceable_by_string/expected.xml b/plugins/InspectionGadgets/test/com/siyeh/igtest/performance/string_buffer_replaceable_by_string/expected.xml index 9d6c57a431fb..73ad82fabbdb 100644 --- a/plugins/InspectionGadgets/test/com/siyeh/igtest/performance/string_buffer_replaceable_by_string/expected.xml +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/performance/string_buffer_replaceable_by_string/expected.xml @@ -36,4 +36,11 @@ <code>StringBuilder d</code> can be replaced with 'String' #loc + + StringBufferReplaceableByString.java + 96 + 'StringBuffer' can be replaced with 'String' + <code>StringBuilder builder</code> can be replaced with 'String' #loc + + \ No newline at end of file