Enable nullable reference types #534
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sharwell
force-pushed
the
enable-nrt
branch
3 times, most recently
from
77f73ff
to
87dd88f
Compare
All new diagnostics reported by the build are reduced to 'suggestion' severity, which allows the project to continue building while the issues are fixed.
…lable type) This diagnostic can be fixed in nearly all cases without signature changes, so it's safe to correct this warning early in the transition to nullable reference types and then maintain it as development proceeds. For the specific case of Utils.IsDerivedFrom (the only signature change in this commit), three options were available: 1) create a nullable local variable which is initialized to the parameter 'type' and operate on the local variable instead of the parameter; 2) redefine the parameter as nullable but apply the [DisallowNull] attribute to it to indicate that the initial value is not null but subsequent assignments can set it to null; or 3) redefine the parameter as nullable and allow null as an input. For this method, option 3 was semantically correct and the method is internal so external callers did not need to be specifically considered.
…-nullable reference type) When CS8625 is reported for a default parameter, the parameter type can be updated to accept null. This signature update retains the behavior of the parameter from prior to nullable reference types. For caller info attributes, we also apply [DisallowNull] to indicate that the compiled code will never pass null to the method.
…ble reference type) Tests that intentionally pass null to members that do not allow null should suppress CS8625. We don't want to update the target signatures as a result of these tests.
These are cases where code is explicitly creating an instance of 'Task<object?>', so we update the generic argument to make this explicit.
This warning was only reported in one location. It was easy to correct by using 'Assert.Equal<int?>' instead of 'Assert.Equal<int>'.
…type or method. Nullability of type argument doesn't match constraint type.) The only case where this diagnostic occurred was test code that intentionally tests the behavior with inputs that production code should not use. The suppression operator is applied rather than changing the signature of 'AsyncEventHandler<TEventArgs>'.
… differences in the nullability of reference types)
This change applies a 'Fix All in Solution' for CS8603, which currently covers all cases where null is explicitly returned from a method.
A helper method 'AsNonNullReturnUnchecked' is added, which performs a specific type conversion with a nullable suppression operator. Encapsulating the suppression operator ensures that only a specific conversion is permitted without introducing a warning, and helps document the code for future readers.
…lized. Consider declaring as nullable.)
…uninitialized. Consider declaring as nullable.)
…d is uninitialized. Consider declaring as nullable.)
…value for a type parameter) C# cannot currently represent the nullable form of a generic type, so some warnings are unavoidable in generic code when using nullable reference types. In this commit, we suppress instances of CS8717 which fall into this category.
Many methods in the code are implemented to allow null arguments. In some cases, the methods also return a Boolean value which can only have a particular value if the parameter is not null. For these cases, the attribute [NotNullWhen] is added to the parameter. While care must be taken to (manually) ensure the accuracy of this attribute, the ideal situation involves methods implemented with a simple null check at the beginning. For example: ``` if (parameterName is null) return false; ``` A method starting with this block can have [NotNullWhen(true)] applied to `parameterName`.
Many tests in this repository rely on the completion of asynchronous operations to set values to non-null. If a failure occurs, the NullReferenceException would result in a test failure, so we don't need to clutter the tests with large numbers of non-null assertions.
…ment) The compiler cannot track "linked" fields that are either both null or both non-null. A debug assertion is added to make one such case clear to both the reader and the compiler.
…ce argument) The compiler cannot track non-null values across `Enumerable.Where`. However, it is clear to a reader of this line (immediate context only) that the property cannot be null inside the `Select` expression. We use the suppression operator to address the compiler warning.
…rgument) IEnumerator<T>.Current is often implemented without state validation for efficiency purposes. We use a suppression operator within this property with documentation explaining that the property will behave correctly provided the caller is using the IEnumerator interface correctly.
…ce argument) Currently the compiler cannot identify that `LockHandle.IsValid` serves as a null check for `LockHandle.awaiter`, so suppressions are used to address warnings for references to the field following an IsValid check.
This change is made with understanding of the specific behavior of the VerifyCodeFixAsync method used by this test. When this API gets the same value for the source and fixedSource parameters, it automatically validates that the code fix provider does not offer any fix for the diagnostic(s) present in the source. This behavior matches the current intent of the test.
…ment) The compiler cannot track assignments made prior to calling a method, so we add an assertion for the expected state on entry to the method.
This commit updates code so the compiler can tell a specific local is not null. The result is not semantically identical to the original code, but the change is not observable because it is not possible to have an InvocationExpressionSyntax that provides a non-null symbol that also does not implement IMethodSymbol. Even if such a case did exist, it would not be relevant for this analyzer that only operates on methods.
* Allow 'sender' to be null * Allow 'args' to be any type
AArnott
approved these changes
Oct 29, 2019
These attributes are not validated by the compiler, so developers need to carefully review APIs to place these in the necessary locations.
Use nullable parameters where applicable in public APIs.
sharwell
force-pushed
the
enable-nrt
branch
2 times, most recently
from
8b6795f
to
25ba7ba
Compare
Value types always have a public default constructor that initializes all fields to null. Value types that do not have any non-nullable reference type fields will better be able to identify problematic behaviors in public members called on default instances of the type before they result in bugs for users.
This suppression is no longer necessary with the 16.4 compiler, which is now enabled for builds in this repository.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request incrementally enables nullable reference types. It is constructed as part of a demonstration for the application of nullable reference types to an existing code base.