From 3de275f6364feef6ebf04c4c16ce57ba8bad6e6e Mon Sep 17 00:00:00 2001 From: Jeff Johnston Date: Sat, 16 Mar 2024 00:59:19 -0400 Subject: [PATCH] Fix string concat to text block handling of redundant constructors - fix StringBuffer/StringBuilder to text block support when original code has one or more redundant constructors preceding the first append - fix StringBuffer/StringBuilder look-ahead to next constructor statement logic so it doesn't change next constructor to VariableDeclarationStatement if the initial declaration never gets removed (i.e. first concenatation was invalid for conversion) - add new test scenarios to CleanUpTest15 - fixes #1238 --- .../fix/StringConcatToTextBlockFixCore.java | 31 +++++++++++++++++-- .../jdt/ui/tests/quickfix/CleanUpTest15.java | 12 +++++++ 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/StringConcatToTextBlockFixCore.java b/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/StringConcatToTextBlockFixCore.java index 23a818182ba..5aad25aa299 100644 --- a/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/StringConcatToTextBlockFixCore.java +++ b/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/StringConcatToTextBlockFixCore.java @@ -442,12 +442,14 @@ public static final class StringBufferFinder extends ASTVisitor { private final List fOperations; private List fLiterals= new ArrayList<>(); private List statementList= new ArrayList<>(); + private Set ignoredConstructorStatements= new HashSet<>(); private SimpleName originalVarName; private Map conversions= new HashMap<>(); private static final String APPEND= "append"; //$NON-NLS-1$ private static final String TO_STRING= "toString"; //$NON-NLS-1$ private BodyDeclaration fLastBodyDecl; private final Set fExcludedNames; + private final Set fRemovedDeclarations= new HashSet<>(); public StringBufferFinder(List operations, Set excludedNames) { super(true); @@ -540,6 +542,10 @@ public boolean visit(ClassInstanceCreation node) { !(statement instanceof ExpressionStatement)) { return false; } + if (ignoredConstructorStatements.contains(statement)) { + statementList.remove(statement); + return false; + } boolean nonNLS= false; CompilationUnit cu= (CompilationUnit) node.getRoot(); ICompilationUnit icu= (ICompilationUnit) cu.getJavaElement(); @@ -551,6 +557,7 @@ public boolean visit(ClassInstanceCreation node) { List stmtList= block.statements(); int startIndex= stmtList.indexOf(statement); int i= startIndex; + Statement previousStatement= null; while (++i < stmtList.size()) { Statement s= stmtList.get(i); if (s instanceof ExpressionStatement) { @@ -598,8 +605,7 @@ public boolean visit(ClassInstanceCreation node) { return false; } } - } - else { + } else { statementList.clear(); fLiterals.clear(); return false; @@ -607,12 +613,27 @@ public boolean visit(ClassInstanceCreation node) { } else { break; } + } else if (exp instanceof Assignment assignment && assignment.getLeftHandSide() instanceof SimpleName name + && name.getFullyQualifiedName().equals(originalVarName.getFullyQualifiedName()) + && (i - startIndex == 1 || ignoredConstructorStatements.contains(previousStatement))) { + Expression rightSide= ASTNodes.getUnparenthesedExpression(assignment.getRightHandSide()); + if (rightSide instanceof ClassInstanceCreation cic && isStringBufferType(cic.getType())) { + List cicArgs= cic.arguments(); + ignoredConstructorStatements.add(s); + statementList.add(s); + if (!cicArgs.isEmpty() || !fLiterals.isEmpty()) { + statementList.clear(); + fLiterals.clear(); + return false; + } + } } else { break; } } else { break; } + previousStatement= s; } Statement endStatement= stmtList.get(i - 1); int lastStatementEnd= endStatement.getStartPosition() + endStatement.getLength(); @@ -636,7 +657,11 @@ public boolean visit(ClassInstanceCreation node) { List literals= new ArrayList<>(fLiterals); List toStringList= new ArrayList<>(checkValidityVisitor.getToStringList()); BodyDeclaration bodyDecl= ASTNodes.getFirstAncestorOrNull(node, BodyDeclaration.class); - ChangeStringBufferToTextBlock operation= new ChangeStringBufferToTextBlock(toStringList, statements, literals, assignmentToConvert, fExcludedNames, fLastBodyDecl, nonNLS); + if (statements.get(0) instanceof VariableDeclarationStatement) { + fRemovedDeclarations.add(originalVarName); + } + ChangeStringBufferToTextBlock operation= new ChangeStringBufferToTextBlock(toStringList, statements, literals, + fRemovedDeclarations.contains(originalVarName) ? assignmentToConvert : null, fExcludedNames, fLastBodyDecl, nonNLS); fLastBodyDecl= bodyDecl; fOperations.add(operation); conversions.put(assignmentToConvert, operation); diff --git a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTest15.java b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTest15.java index 7d25ac63996..08ec05b7436 100644 --- a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTest15.java +++ b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTest15.java @@ -309,6 +309,8 @@ public void testConcatToTextBlock2() throws Exception { + " buf3.append(\"\\n\");\n" // + " // comment 6\n" // + " k = buf3.toString();\n" // + + " buf3= new StringBuilder();\n" // + + " buf3.append(4);\n" // + "\n" // + " String x = \"abc\\n\" +\n" + " \"def\\n\" +\n" // @@ -325,6 +327,8 @@ public void testConcatToTextBlock2() throws Exception { + " buf4.append(\" */\");\n" // + " String expected= buf4.toString();\n" // + " StringBuilder buf5= new StringBuilder();\n" // + + " buf5.append(3);\n" // + + " buf5= new StringBuilder();\n" // + " buf5.append(\n" // + " \"package pack1;\\n\" +\n" // + " \"\\n\" +\n" // @@ -333,6 +337,8 @@ public void testConcatToTextBlock2() throws Exception { + " \"public class C {\\n\" +\n" // + " \"}\");\n" // + " System.out.println(buf5.toString());\n" // + + " buf5= new StringBuilder();\n" // + + " buf5.append(7);\n" // + " String str3= \"abc\";\n" // + " }\n" // + "}"; @@ -382,6 +388,8 @@ public void testConcatToTextBlock2() throws Exception { + " }\n" // + " \n" // + " \"\"\";\n" // + + " StringBuilder buf3 = new StringBuilder();\n" // + + " buf3.append(4);\n" // + "\n" // + " String x = \"\"\"\n" // + " abc\n" // @@ -403,6 +411,8 @@ public void testConcatToTextBlock2() throws Exception { + " * foo\n" // + " */\\\n" // + " \"\"\";\n" // + + " StringBuilder buf5= new StringBuilder();\n" // + + " buf5.append(3);\n" // + " String str4 = \"\"\"\n" // + " package pack1;\n" // + " \n" // @@ -411,6 +421,8 @@ public void testConcatToTextBlock2() throws Exception { + " public class C {\n" // + " }\"\"\";\n" // + " System.out.println(str4);\n" // + + " buf5= new StringBuilder();\n" // + + " buf5.append(7);\n" // + " String str3= \"abc\";\n" // + " }\n" // + "}";