Replace Mockito 1.x any(Class) and anyString() with nullable(Class)

[timtebeek]

Fixes #319

[timtebeek]

Kun Li also reported a possible workaround; captured here in case we stick to AND and update the docs to reflect that:

---
type: specs.openrewrite.org/v1beta/recipe
name: org.openrewrite.java.testing.mockito.AnyToNullableApplicabilityCheck
displayName: todo
description: todo
tags:
  - testing
  - mockito
recipeList:
  - org.openrewrite.maven.search.FindDependency:
      groupId: org.mockito
      artifactId: mockito-all
  - org.openrewrite.gradle.search.FindDependency:
      groupId: org.mockito
      artifactId: mockito-all
---
type: specs.openrewrite.org/v1beta/recipe
name: org.openrewrite.java.testing.mockito.AnyToNullable
displayName: Replace Mockito 1.x `any(Class)` and `anyString()` with `nullable(Class)`
description: Since Mockito 2.10 `any(Class)` and `anyString()` no longer match null values. Use `nullable(Class)` instead.
tags:
  - testing
  - mockito
applicability:
  anySource:
    - org.openrewrite.java.testing.mockito.AnyToNullableApplicabilityCheck
recipeList:
  - org.openrewrite.java.ChangeMethodName:
      methodPattern: org.mockito.Mockito any(java.lang.Class)
      newMethodName: nullable
  - org.openrewrite.java.ChangeMethodTargetToStatic:
      methodPattern: org.mockito.Mockito nullable(java.lang.Class)
      fullyQualifiedTargetTypeName: org.mockito.ArgumentMatchers
  - org.openrewrite.java.testing.mockito.AnyStringToNullable
---

[shanman190]

I think this may still yield some missing cases as you're just checking to see if the dependency is there. Wouldn't it be better to check that the type or method was in use instead? That'd be a lot more exact and no longer be dependent upon GAV coordinates (since you could pull in the smaller mockito dependencies and desire this recipe to run for that case).

[timtebeek]

Thanks for chiming in! Appreciate your concern to be exact. The issue here is there's a semantic change to the same methods any(Class) and anyString() when going from 1.x to 2.10+. Their signatures stay the same, but their handling of null changes (no longer allowed). I don't know if there's another way to detect that, but I'm open to suggestions.

On main currently we're sure to break those assumptions about null handling, by not changing anything at all.
With this change I'm hoping to selectively change to nullable methods where we can detect that we should (exclusively when going from 1.x to 2.x), while avoiding we change any methods where we should not (anything after 2.10).

Still missing some cases with this PR as-is might then still be preferred over doing nothing, and strongly preferred over incorrectly changing anything when users are already on 2.10+.

How would you feel we should proceed here given the above?

[shanman190]

So this one is going to be difficult for a few reasons.

• We're won't be able to rely upon the LST itself because of the case of if the upgrade mockito dependency came first
• As you said, you only want to apply this recipe IF AND ONLY IF, the project was on a Mockito <2.10.

The only way that I can think of to achieve this would be to use the visit(List<SourceFile>, ExecutionContext) mechanic. We would then use an internal visitor to visit the Maven and Gradle dependency descriptors and interrogate the markers to see what mockito version was present at the time the LST was generated, then if we find our true case, we would schedule the other Recipe instances via doNext(Recipe) or doNext(Visitor).

[shanman190]

Actually, just had an idea. Both Gradle and Maven have a Dependency Insight recipe. If we instead used this, which does look at the markers instead of the LST, then we can achieve our goal.

So what I'm envisioning is something like this (1 programmatic recipes (already written), 3 declarative recipes (1 written, but needs modifications)):

• Your any() to nullable(Class) recipe
• Gradle specific selection recipe (Gradle Dependency Insight, that triggers your existing recipe)
• Maven specific selection recipe (Maven Dependency Insight, that triggers your existing recipe)
• Your upgrade recipe that now depends upon each of the Gradle and Maven specific ones in the recipeList. (Note: this would have no applicability tests and would just trigger the build tool specific recipes always relying upon their applicability tests)

For the DependencyInsight, we could do something like:
Group = org.mockito
Artifact = mockito*
Version = (,2.10)

This would grant us our exact desired outcome. WDYT?

[sambsnyd]

Thanks @timtebeek. Good ideas @shanman190 , would be happy to see those improvements made