Skip to content

Commit

Permalink
Fix string concat to String.format to use text block when possible (#…
Browse files Browse the repository at this point in the history 
…1365)

- change ConvertToStringFormatFixCore to use a text block for
  JVM 15 and above
- add new tests to AssistQuickFixTest15
- fixes #1311
  • Loading branch information
@jjohnstn
jjohnstn committed Apr 24, 2024
1 parent 2630820 commit e489f19
Show file tree
Hide file tree
Showing 2 changed files with 370 additions and 3 deletions.
145 changes: 143 additions & 2 deletions ...tion/core extension/org/eclipse/jdt/internal/corext/fix/ConvertToStringFormatFixCore.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,14 @@

import java.util.ArrayList;
import java.util.List;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import org.eclipse.core.runtime.CoreException;

import org.eclipse.jface.text.BadLocationException;

import org.eclipse.jdt.core.IBuffer;
import org.eclipse.jdt.core.ICompilationUnit;
import org.eclipse.jdt.core.JavaModelException;
import org.eclipse.jdt.core.compiler.InvalidInputException;
Expand All @@ -34,6 +37,7 @@
import org.eclipse.jdt.core.dom.Initializer;
import org.eclipse.jdt.core.dom.MethodDeclaration;
import org.eclipse.jdt.core.dom.MethodInvocation;
import org.eclipse.jdt.core.dom.Statement;
import org.eclipse.jdt.core.dom.StringLiteral;
import org.eclipse.jdt.core.dom.VariableDeclarationFragment;
import org.eclipse.jdt.core.dom.rewrite.ASTRewrite;
Expand All @@ -50,6 +54,11 @@

public class ConvertToStringFormatFixCore extends CompilationUnitRewriteOperationsFixCore {

/**
* Should match the last NLS comment before end of the line
*/
static final Pattern comment= Pattern.compile("([ ]*\\/\\/\\$NON-NLS-[0-9]\\$) *$"); //$NON-NLS-1$

public ConvertToStringFormatFixCore(String name, CompilationUnit compilationUnit, CompilationUnitRewriteOperation operation) {
super(name, compilationUnit, operation);
}
Expand Down Expand Up @@ -172,6 +181,30 @@ private static void collectInfixPlusOperands(Expression expression, List<Express
}
}

private static String indentOf(ICompilationUnit cu, Expression exp) {
CompilationUnit cUnit= (CompilationUnit)exp.getRoot();
int startLine= cUnit.getLineNumber(exp.getStartPosition());
int startLinePos= cUnit.getPosition(startLine, 0);
int endOfLine= cUnit.getPosition(startLine + 1, 0);
String indent= ""; //$NON-NLS-1$
IBuffer buffer;
try {
buffer= cu.getBuffer();
String line= buffer.getText(startLinePos, endOfLine - startLinePos);
for (int i= 0; i < line.length(); ++i) {
char ch= line.charAt(i);
if (Character.isSpaceChar(ch)) {
indent+= ch;
} else {
break;
}
}
} catch (JavaModelException e) {
// ignore
}
return indent;
}

private static class ConvertToStringFormatProposalOperation extends CompilationUnitRewriteOperation {
private InfixExpression infixExpression;

Expand All @@ -181,8 +214,11 @@ public ConvertToStringFormatProposalOperation(InfixExpression infixExpression) {

@Override
public void rewriteAST(CompilationUnitRewrite cuRewrite, LinkedProposalModelCore linkedModel) throws CoreException {
final List<String> fLiterals= new ArrayList<>();
String fIndent= ""; //$NON-NLS-1$
ASTRewrite rewrite= cuRewrite.getASTRewrite();
ICompilationUnit cu= cuRewrite.getCu();
boolean is15OrHigher= JavaModelUtil.is15OrHigher(cu.getJavaProject());
CompilationUnit root= cuRewrite.getRoot();
String cuContents= cuRewrite.getCu().getBuffer().getContents();

Expand All @@ -193,8 +229,13 @@ public void rewriteAST(CompilationUnitRewrite cuRewrite, LinkedProposalModelCore
List<String> formatArguments= new ArrayList<>();
StringBuilder formatString= new StringBuilder();
int tagsCount= 0;
boolean firstStringLiteral= true;
for (Expression operand : operands) {
if (operand instanceof StringLiteral) {
if (firstStringLiteral) {
fIndent= indentOf(cu, operand);
firstStringLiteral= false;
}
NLSLine nlsLine= scanCurrentLine(cu, operand);
if (nlsLine != null) {
for (NLSElement element : nlsLine.getElements()) {
Expand All @@ -206,10 +247,12 @@ public void rewriteAST(CompilationUnitRewrite cuRewrite, LinkedProposalModelCore
}
}
String value= ((StringLiteral) operand).getEscapedValue();
fLiterals.add(value);
value= value.substring(1, value.length() - 1);
formatString.append(value);
} else {
ITypeBinding binding= operand.resolveTypeBinding();
fLiterals.add("\"%" + stringFormatConversion(binding) + "\""); //$NON-NLS-1$ //$NON-NLS-2$
formatString.append("%").append(stringFormatConversion(binding)); //$NON-NLS-1$
int origStart= root.getExtendedStartPosition(operand);
int origLength= root.getExtendedLength(operand);
Expand All @@ -220,14 +263,112 @@ public void rewriteAST(CompilationUnitRewrite cuRewrite, LinkedProposalModelCore

StringBuilder buffer= new StringBuilder();
buffer.append("String.format("); //$NON-NLS-1$
buffer.append("\"" + formatString.toString().replaceAll("\"", "\\\"") + "\""); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$

if (is15OrHigher) {
StringBuilder buf= new StringBuilder();

List<String> parts= new ArrayList<>();
fLiterals.stream().forEach((t) -> { parts.addAll(StringConcatToTextBlockFixCore.unescapeBlock(t.substring(1, t.length() - 1))); });


buf.append("\"\"\"\n"); //$NON-NLS-1$
boolean newLine= false;
boolean allWhiteSpaceStart= true;
boolean allEmpty= true;
for (String part : parts) {
if (buf.length() > 4) {// the first part has been added after the text block delimiter and newline
if (!newLine) {
// no line terminator in this part: merge the line by emitting a line continuation escape
buf.append("\\").append(System.lineSeparator()); //$NON-NLS-1$
}
}
newLine= part.endsWith(System.lineSeparator());
allWhiteSpaceStart= allWhiteSpaceStart && (part.isEmpty() || Character.isWhitespace(part.charAt(0)));
allEmpty= allEmpty && part.isEmpty();
buf.append(fIndent).append(part);
}

if (newLine || allEmpty) {
buf.append(fIndent);
} else if (allWhiteSpaceStart) {
buf.append("\\").append(System.lineSeparator()); //$NON-NLS-1$
buf.append(fIndent);
} else {
// Replace trailing un-escaped quotes with escaped quotes before adding text block end
int readIndex= buf.length() - 1;
int count= 0;
while (readIndex >= 0 && buf.charAt(readIndex) == '"' && count <= 3) {
--readIndex;
++count;
}
if (readIndex >= 0 && buf.charAt(readIndex) == '\\') {
--count;
}
for (int i1= count; i1 > 0; --i1) {
buf.deleteCharAt(buf.length() - 1);
}
for (int i1= count; i1 > 0; --i1) {
buf.append("\\\""); //$NON-NLS-1$
}
}
buf.append("\"\"\""); //$NON-NLS-1$
buffer.append(buf.toString());
} else {
buffer.append("\"" + formatString.toString().replaceAll("\"", "\\\"") + "\""); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$
}

for (String formatArgument : formatArguments) {
buffer.append(", " + formatArgument); //$NON-NLS-1$
}
buffer.append(")"); //$NON-NLS-1$

if (tagsCount > 1) {
ASTNodes.replaceAndRemoveNLSByCount(rewrite, infixExpression, buffer.toString(), tagsCount - 1, null, cuRewrite);
if (is15OrHigher) {
Expression lastOperand= operands.get(operands.size() - 1);
NLSLine nlsLine= scanCurrentLine(cu, lastOperand);
tagsCount= 0;
if (nlsLine != null) {
for (NLSElement element : nlsLine.getElements()) {
if (element.hasTag()) {
++tagsCount;
}
}
}
if (!(lastOperand instanceof StringLiteral) || tagsCount > 1) {
// if last operand is not a StringLiteral, we have to replace the statement
// and add a non-NLS marker because we can't add one via expression replacement
ASTNode statement= ASTNodes.getFirstAncestorOrNull(infixExpression, Statement.class);
if (statement == null) {
return;
}
CompilationUnit cUnit= (CompilationUnit)infixExpression.getRoot();
int extendedStart= cUnit.getExtendedStartPosition(statement);
int extendedLength= cUnit.getExtendedLength(statement);
String completeStatement= cu.getBuffer().getText(extendedStart, extendedLength);
if (tagsCount > 1) {
// remove all non-NLS comments and then replace with just one
Matcher commentMatcher= comment.matcher(completeStatement);
while (tagsCount-- > 0) {
completeStatement= commentMatcher.replaceFirst(""); //$NON-NLS-1$
commentMatcher= comment.matcher(completeStatement);
}
extendedLength= completeStatement.length();
}
StringBuilder newBuffer= new StringBuilder();
newBuffer= newBuffer.append(completeStatement.substring(0, infixExpression.getStartPosition() - extendedStart));
newBuffer= newBuffer.append(buffer.toString());
int infixExpressionEnd= infixExpression.getStartPosition() + infixExpression.getLength();
newBuffer= newBuffer.append(cu.getBuffer().getText(infixExpressionEnd, extendedStart + extendedLength - infixExpressionEnd));
newBuffer= newBuffer.append(" //$NON-NLS-1$"); //$NON-NLS-1$
Statement newStatement= (Statement)rewrite.createStringPlaceholder(newBuffer.toString(), statement.getNodeType());
rewrite.replace(statement, newStatement, null);
} else {
MethodInvocation formatInvocation= (MethodInvocation)rewrite.createStringPlaceholder(buffer.toString(), ASTNode.METHOD_INVOCATION);
rewrite.replace(infixExpression, formatInvocation, null);
}
} else {
ASTNodes.replaceAndRemoveNLSByCount(rewrite, infixExpression, buffer.toString(), tagsCount - 1, null, cuRewrite);
}
} else {
MethodInvocation formatInvocation= (MethodInvocation)rewrite.createStringPlaceholder(buffer.toString(), ASTNode.METHOD_INVOCATION);
rewrite.replace(infixExpression, formatInvocation, null);
Expand Down

0 comments on commit e489f19

Please sign in to comment.