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 to Text-Block quick-fix/clean-up not applicable for StringBuilder/Buffer concatenation #1554

Closed
HannesWell opened this issue Jul 26, 2024 · 2 comments · Fixed by #1561
Assignees
Labels
bug Something isn't working

Comments

@HannesWell
Copy link
Contributor

The clean-up Convert String/StringBuffer/StringBuilder concatenation to Text Block is often not applicable on multi-line Strings that are built using StringBuffer/StringBuilder chains. The corresponding quick-fix is also not available when pressing Ctrl+1 at such expression.
When I manually convert the StringBuffer/StringBuilder chain into a simple String-concatenation using +, then the conversion to Text-block is possible.

One specific example is
https://github.com/eclipse-pde/eclipse.pde/blob/e1b548604a23c54170625a6f9725e7465286624b/build/org.eclipse.pde.build.tests/src/org/eclipse/pde/build/internal/tests/p2/PublishingTests.java#L371-L396

But that file contains many more places where this does not work
https://github.com/eclipse-pde/eclipse.pde/blob/e1b548604a23c54170625a6f9725e7465286624b/build/org.eclipse.pde.build.tests/src/org/eclipse/pde/build/internal/tests/p2/PublishingTests.java

@jjohnstn
Copy link
Contributor

In the first example, the StringBuffer is being passed as an argument to a method. The code generally looks to ensure the StringBuffer isn't being used again and also looks for a toString() call. In this case, additional checks need to be made to see if the StringBuffer is being passed as a String argument so toString() is implied. When you changed it to a concatenated String, it is different logic.

@jjohnstn jjohnstn self-assigned this Jul 26, 2024
@jjohnstn jjohnstn added the bug Something isn't working label Jul 26, 2024
@HannesWell
Copy link
Contributor Author

Thanks for your answer, that makes sense.
With that in mind I have to added that I changed the called method from accepting a StringBuffer to accept a CharSequence.
Without that it would indeed not be possible to change the concatenation to a Text-Block, but for a case like the following it should work theoretically:

	public static void main(String[] args) {
		StringBuilder builder = new StringBuilder();
		builder.append("Hello \n");
		builder.append("World \n");
		write(builder);
	}

	private static void write(CharSequence sequence);

jjohnstn added a commit to jjohnstn/eclipse.jdt.ui-1 that referenced this issue Jul 31, 2024
- fix StringConcatToTextBlockFixCore to allow conversion if the
  StringBuffer/StringBuilder is passed to a method as a String or
  CharSequence argument, even if no toString() call is made
- add new tests to CleanUpTest15 and AssistQuickFixTest15
- fixes eclipse-jdt#1554
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants