Fixes mockito#1516 and mockito#1631 : Allows @SPY with @Injectmocks t…

…o be injected into other @Injectmocks

src/main/java/org/mockito/internal/configuration/DefaultInjectionEngine.java

@@ -16,6 +16,38 @@
  */
 public class DefaultInjectionEngine {
 
+    /**
+     * Proceeds to ongoing mocks injection on fields with:
+     * <ul>
+     * <li>strict constructor injection strategy to not allow semi-initialized fields at this step
+     * <li>lenient field/property injection strategy to skip fields without no-args constructor
+     * </ul>
+     *
+     * @param needingInjection  fields needing injection
+     * @param mocks             mocks available for injection
+     * @param testClassInstance instance of the test
+     */
+    public void injectOngoingMocksOnFields(
+            Set<Field> needingInjection, Set<Object> mocks, Object testClassInstance) {
+        MockInjection.onFields(needingInjection, testClassInstance)
+                .withMocks(mocks)
+                .tryStrictConstructorInjection()
+                .tryLenientPropertyOrFieldInjection()
+                .handleSpyAnnotation()
+                .apply();
+    }
+
+    /**
+     * Proceeds to terminal mocks injection on fields with:
+     * <ul>
+     * <li>lenient constructor injection strategy to initialize fields even with null arguments
+     * <li>strict field/property injection strategy to fail on fields without no-args constructor
+     * </ul>
+     *
+     * @param needingInjection fields needing injection
+     * @param mocks mocks available for injection
+     * @param testClassInstance instance of the test
+     */
     public void injectMocksOnFields(
             Set<Field> needingInjection, Set<Object> mocks, Object testClassInstance) {
         MockInjection.onFields(needingInjection, testClassInstance)

src/main/java/org/mockito/internal/configuration/InjectingAnnotationEngine.java

@@ -8,7 +8,6 @@
 
 import java.lang.reflect.Field;
 import java.util.ArrayList;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
 
@@ -24,6 +23,7 @@
 public class InjectingAnnotationEngine implements AnnotationEngine {
     private final AnnotationEngine delegate = new IndependentAnnotationEngine();
     private final AnnotationEngine spyAnnotationEngine = new SpyAnnotationEngine();
+    private final DefaultInjectionEngine injectionEngine = new DefaultInjectionEngine();
 
     /**
      * Process the fields of the test instance and create Mocks, Spies, Captors and inject them on fields
@@ -45,25 +45,14 @@ public class InjectingAnnotationEngine implements AnnotationEngine {
     public AutoCloseable process(Class<?> clazz, Object testInstance) {
         List<AutoCloseable> closeables = new ArrayList<>();
         closeables.addAll(processIndependentAnnotations(testInstance.getClass(), testInstance));
-        closeables.addAll(processInjectMocks(testInstance.getClass(), testInstance));
+        closeables.add(injectCloseableMocks(testInstance));
         return () -> {
             for (AutoCloseable closeable : closeables) {
                 closeable.close();
             }
         };
     }
 
-    private List<AutoCloseable> processInjectMocks(
-            final Class<?> clazz, final Object testInstance) {
-        List<AutoCloseable> closeables = new ArrayList<>();
-        Class<?> classContext = clazz;
-        while (classContext != Object.class) {
-            closeables.add(injectCloseableMocks(testInstance));
-            classContext = classContext.getSuperclass();
-        }
-        return closeables;
-    }
-
     private List<AutoCloseable> processIndependentAnnotations(
             final Class<?> clazz, final Object testInstance) {
         List<AutoCloseable> closeables = new ArrayList<>();
@@ -104,18 +93,16 @@ public void injectMocks(Object testClassInstance) {
      */
     private AutoCloseable injectCloseableMocks(final Object testClassInstance) {
         Class<?> clazz = testClassInstance.getClass();
-        Set<Field> mockDependentFields = new HashSet<>();
+        Set<Field> fieldsToMock = new InjectMocksScanner(clazz).scanHierarchy();
         Set<Object> mocks = newMockSafeHashSet();
-
-        while (clazz != Object.class) {
-            new InjectMocksScanner(clazz).addTo(mockDependentFields);
-            new MockScanner(testClassInstance, clazz).addPreparedMocks(mocks);
-            onInjection(testClassInstance, clazz, mockDependentFields, mocks);
-            clazz = clazz.getSuperclass();
-        }
-
-        new DefaultInjectionEngine()
-                .injectMocksOnFields(mockDependentFields, mocks, testClassInstance);
+        Set<Object> previousMocks = newMockSafeHashSet();
+        do {
+            previousMocks.addAll(mocks);
+            mocks.addAll(new MockScanner(testClassInstance, clazz).scanHierarchy());
+            onInjection(testClassInstance, clazz, fieldsToMock, mocks);
+            injectionEngine.injectOngoingMocksOnFields(fieldsToMock, mocks, testClassInstance);
+        } while (!mocks.equals(previousMocks));
+        injectionEngine.injectMocksOnFields(fieldsToMock, mocks, testClassInstance);
 
         return () -> {
             for (Object mock : mocks) {

src/main/java/org/mockito/internal/configuration/SpyAnnotationEngine.java

@@ -39,8 +39,9 @@
  * </p>
  * <p/>
  * <p>
- * If the field is also annotated with the <strong>compatible</strong> &#64;InjectMocks then the field will be ignored,
- * The injection engine will handle this specific case.
+ * If the field is also annotated with the <strong>compatible</strong> &#64;InjectMocks and has
+ * parameterized constructor then the field will be ignored, the injection engine will handle this
+ * specific case.
  * </p>
  * <p/>
  * <p>This engine will fail, if the field is also annotated with incompatible Mockito annotations.
@@ -53,8 +54,7 @@ public AutoCloseable process(Class<?> context, Object testInstance) {
         Field[] fields = context.getDeclaredFields();
         MemberAccessor accessor = Plugins.getMemberAccessor();
         for (Field field : fields) {
-            if (field.isAnnotationPresent(Spy.class)
-                    && !field.isAnnotationPresent(InjectMocks.class)) {
+            if (shouldProcess(field)) {
                 assertNoIncompatibleAnnotations(Spy.class, field, Mock.class, Captor.class);
                 Object instance;
                 try {
@@ -82,6 +82,19 @@ public AutoCloseable process(Class<?> context, Object testInstance) {
         return new NoAction();
     }
 
+    private boolean shouldProcess(Field field) {
+        if (!field.isAnnotationPresent(Spy.class)) {
+            return false;
+        }
+        if (!field.isAnnotationPresent(InjectMocks.class)) {
+            return true;
+        }
+        if (field.getType().isInterface()) {
+            return false;
+        }
+        return !hasParameterizedConstructor(field.getType());
+    }
+
     private static Object spyInstance(Field field, Object instance) {
         return Mockito.mock(
                 instance.getClass(),
@@ -131,6 +144,15 @@ private static Object spyNewInstance(Object testInstance, Field field)
         }
     }
 
+    private static boolean hasParameterizedConstructor(Class<?> type) {
+        for (Constructor<?> constructor : type.getDeclaredConstructors()) {
+            if (constructor.getParameterTypes().length > 0) {
+                return true;
+            }
+        }
+        return false;
+    }
+
     private static Constructor<?> noArgConstructorOf(Class<?> type) {
         Constructor<?> constructor;
         try {

src/main/java/org/mockito/internal/configuration/injection/ConstructorInjection.java

@@ -8,46 +8,33 @@
 
 import java.lang.reflect.Field;
 import java.lang.reflect.InvocationTargetException;
-import java.util.ArrayList;
-import java.util.List;
 import java.util.Set;
 
 import org.mockito.exceptions.base.MockitoException;
+import org.mockito.internal.util.reflection.ConstructorResolver;
+import org.mockito.internal.util.reflection.ConstructorResolver.BiggestConstructorResolver;
 import org.mockito.internal.util.reflection.FieldInitializationReport;
 import org.mockito.internal.util.reflection.FieldInitializer;
-import org.mockito.internal.util.reflection.FieldInitializer.ConstructorArgumentResolver;
 
 /**
  * Injection strategy based on constructor.
  *
  * <p>
  * The strategy will search for the constructor with most parameters
- * and try to resolve mocks by type.
- * </p>
- *
- * <blockquote>
- * TODO on missing mock type, shall it abandon or create "noname" mocks.
- * TODO and what if the arg type is not mockable.
- * </blockquote>
- *
- * <p>
- * For now the algorithm tries to create anonymous mocks if an argument type is missing.
- * If not possible the algorithm abandon resolution.
+ * and try to resolve mocks by type, or null if there is no mocks matching a parameter.
  * </p>
  */
 public class ConstructorInjection extends MockInjectionStrategy {
 
-    public ConstructorInjection() {}
-
     @Override
     public boolean processInjection(Field field, Object fieldOwner, Set<Object> mockCandidates) {
         try {
-            SimpleArgumentResolver simpleArgumentResolver =
-                    new SimpleArgumentResolver(mockCandidates);
+            ConstructorResolver constructorResolver =
+                    createConstructorResolver(field.getType(), mockCandidates);
             FieldInitializationReport report =
-                    new FieldInitializer(fieldOwner, field, simpleArgumentResolver).initialize();
+                    new FieldInitializer(fieldOwner, field, constructorResolver).initialize();
 
-            return report.fieldWasInitializedUsingContructorArgs();
+            return report.fieldWasInitialized();
         } catch (MockitoException e) {
             if (e.getCause() instanceof InvocationTargetException) {
                 Throwable realCause = e.getCause().getCause();
@@ -58,32 +45,8 @@ public boolean processInjection(Field field, Object fieldOwner, Set<Object> mock
         }
     }
 
-    /**
-     * Returns mocks that match the argument type, if not possible assigns null.
-     */
-    static class SimpleArgumentResolver implements ConstructorArgumentResolver {
-        final Set<Object> objects;
-
-        public SimpleArgumentResolver(Set<Object> objects) {
-            this.objects = objects;
-        }
-
-        @Override
-        public Object[] resolveTypeInstances(Class<?>... argTypes) {
-            List<Object> argumentInstances = new ArrayList<>(argTypes.length);
-            for (Class<?> argType : argTypes) {
-                argumentInstances.add(objectThatIsAssignableFrom(argType));
-            }
-            return argumentInstances.toArray();
-        }
-
-        private Object objectThatIsAssignableFrom(Class<?> argType) {
-            for (Object object : objects) {
-                if (argType.isAssignableFrom(object.getClass())) {
-                    return object;
-                }
-            }
-            return null;
-        }
+    protected ConstructorResolver createConstructorResolver(
+            Class<?> fieldType, Set<Object> mockCandidates) {
+        return new BiggestConstructorResolver(fieldType, mockCandidates);
     }
 }

src/main/java/org/mockito/internal/configuration/injection/LenientPropertyAndSetterInjection.java

@@ -0,0 +1,26 @@
+/*
+ * Copyright (c) 2021 Mockito contributors
+ * This program is made available under the terms of the MIT License.
+ */
+package org.mockito.internal.configuration.injection;
+
+import org.mockito.internal.util.reflection.ConstructorResolver;
+import org.mockito.internal.util.reflection.ConstructorResolver.LenientNoArgsConstructorResolver;
+
+/**
+ * Inject mocks using setters then fields, if no setters available, see
+ * {@link PropertyAndSetterInjection parent class} for more information on algorithm.
+ * <p>
+ * The strategy to instantiate field (if needed) is to try to find no-args constructor on field type
+ * and skip the field otherwise.
+ * </p>
+ *
+ * @see org.mockito.internal.configuration.injection.PropertyAndSetterInjection
+ */
+public class LenientPropertyAndSetterInjection extends PropertyAndSetterInjection {
+
+    @Override
+    protected ConstructorResolver createConstructorResolver(Class<?> fieldType) {
+        return new LenientNoArgsConstructorResolver(fieldType);
+    }
+}

src/main/java/org/mockito/internal/configuration/injection/MockInjection.java

@@ -73,11 +73,21 @@ public OngoingMockInjection tryConstructorInjection() {
             return this;
         }
 
+        public OngoingMockInjection tryStrictConstructorInjection() {
+            injectionStrategies.thenTry(new StrictConstructorInjection());
+            return this;
+        }
+
         public OngoingMockInjection tryPropertyOrFieldInjection() {
             injectionStrategies.thenTry(new PropertyAndSetterInjection());
             return this;
         }
 
+        public OngoingMockInjection tryLenientPropertyOrFieldInjection() {
+            injectionStrategies.thenTry(new LenientPropertyAndSetterInjection());
+            return this;
+        }
+
         public OngoingMockInjection handleSpyAnnotation() {
             postInjectionStrategies.thenTry(new SpyOnInjectedFieldsHandler());
             return this;

src/main/java/org/mockito/internal/configuration/injection/PropertyAndSetterInjection.java

@@ -23,6 +23,8 @@
 import org.mockito.internal.configuration.injection.filter.TerminalMockCandidateFilter;
 import org.mockito.internal.configuration.injection.filter.TypeBasedCandidateFilter;
 import org.mockito.internal.util.collections.ListUtil;
+import org.mockito.internal.util.reflection.ConstructorResolver;
+import org.mockito.internal.util.reflection.ConstructorResolver.NoArgsConstructorResolver;
 import org.mockito.internal.util.reflection.FieldInitializationReport;
 import org.mockito.internal.util.reflection.FieldInitializer;
 
@@ -56,8 +58,8 @@
  * </p>
  *
  * <p>
- * <u>Note:</u> If the field needing injection is not initialized, the strategy tries
- * to create one using a no-arg constructor of the field type.
+ * <u>Note:</u> If the field needing injection is not initialized, the strategy tries to create one
+ * using a no-arg constructor of the field type or fails with an explicit message.
  * </p>
  */
 public class PropertyAndSetterInjection extends MockInjectionStrategy {
@@ -81,6 +83,10 @@ public boolean processInjection(
         FieldInitializationReport report =
                 initializeInjectMocksField(injectMocksField, injectMocksFieldOwner);
 
+        if (!report.fieldIsInitialized()) {
+            return false;
+        }
+
         // for each field in the class hierarchy
         boolean injectionOccurred = false;
         Class<?> fieldClass = report.fieldClass();
@@ -98,7 +104,9 @@ public boolean processInjection(
 
     private FieldInitializationReport initializeInjectMocksField(Field field, Object fieldOwner) {
         try {
-            return new FieldInitializer(fieldOwner, field).initialize();
+            final ConstructorResolver constructorResolver =
+                    createConstructorResolver(field.getType());
+            return new FieldInitializer(fieldOwner, field, constructorResolver).initialize();
         } catch (MockitoException e) {
             if (e.getCause() instanceof InvocationTargetException) {
                 Throwable realCause = e.getCause().getCause();
@@ -108,6 +116,10 @@ private FieldInitializationReport initializeInjectMocksField(Field field, Object
         }
     }
 
+    protected ConstructorResolver createConstructorResolver(Class<?> fieldType) {
+        return new NoArgsConstructorResolver(fieldType);
+    }
+
     private boolean injectMockCandidates(
             Class<?> awaitingInjectionClazz, Object injectee, Set<Object> mocks) {
         boolean injectionOccurred;

src/main/java/org/mockito/internal/configuration/injection/StrictConstructorInjection.java

@@ -0,0 +1,26 @@
+/*
+ * Copyright (c) 2021 Mockito contributors
+ * This program is made available under the terms of the MIT License.
+ */
+package org.mockito.internal.configuration.injection;
+
+import java.util.Set;
+
+import org.mockito.internal.util.reflection.ConstructorResolver;
+import org.mockito.internal.util.reflection.ConstructorResolver.StrictBiggestConstructorResolver;
+
+/**
+ * Injection strategy based on constructor.
+ * <p>
+ * The strategy will search for the constructor with most parameters and try to resolve mocks by
+ * type or skip the field if there is no mocks matching a parameter.
+ * </p>
+ */
+public class StrictConstructorInjection extends ConstructorInjection {
+
+    @Override
+    protected ConstructorResolver createConstructorResolver(
+            Class<?> fieldType, Set<Object> mockCandidates) {
+        return new StrictBiggestConstructorResolver(fieldType, mockCandidates);
+    }
+}

src/main/java/org/mockito/internal/configuration/injection/filter/TerminalMockCandidateFilter.java

@@ -40,7 +40,8 @@ public OngoingInjector filterCandidate(
                         accessor.set(candidateFieldToBeInjected, injectee, matchingMock);
                     }
                 } catch (RuntimeException | IllegalAccessException e) {
-                    throw cannotInjectDependency(candidateFieldToBeInjected, matchingMock, e);
+                    final Throwable details = e.getCause() == null ? e : e.getCause();
+                    throw cannotInjectDependency(candidateFieldToBeInjected, matchingMock, details);
                 }
                 return matchingMock;
             };