org.openrewrite.java.migrate.lang.StringFormatted leaves hanging spaces/lf

[blipper]

class A {
    void foo(String bar) {
        String.format(
         "my string %s", bar);
    }
}

What did you expect to see?

class A {
    void foo(String bar) {
         "my string %s".formatted(bar);
    }
}

What did you see instead?

class A {
    void foo(String bar) {

         "my string %s".formatted(bar);
    }
}

This leaves a line feed above. This matters because checkstyle will complain about trailing spaces.

[bsmahi]

Hi @blipper could you please let me know which recipe you have used for the above. I will try to assist you on resolution.
Thanks,
Mahi

[blipper]

org.openrewrite.java.migrate.lang.StringFormatted :)

https://docs.openrewrite.org/recipes/java/migrate/lang/stringformatted
https://github.com/openrewrite/rewrite-migrate-java/blob/main/src/main/java/org/openrewrite/java/migrate/lang/StringFormatted.java

[timtebeek]

Thanks both; looks like we should retain the prefix of the original method invocation, as opposed to the prefix of the first String argument.

[coderustic]

Is anyone working on this? If not I want to contribute

[timtebeek]

Is anyone working on this? If not I want to contribute

Hi! No not yet; feel free to throw up a draft PR with a new unit test and then we can work from there.

[coderustic]

@timtebeek I tried to add a test to reproduce the issue as suggested, but I am wondering if this is fixed by #453. As you can see the previous testcase just above the newly added test in the draft MR.

When I remove the .withPrefix(Space.EMPTY) added as part of the fix for #453 then my test fails with a leading linefeed.

[coderustic]

@blipper what version you were using?

[timtebeek]

Thanks for the effort to reproduce @coderustic ! We indeed have the version number as part of the bug report template:
https://github.com/openrewrite/.github/blob/46bd203a479256d1afa37e1ac3c4146c4bab201a/.github/ISSUE_TEMPLATE/bug_report.md?plain=1#L15-L31
I'll optimistically close this issue; we can reopen if @blipper was using a newer version; but then ideally we'd see a failing unit test as an example.

[blipper]

Sorry. I was using 8.34.0

[coderustic]

What's rewrite-migrate-java version? Looks like 8.34.0 is released 8/28/24 while #556 is merged 9/15/24. So I would ask you to retest with latest version.