Allow transitive dependencies (#86)

* Allow transitive dependencies

Moq allows passing constructor args to mock instances via
`new Mock<Foo>(arg1, arg2, ...)`. This pull request enables doing this
for automocker, enhancing the "auto" part of "automocker".

Why do this? Because forcing every class to have an interface seems
unreasonable. Feel free to subscribe to whatever philosophy you prefer,
but the interface-to-class pairing does generate a lot of boilerplate.
Just as Moq allows as many scenarios as possible, Moq.Contrib should
also.

* Suggested change to clean up CA warnings (#87)

* Cleanup expression

* Handle recursive object graph (error)

* Apply suggestions from code review

Co-authored-by: Kevin B <Keboo@users.noreply.github.com>

* Remaining PR feedback.
Standardize test objects as public rather than suppressing errors

Co-authored-by: Kevin B <Keboo@users.noreply.github.com>
Co-authored-by: Adam Hewitt <adamhewitt627@users.noreply.github.com>
Co-authored-by: Adam Hewitt <adamhewitt@outlook.com>

Moq.AutoMock.Tests/DescribeCreateInstance.cs

@@ -95,5 +95,58 @@ public void It_throws_original_exception_caught_whilst_creating_object_with_orig
             ArgumentException exception = Assert.ThrowsException<ArgumentException>(() => mocker.CreateInstance<ConstructorThrows>());
             StringAssert.Contains(exception.StackTrace, typeof(ConstructorThrows).Name);
         }
+
+        [TestMethod]
+        public void It_creates_object_when_first_level_dependencies_are_classes()
+        {
+            var mocker = new AutoMocker();
+            HasClassDependency instance = mocker.CreateInstance<HasClassDependency>();
+            var dependency = instance.WithService;
+            Assert.IsNotNull(dependency);
+            Assert.IsInstanceOfType(dependency, typeof(WithService));
+            Assert.IsInstanceOfType(Mock.Get(dependency), typeof(Mock<WithService>));
+            Assert.AreSame(dependency, mocker.Get<WithService>());
+        }
+
+        [TestMethod]
+        public void It_creates_object_with_2_first_level_dependencies()
+        {
+            var mocker = new AutoMocker();
+            var instance = mocker.CreateInstance<With2ClassDependencies>();
+
+            var dependency1 = instance.WithService;
+            Assert.IsNotNull(dependency1);
+            Assert.IsInstanceOfType(dependency1, typeof(WithService));
+            Assert.IsInstanceOfType(Mock.Get(dependency1), typeof(Mock<WithService>));
+            Assert.AreSame(dependency1, mocker.Get<WithService>());
+
+            var dependency2 = instance.With3Parameters;
+            Assert.IsNotNull(dependency2);
+            Assert.IsInstanceOfType(dependency2, typeof(With3Parameters));
+            Assert.IsInstanceOfType(Mock.Get(dependency2), typeof(Mock<With3Parameters>));
+        }
+
+        [TestMethod]
+        public void Second_level_dependencies_act_same_as_if_they_were_target()
+        {
+            var mocker = new AutoMocker();
+            var instance = mocker.CreateInstance<HasFuncDependencies>();
+            var dependency = instance.WithServiceFactory();
+            Assert.IsNotNull(dependency);
+            Assert.IsInstanceOfType(dependency, typeof(WithService));
+            Assert.IsInstanceOfType(Mock.Get(dependency), typeof(Mock<WithService>));
+            // Questionable if this is the correct behavior, but it is the current behavior.
+            Assert.AreSame(dependency, mocker.Get<WithService>());
+        }
+
+        [TestMethod]
+        public void It_throws_when_creating_object_with_recursive_dependency()
+        {
+            var mocker = new AutoMocker();
+            // I could see this changing to something else in the future, like null. Right now, it seems
+            // best to cause early failure to clarify what went wrong. Also, returning null "allows" the
+            // behavior, so it's easier to move that direction later without breaking backward compatibility.
+            Assert.ThrowsException<InvalidOperationException>(mocker.CreateInstance<WithRecursiveDependency>);
+        }
     }
 }

Moq.AutoMock.Tests/Util/HasClassDependency.cs

