Skip to content

Commit

Permalink
Don't use text block for MessageFormat/String.format if one line conc…
Browse files Browse the repository at this point in the history 
…at (#1385)

- for a single line concat, don't use a text block when converting a
  string concat to MessageFormat or String.format in
  ConvertToMessageFormatFixCore and ConvertToStringFormatFixCore
- add new tests to AssistQuickFixTest15
- fixes #1384
  • Loading branch information
@jjohnstn
jjohnstn committed May 2, 2024
1 parent eae41a0 commit 87af144
Show file tree
Hide file tree
Showing 3 changed files with 132 additions and 8 deletions.
27 changes: 23 additions & 4 deletions ...ion/core extension/org/eclipse/jdt/internal/corext/fix/ConvertToMessageFormatFixCore.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -237,13 +237,20 @@ public void rewriteAST(CompilationUnitRewrite cuRewrite, LinkedProposalModelCore
StringBuilder formatString= new StringBuilder();
int i= 0;
int tagsCount= 0;
boolean firstStringLiteral= true;
boolean isFirstStringLiteral= true;
boolean isFirstArgument= true;
Expression firstStringLiteral= operands.get(0);
Expression lastStringLiteral= firstStringLiteral;
Expression firstArgumentExpression= operands.get(0);
Expression lastArgumentExpression= firstArgumentExpression;
for (Expression operand : operands) {
if (operand instanceof StringLiteral) {
if (firstStringLiteral) {
if (isFirstStringLiteral) {
fIndent= indentOf(cu, operand);
firstStringLiteral= false;
isFirstStringLiteral= false;
firstStringLiteral= operand;
}
lastStringLiteral= operand;
NLSLine nlsLine= scanCurrentLine(cu, operand);
if (nlsLine != null) {
for (NLSElement element : nlsLine.getElements()) {
Expand All @@ -263,6 +270,11 @@ public void rewriteAST(CompilationUnitRewrite cuRewrite, LinkedProposalModelCore
value= value.substring(1, value.length() - 1);
formatString.append(value);
} else {
if (isFirstArgument) {
firstArgumentExpression= operand;
isFirstArgument= false;
}
lastArgumentExpression= operand;
fLiterals.add("\"{" + Integer.toString(i) + "}\""); //$NON-NLS-1$ //$NON-NLS-2$
formatString.append("{").append(i).append("}"); //$NON-NLS-1$ //$NON-NLS-2$

Expand Down Expand Up @@ -298,7 +310,13 @@ public void rewriteAST(CompilationUnitRewrite cuRewrite, LinkedProposalModelCore
StringBuilder buffer= new StringBuilder();
buffer.append("MessageFormat.format("); //$NON-NLS-1$

if (is15OrHigher) {
int minOffset= firstStringLiteral.getStartPosition() < firstArgumentExpression.getStartPosition() ? firstStringLiteral.getStartPosition() : firstArgumentExpression.getStartPosition();
int maxOffset= lastStringLiteral.getStartPosition() > lastArgumentExpression.getStartPosition() ?
lastStringLiteral.getStartPosition() + lastStringLiteral.getLength() : lastArgumentExpression.getStartPosition() + lastArgumentExpression.getLength();

boolean isSingleLine= root.getLineNumber(maxOffset) == root.getLineNumber(minOffset);

if (is15OrHigher && !isSingleLine) {
StringBuilder buf= new StringBuilder();

List<String> parts= new ArrayList<>();
Expand Down Expand Up @@ -419,5 +437,6 @@ public void rewriteAST(CompilationUnitRewrite cuRewrite, LinkedProposalModelCore
rewrite.replace(infixExpression, formatInvocation, null);
}
}

}
}
26 changes: 22 additions & 4 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 @@ -229,13 +229,20 @@ public void rewriteAST(CompilationUnitRewrite cuRewrite, LinkedProposalModelCore
List<String> formatArguments= new ArrayList<>();
StringBuilder formatString= new StringBuilder();
int tagsCount= 0;
boolean firstStringLiteral= true;
boolean isFirstStringLiteral= true;
boolean isFirstArgument= true;
Expression firstStringLiteral= operands.get(0);
Expression lastStringLiteral= firstStringLiteral;
Expression firstArgumentExpression= operands.get(0);
Expression lastArgumentExpression= firstArgumentExpression;
for (Expression operand : operands) {
if (operand instanceof StringLiteral) {
if (firstStringLiteral) {
if (isFirstStringLiteral) {
fIndent= indentOf(cu, operand);
firstStringLiteral= false;
isFirstStringLiteral= false;
firstStringLiteral= operand;
}
lastStringLiteral= operand;
NLSLine nlsLine= scanCurrentLine(cu, operand);
if (nlsLine != null) {
for (NLSElement element : nlsLine.getElements()) {
Expand All @@ -252,6 +259,11 @@ public void rewriteAST(CompilationUnitRewrite cuRewrite, LinkedProposalModelCore
value= value.substring(1, value.length() - 1);
formatString.append(value);
} else {
if (isFirstArgument) {
firstArgumentExpression= operand;
isFirstArgument= false;
}
lastArgumentExpression= operand;
ITypeBinding binding= operand.resolveTypeBinding();
fLiterals.add("\"%" + stringFormatConversion(binding) + "\""); //$NON-NLS-1$ //$NON-NLS-2$
formatString.append("%").append(stringFormatConversion(binding)); //$NON-NLS-1$
Expand All @@ -265,7 +277,13 @@ public void rewriteAST(CompilationUnitRewrite cuRewrite, LinkedProposalModelCore
StringBuilder buffer= new StringBuilder();
buffer.append("String.format("); //$NON-NLS-1$

if (is15OrHigher) {
int minOffset= firstStringLiteral.getStartPosition() < firstArgumentExpression.getStartPosition() ? firstStringLiteral.getStartPosition() : firstArgumentExpression.getStartPosition();
int maxOffset= lastStringLiteral.getStartPosition() > lastArgumentExpression.getStartPosition() ?
lastStringLiteral.getStartPosition() + lastStringLiteral.getLength() : lastArgumentExpression.getStartPosition() + lastArgumentExpression.getLength();

boolean isSingleLine= root.getLineNumber(maxOffset) == root.getLineNumber(minOffset);

if (is15OrHigher && !isSingleLine) {
StringBuilder buf= new StringBuilder();

List<String> parts= new ArrayList<>();
Expand Down
87 changes: 87 additions & 0 deletions 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 @@ -1240,6 +1240,51 @@ public void testConcatToMessageFormatTextBlock4() throws Exception {
assertExpectedExistInProposals(proposals, new String[] { expected });
}

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

StringBuilder buf= new StringBuilder();
buf.append("module test {\n");
buf.append("}\n");
IPackageFragment def= fSourceFolder.createPackageFragment("", false, null);
def.createCompilationUnit("module-info.java", buf.toString(), false, null);

IPackageFragment pack= fSourceFolder.createPackageFragment("test", false, null);
buf= new StringBuilder();
buf.append("package test;\n");
buf.append("public class Cls {\n");
buf.append(" public void foo(String name, String id) {\n");
buf.append(" String title = \"Name: \" + name + \" ID: \" + id;\n");
buf.append(" System.out.println(title);\n");
buf.append(" }\n");
buf.append("}\n");
ICompilationUnit cu= pack.createCompilationUnit("Cls.java", buf.toString(), false, null);
int index= buf.indexOf("title");
IInvocationContext ctx= getCorrectionContext(cu, index, 6);
assertNoErrors(ctx);
ArrayList<IJavaCompletionProposal> proposals= collectAssists(ctx, false);

buf= new StringBuilder();
buf.append("package test;\n");
buf.append("\n");
buf.append("import java.text.MessageFormat;\n");
buf.append("\n");
buf.append("public class Cls {\n");
buf.append(" public void foo(String name, String id) {\n");
buf.append(" String title = MessageFormat.format(\"Name: {0} ID: {1}\", name, id);\n");
buf.append(" System.out.println(title);\n");
buf.append(" }\n");
buf.append("}\n");
String expected= buf.toString();

assertProposalExists(proposals, CorrectionMessages.QuickAssistProcessor_convert_to_string_format);
assertExpectedExistInProposals(proposals, new String[] { expected });
}

@Test
public void testConcatToStringFormatTextBlock1() throws Exception {
fJProject1= JavaProjectHelper.createJavaProject("TestProject1", "bin");
Expand Down Expand Up @@ -1467,4 +1512,46 @@ public void testConcatToStringFormatTextBlock4() throws Exception {
assertExpectedExistInProposals(proposals, new String[] { expected });
}

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

StringBuilder buf= new StringBuilder();
buf.append("module test {\n");
buf.append("}\n");
IPackageFragment def= fSourceFolder.createPackageFragment("", false, null);
def.createCompilationUnit("module-info.java", buf.toString(), false, null);

IPackageFragment pack= fSourceFolder.createPackageFragment("test", false, null);
buf= new StringBuilder();
buf.append("package test;\n");
buf.append("public class Cls {\n");
buf.append(" public void foo(String name, String id) {\n");
buf.append(" String title = \"Name: \" + name + \" ID: \" + id;\n");
buf.append(" System.out.println(title);\n");
buf.append(" }\n");
buf.append("}\n");
ICompilationUnit cu= pack.createCompilationUnit("Cls.java", buf.toString(), false, null);
int index= buf.indexOf("title");
IInvocationContext ctx= getCorrectionContext(cu, index, 6);
assertNoErrors(ctx);
ArrayList<IJavaCompletionProposal> proposals= collectAssists(ctx, false);

buf= new StringBuilder();
buf.append("package test;\n");
buf.append("public class Cls {\n");
buf.append(" public void foo(String name, String id) {\n");
buf.append(" String title = String.format(\"Name: %s ID: %s\", name, id);\n");
buf.append(" System.out.println(title);\n");
buf.append(" }\n");
buf.append("}\n");
String expected= buf.toString();

assertProposalExists(proposals, CorrectionMessages.QuickAssistProcessor_convert_to_string_format);
assertExpectedExistInProposals(proposals, new String[] { expected });
}

}

0 comments on commit 87af144

Please sign in to comment.