openrewrite / rewrite-static-analysis

OpenRewrite recipes for identifying and fixing static analysis issues.
Apache License 2.0
30 stars 44 forks source link

ReplaceStringBuilderWithString should keep newlines #88

Open koppor opened 1 year ago

koppor commented 1 year ago
-        String content = new StringBuilder()
-                .append(Localization.lang("A backup file for '%0' was found at [%1]",
+        String content = Localization.lang("A backup file for '%0' was found at [%1]",
                         originalPath.getFileName().toString(),
-                        backupFilename))
-                .append("\n")
-                .append(Localization.lang("This could indicate that JabRef did not shut down cleanly last time the file was used."))
-                .append("\n\n")
-                .append(Localization.lang("Do you want to recover the library from the backup file?"))
-                .toString();
+                        backupFilename) + "\n" + Localization.lang("This could indicate that JabRef did not shut down cleanly last time the file was used.") + "\n\n" + Localization.lang("Do you want to recover the library from the backup file?");
timtebeek commented 1 year ago

Sounds like we ought to pretend the white space prefix of the removed method calls to the method arguments. We could start with a unit test that replicates the issue, based on the above, and then step through with the debugger to figure out how best to add that in.

pramud commented 1 year ago

Hi @koppor I want to pick this up, but I am facing issues with setting up in my local and cannot find any documentation related to it, can you please help me or point me to any documentation if present. Thanks, Pramud

timtebeek commented 1 year ago

Hi @pramud ! Thank you for your offer to help; here's a few links which I hope are helpful in setting up your development environment:

  1. https://docs.openrewrite.org/authoring-recipes/recipe-development-environment
  2. https://github.com/openrewrite/.github/blob/main/CONTRIBUTING.md
  3. https://docs.openrewrite.org/reference/building-openrewrite-from-source
  4. https://github.com/openrewrite/rewrite-docs/issues/138

If you need any further help feel free to join us on Slack.

pramud commented 1 year ago

Sure, thanks @timtebeek

pramud commented 1 year ago

I am debugging this, looks like this is not because of new lines but another method invocation inside the append.

timtebeek commented 1 year ago

Let us know if you'd need help with that @pramud ; I've not looked into the details yet. Welcome to open a draft pull request with only an unit test that replicates the issue as a start; that helps us all look at the same test failure.

pramud commented 1 year ago

As there were build issues when I tried to build locally, raised a PR in here https://github.com/openrewrite/rewrite/pull/3202

timtebeek commented 1 year ago

Yes we're in the middle of an upgrade to Rewrite 8.0; if you check out this branch locally, and run ./gradlew pTML then the rewrite-static-analysis should pick that up out of your local .m2/repository. It's only this week that things are slightly different; once the 8.0 branch is merged, and especially once that's released things should become easier again.