Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Annotate Microsoft.Extensions.* assemblies for nullable reference types #43605

Closed
36 tasks done
Tracked by #43619 ...
maryamariyan opened this issue Oct 19, 2020 · 11 comments · Fixed by #67513
Closed
36 tasks done
Tracked by #43619 ...

Annotate Microsoft.Extensions.* assemblies for nullable reference types #43605

maryamariyan opened this issue Oct 19, 2020 · 11 comments · Fixed by #67513
Labels
area-Extensions-Primitives User Story A single user-facing feature. Can be grouped under an epic.
Milestone

Comments

@maryamariyan
Copy link
Member

maryamariyan commented Oct 19, 2020

In .NET 6.0, we plan to annotate Microsoft.Extensions.* assemblies built from the dotnet/runtime repo. Work involves:

  • Submit individual PRs, one for each assembly. Each PR should include changes to both the src and the ref. Each PR should contain only changes related to the nullable annotations/attributes, no other changes.
  • PRs can be merged once the annotations have been appropriately reviewed in PR.

In .NET 5.0, nullable references types was tracked in #2339, and remainder are being tracked in #41720 for .NET 6.0

Group 1

Group 2

  • Microsoft.Extensions.Caching.Abstractions
  • Microsoft.Extensions.Configuration.Abstractions
  • Microsoft.Extensions.FileProviders.Abstractions
  • Microsoft.Extensions.Options
  • Microsoft.Extensions.DependencyInjection

Group 3

  • Microsoft.Extensions.FileProviders.Physical
  • Microsoft.Extensions.FileProviders.Composite
  • Microsoft.Extensions.Hosting.Abstractions
  • Microsoft.Extensions.Configuration
  • Microsoft.Extensions.Logging
  • Microsoft.Extensions.Caching.Memory
  • Microsoft.Extensions.Options.DataAnnotation

Group 4

  • Microsoft.Extensions.Configuration.Binder (Check first if System.ComponentModel.TypeConverter nullable enabled)
  • Microsoft.Extensions.Configuration.CommandLine
  • Microsoft.Extensions.Configuration.EnvironmentVariables
  • Microsoft.Extensions.Configuration.FileExtensions
  • Microsoft.Extensions.Http
  • Microsoft.Extensions.Logging.EventLog (Check first if System.Diagnostics.EventLog nullable enabled)
  • Microsoft.Extensions.Logging.EventSource
  • Microsoft.Extensions.Logging.TraceSource (Check first if System.Diagnostics.TraceSource nullable enabled)
  • Microsoft.Extensions.Logging.Debug

Group 5

  • Microsoft.Extensions.Configuration.Ini
  • Microsoft.Extensions.Configuration.Json
  • Microsoft.Extensions.Configuration.Xml (Check first if System.Security.Cryptography.Xml nullable enabled)
  • Microsoft.Extensions.Options.ConfigurationExtensions

Group 6

  • Microsoft.Extensions.Configuration.UserSecrets
  • Microsoft.Extensions.Logging.Configuration

Group 7

  • Microsoft.Extensions.Logging.Console

Group 8

  • Microsoft.Extensions.Hosting

Group 9

  • Microsoft.Extensions.Hosting.Systemd
  • Microsoft.Extensions.Hosting.WindowsServices

Guidance here:

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-Meta untriaged New issue has not been triaged by the area owner labels Oct 19, 2020
@maryamariyan maryamariyan added this to Uncommitted in ML, Extensions, Globalization, etc, POD. via automation Oct 19, 2020
maryamariyan added a commit to maryamariyan/runtime that referenced this issue Oct 19, 2020
- include NetCoreAppCurrent
- prep for nullable annotations work

