From 68bc52c6deea44de9a2754fbff61aa27fbbae46f Mon Sep 17 00:00:00 2001
From: Steve Harter <steveharter@users.noreply.github.com>
Date: Wed, 31 Jul 2024 14:56:59 -0500
Subject: [PATCH 1/2] Return null instead of throwing in
 ServiceDescriptor.ImplementationInstance\Type\Factory if a keyed service

---
 .../src/Resources/Strings.resx                |  5 +-
 .../src/ServiceDescriptor.cs                  | 70 ++++++++-----------
 ...iceCollectionKeyedServiceExtensionsTest.cs |  6 +-
 3 files changed, 33 insertions(+), 48 deletions(-)

diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/Resources/Strings.resx b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/Resources/Strings.resx
index d9eb294211515b..0cdd3bff63ed00 100644
--- a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/Resources/Strings.resx
+++ b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/Resources/Strings.resx
@@ -170,10 +170,7 @@
   <data name="KeyedServicesNotSupported" xml:space="preserve">
     <value>This service provider doesn't support keyed services.</value>
   </data>
-  <data name="KeyedDescriptorMisuse" xml:space="preserve">
-    <value>This service descriptor is keyed. Your service provider may not support keyed services.</value>
-  </data>
   <data name="NonKeyedDescriptorMisuse" xml:space="preserve">
     <value>This service descriptor is not keyed.</value>
   </data>
-</root>
\ No newline at end of file
+</root>
diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ServiceDescriptor.cs b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ServiceDescriptor.cs
index 26c1f1007e1f25..1eca40092f6cea 100644
--- a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ServiceDescriptor.cs
+++ b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ServiceDescriptor.cs
@@ -152,24 +152,20 @@ private ServiceDescriptor(Type serviceType, object? serviceKey, ServiceLifetime
         private Type? _implementationType;
 
         /// <summary>
-        /// Gets the <see cref="Type"/> that implements the service.
+        /// Gets the <see cref="Type"/> that implements the service,
+        /// or returns <see langword="null"/> if <see cref="IsKeyedService"/> is <see langword="true"/>.
         /// </summary>
+        /// If <see cref="IsKeyedService"/> is <see langword="true"/>, <see cref="KeyedImplementationType"/> should be called instead.
         [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)]
-        public Type? ImplementationType
-        {
-            get
-            {
-                if (IsKeyedService)
-                {
-                    ThrowKeyedDescriptor();
-                }
-                return _implementationType;
-            }
-        }
+        public Type? ImplementationType => IsKeyedService ? null : _implementationType;
 
         /// <summary>
-        /// Gets the <see cref="Type"/> that implements the service.
+        /// Gets the <see cref="Type"/> that implements the service,
+        /// or throws <see cref="InvalidOperationException"/> if <see cref="IsKeyedService"/> is <see langword="false"/>.
         /// </summary>
+        /// <remarks>
+        /// If <see cref="IsKeyedService"/> is <see langword="false"/>, <see cref="ImplementationType"/> should be called instead.
+        /// </remarks>>
         [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)]
         public Type? KeyedImplementationType
         {
@@ -186,23 +182,19 @@ public Type? KeyedImplementationType
         private object? _implementationInstance;
 
         /// <summary>
-        /// Gets the instance that implements the service.
+        /// Gets the instance that implements the service,
+        /// or returns <see langword="null"/> if <see cref="IsKeyedService"/> is <see langword="true"/>.
         /// </summary>
-        public object? ImplementationInstance
-        {
-            get
-            {
-                if (IsKeyedService)
-                {
-                    ThrowKeyedDescriptor();
-                }
-                return _implementationInstance;
-            }
-        }
+        /// If <see cref="IsKeyedService"/> is <see langword="true"/>, <see cref="KeyedImplementationInstance"/> should be called instead.
+        public object? ImplementationInstance =>  IsKeyedService ? null : _implementationInstance;
 
         /// <summary>
-        /// Gets the instance that implements the service.
+        /// Gets the instance that implements the service,
+        /// or throws <see cref="InvalidOperationException"/> if <see cref="IsKeyedService"/> is <see langword="false"/>.
         /// </summary>
+        /// <remarks>
+        /// If <see cref="IsKeyedService"/> is <see langword="false"/>, <see cref="ImplementationInstance"/> should be called instead.
+        /// </remarks>>
         public object? KeyedImplementationInstance
         {
             get
@@ -218,23 +210,21 @@ public object? KeyedImplementationInstance
         private object? _implementationFactory;
 
         /// <summary>
-        /// Gets the factory used for creating service instances.
+        /// Gets the factory used for creating service instance,
+        /// or returns <see langword="null"/> if <see cref="IsKeyedService"/> is <see langword="true"/>.
         /// </summary>
-        public Func<IServiceProvider, object>? ImplementationFactory
-        {
-            get
-            {
-                if (IsKeyedService)
-                {
-                    ThrowKeyedDescriptor();
-                }
-                return (Func<IServiceProvider, object>?)_implementationFactory;
-            }
-        }
+        /// <remarks>
+        /// If <see cref="IsKeyedService"/> is <see langword="true"/>, <see cref="KeyedImplementationFactory"/> should be called instead.
+        /// </remarks>>
+        public Func<IServiceProvider, object>? ImplementationFactory => IsKeyedService ? null : (Func<IServiceProvider, object>?) _implementationFactory;
 
         /// <summary>
-        /// Gets the factory used for creating Keyed service instances.
+        /// Gets the factory used for creating Keyed service instances,
+        /// or throws <see cref="InvalidOperationException"/> if <see cref="IsKeyedService"/> is <see langword="false"/>.
         /// </summary>
+        /// <remarks>
+        /// If <see cref="IsKeyedService"/> is <see langword="false"/>, <see cref="ImplementationFactory"/> should be called instead.
+        /// </remarks>>
         public Func<IServiceProvider, object?, object>? KeyedImplementationFactory
         {
             get
@@ -1058,8 +1048,6 @@ private string DebuggerToString()
             return debugText;
         }
 
-        private static void ThrowKeyedDescriptor() => throw new InvalidOperationException(SR.KeyedDescriptorMisuse);
-
         private static void ThrowNonKeyedDescriptor() => throw new InvalidOperationException(SR.NonKeyedDescriptorMisuse);
     }
 }
diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceCollectionKeyedServiceExtensionsTest.cs b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceCollectionKeyedServiceExtensionsTest.cs
index 3715c619c8fa42..681336a0a4d542 100644
--- a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceCollectionKeyedServiceExtensionsTest.cs
+++ b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceCollectionKeyedServiceExtensionsTest.cs
@@ -546,9 +546,9 @@ public static TheoryData NotNullServiceKeyData
         public void NotNullServiceKey_IsKeyedServiceTrue(ServiceDescriptor serviceDescriptor)
         {
             Assert.True(serviceDescriptor.IsKeyedService);
-            Assert.Throws<InvalidOperationException>(() => serviceDescriptor.ImplementationInstance);
-            Assert.Throws<InvalidOperationException>(() => serviceDescriptor.ImplementationType);
-            Assert.Throws<InvalidOperationException>(() => serviceDescriptor.ImplementationFactory);
+            Assert.Null(serviceDescriptor.ImplementationInstance);
+            Assert.Null(serviceDescriptor.ImplementationType);
+            Assert.Null(serviceDescriptor.ImplementationFactory);
         }
     }
 }

From d38a4f6a0d3055b79d0d3ea06557d0ef3ecc08f4 Mon Sep 17 00:00:00 2001
From: Steve Harter <steveharter@users.noreply.github.com>
Date: Wed, 7 Aug 2024 09:46:19 -0500
Subject: [PATCH 2/2] Fix some xml doc remarks

---
 .../src/ServiceDescriptor.cs                         | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ServiceDescriptor.cs b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ServiceDescriptor.cs
index 1eca40092f6cea..6eef2ab84674de 100644
--- a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ServiceDescriptor.cs
+++ b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ServiceDescriptor.cs
@@ -155,7 +155,9 @@ private ServiceDescriptor(Type serviceType, object? serviceKey, ServiceLifetime
         /// Gets the <see cref="Type"/> that implements the service,
         /// or returns <see langword="null"/> if <see cref="IsKeyedService"/> is <see langword="true"/>.
         /// </summary>
+        /// <remarks>
         /// If <see cref="IsKeyedService"/> is <see langword="true"/>, <see cref="KeyedImplementationType"/> should be called instead.
+        /// </remarks>
         [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)]
         public Type? ImplementationType => IsKeyedService ? null : _implementationType;
 
@@ -165,7 +167,7 @@ private ServiceDescriptor(Type serviceType, object? serviceKey, ServiceLifetime
         /// </summary>
         /// <remarks>
         /// If <see cref="IsKeyedService"/> is <see langword="false"/>, <see cref="ImplementationType"/> should be called instead.
-        /// </remarks>>
+        /// </remarks>
         [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)]
         public Type? KeyedImplementationType
         {
@@ -185,7 +187,9 @@ public Type? KeyedImplementationType
         /// Gets the instance that implements the service,
         /// or returns <see langword="null"/> if <see cref="IsKeyedService"/> is <see langword="true"/>.
         /// </summary>
+        /// <remarks>
         /// If <see cref="IsKeyedService"/> is <see langword="true"/>, <see cref="KeyedImplementationInstance"/> should be called instead.
+        /// </remarks>
         public object? ImplementationInstance =>  IsKeyedService ? null : _implementationInstance;
 
         /// <summary>
@@ -194,7 +198,7 @@ public Type? KeyedImplementationType
         /// </summary>
         /// <remarks>
         /// If <see cref="IsKeyedService"/> is <see langword="false"/>, <see cref="ImplementationInstance"/> should be called instead.
-        /// </remarks>>
+        /// </remarks>
         public object? KeyedImplementationInstance
         {
             get
@@ -215,7 +219,7 @@ public object? KeyedImplementationInstance
         /// </summary>
         /// <remarks>
         /// If <see cref="IsKeyedService"/> is <see langword="true"/>, <see cref="KeyedImplementationFactory"/> should be called instead.
-        /// </remarks>>
+        /// </remarks>
         public Func<IServiceProvider, object>? ImplementationFactory => IsKeyedService ? null : (Func<IServiceProvider, object>?) _implementationFactory;
 
         /// <summary>
@@ -224,7 +228,7 @@ public object? KeyedImplementationInstance
         /// </summary>
         /// <remarks>
         /// If <see cref="IsKeyedService"/> is <see langword="false"/>, <see cref="ImplementationFactory"/> should be called instead.
-        /// </remarks>>
+        /// </remarks>
         public Func<IServiceProvider, object?, object>? KeyedImplementationFactory
         {
             get