By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement . We’ll occasionally send you account related emails.

This issue is related to a resolved issue #2644 , which was resolved in 4.7.0 by #2664 .

As per the comment by @perlun in the above PR: #2664 (comment)

@big-andy-coates I found a similar semi-related issue I think... With 4.7.0, this now works correctly:

        doAnswer(answerVoid((VoidAnswer2<String, Object[]>)
                logger::info
        )).when(mock)
                .info(any(), (Object[]) any());

However , if I try to use this form which is slightly more ergonomic:

        doAnswer( answerVoid( (VoidAnswer2<String, Object[]>)
                logger::info
        ) ).when( mock )
                .info( any(), any( Object[].class ) );

...the mocking will fail to intercept these method calls. 🤔 No error when creating the mock, but it just won't detect the method calls as expected.

I presume this has again something to do with the varargs parsing. javac resolves those two calls to the correct/same logger.info() overload, so so far so good.

Do you want me to create a separate issue/repro repo for this? I'm not even sure if it's something that can be (easily) fixed, I just happened to see it now while trying to "clean up our code"... 😅

Originally posted by @perlun in #2664 (comment)

I've done some digging and the reason the above fails is that Mockito is using the last matcher to match all varags parameters.

Let's simplify the example to make it easier to explain:

For example,

Given:

   String varargMethod(int x, String... args);

With setup:

