Skip to content

Commit

Permalink
[release/8.0-rc1] Improve filtering of candidate binding invocations …
Browse files Browse the repository at this point in the history
…in config binder gen (#90746)

* 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

---------

Co-authored-by: Layomi Akinrinade <laakinri@microsoft.com>
  • Loading branch information
github-actions[bot] and layomia authored Aug 17, 2023
1 parent 7788472 commit 702d02c
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 46 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Operations;

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

foreach (BinderInvocation invocation in _invocations)
{
IInvocationOperation invocationOperation = invocation.Operation!;
if (!invocationOperation.TargetMethod.IsExtensionMethod)
{
continue;
}
IMethodSymbol targetMethod = invocation.Operation.TargetMethod;
INamedTypeSymbol? candidateBinderType = targetMethod.ContainingType;
Debug.Assert(targetMethod.IsExtensionMethod);

INamedTypeSymbol? candidateBinderType = invocationOperation.TargetMethod.ContainingType;
if (SymbolEqualityComparer.Default.Equals(candidateBinderType, _typeSymbols.ConfigurationBinder))
{
RegisterMethodInvocation_ConfigurationBinder(invocation);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
? new CompilationData((CSharpCompilation)compilation)
: null);

IncrementalValuesProvider<BinderInvocation> inputCalls = context.SyntaxProvider
IncrementalValuesProvider<BinderInvocation?> inputCalls = context.SyntaxProvider
.CreateSyntaxProvider(
(node, _) => node is InvocationExpressionSyntax invocation,
(node, _) => BinderInvocation.IsCandidateSyntaxNode(node),
BinderInvocation.Create)
.Where(operation => operation is not null);
.Where(invocation => invocation is not null);

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

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

Parser parser = new(context, compilationData.TypeSymbols!, inputCalls);
if (parser.GetSourceGenerationSpec() is SourceGenerationSpec { } spec)
if (parser.GetSourceGenerationSpec() is SourceGenerationSpec spec)
{
Emitter emitter = new(context, spec);
emitter.Emit();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,74 +1,63 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics;
using System.Threading;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Operations;
using Microsoft.CodeAnalysis;

namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
{
internal sealed record BinderInvocation
internal sealed record BinderInvocation(IInvocationOperation Operation, Location Location)
{
public IInvocationOperation Operation { get; private set; }
public Location? Location { get; private set; }

public static BinderInvocation? Create(GeneratorSyntaxContext context, CancellationToken cancellationToken)
{
if (!IsCandidateInvocationExpressionSyntax(context.Node, out InvocationExpressionSyntax? invocationSyntax) ||
context.SemanticModel.GetOperation(invocationSyntax, cancellationToken) is not IInvocationOperation operation ||
!IsCandidateInvocation(operation))
{
return null;
}
Debug.Assert(IsCandidateSyntaxNode(context.Node));
InvocationExpressionSyntax invocationSyntax = (InvocationExpressionSyntax)context.Node;

return new BinderInvocation()
{
Operation = operation,
Location = invocationSyntax.GetLocation()
};
return context.SemanticModel.GetOperation(invocationSyntax, cancellationToken) is IInvocationOperation operation &&
IsBindingOperation(operation)
? new BinderInvocation(operation, invocationSyntax.GetLocation())
: null;
}

private static bool IsCandidateInvocationExpressionSyntax(SyntaxNode node, out InvocationExpressionSyntax? invocationSyntax)
public static bool IsCandidateSyntaxNode(SyntaxNode node)
{
if (node is InvocationExpressionSyntax
{
Expression: MemberAccessExpressionSyntax
{
Name.Identifier.ValueText: string memberName
}
} syntax && IsCandidateBindingMethodName(memberName))
return node is InvocationExpressionSyntax
{
invocationSyntax = syntax;
return true;
}

invocationSyntax = null;
return false;
// TODO: drill further into this evaluation for a declaring-type name check.
// https://github.com/dotnet/runtime/issues/90687.
Expression: MemberAccessExpressionSyntax
{
Name.Identifier.ValueText: string memberName,
}
} && IsCandidateBindingMethodName(memberName);

static bool IsCandidateBindingMethodName(string name) =>
IsCandidateMethodName_ConfigurationBinder(name) ||
IsCandidateMethodName_OptionsBuilderConfigurationExtensions(name) ||
IsValidMethodName_OptionsConfigurationServiceCollectionExtensions(name);
}

private static bool IsCandidateInvocation(IInvocationOperation operation)
private static bool IsBindingOperation(IInvocationOperation operation)
{
if (operation.TargetMethod is not IMethodSymbol
{
IsExtensionMethod: true,
Name: string methodName,
ContainingType: ITypeSymbol
ContainingType: INamedTypeSymbol
{
Name: string containingTypeName,
ContainingNamespace: INamespaceSymbol { } containingNamespace,
} containingType
} method ||
containingNamespace.ToDisplayString() is not string containingNamespaceName)
ContainingNamespace: INamespaceSymbol containingNamespace,
}
})
{
return false;
}

string containingNamespaceName = containingNamespace.ToDisplayString();

return (containingTypeName) switch
{
"ConfigurationBinder" =>
Expand Down

0 comments on commit 702d02c

Please sign in to comment.