Recipe UseTextBlocks: Textblock not constructed AFTER it encounters a variable in a string.

[jbessels]

Was testing with the UseTextBlocks recipe. Wanted to use it for our SQL statements. Noticed that it works partially. Our SQL statement consists out of + variable or constant + . It appears that after encountering a variable a textblock is no longer created. Also reran mvn rewrite:run again just to see if it would be picked up in a second run. It doesn't. During the test the ONLY active recipe was UseTextBlocks

Question: Is there a minimum number of string concats needed before a textblock is used??

Gonna check all the converted files, should I find more 'interesting things' I can enter it here in this thread.

Before

        int thisVariableStopsTextBlocks = 1; 

        String stringWithVariableInIt =
                "This " +
                "is  " +
                "text " +
                "BEFORE the variable " +
                thisVariableStopsTextBlocks +
                "This " +
                "is  " +
                "text " +
                "AFTER the variable. " +
                "As is visible a TextBlock is no longer constructed";

After org.openrewrite.java.migrate.lang.UseTextBlocks: {convertStringsWithoutNewlines=true}

        int thisVariableStopsTextBlocks = 1;

        String stringWithVariableInIt =
                """
                This \
                is  \
                text \
                BEFORE the variable \
                """ +
                thisVariableStopsTextBlocks +
                "This " +
                "is  " +
                "text " +
                "AFTER the variable. " +
                "As is visible a TextBlock is no longer constructed";

[timtebeek]

Question: Is there a minimum number of string concats needed before a textblock is used??

I'm not immediately seeing a minimum number of lines in UseTextBlocks, nor an explicit test to verify such behavior in UseTextBlocksTest. It appears to look for string concatenations on new lines. Does that answer your question?

[jbessels]

Tim thanks for the effort, appreciated. I currently have an 'ExamplesForOpenRewrite' class for other tickets I created. Was to lazy for the first ticket of the day......Have played a bit with TextBlocks. Looks what you say is true. They must be on separate lines. I Can NOT force it with \n in a string.

Before

String noTextBlockCreated= "no" + "textblock\n" + "will" + "be" + "created despite newline \n IN the string";
String textBlockCreated = "yes" +
                "textblock";

After

 String noTextBlockCreated= "no" + "textblock\n" + "will" + "be" + "created despite newline \n IN the string";
        String textBlockCreated = """
                yes\
                textblock\
                """;

[kunli2]

Hi @jbessels , Thanks for reporting this, yeah, for the question Question: Is there a minimum number of string concatenation needed before a text block is used?, @timtebeek answered it, there is no minimum number of string concatenation needed but new line/lines are required in string concatenation.

This is a known unsupported string grouping now, it will be good to have an enhancement for this.

[jbessels]

Tbh a wee bit bit sad that the ticket degraded from 'bug' to enhancement' but I fully understand. Bigger fish to fry first. I've some to offer ;-) It doesn't happen in that many places. I guess I can put the existing contcats part after the ' breaking variable' in a variable itself, rerun UseTextBlocks and have a new TextBlock. remove the variable and then concat it after the 'breaking variable.'

[kunli2]

TextBlock

Yeah, that works, but it will be easier for you if we can support grouping, I think I can do this enhancement in two weeks (probably this week), so you don't need to change the code. :)

[jbessels]

I think I can restrain myself for that long ;-) Thanks for the heads up, had almost started with the 'manual labor' task. Your very fast response is very much appreciated!!

[jbessels]

Any news on the 'grouping' improvement??

[kunli2]

Hi @jbessels, our team just prioritized the tasks, and we are planning to work on some high-priority tasks at this moment.
and of course, we always welcome a PR implementing from the community.

[jbessels]

No problem and I fully understand. So far (including this issue) I've had very good support from Moderne and its all very much appreciated. Then I'm just going for my original solution, some manual labor. As said before: I guess I can put the existing contcats part after the ' breaking variable' in a variable itself, rerun UseTextBlocks and have a new TextBlock. remove the variable and then concat it after the 'breaking variable.'

[kunli2]

Yes, I think it works.

[timtebeek]

As of #273 we no longer create partial text blocks; we can track creating either multiple text blocks, or switching over to a single text block with for instance """.formatted here.

[jbessels]

I tested the tmp fix (#273) and AFAICT it works.

I REALLY hope a real fix like "switching over to a single text block with for instance """.formatted is going to be implemented as we have LOTS of constructions as mentioned in the OP. Now we currently end up with lots of classes where textblocks and the string concats reside in one file, which is suboptimal.....