openrewrite / rewrite-testing-frameworks

OpenRewrite recipes that perform common Java testing migration tasks.
Apache License 2.0
77 stars 73 forks source link

Add recipe for replacing unnecesary Mockito#eq with direct parameters #615

Closed SanderKnauff closed 1 month ago

SanderKnauff commented 1 month ago

What's changed?

A recipe has been added to update unnecessary usages of Mockito#eq in favour of directly passing through parameters.

What's your motivation?

This updates code to comply with SonarQube rule java:S6068.

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

This is my first recipe that is authored for public use. Any tips and improvements are very welcome as I do not have a great feel yet for the best practices and standards in this community.

Anyone you would like to review specifically?

Have you considered any alternatives or workarounds?

N/A. A search and replace could work, but you'd have to be very careful not to change any wrong places. Having a recipe to fix this is saver

Any additional context

What is the Java language level used for writing recipes in this project? Judging by the code, it seems that 8 is the standard. Are there any plans to update that? I notice quite some friction with instanceof checks and casting for example as more recent Java versions have improved the ergonomics a lot.

Checklist

SanderKnauff commented 1 month ago

Great to see this first contribution @SanderKnauff ! Especially like how thorough you are with your tests. Great to see this Sonar rule automatically resolved going forward. I've made it part of org.openrewrite.java.testing.mockito.MockitoBestPractices such that it runs automatically for folks using that, as opposed to having to run this separately.

I've applied some small polishing fixed to you PR, mostly to simplify things. In adopting MethodMatcher you can immediately pass that an expression, which removed the need for a few of the methods you'd spawned off from when you were still checking the method name manually. I've also adopted a few other conventions, like using ListUtils instead of the streams API, and adding a Precondition such that the recipe only visits classes that are using eq at all. Those help performance when running at scale.

As to your question about the Java level: for now we keep recipes at Java 8, but the tests can use Java 17. That way folks coming from older versions of Java can still run recipes while they gradually modernize their applications, as some projects will fail to build when you try to run them on newer Java versions.

I hope that answers your questions; I look forward to seeing what other ideas you have for further improvements.

Thank you for your help on this as well. I've written some smaller recipes that were specific only to internal projects in the past, so it was nice to get some tips on how to make things better. I'll be sure to create another PR the next time I create a similar recipe :)