Fail when InjectMocks has multiple candidate mocks to insert into field (#2942)

jfrantzius · Jörg von Frantzius · web-flow · commit 393f0a16982e · 2023-03-18T20:28:50.000+01:00
Fixes #2934 Co-authored-by: Jörg von Frantzius <joerg.frantzius@aperto.com>
diff --git a/src/main/java/org/mockito/internal/configuration/injection/filter/TypeBasedCandidateFilter.java b/src/main/java/org/mockito/internal/configuration/injection/filter/TypeBasedCandidateFilter.java
@@ -4,6 +4,8 @@
  */
 package org.mockito.internal.configuration.injection.filter;
 
+import static org.mockito.internal.exceptions.Reporter.moreThanOneMockCandidate;
+
 import java.lang.reflect.Field;
 import java.lang.reflect.ParameterizedType;
 import java.lang.reflect.Type;
@@ -136,11 +138,24 @@ public OngoingInjector filterCandidate(
             // being wrapped), and MockUtil.getMockSettings() throws exception for those
         }
 
-        return next.filterCandidate(
-                mockTypeMatches,
-                candidateFieldToBeInjected,
-                allRemainingCandidateFields,
-                injectee,
-                injectMocksField);
+        boolean wasMultipleMatches = mockTypeMatches.size() > 1;
+
+        OngoingInjector result =
+                next.filterCandidate(
+                        mockTypeMatches,
+                        candidateFieldToBeInjected,
+                        allRemainingCandidateFields,
+                        injectee,
+                        injectMocksField);
+
+        if (wasMultipleMatches) {
+            // we had found multiple mocks matching by type, see whether following filters
+            // were able to reduce this to single match (e.g. by filtering for matching field names)
+            if (result == OngoingInjector.nop) {
+                // nope, following filters cannot reduce this to a single match
+                throw moreThanOneMockCandidate(candidateFieldToBeInjected, mocks);
+            }
+        }
+        return result;
     }
 }
diff --git a/src/main/java/org/mockito/internal/exceptions/Reporter.java b/src/main/java/org/mockito/internal/exceptions/Reporter.java
@@ -14,6 +14,7 @@
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.List;
+import java.util.stream.Collectors;
 
 import org.mockito.exceptions.base.MockitoAssertionError;
 import org.mockito.exceptions.base.MockitoException;
@@ -53,6 +54,7 @@
 import org.mockito.invocation.Location;
 import org.mockito.invocation.MatchableInvocation;
 import org.mockito.listeners.InvocationListener;
+import org.mockito.mock.MockName;
 import org.mockito.mock.SerializableMode;
 
 /**
@@ -893,6 +895,29 @@ public static MockitoException cannotInjectDependency(
                 details);
     }
 
+    public static MockitoException moreThanOneMockCandidate(
+            Field field, Collection<?> mockCandidates) {
+        List<String> mockNames =
+                mockCandidates.stream()
+                        .map(MockUtil::getMockName)
+                        .map(MockName::toString)
+                        .collect(Collectors.toList());
+        return new MockitoException(
+                join(
+                        "Mockito couldn't inject mock dependency on field "
+                                + "'"
+                                + field
+                                + "' that is annotated with @InjectMocks in your test, ",
+                        "because there were multiple matching mocks (i.e. "
+                                + "fields annotated with @Mock and having matching type): "
+                                + String.join(", ", mockNames)
+                                + ".",
+                        "If you have multiple fields of same type in your class under test "
+                                + "then consider naming the @Mock fields "
+                                + "identically to the respective class under test's fields, "
+                                + "so Mockito can match them by name."));
+    }
+
     private static String exceptionCauseMessageIfAvailable(Exception details) {
         if (details.getCause() == null) {
             return details.getMessage();
diff --git a/subprojects/junit-jupiter/src/test/java/org/mockitousage/GenericTypeMockMultipleMatchesTest.java b/subprojects/junit-jupiter/src/test/java/org/mockitousage/GenericTypeMockMultipleMatchesTest.java
@@ -0,0 +1,69 @@
+/*
+ * Copyright (c) 2023 Mockito contributors
+ * This program is made available under the terms of the MIT License.
+ */
+
+package org.mockitousage;
+
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import java.util.List;
+
+import org.junit.jupiter.api.Nested;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.BeforeEachCallback;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.api.extension.ExtensionContext;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.exceptions.base.MockitoException;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+/**
+ * Verify that a {@link MockitoException} is thrown when there are multiple {@link Mock} fields that
+ * do match a candidate field by type, but cannot be matched by name.
+ *
+ * Uses a JUnit 5 extension to obtain the JUnit 5 {@link ExtensionContext} and
+ * pass it to {@link MockitoExtension#beforeEach(ExtensionContext)}, as the exception
+ * is thrown during {@link org.junit.jupiter.api.BeforeEach}.
+ */
+@ExtendWith(GenericTypeMockMultipleMatchesTest.ContextProvidingExtension.class)
+public class GenericTypeMockMultipleMatchesTest {
+
+    private static ExtensionContext currentExtensionContext;
+
+    public static class ContextProvidingExtension implements BeforeEachCallback {
+        @Override
+        public void beforeEach(ExtensionContext context) throws Exception {
+            currentExtensionContext = context;
+        }
+    }
+
+    private void startMocking(Object testInstance) {
+        MockitoExtension mockitoExtension = new MockitoExtension();
+        mockitoExtension.beforeEach(currentExtensionContext);
+    }
+
+    @Nested
+    public class MultipleCandidatesByTypeTest {
+        public class UnderTestWithMultipleCandidatesByType {
+            List<String> stringList;
+        }
+
+        @Mock
+        List<String> stringList1;
+
+        @Mock
+        List<String> stringList2;
+
+        @InjectMocks
+        UnderTestWithMultipleCandidatesByType underTestWithMultipleCandidates = new UnderTestWithMultipleCandidatesByType();
+
+        @Test
+        void testMultipleCandidatesByTypes() {
+            assertThrows(MockitoException.class, () -> startMocking(this));
+        }
+    }
+
+
+}
diff --git a/subprojects/junit-jupiter/src/test/java/org/mockitousage/GenericTypeMockTest.java b/subprojects/junit-jupiter/src/test/java/org/mockitousage/GenericTypeMockTest.java
@@ -33,7 +33,6 @@
 @ExtendWith(MockitoExtension.class)
 public class GenericTypeMockTest {
 
-
     @Nested
     public class SingleTypeParamTest {
         public class UnderTestWithSingleTypeParam {
@@ -147,31 +146,6 @@ void testGenericSubclass() {
         }
     }
 
-    @Nested
-    public class MultipleCandidatesByTypeTest {
-        public class UnderTestWithMultipleCandidatesByType {
-            List<String> stringList;
-        }
-
-        @Mock
-        List<String> stringList1;
-
-        @Mock
-        List<String> stringList2;
-
-        @InjectMocks
-        UnderTestWithMultipleCandidatesByType underTestWithMultipleCandidates = new UnderTestWithMultipleCandidatesByType();
-
-        @Test
-        void testMultipleCandidatesByTypes() {
-            assertNotNull(stringList1);
-            assertNotNull(stringList2);
-
-            // verify that when mutiple mock candidates exist with same type (but not matching by field names), none will be injected
-            assertNull(underTestWithMultipleCandidates.stringList);
-        }
-    }
-
     @Nested
     public class MultipleCandidatesOneByNameTest {
         public class UnderTestWithMultipleCandidatesOneByName {
