Skip to content

Commit 6310b37

Browse files
RikkiGibsonRikkiGibson
authored
Nullable extensions: Add assertion to AsMemberOfType and handle failures (#79428)
1 parent e03909a commit 6310b37

File tree

4 files changed

+214
-33
lines changed

4 files changed

+214
-33
lines changed

‎src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs‎

Lines changed: 34 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -368,7 +368,13 @@ private void SetAnalyzedNullability(BoundExpression? expression, TypeWithState r
368368
/// </summary>
369369
private void SetAnalyzedNullability(BoundExpression? expr, VisitResult result, bool? isLvalue = null)
370370
{
371-
if (expr == null || _disableNullabilityAnalysis) return;
371+
if (expr == null
372+
// BoundExpressionWithNullability is not produced by the binder but is used within nullability analysis to pass information to internal components.
373+
|| expr.Kind == BoundKind.ExpressionWithNullability
374+
|| _disableNullabilityAnalysis)
375+
{
376+
return;
377+
}
372378

373379
#if DEBUG
374380
// https://github.com/dotnet/roslyn/issues/34993: This assert is essential for ensuring that we aren't
@@ -3901,7 +3907,7 @@ void visitElement(BoundNode element, BoundCollectionExpression node, TypeWithAnn
39013907
// Reinfer the addMethod signature and convert the argument to parameter type
39023908
var addMethod = initializer.AddMethod;
39033909
MethodSymbol reinferredAddMethod;
3904-
if (!addMethod.IsExtensionMethod)
3910+
if (!addMethod.IsExtensionMethod && !addMethod.GetIsNewExtensionMember())
39053911
{
39063912
reinferredAddMethod = (MethodSymbol)AsMemberOfType(targetCollectionType, addMethod);
39073913
}
@@ -4406,7 +4412,8 @@ void setAnalyzedNullabilityAsContinuation(
44064412
{
44074413
var symbol = objectInitializer.MemberSymbol;
44084414

4409-
if (symbol != null)
4415+
// https://github.com/dotnet/roslyn/issues/78828: adjust extension member based on new containing type
4416+
if (symbol != null && !symbol.GetIsNewExtensionMember())
44104417
{
44114418
Debug.Assert(TypeSymbol.Equals(objectInitializer.Type, GetTypeOrReturnType(symbol), TypeCompareKind.IgnoreNullableModifiersForReferenceTypes));
44124419
symbol = AsMemberOfType(containingType, symbol);
@@ -4611,7 +4618,7 @@ static MethodSymbol addMethodAsMemberOfContainingType(BoundCollectionElementInit
46114618
argumentResults = builder.ToImmutableAndFree();
46124619
}
46134620
}
4614-
else
4621+
else if (!method.GetIsNewExtensionMember())
46154622
{
46164623
// Tracked by https://github.com/dotnet/roslyn/issues/78828: Do we need to do anything special for new extensions here?
46174624
method = (MethodSymbol)AsMemberOfType(containingType, method);
@@ -7098,7 +7105,8 @@ private static bool HasImplicitTypeArguments(BoundNode node)
70987105
or BoundForEachStatement
70997106
or BoundPropertyAccess
71007107
or BoundIncrementOperator
7101-
or BoundCompoundAssignmentOperator)
7108+
or BoundCompoundAssignmentOperator
7109+
or BoundDagPropertyEvaluation)
71027110
{
71037111
return true;
71047112
}
@@ -8656,8 +8664,7 @@ private TypeWithState GetAdjustedResult(TypeWithState type, int slot)
86568664
private static Symbol AsMemberOfType(TypeSymbol? type, Symbol symbol)
86578665
{
86588666
Debug.Assert((object)symbol != null);
8659-
// https://github.com/dotnet/roslyn/issues/78828: This method should not be used with new extension members.
8660-
//Debug.Assert(!symbol.GetIsNewExtensionMember());
8667+
Debug.Assert(!symbol.GetIsNewExtensionMember(), symbol.ToDisplayString());
86618668

86628669
var containingType = type as NamedTypeSymbol;
86638670
if (containingType is null || containingType.IsErrorType() || symbol is ErrorMethodSymbol)
@@ -11326,6 +11333,25 @@ private TypeWithAnnotations GetDeclaredParameterResult(ParameterSymbol parameter
1132611333
return null;
1132711334
}
1132811335

11336+
private (PropertySymbol updatedProperty, bool returnNotNull) ReInferAndVisitExtensionPropertyAccess(BoundNode node, PropertySymbol property, BoundExpression receiver)
11337+
{
11338+
Debug.Assert(property.GetIsNewExtensionMember());
11339+
ImmutableArray<BoundExpression> arguments = [receiver];
11340+
11341+
var extensionParameter = property.ContainingType.ExtensionParameter;
11342+
Debug.Assert(extensionParameter is not null);
11343+
ImmutableArray<ParameterSymbol> parameters = [extensionParameter];
11344+
11345+
ImmutableArray<RefKind> refKindsOpt = extensionParameter.RefKind == RefKind.Ref ? [RefKind.Ref] : default;
11346+
11347+
// Tracked by https://github.com/dotnet/roslyn/issues/37238 : properties/indexers should account for NotNullIfNotNull
11348+
var (updatedProperty, _, returnNotNull) = VisitArguments(node, arguments, refKindsOpt, parameters, default, defaultArguments: default,
11349+
expanded: false, invokedAsExtensionMethod: false, property, firstArgumentResult: null);
11350+
11351+
Debug.Assert(updatedProperty is not null);
11352+
return (updatedProperty, returnNotNull);
11353+
}
11354+
1132911355
public override BoundNode? VisitPropertyAccess(BoundPropertyAccess node)
1133011356
{
1133111357
var property = node.PropertySymbol;
@@ -11334,20 +11360,7 @@ private TypeWithAnnotations GetDeclaredParameterResult(ParameterSymbol parameter
1133411360
if (property.GetIsNewExtensionMember())
1133511361
{
1133611362
Debug.Assert(node.ReceiverOpt is not null);
11337-
ImmutableArray<BoundExpression> arguments = [node.ReceiverOpt];
11338-
11339-
var extensionParameter = property.ContainingType.ExtensionParameter;
11340-
Debug.Assert(extensionParameter is not null);
11341-
ImmutableArray<ParameterSymbol> parameters = [extensionParameter];
11342-
11343-
ImmutableArray<RefKind> refKindsOpt = extensionParameter.RefKind == RefKind.Ref ? [RefKind.Ref] : default;
11344-
11345-
// Tracked by https://github.com/dotnet/roslyn/issues/37238 : properties/indexers should account for NotNullIfNotNull
11346-
bool returnNotNull;
11347-
(updatedProperty, _, returnNotNull) = VisitArguments(node, arguments, refKindsOpt, parameters, default, defaultArguments: default,
11348-
expanded: false, invokedAsExtensionMethod: false, property, firstArgumentResult: null);
11349-
11350-
Debug.Assert(updatedProperty is not null);
11363+
(updatedProperty, _) = ReInferAndVisitExtensionPropertyAccess(node, property, node.ReceiverOpt);
1135111364

1135211365
TypeWithAnnotations typeWithAnnotations = GetTypeOrReturnTypeWithAnnotations(updatedProperty);
1135311366
FlowAnalysisAnnotations memberAnnotations = GetRValueAnnotations(updatedProperty);

‎src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker_Patterns.cs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -531,7 +531,9 @@ public PossiblyConditionalState Clone()
531531
case BoundDagPropertyEvaluation e:
532532
{
533533
Debug.Assert(inputSlot > 0);
534-
var property = (PropertySymbol)AsMemberOfType(inputType, e.Property); // Tracked by https://github.com/dotnet/roslyn/issues/78828 : this needs to handle extension properties
534+
var property = e.Property.GetIsNewExtensionMember()
535+
? ReInferAndVisitExtensionPropertyAccess(e, e.Property, new BoundExpressionWithNullability(e.Syntax, expression, NullableAnnotation.NotAnnotated, inputType)).updatedProperty
536+
: (PropertySymbol)AsMemberOfType(inputType, e.Property);
535537
var type = property.TypeWithAnnotations;
536538
var output = new BoundDagTemp(e.Syntax, type.Type, e);
537539
int outputSlot = GetOrCreateSlot(property, inputSlot, forceSlotEvenIfEmpty: true);

‎src/Compilers/CSharp/Test/Emit3/Semantics/ExtensionTests.cs‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27761,7 +27761,7 @@ static class E
2776127761
Diagnostic(ErrorCode.ERR_NameofExtensionMember, "c.Property.Property").WithLocation(2, 29));
2776227762
}
2776327763

27764-
[Fact(Skip = "Assertion in NullableWalker.AsMemberOfType")] // Tracked by https://github.com/dotnet/roslyn/issues/78828 : Nullability analysis of properties
27764+
[Fact]
2776527765
public void Nameof_Instance_Property_Generic_01()
2776627766
{
2776727767
var src = """
@@ -27779,7 +27779,10 @@ static class E
2777927779
}
2778027780
""";
2778127781
var comp = CreateCompilation(src);
27782-
CompileAndVerify(comp, expectedOutput: "Property").VerifyDiagnostics();
27782+
comp.VerifyEmitDiagnostics(
27783+
// (2,29): error CS9316: Extension members are not allowed as an argument to 'nameof'.
27784+
// System.Console.Write(nameof(i.Property));
27785+
Diagnostic(ErrorCode.ERR_NameofExtensionMember, "i.Property").WithLocation(2, 29));
2778327786

2778427787
var tree = comp.SyntaxTrees.Single();
2778527788
var model = comp.GetSemanticModel(tree);

‎src/Compilers/CSharp/Test/Emit3/Semantics/ExtensionTests2.cs‎

Lines changed: 172 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -386,15 +386,8 @@ public static class E
386386
}
387387
}
388388
""";
389-
try
390-
{
391-
var comp = CreateCompilation([src, OverloadResolutionPriorityAttributeDefinition]);
392-
// Tracked by https://github.com/dotnet/roslyn/issues/78828 : assertion in NullableWalker
393-
CompileAndVerify(comp, expectedOutput: "42").VerifyDiagnostics();
394-
}
395-
catch (InvalidOperationException)
396-
{
397-
}
389+
var comp = CreateCompilation([src, OverloadResolutionPriorityAttributeDefinition]);
390+
CompileAndVerify(comp, expectedOutput: "42").VerifyDiagnostics();
398391
}
399392

400393
[Fact]
@@ -2282,6 +2275,176 @@ class C
22822275
Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "x2").WithLocation(7, 5));
22832276
}
22842277

2278+
[Fact]
2279+
public void Nullability_PropertyPattern_Reinference_01()
2280+
{
2281+
var source = """
2282+
#nullable enable
2283+
using System.Collections.Generic;
2284+
2285+
class Program
2286+
{
2287+
void M1(bool b)
2288+
{
2289+
var item = "a";
2290+
if (b)
2291+
{
2292+
item = null;
2293+
}
2294+
2295+
var list = M2(item)/*T:System.Collections.Generic.List<string?>!*/;
2296+
if (list is { First: var first })
2297+
{
2298+
first.ToString(); // 1
2299+
}
2300+
}
2301+
2302+
List<T> M2<T>(T item) => [item];
2303+
}
2304+
2305+
static class ListExtensions
2306+
{
2307+
extension<T>(List<T> list)
2308+
{
2309+
public T First => list[0];
2310+
}
2311+
}
2312+
""";
2313+
2314+
var comp = CreateCompilation(source);
2315+
comp.VerifyTypes();
2316+
comp.VerifyEmitDiagnostics(
2317+
// (17,13): warning CS8602: Dereference of a possibly null reference.
2318+
// first.ToString(); // 1
2319+
Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "first").WithLocation(17, 13));
2320+
}
2321+
2322+
[Fact]
2323+
public void Nullability_PropertyPattern_Reinference_02()
2324+
{
2325+
var source = """
2326+
#nullable enable
2327+
using System.Collections.Generic;
2328+
2329+
class Program
2330+
{
2331+
void M1(bool b)
2332+
{
2333+
var item = "a";
2334+
if (b)
2335+
{
2336+
item = null;
2337+
}
2338+
2339+
var list = M2(item)/*T:System.Collections.Generic.List<string?>!*/;
2340+
if (list is { First: var first }) // 1
2341+
{
2342+
first.ToString();
2343+
}
2344+
}
2345+
2346+
List<T> M2<T>(T item) => [item];
2347+
}
2348+
2349+
static class ListExtensions
2350+
{
2351+
extension(List<string> list)
2352+
{
2353+
public string First => list[0];
2354+
}
2355+
}
2356+
""";
2357+
2358+
var comp = CreateCompilation(source);
2359+
comp.VerifyTypes();
2360+
comp.VerifyEmitDiagnostics(
2361+
// (15,23): warning CS8620: Argument of type 'List<string?>' cannot be used for parameter 'list' of type 'List<string>' in 'extension(List<string>)' due to differences in the nullability of reference types.
2362+
// if (list is { First: var first }) // 1
2363+
Diagnostic(ErrorCode.WRN_NullabilityMismatchInArgument, "First").WithArguments("System.Collections.Generic.List<string?>", "System.Collections.Generic.List<string>", "list", "extension(List<string>)").WithLocation(15, 23));
2364+
}
2365+
2366+
[Fact]
2367+
public void Nullability_PropertyPattern_Reinference_03()
2368+
{
2369+
var source = """
2370+
#nullable enable
2371+
using System.Collections.Generic;
2372+
2373+
class Program
2374+
{
2375+
void M1(bool b)
2376+
{
2377+
var item = "a";
2378+
if (b)
2379+
{
2380+
item = null;
2381+
}
2382+
2383+
var list = M2(item)/*T:System.Collections.Generic.List<string?>!*/;
2384+
if (list is { First: var first }) // 1
2385+
{
2386+
first.ToString(); // 2
2387+
}
2388+
}
2389+
2390+
List<T> M2<T>(T item) => [item];
2391+
}
2392+
2393+
static class ListExtensions
2394+
{
2395+
extension<T>(List<T> list) where T : class
2396+
{
2397+
public T First => list[0];
2398+
}
2399+
}
2400+
""";
2401+
2402+
// Tracked by https://github.com/dotnet/roslyn/issues/78830 : diagnostic quality consider reporting a better containing symbol
2403+
var comp = CreateCompilation(source);
2404+
comp.VerifyTypes();
2405+
comp.VerifyEmitDiagnostics(
2406+
// (15,23): warning CS8634: The type 'string?' cannot be used as type parameter 'T' in the generic type or method 'ListExtensions.extension<T>(List<T>)'. Nullability of type argument 'string?' doesn't match 'class' constraint.
2407+
// if (list is { First: var first }) // 1
2408+
Diagnostic(ErrorCode.WRN_NullabilityMismatchInTypeParameterReferenceTypeConstraint, "First").WithArguments("ListExtensions.extension<T>(System.Collections.Generic.List<T>)", "T", "string?").WithLocation(15, 23),
2409+
// (17,13): warning CS8602: Dereference of a possibly null reference.
2410+
// first.ToString(); // 2
2411+
Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "first").WithLocation(17, 13));
2412+
}
2413+
2414+
[Fact]
2415+
public void Nullability_PropertyPattern_Postcondition_01()
2416+
{
2417+
var source = """
2418+
#nullable enable
2419+
using System.Diagnostics.CodeAnalysis;
2420+
2421+
class Program
2422+
{
2423+
void M(object? obj)
2424+
{
2425+
if (obj is { AsNotNull: var notNull })
2426+
notNull.ToString();
2427+
}
2428+
}
2429+
2430+
static class Extensions
2431+
{
2432+
extension(object? obj)
2433+
{
2434+
[NotNull]
2435+
public object? AsNotNull => obj!;
2436+
}
2437+
}
2438+
""";
2439+
2440+
var comp = CreateCompilation([source, NotNullAttributeDefinition]);
2441+
// Tracked by https://github.com/dotnet/roslyn/issues/78828 : should we extend member post-conditions to work with extension members?
2442+
comp.VerifyEmitDiagnostics(
2443+
// (9,13): warning CS8602: Dereference of a possibly null reference.
2444+
// notNull.ToString();
2445+
Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "notNull").WithLocation(9, 13));
2446+
}
2447+
22852448
[Fact]
22862449
public void WellKnownAttribute_Conditional()
22872450
{

0 commit comments

Comments
 (0)