Skip to content

Commit 77f5098

Browse files
jjohnstnjjohnstn
authored
Enhance StringBuffer/StringBuilder to text block cleanup/quickfix (#1842)
- modify StringConcatToTextBlockFixCore to support the case where the StringBuilder/StringBuffer is passed as an argument and in that case do not replace the StringBuilder/StringBuffer but change it to initialize with a text block - add new tests to AssistQuickFixTest15 and CleanUpTest15 - fixes #1841
1 parent 44356af commit 77f5098

File tree

3 files changed

+785
-603
lines changed

3 files changed

+785
-603
lines changed

‎org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/StringConcatToTextBlockFixCore.java‎

Lines changed: 131 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@
6767
import org.eclipse.jdt.core.dom.VariableDeclarationStatement;
6868
import org.eclipse.jdt.core.dom.rewrite.ASTRewrite;
6969
import org.eclipse.jdt.core.dom.rewrite.ImportRewrite;
70+
import org.eclipse.jdt.core.dom.rewrite.ListRewrite;
7071
import org.eclipse.jdt.core.dom.rewrite.TargetSourceRangeComputer;
7172
import org.eclipse.jdt.core.formatter.DefaultCodeFormatterConstants;
7273

@@ -509,6 +510,7 @@ private class CheckValidityVisitor extends ASTVisitor {
509510
private List<MethodInvocation> toStringList= new ArrayList<>();
510511
private List<MethodInvocation> indexOfList= new ArrayList<>();
511512
private List<SimpleName> argList= new ArrayList<>();
513+
private boolean passedAsArgument= false;
512514

513515
public CheckValidityVisitor(int lastStatementEnd, IBinding varBinding) {
514516
this.lastStatementEnd= lastStatementEnd;
@@ -519,6 +521,10 @@ public boolean isValid() {
519521
return valid;
520522
}
521523

524+
public boolean isPassedAsArgument() {
525+
return passedAsArgument;
526+
}
527+
522528
public ExpressionStatement getNextAssignment() {
523529
return nextAssignment;
524530
}
@@ -581,8 +587,11 @@ public boolean visit(SimpleName name) {
581587
paramType.getQualifiedName().equals("java.lang.CharSequence")) { //$NON-NLS-1$
582588
argList.add(name);
583589
valid= true;
590+
return true;
584591
} else {
585-
valid= false;
592+
passedAsArgument= true;
593+
valid= true;
594+
return true;
586595
}
587596
}
588597
} else {
@@ -739,9 +748,22 @@ public boolean visit(SimpleName simpleName) {
739748
} catch (AbortSearchException e) {
740749
// do nothing
741750
}
742-
if (!checkValidityVisitor.isValid() || (checkValidityVisitor.getToStringList().isEmpty() && checkValidityVisitor.getArgList().isEmpty())) {
751+
752+
if (!checkValidityVisitor.isValid()) {
753+
return failure();
754+
} else if (checkValidityVisitor.isPassedAsArgument()) {
755+
List<Statement> statements= new ArrayList<>(statementList);
756+
List<StringLiteral> literals= new ArrayList<>(fLiterals);
757+
ModifyStringBufferToUseTextBlock operation= new ModifyStringBufferToUseTextBlock(node, statements,
758+
literals);
759+
fOperations.add(operation);
760+
statementList.clear();
761+
fLiterals.clear();
762+
return false;
763+
} else if (checkValidityVisitor.getToStringList().isEmpty() && checkValidityVisitor.getArgList().isEmpty()) {
743764
return failure();
744765
}
766+
745767
ExpressionStatement assignmentToConvert= checkValidityVisitor.getNextAssignment();
746768
List<Statement> statements= new ArrayList<>(statementList);
747769
List<StringLiteral> literals= new ArrayList<>(fLiterals);
@@ -884,14 +906,54 @@ private static boolean hasNLSMarker(ASTNode node, ICompilationUnit cu) throws Ab
884906
return hasNLS;
885907
}
886908

909+
public static class ModifyStringBufferToUseTextBlock extends CompilationUnitRewriteOperation {
910+
911+
private final List<Statement> fStatements;
912+
private final List<StringLiteral> fLiterals;
913+
private final ClassInstanceCreation fConstructor;
914+
915+
public ModifyStringBufferToUseTextBlock(final ClassInstanceCreation constructor, final List<Statement> statements, final List<StringLiteral> literals) {
916+
fStatements= statements;
917+
fLiterals= literals;
918+
fConstructor= constructor;
919+
}
920+
921+
@Override
922+
public void rewriteAST(final CompilationUnitRewrite cuRewrite, final LinkedProposalModelCore linkedModel) throws CoreException {
923+
ASTRewrite rewrite= cuRewrite.getASTRewrite();
924+
TextEditGroup group= createTextEditGroup(MultiFixMessages.StringConcatToTextBlockCleanUp_description, cuRewrite);
925+
rewrite.setTargetSourceRangeComputer(new TargetSourceRangeComputer() {
926+
@Override
927+
public SourceRange computeSourceRange(final ASTNode nodeWithComment) {
928+
if (Boolean.TRUE.equals(nodeWithComment.getProperty(ASTNodes.UNTOUCH_COMMENT))) {
929+
return new SourceRange(nodeWithComment.getStartPosition(), nodeWithComment.getLength());
930+
}
931+
932+
return super.computeSourceRange(nodeWithComment);
933+
}
934+
});
935+
936+
StringBuilder buf= createTextBlockBuffer(fLiterals, cuRewrite);
937+
TextBlock textBlock= (TextBlock) rewrite.createStringPlaceholder(buf.toString(), ASTNode.TEXT_BLOCK);
938+
ListRewrite argList= rewrite.getListRewrite(fConstructor, ClassInstanceCreation.ARGUMENTS_PROPERTY);
939+
List<ASTNode> original= argList.getOriginalList();
940+
for (ASTNode node : original) {
941+
rewrite.remove(node, group);
942+
}
943+
argList.insertFirst(textBlock, group);
944+
for (int i= 1; i < fStatements.size(); ++i) {
945+
rewrite.remove(fStatements.get(i), group);
946+
}
947+
}
948+
}
949+
887950
public static class ChangeStringBufferToTextBlock extends CompilationUnitRewriteOperation {
888951

889952
private final List<MethodInvocation> fToStringList;
890953
private final List<MethodInvocation> fIndexOfList;
891954
private final List<SimpleName> fArgList;
892955
private final List<Statement> fStatements;
893956
private final List<StringLiteral> fLiterals;
894-
private String fIndent;
895957
private final Set<String> fExcludedNames;
896958
private final BodyDeclaration fLastBodyDecl;
897959
private final boolean fNonNLS;
@@ -905,7 +967,6 @@ public ChangeStringBufferToTextBlock(final List<MethodInvocation> toStringList,
905967
this.fStatements= statements;
906968
this.fLiterals= literals;
907969
this.fAssignmentToConvert= assignmentToConvert;
908-
this.fIndent= "\t"; //$NON-NLS-1$
909970
this.fExcludedNames= excludedNames;
910971
this.fLastBodyDecl= lastBodyDecl;
911972
this.fNonNLS= nonNLS;
@@ -927,19 +988,6 @@ public void rewriteAST(final CompilationUnitRewrite cuRewrite, final LinkedPropo
927988
fExcludedNames.clear();
928989
}
929990
ASTRewrite rewrite= cuRewrite.getASTRewrite();
930-
IJavaElement root= cuRewrite.getRoot().getJavaElement();
931-
if (root != null) {
932-
IJavaProject project= root.getJavaProject();
933-
if (project != null) {
934-
String tab_option= project.getOption(DefaultCodeFormatterConstants.FORMATTER_TAB_CHAR, true);
935-
if (JavaCore.SPACE.equals(tab_option)) {
936-
fIndent= ""; //$NON-NLS-1$
937-
for (int i= 0; i < CodeFormatterUtil.getTabWidth(project); ++i) {
938-
fIndent += " "; //$NON-NLS-1$
939-
}
940-
}
941-
}
942-
}
943991
TextEditGroup group= createTextEditGroup(MultiFixMessages.StringConcatToTextBlockCleanUp_description, cuRewrite);
944992
rewrite.setTargetSourceRangeComputer(new TargetSourceRangeComputer() {
945993
@Override
@@ -952,53 +1000,7 @@ public SourceRange computeSourceRange(final ASTNode nodeWithComment) {
9521000
}
9531001
});
9541002

955-
StringBuilder buf= new StringBuilder();
956-
957-
List<String> parts= new ArrayList<>();
958-
fLiterals.stream().forEach((t) -> { String value= t.getEscapedValue(); parts.addAll(unescapeBlock(value.substring(1, value.length() - 1))); });
959-
960-
961-
buf.append("\"\"\"\n"); //$NON-NLS-1$
962-
boolean newLine= false;
963-
boolean allWhiteSpaceStart= true;
964-
boolean allEmpty= true;
965-
for (String part : parts) {
966-
if (buf.length() > 4) {// the first part has been added after the text block delimiter and newline
967-
if (!newLine) {
968-
// no line terminator in this part: merge the line by emitting a line continuation escape
969-
buf.append("\\").append(System.lineSeparator()); //$NON-NLS-1$
970-
}
971-
}
972-
newLine= part.endsWith(System.lineSeparator());
973-
allWhiteSpaceStart= allWhiteSpaceStart && (part.isEmpty() || Character.isWhitespace(part.charAt(0)));
974-
allEmpty= allEmpty && part.isEmpty();
975-
buf.append(fIndent).append(part);
976-
}
977-
978-
if (newLine || allEmpty) {
979-
buf.append(fIndent);
980-
} else if (allWhiteSpaceStart) {
981-
buf.append("\\").append(System.lineSeparator()); //$NON-NLS-1$
982-
buf.append(fIndent);
983-
} else {
984-
// Replace trailing un-escaped quotes with escaped quotes before adding text block end
985-
int readIndex= buf.length() - 1;
986-
int count= 0;
987-
while (readIndex >= 0 && buf.charAt(readIndex) == '"' && count <= 3) {
988-
--readIndex;
989-
++count;
990-
}
991-
if (readIndex >= 0 && buf.charAt(readIndex) == '\\') {
992-
--count;
993-
}
994-
for (int i= count; i > 0; --i) {
995-
buf.deleteCharAt(buf.length() - 1);
996-
}
997-
for (int i= count; i > 0; --i) {
998-
buf.append("\\\""); //$NON-NLS-1$
999-
}
1000-
}
1001-
buf.append("\"\"\""); //$NON-NLS-1$
1003+
StringBuilder buf= createTextBlockBuffer(fLiterals, cuRewrite);
10021004
AST ast= fStatements.get(0).getAST();
10031005
if (fToStringList.size() == 1 &&
10041006
fIndexOfList.isEmpty() &&
@@ -1146,6 +1148,71 @@ public static StringConcatToTextBlockFixCore createStringConcatToTextBlockFix(AS
11461148
return new StringConcatToTextBlockFixCore(FixMessages.StringConcatToTextBlockFix_convert_msg, root, new CompilationUnitRewriteOperationsFixCore.CompilationUnitRewriteOperation[] { operations.get(0) });
11471149
}
11481150

1151+
private static StringBuilder createTextBlockBuffer(List<StringLiteral> literals, CompilationUnitRewrite cuRewrite) {
1152+
IJavaElement root= cuRewrite.getRoot().getJavaElement();
1153+
String fIndent= "\t"; //$NON-NLS-1$
1154+
if (root != null) {
1155+
IJavaProject project= root.getJavaProject();
1156+
if (project != null) {
1157+
String tab_option= project.getOption(DefaultCodeFormatterConstants.FORMATTER_TAB_CHAR, true);
1158+
if (JavaCore.SPACE.equals(tab_option)) {
1159+
fIndent= ""; //$NON-NLS-1$
1160+
for (int i= 0; i < CodeFormatterUtil.getTabWidth(project); ++i) {
1161+
fIndent += " "; //$NON-NLS-1$
1162+
}
1163+
}
1164+
}
1165+
}
1166+
1167+
StringBuilder buf= new StringBuilder();
1168+
1169+
List<String> parts= new ArrayList<>();
1170+
literals.stream().forEach((t) -> { String value= t.getEscapedValue(); parts.addAll(unescapeBlock(value.substring(1, value.length() - 1))); });
1171+
1172+
buf.append("\"\"\"\n"); //$NON-NLS-1$
1173+
boolean newLine= false;
1174+
boolean allWhiteSpaceStart= true;
1175+
boolean allEmpty= true;
1176+
for (String part : parts) {
1177+
if (buf.length() > 4) {// the first part has been added after the text block delimiter and newline
1178+
if (!newLine) {
1179+
// no line terminator in this part: merge the line by emitting a line continuation escape
1180+
buf.append("\\").append(System.lineSeparator()); //$NON-NLS-1$
1181+
}
1182+
}
1183+
newLine= part.endsWith(System.lineSeparator());
1184+
allWhiteSpaceStart= allWhiteSpaceStart && (part.isEmpty() || Character.isWhitespace(part.charAt(0)));
1185+
allEmpty= allEmpty && part.isEmpty();
1186+
buf.append(fIndent).append(part);
1187+
}
1188+
1189+
if (newLine || allEmpty) {
1190+
buf.append(fIndent);
1191+
} else if (allWhiteSpaceStart) {
1192+
buf.append("\\").append(System.lineSeparator()); //$NON-NLS-1$
1193+
buf.append(fIndent);
1194+
} else {
1195+
// Replace trailing un-escaped quotes with escaped quotes before adding text block end
1196+
int readIndex= buf.length() - 1;
1197+
int count= 0;
1198+
while (readIndex >= 0 && buf.charAt(readIndex) == '"' && count <= 3) {
1199+
--readIndex;
1200+
++count;
1201+
}
1202+
if (readIndex >= 0 && buf.charAt(readIndex) == '\\') {
1203+
--count;
1204+
}
1205+
for (int i= count; i > 0; --i) {
1206+
buf.deleteCharAt(buf.length() - 1);
1207+
}
1208+
for (int i= count; i > 0; --i) {
1209+
buf.append("\\\""); //$NON-NLS-1$
1210+
}
1211+
}
1212+
buf.append("\"\"\""); //$NON-NLS-1$
1213+
return buf;
1214+
}
1215+
11491216
protected StringConcatToTextBlockFixCore(final String name, final CompilationUnit compilationUnit, final CompilationUnitRewriteOperationsFixCore.CompilationUnitRewriteOperation[] fixRewriteOperations) {
11501217
super(name, compilationUnit, fixRewriteOperations);
11511218
}

0 commit comments

Comments
 (0)