Merge pull request #1362 from autofac/feature/1360

Fix #1360: Unify the "no constructors found" reporting

Author: tillig

bench/Autofac.BenchmarkProfiling/Program.cs

@@ -84,14 +84,14 @@ static void Main(string[] args)
 
         // Workload method is generated differently when BenchmarkDotNet actually runs; we'll need to wrap it in the set of parameters.
         // It's way slower than they way they do it, but it should still give us good profiler results.
-        Action<int> workloadAction = (repeat) =>
+        void workloadAction(int repeat)
         {
             while (repeat > 0)
             {
                 selectedCase.Descriptor.WorkloadMethod.Invoke(benchInstance, selectedCase.Parameters.Items.Select(x => x.Value).ToArray());
                 repeat--;
             }
-        };
+        }
 
         setupAction.InvokeSingle();
 

src/Autofac/Core/Activators/Reflection/DefaultConstructorFinder.cs

@@ -45,14 +45,7 @@ public ConstructorInfo[] FindConstructors(Type targetType)
 
     private static ConstructorInfo[] GetDefaultPublicConstructors(Type type)
     {
-        var retval = ReflectionCacheSet.Shared.Internal.DefaultPublicConstructors
+        return ReflectionCacheSet.Shared.Internal.DefaultPublicConstructors
             .GetOrAdd(type, t => t.GetDeclaredPublicConstructors());
-
-        if (retval.Length == 0)
-        {
-            throw new NoConstructorsFoundException(type);
-        }
-
-        return retval;
     }
 }

src/Autofac/Core/Activators/Reflection/NoConstructorsFoundException.cs

@@ -13,61 +13,82 @@ public class NoConstructorsFoundException : Exception
     /// <summary>
     /// Initializes a new instance of the <see cref="NoConstructorsFoundException"/> class.
     /// </summary>
-    /// <param name="offendingType">The <see cref="System.Type"/> whose constructor was not found.</param>
-    public NoConstructorsFoundException(Type offendingType)
-        : this(offendingType, FormatMessage(offendingType))
+    /// <param name="offendingType">The <see cref="Type"/> whose constructor was not found.</param>
+    /// <param name="constructorFinder">The <see cref="IConstructorFinder"/> that was used to scan for the constructors.</param>
+    public NoConstructorsFoundException(Type offendingType, IConstructorFinder constructorFinder)
+        : this(offendingType, constructorFinder, FormatMessage(offendingType, constructorFinder))
     {
     }
 
     /// <summary>
     /// Initializes a new instance of the <see cref="NoConstructorsFoundException"/> class.
     /// </summary>
-    /// <param name="offendingType">The <see cref="System.Type"/> whose constructor was not found.</param>
+    /// <param name="offendingType">The <see cref="Type"/> whose constructor was not found.</param>
+    /// <param name="constructorFinder">The <see cref="IConstructorFinder"/> that was used to scan for the constructors.</param>
     /// <param name="message">Exception message.</param>
-    public NoConstructorsFoundException(Type offendingType, string message)
+    public NoConstructorsFoundException(Type offendingType, IConstructorFinder constructorFinder, string message)
         : base(message)
     {
         OffendingType = offendingType ?? throw new ArgumentNullException(nameof(offendingType));
+        ConstructorFinder = constructorFinder ?? throw new ArgumentNullException(nameof(constructorFinder));
     }
 
     /// <summary>
     /// Initializes a new instance of the <see cref="NoConstructorsFoundException"/> class.
     /// </summary>
-    /// <param name="offendingType">The <see cref="System.Type"/> whose constructor was not found.</param>
+    /// <param name="offendingType">The <see cref="Type"/> whose constructor was not found.</param>
+    /// <param name="constructorFinder">The <see cref="IConstructorFinder"/> that was used to scan for the constructors.</param>
     /// <param name="innerException">The inner exception.</param>
-    public NoConstructorsFoundException(Type offendingType, Exception innerException)
-        : this(offendingType, FormatMessage(offendingType), innerException)
+    public NoConstructorsFoundException(Type offendingType, IConstructorFinder constructorFinder, Exception innerException)
+        : this(offendingType, constructorFinder, FormatMessage(offendingType, constructorFinder), innerException)
     {
     }
 
     /// <summary>
     /// Initializes a new instance of the <see cref="NoConstructorsFoundException"/> class.
     /// </summary>
