Fixes mockito#1516 and mockito#1631

Allows Spies with @Injectmocks to be injected into other @Injectmocks

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

@@ -16,6 +16,37 @@
  */
 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)
                 .withMocks(mocks)

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

@@ -40,15 +40,7 @@ public class InjectingAnnotationEngine implements AnnotationEngine, org.mockito.
      */
     public void process(Class<?> clazz, Object testInstance) {
         processIndependentAnnotations(testInstance.getClass(), testInstance);
-        processInjectMocks(testInstance.getClass(), testInstance);
-    }
-
-    private void processInjectMocks(final Class<?> clazz, final Object testInstance) {
-        Class<?> classContext = clazz;
-        while (classContext != Object.class) {
-            injectMocks(testInstance);
-            classContext = classContext.getSuperclass();
-        }
+        injectMocks(testInstance);
     }
 
     private void processIndependentAnnotations(final Class<?> clazz, final Object testInstance) {
@@ -85,6 +77,12 @@ public void injectMocks(final Object testClassInstance) {
             clazz = clazz.getSuperclass();
         }
 
+        Set<Object> previousMocks;
+        do {
+            previousMocks = mocks;
+            new DefaultInjectionEngine().injectOngoingMocksOnFields(mockDependentFields, mocks, testClassInstance);
+            mocks = new MockScanner(testClassInstance, testClassInstance.getClass()).scanHierarchy();
+        } while (!previousMocks.equals(mocks));
         new DefaultInjectionEngine().injectMocksOnFields(mockDependentFields, mocks, testClassInstance);
     }
 

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

@@ -37,8 +37,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.
@@ -50,7 +51,7 @@ public class SpyAnnotationEngine implements AnnotationEngine, org.mockito.config
     public void process(Class<?> context, Object testInstance) {
         Field[] fields = context.getDeclaredFields();
         for (Field field : fields) {
-            if (field.isAnnotationPresent(Spy.class) && !field.isAnnotationPresent(InjectMocks.class)) {
+            if (shouldProcess(field)) {
                 assertNoIncompatibleAnnotations(Spy.class, field, Mock.class, Captor.class);
                 field.setAccessible(true);
                 Object instance;
@@ -72,6 +73,19 @@ public void process(Class<?> context, Object testInstance) {
         }
     }
 
+    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(),
                             withSettings().spiedInstance(instance)
@@ -116,6 +130,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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2007 Mockito contributors
+ * Copyright (c) 2020 Mockito contributors
  * This program is made available under the terms of the MIT License.
  */
 package org.mockito.internal.configuration.injection;
@@ -8,45 +8,32 @@
 
 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() { }
-
     public boolean processInjection(Field field, Object fieldOwner, Set<Object> mockCandidates) {
         try {
-            SimpleArgumentResolver simpleArgumentResolver = new SimpleArgumentResolver(mockCandidates);
-            FieldInitializationReport report = new FieldInitializer(fieldOwner, field, simpleArgumentResolver).initialize();
+            ConstructorResolver constructorResolver = createConstructorResolver(field.getType(), mockCandidates);
+            FieldInitializationReport report = new FieldInitializer(fieldOwner, field, constructorResolver).initialize();
 
-            return report.fieldWasInitializedUsingContructorArgs();
+            return report.fieldWasInitialized();
         } catch (MockitoException e) {
-            if(e.getCause() instanceof InvocationTargetException) {
+            if (e.getCause() instanceof InvocationTargetException) {
                 Throwable realCause = e.getCause().getCause();
                 throw fieldInitialisationThrewException(field, realCause);
             }
@@ -56,30 +43,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;
-        }
-
-        public Object[] resolveTypeInstances(Class<?>... argTypes) {
-            List<Object> argumentInstances = new ArrayList<Object>(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,27 @@
+/*
+ * Copyright (c) 2019 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

@@ -76,11 +76,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 {
@@ -77,6 +79,10 @@ public boolean isOut(Field object) {
     public boolean processInjection(Field injectMocksField, Object injectMocksFieldOwner, Set<Object> mockCandidates) {
         FieldInitializationReport report = initializeInjectMocksField(injectMocksField, injectMocksFieldOwner);
 
+        if (!report.fieldIsInitialized()) {
+            return false;
+        }
+
         // for each field in the class hierarchy
         boolean injectionOccurred = false;
         Class<?> fieldClass = report.fieldClass();
@@ -90,7 +96,8 @@ public boolean processInjection(Field injectMocksField, Object injectMocksFieldO
 
     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();
@@ -100,6 +107,9 @@ 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) 2020 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

@@ -36,7 +36,8 @@ public Object thenInject() {
                             setField(injectee, candidateFieldToBeInjected,matchingMock);
                         }
                     } catch (RuntimeException e) {
-                        throw cannotInjectDependency(candidateFieldToBeInjected, matchingMock, e);
+                        final Throwable details = e.getCause() == null ? e : e.getCause();
+                        throw cannotInjectDependency(candidateFieldToBeInjected, matchingMock, details);
                     }
                     return matchingMock;
                 }

src/main/java/org/mockito/internal/configuration/injection/scanner/MockScanner.java

@@ -42,16 +42,30 @@ public MockScanner(Object instance, Class<?> clazz) {
      * @param mocks Set of mocks
      */
     public void addPreparedMocks(Set<Object> mocks) {
-        mocks.addAll(scan());
+        scan(clazz, mocks);
+    }
+
+    /**
+     * Scan and prepare mocks for the whole hierarchy of given <code>testClassInstance</code>.
+     *
+     * @return A prepared set of mock
+     */
+    public Set<Object> scanHierarchy() {
+        final Set<Object> mocks = newMockSafeHashSet();
+        Class<?> currentClass = clazz;
+        while (currentClass != Object.class) {
+            scan(currentClass, mocks);
+            currentClass = currentClass.getSuperclass();
+        }
+        return mocks;
     }
 
     /**
      * Scan and prepare mocks for the given <code>testClassInstance</code> and <code>clazz</code> in the type hierarchy.
      *
      * @return A prepared set of mock
      */
-    private Set<Object> scan() {
-        Set<Object> mocks = newMockSafeHashSet();
+    private void scan(Class<?> clazz, Set<Object> mocks) {
         for (Field field : clazz.getDeclaredFields()) {
             // mock or spies only
             FieldReader fieldReader = new FieldReader(instance, field);
@@ -61,7 +75,6 @@ private Set<Object> scan() {
                 mocks.add(mockInstance);
             }
         }
-        return mocks;
     }
 
     private Object preparedMock(Object instance, Field field) {