Fix concat to MessageFormat and String.format (#1423)

* Fix concat to MessageFormat and String.format - fix ConvertToMessageFormatFixCore and ConvertToStringFormatFixCore to refine the logic for determining when to use a text block vs a single line - logic now only uses text block if there are 3 or more lines used in the concatenation AND the number of string literal characters is greater than 80 - modify existing tests and add new tests to AssistQuickFixTest15 - fixes #1406
eclipse-jdt · Jun 7, 2024 · 048557e · 048557e
1 parent bf11c06
commit 048557e
Show file tree

Hide file tree

Showing 5 changed files with 141 additions and 39 deletions.
diff --git a/...ion/core extension/org/eclipse/jdt/internal/corext/fix/ConvertToMessageFormatFixCore.java b/...ion/core extension/org/eclipse/jdt/internal/corext/fix/ConvertToMessageFormatFixCore.java
@@ -243,6 +243,7 @@ public void rewriteAST(CompilationUnitRewrite cuRewrite, LinkedProposalModelCore
 			Expression lastStringLiteral= firstStringLiteral;
 			Expression firstArgumentExpression= operands.get(0);
 			Expression lastArgumentExpression= firstArgumentExpression;
+			int totalSize= 0;
 			for (Expression operand : operands) {
 				if (operand instanceof StringLiteral) {
 					if (isFirstStringLiteral) {
@@ -262,6 +263,7 @@ public void rewriteAST(CompilationUnitRewrite cuRewrite, LinkedProposalModelCore
 						}
 					}
 					String value= ((StringLiteral) operand).getEscapedValue();
+					totalSize += value.length();
 					value= value.replace("'", "''"); //$NON-NLS-1$ //$NON-NLS-2$
 					value= value.replace("{", "'{'"); //$NON-NLS-1$ //$NON-NLS-2$
 					value= value.replace("}", "'}'"); //$NON-NLS-1$ //$NON-NLS-2$
@@ -304,7 +306,6 @@ public void rewriteAST(CompilationUnitRewrite cuRewrite, LinkedProposalModelCore
 				}
 			}
 
-
 			importRewrite.addImport("java.text.MessageFormat", importContext); //$NON-NLS-1$
 
 			StringBuilder buffer= new StringBuilder();
@@ -314,9 +315,9 @@ public void rewriteAST(CompilationUnitRewrite cuRewrite, LinkedProposalModelCore
 			int maxOffset= lastStringLiteral.getStartPosition() > lastArgumentExpression.getStartPosition() ?
 					lastStringLiteral.getStartPosition() + lastStringLiteral.getLength() : lastArgumentExpression.getStartPosition() + lastArgumentExpression.getLength();
 
-			boolean isSingleLine= root.getLineNumber(maxOffset) == root.getLineNumber(minOffset);
+			boolean isLessThanThreeLines= root.getLineNumber(maxOffset) - root.getLineNumber(minOffset) < 2;
 
-			if (is15OrHigher && !isSingleLine) {
+			if (is15OrHigher && !isLessThanThreeLines && totalSize > 80) {
 				StringBuilder buf= new StringBuilder();
 
 				List<String> parts= new ArrayList<>();

diff --git a/...tion/core extension/org/eclipse/jdt/internal/corext/fix/ConvertToStringFormatFixCore.java b/...tion/core extension/org/eclipse/jdt/internal/corext/fix/ConvertToStringFormatFixCore.java
@@ -235,6 +235,7 @@ public void rewriteAST(CompilationUnitRewrite cuRewrite, LinkedProposalModelCore
 			Expression lastStringLiteral= firstStringLiteral;
 			Expression firstArgumentExpression= operands.get(0);
 			Expression lastArgumentExpression= firstArgumentExpression;
+			int totalSize= 0;
 			for (Expression operand : operands) {
 				if (operand instanceof StringLiteral) {
 					if (isFirstStringLiteral) {
@@ -254,6 +255,7 @@ public void rewriteAST(CompilationUnitRewrite cuRewrite, LinkedProposalModelCore
 						}
 					}
 					String value= ((StringLiteral) operand).getEscapedValue();
+					totalSize += value.length();
 					value= value.replace("%", "%%"); //$NON-NLS-1$ //$NON-NLS-2$
 					fLiterals.add(value);
 					value= value.substring(1, value.length() - 1);
@@ -281,9 +283,9 @@ public void rewriteAST(CompilationUnitRewrite cuRewrite, LinkedProposalModelCore
 			int maxOffset= lastStringLiteral.getStartPosition() > lastArgumentExpression.getStartPosition() ?
 					lastStringLiteral.getStartPosition() + lastStringLiteral.getLength() : lastArgumentExpression.getStartPosition() + lastArgumentExpression.getLength();
 
-			boolean isSingleLine= root.getLineNumber(maxOffset) == root.getLineNumber(minOffset);
+			boolean isLessThanThreeLines= root.getLineNumber(maxOffset) - root.getLineNumber(minOffset) < 2;
 
-			if (is15OrHigher && !isSingleLine) {
+			if (is15OrHigher && !isLessThanThreeLines && totalSize > 80) {
 				StringBuilder buf= new StringBuilder();
 
 				List<String> parts= new ArrayList<>();

diff --git a/org.eclipse.jdt.ui.tests/META-INF/MANIFEST.MF b/org.eclipse.jdt.ui.tests/META-INF/MANIFEST.MF
@@ -3,7 +3,7 @@ Automatic-Module-Name: org.eclipse.jdt.ui.tests
 Bundle-ManifestVersion: 2
 Bundle-Name: %Plugin.name
 Bundle-SymbolicName: org.eclipse.jdt.ui.tests; singleton:=true
-Bundle-Version: 3.15.400.qualifier
+Bundle-Version: 3.15.500.qualifier
 Bundle-Activator: org.eclipse.jdt.testplugin.JavaTestPlugin
 Bundle-Vendor: %Plugin.providerName
 Bundle-Localization: plugin

diff --git a/org.eclipse.jdt.ui.tests/pom.xml b/org.eclipse.jdt.ui.tests/pom.xml
@@ -20,7 +20,7 @@
   </parent>
   <groupId>org.eclipse.jdt</groupId>
   <artifactId>org.eclipse.jdt.ui.tests</artifactId>
-  <version>3.15.400-SNAPSHOT</version>
+  <version>3.15.500-SNAPSHOT</version>
   <packaging>eclipse-test-plugin</packaging>
   <properties>
     <defaultSigning-excludeInnerJars>true</defaultSigning-excludeInnerJars>