Annotate remainder of .NET Core assemblies for nullable reference types #2339
Comments
|
Do you think we should take the effort to annotate our test projects as well? It seems that it might be a good way to catch annotations bugs, particularly in the public API surface. |
No.
I don't see the ROI. |
Dotnet-GitSync-Bot
added
area-Meta
untriaged
eiriktsarpalis
removed
the
untriaged
|
I looked |
Inconsistencies?
In general we strongly prefer to do a full source annotation (details at https://github.com/dotnet/runtime/blob/master/docs/coding-guidelines/api-guidelines/nullability.md). This isn't always possible or fruitful, though. For example, we have some libraries that build against netstandard2.0, and the benefit of doing full source annotation there is significantly diminished because the targeted surface area isn't annotated; as such, we actually disable all nullable warnings for such builds. As another example, Microsoft.VisualBasic.Core is ref only because we can't annotate the VB source, and Microsoft.CSharp is ref only because the code is mostly a direct port of an old C++ codebase that's also generally only consumed directly by compiler-generated code, and there'd be little value in exerting the effort to null annotate the implementation. System.DirectoryServices, though, references netcoreapp, so we would want to fully annotate its implementation. |
It seems it should be I found some points with such inconsistencies.
There are over 120 files. Makes sense to split the work to some PRs? |
Personally I think it'd be better to do it all in one. I expect you'll find that updates to annotations in one file cause new warnings in other files that have already been annotated. |
joperezr
added
the
tracking
Could you add absolute numbers to the issues, alone the lines of: Annotated APIs (inclusive counts):
|
Will do |
|
Only looking at
Currently missing in .NET 5:
|
…Runtime.Serialization.Json (#41476) * Initial nullable annotations of System.Private.DataContractSerialization Contributes to #2339 * Mark DataMember.Name as non-nullable. * Fix a few simple nullable compile errors. * Assert attributes is non-null in XmlObjectSerializerReadContext * Ensure XmlObjectSerializerContext.serializer is never null * Fix a few simple nullable errors * Remove any checks that DataMember.MemberInfo can be null. * Mark EnumDataContract.Members as non-nullable. Fix nullable errors in SchemaExporter. * Correctly annotate CollectionDataContract.IsCollectionOrTryCreate. * Assert DataContractResolver is non-null. * Suppress #41465 * Update System.Runtime.Serialization.Json ref source for nullable annotations. * Update System.Runtime.Serializaiton.Xml ref source for nullable annotations. * Update for Xml.ReaderWriter nullable annotations * Work around compiler issue. * Fix test failure. Reference compiler issue in TODO comment. * Revert nullable suppression now that XmlSchemaAppInfo.Markup is annotated correctly. * Fix build for latest annotations in master. * PR feedback round 1 * Address PR feedback round 2
…Runtime.Serialization.Json (dotnet#41476) * Initial nullable annotations of System.Private.DataContractSerialization Contributes to dotnet#2339 * Mark DataMember.Name as non-nullable. * Fix a few simple nullable compile errors. * Assert attributes is non-null in XmlObjectSerializerReadContext * Ensure XmlObjectSerializerContext.serializer is never null * Fix a few simple nullable errors * Remove any checks that DataMember.MemberInfo can be null. * Mark EnumDataContract.Members as non-nullable. Fix nullable errors in SchemaExporter. * Correctly annotate CollectionDataContract.IsCollectionOrTryCreate. * Assert DataContractResolver is non-null. * Suppress dotnet#41465 * Update System.Runtime.Serialization.Json ref source for nullable annotations. * Update System.Runtime.Serializaiton.Xml ref source for nullable annotations. * Update for Xml.ReaderWriter nullable annotations * Work around compiler issue. * Fix test failure. Reference compiler issue in TODO comment. * Revert nullable suppression now that XmlSchemaAppInfo.Markup is annotated correctly. * Fix build for latest annotations in master. * PR feedback round 1 * Address PR feedback round 2
|
Closing this 5.0.0 issue; the remaining annotations will be covered in 6.0 with #41720. |
ghost
locked as resolved and limited conversation to collaborators
In .NET Core 3.0, we annotated System.Private.CoreLib for nullable reference types, along with the reference assemblies that surface those types, and the implementations for all of the partial facades behind those reference assemblies. That represents something around 20% of the surface area. Post-.NET Core 3.0, we plan to annotate the rest.
Our desired approach to annotating an assembly is to only do so once all of its dependencies are annotated. That way, the assembly can be annotated as a unit, and barring any bugs that need to be fixed in a dependency, we won't need to spend time fixing up new warnings in higher-level assemblies as lower-level assemblies are annotated. This leads to a semi-ordering of how assemblies should be annotated, from the "bottom up". Here is a rough grouping. We can annotate everything in group 1 (in parallel if desired), then everything in group 2 (in parallel if desired), etc. There is of course more parallelism than this available, as not every assembly in group N has a dependency on every assembly in group N - 1, but the graph is complicated and this is easier to visualize and reason about.
We will:
cc: @dotnet/nullablefc, @cartermp
0 (Completed in .NET Core 3.0)
1
2
3
4
5
6
7
8
9
10 (part of netcoreapp but reference netstandard)
With netstandard not annotated, we will need to be cognizant of the fact that all dependencies will be viewed as oblivious.
11 (not part of netcoreapp but reference netcoreapp)
12 (built from dotnet/runtime but not in netcoreapp and reference netstandard)
With netstandard not annotated, we will need to be cognizant of the fact that all dependencies will be viewed as oblivious:
13
#nullable enablefrom individual files after all dependent projects annotatedThe text was updated successfully, but these errors were encountered: