Skip to content

Commit 22a4ff3

Browse files
authored
Improve filtering of candidate binding invocations in config binder gen (#90619)
* Improve filtering of candidate binding invocations in config binder gen * Address feedback * Address feedback: rename helper; further tighten constraint * Add follow-up TODO * Revert TypeSyntax clause to fix failing tests
1 parent 82fdd0b commit 22a4ff3

File tree

3 files changed

+31
-46
lines changed

3 files changed

+31
-46
lines changed

src/libraries/Microsoft.Extensions.Configuration.Binder/gen/ConfigurationBindingGenerator.Parser.cs

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
using System.Diagnostics.CodeAnalysis;
88
using System.Linq;
99
using Microsoft.CodeAnalysis;
10-
using Microsoft.CodeAnalysis.Operations;
1110

1211
namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
1312
{
@@ -44,13 +43,10 @@ public Parser(SourceProductionContext context, KnownTypeSymbols typeSymbols, Imm
4443

4544
foreach (BinderInvocation invocation in _invocations)
4645
{
47-
IInvocationOperation invocationOperation = invocation.Operation!;
48-
if (!invocationOperation.TargetMethod.IsExtensionMethod)
49-
{
50-
continue;
51-
}
46+
IMethodSymbol targetMethod = invocation.Operation.TargetMethod;
47+
INamedTypeSymbol? candidateBinderType = targetMethod.ContainingType;
48+
Debug.Assert(targetMethod.IsExtensionMethod);
5249

53-
INamedTypeSymbol? candidateBinderType = invocationOperation.TargetMethod.ContainingType;
5450
if (SymbolEqualityComparer.Default.Equals(candidateBinderType, _typeSymbols.ConfigurationBinder))
5551
{
5652
RegisterMethodInvocation_ConfigurationBinder(invocation);

src/libraries/Microsoft.Extensions.Configuration.Binder/gen/ConfigurationBindingGenerator.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,11 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
3131
? new CompilationData((CSharpCompilation)compilation)
3232
: null);
3333

34-
IncrementalValuesProvider<BinderInvocation> inputCalls = context.SyntaxProvider
34+
IncrementalValuesProvider<BinderInvocation?> inputCalls = context.SyntaxProvider
3535
.CreateSyntaxProvider(
36-
(node, _) => node is InvocationExpressionSyntax invocation,
36+
(node, _) => BinderInvocation.IsCandidateSyntaxNode(node),
3737
BinderInvocation.Create)
38-
.Where(operation => operation is not null);
38+
.Where(invocation => invocation is not null);
3939

4040
IncrementalValueProvider<(CompilationData?, ImmutableArray<BinderInvocation>)> inputData = compilationData.Combine(inputCalls.Collect());
4141

@@ -59,7 +59,7 @@ private static void Execute(CompilationData compilationData, ImmutableArray<Bind
5959
}
6060

6161
Parser parser = new(context, compilationData.TypeSymbols!, inputCalls);
62-
if (parser.GetSourceGenerationSpec() is SourceGenerationSpec { } spec)
62+
if (parser.GetSourceGenerationSpec() is SourceGenerationSpec spec)
6363
{
6464
Emitter emitter = new(context, spec);
6565
emitter.Emit();

src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Helpers/Parser/BinderInvocation.cs

Lines changed: 24 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,74 +1,63 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4+
using System.Diagnostics;
45
using System.Threading;
56
using Microsoft.CodeAnalysis.CSharp.Syntax;
67
using Microsoft.CodeAnalysis.Operations;
78
using Microsoft.CodeAnalysis;
89

910
namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
1011
{
11-
internal sealed record BinderInvocation
12+
internal sealed record BinderInvocation(IInvocationOperation Operation, Location Location)
1213
{
13-
public IInvocationOperation Operation { get; private set; }
14-
public Location? Location { get; private set; }
15-
1614
public static BinderInvocation? Create(GeneratorSyntaxContext context, CancellationToken cancellationToken)
1715
{
18-
if (!IsCandidateInvocationExpressionSyntax(context.Node, out InvocationExpressionSyntax? invocationSyntax) ||
19-
context.SemanticModel.GetOperation(invocationSyntax, cancellationToken) is not IInvocationOperation operation ||
20-
!IsCandidateInvocation(operation))
21-
{
22-
return null;
23-
}
16+
Debug.Assert(IsCandidateSyntaxNode(context.Node));
17+
InvocationExpressionSyntax invocationSyntax = (InvocationExpressionSyntax)context.Node;
2418

25-
return new BinderInvocation()
26-
{
27-
Operation = operation,
28-
Location = invocationSyntax.GetLocation()
29-
};
19+
return context.SemanticModel.GetOperation(invocationSyntax, cancellationToken) is IInvocationOperation operation &&
20+
IsBindingOperation(operation)
21+
? new BinderInvocation(operation, invocationSyntax.GetLocation())
22+
: null;
3023
}
3124

32-
private static bool IsCandidateInvocationExpressionSyntax(SyntaxNode node, out InvocationExpressionSyntax? invocationSyntax)
25+
public static bool IsCandidateSyntaxNode(SyntaxNode node)
3326
{
34-
if (node is InvocationExpressionSyntax
35-
{
36-
Expression: MemberAccessExpressionSyntax
37-
{
38-
Name.Identifier.ValueText: string memberName
39-
}
40-
} syntax && IsCandidateBindingMethodName(memberName))
27+
return node is InvocationExpressionSyntax
4128
{
42-
invocationSyntax = syntax;
43-
return true;
44-
}
45-
46-
invocationSyntax = null;
47-
return false;
29+
// TODO: drill further into this evaluation for a declaring-type name check.
30+
// https://github.com/dotnet/runtime/issues/90687.
31+
Expression: MemberAccessExpressionSyntax
32+
{
33+
Name.Identifier.ValueText: string memberName,
34+
}
35+
} && IsCandidateBindingMethodName(memberName);
4836

4937
static bool IsCandidateBindingMethodName(string name) =>
5038
IsCandidateMethodName_ConfigurationBinder(name) ||
5139
IsCandidateMethodName_OptionsBuilderConfigurationExtensions(name) ||
5240
IsValidMethodName_OptionsConfigurationServiceCollectionExtensions(name);
5341
}
5442

55-
private static bool IsCandidateInvocation(IInvocationOperation operation)
43+
private static bool IsBindingOperation(IInvocationOperation operation)
5644
{
5745
if (operation.TargetMethod is not IMethodSymbol
5846
{
5947
IsExtensionMethod: true,
6048
Name: string methodName,
61-
ContainingType: ITypeSymbol
49+
ContainingType: INamedTypeSymbol
6250
{
6351
Name: string containingTypeName,
64-
ContainingNamespace: INamespaceSymbol { } containingNamespace,
65-
} containingType
66-
} method ||
67-
containingNamespace.ToDisplayString() is not string containingNamespaceName)
52+
ContainingNamespace: INamespaceSymbol containingNamespace,
53+
}
54+
})
6855
{
6956
return false;
7057
}
7158

59+
string containingNamespaceName = containingNamespace.ToDisplayString();
60+
7261
return (containingTypeName) switch
7362
{
7463
"ConfigurationBinder" =>

0 commit comments

Comments
 (0)