Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert String/StringBuffer/StringBuilder concatenation to Text Block not working on JDT.core UnnecessaryArrayCreationQuickFixTest #1841

Closed
vogella opened this issue Dec 6, 2024 · 4 comments · Fixed by #1842
Assignees
Labels
enhancement New feature or request noteworthy Noteworthy feature
Milestone

Comments

@vogella
Copy link
Contributor

vogella commented Dec 6, 2024

/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/UnnecessaryArrayCreationQuickFixTest.java contains statements like:

StringBuilder buf= new StringBuilder();
		buf.append("package test1;\n");
		buf.append("import java.util.Arrays;\n");
		buf.append("import java.util.List;\n");
		buf.append("public class A {\n");
		buf.append("    private class A1 {\n");
		buf.append("        public String[] foo(int x, String[] ... b) {\n");
		buf.append("            return b[0];\n");
		buf.append("        }\n");
		buf.append("    }\n");
		buf.append("    private class A2 extends A1 {\n");
		buf.append("        public String[] foo(int x) {\n");
		buf.append("            return super.foo(x, new String[][] { new String[] {\"a\"}});\n");
		buf.append("        }\n");
		buf.append("    }\n");
		buf.append("}\n");

I would expect that the Source clean up convert to Text Block would convert this. It currently does not.

image

cc @jjohnstn

@jjohnstn jjohnstn self-assigned this Dec 6, 2024
@jjohnstn
Copy link
Contributor

jjohnstn commented Dec 6, 2024

@vogella The reason the conversion does not occur is that the buf is later passed to a method: fetchConvertingProposal2. Removing the StringBuilder makes that impossible. If the methods that take a StringBuilder/StringBuffer were converted to take a String, then the buf.toString() could passed and this would trigger the clean-up to do what you expect.

@carstenartur
Copy link
Contributor

Hi @jjohnstn
Imho it might be possible to aggregate consecutive lines into a text block without substitution of the Stringbuffer object in this case. Wdyt?

@jjohnstn
Copy link
Contributor

jjohnstn commented Dec 6, 2024

So, initializing the StringBuilder/StringBuffer with a text block. Yes, it would be possible. I'll take a look at it.

@jjohnstn jjohnstn added the enhancement New feature or request label Dec 6, 2024
@jjohnstn jjohnstn modified the milestones: 4.35 M3, 4.35 M1 Dec 6, 2024
@jjohnstn jjohnstn added the noteworthy Noteworthy feature label Dec 6, 2024
jjohnstn added a commit to jjohnstn/eclipse.jdt.ui-1 that referenced this issue Dec 6, 2024
- modify StringConcatToTextBlockFixCore to support the case where the
  StringBuilder/StringBuffer is passed as an argument and in that
  case do not replace the StringBuilder/StringBuffer but change it
  to initialize with a text block
- add new tests to AssistQuickFixTest15 and CleanUpTest15
- fixes eclipse-jdt#1841
@vogella
Copy link
Contributor Author

vogella commented Dec 10, 2024

Thanks @jjohnstn I currently cannot test due to #1843

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request noteworthy Noteworthy feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants