diff --git a/src/EditorFeatures/CSharpTest2/Recommendations/WhenKeywordRecommenderTests.cs b/src/EditorFeatures/CSharpTest2/Recommendations/WhenKeywordRecommenderTests.cs index c31c4bae08423..46865ac0276e0 100644 --- a/src/EditorFeatures/CSharpTest2/Recommendations/WhenKeywordRecommenderTests.cs +++ b/src/EditorFeatures/CSharpTest2/Recommendations/WhenKeywordRecommenderTests.cs @@ -195,31 +195,31 @@ public async Task TestForSwitchCase_NotInEmptySwitchStatement() => [Fact, Trait(Traits.Feature, Traits.Features.KeywordRecommending)] public async Task TestForSwitchCase_SemanticCheck_NotAfterPredefinedType() => - await VerifyAbsenceAsync(AddInsideMethod(@"switch (new object()) { case int $$ }")); + await VerifyKeywordAsync(AddInsideMethod(@"switch (new object()) { case int $$ }")); [Fact, Trait(Traits.Feature, Traits.Features.KeywordRecommending)] public async Task TestForSwitchCase_SemanticCheck_NotAfterPredefinedType_BeforeBreak() => - await VerifyAbsenceAsync(AddInsideMethod(@"switch (new object()) { case int $$ break; }")); + await VerifyKeywordAsync(AddInsideMethod(@"switch (new object()) { case int $$ break; }")); [Fact, Trait(Traits.Feature, Traits.Features.KeywordRecommending)] public async Task TestForSwitchCase_SemanticCheck_NotAfterPredefinedType_BeforeWhen() => - await VerifyAbsenceAsync(AddInsideMethod(@"switch (new object()) { case int $$ when }")); + await VerifyKeywordAsync(AddInsideMethod(@"switch (new object()) { case int $$ when }")); [Fact, Trait(Traits.Feature, Traits.Features.KeywordRecommending)] public async Task TestForSwitchCase_SemanticCheck_NotAfterGenericType() => - await VerifyAbsenceAsync(AddInsideMethod(@"switch (new object()) { case Dictionary $$ }")); + await VerifyKeywordAsync(AddInsideMethod(@"switch (new object()) { case Dictionary $$ }")); [Fact, Trait(Traits.Feature, Traits.Features.KeywordRecommending)] public async Task TestForSwitchCase_SemanticCheck_NotAfterGenericType_BeforeBreak() => - await VerifyAbsenceAsync(AddInsideMethod(@"switch (new object()) { case Dictionary $$ break; }")); + await VerifyKeywordAsync(AddInsideMethod(@"switch (new object()) { case Dictionary $$ break; }")); [Fact, Trait(Traits.Feature, Traits.Features.KeywordRecommending)] public async Task TestForSwitchCase_SemanticCheck_NotAfterGenericType_BeforeWhen() => - await VerifyAbsenceAsync(AddInsideMethod(@"switch (new object()) { case Dictionary $$ when }")); + await VerifyKeywordAsync(AddInsideMethod(@"switch (new object()) { case Dictionary $$ when }")); [Fact, Trait(Traits.Feature, Traits.Features.KeywordRecommending)] public async Task TestForSwitchCase_SemanticCheck_NotAfterCustomType() => - await VerifyAbsenceAsync(@" + await VerifyKeywordAsync(@" class SyntaxNode { } class C { @@ -228,7 +228,7 @@ class C [Fact, Trait(Traits.Feature, Traits.Features.KeywordRecommending)] public async Task TestForSwitchCase_SemanticCheck_NotAfterCustomType_BeforeBreak() => - await VerifyAbsenceAsync(@" + await VerifyKeywordAsync(@" class SyntaxNode { } class C { @@ -237,7 +237,7 @@ class C [Fact, Trait(Traits.Feature, Traits.Features.KeywordRecommending)] public async Task TestForSwitchCase_SemanticCheck_NotAfterCustomType_BeforeWhen() => - await VerifyAbsenceAsync(@" + await VerifyKeywordAsync(@" class SyntaxNode { } class C { @@ -246,7 +246,7 @@ class C [Fact, Trait(Traits.Feature, Traits.Features.KeywordRecommending)] public async Task TestForSwitchCase_SemanticCheck_NotAfterTypeAlias() => - await VerifyAbsenceAsync(@" + await VerifyKeywordAsync(@" using Type = System.String; class C { @@ -255,7 +255,7 @@ class C [Fact, Trait(Traits.Feature, Traits.Features.KeywordRecommending)] public async Task TestForSwitchCase_SemanticCheck_NotAfterTypeAlias_BeforeBreak() => - await VerifyAbsenceAsync(@" + await VerifyKeywordAsync(@" using Type = System.String; class C { @@ -264,7 +264,7 @@ class C [Fact, Trait(Traits.Feature, Traits.Features.KeywordRecommending)] public async Task TestForSwitchCase_SemanticCheck_NotAfterTypeAlias_BeforeWhen() => - await VerifyAbsenceAsync(@" + await VerifyKeywordAsync(@" using Type = System.String; class C { @@ -273,7 +273,7 @@ class C [Fact, Trait(Traits.Feature, Traits.Features.KeywordRecommending)] public async Task TestForSwitchCase_SemanticCheck_NotAfterOverloadedTypeName() => - await VerifyAbsenceAsync(@" + await VerifyKeywordAsync(@" class ValueTuple { } class ValueTuple { } class C @@ -283,7 +283,7 @@ class C [Fact, Trait(Traits.Feature, Traits.Features.KeywordRecommending)] public async Task TestForSwitchCase_SemanticCheck_NotAfterOverloadedTypeName_BeforeBreak() => - await VerifyAbsenceAsync(@" + await VerifyKeywordAsync(@" class ValueTuple { } class ValueTuple { } class C @@ -293,7 +293,7 @@ class C [Fact, Trait(Traits.Feature, Traits.Features.KeywordRecommending)] public async Task TestForSwitchCase_SemanticCheck_NotAfterOverloadedTypeName_BeforeWhen() => - await VerifyAbsenceAsync(@" + await VerifyKeywordAsync(@" class ValueTuple { } class ValueTuple { } class C @@ -462,19 +462,19 @@ class C [Fact, Trait(Traits.Feature, Traits.Features.KeywordRecommending)] public async Task TestForSwitchCase_SemanticCheck_AfterLocalConstantVar() => - await VerifyKeywordAsync(AddInsideMethod(@"const object var = null; switch (new object()) { case var $$ }")); + await VerifyAbsenceAsync(AddInsideMethod(@"const object var = null; switch (new object()) { case var $$ }")); [Fact, Trait(Traits.Feature, Traits.Features.KeywordRecommending)] public async Task TestForSwitchCase_SemanticCheck_AfterLocalConstantVar_BeforeBreak() => - await VerifyKeywordAsync(AddInsideMethod(@"const object var = null; switch (new object()) { case var $$ break; }")); + await VerifyAbsenceAsync(AddInsideMethod(@"const object var = null; switch (new object()) { case var $$ break; }")); [Fact, Trait(Traits.Feature, Traits.Features.KeywordRecommending)] public async Task TestForSwitchCase_SemanticCheck_AfterLocalConstantVar_BeforeWhen() => - await VerifyKeywordAsync(AddInsideMethod(@"const object var = null; switch (new object()) { case var $$ when }")); + await VerifyAbsenceAsync(AddInsideMethod(@"const object var = null; switch (new object()) { case var $$ when }")); [Fact, Trait(Traits.Feature, Traits.Features.KeywordRecommending)] public async Task TestForSwitchCase_SemanticCheck_AfterClassAndLocalConstantVar() => - await VerifyKeywordAsync(@" + await VerifyAbsenceAsync(@" class var { } class C { @@ -483,7 +483,7 @@ class C [Fact, Trait(Traits.Feature, Traits.Features.KeywordRecommending)] public async Task TestForSwitchCase_SemanticCheck_AfterClassAndLocalConstantVar_BeforeBreak() => - await VerifyKeywordAsync(@" + await VerifyAbsenceAsync(@" class var { } class C { @@ -492,7 +492,7 @@ class C [Fact, Trait(Traits.Feature, Traits.Features.KeywordRecommending)] public async Task TestForSwitchCase_SemanticCheck_AfterClassAndLocalConstantVar_BeforeWhen() => - await VerifyKeywordAsync(@" + await VerifyAbsenceAsync(@" class var { } class C { @@ -502,7 +502,7 @@ class C [Fact, Trait(Traits.Feature, Traits.Features.KeywordRecommending)] public async Task TestForSwitchCase_SemanticCheck_AfterTypeAliasAndFieldConstantVar() { - await VerifyKeywordAsync(@" + await VerifyAbsenceAsync(@" using var = System.String; class C { @@ -514,7 +514,7 @@ class C [Fact, Trait(Traits.Feature, Traits.Features.KeywordRecommending)] public async Task TestForSwitchCase_SemanticCheck_AfterTypeAliasAndFieldConstantVar_BeforeBreak() { - await VerifyKeywordAsync(@" + await VerifyAbsenceAsync(@" using var = System.String; class C { @@ -526,7 +526,7 @@ class C [Fact, Trait(Traits.Feature, Traits.Features.KeywordRecommending)] public async Task TestForSwitchCase_SemanticCheck_AfterTypeAliasAndFieldConstantVar_BeforeWhen() { - await VerifyKeywordAsync(@" + await VerifyAbsenceAsync(@" using var = System.String; class C { diff --git a/src/Features/CSharp/Portable/Completion/KeywordRecommenders/WhenKeywordRecommender.cs b/src/Features/CSharp/Portable/Completion/KeywordRecommenders/WhenKeywordRecommender.cs index f7c27f9902352..50386626f3f1c 100644 --- a/src/Features/CSharp/Portable/Completion/KeywordRecommenders/WhenKeywordRecommender.cs +++ b/src/Features/CSharp/Portable/Completion/KeywordRecommenders/WhenKeywordRecommender.cs @@ -22,20 +22,14 @@ public WhenKeywordRecommender() protected override bool IsValidContext(int position, CSharpSyntaxContext context, CancellationToken cancellationToken) { return context.IsCatchFilterContext || - (IsAfterCompleteExpressionOrPatternInCaseLabel(context, out var expressionOrPattern) && - !IsTypeName(expressionOrPattern, context.SemanticModel, cancellationToken)); + IsAfterCompleteExpressionOrPatternInCaseLabel(context); } - private static bool IsAfterCompleteExpressionOrPatternInCaseLabel(CSharpSyntaxContext context, - out SyntaxNodeOrToken nodeOrToken) + private static bool IsAfterCompleteExpressionOrPatternInCaseLabel(CSharpSyntaxContext context) { - nodeOrToken = null; - var switchLabel = context.TargetToken.GetAncestor(); if (switchLabel == null) - { return false; - } var expressionOrPattern = switchLabel.ChildNodes().FirstOrDefault(); if (expressionOrPattern == null) @@ -44,6 +38,9 @@ private static bool IsAfterCompleteExpressionOrPatternInCaseLabel(CSharpSyntaxCo return false; } + if (context.TargetToken.Text == "var") + 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 | @@ -55,122 +52,27 @@ private static bool IsAfterCompleteExpressionOrPatternInCaseLabel(CSharpSyntaxCo // case (1 + ) | if (expressionOrPattern.GetLastToken(includeZeroWidth: true).IsMissing) - { return false; - } - - // There are zero width tokens that are not "missing" (inserted by the parser) because they are optional, - // such as the identifier in a recursive pattern. We want to ignore those now, so we exclude all zero width. var lastToken = expressionOrPattern.GetLastToken(includeZeroWidth: false); + + // We're writing past the end of a complete pattern. This is a place where 'when' could be added to add + // restrictions on the pattern. if (lastToken == context.TargetToken) - { - nodeOrToken = expressionOrPattern; return true; - } + // We're writing the last token of a pattern. In this case, we might either be writing a name for the pattern + // (like `case Wolf w:`) or we might be starting to write `when` (like `case Wolf when ...:`). if (lastToken == context.LeftToken) { - // The user is typing a new word (might be a partially written 'when' keyword), - // which is part of the pattern as opposed to appearing outside of it. In a few special cases, - // this word can actually be replaced with 'when' and the resulting pattern would still be valid. - - if (expressionOrPattern is DeclarationPatternSyntax declarationPattern) - { - // The new token causes this to be parsed as a declaration pattern: - // case constant w| ('w' = LeftToken, 'constant' = TargetToken) - - // However 'constant' itself might end up being a valid constant pattern. - // We will pretend as if 'w' didn't exist so that the later check - // for whether 'constant' is actually a type can still work properly. - nodeOrToken = declarationPattern.Type; + if (expressionOrPattern is DeclarationPatternSyntax) return true; - } - - if (expressionOrPattern is VarPatternSyntax varPattern) - { - // The new token causes this to be parsed as a var pattern: - // case var w| ('w' = LeftToken, 'var' = TargetToken) - // However 'var' itself might end up being a valid constant pattern. - nodeOrToken = varPattern.VarKeyword; + if (expressionOrPattern is RecursivePatternSyntax) return true; - } - - if (expressionOrPattern is RecursivePatternSyntax recursivePattern) - { - // The new token is consumed as the identifier in a recursive pattern: - // case { } w| ('w' = LeftToken, '}' = TargetToken) - - // However the identifier is optional and can be replaced by 'when'. - nodeOrToken = recursivePattern.Type; - return true; - } - - // In other cases, this would not be true because the pattern would be incomplete without this word: - // case 1 + w| } return false; } - - private static bool IsTypeName( - SyntaxNodeOrToken nodeOrToken, - SemanticModel semanticModel, - CancellationToken cancellationToken) - { - // 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. - - bool isVar; - ImmutableArray symbols; - - if (nodeOrToken.IsNode) - { - var node = nodeOrToken.AsNode(); - var expression = node as ExpressionSyntax - ?? (node 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; - } - - isVar = typeSyntax.IsVar; - symbols = semanticModel.LookupName(typeSyntax, cancellationToken); - } - else - { - // In a var pattern, the 'var' keyword is not wrapped in a type syntax, so we might just have a naked token. - var token = nodeOrToken.AsToken(); - - isVar = token.Text == SyntaxFacts.GetText(SyntaxKind.VarKeyword); - symbols = semanticModel.LookupSymbols(nodeOrToken.AsToken().SpanStart, null, token.Text); - } - - 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 isVar; - } - - return symbols.All(symbol => symbol is IAliasSymbol || symbol is ITypeSymbol); - } } }