MDS/openide
1
0
Fork 0
mirror of https://gitflic.ru/project/openide/openide.git synced 2026-04-19 21:11:28 +07:00
Code Issues Packages Projects Releases Wiki Activity

IDEA-165193 Provide a quick-fix to replace String concatenation in loop with StringBuilder

Browse Source
This commit is contained in:
Tagir Valeev
2016-12-08 15:55:26 +07:00
parent de76e7ed3a
commit e7e0982841
15 changed files with 457 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/stringConcatInLoop/afterConcatInversed.java Normal file
View File

@@ -0,0 +1,14 @@
// "Change type of 'res' to StringBuilder" "true"
public class Main {
String test(String[] strings) {
StringBuilder res = new StringBuilder();
for (String s : strings) {
if (res.length() > 0) {
res.insert(0, ".");
}
res.insert(0, s);
}
return res.toString();
}
}

15
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/stringConcatInLoop/afterConcatNull.java Normal file
View File

@@ -0,0 +1,15 @@
// "Change type of 'res' to StringBuilder" "true"
public class Main {
String test(String[] strings) {
StringBuilder res = null;
for (String s : strings) {
if(res == null) {
res = new StringBuilder(s);
} else {
res.append(s);
}
}
return res.toString();
}
}

15
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/stringConcatInLoop/afterConcatPrint.java Normal file
View File

@@ -0,0 +1,15 @@
// "Change type of 'res' to StringBuilder" "true"
public class Main {
void test(String[] strings) {
StringBuilder res = new StringBuilder();
for (String s : strings) {
if(res.length()+s.length() > 80) {
System.out.println(res);
res = new StringBuilder();
}
res.append(s);
}
System.out.println(res);
}
}

11
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/stringConcatInLoop/afterConcatSimple.java Normal file
View File

@@ -0,0 +1,11 @@
// "Change type of 'res' to StringBuilder" "true"
public class Main {
String test(String[] strings) {
StringBuilder res = new StringBuilder();
for (String s : strings) {
res.append(s);
}
return res.toString();
}
}

12
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/stringConcatInLoop/afterConcatTrim.java Normal file
View File

@@ -0,0 +1,12 @@
// "Change type of 'res' to StringBuilder" "true"
public class Main {
String test(String[] strings) {
StringBuilder res = new StringBuilder();
for (String s : strings) {
res.append(s);
}
res = new StringBuilder(res.toString().trim());
return (res.length() == 0) ? null : res.toString();
}
}

15
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/stringConcatInLoop/afterConcatWithDelimiter.java Normal file
View File

@@ -0,0 +1,15 @@
// "Change type of 'res' to StringBuilder" "true"
public class Main {
String test(String[] strings) {
/*within*/
StringBuilder res = new StringBuilder();
for (String s : strings) {
if (/*before*/res.length() > 0) {
res.append(", ");
}
res.append(s);
}
return res.toString();
}
}

14
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/stringConcatInLoop/beforeConcatInversed.java Normal file
View File

@@ -0,0 +1,14 @@
// "Change type of 'res' to StringBuilder" "true"
public class Main {
String test(String[] strings) {
String res = "";
for (String s : strings) {
if (!res.isEmpty()) {
res = "." + res;
}
res = s <caret>+ res;
}
return res;
}
}

15
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/stringConcatInLoop/beforeConcatNull.java Normal file
View File

@@ -0,0 +1,15 @@
// "Change type of 'res' to StringBuilder" "true"
public class Main {
String test(String[] strings) {
String res = null;
for (String s : strings) {
if(res == null) {
res = s;
} else {
res<caret>+=s;
}
}
return res;
}
}

15
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/stringConcatInLoop/beforeConcatPrint.java Normal file
View File

@@ -0,0 +1,15 @@
// "Change type of 'res' to StringBuilder" "true"
public class Main {
void test(String[] strings) {
String res = "";
for (String s : strings) {
if(res.length()+s.length() > 80) {
System.out.println(res);
res = "";
}
res<caret>+=s;
}
System.out.println(res);
}
}

11
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/stringConcatInLoop/beforeConcatSimple.java Normal file
View File

