Skip to content

Commit

Permalink
Text block conversion support for indexOf() (#1709)
Browse files Browse the repository at this point in the history 
* Enhance StringBuilder to text block cleanup to support indexOf

- add new support to StringConcatToTextBlockFixCore to allow
  an indexOf call for StringBuilder/StringBuffer which can also work
  on a String once the conversion is complete (change the variable
  name of the indexOf call appropriately)
- add new tests to CleanUpTest15 and AssistQuickFixTest15
- for #1703
  • Loading branch information
@jjohnstn
jjohnstn authored Oct 12, 2024
1 parent 5b3e309 commit 5da0735
Show file tree
Hide file tree
Showing 3 changed files with 135 additions and 9 deletions.
48 changes: 40 additions & 8 deletions ...on/core extension/org/eclipse/jdt/internal/corext/fix/StringConcatToTextBlockFixCore.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -487,6 +487,7 @@ public static final class StringBufferFinder extends ASTVisitor {
private Map<ExpressionStatement, ChangeStringBufferToTextBlock> conversions= new HashMap<>();
private static final String APPEND= "append"; //$NON-NLS-1$
private static final String TO_STRING= "toString"; //$NON-NLS-1$
private static final String INDEX_OF= "indexOf"; //$NON-NLS-1$
private BodyDeclaration fLastBodyDecl;
private final Set<String> fExcludedNames;
private final Set<IBinding> fRemovedDeclarations= new HashSet<>();
Expand All @@ -503,6 +504,7 @@ private class CheckValidityVisitor extends ASTVisitor {
private int lastStatementEnd;
private IBinding varBinding;
private List<MethodInvocation> toStringList= new ArrayList<>();
private List<MethodInvocation> indexOfList= new ArrayList<>();
private List<SimpleName> argList= new ArrayList<>();

public CheckValidityVisitor(int lastStatementEnd, IBinding varBinding) {
Expand All @@ -522,6 +524,10 @@ public List<MethodInvocation> getToStringList() {
return toStringList;
}

public List<MethodInvocation> getIndexOfList() {
return indexOfList;
}

public List<SimpleName> getArgList() {
return argList;
}
Expand All @@ -547,6 +553,10 @@ public boolean visit(SimpleName name) {
valid= true;
toStringList.add(invocation);
return true;
} else if (invocation.getName().getFullyQualifiedName().equals(INDEX_OF)) {
valid= true;
indexOfList.add(invocation);
return true;
} else {
valid= false;
}
Expand Down Expand Up @@ -645,6 +655,7 @@ public boolean visit(ClassInstanceCreation node) {
if (exp instanceof MethodInvocation) {
class MethodVisitor extends ASTVisitor {
private boolean valid= false;
private boolean indexOfSeen= false;
public boolean isValid() {
return valid;
}
Expand All @@ -653,11 +664,19 @@ public boolean visit(SimpleName simpleName) {
if (simpleName.getFullyQualifiedName().equals(APPEND) && simpleName.getLocationInParent() == MethodInvocation.NAME_PROPERTY) {
return true;
}
if (simpleName.getLocationInParent() == MethodInvocation.EXPRESSION_PROPERTY && simpleName.getFullyQualifiedName().equals(originalVarName.getFullyQualifiedName())
&& ((MethodInvocation)simpleName.getParent()).getName().getFullyQualifiedName().equals(APPEND)) {
extractConcatenatedAppends(simpleName);
valid= true;
return false;
if (simpleName.getLocationInParent() == MethodInvocation.EXPRESSION_PROPERTY && simpleName.getFullyQualifiedName().equals(originalVarName.getFullyQualifiedName())) {
if (((MethodInvocation)simpleName.getParent()).getName().getFullyQualifiedName().equals(APPEND)) {
if (!indexOfSeen) {
extractConcatenatedAppends(simpleName);
valid= true;
} else {
valid= false;
}
return false;
} else if (((MethodInvocation)simpleName.getParent()).getName().getFullyQualifiedName().equals(INDEX_OF)) {
indexOfSeen= true;
return false;
}
}
return true;
}
Expand Down Expand Up @@ -724,12 +743,13 @@ public boolean visit(SimpleName simpleName) {
List<Statement> statements= new ArrayList<>(statementList);
List<StringLiteral> literals= new ArrayList<>(fLiterals);
List<MethodInvocation> toStringList= new ArrayList<>(checkValidityVisitor.getToStringList());
List<MethodInvocation> indexOfList= new ArrayList<>(checkValidityVisitor.getIndexOfList());
List<SimpleName> argList= new ArrayList<>(checkValidityVisitor.getArgList());
BodyDeclaration bodyDecl= ASTNodes.getFirstAncestorOrNull(node, BodyDeclaration.class);
if (statements.get(0) instanceof VariableDeclarationStatement) {
fRemovedDeclarations.add(originalVarName.resolveBinding());
}
ChangeStringBufferToTextBlock operation= new ChangeStringBufferToTextBlock(toStringList, argList, statements, literals,
ChangeStringBufferToTextBlock operation= new ChangeStringBufferToTextBlock(toStringList, indexOfList, argList, statements, literals,
fRemovedDeclarations.contains(originalVarName.resolveBinding()) ? assignmentToConvert : null, fExcludedNames, fLastBodyDecl, nonNLS);
fLastBodyDecl= bodyDecl;
fOperations.add(operation);
Expand Down Expand Up @@ -864,6 +884,7 @@ private static boolean hasNLSMarker(ASTNode node, ICompilationUnit cu) throws Ab
public static class ChangeStringBufferToTextBlock extends CompilationUnitRewriteOperation {

private final List<MethodInvocation> fToStringList;
private final List<MethodInvocation> fIndexOfList;
private final List<SimpleName> fArgList;
private final List<Statement> fStatements;
private final List<StringLiteral> fLiterals;
Expand All @@ -873,9 +894,10 @@ public static class ChangeStringBufferToTextBlock extends CompilationUnitRewrite
private final boolean fNonNLS;
private ExpressionStatement fAssignmentToConvert;

public ChangeStringBufferToTextBlock(final List<MethodInvocation> toStringList, final List<SimpleName> argList, List<Statement> statements,
List<StringLiteral> literals, ExpressionStatement assignmentToConvert, Set<String> excludedNames, BodyDeclaration lastBodyDecl, boolean nonNLS) {
public ChangeStringBufferToTextBlock(final List<MethodInvocation> toStringList, final List<MethodInvocation> indexOfList, final List<SimpleName> argList,
List<Statement> statements, List<StringLiteral> literals, ExpressionStatement assignmentToConvert, Set<String> excludedNames, BodyDeclaration lastBodyDecl, boolean nonNLS) {
this.fToStringList= toStringList;
this.fIndexOfList= indexOfList;
this.fArgList= argList;
this.fStatements= statements;
this.fLiterals= literals;
Expand Down Expand Up @@ -976,6 +998,7 @@ public SourceRange computeSourceRange(final ASTNode nodeWithComment) {
buf.append("\"\"\""); //$NON-NLS-1$
AST ast= fStatements.get(0).getAST();
if (fToStringList.size() == 1 &&
fIndexOfList.isEmpty() &&
!fNonNLS &&
ASTNodes.getLeadingComments(fStatements.get(0)).size() == 0 &&
(fToStringList.get(0).getLocationInParent() == Assignment.RIGHT_HAND_SIDE_PROPERTY ||
Expand Down Expand Up @@ -1024,6 +1047,15 @@ public SourceRange computeSourceRange(final ASTNode nodeWithComment) {
SimpleName name= ast.newSimpleName(newVarName);
rewrite.replace(toStringCall, name, group);
}
for (MethodInvocation indexOfCall : fIndexOfList) {
MethodInvocation newCall= ast.newMethodInvocation();
newCall.setName(ast.newSimpleName("indexOf")); //$NON-NLS-1$
SimpleName caller= ast.newSimpleName(newVarName);
newCall.setExpression(caller);
List<Expression> arguments= indexOfCall.arguments();
newCall.arguments().add(rewrite.createCopyTarget(arguments.get(0)));
rewrite.replace(indexOfCall, newCall, group);
}
for (SimpleName arg : fArgList) {
SimpleName name= ast.newSimpleName(newVarName);
rewrite.replace(arg, name, group);
Expand Down
71 changes: 70 additions & 1 deletion org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/AssistQuickFixTest15.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -717,6 +717,65 @@ private void write(CharSequence c) {
assertExpectedExistInProposals(proposals, new String[] { expected });
}

@Test
public void testConcatToTextBlock13() throws Exception {
fJProject1= JavaProjectHelper.createJavaProject("TestProject1", "bin");
fJProject1.setRawClasspath(projectSetup.getDefaultClasspath(), null);
JavaProjectHelper.set15CompilerOptions(fJProject1, false);
fSourceFolder= JavaProjectHelper.addSourceContainer(fJProject1, "src");

String str= """
module test {
}
""";
IPackageFragment def= fSourceFolder.createPackageFragment("", false, null);
def.createCompilationUnit("module-info.java", str, false, null);

IPackageFragment pack= fSourceFolder.createPackageFragment("test", false, null);
StringBuilder buf= new StringBuilder();
buf.append("package test;\n");
buf.append("public class Cls {\n");
buf.append(" public void foo() {\n");
buf.append(" StringBuilder buf3= new StringBuilder();\n");
buf.append(" buf3.append(\"public void foo() {\\n\");\n");
buf.append(" buf3.append(\" return null;\\n\");\n");
buf.append(" buf3.append(\"}\\n\");\n");
buf.append(" buf3.append(\"\\n\");\n");
buf.append(" // comment 1\n");
buf.append(" int index = buf3.indexOf(\"null\");\n");
buf.append(" String k = buf3.toString();\n");
buf.append(" \n");
buf.append(" }\n");
buf.append("}\n");
ICompilationUnit cu= pack.createCompilationUnit("Cls.java", buf.toString(), false, null);

int index= buf.indexOf("StringBuilder");
IInvocationContext ctx= getCorrectionContext(cu, index, 4);
assertNoErrors(ctx);
ArrayList<IJavaCompletionProposal> proposals= collectAssists(ctx, false);

String expected= """
package test;
public class Cls {
public void foo() {
String str = \"""
public void foo() {
return null;
}
\t
\""";
// comment 1
int index = str.indexOf("null");
String k = str;
\s
}
}
""";

assertProposalExists(proposals, FixMessages.StringConcatToTextBlockFix_convert_msg);
assertExpectedExistInProposals(proposals, new String[] { expected });
}

@Test
public void testNoConcatToTextBlock1() throws Exception {
fJProject1= JavaProjectHelper.createJavaProject("TestProject1", "bin");
Expand Down Expand Up @@ -1001,8 +1060,18 @@ public void testNoConcatToTextBlock8() throws Exception {
buf.append(" String k = buf3.toString();\n");
buf.append(" \n");
buf.append(" }\n");
buf.append(" public void indexOfInside() {\n");
buf.append(" StringBuffer buf3= new StringBuffer();\n");
buf.append(" buf3.append(\"public void foo() {\\n\");\n");
buf.append(" buf3.append(\" return null;\\n\");\n");
buf.append(" buf3.append(\"}\\n\");\n");
buf.append(" int index = buf3.indexOf(\"foo\");\n");
buf.append(" buf3.append(\"\\n\");\n");
buf.append(" String k = buf3.toString();\n");
buf.append(" \n");
buf.append(" }\n");
buf.append("}\n");
ICompilationUnit cu= pack.createCompilationUnit("Cls.java", buf.toString(), false, null);
ICompilationUnit cu= pack.createCompilationUnit("Cls.java", buf.toString(), false, null);

int index= buf.indexOf("buf3");
IInvocationContext ctx= getCorrectionContext(cu, index, 6);
Expand Down
25 changes: 25 additions & 0 deletions org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTest15.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,13 @@ public void foo() {
buf10.append(" * foo\\n");
buf10.append(" */");
write(buf10);
StringBuilder buf11 = new StringBuilder();
buf11.append("abcd\\n");
buf11.append("efgh\\n");
buf11.append("ijkl\\n");
buf11.append("mnop\\n");
int index = buf11.indexOf("fg");
System.out.println(buf11.toString());
}
private void write(CharSequence c) {
System.out.println(c);
Expand Down Expand Up @@ -522,6 +529,14 @@ public class C {
*/\\
\""";
write(str8);
String str9 = \"""
abcd
efgh
ijkl
mnop
\""";
int index = str9.indexOf("fg");
System.out.println(str9);
}
private void write(CharSequence c) {
System.out.println(c);
Expand Down Expand Up @@ -792,6 +807,16 @@ public void testInconsistentNLS() {
buf.append(3);
String x = buf.toString();
}
public void testIndexOfInBetween() {
StringBuffer buf = new StringBuffer();
buf.append("abcdef\\n");
buf.append("123456\\n");
buf.append("ghijkl\\n");
int index = buf.indexOf("123");
buf.append(3);
String x = buf.toString();
}
}
""";
ICompilationUnit cu1= pack1.createCompilationUnit("E.java", sample, false, null);
Expand Down

0 comments on commit 5da0735

Please sign in to comment.