@@ -0,0 +1,12 @@
+namespace Moq.AutoMock.Tests.Util
+{
+    public class HasClassDependency
+    {
+        public WithService WithService { get; }
+
+        public HasClassDependency(WithService withService)
+        {
+            WithService = withService;
+        }
+    }
+}

Moq.AutoMock.Tests/Util/HasFuncDependencies.cs

@@ -0,0 +1,14 @@
+using System;
+
+namespace Moq.AutoMock.Tests.Util
+{
+    public class HasFuncDependencies
+    {
+        public Func<WithService> WithServiceFactory { get; }
+
+        public HasFuncDependencies(Func<WithService> withServiceFactory)
+        {
+            WithServiceFactory = withServiceFactory;
+        }
+    }
+}

Moq.AutoMock.Tests/Util/With2ClassDependencies.cs

@@ -0,0 +1,14 @@
+namespace Moq.AutoMock.Tests.Util
+{
+    public class With2ClassDependencies
+    {
+        public WithService WithService { get; }
+        public With3Parameters With3Parameters { get; }
+
+        public With2ClassDependencies(WithService withService, With3Parameters with3Parameters)
+        {
+            WithService = withService;
+            With3Parameters = with3Parameters;
+        }
+    }
+}

Moq.AutoMock.Tests/Util/With3Parameters.cs

