Extensions: address some diagnostic quality issues

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

@@ -8678,9 +8678,10 @@ static bool tryResolveExtensionInScope(
                     {
                         if (propertyResult != null)
                         {
+                            Debug.Assert(actualReceiverArguments is not null);
+                            firstResult = makeErrorResult(methodResult, propertyResult, expression, left, memberName, arity, lookupResult, actualReceiverArguments, binder, diagnostics);
                             methodResult.Free(keepArguments: true);
                             propertyResult.Free();
-                            firstResult = makeErrorResult(left.Type, memberName, arity, lookupResult, expression, diagnostics);
                         }
                         else
                         {
@@ -8704,28 +8705,30 @@ static bool tryResolveExtensionInScope(
                     }
 
                     // ambiguous between methods and properties
+                    Debug.Assert(actualReceiverArguments is not null);
+                    result = makeErrorResult(methodResult, propertyResult, expression, left, memberName, arity, lookupResult, actualReceiverArguments, binder, diagnostics);
                     methodResult.Free(keepArguments: true);
                     propertyResult?.Free();
-                    result = makeErrorResult(left.Type, memberName, arity, lookupResult, expression, diagnostics);
                     return true;
                 }
 
                 // If the search in the current scope resulted in any applicable property (regardless of whether a best
                 // applicable property could be determined) then our search is complete.
                 Debug.Assert(propertyResult?.HasAnyApplicableMember == true);
-                methodResult.Free(keepArguments: true);
                 if (propertyResult.Succeeded && propertyResult.BestResult.Member is { } bestProperty)
                 {
                     // property wins
+                    methodResult.Free(keepArguments: true);
                     propertyResult.Free();
                     result = new MethodGroupResolution(bestProperty, LookupResultKind.Viable, diagnostics.ToReadOnly());
                     return true;
                 }
 
                 // ambiguous between multiple applicable properties
+                Debug.Assert(actualReceiverArguments is not null);
+                result = makeErrorResult(methodResult, propertyResult, expression, left, memberName, arity, lookupResult, actualReceiverArguments, binder, diagnostics);
+                methodResult.Free(keepArguments: true);
                 propertyResult.Free();
-                // Tracked by https://github.com/dotnet/roslyn/issues/78830 : diagnostic quality, consider using the property overload resolution result in the result to improve reported diagnostics
-                result = makeErrorResult(left.Type, memberName, arity, lookupResult, expression, diagnostics);
                 return true;
             }
 
@@ -8852,13 +8855,45 @@ static MethodGroupResolution resolveMethods(
                 return overloadResolutionResult;
             }
 
-            static MethodGroupResolution makeErrorResult(TypeSymbol receiverType, string memberName, int arity, LookupResult lookupResult, SyntaxNode expression, BindingDiagnosticBag diagnostics)
+            static MethodGroupResolution makeErrorResult(
+                MethodGroupResolution methodResult,
+                OverloadResolutionResult<PropertySymbol> propertyResult,
+                SyntaxNode expression,
+                BoundExpression left,
+                string memberName,
+                int arity,
+                LookupResult lookupResult,
+                AnalyzedArguments actualReceiverArguments,
+                Binder binder,
+                BindingDiagnosticBag diagnostics)
             {
-                // Tracked by https://github.com/dotnet/roslyn/issues/78830 : diagnostic quality, we'll want to describe what went wrong in a useful way (see OverloadResolutionResult.ReportDiagnostics)
-                var errorInfo = new CSDiagnosticInfo(ErrorCode.ERR_ExtensionResolutionFailed, receiverType, memberName);
-                diagnostics.Add(errorInfo, expression.Location);
-                var resultSymbol = new ExtendedErrorTypeSymbol(containingSymbol: null, lookupResult.Symbols.ToImmutable(), LookupResultKind.OverloadResolutionFailure, errorInfo, arity);
-                return new MethodGroupResolution(resultSymbol, LookupResultKind.Viable, diagnostics.ToReadOnly());
+                Debug.Assert(propertyResult is not null);
+                ImmutableArray<Symbol> symbols = lookupResult.Symbols.ToImmutable();
+
+                DiagnosticInfo errorInfo;
+                if (methodResult.HasAnyApplicableMethod && propertyResult.HasAnyApplicableMember)
+                {
+                    MethodSymbol representativeMethod = methodResult.OverloadResolutionResult is { } methodResolution
+                        ? methodResolution.PickRepresentativeMember()
+                        : methodResult.MethodGroup.Methods[0];
+
+                    PropertySymbol representativeProperty = propertyResult.PickRepresentativeMember();
+
+                    errorInfo = OverloadResolutionResult<Symbol>.CreateAmbiguousCallDiagnosticInfo(binder.Compilation, representativeMethod, representativeProperty, symbols, isExtension: true);
+
+                    diagnostics.Add(errorInfo, expression.Location);
+                }
+                else
+                {
+                    propertyResult.ReportDiagnostics(binder, expression.Location, expression, diagnostics, memberName, left, left.Syntax, actualReceiverArguments, symbols,
+                        typeContainingConstructor: null, delegateTypeBeingInvoked: null, isMethodGroupConversion: false, isExtension: true);
+
+                    errorInfo = new CSDiagnosticInfo(ErrorCode.ERR_ExtensionResolutionFailed, left.Type, memberName);
+                }
+
+                ExtendedErrorTypeSymbol resultSymbol = new ExtendedErrorTypeSymbol(containingSymbol: null, symbols, LookupResultKind.OverloadResolutionFailure, errorInfo, arity);
+                Debug.Assert(lookupResult.Kind == LookupResultKind.Viable);
+                return new MethodGroupResolution(resultSymbol, lookupResult.Kind, diagnostics.ToReadOnly());
             }
         }
 

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

