Consider logic change of mockito argument matchers

[klauerm]

From Mockito 1.x to 3.x the logic of the argument matchers has changed. Mockito.any() doesn't match a null value that is passed to the mocked method as parameter. (see https://site.mockito.org/javadoc/current/org/mockito/ArgumentMatchers.html and https://javadoc.io/static/org.mockito/mockito-core/5.1.1/org/mockito/ArgumentMatchers.html#nullable(java.lang.Class))

Therefore, it is necessary to replace all instances of Mockito.anyString() and any() with nullable(String) or nullable(). Nullable behaves exactly like anyString() or any() in the previous Mockito version 1.x.

[timtebeek]

Good that you've pointed out these specifics; I've extracted the below table from the 5.1.1 documentation for clarity:

Modifier and Type	Method	Description
static <T> T	any()	Matches anything, including nulls and varargs.
static <T> T	any​(Class type)	Matches any object of given type, excluding nulls.
static <T> T	nullable​(Class clazz)	Argument that is either null or of the given type.
static String	anyString()	Any non-null String

With this further note on any​(Class type):

Since Mockito 2.1.0, only allow non-null instance of , thus null is not anymore a valid value. As reference are nullable, the suggested API to match null would be isNull(). We felt this change would make tests harness much safer that it was with Mockito 1.x.

[timtebeek]

Are you proposing then that for the migration from 1.x to 3.x+ we swap any(Class) to nullable(Class) to adhere to the exact same semantics, rather than retain any(Class) with slightly changed null handling semantics?

As a developer I'd be mildly surprised about such a change, or at least until I read up on the specifics like we see above. It's not been pointed out as a bug before; but perhaps like you said nullable(Class) is the correct one-to-one replacement to do no harm.

I imagine this would be a yaml only change in Mockito1to3Migration, which could make this relatively straightforward.

[klauerm]

We did a huge Mockito upgrade in our company with million lines of code. The main root cause for the afterwards failing tests was this semantic change. We weren't aware of it upfront and so the affected colleagues did search and replace in their IDEs which costs a lot of time. So having this fix would be beneficial for all upcoming Mockito upgrades.

[timtebeek]

Yes sorry to hear this particular aspect of that migration has not been as smooth as you would have hoped; I think just inserting

  - org.openrewrite.java.ChangeMethodName:
      methodPattern: org.mockito.ArgumentMatchers any(java.lang.Class)
      newMethodName: nullable

into Mockito1to3Migration would be enough to resolve this issue, as well as any test fixes; would you agree?

You're also able to run this change separately with a temporary rewrite.yml at the project root, which is even easier (and faster!) with our platform if you'd like to explore that.

[klauerm]

I'm not sure how to rewrite anyString() to nullable(java.lang.Sring). Anyhow, I'm not able to make this fix due to capacity reasons and as we do not require this fix as our upgrade is finished. Sorry for that.

[timtebeek]

No need to be sorry! We grateful for all your help, both in thorough contributions such as the PowerMock recipe, as well as detailed reports on imperfections such as this one. I've added the anyString case to the table above and added this to the backlog.

[timtebeek]

Picked up as a quick attempt in #320; welcome to look that over, but no requirement whatsoever. :)