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

* 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>

diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/ConfigurationBindingGenerator.Parser.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/ConfigurationBindingGenerator.Parser.cs
index 7583119..b0f53ef 100644 (file)
--- a/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/ConfigurationBindingGenerator.Parser.cs
+++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/ConfigurationBindingGenerator.Parser.cs
@@ -7,7 +7,6 @@ using System.Diagnostics;
 using System.Diagnostics.CodeAnalysis;
 using System.Linq;
 using Microsoft.CodeAnalysis;
-using Microsoft.CodeAnalysis.Operations;
 
 namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
 {
@@ -44,13 +43,10 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
 
                 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);

diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/ConfigurationBindingGenerator.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/ConfigurationBindingGenerator.cs
index 5d7a830..70da582 100644 (file)
--- a/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/ConfigurationBindingGenerator.cs
+++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/ConfigurationBindingGenerator.cs
@@ -31,11 +31,11 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
                         ? 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());
 
@@ -59,7 +59,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
             }
 
             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();

diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Helpers/Parser/BinderInvocation.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Helpers/Parser/BinderInvocation.cs
index 3029a8d..5547865 100644 (file)
--- a/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Helpers/Parser/BinderInvocation.cs
+++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Helpers/Parser/BinderInvocation.cs
@@ -1,6 +1,7 @@
 // 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;
@@ -8,43 +9,30 @@ 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) ||
@@ -52,23 +40,24 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
                 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" =>