Resolve MakeGenericType ILLink warning in DependencyInjection (dotnet…

…#55102)

* Resolve MakeGenericType ILLink warning in DependencyInjection

Resolve the ILLink warning in DependencyInjection by adding a runtime check that is behind a new AppContext switch 'Microsoft.Extensions.DependencyInjection.VerifyOpenGenericServiceTrimmability'. The runtime check ensures the trimming annotations on the open generic types are compatible between the service and implementation types. The check is enabled by default when PublishTrimmed=true.

* Make VerifyOpenGenericServiceTrimmability a full feature switch

docs/workflow/trimming/feature-switches.md

@@ -25,6 +25,7 @@ configurations but their defaults might vary as any SDK can set the defaults dif
 | EnableCppCLIHostActivation | System.Runtime.InteropServices.EnableCppCLIHostActivation | C++/CLI host activation code is disabled when set to false and related functionality can be trimmed. |
 | MetadataUpdaterSupport | System.Reflection.Metadata.MetadataUpdater.IsSupported | Metadata update related code to be trimmed when set to false |
 | _EnableConsumingManagedCodeFromNativeHosting | System.Runtime.InteropServices.EnableConsumingManagedCodeFromNativeHosting | Getting a managed function from native hosting is disabled when set to false and related functionality can be trimmed. |
+| VerifyDependencyInjectionOpenGenericServiceTrimmability | Microsoft.Extensions.DependencyInjection.VerifyOpenGenericServiceTrimmability | When set to true, DependencyInjection will verify trimming annotations applied to open generic services are correct |
 | NullabilityInfoContextSupport | System.Reflection.NullabilityInfoContext.IsSupported | Nullable attributes can be trimmed when set to false |
 | _AggressiveAttributeTrimming | System.AggressiveAttributeTrimming | When set to true, aggressively trims attributes to allow for the most size savings possible, even if it could result in runtime behavior changes |
 

src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/Microsoft.Extensions.DependencyInjection.Abstractions.csproj

@@ -1,4 +1,4 @@
-<Project Sdk="Microsoft.NET.Sdk">
+<Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
     <TargetFrameworks>$(NetCoreAppCurrent);netstandard2.1;netstandard2.0;net461</TargetFrameworks>

src/libraries/Microsoft.Extensions.DependencyInjection/src/ILLink/ILLink.Substitutions.xml

@@ -0,0 +1,7 @@
+<linker>
+  <assembly fullname="Microsoft.Extensions.DependencyInjection">
+    <type fullname="Microsoft.Extensions.DependencyInjection.ServiceProvider">
+      <method signature="System.Boolean get_VerifyOpenGenericServiceTrimmability()" body="stub" value="true" feature="Microsoft.Extensions.DependencyInjection.VerifyOpenGenericServiceTrimmability" featurevalue="true" />
+    </type>
+  </assembly>
+</linker>

src/libraries/Microsoft.Extensions.DependencyInjection/src/Microsoft.Extensions.DependencyInjection.csproj

@@ -1,4 +1,4 @@
-<Project Sdk="Microsoft.NET.Sdk">
+<Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
     <TargetFrameworks>$(NetCoreAppCurrent);netstandard2.1;netstandard2.0;net461</TargetFrameworks>
@@ -19,6 +19,10 @@
     <DefineConstants Condition="$(TargetFramework.StartsWith('net4')) and '$(ILEmitBackendSaveAssemblies)' == 'True'">$(DefineConstants);SAVE_ASSEMBLIES</DefineConstants>  
   </PropertyGroup>
 
+  <ItemGroup>
+    <ILLinkSubstitutionsXmls Include="$(ILLinkDirectory)ILLink.Substitutions.xml" />
+  </ItemGroup>
+
   <ItemGroup Condition="'$(TargetFrameworkIdentifier)' == '.NETCoreApp'">
     <Compile Include="$(CommonPath)Extensions\ParameterDefaultValue\ParameterDefaultValue.netcoreapp.cs"
              Link="Common\src\Extensions\ParameterDefaultValue\ParameterDefaultValue.netcoreapp.cs" />
@@ -32,7 +36,7 @@
     <Compile Include="$(CoreLibSharedDir)System\Diagnostics\CodeAnalysis\DynamicallyAccessedMemberTypes.cs" />
     <Compile Include="$(CoreLibSharedDir)System\Diagnostics\CodeAnalysis\UnconditionalSuppressMessageAttribute.cs" />
   </ItemGroup>
-
+  
   <ItemGroup>
     <Compile Include="**\*.cs" />
     <Compile Remove="ServiceLookup\ILEmit\**\*.cs" />
@@ -41,7 +45,6 @@
              Link="Common\src\Extensions\ParameterDefaultValue\ParameterDefaultValue.cs" />
     <Compile Include="$(CommonPath)Extensions\TypeNameHelper\TypeNameHelper.cs"
              Link="Common\src\Extensions\TypeNameHelper\TypeNameHelper.cs" />
-
   </ItemGroup>
 
   <ItemGroup>

src/libraries/Microsoft.Extensions.DependencyInjection/src/Resources/Strings.resx

@@ -174,4 +174,10 @@
   <data name="CallSiteTypeNotSupported" xml:space="preserve">
     <value>Call site type {0} is not supported</value>
   </data>
-</root>
+  <data name="TrimmingAnnotationsDoNotMatch" xml:space="preserve">
+    <value>Generic implementation type '{0}' has a DynamicallyAccessedMembers attribute applied to a generic argument type, but the service type '{1}' doesn't have a matching DynamicallyAccessedMembers attribute on its generic argument type.</value>
+  </data>
+  <data name="TrimmingAnnotationsDoNotMatch_NewConstraint" xml:space="preserve">
+    <value>Generic implementation type '{0}' has a DefaultConstructorConstraint ('new()' constraint), but the generic service type '{1}' doesn't.</value>
+  </data>
+</root>

src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceLookup/CallSiteFactory.cs

@@ -51,11 +51,18 @@ private void Populate()
                             SR.Format(SR.TypeCannotBeActivated, implementationType, serviceType));
                     }
 
