Merge pull request #8586 from balajikris/TestDelegateInvoke

More test coverage for delegate invoke with conditional access

src/EditorFeatures/CSharpTest/Diagnostics/InvokeDelegateWithConditionalAccess/InvokeDelegateWithConditionalAccessTests.cs

@@ -150,7 +150,7 @@ void Foo()
         }
 
         [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsInvokeDelegateWithConditionalAccess)]
-        public async Task TestMissingWithMultipleVariables()
+        public async Task TestMissingOnDeclarationWithMultipleVariables()
         {
             await TestMissingAsync(
 @"class C
@@ -163,13 +163,47 @@ void Foo()
         {
             v();
         }
-        else {}
     }
 }");
         }
 
+        /// <remarks>
+        /// With multiple variables in the same declaration, the fix _is not_ offered on the declaration
+        /// itself, but _is_ offered on the invocation pattern.
+        /// </remarks>
         [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsInvokeDelegateWithConditionalAccess)]
-        public async Task TestMissingIfUsedOutside()
+        public async Task TestLocationWhereOfferedWithMultipleVariables()
+        {
+            await TestAsync(
+@"class C
+{
+    System.Action a;
+    void Foo()
+    {
+        var v = a, x = a;
+        [||]if (v != null)
+        {
+            v();
+        }
+    }
+}",
+@"class C
+{
+    System.Action a;
+    void Foo()
+    {
+        var v = a, x = a;
+        v?.Invoke();
+    }
+}");
+        }
+
+        /// <remarks>
+        /// If we have a variable declaration and if it is read/written outside the delegate 
+        /// invocation pattern, the fix is not offered on the declaration.
+        /// </remarks>
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsInvokeDelegateWithConditionalAccess)]
+        public async Task TestMissingOnDeclarationIfUsedOutside()
         {
             await TestMissingAsync(
 @"class C
@@ -188,6 +222,42 @@ void Foo()
 }");
         }
 
+        /// <remarks>
+        /// If we have a variable declaration and if it is read/written outside the delegate 
+        /// invocation pattern, the fix is not offered on the declaration but is offered on
+        /// the invocation pattern itself.
+        /// </remarks>
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsInvokeDelegateWithConditionalAccess)]
+        public async Task TestLocationWhereOfferedIfUsedOutside()
+        {
+            await TestAsync(
+@"class C
+{
+    System.Action a;
+    void Foo()
+    {
+        var v = a;
+        [||]if (v != null)
+        {
+            v();
+        }
+
+        v = null;
+    }
+}",
+@"class C
+{
+    System.Action a;
+    void Foo()
+    {
+        var v = a;
+        v?.Invoke();
+
+        v = null;
+    }
+}");
+        }
+
         [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsInvokeDelegateWithConditionalAccess)]
         public async Task TestSimpleForm1()
         {
@@ -348,5 +418,219 @@ void Foo()
     }
 }", compareTokens: false);
         }
