Handle nullability attributes in param-nullchecking #59393
Conversation
| { | ||
| diagnostics.Add(ErrorCode.WRN_NullCheckingOnNullableType, location, parameter); | ||
| } | ||
| else if (parameter.Type.IsValueType && !parameter.Type.IsPointerOrFunctionPointer()) | ||
|
|
||
| if (parameter.Type.IsNonNullableValueType() && !parameter.Type.IsPointerOrFunctionPointer()) |
There was a problem hiding this comment.
I tend to dislike if (...) { warn } else if (...) { error } conditions anyway--can be a bit brittle.
| var annotations = parameter.FlowAnalysisAnnotations; | ||
| if ((annotations & FlowAnalysisAnnotations.NotNull) == 0 | ||
| && !NullableWalker.GetParameterState(parameter.TypeWithAnnotations, annotations, applyParameterNullCheck: false).IsNotNull | ||
| && (!parameter.Type.IsTypeParameter() || parameter.Type.IsNullableTypeOrTypeParameter() || parameter.TypeWithAnnotations.NullableAnnotation.IsAnnotated() || (annotations & FlowAnalysisAnnotations.AllowNull) != 0)) |
There was a problem hiding this comment.
Generally, if it's possible for a parameter of a type parameter type to be non-nullable after substitution, we want to avoid giving the warning.
src/Compilers/CSharp/Test/Semantic/Semantics/NullCheckedParameterTests.cs
Show resolved
Hide resolved
This comment was marked as outdated.
This comment was marked as outdated.
|
ping @dotnet/roslyn-compiler for second review |
|
|
||
| var annotations = parameter.FlowAnalysisAnnotations; | ||
| if ((annotations & FlowAnalysisAnnotations.NotNull) == 0 | ||
| && !NullableWalker.GetParameterState(parameter.TypeWithAnnotations, annotations, applyParameterNullCheck: false).IsNotNull |
There was a problem hiding this comment.
Consider simplifying the double-negation: && NullableWalker.GetParameterState(....).MayBeNull
Correction: should use the extension method MayBeNull(). Somehow, MaybeNull property and MayBeNull() extension method diverged for the maybe-default case :-/
| var annotations = parameter.FlowAnalysisAnnotations; | ||
| if ((annotations & FlowAnalysisAnnotations.NotNull) == 0 | ||
| && !NullableWalker.GetParameterState(parameter.TypeWithAnnotations, annotations, applyParameterNullCheck: false).IsNotNull | ||
| && (!parameter.Type.IsTypeParameter() || parameter.Type.IsNullableTypeOrTypeParameter() || parameter.TypeWithAnnotations.NullableAnnotation.IsAnnotated() || (annotations & FlowAnalysisAnnotations.AllowNull) != 0)) |
There was a problem hiding this comment.
From offline chat:
There's an open question on type parameter constrained to C?.
Is there any existing helper/abstraction we might use to simplify this?
Consider extracting this line to a local function with comments.
| { | ||
| diagnostics.Add(ErrorCode.ERR_NonNullableValueTypeIsNullChecked, location, parameter); | ||
| } | ||
|
|
||
| // For type parameters, we only want to give the warning if no type argument would result in a non-nullable type. | ||
| static bool isTypeParameterWithPossiblyNonNullableType(TypeWithAnnotations typeWithAnnotations, FlowAnalysisAnnotations annotations) |
There was a problem hiding this comment.
Thanks, that was much easier to follow :-)
ghost
added this to the 
Closes #59226
Relates to test plan #36024