Skip to content

Commit 483e15f

Browse files
big-andy-coatesTimvdLippe
big-andy-coates
authored and
TimvdLippe
committed
Add type() method to ArgumentMatcher (#2807)
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
1 parent a4e2e48 commit 483e15f

26 files changed

+723
-67
lines changed

‎src/main/java/org/mockito/ArgumentCaptor.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -62,11 +62,12 @@
6262
@CheckReturnValue
6363
public class ArgumentCaptor<T> {
6464

65-
private final CapturingMatcher<T> capturingMatcher = new CapturingMatcher<T>();
65+
private final CapturingMatcher<T> capturingMatcher;
6666
private final Class<? extends T> clazz;
6767

6868
private ArgumentCaptor(Class<? extends T> clazz) {
6969
this.clazz = clazz;
70+
this.capturingMatcher = new CapturingMatcher<T>(clazz);
7071
}
7172

7273
/**

‎src/main/java/org/mockito/ArgumentMatcher.java

+41
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,7 @@
110110
* @param <T> type of argument
111111
* @since 2.1.0
112112
*/
113+
@FunctionalInterface
113114
public interface ArgumentMatcher<T> {
114115

115116
/**
@@ -125,4 +126,44 @@ public interface ArgumentMatcher<T> {
125126
* @return true if this matcher accepts the given argument.
126127
*/
127128
boolean matches(T argument);
129+
130+
/**
131+
* The type of the argument this matcher matches.
132+
*
133+
* <p>This method is used to differentiate between a matcher used to match a raw vararg array parameter
134+
* from a matcher used to match a single value passed as a vararg parameter.
135+
*
136+
* <p>Where the matcher:
137+
* <ul>
138+
* <li>is at the parameter index of a vararg parameter</li>
139+
* <li>is the last matcher passed</li>
140+
* <li>this method returns a type assignable to the vararg parameter's raw type, i.e. its array type.</li>
141+
* </ul>
142+
*
143+
* ...then the matcher is matched against the raw vararg parameter, rather than the first element of the raw parameter.
144+
*
145+
* <p>For example:
146+
*
147+
* <pre class="code"><code class="java">
148+
* // Given vararg method with signature:
149+
* int someVarargMethod(String... args);
150+
*
151+
* // The following will match invocations with any number of parameters, i.e. any number of elements in the raw array.
152+
* mock.someVarargMethod(isA(String[].class));
153+
*
154+
* // The following will match invocations with a single parameter, i.e. one string in the raw array.
155+
* mock.someVarargMethod(isA(String.class));
156+
*
157+
* // The following will match invocations with two parameters, i.e. two strings in the raw array
158+
* mock.someVarargMethod(isA(String.class), isA(String.class));
159+
* </code></pre>
160+
*
161+
* <p>Only matcher implementations that can conceptually match a raw vararg parameter should override this method.
162+
*
163+
* @return the type this matcher handles. The default value of {@link Void} means the type is not known.
164+
* @since 5.0.0
165+
*/
166+
default Class<?> type() {
167+
return Void.class;
168+
}
128169
}

‎src/main/java/org/mockito/ArgumentMatchers.java

+57
Original file line numberDiff line numberDiff line change
@@ -699,6 +699,23 @@ public static <T> T isNull() {
699699
return null;
700700
}
701701

702+
/**
703+
* <code>null</code> argument.
704+
*
705+
* <p>
706+
* See examples in javadoc for {@link ArgumentMatchers} class
707+
* </p>
708+
*
709+
* @param type the type of the argument being matched.
710+
* @return <code>null</code>.
711+
* @see #isNotNull(Class)
712+
* @since 5.0.0
713+
*/
714+
public static <T> T isNull(Class<T> type) {
715+
reportMatcher(new Null<>(type));
716+
return null;
717+
}
718+
702719
/**
703720
* Not <code>null</code> argument.
704721
*
@@ -717,6 +734,26 @@ public static <T> T notNull() {
717734
return null;
718735
}
719736

737+
/**
738+
* Not <code>null</code> argument.
739+
*
740+
* <p>
741+
* Alias to {@link ArgumentMatchers#isNotNull()}
742+
* </p>
743+
*
744+
* <p>
745+
* See examples in javadoc for {@link ArgumentMatchers} class
746+
* </p>
747+
*
748+
* @param type the type of the argument being matched.
749+
* @return <code>null</code>.
750+
* @since 5.0.0
751+
*/
752+
public static <T> T notNull(Class<T> type) {
753+
reportMatcher(new NotNull<>(type));
754+
return null;
755+
}
756+
720757
/**
721758
* Not <code>null</code> argument.
722759
*
@@ -735,6 +772,26 @@ public static <T> T isNotNull() {
735772
return notNull();
736773
}
737774

775+
/**
776+
* Not <code>null</code> argument.
777+
*
778+
* <p>
779+
* Alias to {@link ArgumentMatchers#notNull(Class)}
780+
* </p>
781+
*
782+
* <p>
783+
* See examples in javadoc for {@link ArgumentMatchers} class
784+
* </p>
785+
*
786+
* @param type the type of the argument being matched.
787+
* @return <code>null</code>.
788+
* @see #isNull()
789+
* @since 5.0.0
790+
*/
791+
public static <T> T isNotNull(Class<T> type) {
792+
return notNull(type);
793+
}
794+
738795
/**
739796
* Argument that is either <code>null</code> or of the given type.
740797
*

‎src/main/java/org/mockito/internal/hamcrest/HamcrestArgumentMatcher.java

+1
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ public boolean matches(Object argument) {
2222
return this.matcher.matches(argument);
2323
}
2424

25+
@SuppressWarnings("deprecation")
2526
public boolean isVarargMatcher() {
2627
return matcher instanceof VarargMatcher;
2728
}

‎src/main/java/org/mockito/internal/invocation/MatcherApplicationStrategy.java

+24-12
Original file line numberDiff line numberDiff line change
@@ -58,17 +58,24 @@ public static MatcherApplicationStrategy getMatcherApplicationStrategyFor(
5858
* </ul>
5959
*/
6060
public boolean forEachMatcherAndArgument(ArgumentMatcherAction action) {
61+
final boolean maybeVararg =
62+
invocation.getMethod().isVarArgs()
63+
&& invocation.getRawArguments().length == matchers.size();
64+
65+
if (maybeVararg) {
66+
final Class<?> matcherType = lastMatcher().type();
67+
final Class<?> paramType = lastParameterType();
68+
if (paramType.isAssignableFrom(matcherType)) {
69+
return argsMatch(invocation.getRawArguments(), matchers, action);
70+
}
71+
}
72+
6173
if (invocation.getArguments().length == matchers.size()) {
6274
return argsMatch(invocation.getArguments(), matchers, action);
6375
}
6476

65-
final boolean isVararg =
66-
invocation.getMethod().isVarArgs()
67-
&& invocation.getRawArguments().length == matchers.size()
68-
&& isLastMatcherVarargMatcher(matchers);
69-
70-
if (isVararg) {
71-
int times = varargLength(invocation);
77+
if (maybeVararg && isLastMatcherVarargMatcher()) {
78+
int times = varargLength();
7279
final List<? extends ArgumentMatcher<?>> matchers = appendLastMatcherNTimes(times);
7380
return argsMatch(invocation.getArguments(), matchers, action);
7481
}
@@ -91,8 +98,8 @@ private boolean argsMatch(
9198
return true;
9299
}
93100

94-
private static boolean isLastMatcherVarargMatcher(List<? extends ArgumentMatcher<?>> matchers) {
95-
ArgumentMatcher<?> argumentMatcher = lastMatcher(matchers);
101+
private boolean isLastMatcherVarargMatcher() {
102+
ArgumentMatcher<?> argumentMatcher = lastMatcher();
96103
if (argumentMatcher instanceof HamcrestArgumentMatcher<?>) {
97104
return ((HamcrestArgumentMatcher<?>) argumentMatcher).isVarargMatcher();
98105
}
@@ -101,7 +108,7 @@ private static boolean isLastMatcherVarargMatcher(List<? extends ArgumentMatcher
101108

102109
private List<? extends ArgumentMatcher<?>> appendLastMatcherNTimes(
103110
int timesToAppendLastMatcher) {
104-
ArgumentMatcher<?> lastMatcher = lastMatcher(matchers);
111+
ArgumentMatcher<?> lastMatcher = lastMatcher();
105112

106113
List<ArgumentMatcher<?>> expandedMatchers = new ArrayList<ArgumentMatcher<?>>(matchers);
107114
for (int i = 0; i < timesToAppendLastMatcher; i++) {
@@ -110,13 +117,18 @@ private List<? extends ArgumentMatcher<?>> appendLastMatcherNTimes(
110117
return expandedMatchers;
111118
}
112119

113-
private static int varargLength(Invocation invocation) {
120+
private int varargLength() {
114121
int rawArgumentCount = invocation.getRawArguments().length;
115122
int expandedArgumentCount = invocation.getArguments().length;
116123
return expandedArgumentCount - rawArgumentCount;
117124
}
118125

119-
private static ArgumentMatcher<?> lastMatcher(List<? extends ArgumentMatcher<?>> matchers) {
126+
private ArgumentMatcher<?> lastMatcher() {
120127
return matchers.get(matchers.size() - 1);
121128
}
129+
130+
private Class<?> lastParameterType() {
131+
final Class<?>[] parameterTypes = invocation.getMethod().getParameterTypes();
132+
return parameterTypes[parameterTypes.length - 1];
133+
}
122134
}

‎src/main/java/org/mockito/internal/matchers/And.java

+7
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,13 @@ public boolean matches(Object actual) {
2323
return m1.matches(actual) && m2.matches(actual);
2424
}
2525

26+
@Override
27+
public Class<?> type() {
28+
return m1.type().isAssignableFrom(m2.type())
29+
? m1.type()
30+
: m2.type().isAssignableFrom(m1.type()) ? m2.type() : ArgumentMatcher.super.type();
31+
}
32+
2633
@Override
2734
public String toString() {
2835
return "and(" + m1 + ", " + m2 + ")";

‎src/main/java/org/mockito/internal/matchers/Any.java

+5
Original file line numberDiff line numberDiff line change
@@ -21,4 +21,9 @@ public boolean matches(Object actual) {
2121
public String toString() {
2222
return "<any>";
2323
}
24+
25+
@Override
26+
public Class<?> type() {
27+
return Object.class;
28+
}
2429
}

‎src/main/java/org/mockito/internal/matchers/CapturingMatcher.java

+11
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import java.io.Serializable;
1010
import java.util.ArrayList;
1111
import java.util.List;
12+
import java.util.Objects;
1213
import java.util.concurrent.locks.Lock;
1314
import java.util.concurrent.locks.ReadWriteLock;
1415
import java.util.concurrent.locks.ReentrantReadWriteLock;
@@ -19,12 +20,17 @@
1920
public class CapturingMatcher<T>
2021
implements ArgumentMatcher<T>, CapturesArguments, VarargMatcher, Serializable {
2122

23+
private final Class<? extends T> clazz;
2224
private final List<Object> arguments = new ArrayList<>();
2325

2426
private final ReadWriteLock lock = new ReentrantReadWriteLock();
2527
private final Lock readLock = lock.readLock();
2628
private final Lock writeLock = lock.writeLock();
2729

30+
public CapturingMatcher(final Class<? extends T> clazz) {
31+
this.clazz = Objects.requireNonNull(clazz);
32+
}
33+
2834
@Override
2935
public boolean matches(Object argument) {
3036
return true;
@@ -66,4 +72,9 @@ public void captureFrom(Object argument) {
6672
writeLock.unlock();
6773
}
6874
}
75+
76+
@Override
77+
public Class<?> type() {
78+
return clazz;
79+
}
6980
}

‎src/main/java/org/mockito/internal/matchers/Equals.java

+5
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,11 @@ public boolean matches(Object actual) {
2222
return Equality.areEqual(this.wanted, actual);
2323
}
2424

25+
@Override
26+
public Class<?> type() {
27+
return wanted != null ? wanted.getClass() : ArgumentMatcher.super.type();
28+
}
29+
2530
@Override
2631
public String toString() {
2732
return describe(wanted);

‎src/main/java/org/mockito/internal/matchers/InstanceOf.java

+11-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111

1212
public class InstanceOf implements ArgumentMatcher<Object>, Serializable {
1313

14-
private final Class<?> clazz;
14+
final Class<?> clazz;
1515
private final String description;
1616

1717
public InstanceOf(Class<?> clazz) {
@@ -30,6 +30,11 @@ public boolean matches(Object actual) {
3030
|| clazz.isAssignableFrom(actual.getClass()));
3131
}
3232

33+
@Override
34+
public Class<?> type() {
35+
return clazz;
36+
}
37+
3338
@Override
3439
public String toString() {
3540
return description;
@@ -44,5 +49,10 @@ public VarArgAware(Class<?> clazz) {
4449
public VarArgAware(Class<?> clazz, String describedAs) {
4550
super(clazz, describedAs);
4651
}
52+
53+
@Override
54+
public Class<?> type() {
55+
return clazz;
56+
}
4757
}
4858
}

‎src/main/java/org/mockito/internal/matchers/Not.java

+5
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,11 @@ public boolean matches(Object actual) {
2222
return !matcher.matches(actual);
2323
}
2424

25+
@Override
26+
public Class<?> type() {
27+
return matcher.type();
28+
}
29+
2530
@Override
2631
public String toString() {
2732
return "not(" + matcher + ")";

‎src/main/java/org/mockito/internal/matchers/NotNull.java

+13-3
Original file line numberDiff line numberDiff line change
@@ -5,20 +5,30 @@
55
package org.mockito.internal.matchers;
66

77
import java.io.Serializable;
8+
import java.util.Objects;
89

910
import org.mockito.ArgumentMatcher;
1011

11-
public class NotNull implements ArgumentMatcher<Object>, Serializable {
12+
public class NotNull<T> implements ArgumentMatcher<T>, Serializable {
1213

13-
public static final NotNull NOT_NULL = new NotNull();
14+
public static final NotNull<Object> NOT_NULL = new NotNull<>(Object.class);
1415

15-
private NotNull() {}
16+
private final Class<T> type;
17+
18+
public NotNull(Class<T> type) {
19+
this.type = Objects.requireNonNull(type);
20+
}
1621

1722
@Override
1823
public boolean matches(Object actual) {
1924
return actual != null;
2025
}
2126

27+
@Override
28+
public Class<T> type() {
29+
return type;
30+
}
31+
2232
@Override
2333
public String toString() {
2434
return "notNull()";

0 commit comments

Comments
 (0)