Flatten Hamcrest assertThat allOf

[timtebeek]

What's changed?

Convert Hamcrest allOf(Matcher...) to individual assertThat statements for easier migration.

What's your motivation?

For #357

Anything in particular you'd like reviewers to focus on?

Correct use of cursor & coordinates.

Have you considered any alternatives or workarounds?

Initially tried with visitMethodInvocation, but that also complained when inserting more than one statement, even when part of the same JavaTemplate string.

Checklist

• I've added unit tests to cover both positive and negative cases
• I've added the license header to any new files through ./gradlew licenseFormat
• I've used the IntelliJ auto-formatter on affected files
• I've updated the documentation (if applicable)

[timtebeek]

@AlekSimpson I'd started on this one but it's late and I don't quite know right now how to resolve the test failures; perhaps you've solved this elsewhere already by now? Otherwise we can ask someone else to have a look.

[AlekSimpson]

@AlekSimpson I'd started on this one but it's late and I don't quite know right now how to resolve the test failures; perhaps you've solved this elsewhere already by now? Otherwise we can ask someone else to have a look.

I don't believe I've seen this error before. I looked at the code and it seems like you are passing null into template.apply(...). The tests seem to failing for that reason. I tried just passing in getCursor() but that didn't work. Is there some way maybe to get the cursor of the assertThat invocation? Maybe one of those cursors would work.

[AlekSimpson]

@timtebeek Ok so I thought of a different approach that maybe would be simpler. Basically it uses the visitMethodInvocation visitor to find assertThat instances with allOf and then it loops through each matcher argument in allOf and adds a new assertThat(...) statement string to a StringBuilder while also updating an array with the two corresponding arguments (actual and the matcher). Then after that it just applies the template with the array. I thought this might be easier since now you don't have to worry about what coordinates or cursor each time you apply a new template. Only problem is that I am again running into another error where it says it was expecting one statement to replace one statement but got 2. I tried using .contextSensitive() which is what resolved this issue last time I bumped into it but that doesn't seem to be doing the trick. I'm honestly not sure why else this error would be given. I thought JavaVisitor should allow for it.

[timtebeek]

Thanks for picking this up! Generating multiple statements at once is an approach I had also tried indeed, but got the same issues that you're having. Would need to see if we recall this pattern used elsewhere, but didn't manage to fit that in at the time. 🤔

[timtebeek]

Thanks for the trigger to rethink this! I now remembered that I had done something similar a while ago in cucumber-jvm:
https://github.com/openrewrite/rewrite-cucumber-jvm/blob/53ccf2c566c2398edde82a9dcb98f0462326686e/src/main/java/org/openrewrite/cucumber/jvm/CucumberJava8ClassVisitor.java#L72C1-L73

The JavaTemplate can only return a J, but not a collection of statements. By wrapping those statements in a block and then afterwards cleaning up the block we achieve what we want, with a slight detour. Makes the code much simpler, so thanks again!