Formed in 2009, the Archive Team (not to be confused with the archive.org Archive-It Team) is a rogue archivist collective dedicated to saving copies of rapidly dying or deleted websites for the sake of history and digital heritage. The group is 100% composed of volunteers and interested parties, and has expanded into a large amount of related projects for saving online and digital history.
Update ASP.NET Core to use C# 8's nullable reference types #5680
Comments
|
I hereby certify this idea as good. |
JamesNK
changed the title
|
Assigning to @JamesNK to give this some more thought on what can concretely be done in ASP.NET Core and what value it would provide. |
|
Over Christmas I'm going to play around with C# 8.0 and update Newtonsoft.Json to use nullable reference types. Will be able to give some more feedback then about how ready the feature is and much work it will be to use it. I suspect it will be large. Some random thoughts:
|
If this is the case, then we'd probably want to wait until 2019 is RTM. We try to avoid requiring that contributors have previews of stuff. But given all of the other stuff going on in 3.0 it's possible we might need to require that for other reasons. |
|
@Eilon thanks for creating that label. As you know I am unable to create labels. |
|
If you had certified it spicy then you would have this label to use
|
|
Findings from converting Newtonsoft.Json over the break:
|
This is concerning because we all really enjoy warnings as errors. I think it in this case it would probably be fine to support this guy into the libraries get stables. Something interesting about this is that you get a new flavour of the classic warnings problem. Adding a warning is always a breaking change. Is nullability part of netstandard2.0? Can it ever change? |
Totes, warning as errors is the best. But there is a way to allow-list (@Eilon |
|
I discussed this with Mads and Immo. The corefx approach is to have a sidecar file format containing metadata. The file format should be usable by all packages. Some reasons for this approach:
I don't think (1) is much of a concern for us. We'd be happy including nullable metadata from the latest version going forward. (2) would be much faster for us. Disadvantage is we'd miss out on catching nulls in our own code, and there would be ongoing upkeep of the sidecar file. When a new API is added then the nullable metadata would also need to be added to the file. |
|
Moving this to backlog as we have no capacity to handle this in 3.0 release. |
|
Now that C#8 and VS16.3, etc. have been officially released, will you start looking into this again? |
|
Any plans for this feature? I currently have to work around this issue when I implement my own |
|
The main place this becomes an annoyance is when you're working in a project with Nullable=true and implementing an ASP.NET Core interface/abstract class with methods that ought to have question marks in them, but of course don't. I think the only place I've actually run into myself is in When you're calling an ASP.NET Core method that ought to return |
|
My workaround is to temporarily disable NRT and restore it again, like this: class YourImpl : IConnectionUserFeature {
#nullable disable
public ClaimsPrincipal User { get; set; }
#nullable restore
}It works, but the need to disable NRT checks is not what I expected. |
* [x] CORS * [x] Diagnostics (public types only) * [x] HeaderPropagation * [x] HealthChecks * [x] HostFiltering * [x] HttpOverrides * [x] HttpsPolicy * [x] Localization * [x] MiddlewareAnalysis * [x] ResponseCaching Contributes to #5680
* [x] CORS * [x] Diagnostics (public types only) * [x] HeaderPropagation * [x] HealthChecks * [x] HostFiltering * [x] HttpOverrides * [x] HttpsPolicy * [x] Localization * [x] MiddlewareAnalysis * [x] ResponseCaching Contributes to #5680
* Add nullable annotations to middleware * [x] CORS * [x] Diagnostics (public types only) * [x] HeaderPropagation * [x] HealthChecks * [x] HostFiltering * [x] HttpOverrides * [x] HttpsPolicy * [x] Localization * [x] MiddlewareAnalysis * [x] ResponseCaching Contributes to #5680
Updating ASP.NET Core to use C# 8's nullable reference types would:
@rynowak @davidfowl @DamianEdwards @Eilon
Projects in no particular order
Skipped (potentially no longer commonly used, potential future removal, only used in tests, internal project, etc)
The text was updated successfully, but these errors were encountered: