JMockit to Mockito - rewrite expectation results with the correct type

[kernanj]

What problem are you trying to solve?

Within an Expectations block in JMockit, we can record method invocations and the expected result. JMockit has some internal logic to handle cases where there is a type mismatch between the result and the return type of the method and performs auto conversion.

Raising this ticket to kick off a discussion about creating a standalone recipe to clean this up or to add to the existing recipe created here - #415

See https://javadoc.io/static/org.jmockit/jmockit/1.49/mockit/Expectations.html#result for more info

Example

Consider the following class below:

class SomeObject {
        public long longMethod() {
            return 1L;
        }

        public List<Long> listMethod() {
            return Arrays.asList(1L);
        }
}

A test mocking the methods above might look something like follows:

@Injectable SomeObject someObject;

@Test
void test() {

    new Expectations() {{
        someObject.longMethod();
        result = 1;

        someObject.listMethod();
        result = 2;
    }};

    ....
}

Running the existing JMockitExpectationsToMockito recipe

@Mock SomeObject someObject;

@Test
void test() {
    
    when(someObject.longMethod()).thenReturn(1);  // Should be converted to the long literal 1L 
    when(someObject.listMethod()).thenReturn(2);  // Also doesn't compile because of type mismatch
    
   ...
}

As you can see above, the existing recipe to rewrite expectations doesn't account for this type conversion. Should we look at creating a separate recipe to first look at fixing up these issues and then apply the JMockitExpectationsToMockito recipe?

Are you interested in contributing this recipe to OpenRewrite?

Yes

[kernanj]

Related to #325

[timtebeek]

Thanks for branching off this item for more detailed discussion. I'll tag @tinder-dthomson since he likely has more in depth knowledge to share on what's the best approach here, given his previous involvement with this recipe.

[tinder-dthomson]

I did consider handling this in the recipe, but it also felt like an edge case since I encountered it quite infrequently. Using the wrong type as a return value for a stub feels like a bad practice in general. That being said, it is indeed a feature of JMockit, so supporting it in JMockitExpectationsToMockito is certainly appreciated!

You will need to make your changes in ExpectationsBlockRewriter.rewriteExpectationResult. There, we construct the templateParams which are the values that we apply in the rewrite template. You will need to iterate over the results and cross check against the invocation parameters. Coerce any primitives in results with type mismatches to the correct type before templateParams.addAll(results) is invoked.

[tinder-dthomson]

@kernanj just checking if you're planning to implement this one? I might take a stab at it one of these days if I catch inspiration.

[kernanj]

@tinder-dthomson I spent some time digging a little deeper into the code to try and understand what the change might look like but haven't implemented anything yet other than writing some failing test cases so be my guest if you want to work on providing a solution 🙂