@@ -1,13 +1,16 @@
 namespace Moq.AutoMock.Tests.Util
 {
-#pragma warning disable CA1801, CA1812  //is an internal class that is apparently never instantiated
-    internal class With3Parameters
+    public class With3Parameters
     {
         // ReSharper disable UnusedMember.Global
         // ReSharper disable UnusedParameter.Local
         public With3Parameters() { }
+
+#pragma warning disable CA1801  //Parameter  is never used. Remove the parameter or use it in the method body
         public With3Parameters(IService1 service1) { }
         public With3Parameters(IService1 service1, IService2 service2) { }
+#pragma warning restore CA1801
+
         // ReSharper restore UnusedParameter.Local
         // ReSharper restore UnusedMember.Global
     }

Moq.AutoMock.Tests/Util/WithArrayParameter.cs

@@ -1,13 +1,16 @@
 namespace Moq.AutoMock.Tests.Util
 {
-#pragma warning disable CA1801, CA1812  //is an internal class that is apparently never instantiated
-    internal class WithArrayParameter
+    public class WithArrayParameter
     {
         // ReSharper disable UnusedMember.Global
         // ReSharper disable UnusedParameter.Local
         public WithArrayParameter() { }
+
+#pragma warning disable CA1801  //Parameter  is never used. Remove the parameter or use it in the method body
         public WithArrayParameter(string[] array) { }
         public WithArrayParameter(string[] array, string @sealed) { }
+#pragma warning restore CA1801
+
         // ReSharper restore UnusedParameter.Local
         // ReSharper restore UnusedMember.Global
     }

Moq.AutoMock.Tests/Util/WithDefaultAndSingleParameter.cs

@@ -1,12 +1,15 @@
 namespace Moq.AutoMock.Tests.Util
 {
-#pragma warning disable CA1801, CA1812  //is an internal class that is apparently never instantiated
-    internal class WithDefaultAndSingleParameter
+    public class WithDefaultAndSingleParameter
     {
         // ReSharper disable UnusedMember.Global
         // ReSharper disable UnusedParameter.Local
         public WithDefaultAndSingleParameter() { }
+
+#pragma warning disable CA1801  //Parameter  is never used. Remove the parameter or use it in the method body
         public WithDefaultAndSingleParameter(IService1 service1) { }
+#pragma warning restore CA1801
+
         // ReSharper restore UnusedParameter.Local
         // ReSharper restore UnusedMember.Global
     }

Moq.AutoMock.Tests/Util/WithPrivateConstructor.cs

@@ -1,13 +1,16 @@
 namespace Moq.AutoMock.Tests.Util
 {
-#pragma warning disable CA1801, CA1812  //is an internal class that is apparently never instantiated
-    internal class WithPrivateConstructor
+    public class WithPrivateConstructor
     {
         // ReSharper disable UnusedMember.Global
         // ReSharper disable UnusedParameter.Local
         // ReSharper disable UnusedMember.Local
+
+#pragma warning disable CA1801  //Parameter  is never used. Remove the parameter or use it in the method body
         public WithPrivateConstructor(IService1 service1) { }
         private WithPrivateConstructor(IService1 service1, IService2 service2) { }
+#pragma warning restore CA1801
+
         // ReSharper restore UnusedMember.Local
         // ReSharper restore UnusedParameter.Local
         // ReSharper restore UnusedMember.Global

Moq.AutoMock.Tests/Util/WithRecursiveDependency.cs

@@ -0,0 +1,12 @@
+namespace Moq.AutoMock.Tests.Util
+{
+    public class WithRecursiveDependency
+    {
+        public WithRecursiveDependency Child { get; }
+
+        public WithRecursiveDependency(WithRecursiveDependency child)
+        {
+            Child = child;
+        }
+    }
+}

Moq.AutoMock.Tests/Util/WithSealedParameter.cs

@@ -1,12 +1,15 @@
 namespace Moq.AutoMock.Tests.Util
 {
-#pragma warning disable CA1801, CA1812  //is an internal class that is apparently never instantiated
-    internal class WithSealedParameter
+    public class WithSealedParameter
     {
         // ReSharper disable UnusedMember.Global
         // ReSharper disable UnusedParameter.Local
         public WithSealedParameter() { }
+
+#pragma warning disable CA1801  //Parameter  is never used. Remove the parameter or use it in the method body
         public WithSealedParameter(string @sealed) { }
+#pragma warning restore CA1801  //Parameter  is never used. Remove the parameter or use it in the method body
+
         // ReSharper restore UnusedParameter.Local
         // ReSharper restore UnusedMember.Global
     }

Moq.AutoMock/AutoMocker.cs

@@ -87,7 +87,7 @@ public AutoMocker(MockBehavior mockBehavior, DefaultValue defaultValue, bool cal
         /// </summary>
         public ICollection<IMockResolver> Resolvers { get; }
 
-        private IInstance Resolve(Type serviceType)
+        private IInstance Resolve(Type serviceType, ObjectGraphContext resolutionContext)
         {
             if (serviceType.IsArray)
             {
@@ -98,7 +98,7 @@ private IInstance Resolve(Type serviceType)
                 return instance;
             }
 
-            object? resolved = Resolve(serviceType, null);
+            object? resolved = Resolve(serviceType, null, resolutionContext);
             return resolved switch
             {
                 Mock mock => new MockInstance(mock),
@@ -107,9 +107,19 @@ private IInstance Resolve(Type serviceType)
             };
         }
 
-        private object? Resolve(Type serviceType, object? initialValue)
+        private object? Resolve(Type serviceType, object? initialValue, ObjectGraphContext resolutionContext)
         {
-            var context = new MockResolutionContext(this, serviceType, initialValue);
+            if (resolutionContext.VisitedTypes.Contains(serviceType))
+            {
+                var message = string.Join(Environment.NewLine,
+                     $"Class could not be constructed because it appears to be used recursively: '{serviceType}'",
+                     string.Join($"{Environment.NewLine} --> ", resolutionContext.VisitedTypes)
+                );
+                throw new InvalidOperationException(message);
+            }
+
+            resolutionContext.VisitedTypes.Add(serviceType);
+            var context = new MockResolutionContext(this, serviceType, initialValue, resolutionContext);
 
             foreach (var r in Resolvers)
                 r.Resolve(context);
@@ -167,11 +177,11 @@ public object CreateInstance(Type type, bool enablePrivate)
         {
             if (type is null) throw new ArgumentNullException(nameof(type));
 
-            BindingFlags bindingFlags = GetBindingFlags(enablePrivate);
-            object?[] arguments = CreateArguments(type, bindingFlags);
+            var context = new ObjectGraphContext(enablePrivate);
+            object?[] arguments = CreateArguments(type, context);
             try
             {
-                var ctor = type.SelectCtor(_typeMap.Keys.ToArray(), bindingFlags);
+                var ctor = type.SelectCtor(_typeMap.Keys.ToArray(), context.BindingFlags);
                 return ctor.Invoke(arguments);
             }
             catch (TargetInvocationException e)
@@ -204,15 +214,16 @@ public object CreateInstance(Type type, bool enablePrivate)
         /// <returns>An instance with virtual and abstract members mocked</returns>
         public T CreateSelfMock<T>(bool enablePrivate) where T : class?
         {
-            var arguments = CreateArguments(typeof(T), GetBindingFlags(enablePrivate));
+            var context = new ObjectGraphContext(enablePrivate);
+            var arguments = CreateArguments(typeof(T), context);
 
             var mock = new Mock<T>(MockBehavior, arguments)
             {
                 DefaultValue = DefaultValue,
                 CallBase = CallBase
             };
 
-            var resolved = Resolve(typeof(T), mock);
+            var resolved = Resolve(typeof(T), mock, context);
             if (resolved is Mock<T> m)
                 return m.Object;
 
@@ -229,7 +240,7 @@ public object CreateInstance(Type type, bool enablePrivate)
         /// <typeparam name="TService">The type that the instance will be registered as</typeparam>
         /// <param name="service"></param>
         public void Use<TService>([DisallowNull] TService service)
-            => Use(typeof(TService), service);
+            => Use(typeof(TService), service ?? throw new ArgumentNullException(nameof(service)));
 
         /// <summary>
         /// Adds an instance to the container.
@@ -293,11 +304,16 @@ public void Use<TService>(Expression<Func<TService, bool>> setup)
         /// <param name="serviceType">The type of service to retrieve</param>
         /// <returns></returns>
         public object Get(Type serviceType)
+        {
+            return Get(serviceType, new ObjectGraphContext(false));
+        }
+
+        private object Get(Type serviceType, ObjectGraphContext context)
         {
             if (serviceType is null) throw new ArgumentNullException(nameof(serviceType));
 
             if (!_typeMap.TryGetValue(serviceType, out var instance) || instance is null)
-                instance = _typeMap[serviceType] = Resolve(serviceType);
+                instance = _typeMap[serviceType] = Resolve(serviceType, context);
 
             if (instance is null)
                 throw new ArgumentException($"{serviceType} could not resolve to an object.", nameof(serviceType));
@@ -332,7 +348,7 @@ public Mock GetMock(Type serviceType)
         private Mock GetMockImplementation(Type serviceType)
         {
             if (!_typeMap.TryGetValue(serviceType, out var instance) || instance is null)
-                instance = _typeMap[serviceType] = Resolve(serviceType);
+                instance = _typeMap[serviceType] = Resolve(serviceType, new ObjectGraphContext(false));
 
             if (instance == null || !instance.IsMock)
                 throw new ArgumentException($"Registered service `{Get(serviceType)?.GetType()}` was not a mock");
@@ -418,7 +434,7 @@ public void Combine(Type type, params Type[] forwardTo)
         {
             if (type is null) throw new ArgumentNullException(nameof(type));
 
-            if (!(Resolve(type) is MockInstance mockObject))
+            if (!(Resolve(type, new ObjectGraphContext(false)) is MockInstance mockObject))
                 throw new ArgumentException($"{type} did not resolve to a Mock", nameof(type));
 
             forwardTo.Aggregate(mockObject.Mock, As);
@@ -548,26 +564,19 @@ public void Verify()
 
         #region Utilities
 
-        private static BindingFlags GetBindingFlags(bool enablePrivate)
-        {
-            var bindingFlags = BindingFlags.Instance | BindingFlags.Public;
-            if (enablePrivate) bindingFlags |= BindingFlags.NonPublic;
-            return bindingFlags;
-        }
-
-        private object?[] CreateArguments(Type type, BindingFlags bindingFlags)
+        internal object?[] CreateArguments(Type type, ObjectGraphContext context)
         {
-            ConstructorInfo ctor = type.SelectCtor(_typeMap.Keys.ToArray(), bindingFlags);
+            ConstructorInfo ctor = type.SelectCtor(_typeMap.Keys.ToArray(), context.BindingFlags);
             if (ctor is null)
                 throw new ArgumentException($"`{type}` does not have an acceptable constructor.", nameof(type));
 
-            return ctor.GetParameters().Select(x => Get(x.ParameterType)).ToArray();
+            return ctor.GetParameters().Select(x => Get(x.ParameterType, context)).ToArray();
         }
 
         private Mock GetOrMakeMockFor(Type type)
         {
             if (!_typeMap.TryGetValue(type, out var instance) || !instance.IsMock)
-                instance = Resolve(type);
+                instance = Resolve(type, new ObjectGraphContext(false));
 
             if (!(instance is MockInstance mockInstance))
                 throw new ArgumentException($"{type} does not resolve to a Mock");