Extensions: address or split remaining open issues directly associated with test plan (#79452)

Author: jcouv

src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs

@@ -92,9 +92,9 @@ internal MethodInfo ReplaceWithExtensionImplementation(out bool wasError)
                 var setMethod = replace(SetMethod);
                 Symbol symbol = ReferenceEquals(Symbol, Method) && method is not null ? method : Symbol;
 
+                Debug.Assert(SetMethod?.GetIsNewExtensionMember() != true);
                 wasError = (Method is not null && method is null) || (SetMethod is not null && setMethod is null);
 
-                // Tracked by https://github.com/dotnet/roslyn/issues/76130 : Test in compound assignment (ie. "setMethod")
                 return new MethodInfo(symbol, method, setMethod);
 
                 static MethodSymbol? replace(MethodSymbol? method)
@@ -110,8 +110,7 @@ internal MethodInfo ReplaceWithExtensionImplementation(out bool wasError)
                             ConstructIfGeneric(method.ContainingType.TypeArgumentsWithAnnotationsNoUseSiteDiagnostics.Concat(method.TypeArgumentsWithAnnotations));
                     }
 
-                    // Tracked by https://github.com/dotnet/roslyn/issues/76130 : Test this code path
-                    return null;
+                    throw ExceptionUtilities.Unreachable(); // we don't bind to extension methods with unsupported metadata
                 }
             }
 
