Merge pull request #25133 from Neme12/whenRecommender

Adding missing completion for switch when clause

src/EditorFeatures/CSharpTest/Completion/CompletionProviders/SymbolCompletionProviderTests.cs

@@ -1264,9 +1264,15 @@ public async Task SwitchStatement()
         [Fact, Trait(Traits.Feature, Traits.Features.Completion)]
         public async Task SwitchLabelCase()
         {
-            var content = @"switch(i)
-    {
-        case $$";
+            var content = @"switch(i) { case $$";
+            await VerifyItemExistsAsync(AddUsingDirectives("using System;", AddInsideMethod(content)), @"String");
+            await VerifyItemExistsAsync(AddUsingDirectives("using System;", AddInsideMethod(content)), @"System");
+        }
+
+        [Fact, Trait(Traits.Feature, Traits.Features.Completion)]
+        public async Task SwitchPatternLabelCase()
+        {
+            var content = @"switch(i) { case $$ when";
             await VerifyItemExistsAsync(AddUsingDirectives("using System;", AddInsideMethod(content)), @"String");
             await VerifyItemExistsAsync(AddUsingDirectives("using System;", AddInsideMethod(content)), @"System");
         }
@@ -2684,6 +2690,36 @@ void M()
             await VerifyItemExistsAsync(markup, "Length");
         }
 
