openrewrite / rewrite-migrate-java

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

UseTextBlocks incorrectly converts multi-line concatenated string to text block in Kotlin #501

Closed eocantu closed 2 weeks ago

eocantu commented 2 weeks ago

What version of OpenRewrite are you using?

I am using

How are you running OpenRewrite?

I am using the Maven plugin, and my project is a multi-module project: https://github.com/eocantu/spring-boot-demo

What is the smallest, simplest way to reproduce the problem?

const val MULTI_LINE_MESSAGE =
    "This is a multi-line message to show what happens. " +
            "This is the second line of such message."

What did you expect to see?

No change

What did you see instead?

 const val MULTI_LINE_MESSAGE =
    """
    This is a multi-line message to show what happens. \
    This is the second line of such message.\
    """

Are you interested in contributing a fix to OpenRewrite?

Maybe

timtebeek commented 2 weeks ago

Hi @eocantu ; Would you mind briefly stating what problems you're seeing as a result of this? Does it change semantics, or not compile for instance? I'd like to understand better to judge the severity and potential solution space.

If we want to completely exclude running this recipe for Kotlin, then we could do something similar as to what we do here for StringLiteralEquality.

eocantu commented 2 weeks ago

Thanks for the quick reply, @timtebeek!

The problem I see is that the string is no longer the same. In this example, it was originally:

This is a multi-line message to show what happens. This is the second line of such message.

And became the following (leading spaces, backslash character, new lines):


    This is a multi-line message to show what happens. \
    This is the second line of such message.\

In Java, the indentation is trimmed automatically in a text block. That's not the case in Kotlin, you'd need to trim explicitly. And even then, there are places where using something like .trimIndent() isn't possible (e.g. constants).

If we want to completely exclude running this recipe for Kotlin, then we could do something similar as to what we do here for StringLiteralEquality.

I think that would be better than ending up with a different string.

timtebeek commented 2 weeks ago

Thanks for the quick and detailed feedback; I forgot about the trimIdent momentarily. Best then to skip it indeed. Would that be something you're open to add & can you fit that in? Not expected of course, but certainly appreciated if it were possible.

eocantu commented 2 weeks ago

It seems pretty straight forward based on the example you linked. I can give it a try.