@@ -0,0 +1,11 @@
// "Change type of 'res' to StringBuilder" "true"
public class Main {
String test(String[] strings) {
String res = "";
for (String s : strings) {
res <caret>+= s;
}
return res;
}
}

12
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/stringConcatInLoop/beforeConcatTrim.java Normal file
View File

@@ -0,0 +1,12 @@
// "Change type of 'res' to StringBuilder" "true"
public class Main {
String test(String[] strings) {
String res = "";
for (String s : strings) {
res <caret>+= s;
}
res = res.trim();
return res.isEmpty() ? null : res;
}
}

14
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/stringConcatInLoop/beforeConcatWithDelimiter.java Normal file
View File

@@ -0,0 +1,14 @@
// "Change type of 'res' to StringBuilder" "true"
public class Main {
String test(String[] strings) {
String res = "";
for (String s : strings) {
if (/*before*/!res/*within*/.isEmpty()) {
res += ", ";
}
res <caret>+= s;
}
return res;
}
}

2
plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties
View File

@@ -2202,3 +2202,5 @@ junit5.converter.display.name=JUnit 4 test can be JUnit 5
junit5.converter.fix.name=Migrate to JUnit 5
call.to.suspicious.string.method.display.name=Call to suspicious String method
call.to.suspicious.string.method.problem.descriptor=<code>String.#ref()</code> called in internationalized context #loc
string.concatenation.replace.fix=Replace with StringBuilder
string.concatenation.replace.fix.name=Change type of ''{0}'' to {1}

268
plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/performance/StringConcatenationInLoopsInspection.java
View File

