Skip to content

Commit

Permalink
Respect [MemberNotNull] on local function
Browse files Browse the repository at this point in the history
  • Loading branch information
@jcouv
jcouv committed Oct 9, 2024
1 parent a656d1a commit f38640c
Show file tree
Hide file tree
Showing 2 changed files with 257 additions and 29 deletions.
28 changes: 18 additions & 10 deletions src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -7012,6 +7012,14 @@ private void ApplyMemberPostConditions(BoundExpression? receiverOpt, MethodSymbo
receiverOpt is null ? -1 :
MakeSlot(receiverOpt);

if (method.MethodKind == MethodKind.LocalFunction)
{
var topLevelMethod = GetTopLevelMethod(method);
Debug.Assert(topLevelMethod is not null);
var thisParameter = topLevelMethod.ThisParameter;
receiverSlot = thisParameter is { } ? GetOrCreateSlot(thisParameter) : -1;
}

if (receiverSlot < 0)
{
return;
Expand Down
258 changes: 239 additions & 19 deletions src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -20952,32 +20952,252 @@ public class C
public C()
{
init();
field1.ToString(); // 1
field2.ToString(); // 2
field3.ToString(); // 3
field4.ToString(); // 4
field1.ToString();
field2.ToString();
field3.ToString(); // 1
field4.ToString(); // 2

[MemberNotNull(nameof(field1), nameof(field2))]
void init() => throw null!;
}
}
", MemberNotNullAttributeDefinition }, parseOptions: TestOptions.Regular9);
""", MemberNotNullAttributeDefinition], parseOptions: TestOptions.Regular9);

// Note: the local function is not invoked on this or base
c.VerifyDiagnostics(
// (13,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.
// field2.ToString(); // 2
Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "field2").WithLocation(14, 9),
// field3.ToString(); // 1
Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "field3").WithLocation(14, 9),
// (15,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.
// field4.ToString(); // 4
Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "field4").WithLocation(16, 9)
);
// 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 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]
Expand Down

0 comments on commit f38640c

Please sign in to comment.