@@ -615,7 +614,7 @@ private BoundExpression CheckValue(BoundExpression expr, BindValueKind valueKind
             {
                 var methodGroup = (BoundMethodGroup)expr;
                 CompoundUseSiteInfo<AssemblySymbol> useSiteInfo = GetNewCompoundUseSiteInfo(diagnostics);
-                var resolution = this.ResolveMethodGroup(methodGroup, analyzedArguments: null, useSiteInfo: ref useSiteInfo, options: OverloadResolution.Options.None);
+                var resolution = this.ResolveMethodGroup(methodGroup, analyzedArguments: null, useSiteInfo: ref useSiteInfo, options: OverloadResolution.Options.None, acceptOnlyMethods: true);
                 Debug.Assert(!resolution.IsNonMethodExtensionMember(out _));
                 diagnostics.Add(expr.Syntax, useSiteInfo);
                 Symbol otherSymbol = null;

src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs

@@ -7970,7 +7970,7 @@ private BoundExpression MakeMemberAccessValue(BoundExpression expr, BindingDiagn
                     {
                         var methodGroup = (BoundMethodGroup)expr;
                         CompoundUseSiteInfo<AssemblySymbol> useSiteInfo = GetNewCompoundUseSiteInfo(diagnostics);
-                        var resolution = this.ResolveMethodGroup(methodGroup, analyzedArguments: null, useSiteInfo: ref useSiteInfo, options: OverloadResolution.Options.None);
+                        var resolution = this.ResolveMethodGroup(methodGroup, analyzedArguments: null, useSiteInfo: ref useSiteInfo, options: OverloadResolution.Options.None, acceptOnlyMethods: true);
                         Debug.Assert(!resolution.IsNonMethodExtensionMember(out _));
                         diagnostics.Add(expr.Syntax, useSiteInfo);
                         if (!expr.HasAnyErrors)
@@ -8851,60 +8851,6 @@ private bool AllowRefOmittedArguments(BoundExpression receiver)
         }
 #nullable disable
 
-        private void PopulateExtensionMethodsFromSingleBinder(
-            ExtensionScope scope,
-            MethodGroup methodGroup,
-            SyntaxNode node,
-            BoundExpression left,
-            string rightName,
-            ImmutableArray<TypeWithAnnotations> typeArgumentsWithAnnotations,
-            BindingDiagnosticBag diagnostics)
-        {
-            int arity;
-            if (typeArgumentsWithAnnotations.IsDefault)
-            {
-                arity = 0;
-            }
-            else
-            {
-                arity = typeArgumentsWithAnnotations.Length;
-            }
-
-            var lookupResult = LookupResult.GetInstance();
-            CompoundUseSiteInfo<AssemblySymbol> useSiteInfo = GetNewCompoundUseSiteInfo(diagnostics);
-            this.LookupExtensionMethods(lookupResult, scope, rightName, arity, ref useSiteInfo);
-            diagnostics.Add(node, useSiteInfo);
-
-            if (lookupResult.IsMultiViable)
-            {
-                Debug.Assert(lookupResult.Symbols.Any());
-                var members = ArrayBuilder<Symbol>.GetInstance();
-                bool wasError;
-                Symbol symbol = GetSymbolOrMethodOrPropertyGroup(lookupResult, node, rightName, arity, members, diagnostics, out wasError, qualifierOpt: null);
-                Debug.Assert((object)symbol == null);
-                Debug.Assert(members.Count > 0);
-                methodGroup.PopulateWithExtensionMethods(left, members, typeArgumentsWithAnnotations, lookupResult.Kind);
-                members.Free();
-            }
-
-            lookupResult.Free();
-        }
-
-        private void LookupExtensionMethods(LookupResult lookupResult, ExtensionScope scope, string rightName, int arity, ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
-        {
-            LookupOptions options;
-            if (arity == 0)
-            {
-                options = LookupOptions.AllMethodsOnArityZero;
-            }
-            else
-            {
-                options = LookupOptions.Default;
-            }
-
-            this.LookupExtensionMethodsInSingleBinder(scope, lookupResult, rightName, arity, options, ref useSiteInfo);
-        }
-
         protected BoundExpression BindFieldAccess(
             SyntaxNode node,
             BoundExpression receiver,
@@ -10520,7 +10466,8 @@ void makeCall(SyntaxNode syntax, BoundExpression receiver, MethodSymbol method,
                 indexerOrSliceAccess = BindMethodGroupInvocation(syntax, syntax, method.Name, boundMethodGroup, analyzedArguments,
                     diagnostics, queryClause: null, ignoreNormalFormIfHasValidParamsParameter: true, anyApplicableCandidates: out bool _,
                     disallowExpandedNonArrayParams: false,
-                    acceptOnlyMethods: true).MakeCompilerGenerated(); // Tracked by https://github.com/dotnet/roslyn/issues/76130 : Test effect of acceptOnlyMethods value
+                    acceptOnlyMethods: true) // acceptOnlyMethods is not relevant since we won't search extensions
+                    .MakeCompilerGenerated();
 
                 analyzedArguments.Free();
             }
@@ -10638,6 +10585,7 @@ internal MethodGroupResolution ResolveMethodGroup(
             AnalyzedArguments analyzedArguments,
             ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo,
             OverloadResolution.Options options,
+            bool acceptOnlyMethods,
             RefKind returnRefKind = default,
             TypeSymbol returnType = null,
             in CallingConventionInfo callingConventionInfo = default)
@@ -10649,7 +10597,7 @@ internal MethodGroupResolution ResolveMethodGroup(
             return ResolveMethodGroup(
                 node, node.Syntax, node.Name, analyzedArguments, ref useSiteInfo,
                 options,
-                acceptOnlyMethods: true, // Tracked by https://github.com/dotnet/roslyn/issues/76130 : Confirm this value is appropriate for all consumers of the enclosing method and test effect of this value for all of them
+                acceptOnlyMethods: acceptOnlyMethods,
                 returnRefKind: returnRefKind, returnType: returnType,
                 callingConventionInfo: callingConventionInfo);
         }
@@ -10900,22 +10848,50 @@ private MethodGroupResolution ResolveDefaultMethodGroup(
                 }
             }
 
-            if (node.ReceiverOpt is not BoundTypeExpression && node.SearchExtensions)
+            if (node.SearchExtensions)
             {
-                var receiver = node.ReceiverOpt!;
+                Debug.Assert(node.ReceiverOpt!.Type is not null); // extensions are only considered on member access
+
+                BoundExpression receiver = node.ReceiverOpt;
+                ImmutableArray<TypeWithAnnotations> typeArguments = node.TypeArgumentsOpt;
+                int arity = typeArguments.IsDefaultOrEmpty ? 0 : typeArguments.Length;
+                LookupOptions options = arity == 0 ? LookupOptions.AllMethodsOnArityZero : LookupOptions.Default;
+                var singleLookupResults = ArrayBuilder<SingleLookupResult>.GetInstance();
+                CompoundUseSiteInfo<AssemblySymbol> discardedUseSiteInfo = CompoundUseSiteInfo<AssemblySymbol>.Discarded;
+
                 foreach (var scope in new ExtensionScopes(this))
                 {
                     methods.Clear();
-                    var methodGroup = MethodGroup.GetInstance();
-                    PopulateExtensionMethodsFromSingleBinder(scope, methodGroup, node.Syntax, receiver, node.Name, node.TypeArgumentsOpt, BindingDiagnosticBag.Discarded);
-                    foreach (var m in methodGroup.Methods)
+                    singleLookupResults.Clear();
+                    scope.Binder.EnumerateAllExtensionMembersInSingleBinder(singleLookupResults, node.Name, arity, options, originalBinder: this, ref discardedUseSiteInfo, ref discardedUseSiteInfo);
+
+                    foreach (SingleLookupResult singleLookupResult in singleLookupResults)
                     {
-                        if (m.ReduceExtensionMethod(receiver.Type, Compilation) is { } reduced)
+                        Symbol extensionMember = singleLookupResult.Symbol;
+                        if (IsStaticInstanceMismatchForUniqueSignatureFromMethodGroup(receiver, extensionMember))
                         {
-                            methods.Add(reduced);
+                            // Remove static/instance mismatches
+                            continue;
+                        }
+
+                        // Note: we only care about methods. If the expression resolved to a non-method extension member, we wouldn't get here to compute the function type for the expression.
+                        if (extensionMember is MethodSymbol m)
+                        {
+                            if (m.GetIsNewExtensionMember())
+                            {
+                                // Note: new extension methods are subject to more stringent checks
+                                var substituted = (MethodSymbol?)extensionMember.GetReducedAndFilteredSymbol(typeArguments, receiver.Type, Compilation, checkFullyInferred: true);
+                                if (substituted is not null)
+                                {
+                                    methods.Add(substituted);
+                                }
+                            }
+                            else if (m.ReduceExtensionMethod(receiver.Type, Compilation) is { } reduced)
+                            {
+                                methods.Add(reduced);
+                            }
                         }
                     }
-                    methodGroup.Free();
 
                     if (methods.Count == 0)
                     {
@@ -10926,6 +10902,7 @@ private MethodGroupResolution ResolveDefaultMethodGroup(
                     {
                         methods.Free();
                         useParams = false;
+                        singleLookupResults.Free();
                         return null;
                     }
 
@@ -10936,6 +10913,7 @@ private MethodGroupResolution ResolveDefaultMethodGroup(
                     {
                         methods.Free();
                         useParams = false;
+                        singleLookupResults.Free();
                         return null;
                     }
 
@@ -10948,10 +10926,13 @@ private MethodGroupResolution ResolveDefaultMethodGroup(
                         {
                             methods.Free();
                             useParams = false;
+                            singleLookupResults.Free();
                             return null;
                         }
                     }
                 }
+
+                singleLookupResults.Free();
             }
 
             methods.Free();
@@ -10989,6 +10970,17 @@ static bool isCandidateUnique(ref MethodSymbol? method, MethodSymbol candidate)
             }
         }
 
+        private static bool IsStaticInstanceMismatchForUniqueSignatureFromMethodGroup(BoundExpression receiver, Symbol extensionMember)
+        {
+            bool memberCountsAsStatic = extensionMember is MethodSymbol { IsExtensionMethod: true } ? false : extensionMember.IsStatic;
+            return receiver switch
+            {
+                BoundTypeOrValueExpression => false,
+                BoundTypeExpression => !memberCountsAsStatic,
+                _ => memberCountsAsStatic,
+            };
+        }
+
         /// <summary>
         /// For C# 13 onwards, returns one of the methods from the method group if all instance methods, or extension methods
         /// in the nearest scope, have the same signature ignoring parameter names and custom modifiers.
@@ -11088,23 +11080,15 @@ static bool isCandidateUnique(ref MethodSymbol? method, MethodSymbol candidate)
                     var methods = ArrayBuilder<MethodSymbol>.GetInstance(capacity: singleLookupResults.Count);
                     foreach (SingleLookupResult singleLookupResult in singleLookupResults)
                     {
-                        // Remove static/instance mismatches
                         Symbol extensionMember = singleLookupResult.Symbol;
-                        bool memberCountsAsStatic = extensionMember is MethodSymbol { IsExtensionMethod: true } ? false : extensionMember.IsStatic;
-                        switch (node.ReceiverOpt)
+                        if (IsStaticInstanceMismatchForUniqueSignatureFromMethodGroup(receiver, extensionMember))
                         {
-                            case BoundTypeOrValueExpression:
-                                break;
-                            case BoundTypeExpression:
-                                if (!memberCountsAsStatic) continue;
-                                break;
-                            default:
-                                if (memberCountsAsStatic) continue;
-                                break;
+                            // Remove static/instance mismatches
+                            continue;
                         }
 
                         // Note: we only care about methods since we're already decided this is a method group (ie. not resolving to some other kind of extension member)
-                        if (extensionMember is MethodSymbol method)
+                        if (extensionMember is MethodSymbol)
                         {
                             var substituted = (MethodSymbol?)extensionMember.GetReducedAndFilteredSymbol(typeArguments, receiver.Type, Compilation, checkFullyInferred: true);
                             if (substituted is not null)

src/Compilers/CSharp/Portable/Binder/Binder_Invocation.cs

@@ -2069,7 +2069,7 @@ private ImmutableArray<BoundExpression> BuildArgumentsForErrorRecovery(AnalyzedA
             var parameterListList = ArrayBuilder<ImmutableArray<ParameterSymbol>>.GetInstance();
             foreach (var p in properties)
             {
-                // Tracked by https://github.com/dotnet/roslyn/issues/76130: Revisit this with new extensions
+                Debug.Assert(!p.GetIsNewExtensionMember());
                 if (p.ParameterCount > 0)
                 {
                     parameterListList.Add(p.Parameters);
@@ -2353,7 +2353,7 @@ private void EnsureNameofExpressionSymbols(BoundMethodGroup methodGroup, Binding
         {
             // Check that the method group contains something applicable. Otherwise error.
             CompoundUseSiteInfo<AssemblySymbol> useSiteInfo = GetNewCompoundUseSiteInfo(diagnostics);
-            var resolution = ResolveMethodGroup(methodGroup, analyzedArguments: null, useSiteInfo: ref useSiteInfo, options: OverloadResolution.Options.None);
+            var resolution = ResolveMethodGroup(methodGroup, analyzedArguments: null, useSiteInfo: ref useSiteInfo, options: OverloadResolution.Options.None, acceptOnlyMethods: true);
             Debug.Assert(!resolution.IsNonMethodExtensionMember(out _));
 
             diagnostics.Add(methodGroup.Syntax, useSiteInfo);

src/Compilers/CSharp/Portable/Binder/Binder_Lookup.cs

@@ -525,28 +525,6 @@ private static void LookupMembersInNamespace(LookupResult result, NamespaceSymbo
             }
         }
 
-        // Tracked by https://github.com/dotnet/roslyn/issues/76130 : we should be able to remove this method once all the callers are updated to account for new extension members
-        /// <summary>
-        /// Lookup extension methods by name and arity in the given binder and
-        /// check viability in this binder. The lookup is performed on a single
-        /// binder because extension method search stops at the first applicable
-        /// method group from the nearest enclosing namespace.
-        /// </summary>
-        private void LookupExtensionMethodsInSingleBinder(ExtensionScope scope, LookupResult result, string name, int arity, LookupOptions options, ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
-        {
-            var methods = ArrayBuilder<MethodSymbol>.GetInstance();
-            var binder = scope.Binder;
-            binder.GetCandidateExtensionMethods(methods, name, arity, options, this);
-
-            foreach (var method in methods)
-            {
-                SingleLookupResult resultOfThisMember = this.CheckViability(method, arity, options, null, diagnose: true, useSiteInfo: ref useSiteInfo);
-                result.MergeEqual(resultOfThisMember);
-            }
-
-            methods.Free();
-        }
-
         #region "AttributeTypeLookup"
 
         /// <summary>

src/Compilers/CSharp/Portable/Binder/Binder_Query.cs

@@ -676,7 +676,7 @@ private void ReduceFrom(FromClauseSyntax from, QueryTranslationState state, Bind
 
         private static BoundExpression? ExtractCastInvocation(BoundCall invocation)
         {
-            int index = invocation.InvokedAsExtensionMethod ? 1 : 0; // Tracked by https://github.com/dotnet/roslyn/issues/76130: Add test coverage for his code path
+            int index = invocation.InvokedAsExtensionMethod ? 1 : 0;
             var c1 = invocation.Arguments[index] as BoundConversion;
             var l1 = c1 != null ? c1.Operand as BoundLambda : null;
             var r1 = l1 != null ? l1.Body.Statements[0] as BoundReturnStatement : null;

src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/Conversions.cs

@@ -258,13 +258,13 @@ private static MethodGroupResolution ResolveDelegateOrFunctionPointerMethodGroup
                 resolution = binder.ResolveMethodGroup(source, analyzedArguments, useSiteInfo: ref useSiteInfo,
                     options: OverloadResolution.Options.InferWithDynamic | OverloadResolution.Options.IsMethodGroupConversion |
                              (isFunctionPointer ? OverloadResolution.Options.IsFunctionPointerResolution : OverloadResolution.Options.None),
-                    returnRefKind: delegateInvokeMethodOpt.RefKind, returnType: delegateInvokeMethodOpt.ReturnType,
+                    acceptOnlyMethods: true, returnRefKind: delegateInvokeMethodOpt.RefKind, returnType: delegateInvokeMethodOpt.ReturnType,
                     callingConventionInfo: callingConventionInfo);
                 analyzedArguments.Free();
             }
             else
             {
-                resolution = binder.ResolveMethodGroup(source, analyzedArguments: null, ref useSiteInfo, options: OverloadResolution.Options.IsMethodGroupConversion);
+                resolution = binder.ResolveMethodGroup(source, analyzedArguments: null, useSiteInfo: ref useSiteInfo, options: OverloadResolution.Options.IsMethodGroupConversion, acceptOnlyMethods: true);
             }
 
             Debug.Assert(!resolution.IsNonMethodExtensionMember(out _));

src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/MethodTypeInference.cs

@@ -1513,7 +1513,7 @@ private TypeWithAnnotations MethodGroupReturnType(
             var resolution = binder.ResolveMethodGroup(source, analyzedArguments, useSiteInfo: ref useSiteInfo,
                 options: OverloadResolution.Options.IsMethodGroupConversion |
                          (isFunctionPointerResolution ? OverloadResolution.Options.IsFunctionPointerResolution : OverloadResolution.Options.None),
-                returnRefKind: delegateRefKind,
+                acceptOnlyMethods: true, returnRefKind: delegateRefKind,
                 // Since we are trying to infer the return type, it is not an input to resolving the method group
                 returnType: null,
                 callingConventionInfo: in callingConventionInfo);