Merge pull request #1389 from autofac/feature/registertypes-filtering

Re-enable RegsiterTypes filtering; better message for RegisterType issues

.vscode/settings.json

@@ -12,6 +12,7 @@
     "notnull",
     "paramref",
     "startable",
+    "subclassing",
     "typeparam",
     "xunit"
   ],

bench/Autofac.Benchmarks/LambdaResolveBenchmark.cs

@@ -1,6 +1,4 @@
-using Autofac.Core;
-
-namespace Autofac.Benchmarks;
+namespace Autofac.Benchmarks;
 
 public class LambdaResolveBenchmark
 {
@@ -13,8 +11,8 @@ public void Setup()
 
         builder.RegisterType<MyDependency1>();
 
-        // The original componentcontext way.
-        builder.Register((ctxt) => new MyComponent(ctxt.Resolve<MyDependency1>(), "arg"))
+        // The original component context way.
+        builder.Register((context) => new MyComponent(context.Resolve<MyDependency1>(), "arg"))
                .As<IServiceExistingMethodCapturedArg>();
 
         // Capture the argument directly.

codegen/Autofac.CodeGen/DelegateRegisterGenerator.cs

@@ -29,11 +29,11 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
         IncrementalValuesProvider<INamedTypeSymbol> classDeclarations = context.SyntaxProvider
             .CreateSyntaxProvider(
                 predicate: static (s, _) => s is ClassDeclarationSyntax classSyn && classSyn.Modifiers.Any(static m => m.IsKind(SyntaxKind.PartialKeyword)),
-                transform: static (ctxt, cancelToken) =>
+                transform: static (context, cancelToken) =>
                 {
-                    var syntax = (ClassDeclarationSyntax)ctxt.Node;
+                    var syntax = (ClassDeclarationSyntax)context.Node;
 
-                    if (ctxt.SemanticModel.GetDeclaredSymbol(syntax, cancelToken) is INamedTypeSymbol symbol)
+                    if (context.SemanticModel.GetDeclaredSymbol(syntax, cancelToken) is INamedTypeSymbol symbol)
                     {
                         // Looking for the exact name of the class.
                         if (symbol.ToDisplayString() == "Autofac.RegistrationExtensions")

src/Autofac/Builder/ContainerBuildOptions.cs

@@ -4,7 +4,7 @@
 namespace Autofac.Builder;
 
 /// <summary>
-/// Parameterises the construction of a container by a <see cref="ContainerBuilder"/>.
+/// Behaviors for the construction of a container by a <see cref="ContainerBuilder"/>.
 /// </summary>
 [Flags]
 public enum ContainerBuildOptions

src/Autofac/Builder/RegistrationBuilder.cs

@@ -6,6 +6,7 @@
 using Autofac.Core.Activators.Delegate;
 using Autofac.Core.Registration;
 using Autofac.Core.Resolving.Pipeline;
+using Autofac.Util;
 
 namespace Autofac.Builder;
 
@@ -61,6 +62,12 @@ public static IRegistrationBuilder<object, SimpleActivatorData, SingleRegistrati
     public static IRegistrationBuilder<TImplementer, ConcreteReflectionActivatorData, SingleRegistrationStyle> ForType<TImplementer>()
         where TImplementer : notnull
     {
+        // Open generics can't be generic type parameters so we don't have to check for that here.
+        if (!typeof(TImplementer).MayAllowReflectionActivation(allowCompilerGenerated: true))
+        {
+            throw new ArgumentException(string.Format(CultureInfo.CurrentCulture, RegistrationBuilderResources.OnlyRegisterableTypesAllowed, typeof(TImplementer)));
+        }
+
         return new RegistrationBuilder<TImplementer, ConcreteReflectionActivatorData, SingleRegistrationStyle>(
             new TypedService(typeof(TImplementer)),
             new ConcreteReflectionActivatorData(typeof(TImplementer)),
@@ -74,6 +81,16 @@ public static IRegistrationBuilder<TImplementer, ConcreteReflectionActivatorData
     /// <returns>A registration builder.</returns>
     public static IRegistrationBuilder<object, ConcreteReflectionActivatorData, SingleRegistrationStyle> ForType(Type implementationType)
     {
+        if (implementationType is null)
+        {
+            throw new ArgumentNullException(nameof(implementationType));
+        }
+
+        if (!implementationType.MayAllowReflectionActivation(allowCompilerGenerated: true) || implementationType.IsGenericTypeDefinition)
+        {
+            throw new ArgumentException(string.Format(CultureInfo.CurrentCulture, RegistrationBuilderResources.OnlyRegisterableTypesAllowed, implementationType));
+        }
+
         return new RegistrationBuilder<object, ConcreteReflectionActivatorData, SingleRegistrationStyle>(
             new TypedService(implementationType),
             new ConcreteReflectionActivatorData(implementationType),

src/Autofac/Builder/RegistrationBuilderResources.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,4 +120,7 @@
   <data name="ComponentDoesNotSupportService" xml:space="preserve">
     <value>The type '{0}' is not assignable to service '{1}'.</value>
   </data>
-</root>
+  <data name="OnlyRegisterableTypesAllowed" xml:space="preserve">
+    <value>The type '{0}' is not concrete. Only concrete types (no interfaces, abstract classes, open generics, etc.) can be registered by type.</value>
+  </data>
+</root>

src/Autofac/Builder/RegistrationBuilder{TLimit,TActivatorData,TRegistrationStyle}.cs

@@ -276,7 +276,7 @@ public IRegistrationBuilder<TLimit, TActivatorData, TRegistrationStyle> As(param
         for (int i = 0; i < services.Length; i++)
         {
             var service = services[i];
-            if (service.FullName != null)
+            if (service.FullName is not null)
             {
                 argArray[i] = new TypedService(service);
             }
@@ -413,16 +413,16 @@ public IRegistrationBuilder<TLimit, TActivatorData, TRegistrationStyle> OnPrepar
             throw new ArgumentNullException(nameof(handler));
         }
 
-        var middleware = new CoreEventMiddleware(ResolveEventType.OnPreparing, PipelinePhase.ParameterSelection, (ctxt, next) =>
+        var middleware = new CoreEventMiddleware(ResolveEventType.OnPreparing, PipelinePhase.ParameterSelection, (context, next) =>
         {
-            var args = new PreparingEventArgs(ctxt, ctxt.Service, ctxt.Registration, ctxt.Parameters);
+            var args = new PreparingEventArgs(context, context.Service, context.Registration, context.Parameters);
 
             handler(args);
 
-            ctxt.ChangeParameters(args.Parameters);
+            context.ChangeParameters(args.Parameters);
 
             // Go down the pipeline now.
-            next(ctxt);
+            next(context);
         });
 
         ResolvePipeline.Use(middleware);
@@ -461,14 +461,14 @@ public IRegistrationBuilder<TLimit, TActivatorData, TRegistrationStyle> OnActiva
             throw new ArgumentNullException(nameof(handler));
         }
 
-        var middleware = new CoreEventMiddleware(ResolveEventType.OnActivating, PipelinePhase.Activation, (ctxt, next) =>
+        var middleware = new CoreEventMiddleware(ResolveEventType.OnActivating, PipelinePhase.Activation, (context, next) =>
         {
-            next(ctxt);
+            next(context);
 
-            var args = new ActivatingEventArgs<TLimit>(ctxt, ctxt.Service, ctxt.Registration, ctxt.Parameters, (TLimit)ctxt.Instance!);
+            var args = new ActivatingEventArgs<TLimit>(context, context.Service, context.Registration, context.Parameters, (TLimit)context.Instance!);
 
             handler(args);
-            ctxt.Instance = args.Instance;
+            context.Instance = args.Instance;
         });
 
         // Activation events have to run at the start of the phase, to make sure
@@ -509,28 +509,28 @@ public IRegistrationBuilder<TLimit, TActivatorData, TRegistrationStyle> OnActiva
             throw new ArgumentNullException(nameof(handler));
         }
 
-        var middleware = new CoreEventMiddleware(ResolveEventType.OnActivated, PipelinePhase.Activation, (ctxt, next) =>
+        var middleware = new CoreEventMiddleware(ResolveEventType.OnActivated, PipelinePhase.Activation, (context, next) =>
         {
             // Go down the pipeline first.
-            next(ctxt);
+            next(context);
 
-            if (!ctxt.NewInstanceActivated)
+            if (!context.NewInstanceActivated)
             {
                 return;
             }
 
             // Make sure we use the instance at this point, before it is replaced by any decorators.
-            var newInstance = (TLimit)ctxt.Instance!;
+            var newInstance = (TLimit)context.Instance!;
 
             // In order to behave in the same manner as the original activation handler,
             // we need to attach to the RequestCompleting event so these run at the end after everything else.
-            ctxt.RequestCompleting += (sender, evArgs) =>
-        {
-            var ctxt = evArgs.RequestContext;
-            var args = new ActivatedEventArgs<TLimit>(ctxt, ctxt.Service, ctxt.Registration, ctxt.Parameters, newInstance);
+            context.RequestCompleting += (sender, evArgs) =>
+            {
+                var eventContext = evArgs.RequestContext;
+                var args = new ActivatedEventArgs<TLimit>(eventContext, eventContext.Service, eventContext.Registration, eventContext.Parameters, newInstance);
 
-            handler(args);
-        };
+                handler(args);
+            };
         });
 
         // Need to insert OnActivated at the start of the phase, to ensure we attach to RequestCompleting in the same order
@@ -568,30 +568,30 @@ public IRegistrationBuilder<TLimit, TActivatorData, TRegistrationStyle> OnActiva
     /// <returns>A registration builder allowing further configuration of the component.</returns>
     public IRegistrationBuilder<TLimit, TActivatorData, TRegistrationStyle> PropertiesAutowired(IPropertySelector propertySelector, bool allowCircularDependencies)
     {
-        ResolvePipeline.Use(nameof(PropertiesAutowired), PipelinePhase.Activation, (ctxt, next) =>
+        ResolvePipeline.Use(nameof(PropertiesAutowired), PipelinePhase.Activation, (context, next) =>
         {
             // Continue down the pipeline.
-            next(ctxt);
+            next(context);
 
-            if (!ctxt.NewInstanceActivated)
+            if (!context.NewInstanceActivated)
             {
                 return;
             }
 
             if (allowCircularDependencies)
             {
-                var capturedInstance = ctxt.Instance;
+                var capturedInstance = context.Instance;
 
                 // If we are allowing circular deps, then we need to run when all requests have completed (similar to Activated).
-                ctxt.RequestCompleting += (o, args) =>
-            {
-                var evCtxt = args.RequestContext;
-                AutowiringPropertyInjector.InjectProperties(evCtxt, capturedInstance!, propertySelector, evCtxt.Parameters);
-            };
+                context.RequestCompleting += (o, args) =>
+                {
+                    var eventContext = args.RequestContext;
+                    AutowiringPropertyInjector.InjectProperties(eventContext, capturedInstance!, propertySelector, eventContext.Parameters);
+                };
             }
             else
             {
-                AutowiringPropertyInjector.InjectProperties(ctxt, ctxt.Instance!, propertySelector, ctxt.Parameters);
+                AutowiringPropertyInjector.InjectProperties(context, context.Instance!, propertySelector, context.Parameters);
             }
         });
 

src/Autofac/Core/Activators/Delegate/DelegateActivator.cs

@@ -10,7 +10,6 @@ namespace Autofac.Core.Activators.Delegate;
 /// <summary>
 /// Activate instances using a delegate.
 /// </summary>
-[SuppressMessage("Microsoft.Design", "CA1063:ImplementIDisposableCorrectly", Justification = "There is nothing in the derived class to dispose so no override is necessary.")]
 public class DelegateActivator : InstanceActivator, IInstanceActivator
 {
     private readonly Func<IComponentContext, IEnumerable<Parameter>, object> _activationFunction;
@@ -34,11 +33,11 @@ public void ConfigurePipeline(IComponentRegistryServices componentRegistryServic
             throw new ArgumentNullException(nameof(pipelineBuilder));
         }
 
-        pipelineBuilder.Use(this.DisplayName(), PipelinePhase.Activation, MiddlewareInsertionMode.EndOfPhase, (ctxt, next) =>
+        pipelineBuilder.Use(this.DisplayName(), PipelinePhase.Activation, MiddlewareInsertionMode.EndOfPhase, (context, next) =>
         {
-            ctxt.Instance = ActivateInstance(ctxt, ctxt.Parameters);
+            context.Instance = ActivateInstance(context, context.Parameters);
 
-            next(ctxt);
+            next(context);
         });
     }
 

src/Autofac/Core/Activators/ProvidedInstance/ProvidedInstanceActivator.cs

@@ -34,11 +34,11 @@ public void ConfigurePipeline(IComponentRegistryServices componentRegistryServic
             throw new ArgumentNullException(nameof(pipelineBuilder));
         }
 
-        pipelineBuilder.Use(this.DisplayName(), PipelinePhase.Activation, MiddlewareInsertionMode.EndOfPhase, (ctxt, next) =>
+        pipelineBuilder.Use(this.DisplayName(), PipelinePhase.Activation, MiddlewareInsertionMode.EndOfPhase, (context, next) =>
         {
-            ctxt.Instance = GetInstance();
+            context.Instance = GetInstance();
 
-            next(ctxt);
+            next(context);
         });
     }
 
@@ -83,7 +83,7 @@ protected override void Dispose(bool disposing)
 
                 // Type only implements IAsyncDisposable. We will need to do sync-over-async.
                 // We want to ensure we lose all context here, because if we don't we can deadlock.
-                // So we push this disposal onto the threadpool.
+                // So we push this disposal onto the thread pool.
                 Task.Run(async () => await asyncDisposable.DisposeAsync().ConfigureAwait(false))
                     .ConfigureAwait(false)
                     .GetAwaiter().GetResult();
@@ -122,7 +122,6 @@ protected override ValueTask DisposeAsync(bool disposing)
         // Do not call the base, otherwise the standard Dispose will fire.
     }
 
-    [SuppressMessage("CA2222", "CA2222", Justification = "False positive. GetType doesn't hide an inherited member.")]
     private static Type GetType(object instance)
     {
         if (instance == null)

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

@@ -74,7 +74,7 @@ public static void InjectProperties(IComponentContext context, object instance,
             var parameter = resolveParameters.FirstOrDefault(p =>
                 p.CanSupplyValue(setParameter, context, out valueProvider) &&
                 !(p is NamedParameter n && n.Name.Equals("value", StringComparison.Ordinal)));
-            if (parameter != null)
+            if (parameter is not null)
             {
                 var setter = ReflectionCacheSet.Shared.Internal.AutowiringPropertySetters.GetOrAdd(property, MakeFastPropertySetter);
                 setter(instance, valueProvider!());

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

@@ -37,7 +37,7 @@ public override bool CanSupplyValue(ParameterInfo pi, IComponentContext context,
             // Workaround for https://github.com/dotnet/corefx/issues/17943
             if (pi.Member.DeclaringType?.Assembly.IsDynamic ?? true)
             {
-                hasDefaultValue = pi.DefaultValue != null && pi.HasDefaultValue;
+                hasDefaultValue = pi.DefaultValue is not null && pi.HasDefaultValue;
             }
             else
             {