From e31a0c4f0a1227f6fc3c5eca4fd39059e42176b2 Mon Sep 17 00:00:00 2001 From: Allison Chou Date: Tue, 21 Apr 2020 14:35:38 -0700 Subject: [PATCH] Code review feedback --- .../AndKeywordRecommenderTests.cs | 51 ++++++++++++ .../OrKeywordRecommenderTests.cs | 51 ++++++++++++ .../Extensions/SyntaxTokenExtensions.cs | 2 +- .../ContextQuery/SyntaxTreeExtensions.cs | 82 ++++++++++--------- 4 files changed, 147 insertions(+), 39 deletions(-) diff --git a/src/EditorFeatures/CSharpTest2/Recommendations/AndKeywordRecommenderTests.cs b/src/EditorFeatures/CSharpTest2/Recommendations/AndKeywordRecommenderTests.cs index 0ae68fb15ef..f0f52bf0651 100644 --- a/src/EditorFeatures/CSharpTest2/Recommendations/AndKeywordRecommenderTests.cs +++ b/src/EditorFeatures/CSharpTest2/Recommendations/AndKeywordRecommenderTests.cs @@ -126,6 +126,23 @@ void M() if (e is C2.P $$"); } + [Fact, Trait(Traits.Feature, Traits.Features.KeywordRecommending)] + public async Task TestAfterQualifiedName2() + { + await VerifyKeywordAsync( +@" +namespace N +{ + class C + { + const int P = 1; + + void M() + { + var e = new object(); + if (e is N.C.P $$"); + } + [Fact, Trait(Traits.Feature, Traits.Features.KeywordRecommending)] public async Task TestAtBeginningOfSwitchExpression() { @@ -180,6 +197,40 @@ public async Task TestAtBeginningOfSwitchStatement_AfterMultipleOpenParens() case (((1 $$")); } + [Fact, Trait(Traits.Feature, Traits.Features.KeywordRecommending)] + public async Task TestAtBeginningOfSwitchStatement_AfterMultipleOpenParens_MemberAccessExpression() + { + await VerifyKeywordAsync( +@"namespace N +{ + class C + { + const int P = 1; + + void M() + { + var e = new object(); + switch (e) + { + case (((N.C.P $$"); + } + + [Fact, Trait(Traits.Feature, Traits.Features.KeywordRecommending)] + public async Task TestAtBeginningOfSwitchStatement_AfterMultipleOpenParens_MemberAccessExpression2() + { + await VerifyKeywordAsync( +@"namespace N +{ + class C + { + void M() + { + var e = new object(); + switch (e) + { + case (((N.C $$"); + } + [Fact, Trait(Traits.Feature, Traits.Features.KeywordRecommending)] public async Task TestMissingAfterIsKeyword() { diff --git a/src/EditorFeatures/CSharpTest2/Recommendations/OrKeywordRecommenderTests.cs b/src/EditorFeatures/CSharpTest2/Recommendations/OrKeywordRecommenderTests.cs index 1db45987a6a..278205853fb 100644 --- a/src/EditorFeatures/CSharpTest2/Recommendations/OrKeywordRecommenderTests.cs +++ b/src/EditorFeatures/CSharpTest2/Recommendations/OrKeywordRecommenderTests.cs @@ -83,6 +83,40 @@ void M() if (C2 is { P: (((1 $$"); } + [Fact, Trait(Traits.Feature, Traits.Features.KeywordRecommending)] + public async Task TestAtBeginningOfSwitchStatement_AfterMultipleOpenParens_MemberAccessExpression() + { + await VerifyKeywordAsync( +@"namespace N +{ + class C + { + const int P = 1; + + void M() + { + var e = new object(); + switch (e) + { + case (((N.C.P $$"); + } + + [Fact, Trait(Traits.Feature, Traits.Features.KeywordRecommending)] + public async Task TestAtBeginningOfSwitchStatement_AfterMultipleOpenParens_MemberAccessExpression2() + { + await VerifyKeywordAsync( +@"namespace N +{ + class C + { + void M() + { + var e = new object(); + switch (e) + { + case (((N.C $$"); + } + [Fact, Trait(Traits.Feature, Traits.Features.KeywordRecommending)] public async Task TestInsideSubpattern_AfterParenPair() { @@ -126,6 +160,23 @@ void M() if (e is C2.P $$"); } + [Fact, Trait(Traits.Feature, Traits.Features.KeywordRecommending)] + public async Task TestAfterQualifiedName2() + { + await VerifyKeywordAsync( +@" +namespace N +{ + class C + { + const int P = 1; + + void M() + { + var e = new object(); + if (e is N.C.P $$"); + } + [Fact, Trait(Traits.Feature, Traits.Features.KeywordRecommending)] public async Task TestAtBeginningOfSwitchExpression() { diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Extensions/SyntaxTokenExtensions.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Extensions/SyntaxTokenExtensions.cs index 16c9adf7b05..e41579a4fe1 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Extensions/SyntaxTokenExtensions.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Extensions/SyntaxTokenExtensions.cs @@ -21,7 +21,7 @@ internal static partial class SyntaxTokenExtensions public static bool IsLastTokenOfNode(this SyntaxToken token, [NotNullWhen(true)] out T node) where T : SyntaxNode { node = token.GetAncestor(); - return node != null && token == node.GetLastToken(); + return node != null && token == node.GetLastToken(includeZeroWidth: true); } public static bool IsKindOrHasMatchingText(this SyntaxToken token, SyntaxKind kind) diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ContextQuery/SyntaxTreeExtensions.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ContextQuery/SyntaxTreeExtensions.cs index 0dc157f2bf9..c549dabca19 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ContextQuery/SyntaxTreeExtensions.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ContextQuery/SyntaxTreeExtensions.cs @@ -1257,20 +1257,20 @@ public static bool IsAtStartOfPattern(this SyntaxTree syntaxTree, SyntaxToken le { leftToken = leftToken.GetPreviousTokenIfTouchingWord(position); - // If we're dealing with an expression surrounded by one or more sets of open parentheses, we need to - // walk up the parens in order to see if we're actually at the start of a valid pattern or not. - // After recursing, we should be left with at most a single open paren to analyze. if (leftToken.IsKind(SyntaxKind.OpenParenToken)) { - if (leftToken.Parent.IsParentKind(SyntaxKind.ParenthesizedExpression)) + if (leftToken.Parent.IsKind(SyntaxKind.ParenthesizedExpression)) { - return IsAtStartOfPattern(syntaxTree, leftToken.Parent.Parent.GetFirstToken(), leftToken.Parent.Parent.SpanStart); + // If we're dealing with an expression surrounded by one or more sets of open parentheses, we need to + // walk up the parens in order to see if we're actually at the start of a valid pattern or not. + var tokenPrecedingParenthesizedExpression = leftToken.Parent.GetFirstToken().GetPreviousToken(); + return IsAtStartOfPattern(syntaxTree, tokenPrecedingParenthesizedExpression, tokenPrecedingParenthesizedExpression.Span.End + 1); } #if !CODE_STYLE - if (leftToken.Parent.IsParentKind(SyntaxKind.ParenthesizedPattern)) + if (leftToken.Parent.IsKind(SyntaxKind.ParenthesizedPattern)) { - return IsAtStartOfPattern(syntaxTree, leftToken.Parent.Parent.GetFirstToken(), leftToken.Parent.Parent.SpanStart); + return true; } #endif } @@ -1282,12 +1282,6 @@ public static bool IsAtStartOfPattern(this SyntaxTree syntaxTree, SyntaxToken le return true; } - // case ($ - if (leftToken.IsKind(SyntaxKind.OpenParenToken) && leftToken.Parent.IsParentKind(SyntaxKind.CaseSwitchLabel)) - { - return true; - } - // e switch { $$ // e switch { ..., $$ if (leftToken.IsKind(SyntaxKind.OpenBraceToken, SyntaxKind.CommaToken) && leftToken.Parent.IsKind(SyntaxKind.SwitchExpression)) @@ -1310,12 +1304,6 @@ public static bool IsAtStartOfPattern(this SyntaxTree syntaxTree, SyntaxToken le return true; } - // e is { P: ($$ - if (leftToken.IsKind(SyntaxKind.OpenParenToken) && leftToken.Parent.IsParentKind(SyntaxKind.ConstantPattern)) - { - return true; - } - #if !CODE_STYLE // e is 1 and $$ // e is 1 or $$ @@ -1329,12 +1317,6 @@ public static bool IsAtStartOfPattern(this SyntaxTree syntaxTree, SyntaxToken le { return true; } - - // e is ($$ 1 or 2) - if (leftToken.IsKind(SyntaxKind.OpenParenToken) && leftToken.Parent.IsKind(SyntaxKind.ParenthesizedPattern)) - { - return true; - } #endif return false; @@ -1344,8 +1326,7 @@ public static bool IsAtEndOfPattern(this SyntaxTree syntaxTree, SyntaxToken left { #if CODE_STYLE return false; -#endif - +#else var originalLeftToken = leftToken; leftToken = leftToken.GetPreviousTokenIfTouchingWord(position); @@ -1383,30 +1364,55 @@ public static bool IsAtEndOfPattern(this SyntaxTree syntaxTree, SyntaxToken left // e is int $$ if (leftToken.IsLastTokenOfNode(out var typeSyntax)) { + // If typeSyntax is part of a qualified name, we want to get the fully-qualified name so that we can + // later accurately perform the check comparing the right side of the BinaryExpressionSyntax to + // the typeSyntax. + while (typeSyntax.Parent is QualifiedNameSyntax qualifiedNameSyntax) + { + typeSyntax = qualifiedNameSyntax; + } + var binaryExpressionSyntax = typeSyntax.GetAncestor(); if (binaryExpressionSyntax != null && binaryExpressionSyntax.OperatorToken.IsKind(SyntaxKind.IsKeyword) && - binaryExpressionSyntax.Right.Contains(typeSyntax)) + binaryExpressionSyntax.Right == typeSyntax) { return true; } } // We need to include a special case for switch statement cases, as some are not currently parsed as patterns, e.g. case (1 $$ - var node = leftToken.Parent; - - // Getting rid of the extra parentheses to deal with cases such as 'case (((1 $$' - while (node.IsParentKind(SyntaxKind.ParenthesizedExpression)) - { - node = node.Parent; - } - - // case (1 $$ - if (node.IsParentKind(SyntaxKind.CaseSwitchLabel) && node.Parent.IsParentKind(SyntaxKind.SwitchSection)) + if (IsAtEndOfSwitchStatementPattern(leftToken)) { return true; } return false; + + static bool IsAtEndOfSwitchStatementPattern(SyntaxToken leftToken) + { + var node = leftToken.Parent; + + // Walking up the tree for expressions such as 'case (((N.C.P $$' + while (node.IsParentKind(SyntaxKind.SimpleMemberAccessExpression)) + { + node = node.Parent; + } + + // Getting rid of the extra parentheses to deal with cases such as 'case (((1 $$' + while (node.IsParentKind(SyntaxKind.ParenthesizedExpression)) + { + node = node.Parent; + } + + // case (1 $$ + if (node.IsParentKind(SyntaxKind.CaseSwitchLabel) && node.Parent.IsParentKind(SyntaxKind.SwitchSection)) + { + return true; + } + + return false; + } +#endif } private static SyntaxToken FindTokenOnLeftOfNode(SyntaxNode node) -- GitLab