Nullable annotations for Microsoft.Extensions.Configuration

[maxkoshevoi]

Description

Indexer and GetConnectionString can return null, but they are not annotated to indicate that.

https://github.com/dotnet/runtime/blob/main/src/libraries/Microsoft.Extensions.Configuration.Abstractions/src/ConfigurationExtensions.cs#L34

Regression?

No

Other information

I'm willing to fix it myself

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[stephentoub]

This library hasn't been annotated for nullability at all.

[maxkoshevoi]

@stephentoub Would PR that annotates it be accepted? If so, I can do it.

[stephentoub]

Yes, that would be welcome. Thanks.

[danmoseley]

Seconding that, we would welcome help annotating our last remaining libraries. This may help you:

https://github.com/dotnet/runtime/blob/main/docs/coding-guidelines/api-guidelines/nullability.md

Reach out if you need help from us.

[maxkoshevoi]

Do you know why Visual Studio 2019 might not generate nullable errors even though nullable is enabled (I tried enabling it everywhere down to `#nullable enabled in the source file)?

VS does generate them for other projects in libraries folder, but not for Configuration ones for some reason.

[ghost]

Tagging subscribers to this area: @maryamariyan, @safern
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Description

Indexer and GetConnectionString can return null, but they are not annotated to indicate that.

https://github.com/dotnet/runtime/blob/main/src/libraries/Microsoft.Extensions.Configuration.Abstractions/src/ConfigurationExtensions.cs#L34

Regression?

No

Other information

I'm willing to fix it myself

Author:	maxkoshevoi
Assignees:	-
Labels:	untriaged, area-Extensions-Configuration
Milestone:	-

[stephentoub]

Do you know why Visual Studio 2019 might not generate nullable errors even though nullable is enabled (I tried enabling it everywhere down to `#nullable enabled in the source file)?

The library only has a netstandard2.0 and net461 configuration today:

runtime/src/libraries/Microsoft.Extensions.Configuration.Abstractions/src/Microsoft.Extensions.Configuration.Abstractions.csproj

Line 4 in 3b40c7c

<TargetFrameworks>netstandard2.0;net461</TargetFrameworks>

and we explicitly disable nullable warnings across the repo for such projects:

runtime/src/libraries/Directory.Build.targets

Line 27 in 3b40c7c

<NoWarn Condition="'$(TargetFrameworkIdentifier)' == '.NETFramework' or '$(TargetFrameworkIdentifier)' == '.NETStandard' or ('$(TargetFrameworkIdentifier)' == '.NETCoreApp' and $([MSBuild]::VersionLessThan($(TargetFrameworkVersion), '3.0')))">$(NoWarn);nullable</NoWarn>

You can try overriding it by explicitly adding <NoWarn/> to the csproj, but I expect you may run into some additional complications, including the reasons why we suppress them for this configuration. For example, because of lack of annotations on Debug.Assert in the reference assemblies for those target frameworks, the compiler won't consider Debug.Assert(x != null) as part of its nullable flow analysis. The library will probably need a newer target framework added to do this correctly.

[danmoseley]

Cc @eerhardt

[eerhardt]

Duplicate of #43605. I'm going to close this issue as there is no reason to track this in multiple places.

The library only has a netstandard2.0 and net461 configuration today

The plan for all Microsoft.Extensions.* libraries is to add a $(NetCoreAppCurrent) TFM to the project. That will allow the compiler to give the correct nullable warnings. (Plus it will give us other benefits as well.) See #54012.

We've been adding the $(NetCoreAppCurrent) TFM to other Extensions libraries as we've needed. e.g. #56324. So @maxkoshevoi, if you were going to add nullable annotations for Configuration, you can just add the new TFM as well in the same PR. But be sure to check all the #ifs are still correct for the new TFM. We've hit that issue in the past when adding new TFMs - for example #56843.