@@ -128,6 +128,23 @@ internal bool HasAnyApplicableMember
             }
         }
 
+        internal TMember PickRepresentativeMember()
+        {
+            Debug.Assert(HasAnyApplicableMember);
+
+            if (Succeeded)
+            {
+                return BestResult.Member;
+            }
+
+            if (ResultsBuilder.FirstOrDefault(r => r.Result.Kind == MemberResolutionKind.Worse).Member is { } worse)
+            {
+                return worse;
+            }
+
+            return GetAllApplicableMembers()[0];
+        }
+
         /// <summary>
         /// Returns all methods in the group that are applicable, <see cref="HasAnyApplicableMember"/>.
         /// </summary>
@@ -201,7 +218,8 @@ internal void ReportDiagnostics<T>(
             bool isMethodGroupConversion = false,
             RefKind? returnRefKind = null,
             TypeSymbol delegateOrFunctionPointerType = null,
-            bool isParamsModifierValidation = false) where T : Symbol
+            bool isParamsModifierValidation = false,
+            bool isExtension = false) where T : Symbol
         {
             Debug.Assert(!this.Succeeded, "Don't ask for diagnostic info on a successful overload resolution result.");
 
@@ -220,7 +238,7 @@ internal void ReportDiagnostics<T>(
             // however, be more than one.  We'll check for that first, since applicable candidates are
             // always better than inapplicable candidates.
 
-            if (HadAmbiguousBestMethods(binder.Compilation, diagnostics, symbols, location))
+            if (HadAmbiguousBestMethods(binder.Compilation, diagnostics, symbols, location, isExtension))
             {
                 return;
             }
@@ -243,7 +261,7 @@ internal void ReportDiagnostics<T>(
             // (Obviously, there can't be a LessDerived cycle, since we break type hierarchy cycles during
             // symbol table construction.)
 
-            if (HadAmbiguousWorseMethods(binder.Compilation, diagnostics, symbols, location, queryClause != null, receiver, name))
+            if (HadAmbiguousWorseMethods(binder.Compilation, diagnostics, symbols, location, queryClause != null, receiver, name, isExtension))
             {
                 return;
             }
@@ -774,10 +792,22 @@ private bool TypeInferenceFailed(
                 }
                 else
                 {
-                    diagnostics.Add(new DiagnosticInfoWithSymbols(
-                        ErrorCode.ERR_NoSuchMemberOrExtension,
-                        new object[] { instanceArgument.Type, inferenceFailed.Member.Name },
-                        symbols), location);
+                    if (inferenceFailed.Member.Kind == SymbolKind.Method)
+                    {
+                        // error CS0411: The type arguments for method 'M<T>(T)' cannot be inferred
+                        // from the usage. Try specifying the type arguments explicitly.
+                        diagnostics.Add(new DiagnosticInfoWithSymbols(
+                            ErrorCode.ERR_CantInferMethTypeArgs,
+                            new object[] { inferenceFailed.Member },
+                            symbols), location);
+                    }
+                    else
+                    {
+                        diagnostics.Add(new DiagnosticInfoWithSymbols(
+                            ErrorCode.ERR_NoSuchMemberOrExtension,
+                            new object[] { instanceArgument.Type, inferenceFailed.Member.Name },
+                            symbols), location);
+                    }
                 }
 
                 return true;
@@ -1388,7 +1418,7 @@ static Symbol unwrapIfParamsCollection(MemberResolutionResult<TMember> badArg, P
             }
         }
 
-        private bool HadAmbiguousWorseMethods(CSharpCompilation compilation, BindingDiagnosticBag diagnostics, ImmutableArray<Symbol> symbols, Location location, bool isQuery, BoundExpression receiver, string name)
+        private bool HadAmbiguousWorseMethods(CSharpCompilation compilation, BindingDiagnosticBag diagnostics, ImmutableArray<Symbol> symbols, Location location, bool isQuery, BoundExpression receiver, string name, bool isExtension)
         {
             MemberResolutionResult<TMember> worseResult1;
             MemberResolutionResult<TMember> worseResult2;
@@ -1417,7 +1447,8 @@ private bool HadAmbiguousWorseMethods(CSharpCompilation compilation, BindingDiag
                         compilation,
                         worseResult1.LeastOverriddenMember.ConstructedFrom(),
                         worseResult2.LeastOverriddenMember.ConstructedFrom(),
-                        symbols),
+                        symbols,
+                        isExtension),
                     location);
             }
 
