Make tracking of stubbings with STRICT_STUBS null-safe with respect to sourceFiles

[chadlwilson]

The current implementation in DefaultStubbingLookupListener that is used to track stubbings and arg mismatches for STRICT_STUBS is not null-safe with respect to source file locations of the stubbing code.

This is a problem because sourceFile locations are not guaranteed to be available from StackTraceElements. In particular, they are null for tests we have written in Groovy (maybe due to use of Traits?), so when a mock is defined in Groovy code, we are forced to define all our mocks as lenient. I'm not sure exactly why this is, possibly there is some magic possible to make this available as we do see such information in exceptions - I'm not a Groovy expert. Nevertheless, it's not guaranteed by the spec to be available.

Details

From spec https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/StackTraceElement.html#getFileName()

Returns:
the name of the file containing the execution point represented by this stack trace element, or null if this information is unavailable.

This data is retrieved via the below code, where filtered.getFileName() is possible to be null:

mockito/src/main/java/org/mockito/internal/debugging/LocationImpl.java

Lines 48 to 61 in 6f9108b

	private void computeStackTraceInformation(
	StackTraceFilter stackTraceFilter, Throwable stackTraceHolder, boolean isInline) {
	StackTraceElement filtered = stackTraceFilter.filterFirst(stackTraceHolder, isInline);
	// there are corner cases where exception can have a null or empty stack trace
	// for example, a custom exception can override getStackTrace() method
	if (filtered == null) {
	this.stackTraceLine = "-> at <<unknown line>>";
	this.sourceFile = "<unknown source file>";
	} else {
	this.stackTraceLine = "-> at " + filtered;
	this.sourceFile = filtered.getFileName();
	}
	}

It is then used at the below, where it is assumed to be non-null from the stubbed location.

mockito/src/main/java/org/mockito/internal/junit/DefaultStubbingLookupListener.java

Lines 75 to 81 in 6f9108b

	// If stubbing and invocation are in the same source file we assume they are in
	// the test code,
	// and we don't flag it as mismatch:
	&& !s.getInvocation()
	.getLocation()
	.getSourceFile()
	.equals(invocation.getLocation().getSourceFile())) {

Which yields the below:

java.lang.NullPointerException: Cannot invoke "String.equals(Object)" because the return value of "org.mockito.invocation.Location.getSourceFile()" is null
        at org.mockito.internal.junit.DefaultStubbingLookupListener.potentialArgMismatches(DefaultStubbingLookupListener.java:81)
        at org.mockito.internal.junit.DefaultStubbingLookupListener.onStubbingLookup(DefaultStubbingLookupListener.java:52)
        at org.mockito.internal.listeners.StubbingLookupNotifier.notifyStubbedAnswerLookup(StubbingLookupNotifier.java:31)
        at org.mockito.internal.handler.MockHandlerImpl.handle(MockHandlerImpl.java:93)
        at org.mockito.internal.handler.NullResultGuardian.handle(NullResultGuardian.java:29)
        at org.mockito.internal.handler.InvocationNotifierHandler.handle(InvocationNotifierHandler.java:34)

Possible Solutions

I'm not quite sure the right way to address this. If it was made naively null-safe with Objects.equals, the code wouldn't really know whether the stubbing and/or invocation was from test code or not, since it seemingly doesn't know the test file and stub.sourceFile == null == invocation.sourceFile probably isn't enough to conclude it's an arg mismatch. In this case probably it should not assume an "arg mismatch" and these would have to go unreported.

This would still have benefit, as you'd still have the unused stubbing reporting even without the PotentialStubbingProblems.

[michaellee8]

Also faced this problem when using mockito 4.5.1 with groovy.