Fix go to definition for conversions on invalid constructor overloads

[Rekkonnect]

Closes #73498
Closes #77545

Fixes one part of #75833, the appropriate tests were adjusted to reflect the new behavior

With this PR, the GetSymbolInfo API and all the corresponding APIs using it will be affected for the scenarios where an object creation expression used a constructor overload that doesn't exist, whilst the expression was being implicitly converted to another type. The previous behavior would resolve to the target type of the conversion, since the binding result would be an overload resolution, whereas now it has been changed to resolve to the existing overload resolution result.

The goal of this change is also reflected in GTD which will now correctly navigate to the type whose constructor was not found, instead of the target conversion type, or the implicit conversion method.

This also adds tests for explicit conversions on object creation expressions, covering the above scenaria, as well as testing the existing correct behaviors of when there is no conversion, or when there is no overload resolution failure.

[AlekseyTs]

We need to have direct tests for SemanticModel API, plus an explanation what was the old behavior, why it was wrong, etc. #Closed

[Rekkonnect]

@AlekseyTs this is ready for review

[AlekseyTs]

plus an explanation what was the old behavior, why it was wrong, etc.

Was the explanation provided? Basically we need a clear description of the problem with the SemanticModel API. The reason why you think the present behavior is wrong, a description of the expected behavior with a reason behind the expectation.

#Closed

[Rekkonnect]

Was the explanation provided?
I added an explanation here: https://github.com/dotnet/roslyn/pull/78514/files#diff-d901fc0a7ce2d67b0336c3708a45962ee31ae1ce7c9c67e79d3def4fc8f947bfR1913-R1916

I can reword it if you feel like it doesn't provide enough information

[AlekseyTs]

I think this isn't quite what I was looking for. I was looking for a description of your intent in this PR and the reason behind the intent. Something like: "I intend to change behavior *** API for the scenarios ***. The current behavior of the API is ***. I think it should be *** instead, because ***." Obviously your ultimate goal is to get some IDE scenarios working, but simply stating that is not sufficient for changing how compiler API behaves. In order for the Compiler team to review the PR, we need the intent stated in "Compiler API" terms. #Closed

[Rekkonnect]

@AlekseyTs updated the description and brought the PR up to date, ptal

[AlekseyTs]

if (!overloadResolutionSuffices && highestBoundNode is BoundExpression highestBoundExpr)

The original if statement looked like a "voodoo magic" and with this adjustment it looks even more "magical".
I was able to clarify/simplify it to the following:

            if (highestBoundNode is BoundBadExpression badExpression)
            {
                // Downgrade result kind if highest node is BoundBadExpression
                LookupResultKind highestResultKind;
                bool highestIsDynamic;
                ImmutableArray<Symbol> unusedHighestMemberGroup;
                OneOrMany<Symbol> highestSymbols = GetSemanticSymbols(
                    badExpression, boundNodeForSyntacticParent, binderOpt, options, out highestIsDynamic, out highestResultKind, out unusedHighestMemberGroup);

                if (highestResultKind != LookupResultKind.Empty && highestResultKind < resultKind)
                {
                    resultKind = highestResultKind;
                    isDynamic = highestIsDynamic;
                }
            }
            else if (boundExpr is BoundMethodGroup &&
                resultKind == LookupResultKind.OverloadResolutionFailure &&
                highestBoundNode is BoundConversion { ConversionKind: ConversionKind.MethodGroup } boundConversion)
            {
                LookupResultKind highestResultKind;
                bool highestIsDynamic;
                ImmutableArray<Symbol> unusedHighestMemberGroup;
                OneOrMany<Symbol> highestSymbols = GetSemanticSymbols(
                    boundConversion, boundNodeForSyntacticParent, binderOpt, options, out highestIsDynamic, out highestResultKind, out unusedHighestMemberGroup);

                if (highestSymbols.Count > 0)
                {
                    symbols = highestSymbols;
                    resultKind = highestResultKind;
                    isDynamic = highestIsDynamic;
                }
            }

With this code all tests but two are passing, and the two failing tests are:

• GetSymbolInfo_ImplicitUserDefinedConversionOnMethodGroup_InLocalDeclaration
• GetSymbolInfo_ImplicitUserDefinedConversionOnMethodGroup_InAssignment

According to #75833, the failures is a desired behavior change. Though, #75833 is not completely fixed. #Closed

[Rekkonnect]

Changed code

[AlekseyTs]

Done with review pass (commit 8) #Closed

[Rekkonnect]

@AlekseyTs resolved your comments, could you ptal again?

[AlekseyTs]

LGTM (commit 11)

[AlekseyTs]

@dotnet/roslyn-compiler For a second review for a community PR.

[AlekseyTs]

@dotnet/roslyn-compiler For a second review for a community PR.

[AlekseyTs]

@dotnet/roslyn-compiler For a second review for a community PR.