Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Handle nullability attributes in param-nullchecking #59393

Merged
merged 5 commits into from
Feb 16, 2022
Merged

Handle nullability attributes in param-nullchecking #59393

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
48e5ecb
Handle nullability attributes in param-nullchecking
RikkiGibson Feb 8, 2022
07e63fc
fix
RikkiGibson Feb 8, 2022
3618a98
Avoid breaking 'TestNullableGenericsImplementingGenericAbstractClass'
RikkiGibson Feb 8, 2022
aa6f565
More tests
RikkiGibson Feb 8, 2022
0d8c58d
Address feedback
RikkiGibson Feb 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions src/Compilers/CSharp/Portable/Symbols/Source/ParameterHelpers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -809,12 +809,17 @@ 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.AllowNull) != 0
|| ((annotations & (FlowAnalysisAnnotations.DisallowNull | FlowAnalysisAnnotations.NotNull)) == 0
&& (parameter.TypeWithAnnotations.NullableAnnotation.IsAnnotated()
|| parameter.Type.IsNullableTypeOrTypeParameter())))
{
diagnostics.Add(ErrorCode.WRN_NullCheckingOnNullableType, location, parameter);
}
else if (parameter.Type.IsValueType && !parameter.Type.IsPointerOrFunctionPointer())

if (parameter.Type.IsNonNullableValueType() && !parameter.Type.IsPointerOrFunctionPointer())
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tend to dislike if (...) { warn } else if (...) { error } conditions anyway--can be a bit brittle.

{
diagnostics.Add(ErrorCode.ERR_NonNullableValueTypeIsNullChecked, location, parameter);
}
Expand Down
92 changes: 92 additions & 0 deletions src/Compilers/CSharp/Test/Semantic/Semantics/NullCheckedParameterTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1034,6 +1034,9 @@ static void M4<T>([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<T>([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));
Expand Down Expand Up @@ -1070,5 +1073,94 @@ 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 M0([AllowNull, DisallowNull] string s!!) { } // 1

void M1(string s!!) { }
void M2([NotNull] string s!!) { }
void M3([DisallowNull] string s!!) { }
void M4([AllowNull] string s!!) { } // 2

void M5(string? s!!) { } // 3
void M6([NotNull] string? s!!) { } // ok: this is a typical signature for an 'AssertNotNull' style method.
void M7([DisallowNull] string? s!!) { }
void M8([AllowNull] string? s!!) { } // 4

void M9<T>(T s!!) { }
void M10<T>([NotNull] T s!!) { }
void M11<T>([DisallowNull] T s!!) { }
void M12<T>([AllowNull] T s!!) { } // 5

void M13<T>(T? s!!) { } // 6
void M14<T>([NotNull] T? s!!) { }
void M15<T>([DisallowNull] T? s!!) { }
void M16<T>([AllowNull] T? s!!) { } // 7

void M17(int s!!) { } // 8
void M18([NotNull] int s!!) { } // 9
void M19([DisallowNull] int s!!) { } // 10
void M20([AllowNull] int s!!) { } // 11, 12

void M21(int? s!!) { } // 13
void M22([NotNull] int? s!!) { }
void M23([DisallowNull] int? s!!) { }
void M24([AllowNull] int? s!!) { } // 14
}";
RikkiGibson marked this conversation as resolved.
Show resolved Hide resolved
var comp = CreateCompilation(new[] { source, AllowNullAttributeDefinition, DisallowNullAttributeDefinition, NotNullAttributeDefinition }, parseOptions: TestOptions.RegularPreview);
comp.VerifyDiagnostics(
// (7,46): warning CS8995: Nullable type 'string' is null-checked and will throw if null.
// void M0([AllowNull, DisallowNull] string s!!) { } // 1
Diagnostic(ErrorCode.WRN_NullCheckingOnNullableType, "s").WithArguments("string").WithLocation(7, 46),
// (12,32): warning CS8995: Nullable type 'string' is null-checked and will throw if null.
// void M4([AllowNull] string s!!) { } // 2
Diagnostic(ErrorCode.WRN_NullCheckingOnNullableType, "s").WithArguments("string").WithLocation(12, 32),
// (14,21): warning CS8995: Nullable type 'string?' is null-checked and will throw if null.
// void M5(string? s!!) { } // 3
Diagnostic(ErrorCode.WRN_NullCheckingOnNullableType, "s").WithArguments("string?").WithLocation(14, 21),
// (17,33): warning CS8995: Nullable type 'string?' is null-checked and will throw if null.
// void M8([AllowNull] string? s!!) { } // 4
Diagnostic(ErrorCode.WRN_NullCheckingOnNullableType, "s").WithArguments("string?").WithLocation(17, 33),
// (22,31): warning CS8995: Nullable type 'T' is null-checked and will throw if null.
// void M12<T>([AllowNull] T s!!) { } // 5
Diagnostic(ErrorCode.WRN_NullCheckingOnNullableType, "s").WithArguments("T").WithLocation(22, 31),
// (24,20): warning CS8995: Nullable type 'T?' is null-checked and will throw if null.
// void M13<T>(T? s!!) { } // 6
Diagnostic(ErrorCode.WRN_NullCheckingOnNullableType, "s").WithArguments("T?").WithLocation(24, 20),
// (27,32): warning CS8995: Nullable type 'T?' is null-checked and will throw if null.
// void M16<T>([AllowNull] T? s!!) { } // 7
Diagnostic(ErrorCode.WRN_NullCheckingOnNullableType, "s").WithArguments("T?").WithLocation(27, 32),
// (29,18): error CS8992: Parameter 'int' is a non-nullable value type and cannot be null-checked.
// void M17(int s!!) { } // 8
Diagnostic(ErrorCode.ERR_NonNullableValueTypeIsNullChecked, "s").WithArguments("int").WithLocation(29, 18),
// (30,28): error CS8992: Parameter 'int' is a non-nullable value type and cannot be null-checked.
// void M18([NotNull] int s!!) { } // 9
Diagnostic(ErrorCode.ERR_NonNullableValueTypeIsNullChecked, "s").WithArguments("int").WithLocation(30, 28),
// (31,33): error CS8992: Parameter 'int' is a non-nullable value type and cannot be null-checked.
// void M19([DisallowNull] int s!!) { } // 10
Diagnostic(ErrorCode.ERR_NonNullableValueTypeIsNullChecked, "s").WithArguments("int").WithLocation(31, 33),
// (32,30): warning CS8995: Nullable type 'int' is null-checked and will throw if null.
// void M20([AllowNull] int s!!) { } // 11, 12
Diagnostic(ErrorCode.WRN_NullCheckingOnNullableType, "s").WithArguments("int").WithLocation(32, 30),
// (32,30): error CS8992: Parameter 'int' is a non-nullable value type and cannot be null-checked.
// void M20([AllowNull] int s!!) { } // 11, 12
Diagnostic(ErrorCode.ERR_NonNullableValueTypeIsNullChecked, "s").WithArguments("int").WithLocation(32, 30),
// (34,19): warning CS8995: Nullable type 'int?' is null-checked and will throw if null.
// void M21(int? s!!) { } // 13
Diagnostic(ErrorCode.WRN_NullCheckingOnNullableType, "s").WithArguments("int?").WithLocation(34, 19),
// (37,31): warning CS8995: Nullable type 'int?' is null-checked and will throw if null.
// void M24([AllowNull] int? s!!) { } // 14
Diagnostic(ErrorCode.WRN_NullCheckingOnNullableType, "s").WithArguments("int?").WithLocation(37, 31)
);
}
}
}