diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs index 6ff752fbfba82..75027bd1d71f5 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs @@ -2067,7 +2067,7 @@ internal static bool AreParameterAnnotationsCompatible( overriddenType, overriddenAnnotations, // We don't consider '!!' when deciding whether 'overridden' is compatible with 'override' - isNullChecked: false); + applyParameterNullCheck: false); if (isBadAssignment(valueState, overridingType, overridingAnnotations)) { return false; @@ -2527,9 +2527,9 @@ private void EnterParameter(ParameterSymbol parameter, TypeWithAnnotations param return null; } - internal static TypeWithState GetParameterState(TypeWithAnnotations parameterType, FlowAnalysisAnnotations parameterAnnotations, bool isNullChecked) + internal static TypeWithState GetParameterState(TypeWithAnnotations parameterType, FlowAnalysisAnnotations parameterAnnotations, bool applyParameterNullCheck) { - if (isNullChecked) + if (applyParameterNullCheck) { return TypeWithState.Create(parameterType.Type, NullableFlowState.NotNull); } diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/ParameterHelpers.cs b/src/Compilers/CSharp/Portable/Symbols/Source/ParameterHelpers.cs index 910cd4273f970..1b51643ada5cc 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/ParameterHelpers.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/ParameterHelpers.cs @@ -809,15 +809,43 @@ internal static void ReportParameterNullCheckingErrors(DiagnosticBag diagnostics { diagnostics.Add(ErrorCode.ERR_DiscardCannotBeNullChecked, location); } - if (parameter.TypeWithAnnotations.NullableAnnotation.IsAnnotated() - || parameter.Type.IsNullableTypeOrTypeParameter()) + + var annotations = parameter.FlowAnalysisAnnotations; + if ((annotations & FlowAnalysisAnnotations.NotNull) == 0 + && NullableWalker.GetParameterState(parameter.TypeWithAnnotations, annotations, applyParameterNullCheck: false).State.MayBeNull() + && !isTypeParameterWithPossiblyNonNullableType(parameter.TypeWithAnnotations, annotations)) { diagnostics.Add(ErrorCode.WRN_NullCheckingOnNullableType, location, parameter); } - else if (parameter.Type.IsValueType && !parameter.Type.IsPointerOrFunctionPointer()) + + if (parameter.Type.IsNonNullableValueType() && !parameter.Type.IsPointerOrFunctionPointer()) { 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) + { + if (!typeWithAnnotations.Type.IsTypeParameter()) + { + return false; + } + + // We avoid checking the nullable annotations, etc. of constraints due to implementation complexity, + // and consider it acceptable to miss "!! on nullable type" warnings in scenarios like `void M(U u!!) where U : T?`. + if (typeWithAnnotations.NullableAnnotation.IsAnnotated()) + { + return false; + } + + // `void M([AllowNull] T t!!)` + if ((annotations & FlowAnalysisAnnotations.AllowNull) != 0) + { + return false; + } + + return true; + } } internal static ImmutableArray ConditionallyCreateInModifiers(RefKind refKind, bool addRefReadOnlyModifier, Binder binder, BindingDiagnosticBag diagnostics, SyntaxNode syntax) diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/NullCheckedParameterTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/NullCheckedParameterTests.cs index 781d1b889736d..5b22538850a77 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/NullCheckedParameterTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/NullCheckedParameterTests.cs @@ -725,16 +725,13 @@ internal override void F(U u!! = default) { } } class B3 : A { - internal override void F(U u!! = default) { } + internal override void F(U u!! = default) { } // note: 'U' is a nullable type here but we don't give a warning due to complexity of accurately searching the constraints. }"; var compilation = CreateCompilation(source, parseOptions: TestOptions.RegularPreview); compilation.VerifyDiagnostics( - // (12,35): warning CS8719: Parameter 'u' is null-checked but is null by default. - // internal override void F(U u!! = default) { } - Diagnostic(ErrorCode.WRN_NullCheckedHasDefaultNull, "u").WithArguments("u").WithLocation(12, 35), - // (16,35): warning CS8721: Nullable value type 'U' is null-checked and will throw if null. - // internal override void F(U u!! = default) { } - Diagnostic(ErrorCode.WRN_NullCheckingOnNullableType, "u").WithArguments("U").WithLocation(16, 35)); + // (12,35): warning CS8993: Parameter 'u' is null-checked but is null by default. + // internal override void F(U u!! = default) { } + Diagnostic(ErrorCode.WRN_NullCheckedHasDefaultNull, "u").WithArguments("u").WithLocation(12, 35)); } [Fact] @@ -1034,6 +1031,9 @@ static void M4([DisallowNull] T x3, [DisallowNull] T y3!!) // (16,9): warning CS8602: Dereference of a possibly null reference. // x1.ToString(); // 3 Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "x1").WithLocation(16, 9), + // (19,55): warning CS8995: Nullable type 'T' is null-checked and will throw if null. + // static void M3([AllowNull] T x2, [AllowNull] T y2!!) + Diagnostic(ErrorCode.WRN_NullCheckingOnNullableType, "y2").WithArguments("T").WithLocation(19, 55), // (21,9): warning CS8602: Dereference of a possibly null reference. // x2.ToString(); // 4 Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "x2").WithLocation(21, 9)); @@ -1070,5 +1070,165 @@ public override void M2(string s) { } // 3 Diagnostic(ErrorCode.WRN_TopLevelNullabilityMismatchInParameterTypeOnOverride, "M2").WithArguments("s").WithLocation(13, 26) ); } + + [Fact] + public void TestNullabilityAttributes() + { + var source = +@" +#nullable enable +using System.Diagnostics.CodeAnalysis; + +class C +{ + void M( + string s1!!, + [NotNull] string s2!!, + [DisallowNull] string s3!!, + [AllowNull] string s4!!, // 1 + [AllowNull, DisallowNull] string s5!!, // 2 + [AllowNull, NotNull] string s6!!, + + string? s7!!, // 3 + [NotNull] string? s8!!, // ok: this is a typical signature for an 'AssertNotNull' style method. + [DisallowNull] string? s9!!, + [AllowNull] string? s10!!, // 4 + [AllowNull, DisallowNull] string? s11!!, // 5 + [AllowNull, NotNull] string? s12!!, + + int i1!!, // 6 + [NotNull] int i2!!, // 7 + [DisallowNull] int i3!!, // 8 + [AllowNull] int i4!!, // 9 + [AllowNull, DisallowNull] int i5!!, // 10 + [AllowNull, NotNull] int i6!!, // 11 + + int? i7!!, // 12 + [NotNull] int? i8!!, + [DisallowNull] int? i9!!, + [AllowNull] int? i10!!, // 13 + [AllowNull, DisallowNull] int? i11!!, // 14 + [AllowNull, NotNull] int? i12!! + ) { } +}"; + var comp = CreateCompilation(new[] { source, AllowNullAttributeDefinition, DisallowNullAttributeDefinition, NotNullAttributeDefinition }, parseOptions: TestOptions.RegularPreview); + comp.VerifyDiagnostics( + // (11,28): warning CS8995: Nullable type 'string' is null-checked and will throw if null. + // [AllowNull] string s4!!, // 1 + Diagnostic(ErrorCode.WRN_NullCheckingOnNullableType, "s4").WithArguments("string").WithLocation(11, 28), + // (12,42): warning CS8995: Nullable type 'string' is null-checked and will throw if null. + // [AllowNull, DisallowNull] string s5!!, // 2 + Diagnostic(ErrorCode.WRN_NullCheckingOnNullableType, "s5").WithArguments("string").WithLocation(12, 42), + // (15,17): warning CS8995: Nullable type 'string?' is null-checked and will throw if null. + // string? s7!!, // 3 + Diagnostic(ErrorCode.WRN_NullCheckingOnNullableType, "s7").WithArguments("string?").WithLocation(15, 17), + // (18,29): warning CS8995: Nullable type 'string?' is null-checked and will throw if null. + // [AllowNull] string? s10!!, // 4 + Diagnostic(ErrorCode.WRN_NullCheckingOnNullableType, "s10").WithArguments("string?").WithLocation(18, 29), + // (19,43): warning CS8995: Nullable type 'string?' is null-checked and will throw if null. + // [AllowNull, DisallowNull] string? s11!!, // 5 + Diagnostic(ErrorCode.WRN_NullCheckingOnNullableType, "s11").WithArguments("string?").WithLocation(19, 43), + // (22,13): error CS8992: Parameter 'int' is a non-nullable value type and cannot be null-checked. + // int i1!!, // 6 + Diagnostic(ErrorCode.ERR_NonNullableValueTypeIsNullChecked, "i1").WithArguments("int").WithLocation(22, 13), + // (23,23): error CS8992: Parameter 'int' is a non-nullable value type and cannot be null-checked. + // [NotNull] int i2!!, // 7 + Diagnostic(ErrorCode.ERR_NonNullableValueTypeIsNullChecked, "i2").WithArguments("int").WithLocation(23, 23), + // (24,28): error CS8992: Parameter 'int' is a non-nullable value type and cannot be null-checked. + // [DisallowNull] int i3!!, // 8 + Diagnostic(ErrorCode.ERR_NonNullableValueTypeIsNullChecked, "i3").WithArguments("int").WithLocation(24, 28), + // (25,25): error CS8992: Parameter 'int' is a non-nullable value type and cannot be null-checked. + // [AllowNull] int i4!!, // 9 + Diagnostic(ErrorCode.ERR_NonNullableValueTypeIsNullChecked, "i4").WithArguments("int").WithLocation(25, 25), + // (26,39): error CS8992: Parameter 'int' is a non-nullable value type and cannot be null-checked. + // [AllowNull, DisallowNull] int i5!!, // 10 + Diagnostic(ErrorCode.ERR_NonNullableValueTypeIsNullChecked, "i5").WithArguments("int").WithLocation(26, 39), + // (27,34): error CS8992: Parameter 'int' is a non-nullable value type and cannot be null-checked. + // [AllowNull, NotNull] int i6!!, // 11 + Diagnostic(ErrorCode.ERR_NonNullableValueTypeIsNullChecked, "i6").WithArguments("int").WithLocation(27, 34), + // (29,14): warning CS8995: Nullable type 'int?' is null-checked and will throw if null. + // int? i7!!, // 12 + Diagnostic(ErrorCode.WRN_NullCheckingOnNullableType, "i7").WithArguments("int?").WithLocation(29, 14), + // (32,26): warning CS8995: Nullable type 'int?' is null-checked and will throw if null. + // [AllowNull] int? i10!!, // 13 + Diagnostic(ErrorCode.WRN_NullCheckingOnNullableType, "i10").WithArguments("int?").WithLocation(32, 26), + // (33,40): warning CS8995: Nullable type 'int?' is null-checked and will throw if null. + // [AllowNull, DisallowNull] int? i11!!, // 14 + Diagnostic(ErrorCode.WRN_NullCheckingOnNullableType, "i11").WithArguments("int?").WithLocation(33, 40) + ); + } + + [Theory] + [InlineData("")] + [InlineData("where T : class")] + [InlineData("where T : class?")] + [InlineData("where T : notnull")] + public void TestNullabilityAttributes_Generic(string constraints) + { + var source = +@" +#nullable enable +using System.Diagnostics.CodeAnalysis; + +class C +{ + void M( + T t1!!, + [NotNull] T t2!!, + [DisallowNull] T t3!!, + [AllowNull] T t4!!, // 1 + [AllowNull, DisallowNull] T t5!!, // 2 + [AllowNull, NotNull] T t6!!, + + T? t7!!, // 3 + [NotNull] T? t8!!, + [DisallowNull] T? t9!!, + [AllowNull] T? t10!!, // 4 + [AllowNull, DisallowNull] T? t11!!, // 5 + [AllowNull, NotNull] T? t12!! + ) " + constraints + @" { } +}"; + var comp = CreateCompilation(new[] { source, AllowNullAttributeDefinition, DisallowNullAttributeDefinition, NotNullAttributeDefinition }, parseOptions: TestOptions.RegularPreview); + comp.VerifyDiagnostics( + // (11,23): warning CS8995: Nullable type 'T' is null-checked and will throw if null. + // [AllowNull] T t4!!, // 1 + Diagnostic(ErrorCode.WRN_NullCheckingOnNullableType, "t4").WithArguments("T").WithLocation(11, 23), + // (12,37): warning CS8995: Nullable type 'T' is null-checked and will throw if null. + // [AllowNull, DisallowNull] T t5!!, // 2 + Diagnostic(ErrorCode.WRN_NullCheckingOnNullableType, "t5").WithArguments("T").WithLocation(12, 37), + // (15,12): warning CS8995: Nullable type 'T?' is null-checked and will throw if null. + // T? t7!!, // 3 + Diagnostic(ErrorCode.WRN_NullCheckingOnNullableType, "t7").WithArguments("T?").WithLocation(15, 12), + // (18,24): warning CS8995: Nullable type 'T?' is null-checked and will throw if null. + // [AllowNull] T? t10!!, // 4 + Diagnostic(ErrorCode.WRN_NullCheckingOnNullableType, "t10").WithArguments("T?").WithLocation(18, 24), + // (19,38): warning CS8995: Nullable type 'T?' is null-checked and will throw if null. + // [AllowNull, DisallowNull] T? t11!! // 5 + Diagnostic(ErrorCode.WRN_NullCheckingOnNullableType, "t11").WithArguments("T?").WithLocation(19, 38) + ); + } + + [Fact] + public void AnnotatedTypeParameter_Indirect() + { + var source = @" +#nullable enable + +class C +{ + void M( + T? t!!, // 1 + U u!!) where U : T? + { + } +}"; + // note: U is always nullable when a reference type, + // but we don't warn on '!!' for it due to complexity of accurately searching the constraints. + var comp = CreateCompilation(source); + comp.VerifyDiagnostics( + // (7,12): warning CS8995: Nullable type 'T?' is null-checked and will throw if null. + // T? t!!, // 1 + Diagnostic(ErrorCode.WRN_NullCheckingOnNullableType, "t").WithArguments("T?").WithLocation(7, 12)); + } } }