-    /// <param name="offendingType">The <see cref="System.Type"/> whose constructor was not found.</param>
+    /// <param name="offendingType">The <see cref="Type"/> whose constructor was not found.</param>
+    /// <param name="constructorFinder">The <see cref="IConstructorFinder"/> that was used to scan for the constructors.</param>
     /// <param name="message">Exception message.</param>
     /// <param name="innerException">The inner exception.</param>
-    public NoConstructorsFoundException(Type offendingType, string message, Exception innerException)
+    public NoConstructorsFoundException(Type offendingType, IConstructorFinder constructorFinder, string message, Exception innerException)
         : base(message, innerException)
     {
         OffendingType = offendingType ?? throw new ArgumentNullException(nameof(offendingType));
+        ConstructorFinder = constructorFinder ?? throw new ArgumentNullException(nameof(constructorFinder));
     }
 
+    /// <summary>
+    /// Gets the finder used when locating constructors.
+    /// </summary>
+    /// <value>
+    /// An <see cref="IConstructorFinder"/> that was used when scanning the
+    /// <see cref="OffendingType"/> to find constructors.
+    /// </value>
+    public IConstructorFinder ConstructorFinder { get; private set; }
+
     /// <summary>
     /// Gets the type without found constructors.
     /// </summary>
     /// <value>
-    /// A <see cref="System.Type"/> that was processed by an <see cref="IConstructorFinder"/>
-    /// or similar mechanism and was determined to have no available constructors.
+    /// A <see cref="Type"/> that was processed by the
+    /// <see cref="ConstructorFinder"/> and was determined to have no available
+    /// constructors.
     /// </value>
     public Type OffendingType { get; private set; }
 
-    private static string FormatMessage(Type offendingType)
+    private static string FormatMessage(Type offendingType, IConstructorFinder constructorFinder)
     {
         if (offendingType == null)
         {
             throw new ArgumentNullException(nameof(offendingType));
         }
 
-        return string.Format(CultureInfo.CurrentCulture, NoConstructorsFoundExceptionResources.Message, offendingType.FullName);
+        if (constructorFinder == null)
+        {
+            throw new ArgumentNullException(nameof(constructorFinder));
+        }
+
+        return string.Format(CultureInfo.CurrentCulture, NoConstructorsFoundExceptionResources.Message, offendingType.FullName, constructorFinder.GetType().FullName);
     }
 }

src/Autofac/Core/Activators/Reflection/NoConstructorsFoundExceptionResources.resx

@@ -1,17 +1,17 @@
 <?xml version="1.0" encoding="utf-8"?>
 <root>
-  <!-- 
-    Microsoft ResX Schema 
-    
+  <!--
+    Microsoft ResX Schema
+
     Version 2.0
-    
-    The primary goals of this format is to allow a simple XML format 
-    that is mostly human readable. The generation and parsing of the 
-    various data types are done through the TypeConverter classes 
+
+    The primary goals of this format is to allow a simple XML format
+    that is mostly human readable. The generation and parsing of the
+    various data types are done through the TypeConverter classes
     associated with the data types.
-    
+
     Example:
-    
+
     ... ado.net/XML headers & schema ...
     <resheader name="resmimetype">text/microsoft-resx</resheader>
     <resheader name="version">2.0</resheader>
@@ -26,36 +26,36 @@
         <value>[base64 mime encoded string representing a byte array form of the .NET Framework object]</value>
         <comment>This is a comment</comment>
     </data>
-                
-    There are any number of "resheader" rows that contain simple 
+
+    There are any number of "resheader" rows that contain simple
     name/value pairs.
-    
-    Each data row contains a name, and value. The row also contains a 
-    type or mimetype. Type corresponds to a .NET class that support 
-    text/value conversion through the TypeConverter architecture. 
-    Classes that don't support this are serialized and stored with the 
+
+    Each data row contains a name, and value. The row also contains a
+    type or mimetype. Type corresponds to a .NET class that support
+    text/value conversion through the TypeConverter architecture.
+    Classes that don't support this are serialized and stored with the
     mimetype set.
-    
-    The mimetype is used for serialized objects, and tells the 
-    ResXResourceReader how to depersist the object. This is currently not 
+
+    The mimetype is used for serialized objects, and tells the
+    ResXResourceReader how to depersist the object. This is currently not
     extensible. For a given mimetype the value must be set accordingly:
-    
-    Note - application/x-microsoft.net.object.binary.base64 is the format 
-    that the ResXResourceWriter will generate, however the reader can 
+
+    Note - application/x-microsoft.net.object.binary.base64 is the format
+    that the ResXResourceWriter will generate, however the reader can
     read any of the formats listed below.
-    
+
     mimetype: application/x-microsoft.net.object.binary.base64
-    value   : The object must be serialized with 
+    value   : The object must be serialized with
             : System.Runtime.Serialization.Formatters.Binary.BinaryFormatter
             : and then encoded with base64 encoding.
-    
+
     mimetype: application/x-microsoft.net.object.soap.base64
-    value   : The object must be serialized with 
+    value   : The object must be serialized with
             : System.Runtime.Serialization.Formatters.Soap.SoapFormatter
             : and then encoded with base64 encoding.
 
     mimetype: application/x-microsoft.net.object.bytearray.base64
-    value   : The object must be serialized into a byte array 
+    value   : The object must be serialized into a byte array
             : using a System.ComponentModel.TypeConverter
             : and then encoded with base64 encoding.
     -->
@@ -118,6 +118,6 @@
     <value>System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</value>
   </resheader>
   <data name="Message" xml:space="preserve">
-    <value>No accessible constructors were found for the type '{0}'.</value>
+    <value>No constructors on type '{0}' can be found with the constructor finder '{1}'.</value>
   </data>
-</root>
+</root>

src/Autofac/Core/Activators/Reflection/ReflectionActivator.cs

@@ -83,7 +83,7 @@ public void ConfigurePipeline(IComponentRegistryServices componentRegistryServic
 
         if (availableConstructors.Length == 0)
         {
-            throw new NoConstructorsFoundException(_implementationType, string.Format(CultureInfo.CurrentCulture, ReflectionActivatorResources.NoConstructorsAvailable, _implementationType, ConstructorFinder));
+            throw new NoConstructorsFoundException(_implementationType, ConstructorFinder);
         }
 
         var binders = new ConstructorBinder[availableConstructors.Length];
@@ -131,7 +131,7 @@ private void UseSingleConstructorActivation(IResolvePipelineBuilder pipelineBuil
                 // This is not going to happen, because there is only 1 constructor, that constructor has no parameters,
                 // so there are no conditions under which GetConstructorInvoker will return null in this path.
                 // Throw an error here just in case (and to satisfy nullability checks).
-                throw new NoConstructorsFoundException(_implementationType, string.Format(CultureInfo.CurrentCulture, ReflectionActivatorResources.NoConstructorsAvailable, _implementationType, ConstructorFinder));
+                throw new NoConstructorsFoundException(_implementationType, ConstructorFinder);
             }
 
             // If there are no arguments to the constructor, bypass all argument binding and pre-bind the constructor.

src/Autofac/Core/Activators/Reflection/ReflectionActivatorResources.resx

@@ -1,17 +1,17 @@
 <?xml version="1.0" encoding="utf-8"?>
 <root>
-  <!-- 
-    Microsoft ResX Schema 
-    
+  <!--
+    Microsoft ResX Schema
+
     Version 2.0
-    
-    The primary goals of this format is to allow a simple XML format 
-    that is mostly human readable. The generation and parsing of the 
-    various data types are done through the TypeConverter classes 
+
+    The primary goals of this format is to allow a simple XML format
+    that is mostly human readable. The generation and parsing of the
+    various data types are done through the TypeConverter classes
     associated with the data types.
-    
+
     Example:
-    
+
     ... ado.net/XML headers & schema ...
     <resheader name="resmimetype">text/microsoft-resx</resheader>
     <resheader name="version">2.0</resheader>
@@ -26,36 +26,36 @@
         <value>[base64 mime encoded string representing a byte array form of the .NET Framework object]</value>
         <comment>This is a comment</comment>
     </data>
-                
-    There are any number of "resheader" rows that contain simple 
+
+    There are any number of "resheader" rows that contain simple
     name/value pairs.
-    
-    Each data row contains a name, and value. The row also contains a 
-    type or mimetype. Type corresponds to a .NET class that support 
-    text/value conversion through the TypeConverter architecture. 
-    Classes that don't support this are serialized and stored with the 
+
+    Each data row contains a name, and value. The row also contains a
+    type or mimetype. Type corresponds to a .NET class that support
+    text/value conversion through the TypeConverter architecture.
+    Classes that don't support this are serialized and stored with the
     mimetype set.
-    
-    The mimetype is used for serialized objects, and tells the 
-    ResXResourceReader how to depersist the object. This is currently not 
+
+    The mimetype is used for serialized objects, and tells the
+    ResXResourceReader how to depersist the object. This is currently not
     extensible. For a given mimetype the value must be set accordingly:
-    
-    Note - application/x-microsoft.net.object.binary.base64 is the format 
-    that the ResXResourceWriter will generate, however the reader can 
+
+    Note - application/x-microsoft.net.object.binary.base64 is the format
+    that the ResXResourceWriter will generate, however the reader can
     read any of the formats listed below.
-    
+
     mimetype: application/x-microsoft.net.object.binary.base64
-    value   : The object must be serialized with 
+    value   : The object must be serialized with
             : System.Runtime.Serialization.Formatters.Binary.BinaryFormatter
             : and then encoded with base64 encoding.
-    
+
     mimetype: application/x-microsoft.net.object.soap.base64
-    value   : The object must be serialized with 
+    value   : The object must be serialized with
             : System.Runtime.Serialization.Formatters.Soap.SoapFormatter
             : and then encoded with base64 encoding.
 
     mimetype: application/x-microsoft.net.object.bytearray.base64
-    value   : The object must be serialized into a byte array 
+    value   : The object must be serialized into a byte array
             : using a System.ComponentModel.TypeConverter
             : and then encoded with base64 encoding.
     -->
@@ -120,9 +120,6 @@
   <data name="ConstructorSelectorCannotSelectAnInvalidBinding" xml:space="preserve">
     <value>The constructor selector provided by '{0}' selected a binding that cannot be instantiated. Selectors must return a constructor where 'CanInstantiate' is true.</value>
   </data>
-  <data name="NoConstructorsAvailable" xml:space="preserve">
-    <value>No constructors on type '{0}' can be found with the constructor finder '{1}'.</value>
-  </data>
   <data name="NoConstructorsBindable" xml:space="preserve">
     <value>None of the constructors found with '{0}' on type '{1}' can be invoked with the available services and parameters:{2}</value>
   </data>

src/Autofac/Features/OpenGenerics/OpenGenericDecoratorRegistrationSource.cs

@@ -64,9 +64,13 @@ public IEnumerable<IComponentRegistration> RegistrationsFor(Service service, Fun
             throw new ArgumentNullException(nameof(registrationAccessor));
         }
 
-        if (OpenGenericServiceBinder.TryBindOpenGenericService(service, _registrationData.Services, _activatorData.ImplementationType, out Type? constructedImplementationType, out Service[]? services))
+        if (service is not IServiceWithType swt)
+        {
+            return Enumerable.Empty<IComponentRegistration>();
+        }
+
+        if (OpenGenericServiceBinder.TryBindOpenGenericTypedService(swt, _registrationData.Services, _activatorData.ImplementationType, out Type? constructedImplementationType, out Service[]? services))
         {
-            var swt = (IServiceWithType)service;
             var fromService = _activatorData.FromService.ChangeType(swt.ServiceType);
 
             return registrationAccessor(fromService)

src/Autofac/Features/OpenGenerics/OpenGenericDelegateRegistrationSource.cs

@@ -47,7 +47,12 @@ public IEnumerable<IComponentRegistration> RegistrationsFor(Service service, Fun
             throw new ArgumentNullException(nameof(registrationAccessor));
         }
 
-        if (OpenGenericServiceBinder.TryBindOpenGenericDelegate(service, _registrationData.Services, _activatorData.Factory, out var constructedFactory, out Service[]? services))
+        if (service is not IServiceWithType swt)
+        {
+            yield break;
+        }
+
+        if (OpenGenericServiceBinder.TryBindOpenGenericDelegateService(swt, _registrationData.Services, _activatorData.Factory, out var constructedFactory, out Service[]? services))
         {
             // Pass the pipeline builder from the original registration to the 'CreateRegistration'.
             // So the original registration will contain all of the pipeline stages originally added, plus anything we want to add.

src/Autofac/Features/OpenGenerics/OpenGenericRegistrationSource.cs

@@ -59,7 +59,12 @@ public IEnumerable<IComponentRegistration> RegistrationsFor(Service service, Fun
             throw new ArgumentNullException(nameof(registrationAccessor));
         }
 
-        if (OpenGenericServiceBinder.TryBindOpenGenericService(service, _registrationData.Services, _activatorData.ImplementationType, out Type? constructedImplementationType, out Service[]? services))
+        if (service is not IServiceWithType swt)
+        {
+            yield break;
+        }
+
+        if (OpenGenericServiceBinder.TryBindOpenGenericTypedService(swt, _registrationData.Services, _activatorData.ImplementationType, out Type? constructedImplementationType, out Service[]? services))
         {
             // Pass the pipeline builder from the original registration to the 'CreateRegistration'.
             // So the original registration will contain all of the pipeline stages originally added, plus anything we want to add.