when(theMock.varargMethod(eq(1), any(String[].class)).thenReturn("whatever");

When the mock encounters a call like:

theMock.varargMethod(1, "a", "b");

It uses the first eq(1) matcher to match the first argument, which passes. It then see's there are more parameters to the call than matchers, so it uses the last matcher for any additional arguments, i.e. it tries to match any(String[].class) against "a" and against "b" . Of course, both of these fail and hence the method returns null , not "whatever" .

If the test code is changed to:

when(theMock.varargMethod(eq(1), any(String.class)).thenReturn("whatever");

...then it works. Indeed, any of the following work:

// Matches any call with one _or more_ String parameters
when(theMock.varargMethod(eq(1), any(String.class)).thenReturn("whatever");
when(theMock.varargMethod(eq(1), any()).thenReturn("whatever");
// Matches any call with two _or more_ String parameters
when(theMock.varargMethod(eq(1), any(String.class), any(String.class)).thenReturn("whatever");
when(theMock.varargMethod(eq(1), any(), any()).thenReturn("whatever");

Conclusion

This could be seen as a bug: the code code see that the last parameter is vararg, and effectively treat the any(String[].class) as any(String.class) . However, we'd need to thing through the edge cases of this and how such a change would affect existing usages.

or...

This could be seen as intentional behaviour.

Releated

Interestingly, there used to be a anyVargs method, but it was removed.

There was also a PRs that got close to merging that might have solved this and other varargs related issues... ( #1224 , #1235 , #1236 , #1461 )

There are also a whole host of other issues pertaining to issues with varargs, ( #1222 , #584 , #1593 )

Unfortunately, I personally don't have time to sift through them all to distill the underlying issues and a suitable solution...

Given our shenanigans with var args, I feel link our current solution is the most pragmatic and least-surprising. As you noted, we have had loads of back-and-forths on the issue of varargs, with numerous competing implementations and surprising behavior with each one. Acknowledging that there are still edge cases that are annoying, I think what we have today is the best we got. So therefore I think I would qualify this as

This could be seen as intentional behaviour.

I'd kind of agree, given I don't have time to fix what looks to be a complicated issue, and there are workarounds.

But then I got looking into this closely related #1593 , and was thinking about a fix, and I may have a solution.

I'll throw up a hack PR to see if we think the approach can work.

This PR contains changes NOT intended to be committed 'as-is', but as a showcase for a potential solution to:
* mockito#2796
* mockito#1593
And potentially other vararg related issues.
The crux of the issue is that Mockito needs to handle the last matcher passed to a method, when that matcher aligns with a vararg parameter and has the same type as the vararg parameter.
For example,
```java
public interface Foo {
 String m1(String... args);  // Vararg param at index 0 and type String[]
@test
public void shouldWork2() throws Exception  {
  // Last matcher at index 0, and with type String[]: needs special handling!
  given(foo.m1(any(String[].class))).willReturn("var arg method");
In such situations that code needs to match the raw argument, _not_ the current functionality, which is to use the last matcher to match the last _non raw_ argument.
Unfortunately, I'm not aware of a way to get at the type of the matcher without adding a method to `VarargMatcher`
to get this information. This is the downside of this approach.

Using the new `type()`, we can differentiate between matching all varargs
or only one argument of the varargs.
# Benefits:
Because this approach leaves `VarargsMatcher` untouched, it does not require additional existing matchers to implement `VarargsMatcher` to fix issues such as #567. Where as the first PR would require `Null` and `NotNull` to be marked `VarargsMatcher`.
This PR creates new variants of `isNotNull` and `isNull` to address #567. 
Having `InstanceOf` override `type()` provides a workable solution to #1593.
Having `equals` override `type` addresses #1222.
# Downsides
The obvious downside is that this changes the public `ArgumentMatcher` interface, though in a backwards compatible way.
## Known limitation
The main limitation I'm aware of, is not a new limitation. It is that it is not possible to assert only a single parameter is passed to the vararg parameter, when using a `VarargMatcher`, e.g. `any()`. (ref: #1593). For example:
```java
// Given method:
int vararg(String... args);
// I want to mock this invocation:
mock.vararag("one param");
// ...but not these:
mock.vararg();
mock.vararg("more than", "one param");
There is no current way to do this.  This is because in the following intuitive mocking:
```java
given(mock.vararg(any(String.class))).willReturn(1);
... matches zero or more vararg parameters, as the `any()` method is using `VarargMatcher`. It seems to me that `VarargMatcher` is... a little broken!  This is maybe something that should be consider a candiate for fixing in the next major version bump.  
While it is not possible to fix any `VarargMatcher` based matchers in a backwards compatible way, this the approach in this PR it is possible to mock/verify exactly one vararg param using `isA`, rather than `any`:
```java
    @test
    public void shouldMatchExactlyOnParam() {
        mock.varargs("one param");
        verify(mock).varargs(isA(String.class));
    @test
    public void shouldNotMatchMoreParams() {
        mock.varargs("two", "params");
        verify(mock, never()).varargs(isA(String.class));
    @test
    public void shouldMatchAnyNumberOfParams() {
        mock.varargs("two", "params");
        verify(mock).varargs(isA(String[].class));
... because `isA` does not implement `VarargsMatcher`, and so can work as expected once it implements `type()`.
Fixes #2796
Fixes #567
Fixes #584
Fixes #1222
Fixes #1498

Using the new `type()`, we can differentiate between matching all varargs
or only one argument of the varargs.
# Benefits:
Because this approach leaves `VarargsMatcher` untouched, it does not require additional existing matchers to implement `VarargsMatcher` to fix issues such as #567. Where as the first PR would require `Null` and `NotNull` to be marked `VarargsMatcher`.
This PR creates new variants of `isNotNull` and `isNull` to address #567. 
Having `InstanceOf` override `type()` provides a workable solution to #1593.
Having `equals` override `type` addresses #1222.
# Downsides
The obvious downside is that this changes the public `ArgumentMatcher` interface, though in a backwards compatible way.
## Known limitation
The main limitation I'm aware of, is not a new limitation. It is that it is not possible to assert only a single parameter is passed to the vararg parameter, when using a `VarargMatcher`, e.g. `any()`. (ref: #1593). For example:
```java
// Given method:
int vararg(String... args);
// I want to mock this invocation:
mock.vararag("one param");
// ...but not these:
mock.vararg();
mock.vararg("more than", "one param");
There is no current way to do this.  This is because in the following intuitive mocking:
```java
given(mock.vararg(any(String.class))).willReturn(1);
... matches zero or more vararg parameters, as the `any()` method is using `VarargMatcher`. It seems to me that `VarargMatcher` is... a little broken!  This is maybe something that should be consider a candiate for fixing in the next major version bump.  
While it is not possible to fix any `VarargMatcher` based matchers in a backwards compatible way, this the approach in this PR it is possible to mock/verify exactly one vararg param using `isA`, rather than `any`:
```java
    @test
    public void shouldMatchExactlyOnParam() {
        mock.varargs("one param");
        verify(mock).varargs(isA(String.class));
    @test
    public void shouldNotMatchMoreParams() {
        mock.varargs("two", "params");
        verify(mock, never()).varargs(isA(String.class));
    @test
    public void shouldMatchAnyNumberOfParams() {
        mock.varargs("two", "params");
        verify(mock).varargs(isA(String[].class));
... because `isA` does not implement `VarargsMatcher`, and so can work as expected once it implements `type()`.
Fixes #2796
Fixes #567
Fixes #584
Fixes #1222
Fixes #1498