From f49a448430ba423721cb3f1125b41b3ee7a6d170 Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Tue, 8 Oct 2024 13:00:16 -0700 Subject: [PATCH] Respect [MemberNotNull] on local function --- .../Portable/FlowAnalysis/NullableWalker.cs | 26 +- .../Semantics/NullableReferenceTypesTests.cs | 359 +++++++++++++++++- 2 files changed, 360 insertions(+), 25 deletions(-) diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs index 9497b1d56b02f..1d5f506d61558 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs @@ -2012,7 +2012,7 @@ int makeSlot(BoundExpression node) case BoundKind.ThisReference: case BoundKind.BaseReference: { - var method = getTopLevelMethod(_symbol as MethodSymbol); + var method = GetTopLevelMethod(_symbol as MethodSymbol); var thisParameter = method?.ThisParameter; return thisParameter is object ? GetOrCreateSlot(thisParameter) : -1; } @@ -2090,20 +2090,20 @@ int getPlaceholderSlot(BoundExpression expr) } return -1; } + } - static MethodSymbol? getTopLevelMethod(MethodSymbol? method) + static MethodSymbol? GetTopLevelMethod(MethodSymbol? method) + { + while (method is object) { - while (method is object) + var container = method.ContainingSymbol; + if (container.Kind == SymbolKind.NamedType) { - var container = method.ContainingSymbol; - if (container.Kind == SymbolKind.NamedType) - { - return method; - } - method = container as MethodSymbol; + return method; } - return null; + method = container as MethodSymbol; } + return null; } protected override int GetOrCreateSlot(Symbol symbol, int containingSlot = 0, bool forceSlotEvenIfEmpty = false, bool createIfMissing = true) @@ -7012,6 +7012,12 @@ private void ApplyMemberPostConditions(BoundExpression? receiverOpt, MethodSymbo receiverOpt is null ? -1 : MakeSlot(receiverOpt); + if (method.MethodKind == MethodKind.LocalFunction + && GetTopLevelMethod(method) is { ThisParameter: { } thisParameter }) + { + receiverSlot = GetOrCreateSlot(thisParameter); + } + if (receiverSlot < 0) { return; diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs index e2a7bc8b97f0d..29ef7ca95d504 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs @@ -20937,10 +20937,10 @@ public C() ); } - [Fact] + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/56256")] public void MemberNotNull_LocalFunction() { - var c = CreateNullableCompilation(new[] { @" + var c = CreateNullableCompilation([""" using System.Diagnostics.CodeAnalysis; public class C { @@ -20952,32 +20952,361 @@ public class C public C() { init(); + field1.ToString(); + field2.ToString(); + field3.ToString(); // 1 + field4.ToString(); // 2 + + [MemberNotNull(nameof(field1), nameof(field2))] + void init() => throw null!; + } +} +""", MemberNotNullAttributeDefinition], parseOptions: TestOptions.Regular9); + + c.VerifyDiagnostics( + // (14,9): warning CS8602: Dereference of a possibly null reference. + // field3.ToString(); // 1 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "field3").WithLocation(14, 9), + // (15,9): warning CS8602: Dereference of a possibly null reference. + // field4.ToString(); // 2 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "field4").WithLocation(15, 9)); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/56256")] + public void MemberNotNull_LocalFunction_BaseField() + { + var c = CreateNullableCompilation([""" +using System.Diagnostics.CodeAnalysis; +public class Base +{ + public string field1; + public string? field2; + public string field3; + public string? field4; +} +public class C : Base +{ + public C() + { + init(); + field1.ToString(); + field2.ToString(); + field3.ToString(); + field4.ToString(); + + [MemberNotNull(nameof(field1), nameof(field2))] + void init() => throw null!; + } +} +""", MemberNotNullAttributeDefinition]); + + c.VerifyDiagnostics( + // (4,19): warning CS8618: Non-nullable field 'field1' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the field as nullable. + // public string field1; + Diagnostic(ErrorCode.WRN_UninitializedNonNullableField, "field1").WithArguments("field", "field1").WithLocation(4, 19), + // (6,19): warning CS8618: Non-nullable field 'field3' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the field as nullable. + // public string field3; + Diagnostic(ErrorCode.WRN_UninitializedNonNullableField, "field3").WithArguments("field", "field3").WithLocation(6, 19), + // (15,9): warning CS8602: Dereference of a possibly null reference. + // field2.ToString(); + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "field2").WithLocation(15, 9), + // (17,9): warning CS8602: Dereference of a possibly null reference. + // field4.ToString(); + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "field4").WithLocation(17, 9), + // (19,10): warning CS8776: Member 'field1' cannot be used in this attribute. + // [MemberNotNull(nameof(field1), nameof(field2))] + Diagnostic(ErrorCode.WRN_MemberNotNullBadMember, "MemberNotNull(nameof(field1), nameof(field2))").WithArguments("field1").WithLocation(19, 10), + // (19,10): warning CS8776: Member 'field2' cannot be used in this attribute. + // [MemberNotNull(nameof(field1), nameof(field2))] + Diagnostic(ErrorCode.WRN_MemberNotNullBadMember, "MemberNotNull(nameof(field1), nameof(field2))").WithArguments("field2").WithLocation(19, 10)); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/56256")] + public void MemberNotNull_LocalFunction_WithinInstanceMethod() + { + var c = CreateNullableCompilation([""" +using System.Diagnostics.CodeAnalysis; +public class C +{ + public string field1 = ""; + public string? field2; + public string field3 = ""; + public string? field4; + + public void M() + { + init(); + field1.ToString(); + field2.ToString(); + field3.ToString(); + field4.ToString(); // 1 + + [MemberNotNull(nameof(field1), nameof(field2))] + bool init() => throw null!; + } +} +""", MemberNotNullAttributeDefinition]); + + c.VerifyDiagnostics( + // (15,9): warning CS8602: Dereference of a possibly null reference. + // field4.ToString(); // 1 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "field4").WithLocation(15, 9)); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/56256")] + public void MemberNotNull_LocalFunction_WithinStaticMethod() + { + var c = CreateNullableCompilation([""" +using System.Diagnostics.CodeAnalysis; +public class C +{ + public static string field1 = ""; + public static string? field2; + public static string field3 = ""; + public static string? field4; + + public static void M() + { + init(); + field1.ToString(); + field2.ToString(); // 1 + field3.ToString(); + field4.ToString(); // 2 + + [MemberNotNull(nameof(field1), nameof(field2))] + bool init() => throw null!; + } +} +""", MemberNotNullAttributeDefinition]); + + c.VerifyDiagnostics( + // (13,9): warning CS8602: Dereference of a possibly null reference. + // field2.ToString(); // 1 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "field2").WithLocation(13, 9), + // (15,9): warning CS8602: Dereference of a possibly null reference. + // field4.ToString(); // 2 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "field4").WithLocation(15, 9)); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/56256")] + public void MemberNotNull_LocalFunction_WithinLambda() + { + var c = CreateNullableCompilation([""" +using System.Diagnostics.CodeAnalysis; +public class C +{ + public string field1; + public string? field2; + public string field3; + public string? field4; + + public C() + { + System.Action a = () => + { + init(); + + [MemberNotNull(nameof(field1), nameof(field2))] + void init() => throw null!; + }; + field1.ToString(); // 1 field2.ToString(); // 2 field3.ToString(); // 3 field4.ToString(); // 4 + } +} +""", MemberNotNullAttributeDefinition]); - [MemberNotNull(nameof(field1), nameof(field2))] - void init() => throw null!; + c.VerifyDiagnostics( + // (19,9): warning CS8602: Dereference of a possibly null reference. + // field1.ToString(); // 1 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "field1").WithLocation(19, 9), + // (20,9): warning CS8602: Dereference of a possibly null reference. + // field2.ToString(); // 2 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "field2").WithLocation(20, 9), + // (21,9): warning CS8602: Dereference of a possibly null reference. + // field3.ToString(); // 3 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "field3").WithLocation(21, 9), + // (22,9): warning CS8602: Dereference of a possibly null reference. + // field4.ToString(); // 4 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "field4").WithLocation(22, 9)); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/56256")] + public void MemberNotNull_LocalFunction_WithinAnonymousFunction() + { + var c = CreateNullableCompilation([""" +using System.Diagnostics.CodeAnalysis; +public class C +{ + public string field1; + public string? field2; + public string field3; + public string? field4; + + public C() + { + System.Action a = delegate() + { + init(); + + [MemberNotNull(nameof(field1), nameof(field2))] + void init() => throw null!; + }; + + field1.ToString(); // 1 + field2.ToString(); // 2 + field3.ToString(); // 3 + field4.ToString(); // 4 } } -", MemberNotNullAttributeDefinition }, parseOptions: TestOptions.Regular9); +""", MemberNotNullAttributeDefinition]); - // Note: the local function is not invoked on this or base c.VerifyDiagnostics( - // (13,9): warning CS8602: Dereference of a possibly null reference. + // (19,9): warning CS8602: Dereference of a possibly null reference. // field1.ToString(); // 1 - Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "field1").WithLocation(13, 9), - // (14,9): warning CS8602: Dereference of a possibly null reference. + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "field1").WithLocation(19, 9), + // (20,9): warning CS8602: Dereference of a possibly null reference. // field2.ToString(); // 2 - Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "field2").WithLocation(14, 9), - // (15,9): warning CS8602: Dereference of a possibly null reference. + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "field2").WithLocation(20, 9), + // (21,9): warning CS8602: Dereference of a possibly null reference. // field3.ToString(); // 3 - Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "field3").WithLocation(15, 9), - // (16,9): warning CS8602: Dereference of a possibly null reference. + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "field3").WithLocation(21, 9), + // (22,9): warning CS8602: Dereference of a possibly null reference. // field4.ToString(); // 4 - Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "field4").WithLocation(16, 9) - ); + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "field4").WithLocation(22, 9)); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/56256")] + public void MemberNotNull_LocalFunction_TopLevel() + { + var c = CreateCompilation([""" +#nullable enable +using System.Diagnostics.CodeAnalysis; + +init(); + +[MemberNotNull(nameof(field1))] +void init() => throw null!; +""", MemberNotNullAttributeDefinition]); + + c.VerifyDiagnostics( + // (6,23): error CS0103: The name 'field1' does not exist in the current context + // [MemberNotNull(nameof(field1))] + Diagnostic(ErrorCode.ERR_NameNotInContext, "field1").WithArguments("field1").WithLocation(6, 23)); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/56256")] + public void MemberNotNullWhenTrue_LocalFunction() + { + var c = CreateNullableCompilation([""" +using System.Diagnostics.CodeAnalysis; +public class C +{ + public string field1; + public string? field2; + public string field3; + public string? field4; + + public C() + { + if (init()) + { + field1.ToString(); + field2.ToString(); + field3.ToString(); // 1 + field4.ToString(); // 2 + } + else + { + field1.ToString(); // 3 + field2.ToString(); // 4 + field3.ToString(); // 5 + field4.ToString(); // 6 + } + + [MemberNotNullWhen(true, nameof(field1), nameof(field2))] + bool init() => throw null!; + } +} +""", MemberNotNullWhenAttributeDefinition]); + + c.VerifyDiagnostics( + // (15,13): warning CS8602: Dereference of a possibly null reference. + // field3.ToString(); // 1 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "field3").WithLocation(15, 13), + // (16,13): warning CS8602: Dereference of a possibly null reference. + // field4.ToString(); // 2 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "field4").WithLocation(16, 13), + // (20,13): warning CS8602: Dereference of a possibly null reference. + // field1.ToString(); // 3 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "field1").WithLocation(20, 13), + // (21,13): warning CS8602: Dereference of a possibly null reference. + // field2.ToString(); // 4 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "field2").WithLocation(21, 13), + // (22,13): warning CS8602: Dereference of a possibly null reference. + // field3.ToString(); // 5 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "field3").WithLocation(22, 13), + // (23,13): warning CS8602: Dereference of a possibly null reference. + // field4.ToString(); // 6 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "field4").WithLocation(23, 13)); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/56256")] + public void MemberNotNullWhenFalse_LocalFunction() + { + var c = CreateNullableCompilation([""" +using System.Diagnostics.CodeAnalysis; +public class C +{ + public string field1; + public string? field2; + public string field3; + public string? field4; + + public C() + { + if (init()) + { + field1.ToString(); // 1 + field2.ToString(); // 2 + field3.ToString(); // 3 + field4.ToString(); // 4 + } + else + { + field1.ToString(); + field2.ToString(); + field3.ToString(); // 5 + field4.ToString(); // 6 + } + + [MemberNotNullWhen(false, nameof(field1), nameof(field2))] + bool init() => throw null!; + } +} +""", MemberNotNullWhenAttributeDefinition]); + + c.VerifyDiagnostics( + // (13,13): warning CS8602: Dereference of a possibly null reference. + // field1.ToString(); // 1 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "field1").WithLocation(13, 13), + // (14,13): warning CS8602: Dereference of a possibly null reference. + // field2.ToString(); // 2 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "field2").WithLocation(14, 13), + // (15,13): warning CS8602: Dereference of a possibly null reference. + // field3.ToString(); // 3 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "field3").WithLocation(15, 13), + // (16,13): warning CS8602: Dereference of a possibly null reference. + // field4.ToString(); // 4 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "field4").WithLocation(16, 13), + // (22,13): warning CS8602: Dereference of a possibly null reference. + // field3.ToString(); // 5 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "field3").WithLocation(22, 13), + // (23,13): warning CS8602: Dereference of a possibly null reference. + // field4.ToString(); // 6 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "field4").WithLocation(23, 13)); } [Fact]