Do not make assumptions about content of erroneous BoundCall node based on value of InvokedAsExtensionMethod property (#80256)

Related to #78433.

Author: AlekseyTs

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

@@ -3767,6 +3767,11 @@ internal SafeContext GetRefEscape(BoundExpression expr, SafeContext localScopeDe
                     {
                         var call = (BoundCall)expr;
 
+                        if (call.IsErroneousNode)
+                        {
+                            return SafeContext.CallingMethod;
+                        }
+
                         var methodSymbol = call.Method;
                         if (methodSymbol.RefKind == RefKind.None)
                         {
@@ -3826,6 +3831,11 @@ internal SafeContext GetRefEscape(BoundExpression expr, SafeContext localScopeDe
                             return SafeContext.CallingMethod;
 
                         case BoundCall call:
+                            if (call.IsErroneousNode)
+                            {
+                                return SafeContext.CallingMethod;
+                            }
+
                             var methodSymbol = call.Method;
                             if (methodSymbol.RefKind == RefKind.None)
                             {
@@ -4049,6 +4059,11 @@ internal bool CheckRefEscape(SyntaxNode node, BoundExpression expr, SafeContext
                     {
                         var call = (BoundCall)expr;
 
+                        if (call.IsErroneousNode)
+                        {
+                            return true;
+                        }
+
                         var methodSymbol = call.Method;
                         if (methodSymbol.RefKind == RefKind.None)
                         {
@@ -4114,6 +4129,11 @@ internal bool CheckRefEscape(SyntaxNode node, BoundExpression expr, SafeContext
                             return true;
 
                         case BoundCall call:
+                            if (call.IsErroneousNode)
+                            {
+                                return true;
+                            }
+
                             var methodSymbol = call.Method;
                             if (methodSymbol.RefKind == RefKind.None)
                             {
@@ -4411,6 +4431,11 @@ internal SafeContext GetValEscape(BoundExpression expr, SafeContext localScopeDe
                     {
                         var call = (BoundCall)expr;
 
+                        if (call.IsErroneousNode)
+                        {
+                            return SafeContext.CallingMethod;
+                        }
+
                         return GetInvocationEscapeScope(
                             MethodInvocationInfo.FromCall(call),
                             localScopeDepth,
@@ -4457,6 +4482,11 @@ internal SafeContext GetValEscape(BoundExpression expr, SafeContext localScopeDe
                             return localScopeDepth;
 
                         case BoundCall call:
+                            if (call.IsErroneousNode)
+                            {
+                                return SafeContext.CallingMethod;
+                            }
+
                             return GetInvocationEscapeScope(
                                 MethodInvocationInfo.FromCall(call, implicitIndexerAccess.Receiver),
                                 localScopeDepth,
@@ -5088,6 +5118,11 @@ internal bool CheckValEscape(SyntaxNode node, BoundExpression expr, SafeContext
                 case BoundKind.Call:
                     {
                         var call = (BoundCall)expr;
+                        if (call.IsErroneousNode)
+                        {
+                            return true;
+                        }
+
                         var methodSymbol = call.Method;
 
                         return CheckInvocationEscape(
@@ -5152,6 +5187,11 @@ internal bool CheckValEscape(SyntaxNode node, BoundExpression expr, SafeContext
                             return false;
 
                         case BoundCall call:
+                            if (call.IsErroneousNode)
+                            {
+                                return true;
+                            }
+
                             var methodSymbol = call.Method;
 
                             return CheckInvocationEscape(
@@ -5826,7 +5866,7 @@ SafeContext getPartsScope(BoundInterpolatedString interpolatedString, SafeContex
 
                 foreach (var part in interpolatedString.Parts)
                 {
-                    if (part is not BoundCall call)
+                    if (part is not BoundCall { IsErroneousNode: false } call)
                     {
                         // Dynamic calls cannot have ref struct parameters.
                         continue;
@@ -5868,7 +5908,7 @@ bool checkParts(BoundInterpolatedString interpolatedString, SafeContext escapeFr
             {
                 foreach (var part in interpolatedString.Parts)
                 {
-                    if (part is not BoundCall call)
+                    if (part is not BoundCall { IsErroneousNode: false } call)
                     {
                         // Dynamic calls cannot have ref struct parameters.
                         continue;

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

@@ -93,6 +93,7 @@ private bool CouldBeAwaited(BoundExpression expression)
             }
 
             var call = (BoundCall)expression;
+            Debug.Assert(!call.IsErroneousNode);
 
             // First check if the target method is async.
             if ((object)call.Method != null && call.Method.IsAsync)
@@ -591,6 +592,8 @@ private bool GetGetAwaiterMethod(BoundExpression expression, SyntaxNode node, Bi
             }
 
             var call = (BoundCall)getAwaiterCall;
+            Debug.Assert(!call.IsErroneousNode);
+
             var getAwaiterMethod = call.Method;
             if (getAwaiterMethod is ErrorMethodSymbol ||
                 call.Expanded || HasOptionalParameters(getAwaiterMethod) || // We might have been able to resolve a GetAwaiter overload with optional parameters, so check for that here
@@ -699,6 +702,8 @@ private bool GetGetResultMethod(BoundExpression awaiterExpression, SyntaxNode no
             }
 
             var call = (BoundCall)getAwaiterGetResultCall;
+            Debug.Assert(!call.IsErroneousNode);
+
             getResultMethod = call.Method;
             if (getResultMethod.IsExtensionMethod || getResultMethod.GetIsNewExtensionMember())
             {

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

@@ -147,7 +147,7 @@ internal BoundExpression MakeInvocationExpression(
                 // the error has already been reported by BindInvocationExpression
                 Debug.Assert(diagnostics.DiagnosticBag is null || diagnostics.HasAnyErrors());
 
-                result = CreateBadCall(node, boundExpression, LookupResultKind.Viable, analyzedArguments);
+                result = CreateBadCall(node, boundExpression, LookupResultKind.NotInvocable, analyzedArguments);
             }
 
             result.WasCompilerGenerated = true;
@@ -369,7 +369,7 @@ private BoundExpression BindInvocationExpression(
             {
                 if (ReportDelegateInvokeUseSiteDiagnostic(diagnostics, delegateType, node: node))
                 {
-                    return CreateBadCall(node, boundExpression, LookupResultKind.Viable, analyzedArguments);
+                    return CreateBadCall(node, boundExpression, LookupResultKind.NotInvocable, analyzedArguments);
                 }
 
                 result = BindDelegateInvocation(node, expression, methodName, boundExpression, analyzedArguments, diagnostics, queryClause, delegateType);

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

@@ -682,6 +682,11 @@ private void ReduceFrom(FromClauseSyntax from, QueryTranslationState state, Bind
 
         private static BoundExpression? ExtractCastInvocation(BoundCall invocation)
         {
+            if (invocation.IsErroneousNode)
+            {
+                return null;
+            }
+
             int index = invocation.InvokedAsExtensionMethod ? 1 : 0;
             var c1 = invocation.Arguments[index] as BoundConversion;
             var l1 = c1 != null ? c1.Operand as BoundLambda : null;

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

@@ -757,31 +757,60 @@ private void VisitArgumentsAndGetArgumentPlaceholders(BoundExpression? receiverO
 
                 BeforeVisitingSkippedBoundCallChildren(node);
 
-                VisitReceiver(in methodInvocationInfo);
+                visitReceiver(node, in methodInvocationInfo);
 
                 var nodeAndInvocationInfo = (call: node, methodInvocationInfo);
                 do
                 {
-                    VisitArguments(nodeAndInvocationInfo.call, in nodeAndInvocationInfo.methodInvocationInfo);
+                    visitArguments(nodeAndInvocationInfo.call, in nodeAndInvocationInfo.methodInvocationInfo);
                 }
                 while (calls.TryPop(out nodeAndInvocationInfo!));
 
                 calls.Free();
             }
             else
             {
-                VisitReceiver(in methodInvocationInfo);
-                VisitArguments(node, in methodInvocationInfo);
+                visitReceiver(node, in methodInvocationInfo);
+                visitArguments(node, in methodInvocationInfo);
             }
 
             return null;
 
             static MethodInvocationInfo getInvocationInfo(BoundCall node)
             {
                 var methodInvocationInfo = MethodInvocationInfo.FromCall(node);
-                methodInvocationInfo = ReplaceWithExtensionImplementationIfNeeded(in methodInvocationInfo);
+
+                if (!node.IsErroneousNode)
+                {
+                    methodInvocationInfo = ReplaceWithExtensionImplementationIfNeeded(in methodInvocationInfo);
+                }
+
                 return methodInvocationInfo;
             }
+
+            void visitReceiver(BoundCall node, ref readonly MethodInvocationInfo methodInvocationInfo)
+            {
+                if (node.IsErroneousNode)
+                {
+                    Visit(node.ReceiverOpt);
+                }
+                else
+                {
+                    VisitReceiver(in methodInvocationInfo);
+                }
+            }
+
+            void visitArguments(BoundCall node, ref readonly MethodInvocationInfo methodInvocationInfo)
+            {
+                if (node.IsErroneousNode)
+                {
+                    VisitList(node.Arguments);
+                }
+                else
+                {
+                    VisitArguments(node, in methodInvocationInfo);
+                }
+            }
         }
 
         protected override void VisitReceiver(BoundCall node)
@@ -1092,7 +1121,7 @@ private void VisitDeconstructionArguments(ArrayBuilder<DeconstructionVariable> v
                 return;
             }
 
-            if (invocation.Method is null)
+            if (invocation.IsErroneousNode || invocation.Method is null)
             {
                 return;
             }

src/Compilers/CSharp/Portable/BoundTree/BoundCall.cs

@@ -0,0 +1,30 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System.Diagnostics;
+
+namespace Microsoft.CodeAnalysis.CSharp
+{
+    internal partial class BoundCall
+    {
+        public bool IsErroneousNode => ResultKind is not LookupResultKind.Viable;
+
+        private partial void Validate()
+        {
+            Debug.Assert(ResultKind is not LookupResultKind.MemberGroup);
+            Debug.Assert(ResultKind is not LookupResultKind.StaticInstanceMismatch);
+            Debug.Assert(ResultKind is LookupResultKind.Viable || HasErrors);
+
+            /* Tracking issue: https://github.com/dotnet/roslyn/issues/79426
+            Debug.Assert(ResultKind is LookupResultKind.Viable ||
+                         new StackTrace(fNeedFileInfo: false).GetFrame(2)?.GetMethod() switch
+                         {
+                             { Name: nameof(ErrorCall), DeclaringType: { } declaringType } => declaringType == typeof(BoundCall),
+                             { Name: nameof(Update), DeclaringType: { } declaringType } => declaringType == typeof(BoundCall),
+                             _ => false
+                         });
+            */
+        }
+    }
+}

src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml

@@ -1823,7 +1823,7 @@
     <Field Name="Properties" Type="ImmutableArray&lt;PropertySymbol&gt;" />
   </Node>
 
-  <Node Name="BoundCall" Base="BoundExpression">
+  <Node Name="BoundCall" Base="BoundExpression" HasValidate="true">
     <!-- Non-null type is required for this node kind -->
     <Field Name="Type" Type="TypeSymbol" Override="true" Null="disallow"/>
 
@@ -1839,7 +1839,17 @@
     <Field Name="InvokedAsExtensionMethod" Type="bool"/>
     <Field Name="ArgsToParamsOpt" Type="ImmutableArray&lt;int&gt;" Null="allow"/>
     <Field Name="DefaultArguments" Type="BitVector" />
+
+    <!--
+    Result kind for the invocation. Never LookupResultKind.MemberGroup or LookupResultKind.StaticInstanceMismatch.
+    If the value is something other than LookupResultKind.Viable, then the node represents a bad invocation,
+    the consistency among information stored in other properties is not guaranteed, and the Method property might
+    be set to an ErrorMethodSymbol. A special care should be taken when dealing with such nodes. It is recommended
+    to treat them similar to BoundBadExpressions nodes, unless there is a good reason to have more complicated
+    error recovery. 
+    -->
     <Field Name="ResultKind" PropertyOverrides="true" Type="LookupResultKind"/>
+
     <!--The set of method symbols from which this call's method was chosen.
         Only kept in the tree if the call was an error and overload resolution
         was unable to choose a best method.-->

src/Compilers/CSharp/Portable/BoundTree/Constructors.cs

@@ -132,8 +132,15 @@ public static BoundCall ErrorCall(
             Binder binder)
         {
             if (!originalMethods.IsEmpty)
+            {
                 resultKind = resultKind.WorseResultKind(LookupResultKind.OverloadResolutionFailure);
+            }
+            else
+            {
+                Debug.Assert(method.OriginalDefinition is ErrorMethodSymbol);
+            }
 
+            Debug.Assert(resultKind is not LookupResultKind.Viable);
             Debug.Assert(arguments.IsDefaultOrEmpty || (object)receiverOpt != (object)arguments[0]);
 
             return new BoundCall(