-                    if (serviceType.GetGenericArguments().Length != implementationType.GetGenericArguments().Length)
+                    Type[] serviceTypeGenericArguments = serviceType.GetGenericArguments();
+                    Type[] implementationTypeGenericArguments = implementationType.GetGenericArguments();
+                    if (serviceTypeGenericArguments.Length != implementationTypeGenericArguments.Length)
                     {
                         throw new ArgumentException(
                             SR.Format(SR.ArityOfOpenGenericServiceNotEqualArityOfOpenGenericImplementation, serviceType, implementationType), "descriptors");
                     }
+
+                    if (ServiceProvider.VerifyOpenGenericServiceTrimmability)
+                    {
+                        ValidateTrimmingAnnotations(serviceType, serviceTypeGenericArguments, implementationType, implementationTypeGenericArguments);
+                    }
                 }
                 else if (descriptor.ImplementationInstance == null && descriptor.ImplementationFactory == null)
                 {
@@ -77,6 +84,68 @@ private void Populate()
             }
         }
 
+        /// <summary>
+        /// Validates that two generic type definitions have compatible trimming annotations on their generic arguments.
+        /// </summary>
+        /// <remarks>
+        /// When open generic types are used in DI, there is an error when the concrete implementation type
+        /// has [DynamicallyAccessedMembers] attributes on a generic argument type, but the interface/service type
+        /// doesn't have matching annotations. The problem is that the trimmer doesn't see the members that need to
+        /// be preserved on the type being passed to the generic argument. But when the interface/service type also has
+        /// the annotations, the trimmer will see which members need to be preserved on the closed generic argument type.
+        /// </remarks>
+        private static void ValidateTrimmingAnnotations(
+            Type serviceType,
+            Type[] serviceTypeGenericArguments,
+            Type implementationType,
+            Type[] implementationTypeGenericArguments)
+        {
+            Debug.Assert(serviceTypeGenericArguments.Length == implementationTypeGenericArguments.Length);
+
+            for (int i = 0; i < serviceTypeGenericArguments.Length; i++)
+            {
+                Type serviceGenericType = serviceTypeGenericArguments[i];
+                Type implementationGenericType = implementationTypeGenericArguments[i];
+
+                DynamicallyAccessedMemberTypes serviceDynamicallyAccessedMembers = GetDynamicallyAccessedMemberTypes(serviceGenericType);
+                DynamicallyAccessedMemberTypes implementationDynamicallyAccessedMembers = GetDynamicallyAccessedMemberTypes(implementationGenericType);
+
+                if (!AreCompatible(serviceDynamicallyAccessedMembers, implementationDynamicallyAccessedMembers))
+                {
+                    throw new ArgumentException(SR.Format(SR.TrimmingAnnotationsDoNotMatch, implementationType.FullName, serviceType.FullName));
+                }
+
+                bool serviceHasNewConstraint = serviceGenericType.GenericParameterAttributes.HasFlag(GenericParameterAttributes.DefaultConstructorConstraint);
+                bool implementationHasNewConstraint = implementationGenericType.GenericParameterAttributes.HasFlag(GenericParameterAttributes.DefaultConstructorConstraint);
+                if (implementationHasNewConstraint && !serviceHasNewConstraint)
+                {
+                    throw new ArgumentException(SR.Format(SR.TrimmingAnnotationsDoNotMatch_NewConstraint, implementationType.FullName, serviceType.FullName));
+                }
+            }
+        }
+
+        private static DynamicallyAccessedMemberTypes GetDynamicallyAccessedMemberTypes(Type serviceGenericType)
+        {
+            foreach (CustomAttributeData attributeData in serviceGenericType.GetCustomAttributesData())
+            {
+                if (attributeData.AttributeType.FullName == "System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute" &&
+                    attributeData.ConstructorArguments.Count == 1 &&
+                    attributeData.ConstructorArguments[0].ArgumentType.FullName == "System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes")
+                {
+                    return (DynamicallyAccessedMemberTypes)(int)attributeData.ConstructorArguments[0].Value;
+                }
+            }
+
+            return DynamicallyAccessedMemberTypes.None;
+        }
+
+        private static bool AreCompatible(DynamicallyAccessedMemberTypes serviceDynamicallyAccessedMembers, DynamicallyAccessedMemberTypes implementationDynamicallyAccessedMembers)
+        {
+            // The DynamicallyAccessedMemberTypes don't need to exactly match.
+            // The service type needs to preserve a superset of the members required by the implementation type.
+            return serviceDynamicallyAccessedMembers.HasFlag(implementationDynamicallyAccessedMembers);
+        }
+
         // For unit testing
         internal int? GetSlot(ServiceDescriptor serviceDescriptor)
         {
@@ -273,6 +342,10 @@ private ServiceCallSite TryCreateExact(ServiceDescriptor descriptor, Type servic
             return null;
         }
 
+        [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2055:MakeGenericType",
+            Justification = "MakeGenericType here is used to create a closed generic implementation type given the closed service type. " +
+            "Trimming annotations on the generic types are verified when 'Microsoft.Extensions.DependencyInjection.VerifyOpenGenericServiceTrimmability' is set, which is set by default when PublishTrimmed=true. " +
+            "That check informs developers when these generic types don't have compatible trimming annotations.")]
         private ServiceCallSite TryCreateOpenGeneric(ServiceDescriptor descriptor, Type serviceType, CallSiteChain callSiteChain, int slot, bool throwOnConstraintViolation)
         {
             if (serviceType.IsConstructedGenericType &&

src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceProvider.cs

@@ -30,6 +30,9 @@ public sealed class ServiceProvider : IServiceProvider, IDisposable, IAsyncDispo
 
         internal ServiceProviderEngineScope Root { get; }
 
+        internal static bool VerifyOpenGenericServiceTrimmability { get; } =
+            AppContext.TryGetSwitch("Microsoft.Extensions.DependencyInjection.VerifyOpenGenericServiceTrimmability", out bool verifyOpenGenerics) ? verifyOpenGenerics : false;
+
         internal ServiceProvider(IEnumerable<ServiceDescriptor> serviceDescriptors, ServiceProviderOptions options)
         {
             // note that Root needs to be set before calling GetEngine(), because the engine may need to access Root

src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/Microsoft.Extensions.DependencyInjection.Tests.csproj

@@ -1,8 +1,11 @@
-<Project Sdk="Microsoft.NET.Sdk">
+<Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
     <TargetFrameworks>$(NetCoreAppCurrent);net461</TargetFrameworks>
     <EnableDefaultItems>true</EnableDefaultItems>
+    <IncludeRemoteExecutor>true</IncludeRemoteExecutor>
+    <!-- CS0436: DynamicallyAccessedMemberTypes conflicts with imported type -->
+    <NoWarn Condition="'$(TargetFramework)' == 'net461'">$(NoWarn);CS0436</NoWarn>
   </PropertyGroup>
 
   <ItemGroup>
@@ -11,6 +14,12 @@
              Link="Shared\SingleThreadedSynchronizationContext.cs" />
   </ItemGroup>
 
+  <ItemGroup Condition="!$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net5.0'))">
+    <Compile Include="$(CoreLibSharedDir)System\Diagnostics\CodeAnalysis\UnconditionalSuppressMessageAttribute.cs" />
+    <Compile Include="$(CoreLibSharedDir)System\Diagnostics\CodeAnalysis\DynamicallyAccessedMembersAttribute.cs" />
+    <Compile Include="$(CoreLibSharedDir)System\Diagnostics\CodeAnalysis\DynamicallyAccessedMemberTypes.cs" />
+  </ItemGroup>
+
   <ItemGroup>
     <PackageReference Include="Newtonsoft.Json" Version="$(NewtonsoftJsonVersion)" />
     <ProjectReference Include="$(LibrariesProjectRoot)Microsoft.Extensions.DependencyInjection.Abstractions\src\Microsoft.Extensions.DependencyInjection.Abstractions.csproj" SkipUseReferenceAssembly="true" />

src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceLookup/CallSiteFactoryTest.cs

@@ -3,9 +3,11 @@
 
 using System;
 using System.Collections.Generic;
+using System.Diagnostics.CodeAnalysis;
 using System.Linq;
 using System.Reflection;
 using System.Threading.Tasks;
+using Microsoft.DotNet.RemoteExecutor;
 using Microsoft.Extensions.DependencyInjection.Extensions;
 using Microsoft.Extensions.DependencyInjection.Specification.Fakes;
 using Xunit;
@@ -849,6 +851,51 @@ public void CallSitesAreUniquePerServiceTypeAndSlotWithOpenGenericInGraph()
             }
         }
 
+        [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
+        [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)] // RuntimeConfigurationOptions are not supported on .NET Framework (and neither is trimming)
+        public void VerifyOpenGenericTrimmabilityChecks()
+        {
+            RemoteInvokeOptions options = new RemoteInvokeOptions();
+            options.RuntimeConfigurationOptions.Add("Microsoft.Extensions.DependencyInjection.VerifyOpenGenericServiceTrimmability", "true");
+
+            using RemoteInvokeHandle remoteHandle = RemoteExecutor.Invoke(() =>
+            {
+                (Type, Type)[] invalidTestCases = new[]
+                {
+                    (typeof(IFakeOpenGenericService<>), typeof(ClassWithNewConstraint<>)),
+                    (typeof(IServiceWithoutTrimmingAnnotations<>), typeof(ServiceWithTrimmingAnnotations<>)),
+                    (typeof(IServiceWithPublicConstructors<>), typeof(ServiceWithPublicProperties<>)),
+                    (typeof(IServiceWithTwoGenerics<,>), typeof(ServiceWithTwoGenericsInvalid<,>)),
+                };
+                foreach ((Type serviceType, Type implementationType) in invalidTestCases)
+                {
+                    ServiceDescriptor[] serviceDescriptors = new[]
+                    {
+                        new ServiceDescriptor(serviceType, implementationType, ServiceLifetime.Singleton)
+                    };
+
+                    Assert.Throws<ArgumentException>(() => new CallSiteFactory(serviceDescriptors));
+                }
+
+                (Type, Type)[] validTestCases = new[]
+                {
+                    (typeof(IFakeOpenGenericService<>), typeof(FakeOpenGenericService<>)),
+                    (typeof(IServiceWithPublicConstructors<>), typeof(ServiceWithPublicConstructors<>)),
+                    (typeof(IServiceWithTwoGenerics<,>), typeof(ServiceWithTwoGenericsValid<,>)),
+                    (typeof(IServiceWithMoreMemberTypes<>), typeof(ServiceWithLessMemberTypes<>)),
+                };
+                foreach ((Type serviceType, Type implementationType) in validTestCases)
+                {
+                    ServiceDescriptor[] serviceDescriptors = new[]
+                    {
+                        new ServiceDescriptor(serviceType, implementationType, ServiceLifetime.Singleton)
+                    };
+
+                    Assert.NotNull(new CallSiteFactory(serviceDescriptors));
+                }
+            }, options);
+        }
+
         private static Func<Type, ServiceCallSite> GetCallSiteFactory(params ServiceDescriptor[] descriptors)
         {
             var collection = new ServiceCollection();
@@ -887,5 +934,20 @@ private class ClassB { public ClassB(ClassC<object> cc) { } }
         private class ClassC<T> { }
         private class ClassD { public ClassD(ClassC<string> cd) { } }
         private class ClassE { public ClassE(ClassB cb) { } }
+
+        // Open generic with trimming annotations
+        private interface IServiceWithoutTrimmingAnnotations<T> { }
+        private class ServiceWithTrimmingAnnotations<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] T> : IServiceWithoutTrimmingAnnotations<T> { }
+
+        private interface IServiceWithPublicConstructors<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] T> { }
+        private class ServiceWithPublicProperties<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties)] T>: IServiceWithPublicConstructors<T> { }
+        private class ServiceWithPublicConstructors<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] T>: IServiceWithPublicConstructors<T> { }
+
+        private interface IServiceWithTwoGenerics<T1, [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] T2> { }
+        private class ServiceWithTwoGenericsInvalid<T1, [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties)] T2> : IServiceWithTwoGenerics<T1, T2> { }
+        private class ServiceWithTwoGenericsValid<T1, [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] T2> : IServiceWithTwoGenerics<T1, T2> { }
+
+        private interface IServiceWithMoreMemberTypes<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.PublicProperties)] T> { }
+        private class ServiceWithLessMemberTypes<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties)] T> : IServiceWithMoreMemberTypes<T> { }
     }
 }