From 89c9457c89ea9a4e842a8f2a8a46fc166a67defa Mon Sep 17 00:00:00 2001 From: Jeff Johnston Date: Mon, 18 Mar 2024 15:42:36 -0400 Subject: [PATCH] Fix StringBuffer to text block clean-up when constructed with String - fix StringConcatToTextBlockFixCore.StringBufferFinder() to fail if a StringBuffer/StringBuilder constructor is used and isn't passed a StringLiteral or a NumberLiteral or an InfixExpression of StringLiterals concatenated - add new tests to CleanUpTest15 --- .../fix/StringConcatToTextBlockFixCore.java | 70 ++++++++++++------- .../jdt/ui/tests/quickfix/CleanUpTest15.java | 28 ++++++++ 2 files changed, 73 insertions(+), 25 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 5aad25aa299..791bb2f6d10 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 @@ -56,6 +56,7 @@ import org.eclipse.jdt.core.dom.LineComment; import org.eclipse.jdt.core.dom.MethodInvocation; import org.eclipse.jdt.core.dom.NullLiteral; +import org.eclipse.jdt.core.dom.NumberLiteral; import org.eclipse.jdt.core.dom.SimpleName; import org.eclipse.jdt.core.dom.Statement; import org.eclipse.jdt.core.dom.StringLiteral; @@ -552,6 +553,16 @@ public boolean visit(ClassInstanceCreation node) { if (args.size() == 1 && args.get(0) instanceof StringLiteral) { fLiterals.add((StringLiteral)args.get(0)); nonNLS= hasNLSMarker(statement, icu); + } else if (args.size() > 0) { + if (args.get(0) instanceof InfixExpression infix) { + if (!processInfixExpression(statement, infix)) { + statementList.remove(statement); + return false; + } + } else if (!(args.get(0) instanceof NumberLiteral)) { + statementList.remove(statement); + return false; + } } Block block= (Block) statement.getParent(); List stmtList= block.statements(); @@ -579,31 +590,8 @@ public boolean visit(ClassInstanceCreation node) { fLiterals.add((StringLiteral)arg); statementList.add(s); } else if (arg instanceof InfixExpression infix) { - if (infix.getOperator() == Operator.PLUS) { - Expression left= infix.getLeftOperand(); - Expression right= infix.getRightOperand(); - List extendedOps= infix.extendedOperands(); - if (left instanceof StringLiteral && right instanceof StringLiteral) { - List extendedLiterals= new ArrayList<>(); - for (Expression extendedOp : extendedOps) { - if (extendedOp instanceof StringLiteral) { - extendedLiterals.add((StringLiteral)extendedOp); - } else { - statementList.clear(); - fLiterals.clear(); - return false; - } - - } - fLiterals.add((StringLiteral)left); - fLiterals.add((StringLiteral)right); - fLiterals.addAll(extendedLiterals); - statementList.add(s); - } else { - statementList.clear(); - fLiterals.clear(); - return false; - } + if (!processInfixExpression(s, infix)) { + return false; } } else { statementList.clear(); @@ -670,6 +658,38 @@ public boolean visit(ClassInstanceCreation node) { return true; } + private boolean processInfixExpression(Statement s, InfixExpression infix) { + if (infix.getOperator() == Operator.PLUS) { + Expression left= infix.getLeftOperand(); + Expression right= infix.getRightOperand(); + List extendedOps= infix.extendedOperands(); + if (left instanceof StringLiteral && right instanceof StringLiteral) { + List extendedLiterals= new ArrayList<>(); + for (Expression extendedOp : extendedOps) { + if (extendedOp instanceof StringLiteral) { + extendedLiterals.add((StringLiteral)extendedOp); + } else { + statementList.clear(); + fLiterals.clear(); + return false; + } + + } + fLiterals.add((StringLiteral)left); + fLiterals.add((StringLiteral)right); + fLiterals.addAll(extendedLiterals); + if (!statementList.contains(s)) { + statementList.add(s); + } + } else { + statementList.clear(); + fLiterals.clear(); + return false; + } + } + return true; + } + @Override public boolean visit(final VariableDeclarationStatement visited) { Type type= visited.getType(); 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 08ec05b7436..b2c96ba4600 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 @@ -340,6 +340,19 @@ public void testConcatToTextBlock2() throws Exception { + " buf5= new StringBuilder();\n" // + " buf5.append(7);\n" // + " String str3= \"abc\";\n" // + + " String x2= \"\" +\n" // + + " \"abc\\n\" +\n" // + + " \"def\\n\" +\n" // + + " \"ghi\\n\" +\n" // + + " \"jki\\n\";\n" // + + " StringBuilder buf6 = new StringBuilder(x2);\n" // + + " System.out.println(buf6.toString());\n" // + + " StringBuilder buf7 = new StringBuilder(\"\" +\n" // + + " \"abc\\n\" +\n" // + + " \"def\\n\" +\n" // + + " \"ghi\\n\" +\n" // + + " \"jki\\n\");\n" // + + " System.out.println(buf7.toString());\n" // + " }\n" // + "}"; @@ -424,6 +437,21 @@ public void testConcatToTextBlock2() throws Exception { + " buf5= new StringBuilder();\n" // + " buf5.append(7);\n" // + " String str3= \"abc\";\n" // + + " String x2= \"\"\"\n" // + + " abc\n" // + + " def\n" // + + " ghi\n" // + + " jki\n" // + + " \"\"\";\n" // + + " StringBuilder buf6 = new StringBuilder(x2);\n" // + + " System.out.println(buf6.toString());\n" // + + " String str5 = \"\"\"\n" // + + " abc\n" // + + " def\n" // + + " ghi\n" // + + " jki\n" // + + " \"\"\";\n" // + + " System.out.println(str5);\n" // + " }\n" // + "}";