error-prone icon indicating copy to clipboard operation
error-prone copied to clipboard

Qualify `CannotMockFinalClass`

Open Stephan202 opened this issue 7 years ago • 1 comments

As of Mockito 2.1 one can mock final classes. This is an opt-in feature, enabled by storing the value mock-maker-inline in the mockito-extensions/org.mockito.plugins.MockMaker classpath resource.

Suggestions:

  1. Update the CannotMockFinalClass documentation and error message, to inform users about this new feature. (And if used, they'll have to disable this check.)
  2. (Preferred.) Have the checker look for the presence of the aforementioned classpath resource and warn about mocked final classes only in the "inline mock maker" is not enabled.

NB: I started working on a PR for option (2), but hit an issue: the only classloader using which I can seem to find the resource is the one returned by JavacProcessingEnvironment.instance(state.context).getProcessorClassLoader(). Using that classloader I have a passing Error Prone unit test, and it also seems to work in a simple Maven project in which Error Prone is enabled. But this classloader no longer exposes the resource once the maven-compiler-plugin is configured with custom <annotationProcessorPaths>. Given the method's name this doesn't surprise me at all, but the question is then: how does one properly look up resources on the compile classpath?

(If the above blurb of text is unclear, I can open a PR to showcase the current approach.)

Stephan202 avatar Feb 05 '18 07:02 Stephan202

Yesterday Mockito 5.0.0 was released, and with it mock-maker-inline became the default. This means that final classes can now be mocked by default. This might cause more people to hit this issue.

Next to the two options suggested above, there's also third option: rename the check to DoNotMockFinalClass, making it a sibling of DoNotMockAutoValue and DoNotMockChecker. Then users can en- or disable the check on ideological grounds.

@lowasser I see that you're the original author of the check; WDYT?

NB: W.r.t. the issue I hit with solution (2): this approach might work, but that's to be tested.

Stephan202 avatar Jan 15 '23 10:01 Stephan202