Fixing 'else' keyword completion (#25703)

Merging on behalf of @Neme12. Thanks!

src/EditorFeatures/CSharpTest2/Recommendations/ElseKeywordRecommenderTests.cs

@@ -2,6 +2,7 @@
 
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis.Test.Utilities;
+using Roslyn.Test.Utilities;
 using Xunit;
 
 namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.Recommendations
@@ -86,50 +87,355 @@ public async Task TestAfterHashAndSpace()
         }
 
         [Fact, Trait(Traits.Feature, Traits.Features.KeywordRecommending)]
-        public async Task TestAfterIf()
+        public async Task TestNotAfterIf()
         {
-            await VerifyKeywordAsync(AddInsideMethod(
+            await VerifyAbsenceAsync(AddInsideMethod(
 @"if (true)
+$$"));
+        }
+
+        [Theory, Trait(Traits.Feature, Traits.Features.KeywordRecommending)]
+        [InlineData("Console.WriteLine();")]
+        [InlineData("{ }")]
+        [InlineData("while (true) { }")]
+        public async Task TestAfterIfStatement(string statement)
+        {
+            await VerifyKeywordAsync(AddInsideMethod(
+$@"if (true)
+    {statement}
+$$"));
+        }
+
+        [Theory, Trait(Traits.Feature, Traits.Features.KeywordRecommending)]
+        [InlineData("Console.WriteLine();")]
+        [InlineData("{ }")]
+        [InlineData("while (true) { }")]
+        public async Task TestAfterIfStatement_BeforeElse(string statement)
+        {
+            await VerifyKeywordAsync(AddInsideMethod(
+$@"if (true)
+    {statement}
+$$
+else"));
+        }
+
+        [Theory, Trait(Traits.Feature, Traits.Features.KeywordRecommending)]
+        [InlineData("Console.WriteLine();")]
+        [InlineData("{ }")]
+        [InlineData("while (true) { }")]
+        public async Task TestAfterIfNestedIfStatement(string statement)
+        {
+            await VerifyKeywordAsync(AddInsideMethod(
+$@"if (true)
+    if (true)
+        {statement}
+    $$"));
+        }
+
+        [Theory, Trait(Traits.Feature, Traits.Features.KeywordRecommending)]
+        [InlineData("Console.WriteLine();")]
+        [InlineData("{ }")]
+        [InlineData("while (true) { }")]
+        public async Task TestAfterIfNestedIfStatement_BeforeElse(string statement)
+        {
+            await VerifyKeywordAsync(AddInsideMethod(
+$@"if (true)
+    if (true)
+        {statement}
+    $$
+    else"));
+        }
+
+        [WorkItem(25336, "https://github.com/dotnet/roslyn/issues/25336")]
+        [Theory, Trait(Traits.Feature, Traits.Features.KeywordRecommending)]
+        [InlineData("Console.WriteLine();")]
+        [InlineData("{ }")]
+        [InlineData("while (true) { }")]
+        public async Task TestAfterIfNestedIfElseStatement(string statement)
+        {
+            await VerifyKeywordAsync(AddInsideMethod(
+$@"if (true)
+    if (true)
         Console.WriteLine();
+    else
+        {statement}
 $$"));
         }
 
-        [Fact, Trait(Traits.Feature, Traits.Features.KeywordRecommending)]
-        public async Task TestAfterIfBlock()
+        [WorkItem(25336, "https://github.com/dotnet/roslyn/issues/25336")]
+        [Theory, Trait(Traits.Feature, Traits.Features.KeywordRecommending)]
+        [InlineData("Console.WriteLine();")]
+        [InlineData("{ }")]
+        [InlineData("while (true) { }")]
+        public async Task TestAfterIfNestedIfElseStatement_BeforeElse(string statement)
         {
             await VerifyKeywordAsync(AddInsideMethod(
-@"if (true)
-{
+$@"if (true)
+    if (true)
         Console.WriteLine();
-}
+    else
+        {statement}
+$$
+else"));
+        }
+
+        [Theory, Trait(Traits.Feature, Traits.Features.KeywordRecommending)]
+        [InlineData("Console.WriteLine();")]
+        [InlineData("{ }")]
+        [InlineData("while (true) { }")]
+        public async Task TestNotAfterIfNestedIfElseElseStatement(string statement)
+        {
+            await VerifyAbsenceAsync(AddInsideMethod(
+$@"if (true)
+    if (true)
+        Console.WriteLine();
+    else
+        Console.WriteLine();
+else
+    {statement}
 $$"));
         }
 
-        [Fact, Trait(Traits.Feature, Traits.Features.KeywordRecommending)]
-        public async Task TestNotAfterElse1()
+        [Theory, Trait(Traits.Feature, Traits.Features.KeywordRecommending)]
+        [InlineData("Console.WriteLine();")]
+        [InlineData("{ }")]
+        [InlineData("while (true) { }")]
+        public async Task TestNotAfterIfStatementElse(string statement)
         {
             await VerifyAbsenceAsync(AddInsideMethod(
-@"if (true)
+$@"if (true)
+    {statement}
+else
+    $$"));
+        }
+
+        [Theory, Trait(Traits.Feature, Traits.Features.KeywordRecommending)]
+        [InlineData("Console.WriteLine();")]
+        [InlineData("{ }")]
+        [InlineData("while (true) { }")]
+        public async Task TestNotAfterIfElseStatement(string statement)
+        {
+            await VerifyAbsenceAsync(AddInsideMethod(
+$@"if (true)
+    Console.WriteLine();
+else
+    {statement}
+$$"));
+        }
+
+        [Theory, Trait(Traits.Feature, Traits.Features.KeywordRecommending)]
+        [InlineData("Console.WriteLine();")]
+        [InlineData("{ }")]
+        [InlineData("while (true) { }")]
+        public async Task TestAfterIfElseNestedIfStatement(string statement)
+        {
+            await VerifyKeywordAsync(AddInsideMethod(
+$@"if (true)
+    Console.WriteLine();
+else
+    if (true)
+        {statement}
+    $$"));
+        }
+
+        [Theory, Trait(Traits.Feature, Traits.Features.KeywordRecommending)]
+        [InlineData("Console.WriteLine();")]
+        [InlineData("{ }")]
+        [InlineData("while (true) { }")]
+        public async Task TestAfterIfElseNestedIfStatement_BeforeElse(string statement)
+        {
+            await VerifyKeywordAsync(AddInsideMethod(
+$@"if (true)
+    Console.WriteLine();
+else
+    if (true)
+        {statement}
+    $$
+    else"));
+        }
+
+        [Theory, Trait(Traits.Feature, Traits.Features.KeywordRecommending)]
+        [InlineData("Console.WriteLine();")]
+        [InlineData("{ }")]
+        [InlineData("while (true) { }")]
+        public async Task TestNotAfterIfElseNestedIfElseStatement(string statement)
+        {
+            await VerifyAbsenceAsync(AddInsideMethod(
+$@"if (true)
+    Console.WriteLine();
+else
+    if (true)
+        Console.WriteLine();
+    else
+        {statement}
+$$"));
+        }
+
+        [Theory, Trait(Traits.Feature, Traits.Features.KeywordRecommending)]
+        [InlineData("Console.WriteLine();")]
+        [InlineData("{ }")]
+        [InlineData("while (true) { }")]
+        public async Task TestAfterWhileIfWhileNestedIfElseStatement(string statement)
+        {
+            await VerifyKeywordAsync(AddInsideMethod(
+$@"while (true)
+    if (true)
+        while (true)
+            if (true)
+                Console.WriteLine();
+            else
+                {statement}
+    $$"));
+        }
+
+        [Theory, Trait(Traits.Feature, Traits.Features.KeywordRecommending)]
+        [InlineData("Console.WriteLine();")]
+        [InlineData("{ }")]
+        [InlineData("while (true) { }")]
+        public async Task TestAfterWhileIfWhileNestedIfElseStatement_BeforeElse(string statement)
+        {
+            await VerifyKeywordAsync(AddInsideMethod(
+$@"while (true)
+    if (true)
+        while (true)
+            if (true)
+                Console.WriteLine();
+            else
+                {statement}
+    $$
+    else"));
+        }
+
+        [Theory, Trait(Traits.Feature, Traits.Features.KeywordRecommending)]
+        [InlineData("Console.WriteLine();")]
+        [InlineData("{ }")]
+        [InlineData("while (true) { }")]
+        public async Task TestNotAfterWhileIfWhileNestedIfElseElseStatement(string statement)
+        {
+            await VerifyAbsenceAsync(AddInsideMethod(
+$@"while (true)
+    if (true)
+        while (true)
+            if (true)
+                Console.WriteLine();
+            else
+                Console.WriteLine();
+    else
+        {statement}
+$$"));
+        }
+
+        [Theory, Trait(Traits.Feature, Traits.Features.KeywordRecommending)]
+        [InlineData("Console")]
+        [InlineData("Console.")]
+        [InlineData("Console.WriteLine(")]
+        [InlineData("Console.WriteLine()")]
+        [InlineData("{")]
+        [InlineData("{ Console.WriteLine();")]
+        [InlineData("while")]
+        [InlineData("while (true)")]
+        [InlineData("while (true) {")]
+        [InlineData("while (true) { { }")]
+        [InlineData("for (int i = 0;")]
+        public async Task TestNotAfterIfIncompleteStatement(string statement)
+        {
+            await VerifyAbsenceAsync(AddInsideMethod(
+$@"if (true)
+    {statement}
+$$"));
+        }
+
+        [Theory, Trait(Traits.Feature, Traits.Features.KeywordRecommending)]
+        [InlineData("Console")]
+        [InlineData("Console.")]
+        [InlineData("Console.WriteLine(")]
+        [InlineData("Console.WriteLine()")]
+        [InlineData("{")]
+        [InlineData("{ Console.WriteLine();")]
+        [InlineData("while")]
+        [InlineData("while (true)")]
+        [InlineData("while (true) {")]
+        [InlineData("while (true) { { }")]
+        [InlineData("for (int i = 0;")]
+        public async Task TestNotAfterIfNestedIfIncompleteStatement(string statement)
+        {
+            await VerifyAbsenceAsync(AddInsideMethod(
+$@"if (true)
+    if (true)
+        {statement}
+    $$"));
+        }
+
+        [Theory, Trait(Traits.Feature, Traits.Features.KeywordRecommending)]
+        [InlineData("Console")]
+        [InlineData("Console.")]
+        [InlineData("Console.WriteLine(")]
+        [InlineData("Console.WriteLine()")]
+        [InlineData("{")]
+        [InlineData("{ Console.WriteLine();")]
+        [InlineData("while")]
+        [InlineData("while (true)")]
+        [InlineData("while (true) {")]
+        [InlineData("while (true) { { }")]
+        [InlineData("for (int i = 0;")]
+        public async Task TestNotAfterIfNestedIfElseIncompleteStatement(string statement)
+        {
+            await VerifyAbsenceAsync(AddInsideMethod(
+$@"if (true)
+    if (true)
         Console.WriteLine();
-else $$"));
+    else
+        {statement}
+$$"));
+        }
+
+        [Theory, Trait(Traits.Feature, Traits.Features.KeywordRecommending)]
+        [InlineData("Console.WriteLine();")]
+        [InlineData("{ }")]
+        [InlineData("while (true) { }")]
+        public async Task TestAfterIfNestedIfIncompleteStatementElseStatement(string statement)
+        {
+            await VerifyKeywordAsync(AddInsideMethod(
+$@"if (true)
+    if (true)
+        Console // Incomplete, but that's fine. This is not the if statement we care about.
+    else
+        {statement}
+$$"));
+        }
+
+        [Theory, Trait(Traits.Feature, Traits.Features.KeywordRecommending)]
+        [InlineData("Console.WriteLine();")]
+        [InlineData("{ }")]
+        [InlineData("while (true) { }")]
+        public async Task TestAfterIfNestedIfIncompleteStatementElseStatement_BeforeElse(string statement)
+        {
+            await VerifyKeywordAsync(AddInsideMethod(
+$@"if (true)
+    if (true)
+        Console // Incomplete, but that's fine. This is not the if statement we care about.
+    else
+        {statement}
+$$
+else"));
         }
 
         [Fact, Trait(Traits.Feature, Traits.Features.KeywordRecommending)]
-        public async Task TestNotAfterElse2()
+        public async Task TestNotInsideStatement()
         {
             await VerifyAbsenceAsync(AddInsideMethod(
 @"if (true)
-{
-    Console.WriteLine();
-}
-else $$"));
+    Console.WriteLine()$$; // Complete statement, but we're not at the end of it.
+"));
         }
 
         [Fact, Trait(Traits.Feature, Traits.Features.KeywordRecommending)]
-        public async Task TestNotAfterMemberAccess()
+        public async Task TestNotAfterSkippedToken()
         {
             await VerifyAbsenceAsync(AddInsideMethod(
-@"if (true)string.$$"));
+@"if (true)
+    Console.WriteLine();,
+$$"));
         }
     }
 }

src/Features/CSharp/Portable/Completion/KeywordRecommenders/ElseKeywordRecommender.cs

@@ -23,36 +23,24 @@ protected override bool IsValidContext(int position, CSharpSyntaxContext context
 
             var token = context.TargetToken;
 
-            var statement = token.GetAncestor<StatementSyntax>();
-            var ifStatement = statement.GetAncestorOrThis<IfStatementSyntax>();
-
-            if (statement == null || ifStatement == null)
-            {
-                return false;
-            }
-
-            // cases:
-            //   if (goo)
+            // We have to consider all ancestor if statements of the last token until we find a match for this 'else':
+            // while (true)
+            //     if (true)
+            //         while (true)
+            //             if (true)
             //                 Console.WriteLine();
-            //   |
-            //   if (goo)
+            //             else
             //                 Console.WriteLine();
-            //   e|
-            if (token.IsKind(SyntaxKind.SemicolonToken) && ifStatement.Statement.GetLastToken(includeSkipped: true) == token)
+            //     $$
+            foreach (var ifStatement in token.GetAncestors<IfStatementSyntax>())
             {
-                return true;
-            }
-
-            // if (goo) {
-            //     Console.WriteLine();
-            //   } |
-            //   if (goo) {
-            //     Console.WriteLine();
-            //   } e|
-            if (token.IsKind(SyntaxKind.CloseBraceToken) && ifStatement.Statement is BlockSyntax && token == ((BlockSyntax)ifStatement.Statement).CloseBraceToken)
+                // If there's a missing token at the end of the statement, it's incomplete and we do not offer 'else'.
+                // context.TargetToken does not include zero width so in that case these will never be equal.
+                if (ifStatement.Statement.GetLastToken(includeZeroWidth: true) == token)
                 {
                     return true;
                 }
+            }
 
             return false;
         }