Handle multiple blocks in JMockit Expectations

[yurii-yu]

What's changed?

Cope with a corner case in refactoring JMockit Expectations.
Sometimes programmers will use more than one curly braces in one Expectation block. This looks bizarre and confusing, but it is syntactically correct. It will be ideal if such code can also be refactored automatically.

usually people write code like this:

new Expectations() {{
    MethodComparatorTest.this.method.getName(); result = "NoTest";
    MethodComparatorTest.this.method2.getName(); result = "Test";
}};

while some programmers prefer grouping related statements with redundant curly braces

new Expectations() {
    {
        MethodComparatorTest.this.method.getName();
        this.result = "NoTest";
    }
    {
        MethodComparatorTest.this.method2.getName();
        this.result = "Test";
    }
};

I made changes in two places.

On line 39 in JMockitUtils.java, this criterion need to be changed, or else OpenRewrite will just ignore such blocks.

if (nc.getBody() == null || nc.getBody().getStatements().size() != 1)

On line 53 in SetupStatementsRewriter.java, this rewrite logic need to be able to loop over all statements, instead of only the first one, in the Expectations block.

J.Block expectationsBlock = (J.Block) nc.getBody().getStatements().get(0);

// statement needs to be moved directly before expectations class instantiation
JavaCoordinates coordinates = nc.getCoordinates().before();
List<Statement> newExpectationsBlockStatements = new ArrayList<>();
for (Statement expectationStatement : expectationsBlock.getStatements()) {
    if (!isSetupStatement(expectationStatement, spies)) {
        newExpectationsBlockStatements.add(expectationStatement);
        continue;
    }
    rewriteBodyStatement(expectationStatement, coordinates);
    // subsequent setup statements are moved in order
    coordinates = expectationStatement.getCoordinates().after();
}
// the new expectations block has the setup statements removed
J.Block newExpectationsBlock = expectationsBlock.withStatements(newExpectationsBlockStatements);
nc = nc.withBody(nc.getBody().withStatements(Collections.singletonList(newExpectationsBlock)));

I also added one test case in JMockitExpectationsToMockitoTest.java

What's your motivation?

We encountered problem in dealing with several legacy projects.
Unfortunately, OpenWrite just kept most of those test cases untouched. It took us a couple of days to locate the problem.
Benefited a lot from the OpenRewrite project previously, we believe that more people and more legacy projects should be able to take advantage of it.

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

I read the best practice so I kept the LST untouched.
Moreover, I tried my best not to affect the previous logic, so I limit my changes to "curly brace blocks in one Expectations" specifically.
The change has been tested by the entire test suite. As far as I can see, everything works fine.

Have you considered any alternatives or workarounds?

Maybe changing the LST is a better solution, but I do not think it is worthy since we only need to handle a rare case.
Users can also use a script to remove the curly braces before applying this recipe, but that is an invasive way.

Checklist

• I've added unit tests to cover both positive and negative cases
• I've read and applied the recipe conventions and best practices
• I've used the IntelliJ IDEA auto-formatter on affected files

[yurii-yu]

@timtebeek Hello Amigo. I saw the latest commit from you. Those changes do make sense. You are so nice and so patient, thank you.

[timtebeek]

Thanks a lot for the fix here and the detailed explanation of your work and considerations. Much appreciated that you're contributing back here!

We'll do a new release tomorrow, so you only need to wait a day to get to use this in practice. Hope this helps you make the switch to Mockito. Do let us know if you stumble across any other issues!

[timtebeek]

@timtebeek Hello Amigo. I saw the latest commit from you. Those changes do make sense. You are so nice and so patient, thank you.

Thanks so much! You're welcome; glad to have you contribute back to the project. :)

[yurii-yu]

@timtebeek
Hello I am back.
I encountered similar problem, when dealing with multiple blocks in one Verifications block.

new Verifications() {
    {
    myObject.wait(anyLong, anyInt);
    times = 3;
    }
    {
    myObject.wait(anyLong, anyInt);
    times = 4;
    }
};

I will create another pull-request for that in the next couple of days.
Have a nice weekend.

[timtebeek]

I will create another pull-request for that in the next couple of days. Have a nice weekend.

Awesome, thanks! Looking forward to it. Enjoy your weekend as well. :)