From 2c6a3414a3ed9c84609f91eb8d85a7d9ddbf5b36 Mon Sep 17 00:00:00 2001 From: Big Andy <8012398+big-andy-coates@users.noreply.github.com> Date: Mon, 28 Nov 2022 11:00:45 +0000 Subject: [PATCH] Simplify `MatcherApplicationStrategy` Prep work for https://github.com/mockito/mockito/issues/2796 The class is overly complex, with the precomputed `matchingType` adding to value. --- .../MatcherApplicationStrategy.java | 80 +++++++------------ .../MatcherApplicationStrategyTest.java | 11 +-- 2 files changed, 35 insertions(+), 56 deletions(-) diff --git a/src/main/java/org/mockito/internal/invocation/MatcherApplicationStrategy.java b/src/main/java/org/mockito/internal/invocation/MatcherApplicationStrategy.java index acc7382e2c..5e16f27040 100644 --- a/src/main/java/org/mockito/internal/invocation/MatcherApplicationStrategy.java +++ b/src/main/java/org/mockito/internal/invocation/MatcherApplicationStrategy.java @@ -4,10 +4,6 @@ */ package org.mockito.internal.invocation; -import static org.mockito.internal.invocation.MatcherApplicationStrategy.MatcherApplicationType.ERROR_UNSUPPORTED_NUMBER_OF_MATCHERS; -import static org.mockito.internal.invocation.MatcherApplicationStrategy.MatcherApplicationType.MATCH_EACH_VARARGS_WITH_LAST_MATCHER; -import static org.mockito.internal.invocation.MatcherApplicationStrategy.MatcherApplicationType.ONE_MATCHER_PER_ARGUMENT; - import java.util.ArrayList; import java.util.List; @@ -20,22 +16,12 @@ public class MatcherApplicationStrategy { private final Invocation invocation; - private final List> matchers; - private final MatcherApplicationType matchingType; + private final List> matchers; private MatcherApplicationStrategy( - Invocation invocation, - List> matchers, - MatcherApplicationType matchingType) { + Invocation invocation, List> matchers) { this.invocation = invocation; - if (matchingType == MATCH_EACH_VARARGS_WITH_LAST_MATCHER) { - int times = varargLength(invocation); - this.matchers = appendLastMatcherNTimes(matchers, times); - } else { - this.matchers = matchers; - } - - this.matchingType = matchingType; + this.matchers = matchers; } /** @@ -51,10 +37,8 @@ private MatcherApplicationStrategy( * @return never null */ public static MatcherApplicationStrategy getMatcherApplicationStrategyFor( - Invocation invocation, List> matchers) { - - MatcherApplicationType type = getMatcherApplicationType(invocation, matchers); - return new MatcherApplicationStrategy(invocation, matchers, type); + Invocation invocation, List> matchers) { + return new MatcherApplicationStrategy(invocation, matchers); } /** @@ -74,11 +58,28 @@ public static MatcherApplicationStrategy getMatcherApplicationStrategyFor( * */ public boolean forEachMatcherAndArgument(ArgumentMatcherAction action) { - if (matchingType == ERROR_UNSUPPORTED_NUMBER_OF_MATCHERS) { - return false; + if (invocation.getArguments().length == matchers.size()) { + return argsMatch(invocation.getArguments(), matchers, action); } - Object[] arguments = invocation.getArguments(); + final boolean isVararg = + invocation.getMethod().isVarArgs() + && invocation.getRawArguments().length == matchers.size() + && isLastMatcherVarargMatcher(matchers); + + if (isVararg) { + int times = varargLength(invocation); + final List> matchers = appendLastMatcherNTimes(times); + return argsMatch(invocation.getArguments(), matchers, action); + } + + return false; + } + + private boolean argsMatch( + Object[] arguments, + List> matchers, + ArgumentMatcherAction action) { for (int i = 0; i < arguments.length; i++) { ArgumentMatcher matcher = matchers.get(i); Object argument = arguments[i]; @@ -90,24 +91,7 @@ public boolean forEachMatcherAndArgument(ArgumentMatcherAction action) { return true; } - private static MatcherApplicationType getMatcherApplicationType( - Invocation invocation, List> matchers) { - final int rawArguments = invocation.getRawArguments().length; - final int expandedArguments = invocation.getArguments().length; - final int matcherCount = matchers.size(); - - if (expandedArguments == matcherCount) { - return ONE_MATCHER_PER_ARGUMENT; - } - - if (rawArguments == matcherCount && isLastMatcherVarargMatcher(matchers)) { - return MATCH_EACH_VARARGS_WITH_LAST_MATCHER; - } - - return ERROR_UNSUPPORTED_NUMBER_OF_MATCHERS; - } - - private static boolean isLastMatcherVarargMatcher(final List> matchers) { + private static boolean isLastMatcherVarargMatcher(List> matchers) { ArgumentMatcher argumentMatcher = lastMatcher(matchers); if (argumentMatcher instanceof HamcrestArgumentMatcher) { return ((HamcrestArgumentMatcher) argumentMatcher).isVarargMatcher(); @@ -115,8 +99,8 @@ private static boolean isLastMatcherVarargMatcher(final List> return argumentMatcher instanceof VarargMatcher; } - private static List> appendLastMatcherNTimes( - List> matchers, int timesToAppendLastMatcher) { + private List> appendLastMatcherNTimes( + int timesToAppendLastMatcher) { ArgumentMatcher lastMatcher = lastMatcher(matchers); List> expandedMatchers = new ArrayList>(matchers); @@ -132,13 +116,7 @@ private static int varargLength(Invocation invocation) { return expandedArgumentCount - rawArgumentCount; } - private static ArgumentMatcher lastMatcher(List> matchers) { + private static ArgumentMatcher lastMatcher(List> matchers) { return matchers.get(matchers.size() - 1); } - - enum MatcherApplicationType { - ONE_MATCHER_PER_ARGUMENT, - MATCH_EACH_VARARGS_WITH_LAST_MATCHER, - ERROR_UNSUPPORTED_NUMBER_OF_MATCHERS; - } } diff --git a/src/test/java/org/mockito/internal/invocation/MatcherApplicationStrategyTest.java b/src/test/java/org/mockito/internal/invocation/MatcherApplicationStrategyTest.java index 285bfecc66..baf7a5ad5a 100644 --- a/src/test/java/org/mockito/internal/invocation/MatcherApplicationStrategyTest.java +++ b/src/test/java/org/mockito/internal/invocation/MatcherApplicationStrategyTest.java @@ -35,7 +35,7 @@ public class MatcherApplicationStrategyTest extends TestBase { @Mock IMethods mock; private Invocation invocation; - private List matchers; + private List> matchers; private RecordingAction recordAction; @@ -213,7 +213,8 @@ public void shouldMatchAnyEvenIfMatcherIsDecorated() { public void shouldMatchAnyEvenIfMatcherIsWrappedInHamcrestMatcher() { // given invocation = varargs("1", "2"); - HamcrestArgumentMatcher argumentMatcher = new HamcrestArgumentMatcher(new IntMatcher()); + HamcrestArgumentMatcher argumentMatcher = + new HamcrestArgumentMatcher<>(new IntMatcher()); matchers = asList(argumentMatcher); // when @@ -224,7 +225,7 @@ public void shouldMatchAnyEvenIfMatcherIsWrappedInHamcrestMatcher() { recordAction.assertContainsExactly(argumentMatcher, argumentMatcher); } - class IntMatcher extends BaseMatcher implements VarargMatcher { + private static class IntMatcher extends BaseMatcher implements VarargMatcher { public boolean matches(Object o) { return true; } @@ -242,8 +243,8 @@ private Invocation varargs(String... s) { return getLastInvocation(); } - private class RecordingAction implements ArgumentMatcherAction { - private List> matchers = new ArrayList>(); + private static class RecordingAction implements ArgumentMatcherAction { + private final List> matchers = new ArrayList>(); @Override public boolean apply(ArgumentMatcher matcher, Object argument) {