Merge pull request #75644 from CyrusNajmabadi/addAwaitBindingExpressions

Fix crash when 'add await' analyzers binding expressions

src/Features/CSharp/Portable/CodeRefactorings/AddAwait/CSharpAddAwaitCodeRefactoringProvider.cs

@@ -16,14 +16,11 @@ namespace Microsoft.CodeAnalysis.CSharp.CodeRefactorings.AddAwait;
 /// This refactoring complements the AddAwait fixer. It allows adding `await` and `await ... .ConfigureAwait(false)` even there is no compiler error to trigger the fixer.
 /// </summary>
 [ExportCodeRefactoringProvider(LanguageNames.CSharp, Name = PredefinedCodeRefactoringProviderNames.AddAwait), Shared]
-internal sealed partial class CSharpAddAwaitCodeRefactoringProvider : AbstractAddAwaitCodeRefactoringProvider<ExpressionSyntax>
+[method: ImportingConstructor]
+[method: SuppressMessage("RoslynDiagnosticsReliability", "RS0033:Importing constructor should be [Obsolete]", Justification = "Used in test code: https://github.com/dotnet/roslyn/issues/42814")]
+internal sealed partial class CSharpAddAwaitCodeRefactoringProvider()
+    : AbstractAddAwaitCodeRefactoringProvider<ExpressionSyntax>
 {
-    [ImportingConstructor]
-    [SuppressMessage("RoslynDiagnosticsReliability", "RS0033:Importing constructor should be [Obsolete]", Justification = "Used in test code: https://github.com/dotnet/roslyn/issues/42814")]
-    public CSharpAddAwaitCodeRefactoringProvider()
-    {
-    }
-
     protected override string GetTitle()
         => CSharpFeaturesResources.Add_await;
 
@@ -34,14 +31,12 @@ protected override bool IsInAsyncContext(SyntaxNode node)
     {
         foreach (var current in node.Ancestors())
         {
-            switch (current.Kind())
+            switch (current)
             {
-                case SyntaxKind.ParenthesizedLambdaExpression:
-                case SyntaxKind.SimpleLambdaExpression:
-                case SyntaxKind.AnonymousMethodExpression:
-                    return ((AnonymousFunctionExpressionSyntax)current).AsyncKeyword != default;
-                case SyntaxKind.MethodDeclaration:
-                    return ((MethodDeclarationSyntax)current).Modifiers.Any(SyntaxKind.AsyncKeyword);
+                case AnonymousFunctionExpressionSyntax anonymousFunction:
+                    return anonymousFunction.AsyncKeyword != default;
+                case MethodDeclarationSyntax methodDeclaration:
+                    return methodDeclaration.Modifiers.Any(SyntaxKind.AsyncKeyword);
             }
         }
 

src/Features/CSharpTest/AddAwait/AddAwaitTests.cs

@@ -13,7 +13,7 @@ namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.CodeRefactorings.AddAwa
 
 [Trait(Traits.Feature, Traits.Features.AddAwait)]
 [Trait(Traits.Feature, Traits.Features.CodeActionsAddAwait)]
-public class AddAwaitTests : AbstractCSharpCodeActionTest_NoEditor
+public sealed class AddAwaitTests : AbstractCSharpCodeActionTest_NoEditor
 {
     protected override CodeRefactoringProvider CreateCodeRefactoringProvider(TestWorkspace workspace, TestParameters parameters)
         => new CSharpAddAwaitCodeRefactoringProvider();
@@ -1610,4 +1610,89 @@ class Program
             }
             """);
     }
+
+    [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/66726")]
+    public async Task NotOnBindingExpression1()
+    {
+        await TestMissingInRegularAndScriptAsync("""
+            using System.Threading.Tasks;
+
+            class TestClass
+            {
+                private async Task MyTestMethod1Async(TestClass c)
+                {
+                    _ = c?.[|MyIntMethodAsync()|];
+                }
+
+                private Task<int> MyIntMethodAsync()
+                {
+                    return Task.FromResult(result: 1);
+                }
+            }
+            """);
+    }
+
+    [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/66726")]
+    public async Task NotOnBindingExpression2()
+    {
+        await TestMissingInRegularAndScriptAsync("""
+            using System.Threading.Tasks;
+
+            class TestClass
+            {
+                private TestClass C;
+
+                private async Task MyTestMethod1Async(TestClass c)
+                {
+                    _ = c?.C.[|MyIntMethodAsync()|];
+                }
+
+                private Task<int> MyIntMethodAsync()
+                {
+                    return Task.FromResult(result: 1);
+                }
+            }
+            """);
+    }
+
+    [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/66726")]
+    public async Task NotOnBindingExpression3()
+    {
+        await TestMissingInRegularAndScriptAsync("""
+            using System.Threading.Tasks;
+
+            class TestClass
+            {
+                private TestClass this[int i] => null;
+
+                private async Task MyTestMethod1Async(TestClass c)
+                {
+                    _ = c?[0].[|MyIntMethodAsync()|];
+                }
+
+                private Task<int> MyIntMethodAsync()
+                {
+                    return Task.FromResult(result: 1);
+                }
+            }
+            """);
+    }
+
+    [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/66726")]
+    public async Task NotOnBindingExpression4()
+    {
+        await TestMissingInRegularAndScriptAsync("""
+            using System.Threading.Tasks;
+
+            class TestClass
+            {
+                private Task<int> this[int i] => null;
+
+                private async Task MyTestMethod1Async(TestClass c)
+                {
+                    _ = c?[|[0]|];
+                }
+            }
+            """);
+    }
 }

src/Features/Core/Portable/CodeRefactorings/AddAwait/AbstractAddAwaitCodeRefactoringProvider.cs

@@ -2,6 +2,7 @@
 // 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.Linq;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis.CodeActions;
@@ -51,15 +52,15 @@ public sealed override async Task ComputeRefactoringsAsync(CodeRefactoringContex
                 context.RegisterRefactoring(
                     CodeAction.Create(
                         title,
-                        c => AddAwaitAsync(document, expression, withConfigureAwait: false, c),
+                        cancellationToken => AddAwaitAsync(document, expression, withConfigureAwait: false, cancellationToken),
                         title),
                     expression.Span);
 
                 var titleWithConfigureAwait = GetTitleWithConfigureAwait();
                 context.RegisterRefactoring(
                     CodeAction.Create(
                         titleWithConfigureAwait,
-                        c => AddAwaitAsync(document, expression, withConfigureAwait: true, c),
+                        cancellationToken => AddAwaitAsync(document, expression, withConfigureAwait: true, cancellationToken),
                         titleWithConfigureAwait),
                     expression.Span);
             }
@@ -80,6 +81,20 @@ private static bool IsValidAwaitableExpression(
         if (syntaxFacts.IsExpressionOfAwaitExpression(node))
             return false;
 
+        for (var current = node; current != null;)
+        {
+            if (syntaxFacts.IsMemberBindingExpression(current) ||
+                syntaxFacts.IsElementBindingExpression(current))
+            {
+                // Can't add 'await' to the `.X` in `a?.X`.  Nor would we want to.  Those could return null, which
+                // `await` would blow up on.  Note: this could be reconsidered if we end up adding `await?` support to
+                // the language.
+                return false;
+            }
+
+            current = current.ChildNodesAndTokens().FirstOrDefault().AsNode() as TExpressionSyntax;
+        }
+
         // if we're on an actual type symbol itself (like literally `Task`) we don't want to offer to add await.
         // we only want to add for actual expressions whose type is awaitable, not on the awaitable type itself.
         var symbol = model.GetSymbolInfo(node, cancellationToken).GetAnySymbol();

src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Services/SyntaxFacts/CSharpSyntaxFacts.cs

@@ -535,6 +535,9 @@ public bool IsBindableToken(SyntaxToken token)
     public bool IsPostfixUnaryExpression([NotNullWhen(true)] SyntaxNode? node)
         => node is PostfixUnaryExpressionSyntax;
 
+    public bool IsElementBindingExpression([NotNullWhen(true)] SyntaxNode? node)
+        => node is ElementBindingExpressionSyntax;
+
     public bool IsMemberBindingExpression([NotNullWhen(true)] SyntaxNode? node)
         => node is MemberBindingExpressionSyntax;
 

src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Services/SyntaxFacts/ISyntaxFacts.cs

@@ -287,6 +287,7 @@ void GetPartsOfTupleExpression<TArgumentSyntax>(SyntaxNode node,
     SyntaxNode GetExpressionOfAttributeArgument(SyntaxNode node);
     SyntaxNode GetExpressionOfInterpolation(SyntaxNode node);
 
+    bool IsElementBindingExpression([NotNullWhen(true)] SyntaxNode? node);
     bool IsMemberBindingExpression([NotNullWhen(true)] SyntaxNode? node);
     bool IsPostfixUnaryExpression([NotNullWhen(true)] SyntaxNode? node);
 

src/Workspaces/SharedUtilitiesAndExtensions/Compiler/VisualBasic/Services/SyntaxFacts/VisualBasicSyntaxFacts.vb

@@ -1569,6 +1569,11 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.LanguageService
             Return False
         End Function
 
+        Public Function IsElementBindingExpression(node As SyntaxNode) As Boolean Implements ISyntaxFacts.IsElementBindingExpression
+            ' Does not exist in VB.  VB represents an element binding as a InvocationExpression with null target.
+            Return False
+        End Function
+
         Public Function IsMemberBindingExpression(node As SyntaxNode) As Boolean Implements ISyntaxFacts.IsMemberBindingExpression
             ' Does not exist in VB.  VB represents a member binding as a MemberAccessExpression with null target.
             Return False