Extensions: bind unconditionally of LangVer

Author: Julien Couvreur

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

@@ -749,6 +749,19 @@ internal static void ReportDiagnosticsIfObsoleteInternal(BindingDiagnosticBag di
             }
         }
 
+        internal static bool IsDisallowedExtensionInOlderLangVer(MethodSymbol symbol)
+        {
+            return symbol.GetIsNewExtensionMember() && symbol.IsStatic;
+        }
+
+        internal static void ReportDiagnosticsIfDisallowedExtension(BindingDiagnosticBag diagnostics, MethodSymbol method, SyntaxNode syntax)
+        {
+            if (IsDisallowedExtensionInOlderLangVer(method))
+            {
+                MessageID.IDS_FeatureExtensions.CheckFeatureAvailability(diagnostics, syntax);
+            }
+        }
+
         internal static void ReportDiagnosticsIfUnmanagedCallersOnly(BindingDiagnosticBag diagnostics, MethodSymbol symbol, SyntaxNodeOrToken syntax, bool isDelegateConversion)
         {
             var unmanagedCallersOnlyAttributeData = symbol.GetUnmanagedCallersOnlyAttributeData(forceComplete: false);

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

@@ -1069,6 +1069,7 @@ private bool HasCollectionInitializerTypeInProgress(SyntaxNode syntax, TypeSymbo
             ReportDiagnosticsIfObsolete(diagnostics, collectionBuilderMethod.ContainingType, syntax, hasBaseReceiver: false);
             ReportDiagnosticsIfObsolete(diagnostics, collectionBuilderMethod, syntax, hasBaseReceiver: false);
             ReportDiagnosticsIfUnmanagedCallersOnly(diagnostics, collectionBuilderMethod, syntax, isDelegateConversion: false);
+            Debug.Assert(!collectionBuilderMethod.GetIsNewExtensionMember());
 
             return collectionBuilderMethod;
         }
@@ -1430,15 +1431,21 @@ static bool bindMethodGroupInvocation(
                                 {
                                     addMethodBinder.ReportDiagnosticsIfObsolete(diagnostics, addMethods[0], syntax, hasBaseReceiver: false);
                                     ReportDiagnosticsIfUnmanagedCallersOnly(diagnostics, addMethods[0], syntax, isDelegateConversion: false);
+                                    Debug.Assert(!IsDisallowedExtensionInOlderLangVer(addMethods[0]));
                                 }
                             }
                         }
                         else
                         {
-                            result = bindInvocationExpressionContinued(
-                                addMethodBinder, syntax, expression, resolution.OverloadResolutionResult, resolution.AnalyzedArguments,
-                                resolution.MethodGroup, diagnostics: diagnostics, out var addMethod);
-                            addMethods = addMethod is null ? [] : [addMethod];
+                            Debug.Assert(!resolution.OverloadResolutionResult.Succeeded);
+
+                            resolution.OverloadResolutionResult.ReportDiagnostics(
+                                binder: addMethodBinder, location: GetLocationForOverloadResolutionDiagnostic(syntax, expression), nodeOpt: syntax, diagnostics: diagnostics, name: WellKnownMemberNames.CollectionInitializerAddMethodName,
+                                receiver: resolution.MethodGroup.Receiver, invokedExpression: expression, arguments: analyzedArguments, memberGroup: resolution.MethodGroup.Methods.ToImmutable(),
+                                typeContainingConstructor: null, delegateTypeBeingInvoked: null, queryClause: null);
+
+                            addMethods = [];
+                            result = false;
                         }
                     }
                 }
@@ -1554,83 +1561,6 @@ static ImmutableArray<MethodSymbol> filterOutBadGenericMethods(
 
                 return resultBuilder.ToImmutableAndFree();
             }
-
-            // This is what BindInvocationExpressionContinued is doing in terms of reporting diagnostics and detecting a failure
-            static bool bindInvocationExpressionContinued(
-                Binder addMethodBinder,
-                SyntaxNode node,
-                SyntaxNode expression,
-                OverloadResolutionResult<MethodSymbol> result,
-                AnalyzedArguments analyzedArguments,
-                MethodGroup methodGroup,
-                BindingDiagnosticBag diagnostics,
-                out MethodSymbol? addMethod)
-            {
-                Debug.Assert(node != null);
-                Debug.Assert(methodGroup != null);
-                Debug.Assert(methodGroup.Error == null);
-                Debug.Assert(methodGroup.Methods.Count > 0);
-
-                var invokedAsExtensionMethod = methodGroup.IsExtensionMethodGroup;
-
-                // We have already determined that we are not in a situation where we can successfully do
-                // a dynamic binding. We might be in one of the following situations:
-                //
-                // * There were dynamic arguments but overload resolution still found zero applicable candidates.
-                // * There were no dynamic arguments and overload resolution found zero applicable candidates.
-                // * There were no dynamic arguments and overload resolution found multiple applicable candidates
-                //   without being able to find the best one.
-                //
-                // In those three situations we might give an additional error.
-
-                if (!result.Succeeded)
-                {
-                    // Since there were no argument errors to report, we report an error on the invocation itself.
-                    result.ReportDiagnostics(
-                        binder: addMethodBinder, location: GetLocationForOverloadResolutionDiagnostic(node, expression), nodeOpt: node, diagnostics: diagnostics, name: WellKnownMemberNames.CollectionInitializerAddMethodName,
-                        receiver: methodGroup.Receiver, invokedExpression: expression, arguments: analyzedArguments, memberGroup: methodGroup.Methods.ToImmutable(),
-                        typeContainingConstructor: null, delegateTypeBeingInvoked: null, queryClause: null);
-
-                    addMethod = null;
-                    return false;
-                }
-
-                // Otherwise, there were no dynamic arguments and overload resolution found a unique best candidate. 
-                // We still have to determine if it passes final validation.
-
-                var methodResult = result.ValidResult;
-                var method = methodResult.Member;
-
-                // Tracked by https://github.com/dotnet/roslyn/issues/76130: It looks like we added a bunch of code in BindInvocationExpressionContinued at this position
-                //            that specifically deals with new extension methods. It adjusts analyzedArguments, etc.
-                //            It is very likely we need to do the same here. 
-
-                // It is possible that overload resolution succeeded, but we have chosen an
-                // instance method and we're in a static method. A careful reading of the
-                // overload resolution spec shows that the "final validation" stage allows an
-                // "implicit this" on any method call, not just method calls from inside
-                // instance methods. Therefore we must detect this scenario here, rather than in
-                // overload resolution.
-
-                var receiver = methodGroup.Receiver;
-
-                // Note: we specifically want to do final validation (7.6.5.1) without checking delegate compatibility (15.2),
-                // so we're calling MethodGroupFinalValidation directly, rather than via MethodGroupConversionHasErrors.
-                // Note: final validation wants the receiver that corresponds to the source representation
-                // (i.e. the first argument, if invokedAsExtensionMethod).
-                var gotError = addMethodBinder.MemberGroupFinalValidation(receiver, method, expression, diagnostics, invokedAsExtensionMethod);
-
-                addMethodBinder.ReportDiagnosticsIfObsolete(diagnostics, method, node, hasBaseReceiver: false);
-                ReportDiagnosticsIfUnmanagedCallersOnly(diagnostics, method, node, isDelegateConversion: false);
-
-                // No use site errors, but there could be use site warnings.
-                // If there are any use site warnings, they have already been reported by overload resolution.
-                Debug.Assert(!method.HasUseSiteError, "Shouldn't have reached this point if there were use site errors.");
-                Debug.Assert(!method.IsRuntimeFinalizer());
-
-                addMethod = method;
-                return !gotError;
-            }
         }
 
         /// <summary>