@@ -15,8 +15,12 @@
*/
package com.siyeh.ig.performance;
import com.intellij.codeInspection.ProblemDescriptor;
import com.intellij.codeInspection.ui.SingleCheckboxOptionsPanel;
import com.intellij.openapi.project.Project;
import com.intellij.psi.*;
import com.intellij.psi.codeStyle.CodeStyleManager;
import com.intellij.psi.codeStyle.JavaCodeStyleManager;
import com.intellij.psi.controlFlow.DefUseUtil;
import com.intellij.psi.search.LocalSearchScope;
import com.intellij.psi.search.searches.ReferencesSearch;
@@ -26,15 +30,20 @@ import com.intellij.psi.util.PsiUtil;
import com.siyeh.InspectionGadgetsBundle;
import com.siyeh.ig.BaseInspection;
import com.siyeh.ig.BaseInspectionVisitor;
import com.siyeh.ig.psiutils.ControlFlowUtils;
import com.siyeh.ig.psiutils.ExpressionUtils;
import com.siyeh.ig.psiutils.TypeUtils;
import com.siyeh.ig.InspectionGadgetsFix;
import com.siyeh.ig.psiutils.*;
import one.util.streamex.StreamEx;
import org.jetbrains.annotations.Contract;
import org.jetbrains.annotations.Nls;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import javax.swing.*;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Objects;
import java.util.regex.Pattern;
public class StringConcatenationInLoopsInspection extends BaseInspection {
@@ -83,7 +92,7 @@ public class StringConcatenationInLoopsInspection extends BaseInspection {
if (m_ignoreUnlessAssigned && !isAppendedRepeatedly(expression)) return;
final PsiJavaToken sign = expression.getTokenBeforeOperand(operands[1]);
assert sign != null;
registerError(sign);
registerError(sign, getAppendedVariable(expression));
}
@Override
@@ -106,7 +115,7 @@ public class StringConcatenationInLoopsInspection extends BaseInspection {
return;
}
}
registerError(sign);
registerError(sign, getAppendedVariable(expression));
}
private boolean checkExpression(PsiExpression expression, PsiType type) {
@@ -217,20 +226,6 @@ public class StringConcatenationInLoopsInspection extends BaseInspection {
return !ControlFlowUtils.statementMayCompleteNormally(parentStatement);
}
@Contract("null -> null")
@Nullable
private PsiVariable getAppendedVariable(PsiExpression expression) {
if (!(expression instanceof PsiAssignmentExpression)) {
return null;
}
PsiExpression lhs = PsiUtil.skipParenthesizedExprDown(((PsiAssignmentExpression)expression).getLExpression());
if (!(lhs instanceof PsiReferenceExpression)) {
return null;
}
final PsiElement element = ((PsiReferenceExpression)lhs).resolve();
return element instanceof PsiVariable ? (PsiVariable)element : null;
}
private boolean isAppendedRepeatedly(PsiExpression expression) {
PsiElement parent = expression.getParent();
while (parent instanceof PsiParenthesizedExpression || parent instanceof PsiPolyadicExpression) {
@@ -267,4 +262,239 @@ public class StringConcatenationInLoopsInspection extends BaseInspection {
return false;
}
}
@Contract("null -> null")
@Nullable
private static PsiVariable getAppendedVariable(PsiExpression expression) {
PsiElement parent = expression;
while (parent instanceof PsiParenthesizedExpression || parent instanceof PsiPolyadicExpression) {
parent = parent.getParent();
}
if (!(parent instanceof PsiAssignmentExpression)) {
return null;
}
PsiExpression lhs = PsiUtil.skipParenthesizedExprDown(((PsiAssignmentExpression)parent).getLExpression());
if (!(lhs instanceof PsiReferenceExpression)) {
return null;
}
final PsiElement element = ((PsiReferenceExpression)lhs).resolve();
return element instanceof PsiVariable ? (PsiVariable)element : null;
}
@Nullable
@Override
protected InspectionGadgetsFix buildFix(Object... infos) {
return (infos.length > 0 && infos[0] instanceof PsiLocalVariable) ? new ReplaceWithStringBuilderFix((PsiVariable)infos[0]) : null;
}
static class ReplaceWithStringBuilderFix extends InspectionGadgetsFix {
private static final Pattern PRINT_OR_PRINTLN = Pattern.compile("print|println");
private String myName;
private String myTargetType;
public ReplaceWithStringBuilderFix(PsiVariable variable) {
myName = variable.getName();
myTargetType = PsiUtil.isLanguageLevel5OrHigher(variable) ? "StringBuilder" : "StringBuffer";
}
@Override
protected void doFix(Project project, ProblemDescriptor descriptor) {
PsiExpression expression = PsiTreeUtil.getParentOfType(descriptor.getStartElement(), PsiExpression.class);
if (expression == null) return;
PsiVariable variable = getAppendedVariable(expression);
if(!(variable instanceof PsiLocalVariable)) return;
variable.normalizeDeclaration();
PsiElementFactory factory = JavaPsiFacade.getElementFactory(project);
PsiTypeElement typeElement = variable.getTypeElement();
if(typeElement == null) return;
Collection<PsiReference> refs = ReferencesSearch.search(variable).findAll();
List<PsiElement> results = new ArrayList<>();
CommentTracker ct = new CommentTracker();
replaceAll(variable, factory, refs, results, ct);
results.add(ct.replace(typeElement, "java.lang." + myTargetType));
PsiExpression initializer = variable.getInitializer();
if(initializer != null) {
results.add(ct.replace(initializer, generateNewStringBuilder(initializer, ct)));
}
PsiStatement commentPlace = PsiTreeUtil.getParentOfType(variable, PsiStatement.class);
ct.insertCommentsBefore(commentPlace == null ? variable : commentPlace);
for(PsiElement result : results) {
if(result.isValid()) {
result = JavaCodeStyleManager.getInstance(project).shortenClassReferences(result);
CodeStyleManager.getInstance(project).reformat(result);
}
}
}
@NotNull
private String generateNewStringBuilder(PsiExpression initializer, CommentTracker ct) {
if(ExpressionUtils.isNullLiteral(initializer)) {
return ct.text(initializer);
}
String text = initializer == null || ExpressionUtils.isLiteral(initializer, "") ? "" : ct.text(initializer);
return "new java.lang." + myTargetType + "(" + text + ")";
}
private void replaceAll(PsiVariable variable,
PsiElementFactory factory,
Collection<PsiReference> refs,
List<PsiElement> results,
CommentTracker ct) {
for(PsiReference ref : refs) {
PsiElement target = ref.getElement();
if(target instanceof PsiReferenceExpression && target.isValid()) {
replace(variable, factory, results, (PsiReferenceExpression)target, ct);
}
}
}
private void replace(PsiVariable variable,
PsiElementFactory factory,
List<PsiElement> results,
PsiReferenceExpression ref,
CommentTracker ct) {
PsiElement parent = PsiUtil.skipParenthesizedExprUp(ref.getParent());
if(parent instanceof PsiAssignmentExpression) {
PsiAssignmentExpression assignment = (PsiAssignmentExpression)parent;
if(PsiUtil.skipParenthesizedExprDown(assignment.getLExpression()) == ref) {
PsiExpression rValue = assignment.getRExpression();
if(assignment.getOperationTokenType().equals(JavaTokenType.EQ)) {
if (rValue instanceof PsiPolyadicExpression &&
((PsiPolyadicExpression)rValue).getOperationTokenType().equals(JavaTokenType.PLUS)) {
PsiPolyadicExpression concat = (PsiPolyadicExpression)rValue;
PsiExpression[] operands = concat.getOperands();
if (operands.length > 1) {
if (ExpressionUtils.isReferenceTo(operands[0], variable)) {
ct.delete(Objects.requireNonNull(concat.getTokenBeforeOperand(operands[1])));
ct.delete(operands[0]);
replaceAll(variable, factory, ReferencesSearch.search(variable, new LocalSearchScope(rValue)).findAll(), results, ct);
results.add(ct.replace(assignment, variable.getName() + ".append(" + ct.text(rValue) + ")"));
return;
}
PsiExpression lastOp = operands[operands.length - 1];
if (ExpressionUtils.isReferenceTo(lastOp, variable)) {
ct.delete(Objects.requireNonNull(concat.getTokenBeforeOperand(lastOp)));
ct.delete(lastOp);
replaceAll(variable, factory, ReferencesSearch.search(variable, new LocalSearchScope(rValue)).findAll(), results, ct);
results.add(ct.replace(assignment, variable.getName() + ".insert(0, " + ct.text(rValue) + ")"));
return;
}
}
}
}
if(rValue != null) {
replaceAll(variable, factory, ReferencesSearch.search(variable, new LocalSearchScope(rValue)).findAll(), results, ct);
rValue = assignment.getRExpression();
}
if(assignment.getOperationTokenType().equals(JavaTokenType.PLUSEQ)) {
results.add(ct.replace(assignment, variable.getName() + ".append(" + ((rValue == null) ? "" : ct.text(rValue)) + ")"));
return;
}
if(assignment.getOperationTokenType().equals(JavaTokenType.EQ)) {
results.add(ct.replace(assignment, variable.getName() + "="+generateNewStringBuilder(rValue, ct)));
return;
}
} else {
// ref is r-value
if(assignment.getOperationTokenType().equals(JavaTokenType.PLUSEQ)) return;
}
}
if(parent instanceof PsiReferenceExpression &&
((PsiReferenceExpression)parent).getQualifierExpression() == ref &&
parent.getParent() instanceof PsiMethodCallExpression) {
PsiMethodCallExpression call = (PsiMethodCallExpression)parent.getParent();
PsiMethod method = call.resolveMethod();
if(method != null) {
PsiExpression[] args = call.getArgumentList().getExpressions();
String name = method.getName();
switch(name) {
case "length":
case "chars":
case "codePoints":
case "charAt":
case "codePointAt":
case "codePointBefore":
case "codePointAfter":
case "codePointCount":
case "offsetByCodePoints":
case "substring":
case "subSequence":
return;
case "getChars":
if(args.length == 4) return;
break;
case "indexOf":
case "lastIndexOf":
if(args.length >= 1 && args.length <= 2 && TypeUtils.isJavaLangString(args[0].getType())) return;
break;
case "isEmpty": {
String sign = "==";
PsiExpression negation = BoolUtils.findNegation(call);
PsiElement toReplace = call;
if (negation != null) {
sign = ">";
toReplace = negation;
}
PsiExpression emptyCheck = factory.createExpressionFromText(variable.getName() + ".length()" + sign + "0", ref);
PsiElement callParent = toReplace.getParent();
if (callParent instanceof PsiExpression &&
ParenthesesUtils.areParenthesesNeeded(emptyCheck, (PsiExpression)callParent, true)) {
emptyCheck = factory.createExpressionFromText("(" + emptyCheck.getText() + ")", ref);
}
results.add(ct.replace(toReplace, emptyCheck));
return;
}
default:
}
}
}
if(parent instanceof PsiExpressionList && parent.getParent() instanceof PsiMethodCallExpression) {
PsiExpression[] expressions = ((PsiExpressionList)parent).getExpressions();
if(expressions.length == 1 && expressions[0] == ref) {
PsiMethodCallExpression call = (PsiMethodCallExpression)parent.getParent();
if(MethodCallUtils.isCallToMethod(call, CommonClassNames.JAVA_LANG_STRING_BUILDER, null, "append",
(PsiType[])null) ||
MethodCallUtils.isCallToMethod(call, CommonClassNames.JAVA_LANG_STRING_BUFFER, null, "append",
(PsiType[])null) ||
MethodCallUtils.isCallToMethod(call, "java.io.PrintStream", null, PRINT_OR_PRINTLN,
(PsiType[])null) ||
MethodCallUtils.isCallToMethod(call, "java.io.PrintWriter", null, PRINT_OR_PRINTLN,
(PsiType[])null)
) {
return;
}
}
}
if(parent instanceof PsiBinaryExpression) {
PsiBinaryExpression binOp = (PsiBinaryExpression)parent;
if(ExpressionUtils.getValueComparedWithNull(binOp) != null) {
return;
}
}
if(parent instanceof PsiPolyadicExpression && ((PsiPolyadicExpression)parent).getOperationTokenType().equals(JavaTokenType.PLUS)) {
PsiExpression[] operands = ((PsiPolyadicExpression)parent).getOperands();
for (PsiExpression operand : operands) {
if (operand == ref) break;
if (TypeUtils.isJavaLangString(operand.getType())) return;
}
if (operands.length > 1 && operands[0] == ref && TypeUtils.isJavaLangString(operands[1].getType())) return;
}
results.add(ct.replace(ref, variable.getName()+".toString()"));
}
@Nls
@NotNull
@Override
public String getName() {
return InspectionGadgetsBundle.message("string.concatenation.replace.fix.name", myName, myTargetType);
}
@Nls
@NotNull
@Override
public String getFamilyName() {
return InspectionGadgetsBundle.message("string.concatenation.replace.fix");
}
}
}

43
plugins/InspectionGadgets/testsrc/com/siyeh/ig/performance/StringConcatenationInLoopsInspectionFixTest.java Normal file
View File

@@ -0,0 +1,43 @@
/*
* Copyright 2000-2016 JetBrains s.r.o.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.siyeh.ig.performance;
import com.intellij.codeInsight.daemon.quickFix.LightQuickFixParameterizedTestCase;
import com.intellij.codeInspection.LocalInspectionEP;
import com.intellij.codeInspection.LocalInspectionTool;
import org.jetbrains.annotations.NotNull;
public class StringConcatenationInLoopsInspectionFixTest extends LightQuickFixParameterizedTestCase {
@NotNull
@Override
protected LocalInspectionTool[] configureLocalInspectionTools() {
// Instantiation via LocalInspectionEP is necessary as tool ID lacks ending -s (shortName is StringConcatenationInLoop)
// which results in error when registering suppress actions
LocalInspectionEP ep = new LocalInspectionEP();
ep.id = "StringConcatenationInLoop";
ep.implementationClass = StringConcatenationInLoopsInspection.class.getName();
LocalInspectionTool tool = (LocalInspectionTool)ep.instantiateTool();
return new LocalInspectionTool[]{tool};
}
public void test() throws Exception { doAllTests(); }
@Override
protected String getBasePath() {
return "/codeInsight/daemonCodeAnalyzer/quickFix/stringConcatInLoop";
}
}