📝 I don't this will be that useful

initActualArguments

The name feels counterintuitive. We have actual arguments, this method "fakes" an additional argument. Consider dropping "Actual" #Closed

I realized that the "Actual" probably refers to the actualMethodArguments. Both feel confusing, but I guess this is a "pre-existing condition".

propertyResult.BestResult.Member

What is this going to produce in case of an ambiguity? #Closed

Good catch. This was broken. BestResult can only be accessed if the overload resolution has succeeded (ie. there's a single result that's still considered applicable, either in normal or expanded form, after overload resolution completes)

LookupResultKind.Viable

It might be worth explaining why we "call" error result Viable #Closed

Made small adjustment to clarify.
It's meant to flow the kind from the LookupResult, but we can only get here when it's Viable (we have a check for IsMultiViable earlier in ResolveExtensions)

(Format.TypeQualificationStyle == SymbolDisplayTypeQualificationStyle.NameOnly && symbol.IsExtension)

I understand that this is probably gives us desired behavior for specific scenarios, but the condition itself is confusing. Especially for someone who doesn't know that we enumerated all possible SymbolDisplayTypeQualificationStyle values that are defined at the moment. I think the condition will have more sense and will be more robust to future changes to the SymbolDisplayTypeQualificationStyle enum if we simply use symbol.IsExtension as the condition. #Closed

expected

Is this function used? #Closed

methodResult.MethodGroup.Methods[0]

Is this guaranteed to contain an applicable method? #Closed

This thread was marked as resolved, but it is still not obvious to me why we assume that methodResult.MethodGroup.Methods[0] is an applicable method.

Is there a test that takes this code path?

All the methods in that method group are as applicable as it gets (they meet lookup criteria and are compatible/substitutable given the receiver type).

The scenario where we don't have an overload resolution result of methods is when there are no arguments. For example var x = object.M; or System.Action a = object.M;.
This reporting path is tested by ReportDiagnostics_23, ReportDiagnostics_24 and ReportDiagnostics_25.

In such scenarios, we start with a lookup result (which ensures we match name and arity requirements), then resolveMethods prunes methods that are not compatible with the receiver (using ReduceExtensionMember), instead of running normal overload resolution. We then form a MethodGroupResolution that is considered to have applicable methods (ie. its HasAnyApplicableMethod is true), since there's not possible overload resolution we could run.

// MethodGroupResolution criteria that lets us get here
        public bool HasAnyApplicableMethod
        {
            get
            {
                return (this.MethodGroup != null) &&
                    (this.ResultKind == LookupResultKind.Viable) &&
                    ((this.OverloadResolutionResult == null) || this.OverloadResolutionResult.HasAnyApplicableMember);
            }
        }

return result.ResultsBuilder.First(r => r.Result.Kind == MemberResolutionKind.Worse).Member;

This looks fragile because the logic doesn't look equivalent to OverloadResolutionResult<TMember>.HasAnyApplicableMember. I suggest adding an OverloadResolutionResult<TMember>.FirstApplicableMember API next to OverloadResolutionResult<TMember>.HasAnyApplicableMember with shared logic for picking an applicable member, and using it here. That should help with keeping consistent behavior now and in the future. #Closed

Perhaps that API doesn't have to return the first applicable candidate. It might return the "best" applicable candidate. The main point is that there should be an obvious guarantee that we get something when HasAnyApplicableMember is true.

I'm okay to move the logic closer to HasAnyApplicableMember, but the distinction that is left here is Worse vs. Worst and it is intentional.

Because the result had HasAnyApplicableMember but not Succeeded, we know there's at least one Worse in the set. Worst is worse than Worse

actualMethodArguments

This is suspicious. It looks like we are using actualReceiverArguments in resolveProperties. Also, it looks like resolveProperties guarantees that actualReceiverArguments is initialized when propertyResult is not null. We can assert the fact after resolveProperties and on entry into this function. #Closed

Thanks. I failed to noticed that we have separate actual arguments for methods vs. properties, and used the wrong one...

initActualArguments(left, analyzedArguments, ref actualMethodArguments);

Given the comment about actualReceiverArguments this looks unnecessary. #Closed

initActualArguments

Given the comment about actualReceiverArguments this helper looks unnecessary, i.e. original code can stay where it was. #Closed

ref

AnalyzedArguments is a class and we do not try to change the passed in value, I think we don't need it to be a ref. #Closed

AnalyzedArguments? actualReceiverArguments

I think we expect actualReceiverArguments to be not null here #Closed

AnalyzedArguments? analyzedArguments,

It looks like this parameter isn't used #Closed

ref AnalyzedArguments? actualMethodArguments,

It looks like this parameter isn't used #Closed

return ResultsBuilder.First(r => r.Result.Kind == MemberResolutionKind.Worse).Member;

We discussed offline using a more defensive logic here. #Closed