search

openrewrite / rewrite-migrate-java

OpenRewrite recipes for migrating to newer versions of Java.
Apache License 2.0
92 stars 67 forks source link

`UseTextBlocks` should not delete comments #397

Open aholland opened 6 months ago

aholland commented 6 months ago
image

Hi - there are two problems with the way text blocks are being created, illustrated in this image.

  1. The comments in the original have been deleted. There is no good way to insert comments into a text block, but in such cases you could just leave the old syntax in place.
  2. When you're creating text block within an annotation like this, there is no need for the trailing \ . I admit this might be hard to detect and do correctly and reliably. Perhaps there could be an option though.

This bug report is really about 1.

I am using the Gradle plugin, version 6.6.4.

timtebeek commented 6 months ago

Hi @aholland ; thanks for pointing that out! Yes indeed we should not convert to text blocks when there's comments; a case currently missed in UseTextBlocks https://github.com/openrewrite/rewrite-migrate-java/blob/d12d72bed27b6f183fbd7a37e6d6b3cead30411b/src/main/java/org/openrewrite/java/migrate/lang/UseTextBlocks.java#L46

If you'd want to help out retain comments the quickest way is likely to add a draft PR with a new test, based on this existing test, but with a comment thrown in somewhere in the middle. Ideally we convert none of it to text blocks. https://github.com/openrewrite/rewrite-migrate-java/blob/d12d72bed27b6f183fbd7a37e6d6b3cead30411b/src/test/java/org/openrewrite/java/migrate/lang/UseTextBlocksTest.java#L47-L70

timtebeek commented 6 months ago

Your second comment of ending up with trailing slashes in SQL queries is maybe best handled with a run of our Format SQL recipe: https://docs.openrewrite.org/recipes/sql/formatsql. I think that ought to already introduce newlines, as seen in the tests.