@@ -1453,7 +1484,7 @@ private int TryGetFirstTwoWorseResults(out MemberResolutionResult<TMember> first
             return count;
         }
 
-        private bool HadAmbiguousBestMethods(CSharpCompilation compilation, BindingDiagnosticBag diagnostics, ImmutableArray<Symbol> symbols, Location location)
+        private bool HadAmbiguousBestMethods(CSharpCompilation compilation, BindingDiagnosticBag diagnostics, ImmutableArray<Symbol> symbols, Location location, bool isExtension)
         {
             MemberResolutionResult<TMember> validResult1;
             MemberResolutionResult<TMember> validResult2;
@@ -1473,7 +1504,8 @@ private bool HadAmbiguousBestMethods(CSharpCompilation compilation, BindingDiagn
                     compilation,
                     validResult1.LeastOverriddenMember.ConstructedFrom(),
                     validResult2.LeastOverriddenMember.ConstructedFrom(),
-                    symbols),
+                    symbols,
+                    isExtension),
                 location);
 
             return true;
@@ -1508,10 +1540,13 @@ private int TryGetFirstTwoValidResults(out MemberResolutionResult<TMember> first
             return count;
         }
 
-        private static DiagnosticInfoWithSymbols CreateAmbiguousCallDiagnosticInfo(CSharpCompilation compilation, Symbol first, Symbol second, ImmutableArray<Symbol> symbols)
+        internal static DiagnosticInfoWithSymbols CreateAmbiguousCallDiagnosticInfo(CSharpCompilation compilation, Symbol first, Symbol second, ImmutableArray<Symbol> symbols, bool isExtension)
         {
+            // error: The extension resolution is ambiguous between the following members: 'first' and 'second'
+            // OR
+            // error: The call is ambiguous between the following methods or properties: 'first' and 'second'
             var distinguisher = new SymbolDistinguisher(compilation, first, second);
-            return new DiagnosticInfoWithSymbols(ErrorCode.ERR_AmbigCall, [distinguisher.First, distinguisher.Second], symbols);
+            return new DiagnosticInfoWithSymbols(isExtension ? ErrorCode.ERR_AmbigExtension : ErrorCode.ERR_AmbigCall, [distinguisher.First, distinguisher.Second], symbols);
         }
 
         [Conditional("DEBUG")]

