Fix nullability warnings in projects targeting netstandard2.0

[captainsafia]

We have MSBuild targets in the repo that enable nullability on netstandard2.0 projects.

https://github.com/dotnet/aspnetcore/blob/07937702255861896d9ec34e28c7c0c59ceb6a3f/eng/targets/CSharp.Common.targets#L120-L130

However, the nullability warnings are not emitted during build by default.

https://github.com/dotnet/aspnetcore/blob/07937702255861896d9ec34e28c7c0c59ceb6a3f/eng/targets/CSharp.Common.targets#L125

Removing the property above shows that we have ~150 unhandled nullability warnings in the projects that currently "enable" nullability.

[captainsafia]

Consider making this behavior opt-out instead of opt-in so we can catch this in projects that enable it in the future:

https://github.com/dotnet/aspnetcore/pull/47144#discussion_r1134620492

[mkArtakMSFT]

@captainsafia I assume this is a cross-cutting issue which is applicable to most of the areas. Should we instead file separate issues per area and track that way?

[danmoseley]

This might be a task community members would like to contribute to, folks like @maxkoshevoi and @SteveDunn did a lot of this in dotnet/runtime.

[captainsafia]

This might be a task community members would like to contribute to, folks like @maxkoshevoi and @SteveDunn did a lot of this in dotnet/runtime.

If a community member is able to help here, that would be great! Especially if we could do this work in one fell swoop and avoid having to put temporary workarounds like the one described here).

Happy to provide any interested members with guidance!

[ghost]

Thanks for contacting us.

We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

[maxkoshevoi]

@captainsafia How would you like to fix nullability warnings in projects that target multiple TFMs?

If there're no warnings in .NET 8, but there're some in the same code in netstandard, it just means that netstandard SDK is not annotated that well, and warnings are false positive. Here are couple of examples:

But if we just add !, it might suppress valid warnings in the future (after code modification)

[captainsafia]

@maxkoshevoi Yep, I think it's prudent to use the null suppression operator in these cases.

[ghost]

Thanks for contacting us.

We're moving this issue to the .NET 9 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

[ghost]

Thanks for contacting us.

We're moving this issue to the .NET 9 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

[SeanFarrow]

Has anyone picked this up? It's definitely something I'd be interested in working on.

[captainsafia]

@SeanFarrow: Nobody's opened any PRs for this yet so you're definitely welcome to have a go at it! If you want, you can resolve issues for just a single project to avoid having to deal with all ~150 warnings at once. Let me know if you have any questions about this work.

[SeanFarrow]

@captainsafia,

Thanks, I'll start looking at this this week.

A couple of questions: I'm making the assumption we are working against the main branch? Also, do we just want to use he null coalessing operator () or have some conditional compilation if we are compiling against specific frameworks?

Thanks, Sean.

[captainsafia]

I'm making the assumption we are working against the main branch?

Yep, these changes should be made in main.

Also, do we just want to use he null coalessing operator () or have some conditional compilation if we are compiling against specific frameworks?

Not exactly -- depending on the type of nullability warning, we may want to handle it "correctly" by using APIs that use one other nullability attributes ([NotNullWhen()]) or the Debug.Assert API with annotations defined that we've declared in shared source here.

[SeanFarrow]

@captainsafia,

Thanks, would you recommend starting with any specific project?

I'd like to work through this project by project, so do you want a branch/pr for each individual update, or a single branch and then multiple PR's?

Sean.

[SeanFarrow]

@captainsafia,

Also, if you have a list of projects with warnings that would help.

Thanks, Sean.