+
+        /// <remarks>
+        /// tests locations where the fix is offered.
+        /// </remarks>
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsInvokeDelegateWithConditionalAccess)]
+        public async Task TestFixOfferedOnIf()
+        {
+            await TestAsync(
+@"class C
+{
+    System.Action a;
+    void Foo()
+    {
+        var v = a;
+        [||]if (v != null)
+        {
+            v();
+        }
+    }
+}",
+@"
+class C
+{
+    System.Action a;
+    void Foo()
+    {
+        a?.Invoke();
+    }
+}");
+        }
+
+        /// <remarks>
+        /// tests locations where the fix is offered.
+        /// </remarks>
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsInvokeDelegateWithConditionalAccess)]
+        public async Task TestFixOfferedInsideIf()
+        {
+            await TestAsync(
+@"class C
+{
+    System.Action a;
+    void Foo()
+    {
+        var v = a;
+        if (v != null)
+        {
+            [||]v();
+        }
+    }
+}",
+@"
+class C
+{
+    System.Action a;
+    void Foo()
+    {
+        a?.Invoke();
+    }
+}");
+        }
+
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsInvokeDelegateWithConditionalAccess)]
+        public async Task TestMissingOnConditionalInvocation()
+        {
+            await TestMissingAsync(
+@"class C
+{
+    System.Action a;
+    void Foo()
+    {
+        [||]var v = a;
+        v?.Invoke();
+    }
+}");
+        }
+
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsInvokeDelegateWithConditionalAccess)]
+        public async Task TestMissingOnConditionalInvocation2()
+        {
+            await TestMissingAsync(
+@"class C
+{
+    System.Action a;
+    void Foo()
+    {
+        var v = a;
+        [||]v?.Invoke();
+    }
+}");
+        }
+
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsInvokeDelegateWithConditionalAccess)]
+        public async Task TestMissingOnConditionalInvocation3()
+        {
+            await TestMissingAsync(
+@"class C
+{
+    System.Action a;
+    void Foo()
+    {
+        [||]a?.Invoke();
+    }
+}");
+        }
+
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsInvokeDelegateWithConditionalAccess)]
+        public async Task TestMissingOnNonNullCheckExpressions()
+        {
+            await TestMissingAsync(
+@"class C
+{
+    System.Action a;
+    void Foo()
+    {
+        var v = a;
+        if (v == a)
+        {
+            [||]v();
+        }
+    }
+}");
+        }
+
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsInvokeDelegateWithConditionalAccess)]
+        public async Task TestMissingOnNonNullCheckExpressions2()
+        {
+            await TestMissingAsync(
+@"class C
+{
+    System.Action a;
+    void Foo()
+    {
+        var v = a;
+        if (v == null)
+        {
+            [||]v();
+        }
+    }
+}");
+        }
+
+        /// <remarks>
+        /// if local declaration is not immediately preceding the invocation pattern, 
+        /// the fix is not offered on the declaration.
+        /// </remarks>
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsInvokeDelegateWithConditionalAccess)]
+        public async Task TestLocalNotImmediatelyPrecedingNullCheckAndInvokePattern()
+        {
+            await TestMissingAsync(
+@"class C
+{
+    System.Action a;
+    void Foo()
+    {
+        [||]var v = a;
+        int x;
+        if (v != null)
+        {
+            v();
+        }
+    }
+}");
+        }
+
+        /// <remarks>
+        /// if local declaration is not immediately preceding the invocation pattern, 
+        /// the fix is not offered on the declaration but is offered on the invocation pattern itself.
+        /// </remarks>
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsInvokeDelegateWithConditionalAccess)]
+        public async Task TestLocalDNotImmediatelyPrecedingNullCheckAndInvokePattern2()
+        {
+            await TestAsync(
+@"class C
+{
+    System.Action a;
+    void Foo()
+    {
+        var v = a;
+        int x;
+        [||]if (v != null)
+        {
+            v();
+        }
+    }
+}",
+@"class C
+{
+    System.Action a;
+    void Foo()
+    {
+        var v = a;
+        int x;
+        v?.Invoke();
+    }
+}");
+        }
+
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsInvokeDelegateWithConditionalAccess)]
+        public async Task TestMissingOnFunc()
+        {
+            await TestMissingAsync(
+@"class C
+{
+    System.Func<int> a;
+    int Foo()
+    {
+        var v = a;
+        [||]if (v != null)
+        {
+            return v();
+        }
+    }
+}");
+        }
     }
 }
\ No newline at end of file

src/Features/CSharp/Portable/InvokeDelegateWithConditionalAccess/InvokeDelegateWithConditionalAccessAnalyzer.cs

 // Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
 
-using System;
 using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Threading;
@@ -128,6 +127,8 @@ private void SyntaxNodeAction(SyntaxNodeAnalysisContext syntaxContext)
                             Location.Create(tree, TextSpan.FromBounds(expressionStatement.Span.End, ifStatement.Span.End)),
                             additionalLocations, properties));
                     }
+
+                    return true;
                 }
             }
 
@@ -150,8 +151,8 @@ private void SyntaxNodeAction(SyntaxNodeAnalysisContext syntaxContext)
                 return false;
             }
 
-            if (!IsNotEqualsExpression(condition.Left, condition.Right) &&
-                !IsNotEqualsExpression(condition.Right, condition.Left))
+            if (!IsNullCheckExpression(condition.Left, condition.Right) &&
+                !IsNullCheckExpression(condition.Right, condition.Left))
             {
                 return false;
             }
@@ -244,7 +245,7 @@ private void SyntaxNodeAction(SyntaxNodeAnalysisContext syntaxContext)
             return true;
         }
 
-        private bool IsNotEqualsExpression(ExpressionSyntax left, ExpressionSyntax right) =>
+        private bool IsNullCheckExpression(ExpressionSyntax left, ExpressionSyntax right) =>
             left.IsKind(SyntaxKind.IdentifierName) && right.IsKind(SyntaxKind.NullLiteralExpression);
 
         public DiagnosticAnalyzerCategory GetAnalyzerCategory()