Annotate Microsoft.Extensions.DependencyInjection.Abstractions for nu… #37488
Conversation
|
cc: @buyaa-n, @stephentoub |
…llable reference types Contributes to dotnet#2339
...icrosoft.Extensions.DependencyInjection.Abstractions/src/ServiceProviderServiceExtensions.cs
Show resolved
Hide resolved
| @@ -5,13 +5,14 @@ | |||
| using System; | |||
| using System.Reflection; | |||
|
|
|||
| #nullable enable | |||
There was a problem hiding this comment.
Nit: not a big deal, but we've generally been putting these above the using statements when they're needed
...icrosoft.Extensions.DependencyInjection.Abstractions/src/ServiceProviderServiceExtensions.cs
Show resolved
Hide resolved
| @@ -69,6 +71,7 @@ public static object GetRequiredService(this IServiceProvider provider, Type ser | |||
| /// <param name="provider">The <see cref="IServiceProvider"/> to retrieve the service object from.</param> | |||
| /// <returns>A service object of type <typeparamref name="T"/>.</returns> | |||
| /// <exception cref="System.InvalidOperationException">There is no service of type <typeparamref name="T"/>.</exception> | |||
| [return: NotNull] | |||
There was a problem hiding this comment.
Would it be better to constrain T to be non-nullable?
public static T GetRequiredService<T>(this IServiceProvider provider) where T : notnull?
There was a problem hiding this comment.
(Other APIs, like the one below, could use that constraint as well.)
There was a problem hiding this comment.
@davidfowl and I went over some of the other APIs here. We added the notnull constraint to another type, but the rest of these APIs don't qualify for additional constraints.
src/libraries/Common/src/Extensions/ActivatorUtilities/ActivatorUtilities.cs
Outdated
Show resolved
Hide resolved
...ependencyInjection.Abstractions/ref/Microsoft.Extensions.DependencyInjection.Abstractions.cs
Show resolved
Hide resolved
| @@ -2,6 +2,8 @@ | |||
| // The .NET Foundation licenses this file to you under the MIT license. | |||
| // See the LICENSE file in the project root for more information. | |||
|
|
|||
| #nullable enable | |||
There was a problem hiding this comment.
It's a shared source file. So you have to annotate it in case other projects that reference it haven't been updated to nullable.
|
@stephentoub does this need to be API reviewed before this is merged? |
|
@pranavkm, thanks for checking, but nope. We plan to do an all-up .NET 5 annotations review en mass for all the annotations added in the release. cc: @terrajobst |
ericstj
added
area-Extensions-DependencyInjection
and removed
area-Microsoft.Extensions
labels
ghost
locked as resolved and limited conversation to collaborators
…llable reference types
Contributes to #2339