Related to: dotnet#43605
@maryamariyan maryamariyan moved this from Uncommitted to Committed in ML, Extensions, Globalization, etc, POD. Oct 28, 2020
@maryamariyan maryamariyan added Team Epic and removed untriaged New issue has not been triaged by the area owner labels Nov 4, 2020
@maryamariyan maryamariyan added this to the 6.0.0 milestone Nov 5, 2020
@maryamariyan maryamariyan added the User Story A single user-facing feature. Can be grouped under an epic. label Jan 19, 2021
@ghost ghost added this to Needs triage in Triage POD for Meta, Reflection, etc Jan 29, 2021
@safern safern moved this from Committed to Uncommitted in ML, Extensions, Globalization, etc, POD. Feb 2, 2021
@krwq krwq moved this from Needs triage to v-Next in Triage POD for Meta, Reflection, etc Feb 4, 2021
@krwq krwq modified the milestones: 6.0.0, 7.0.0 Jul 22, 2021
@krwq krwq moved this from 6.0 to Future in Triage POD for Meta, Reflection, etc Jul 22, 2021
@ghost ghost moved this from 6.0.0 to Untriaged in ML, Extensions, Globalization, etc, POD. Jul 22, 2021
@krwq
Copy link
Member

krwq commented Jul 22, 2021

@maryamariyan I assume the remainder is not gonna happen for 6.0, correct?

@ghost ghost moved this from Future to Needs triage in Triage POD for Meta, Reflection, etc Jul 22, 2021
@ghost ghost added the needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration label Jul 22, 2021
@SteveDunn
Copy link
Contributor

SteveDunn commented Mar 27, 2022

I was looking at doing this one:

Microsoft.Extensions.Configuration.Xml (Check first if System.Security.Cryptography.Xml nullable enabled)

So, I checked System.Security.Cryptography.Xml has nullable enabled, which it's not. So should I do that one first? (@maryamariyan )

@maxkoshevoi
Copy link
Contributor

maxkoshevoi commented Mar 27, 2022

@SteveDunn regarding Microsoft.Extensions.Configuration.Xml, we decided that it's ok to annotate it without annotating System.Security.Cryptography.Xml first (#66892 (comment)), and I've already created a PR for that.

Though PR for annotating System.Security.Cryptography.Xml would be accepted too.

@SteveDunn
Copy link
Contributor

@SteveDunn regarding Microsoft.Extensions.Configuration.Xml, we decided that it's ok to annotate it without annotating System.Security.Cryptography.Xml first (#66892 (comment)), and I've already created a PR for that.

Though PR for annotating System.Security.Cryptography.Xml would be accepted too.

Thanks @maxkoshevoi - I'll just concentrate on System.Security.Cryptography.Xml. I'm down to 470 compilation errors! 😮

radekdoulik pushed a commit to radekdoulik/runtime that referenced this issue Mar 30, 2022
@SteveDunn
Copy link
Contributor

SteveDunn commented Mar 31, 2022

Microsoft.Extensions.Configuration.Xml (Check first if System.Security.Cryptography.Xml nullable enabled)

I think @maxkoshevoi has done Microsoft.Extensions.Configuration.Xml, and I have an open PR for System.Security.Cryptography.Xml (#67198)

@SteveDunn
Copy link
Contributor

I'm happy to do Microsoft.Extensions.Http

@maxkoshevoi
Copy link
Contributor

I'm happy to do Microsoft.Extensions.Http

Already merged: #66891 😅

@SteveDunn
Copy link
Contributor

I'm happy to do Microsoft.Extensions.Http

Already merged: #66891 😅

I'm too slow! :) I created a PR anyway that I don't intend to merge - I created it purely to compare with yours: SteveDunn#1

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Apr 18, 2022
eerhardt added a commit that referenced this issue Apr 18, 2022
…ng.Systemd` (#67513)

Fix #43605

* Annotate

* Fix API compat check by removing MemberNotNullWhen attribute.

Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Apr 18, 2022
directhex pushed a commit to directhex/runtime that referenced this issue Apr 21, 2022
…ng.Systemd` (dotnet#67513)

Fix dotnet#43605

* Annotate

* Fix API compat check by removing MemberNotNullWhen attribute.

Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>
@ghost ghost locked as resolved and limited conversation to collaborators May 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Extensions-Primitives User Story A single user-facing feature. Can be grouped under an epic.
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

7 participants