Skip to content
Draft
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
50 changes: 42 additions & 8 deletions src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4576,25 +4576,57 @@ conversionCompletion is not null ?
ImmutableArray<VisitResult> argumentResults = default;
MethodSymbol addMethod = addMethodAsMemberOfContainingType(node, containingType, ref argumentResults);

bool adjustForNewExtension = addMethod.IsExtensionBlockMember() && !node.InvokedAsExtensionMethod;
var arguments = getArguments(node.Arguments, adjustForNewExtension, node.ImplicitReceiverOpt, containingType);
var parameters = getParameters(addMethod.Parameters, adjustForNewExtension, addMethod);
var argsToParamsOpt = GetArgsToParamsOpt(node.ArgsToParamsOpt, adjustForNewExtension);
var invokedAsExtensionMethod = node.InvokedAsExtensionMethod || adjustForNewExtension;

// Note: we analyze even omitted calls
(MethodSymbol? reinferredMethod,
argumentResults,
_,
ArgumentsCompletionDelegate<MethodSymbol>? visitArgumentsCompletion) =
VisitArguments(
node,
node.Arguments,
arguments,
refKindsOpt: default,
addMethod.Parameters,
node.ArgsToParamsOpt,
parameters,
argsToParamsOpt,
node.DefaultArguments,
node.Expanded,
node.InvokedAsExtensionMethod,
invokedAsExtensionMethod,
addMethod,
delayCompletionForTargetMember: delayCompletionForType);

static ImmutableArray<BoundExpression> getArguments(ImmutableArray<BoundExpression> arguments, bool isNewExtension, BoundExpression? receiver, TypeSymbol containingType)
{
if (isNewExtension)
{
// For implicit extension member access methods, the receiver is not in the arguments array,
// but we need to include it for nullable analysis
var receiverExpression = receiver ?? new BoundObjectOrCollectionValuePlaceholder(receiver?.Syntax ?? arguments[0].Syntax, isNewInstance: false, containingType) { WasCompilerGenerated = true };
return [receiverExpression, .. arguments];
}

return arguments;
}

static ImmutableArray<ParameterSymbol> getParameters(ImmutableArray<ParameterSymbol> parameters, bool isNewExtension, MethodSymbol method)
{
if (!isNewExtension)
{
return parameters;
}

ParameterSymbol? extensionParameter = method.ContainingType.ExtensionParameter;
Debug.Assert(extensionParameter is not null);

return [extensionParameter, .. parameters];
}

#if DEBUG
if (node.InvokedAsExtensionMethod)
if (node.InvokedAsExtensionMethod || adjustForNewExtension)
{
VisitResult receiverResult = argumentResults[0];
Debug.Assert(TypeSymbol.Equals(containingType, receiverResult.RValueType.Type, TypeCompareKind.IgnoreNullableModifiersForReferenceTypes));
Expand Down Expand Up @@ -4640,8 +4672,11 @@ conversionCompletion is not null ?
{
MethodSymbol addMethod = addMethodAsMemberOfContainingType(node, containingType, ref argumentResults);

bool adjustForNewExtension = addMethod.IsExtensionBlockMember() && !node.InvokedAsExtensionMethod;
var parameters = getParameters(addMethod.Parameters, adjustForNewExtension, addMethod);

setUpdatedSymbol(
node, containingType, visitArgumentsCompletion.Invoke(argumentResults, addMethod.Parameters, addMethod).member,
node, containingType, visitArgumentsCompletion.Invoke(argumentResults, parameters, addMethod).member,
argumentResults, visitArgumentsCompletion: null, delayCompletionForType: false);
};
}
Expand All @@ -4650,7 +4685,7 @@ static MethodSymbol addMethodAsMemberOfContainingType(BoundCollectionElementInit
{
var method = node.AddMethod;

if (node.InvokedAsExtensionMethod)
if (node.InvokedAsExtensionMethod || (method.IsExtensionBlockMember() && !node.InvokedAsExtensionMethod))
{
if (!argumentResults.IsDefault)
{
Expand All @@ -4671,7 +4706,6 @@ static MethodSymbol addMethodAsMemberOfContainingType(BoundCollectionElementInit
}
else if (!method.IsExtensionBlockMember())
{
// Tracked by https://github.com/dotnet/roslyn/issues/78828: Do we need to do anything special for new extensions here?
method = (MethodSymbol)AsMemberOfType(containingType, method);
}

Expand Down
81 changes: 81 additions & 0 deletions src/Compilers/CSharp/Test/Emit3/Semantics/ExtensionTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -52008,4 +52008,85 @@ public static async System.Threading.Tasks.Task M2()
var comp = CreateCompilation(source);
CompileAndVerify(comp, expectedOutput: "True ran ran2").VerifyDiagnostics();
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/78828")]
public void Nullability_CollectionInitializer_WithImplicitExtensionMemberAccess()
Copy link
Member

Choose a reason for hiding this comment

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

@copilot it looks like the scenario covered by this test is covered just as well by Nullability_CollectionInitializer_WithImplicitExtensionMemberAccess_Nullable. Please delete this test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Deleted in commit 93c622a.

{
var src = """
#nullable enable
using System.Collections.Generic;

class Program
{
static void Main()
{
IEnumerable<string> items = new List<string>();
_ = new C()
{
Items = { items } // should not warn
};
}
}

class C
{
public List<string> Items { get; set; } = new List<string>();
}

static class Ext
{
extension<T>(List<T> list)
{
public void Add(IEnumerable<T> values) => list.AddRange(values);
}
}
""";
var comp = CreateCompilation(src);
comp.VerifyEmitDiagnostics();
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/78828")]
public void Nullability_CollectionInitializer_WithImplicitExtensionMemberAccess_Nullable()
{
var src = """
#nullable enable
using System.Collections.Generic;

class Program
{
static void Main()
{
IEnumerable<string>? itemsNull = null;
_ = new C()
{
Items = { itemsNull } // should warn CS8604
};

IEnumerable<string> itemsNotNull = new List<string>();
_ = new C()
{
Items = { itemsNotNull } // should not warn
};
}
}

class C
{
public List<string> Items { get; set; } = new List<string>();
}

static class Ext
{
extension<T>(List<T> list)
{
public void Add(IEnumerable<T> values) => list.AddRange(values);
}
}
""";
var comp = CreateCompilation(src);
comp.VerifyEmitDiagnostics(
// (11,23): warning CS8604: Possible null reference argument for parameter 'values' in 'void Ext.extension<string>(List<string>).Add(IEnumerable<string> values)'.
// Items = { itemsNull } // should warn CS8604
Diagnostic(ErrorCode.WRN_NullReferenceArgument, "itemsNull").WithArguments("values", "void Ext.extension<string>(List<string>).Add(IEnumerable<string> values)").WithLocation(11, 23));
}
}
Loading