Skip to content

Commit

Permalink
Update nullability.md
Browse files Browse the repository at this point in the history
  • Loading branch information
stephentoub authored Nov 24, 2020
1 parent 1fa2054 commit aebd598
Showing 1 changed file with 8 additions and 8 deletions.
16 changes: 8 additions & 8 deletions docs/coding-guidelines/api-guidelines/nullability.md
Original file line number Diff line number Diff line change
@@ -1,20 +1,20 @@
# Nullability annotations

C# 8 provides an opt-in feature that allows for the compiler to track reference type nullability in order to catch potential null dereferences. We are adopting that feature across .NET's libraries, working up from the bottom of the stack. We're doing this for three primary reasons, in order of importance:
As of C# 8 and improved in C# 9, the C# language provides an opt-in feature that allows the compiler to track reference type nullability in order to catch potential null dereferences. We have adopted that feature set across .NET's libraries, working up from the bottom of the stack. We've done this (and continue to do so for all new library development) for three primary reasons, in order of importance:

- **To annotate the .NET surface area with appropriate nullability annotations.** While this could be done solely in the reference assemblies, we're doing it first in the implementation to help validate the selected annotations.
- **To help validate the nullability feature itself.** With millions of lines of C# code, we have a very large and robust codebase with which to try out the feature and find areas in which it shines and areas in which we can improve it. The work to annotate System.Private.CoreLib in .NET Core 3.0 helped to improve the feature as shipped in C# 8, and annotating the rest of the libraries will continue to be helpful in this regard.
- **To find null-related bugs in .NET Runtime itself.** We expect to find relatively few meaningful bugs, due to how relatively well-tested the codebases are and how long they've been around.
- **To annotate the .NET surface area with appropriate nullability annotations.** While this could be done solely in the reference assemblies, we do it in the implementation to help validate the selected annotations.
- **To help validate the nullability feature itself.** With millions of lines of C# code, we have a very large and robust codebase with which to try out features and find areas in which it shines and areas in which we can improve it. The work to annotate System.Private.CoreLib in .NET Core 3.0 helped to improve the feature as shipped in C# 8, and the rest of the core libraries being annotated in .NET 5 helped to further flesh out the design for C# 9.
- **To find null-related bugs in .NET Runtime itself.** We have found few meaningful bugs, due to how relatively well-tested the codebases are and how long they've been around. However, for new code, the annotations do help to highlight places where null values may be erroneously dereferenced, helping to avoid NullReferenceExceptions in places testing may not yet be adequate.

## Breaking Change Guidance

We are striving to get annotations correct the "first time" and are doing due-diligence in an attempt to do so. However, we acknowledge that we are likely to need to augment and change some annotations in the future:
We strive to get annotations correct the "first time" and do due-diligence in an attempt to do so. However, we acknowledge that we are likely to need to augment and change some annotations in the future:

- **Mistakes.** Given the sheer number of APIs being reviewed and annotated, we are likely to make some errors, and we'd like to be able to fix them so that long-term customers get the greatest benefit.
- **Breadth.** Annotation takes time, so annotations will roll out over time rather than being released all at once.
- **Breadth.** Annotation takes time, so annotations have rolled out and will continue to roll out over time rather than all at once for the whole stack.
- **Feedback.** We may need to revisit some "gray area" decisions as to whether a parameter or return type should be nullable or non-nullable (more details later).

Any such additions or changes to annotations can impact the warnings consuming code receives if that code has opted in to nullability analysis and warnings. Even so, for at least the foreseeable future we may still do so. We will be very thoughtful about when and how we do.
Any such additions or changes to annotations can impact the warnings consuming code receives if that code has opted-in to nullability analysis and warnings. Even so, for at least the foreseeable future we may still do so, and we will be very thoughtful about when and how we do.

## Annotation Guidance

Expand All @@ -23,7 +23,7 @@ Nullability annotations are considered to represent intent: they represent the n
- **DO** annotate all new APIs with the desired contract.
- **CONSIDER** changing that contract if overwhelming use suggests a different de facto contract. This is particularly relevant to virtual/abstract/interface methods defined in a library where all implementations may not be under your control, and derived implementations may not have adhered to the original intent.
- **DO** respect documented behaviors. Nullability annotations are about codifying a contract, and documentation is a description of that contract. If, for example, a base abstract or virtual method says that it may throw an exception if `null` is passed in, that dictates that the argument should be non-nullable.
- **DO** continue to validate all arguments as you would have prior to nullability warnings. In particular, if you would have checked an argument for `null` and thrown an `ArgumentNullException` if it was `null`, continue to do so, even if the parameter is defined as non-nullable. Many consumers will not have nullability checked enabled, either because they're using a language other than C#, they're using an older version of C#, they simply haven't opted-in to nullability analysis, or the compiler isn't able to detect the misuse.
- **DO** continue to validate all arguments as you would have prior to nullability warnings. In particular, if you would have checked an argument for `null` and thrown an `ArgumentNullException` if it was `null`, continue to do so, even if the parameter is defined as non-nullable. Many consumers will not have nullability checked enabled, either because they're using a language other than C#, they're using an older version of C#, they simply haven't opted-in to nullability analysis, they've explicitly suppressed warnings, or the compiler isn't able to detect the misuse.
- **DO NOT** remove existing argument validation when annotating existing public/protected APIs. If through the course of annotating it becomes clear that internal/private surface area is unnecessarily checking for nulls when nulls aren't possible (i.e. you found dead code), such use can be removed or replaced by asserts.
- **AVOID** making any changes while annotating that substantively impact the generated IL for an implementation (e.g. `some.Method()` to `some?.Method()`). Any such changes should be thoroughly analyzed and reviewed as a bug fix. In some cases, such changes may be warranted, but they're a red flag that should be scrutinized heavily.

Expand Down

0 comments on commit aebd598

Please sign in to comment.