src/Compilers/CSharp/Portable/CSharpResources.resx

@@ -8168,4 +8168,7 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
   <data name="ERR_BadVisBaseType" xml:space="preserve">
     <value>Inconsistent accessibility: type '{1}' is less accessible than class '{0}' </value>
   </data>
+  <data name="ERR_AmbigExtension" xml:space="preserve">
+    <value>The extension resolution is ambiguous between the following members: '{0}' and '{1}'</value>
+  </data>
 </root>

src/Compilers/CSharp/Portable/Errors/ErrorCode.cs

@@ -2435,6 +2435,7 @@ internal enum ErrorCode
         HDN_RedundantPatternStackGuard = 9337,
 
         ERR_BadVisBaseType = 9338,
+        ERR_AmbigExtension = 9339,
 
         // Note: you will need to do the following after adding errors:
         //  1) Update ErrorFacts.IsBuildOnlyDiagnostic (src/Compilers/CSharp/Portable/Errors/ErrorFacts.cs)

src/Compilers/CSharp/Portable/Errors/ErrorFacts.cs

@@ -2543,6 +2543,7 @@ or ErrorCode.HDN_RedundantPattern
                 or ErrorCode.WRN_RedundantPattern
                 or ErrorCode.HDN_RedundantPatternStackGuard
                 or ErrorCode.ERR_BadVisBaseType
+                or ErrorCode.ERR_AmbigExtension
                     => false,
             };
 #pragma warning restore CS8524 // The switch expression does not handle some values of its input type (it is not exhaustive) involving an unnamed enum value.

src/Compilers/CSharp/Portable/Parser/LanguageParser.cs

@@ -4697,7 +4697,6 @@ private SeparatedSyntaxList<ParameterSyntax> ParseParameterList(
                 allowTrailingSeparator: false,
                 requireOneElement: forExtension, // For extension declarations, we require at least one receiver parameter
                 allowSemicolonAsSeparator: false);
-            // Tracked by https://github.com/dotnet/roslyn/issues/78830 : diagnostic quality, consider suppressing parsing diagnostics on extension parameters beyond the first one
 
             _termState = saveTerm;
             close = this.EatToken(closeKind);

src/Compilers/CSharp/Portable/SymbolDisplay/SymbolDisplayVisitor.Types.cs

@@ -294,7 +294,8 @@ private void VisitNamedTypeWithoutNullability(INamedTypeSymbol symbol)
 
             //visit the enclosing type if the style requires it
             if (Format.TypeQualificationStyle == SymbolDisplayTypeQualificationStyle.NameAndContainingTypes ||
-                Format.TypeQualificationStyle == SymbolDisplayTypeQualificationStyle.NameAndContainingTypesAndNamespaces)
+                Format.TypeQualificationStyle == SymbolDisplayTypeQualificationStyle.NameAndContainingTypesAndNamespaces ||
+                symbol.IsExtension)
             {
                 if (IncludeNamedType(symbol.ContainingType))
                 {