+        [Fact, Trait(Traits.Feature, Traits.Features.Completion)]
+        public async Task ConstantsInIsExpression()
+        {
+            var markup = @"
+class C
+{
+    public const int MAX_SIZE = 10;
+    void M()
+    {
+        int i = 10;
+        if (i is $$ int"; // 'int' to force this to be parsed as an IsExpression rather than IsPatternExpression
+
+            await VerifyItemExistsAsync(markup, "MAX_SIZE");
+        }
+
+        [Fact, Trait(Traits.Feature, Traits.Features.Completion)]
+        public async Task ConstantsInIsPatternExpression()
+        {
+            var markup = @"
+class C
+{
+    public const int MAX_SIZE = 10;
+    void M()
+    {
+        int i = 10;
+        if (i is $$ 1";
+
+            await VerifyItemExistsAsync(markup, "MAX_SIZE");
+        }
+
         [WorkItem(542429, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/542429")]
         [Fact, Trait(Traits.Feature, Traits.Features.Completion)]
         public async Task ConstantsInSwitchCase()
@@ -2702,6 +2738,24 @@ void M()
             await VerifyItemExistsAsync(markup, "MAX_SIZE");
         }
 
+        [WorkItem(25084, "https://github.com/dotnet/roslyn/issues/25084#issuecomment-370148553")]
+        [Fact, Trait(Traits.Feature, Traits.Features.Completion)]
+        public async Task ConstantsInSwitchPatternCase()
+        {
+            var markup = @"
+class C
+{
+    public const int MAX_SIZE = 10;
+    void M()
+    {
+        int i = 10;
+        switch (i)
+        {
+            case $$ when";
+
+            await VerifyItemExistsAsync(markup, "MAX_SIZE");
+        }
+
         [WorkItem(542429, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/542429")]
         [Fact, Trait(Traits.Feature, Traits.Features.Completion)]
         public async Task ConstantsInSwitchGotoCase()
@@ -8698,6 +8752,25 @@ void M(bool x)
             await VerifyItemExistsAsync(markup, "x");
         }
 
+        [Fact, Trait(Traits.Feature, Traits.Features.Completion)]
+        public async Task ExceptionFilter1_NotBeforeOpenParen()
+        {
+            var markup = @"
+using System;
+
+class C
+{
+    void M(bool x)
+    {
+        try
+        {
+        }
+        catch when $$
+";
+
+            await VerifyNoItemsExistAsync(markup);
+        }
+
         [Fact, Trait(Traits.Feature, Traits.Features.Completion)]
         public async Task ExceptionFilter2()
         {
@@ -8717,6 +8790,59 @@ void M(bool x)
             await VerifyItemExistsAsync(markup, "x");
         }
 
+        [Fact, Trait(Traits.Feature, Traits.Features.Completion)]
+        public async Task ExceptionFilter2_NotBeforeOpenParen()
+        {
+            var markup = @"
+using System;
+
+class C
+{
+    void M(bool x)
+    {
+        try
+        {
+        }
+        catch (Exception ex) when $$
+";
+
+            await VerifyNoItemsExistAsync(markup);
+        }
+
+        [WorkItem(25084, "https://github.com/dotnet/roslyn/issues/25084")]
+        [Fact, Trait(Traits.Feature, Traits.Features.Completion)]
+        public async Task SwitchCaseWhenClause1()
+        {
+            var markup = @"
+class C
+{
+    void M(bool x)
+    {
+        switch (1)
+        {
+            case 1 when $$
+";
+
+            await VerifyItemExistsAsync(markup, "x");
+        }
+
+        [WorkItem(25084, "https://github.com/dotnet/roslyn/issues/25084")]
+        [Fact, Trait(Traits.Feature, Traits.Features.Completion)]
+        public async Task SwitchCaseWhenClause2()
+        {
+            var markup = @"
+class C
+{
+    void M(bool x)
+    {
+        switch (1)
+        {
+            case int i when $$
+";
+
+            await VerifyItemExistsAsync(markup, "x");
+        }
+
         [WorkItem(717, "https://github.com/dotnet/roslyn/issues/717")]
         [Fact, Trait(Traits.Feature, Traits.Features.Completion)]
         public async Task ExpressionContextCompletionWithinCast()

src/EditorFeatures/CSharpTest/Extensions/ContextQuery/IsPossibleDeconstructionDesignationTests.cs

@@ -3,6 +3,7 @@
 using System.Threading;
 using Microsoft.CodeAnalysis.CSharp.Extensions.ContextQuery;
 using Microsoft.CodeAnalysis.Test.Utilities;
+using Roslyn.Test.Utilities;
 using Xunit;
 
 namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.IntelliSense.CompletionSetSources
@@ -152,5 +153,26 @@ public void False1()
         {
             VerifyFalse(AddInsideMethod(@"var $$"));
         }
+
+        [WorkItem(25084, "https://github.com/dotnet/roslyn/issues/25084#issuecomment-369075537")]
+        [Fact, Trait(Traits.Feature, Traits.Features.Completion)]
+        public void FalseAfterPattern1()
+        {
+            VerifyFalse(AddInsideMethod(@"if (1 is int i $$"));
+        }
+
+        [WorkItem(25084, "https://github.com/dotnet/roslyn/issues/25084#issuecomment-369075537")]
+        [Fact, Trait(Traits.Feature, Traits.Features.Completion)]
+        public void FalseAfterPattern2()
+        {
+            VerifyFalse(AddInsideMethod(@"if (1 is int i $$);"));
+        }
+
+        [WorkItem(25084, "https://github.com/dotnet/roslyn/issues/25084#issuecomment-369075537")]
+        [Fact, Trait(Traits.Feature, Traits.Features.Completion)]
+        public void FalseAfterPattern3()
+        {
+            VerifyFalse(AddInsideMethod(@"switch (1) { case int i $$ }"));
+        }
     }
 }

src/EditorFeatures/CSharpTest2/Recommendations/NullKeywordRecommenderTests.cs

@@ -248,19 +248,6 @@ public async Task TestInReferenceSwitch()
         }"));
         }
 
-#if false
-        [WorkItem(8486)]
-        [WpfFact, Trait(Traits.Feature, Traits.Features.KeywordRecommending)]
-        public async Task TestNotInValueSwitch()
-        {
-            await VerifyAbsenceAsync(AddInsideMethod(
-@"switch (0)
-        {
-            case $$
-        }"));
-        }
-#endif
-
         [WorkItem(543766, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/543766")]
         [Fact, Trait(Traits.Feature, Traits.Features.KeywordRecommending)]
         public async Task TestNotInDefaultParameterValue()
@@ -306,5 +293,21 @@ public async Task TestInCrefContextNotAfterDot()
 class C { }
 ");
         }
+
+        [Fact, Trait(Traits.Feature, Traits.Features.KeywordRecommending)]
+        public async Task TestAfterIs()
+        {
+            await VerifyKeywordAsync(AddInsideMethod(@"if (x is $$"));
+        }
+
+        [WorkItem(25293, "https://github.com/dotnet/roslyn/issues/25293")]
+        [Fact, Trait(Traits.Feature, Traits.Features.KeywordRecommending)]
+        public async Task TestAfterIs_BeforeExpression()
+        {
+            await VerifyKeywordAsync(AddInsideMethod(@"
+int x;
+int y = x is $$ Method();
+"));
+        }
     }
 }

src/EditorFeatures/Test2/IntelliSense/CSharpCompletionCommandHandlerTests.vb

@@ -3740,6 +3740,109 @@ class C
             End Using
         End Function
 
+        <WpfFact, Trait(Traits.Feature, Traits.Features.Completion)>
+        Public Async Function AfterIdentifierInCaseLabel() As Task
+            Using state = TestState.CreateCSharpTestState(
+                              <Document>
+class C
+{
+    void M()
+    {
+        switch (true)
+        {
+            case identifier $$
+        }
+    }
+}
+                              </Document>)
+
+                state.SendTypeChars("w")
+                Await state.AssertSelectedCompletionItem(displayText:="when", isHardSelected:=False)
+
+                state.SendBackspace()
+                state.SendTypeChars("i")
+                Await state.AssertSelectedCompletionItem(displayText:="identifier", isHardSelected:=False)
+
+            End Using
+        End Function
+
+        <WpfFact, Trait(Traits.Feature, Traits.Features.Completion)>
+        Public Async Function AfterIdentifierInCaseLabel_ColorColor() As Task
+            Using state = TestState.CreateCSharpTestState(
+                              <Document>
+class identifier { }
+class C
+{
+    const identifier identifier = null;
+    void M()
+    {
+        switch (true)
+        {
+            case identifier $$
+        }
+    }
+}
+                              </Document>)
+
+                state.SendTypeChars("w")
+                Await state.AssertSelectedCompletionItem(displayText:="when", isHardSelected:=False)
+
+                state.SendBackspace()
+                state.SendTypeChars("i")
+                Await state.AssertSelectedCompletionItem(displayText:="identifier", isHardSelected:=False)
+
+            End Using
+        End Function
+
+        <WpfFact, Trait(Traits.Feature, Traits.Features.Completion)>
+        Public Async Function AfterIdentifierInCaseLabel_ClassNameOnly() As Task
+            Using state = TestState.CreateCSharpTestState(
+                              <Document>
+class identifier { }
+class C
+{
+    void M()
+    {
+        switch (true)
+        {
+            case identifier $$
+        }
+    }
+}
+                              </Document>)
+
+                state.SendTypeChars("w")
+                Await state.AssertSelectedCompletionItem(displayText:="identifier", isHardSelected:=False)
+
+                state.SendBackspace()
+                state.SendTypeChars("i")
+                Await state.AssertSelectedCompletionItem(displayText:="identifier", isHardSelected:=False)
+
+            End Using
+        End Function
+
+        <WpfFact, Trait(Traits.Feature, Traits.Features.Completion)>
+        Public Async Function AfterDoubleIdentifierInCaseLabel() As Task
+            Using state = TestState.CreateCSharpTestState(
+                              <Document>
+class C
+{
+    void M()
+    {
+        switch (true)
+        {
+            case identifier identifier $$
+        }
+    }
+}
+                              </Document>)
+
+                state.SendTypeChars("w")
+                Await state.AssertSelectedCompletionItem(displayText:="when", isHardSelected:=True)
+
+            End Using
+        End Function
+
         Private Class MultipleChangeCompletionProvider
             Inherits CompletionProvider
 

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

@@ -2,8 +2,6 @@
 
 using System.Threading;
 using Microsoft.CodeAnalysis.CSharp.Extensions.ContextQuery;
-using Microsoft.CodeAnalysis.CSharp.Syntax;
-using Microsoft.CodeAnalysis.Shared.Extensions;
 
 namespace Microsoft.CodeAnalysis.CSharp.Completion.KeywordRecommenders
 {
@@ -15,36 +13,8 @@ public NullKeywordRecommender()
         }
 
         protected override bool IsValidContext(int position, CSharpSyntaxContext context, CancellationToken cancellationToken)
-        {
-            var syntaxTree = context.SyntaxTree;
-
-            return
-                context.IsAnyExpressionContext ||
-                context.IsStatementContext ||
-                context.IsGlobalStatementContext ||
-                IsInSelectCaseContext(context, cancellationToken);
-        }
-
-        private bool IsInSelectCaseContext(CSharpSyntaxContext context, CancellationToken cancellationToken)
-        {
-            var token = context.TargetToken;
-            if (token.Kind() != SyntaxKind.CaseKeyword)
-            {
-                return false;
-            }
-
-            var switchStatement = token.GetAncestor<SwitchStatementSyntax>();
-            if (switchStatement != null)
-            {
-                var info = context.SemanticModel.GetTypeInfo(switchStatement.Expression, cancellationToken);
-                if (info.Type != null &&
-                    info.Type.IsValueType)
-                {
-                    return false;
-                }
-            }
-
-            return true;
-        }
+            => context.IsAnyExpressionContext ||
+               context.IsStatementContext ||
+               context.IsGlobalStatementContext;
     }
 }

src/Features/CSharp/Portable/Completion/KeywordRecommenders/WhenKeywordRecommender.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.Linq;
 using System.Threading;
 using Microsoft.CodeAnalysis.CSharp.Extensions;
 using Microsoft.CodeAnalysis.CSharp.Extensions.ContextQuery;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Shared.Extensions;
 
 namespace Microsoft.CodeAnalysis.CSharp.Completion.KeywordRecommenders
 {
@@ -15,26 +18,99 @@ public WhenKeywordRecommender()
 
         protected override bool IsValidContext(int position, CSharpSyntaxContext context, CancellationToken cancellationToken)
         {
-            if (context.IsCatchFilterContext)
+            return context.IsCatchFilterContext ||
+                (IsAfterCompleteExpressionOrPatternInCaseLabel(context, out var expressionOrPattern) &&
+                !IsTypeName(expressionOrPattern, context.SemanticModel, cancellationToken));
+        }
+
+        private static bool IsAfterCompleteExpressionOrPatternInCaseLabel(CSharpSyntaxContext context,
+            out SyntaxNode expressionOrPattern)
+        {
+            var switchLabel = context.TargetToken.GetAncestor<SwitchLabelSyntax>();
+            if (switchLabel == null)
+            {
+                expressionOrPattern = null;
+                return false;
+            }
+
+            expressionOrPattern = switchLabel.ChildNodes().FirstOrDefault();
+            if (expressionOrPattern == null)
+            {
+                // It must have been a default label.
+                return false;
+            }
+
+            // If the last token is missing, the expression is incomplete - possibly because of missing parentheses,
+            // but not necessarily. We don't want to offer 'when' in those cases. Here are some examples that illustrate this:
+            // case |
+            // case x.|
+            // case 1 + |
+            // case (1 + 1 |
+
+            // Also note that if there's a missing token inside the expression, that's fine and we do offer 'when':
+            // case (1 + ) |
+
+            // context.TargetToken does not include zero width so in case of a missing token, these will never be equal.
+            var lastToken = expressionOrPattern.GetLastToken(includeZeroWidth: true);
+            if (lastToken == context.TargetToken)
             {
                 return true;
             }
 
-            // case int i $$
-            // case int i w$$
-            if (IsIdentifierInCasePattern(context.TargetToken.Parent))
+            if (lastToken == context.LeftToken && expressionOrPattern is DeclarationPatternSyntax declarationPattern)
             {
+                // case constant w|
+
+                // The user is typing a new word (might be a partially written 'when' keyword), which causes this to be parsed
+                // as a declaration pattern. lastToken will be 'w' (LeftToken) as opposed to 'constant' (TargetToken).
+                // However we'd like to pretend that this is not the case and that we just a have single expression
+                // with 'constant' as if the new word didn't exist. Let's do that by adjusting our variable.
+
+                expressionOrPattern = declarationPattern.Type;
                 return true;
             }
 
             return false;
         }
 
-        private static bool IsIdentifierInCasePattern(SyntaxNode node)
+        private static bool IsTypeName(SyntaxNode expressionOrPattern, SemanticModel semanticModel,
+            CancellationToken cancellationToken)
         {
-            return node.IsKind(SyntaxKind.SingleVariableDesignation)
-                && node.IsParentKind(SyntaxKind.DeclarationPattern)
-                && node.Parent.IsParentKind(SyntaxKind.CasePatternSwitchLabel);
+            // Syntactically, everything works out. We're in a pretty good spot to show 'when' now.
+            // But let's not do it just yet... Consider these cases:
+            // case SyntaxNode |
+            // case SyntaxNode w|
+            // If what we have here is known to be a type, we don't want to clutter the variable name suggestion list
+            // with 'when' since we know that the resulting code would be semantically invalid.
+
+            var expression = expressionOrPattern as ExpressionSyntax
+                ?? (expressionOrPattern as ConstantPatternSyntax)?.Expression;
+            
+            if (!(expression is TypeSyntax typeSyntax))
+            {
+                return false;
+            }
+
+            // We don't pass in the semantic model - let IsPotentialTypeName handle the cases where it's clear
+            // from the syntax, but other than that, we need to do our own logic here.
+            if (typeSyntax.IsPotentialTypeName(semanticModelOpt: null, cancellationToken))
+            {
+                return true;
+            }
+
+            var symbols = semanticModel.LookupName(typeSyntax, namespacesAndTypesOnly: false, cancellationToken);
+            if (symbols.Length == 0)
+            {
+                // For all unknown identifiers except var, we return false (therefore 'when' will be offered),
+                // because the user could later create a type with this name OR a constant. We give them both options.
+                // But with var, when there is no type or constant with this name, we instead make the reasonable
+                // assumption that the user didn't just type 'var' to then create a constant named 'var', but really
+                // is about to declare a variable. Therefore we don't want to interfere with the declaration.
+                // However note that if such a constant already exists, we do the right thing and do offer 'when'.
+                return typeSyntax.IsVar;
+            }
+            
+            return symbols.All(symbol => symbol is IAliasSymbol || symbol is ITypeSymbol);
         }
     }
 }

src/Workspaces/CSharp/Portable/Extensions/ContextQuery/SyntaxTreeExtensions.cs

@@ -1322,7 +1322,8 @@ public static bool IsPossibleTupleOpenParenOrComma(SyntaxToken possibleCommaOrPa
             // The well-formed cases:
             // var ($$, y) = e;
             // (var $$, var y) = e;
-            if (leftToken.Parent.IsKind(SyntaxKind.SingleVariableDesignation, SyntaxKind.ParenthesizedVariableDesignation))
+            if (leftToken.Parent.IsKind(SyntaxKind.ParenthesizedVariableDesignation) ||
+                leftToken.Parent.IsParentKind(SyntaxKind.ParenthesizedVariableDesignation))
             {
                 return true;
             }
@@ -1795,17 +1796,16 @@ public static bool IsDefiniteCastTypeContext(this SyntaxTree syntaxTree, int pos
             return false;
         }
 
-        public static bool IsConstantExpressionContext(this SyntaxTree syntaxTree, int position, SyntaxToken tokenOnLeftOfPosition, CancellationToken cancellationToken)
+        public static bool IsConstantExpressionContext(this SyntaxTree syntaxTree, int position,
+            SyntaxToken tokenOnLeftOfPosition, CancellationToken cancellationToken)
         {
-            var token = tokenOnLeftOfPosition.GetPreviousTokenIfTouchingWord(position);
-
-            // case |
-            if (token.IsKind(SyntaxKind.CaseKeyword) &&
-                token.Parent.IsKind(SyntaxKind.CaseSwitchLabel))
+            if (IsPatternContext(syntaxTree, tokenOnLeftOfPosition, position))
             {
                 return true;
             }
 
+            var token = tokenOnLeftOfPosition.GetPreviousTokenIfTouchingWord(position);
+
             // goto case |
             if (token.IsKind(SyntaxKind.CaseKeyword) &&
                 token.Parent.IsKind(SyntaxKind.GotoCaseStatement))
@@ -2410,6 +2410,12 @@ public static bool IsLabelContext(this SyntaxTree syntaxTree, int position, Canc
                 return true;
             }
 
+            // case ... when |
+            if (token.IsKind(SyntaxKind.WhenKeyword) && token.Parent.IsKind(SyntaxKind.WhenClause))
+            {
+                return true;
+            }
+
             // (SomeType) |
             if (token.IsAfterPossibleCast())
             {

src/Workspaces/CSharp/Portable/Extensions/TypeSyntaxExtensions.cs

@@ -51,7 +51,7 @@ public static bool IsPotentialTypeName(this TypeSyntax typeSyntax, SemanticModel
 
             var nameToken = nameSyntax.GetNameToken();
 
-            var symbols = semanticModelOpt.LookupName(nameToken, namespacesAndTypesOnly: true, cancellationToken: cancellationToken);
+            var symbols = semanticModelOpt.LookupName(nameToken, namespacesAndTypesOnly: true, cancellationToken);
             var firstSymbol = symbols.FirstOrDefault();
 
             var typeSymbol = firstSymbol != null && firstSymbol.Kind == SymbolKind.Alias