@@ -3051,6 +2981,7 @@ private bool MethodGroupConversionHasErrors(
                 ReportDiagnosticsIfUnmanagedCallersOnly(diagnostics, selectedMethod, syntax, isDelegateConversion: true);
             }
             ReportDiagnosticsIfObsolete(diagnostics, selectedMethod, syntax, hasBaseReceiver: false);
+            ReportDiagnosticsIfDisallowedExtension(diagnostics, selectedMethod, syntax);
 
             // No use site errors, but there could be use site warnings.
             // If there are use site warnings, they were reported during the overload resolution process

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

@@ -8030,6 +8030,7 @@ private BoundExpression MakeMemberAccessValue(BoundExpression expr, BindingDiagn
 
         private BoundExpression GetExtensionMemberAccess(SyntaxNode syntax, BoundExpression? receiver, Symbol extensionMember, BindingDiagnosticBag diagnostics)
         {
+            MessageID.IDS_FeatureExtensions.CheckFeatureAvailability(diagnostics, syntax);
             receiver = ReplaceTypeOrValueReceiver(receiver, useType: extensionMember.IsStatic, diagnostics);
 
             switch (extensionMember)

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

@@ -1325,6 +1325,7 @@ private BoundCall BindInvocationExpressionContinued(
 
             ReportDiagnosticsIfObsolete(diagnostics, method, node, hasBaseReceiver);
             ReportDiagnosticsIfUnmanagedCallersOnly(diagnostics, method, node, isDelegateConversion: false);
+            ReportDiagnosticsIfDisallowedExtension(diagnostics, method, node);
 
             // No use site errors, but there could be use site warnings.
             // If there are any use site warnings, they have already been reported by overload resolution.

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

@@ -202,37 +202,34 @@ internal void EnumerateAllExtensionMembersInSingleBinder(ArrayBuilder<SingleLook
             PooledHashSet<MethodSymbol>? implementationsToShadow = null;
 
             // 1. Collect new extension members
-            if (this.Compilation.LanguageVersion.AllowNewExtensions())
-            {
-                var extensionDeclarations = ArrayBuilder<NamedTypeSymbol>.GetInstance();
-                this.GetExtensionDeclarations(extensionDeclarations, originalBinder);
+            var extensionDeclarations = ArrayBuilder<NamedTypeSymbol>.GetInstance();
+            this.GetExtensionDeclarations(extensionDeclarations, originalBinder);
 
-                foreach (NamedTypeSymbol extensionDeclaration in extensionDeclarations)
+            foreach (NamedTypeSymbol extensionDeclaration in extensionDeclarations)
+            {
+                if (extensionDeclaration.ExtensionParameter is null)
                 {
-                    if (extensionDeclaration.ExtensionParameter is null)
-                    {
-                        continue;
-                    }
+                    continue;
+                }
 
-                    var candidates = name is null ? extensionDeclaration.GetMembers() : extensionDeclaration.GetMembers(name);
+                var candidates = name is null ? extensionDeclaration.GetMembers() : extensionDeclaration.GetMembers(name);
 
-                    foreach (var candidate in candidates)
-                    {
-                        SingleLookupResult resultOfThisMember = originalBinder.CheckViability(candidate, arity, options, null, diagnose: true, useSiteInfo: ref useSiteInfo);
-                        result.Add(resultOfThisMember);
+                foreach (var candidate in candidates)
+                {
+                    SingleLookupResult resultOfThisMember = originalBinder.CheckViability(candidate, arity, options, null, diagnose: true, useSiteInfo: ref useSiteInfo);
+                    result.Add(resultOfThisMember);
 
-                        if (candidate is MethodSymbol { IsStatic: false } shadows &&
-                            shadows.OriginalDefinition.TryGetCorrespondingExtensionImplementationMethod() is { } toShadow)
-                        {
-                            implementationsToShadow ??= PooledHashSet<MethodSymbol>.GetInstance();
-                            implementationsToShadow.Add(toShadow);
-                        }
+                    if (candidate is MethodSymbol { IsStatic: false } shadows &&
+                        shadows.OriginalDefinition.TryGetCorrespondingExtensionImplementationMethod() is { } toShadow)
+                    {
+                        implementationsToShadow ??= PooledHashSet<MethodSymbol>.GetInstance();
+                        implementationsToShadow.Add(toShadow);
                     }
                 }
-
-                extensionDeclarations.Free();
             }
 
+            extensionDeclarations.Free();
+
             // 2. Collect classic extension methods
             var extensionMethods = ArrayBuilder<MethodSymbol>.GetInstance();
             this.GetCandidateExtensionMethods(extensionMethods, name, arity, options, originalBinder: originalBinder);

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

@@ -334,6 +334,7 @@ private BoundExpression BindCompoundAssignment(AssignmentExpressionSyntax node,
 
                     ReportDiagnosticsIfObsolete(diagnostics, method, node, hasBaseReceiver: false);
                     ReportDiagnosticsIfUnmanagedCallersOnly(diagnostics, method, node, isDelegateConversion: false);
+                    Debug.Assert(!IsDisallowedExtensionInOlderLangVer(method));
 
                     inPlaceResult = new BoundCompoundAssignmentOperator(
                         node,
@@ -2647,6 +2648,7 @@ private BoundExpression BindIncrementOperator(ExpressionSyntax node, ExpressionS
 
                     ReportDiagnosticsIfObsolete(diagnostics, method, node, hasBaseReceiver: false);
                     ReportDiagnosticsIfUnmanagedCallersOnly(diagnostics, method, node, isDelegateConversion: false);
+                    Debug.Assert(!IsDisallowedExtensionInOlderLangVer(method));
 
                     inPlaceResult = new BoundIncrementOperator(
                         node,

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

@@ -465,12 +465,16 @@ private BoundForEachStatement BindForEachPartsWorker(BindingDiagnosticBag diagno
             var foreachKeyword = _syntax.ForEachKeyword;
             ReportDiagnosticsIfObsolete(diagnostics, getEnumeratorMethod, foreachKeyword, hasBaseReceiver: false);
             ReportDiagnosticsIfUnmanagedCallersOnly(diagnostics, getEnumeratorMethod, foreachKeyword, isDelegateConversion: false);
+            Debug.Assert(!IsDisallowedExtensionInOlderLangVer(getEnumeratorMethod));
+
             // MoveNext is an instance method, so it does not need to have unmanaged callers only diagnostics reported.
             // Either a diagnostic was reported at the declaration of the method (for the invalid attribute), or MoveNext
             // is marked as not supported and we won't get here in the first place (for metadata import).
             ReportDiagnosticsIfObsolete(diagnostics, builder.MoveNextInfo.Method, foreachKeyword, hasBaseReceiver: false);
             ReportDiagnosticsIfObsolete(diagnostics, builder.CurrentPropertyGetter, foreachKeyword, hasBaseReceiver: false);
             ReportDiagnosticsIfObsolete(diagnostics, builder.CurrentPropertyGetter.AssociatedSymbol, foreachKeyword, hasBaseReceiver: false);
+            Debug.Assert(!IsDisallowedExtensionInOlderLangVer(builder.MoveNextInfo.Method));
+            Debug.Assert(!IsDisallowedExtensionInOlderLangVer(builder.CurrentPropertyGetter));
 
             // We want to convert from inferredType in the array/string case and builder.ElementType in the enumerator case,
             // but it turns out that these are equivalent (when both are available).

src/Compilers/CSharp/Portable/LanguageVersion.cs

@@ -572,10 +572,5 @@ internal static bool AllowImprovedOverloadCandidates(this LanguageVersion self)
         {
             return self >= MessageID.IDS_FeatureImprovedOverloadCandidates.RequiredVersion();
         }
-
-        internal static bool AllowNewExtensions(this LanguageVersion self)
-        {
-            return self >= MessageID.IDS_FeatureExtensions.RequiredVersion();
-        }
     }
 }