Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release/8.0-rc1] Improve filtering of candidate binding invocations in config binder gen #90746

Merged
merged 5 commits into from
Aug 17, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
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