From 4fc1fa4bf205968b0dd556c502a79dbe80cc3519 Mon Sep 17 00:00:00 2001 From: David Wengier Date: Tue, 28 Jul 2020 22:20:39 +1000 Subject: [PATCH] F1 help for private protected and protected internal go to their expected places (#46343) * F1 help for private protected and protected internal go to their expected places * Clean up code analysis messages * Simplify and extract logic * Allow for modifiers in any order with other modifiers in between them because some people are monsters --- .../CSharpHelpContextService.cs | 40 +++++-- .../CSharp/Test/F1Help/F1HelpTests.cs | 105 ++++++++++++++++- src/VisualStudio/Core/Test/Help/HelpTests.vb | 109 +++++++++++++++++- .../VisualBasic/Impl/Help/HelpKeywords.vb | 2 + .../VisualBasicHelpContextService.Visitor.vb | 51 ++++++-- 5 files changed, 285 insertions(+), 22 deletions(-) diff --git a/src/VisualStudio/CSharp/Impl/LanguageService/CSharpHelpContextService.cs b/src/VisualStudio/CSharp/Impl/LanguageService/CSharpHelpContextService.cs index f3a3bc1d18da8..810a26486f13f 100644 --- a/src/VisualStudio/CSharp/Impl/LanguageService/CSharpHelpContextService.cs +++ b/src/VisualStudio/CSharp/Impl/LanguageService/CSharpHelpContextService.cs @@ -106,7 +106,7 @@ public override async Task GetHelpTermAsync(Document document, TextSpan return string.Empty; } - private bool IsValid(SyntaxToken token, TextSpan span) + private static bool IsValid(SyntaxToken token, TextSpan span) { // If the token doesn't actually intersect with our position, give up return token.Kind() == SyntaxKind.EndIfDirectiveTrivia || token.Span.IntersectsWith(span); @@ -115,6 +115,7 @@ private bool IsValid(SyntaxToken token, TextSpan span) private string TryGetText(SyntaxToken token, SemanticModel semanticModel, Document document, ISyntaxFactsService syntaxFacts, CancellationToken cancellationToken) { if (TryGetTextForContextualKeyword(token, out var text) || + TryGetTextForCombinationKeyword(token, syntaxFacts, out text) || TryGetTextForKeyword(token, syntaxFacts, out text) || TryGetTextForPreProcessor(token, syntaxFacts, out text) || TryGetTextForSymbol(token, semanticModel, document, cancellationToken, out text) || @@ -153,9 +154,9 @@ private bool TryGetTextForSymbol(SyntaxToken token, SemanticModel semanticModel, } // Local: return the name if it's the declaration, otherwise the type - if (symbol is ILocalSymbol && !symbol.DeclaringSyntaxReferences.Any(d => d.GetSyntax().DescendantTokens().Contains(token))) + if (symbol is ILocalSymbol localSymbol && !symbol.DeclaringSyntaxReferences.Any(d => d.GetSyntax().DescendantTokens().Contains(token))) { - symbol = ((ILocalSymbol)symbol).Type; + symbol = localSymbol.Type; } // Range variable: use the type @@ -176,7 +177,7 @@ private bool TryGetTextForSymbol(SyntaxToken token, SemanticModel semanticModel, return symbol != null; } - private bool TryGetTextForOperator(SyntaxToken token, Document document, out string text) + private static bool TryGetTextForOperator(SyntaxToken token, Document document, out string text) { var syntaxFacts = document.GetLanguageService(); if (syntaxFacts.IsOperator(token) || syntaxFacts.IsPredefinedOperator(token) || SyntaxFacts.IsAssignmentExpressionOperatorToken(token.Kind())) @@ -225,7 +226,7 @@ private bool TryGetTextForOperator(SyntaxToken token, Document document, out str return false; } - private bool TryGetTextForPreProcessor(SyntaxToken token, ISyntaxFactsService syntaxFacts, out string text) + private static bool TryGetTextForPreProcessor(SyntaxToken token, ISyntaxFactsService syntaxFacts, out string text) { if (syntaxFacts.IsPreprocessorKeyword(token)) { @@ -243,7 +244,7 @@ private bool TryGetTextForPreProcessor(SyntaxToken token, ISyntaxFactsService sy return false; } - private bool TryGetTextForContextualKeyword(SyntaxToken token, out string text) + private static bool TryGetTextForContextualKeyword(SyntaxToken token, out string text) { if (token.Text == "nameof") { @@ -286,8 +287,31 @@ private bool TryGetTextForContextualKeyword(SyntaxToken token, out string text) text = null; return false; } + private static bool TryGetTextForCombinationKeyword(SyntaxToken token, ISyntaxFactsService syntaxFacts, out string text) + { + switch (token.Kind()) + { + case SyntaxKind.PrivateKeyword when ModifiersContains(token, syntaxFacts, SyntaxKind.ProtectedKeyword): + case SyntaxKind.ProtectedKeyword when ModifiersContains(token, syntaxFacts, SyntaxKind.PrivateKeyword): + text = "privateprotected_CSharpKeyword"; + return true; + + case SyntaxKind.ProtectedKeyword when ModifiersContains(token, syntaxFacts, SyntaxKind.InternalKeyword): + case SyntaxKind.InternalKeyword when ModifiersContains(token, syntaxFacts, SyntaxKind.ProtectedKeyword): + text = "protectedinternal_CSharpKeyword"; + return true; + } + + text = null; + return false; + + static bool ModifiersContains(SyntaxToken token, ISyntaxFactsService syntaxFacts, SyntaxKind kind) + { + return syntaxFacts.GetModifiers(token.Parent).Any(t => t.IsKind(kind)); + } + } - private bool TryGetTextForKeyword(SyntaxToken token, ISyntaxFactsService syntaxFacts, out string text) + private static bool TryGetTextForKeyword(SyntaxToken token, ISyntaxFactsService syntaxFacts, out string text) { if (token.Kind() == SyntaxKind.InKeyword) { @@ -311,7 +335,7 @@ private bool TryGetTextForKeyword(SyntaxToken token, ISyntaxFactsService syntaxF } if (token.ValueText == "var" && token.IsKind(SyntaxKind.IdentifierToken) && - token.Parent.Parent is VariableDeclarationSyntax && token.Parent == ((VariableDeclarationSyntax)token.Parent.Parent).Type) + token.Parent.Parent is VariableDeclarationSyntax declaration && token.Parent == declaration.Type) { text = "var_CSharpKeyword"; return true; diff --git a/src/VisualStudio/CSharp/Test/F1Help/F1HelpTests.cs b/src/VisualStudio/CSharp/Test/F1Help/F1HelpTests.cs index 79cd1b5b7e941..6aa1fdf8730f1 100644 --- a/src/VisualStudio/CSharp/Test/F1Help/F1HelpTests.cs +++ b/src/VisualStudio/CSharp/Test/F1Help/F1HelpTests.cs @@ -24,7 +24,7 @@ public class F1HelpTests private static readonly ComposableCatalog s_catalog = TestExportProvider.EntireAssemblyCatalogWithCSharpAndVisualBasic.WithPart(typeof(CSharpHelpContextService)); private static readonly IExportProviderFactory s_exportProviderFactory = ExportProviderCache.GetOrCreateExportProviderFactory(s_catalog); - private async Task TestAsync(string markup, string expectedText) + private static async Task TestAsync(string markup, string expectedText) { using var workspace = TestWorkspace.CreateCSharp(markup, exportProvider: s_exportProviderFactory.CreateExportProvider()); var caret = workspace.Documents.First().CursorPosition; @@ -34,11 +34,112 @@ private async Task TestAsync(string markup, string expectedText) Assert.Equal(expectedText, actualText); } - private async Task Test_KeywordAsync(string markup, string expectedText) + private static async Task Test_KeywordAsync(string markup, string expectedText) { await TestAsync(markup, expectedText + "_CSharpKeyword"); } + [Fact, Trait(Traits.Feature, Traits.Features.F1Help)] + public async Task TestInternal() + { + await Test_KeywordAsync( +@"intern[||]al class C +{ +}", "internal"); + } + + [Fact, Trait(Traits.Feature, Traits.Features.F1Help)] + public async Task TestProtected() + { + await Test_KeywordAsync( +@"public class C +{ + protec[||]ted void goo(); +}", "protected"); + } + + [Fact, Trait(Traits.Feature, Traits.Features.F1Help)] + public async Task TestProtectedInternal1() + { + await Test_KeywordAsync( +@"public class C +{ + internal protec[||]ted void goo(); +}", "protectedinternal"); + } + + [Fact, Trait(Traits.Feature, Traits.Features.F1Help)] + public async Task TestProtectedInternal2() + { + await Test_KeywordAsync( +@"public class C +{ + protec[||]ted internal void goo(); +}", "protectedinternal"); + } + + [Fact, Trait(Traits.Feature, Traits.Features.F1Help)] + public async Task TestPrivateProtected1() + { + await Test_KeywordAsync( +@"public class C +{ + private protec[||]ted void goo(); +}", "privateprotected"); + } + + [Fact, Trait(Traits.Feature, Traits.Features.F1Help)] + public async Task TestPrivateProtected2() + { + await Test_KeywordAsync( +@"public class C +{ + priv[||]ate protected void goo(); +}", "privateprotected"); + } + + [Fact, Trait(Traits.Feature, Traits.Features.F1Help)] + public async Task TestPrivateProtected3() + { + await Test_KeywordAsync( +@"public class C +{ + protected priv[||]ate void goo(); +}", "privateprotected"); + } + + [Fact, Trait(Traits.Feature, Traits.Features.F1Help)] + public async Task TestPrivateProtected4() + { + await Test_KeywordAsync( +@"public class C +{ + prot[||]ected private void goo(); +}", "privateprotected"); + } + + [Fact, Trait(Traits.Feature, Traits.Features.F1Help)] + public async Task TestModifierSoup() + { + await Test_KeywordAsync( + @"public class C +{ + private new prot[||]ected static unsafe void foo() + { + } +}", "privateprotected"); + } + + [Fact, Trait(Traits.Feature, Traits.Features.F1Help)] + public async Task TestModifierSoupField() + { + await Test_KeywordAsync( + @"public class C +{ + new prot[||]ected static unsafe private goo; +}", "privateprotected"); + } + [Fact, Trait(Traits.Feature, Traits.Features.F1Help)] public async Task TestVoid() { diff --git a/src/VisualStudio/Core/Test/Help/HelpTests.vb b/src/VisualStudio/Core/Test/Help/HelpTests.vb index caaa789ed06ca..d51d201e855cb 100644 --- a/src/VisualStudio/Core/Test/Help/HelpTests.vb +++ b/src/VisualStudio/Core/Test/Help/HelpTests.vb @@ -13,7 +13,7 @@ Imports Roslyn.Utilities Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.Help <[UseExportProvider]> Public Class HelpTests - Public Async Function TestAsync(markup As String, expected As String) As Tasks.Task + Public Shared Async Function TestAsync(markup As String, expected As String) As Tasks.Task Using workspace = TestWorkspace.CreateVisualBasic(markup) Dim caret = workspace.Documents.First().CursorPosition Dim service = New VisualBasicHelpContextService() @@ -21,6 +21,113 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.Help End Using End Function + + Public Async Function TestFriend() As Task + Dim text = +Fri[||]end Class G +End Class + + Await TestAsync(text.Value, "vb.Friend") + End Function + + + Public Async Function TestProtected() As Task + Dim text = +Public Class G + Protec[||]ted Sub M() + End Sub +End Class + + Await TestAsync(text.Value, "vb.Protected") + End Function + + + Public Async Function TestProtectedFriend1() As Task + Dim text = +Public Class G + Protec[||]ted Friend Sub M() + End Sub +End Class + + Await TestAsync(text.Value, "vb.ProtectedFriend") + End Function + + + Public Async Function TestProtectedFriend2() As Task + Dim text = +Public Class G + Friend Protec[||]ted Sub M() + End Sub +End Class + + Await TestAsync(text.Value, "vb.ProtectedFriend") + End Function + + + Public Async Function TestPrivateProtected1() As Task + Dim text = +Public Class G + Private Protec[||]ted Sub M() + End Sub +End Class + + Await TestAsync(text.Value, "vb.PrivateProtected") + End Function + + + Public Async Function TestPrivateProtected2() As Task + Dim text = +Public Class G + Priv[||]ate Protected Sub M() + End Sub +End Class + + Await TestAsync(text.Value, "vb.PrivateProtected") + End Function + + + Public Async Function TestPrivateProtected3() As Task + Dim text = +Public Class G + Protected Priv[||]ate Sub M() + End Sub +End Class + + Await TestAsync(text.Value, "vb.PrivateProtected") + End Function + + + Public Async Function TestPrivateProtected4() As Task + Dim text = +Public Class G + Protec[||]ted Private Sub M() + End Sub +End Class + + Await TestAsync(text.Value, "vb.PrivateProtected") + End Function + + + Public Async Function TestModifierSoup() As Task + Dim text = +Public Class G + Protec[||]ted Async Shared Private Sub M() + End Sub +End Class + + Await TestAsync(text.Value, "vb.PrivateProtected") + End Function + + + Public Async Function TestModifierSoupField() As Task + Dim text = +Public Class G + Private Shadows Shared Prot[||]ected foo as Boolean +End Class + + Await TestAsync(text.Value, "vb.PrivateProtected") + End Function + Public Async Function TestAddHandler1() As Task Dim text = diff --git a/src/VisualStudio/VisualBasic/Impl/Help/HelpKeywords.vb b/src/VisualStudio/VisualBasic/Impl/Help/HelpKeywords.vb index ec8d447132d09..f83ea56ea2d8a 100644 --- a/src/VisualStudio/VisualBasic/Impl/Help/HelpKeywords.vb +++ b/src/VisualStudio/VisualBasic/Impl/Help/HelpKeywords.vb @@ -91,6 +91,8 @@ Namespace Microsoft.VisualStudio.LanguageServices.VisualBasic.Help Friend Const Iterator As String = "vb.Iterator" Friend Const Await As String = "vb.Await" Friend Const Yield As String = "vb.Yield" + Friend Const PrivateProtected As String = "vb.PrivateProtected" + Friend Const ProtectedFriend As String = "vb.ProtectedFriend" End Class diff --git a/src/VisualStudio/VisualBasic/Impl/Help/VisualBasicHelpContextService.Visitor.vb b/src/VisualStudio/VisualBasic/Impl/Help/VisualBasicHelpContextService.Visitor.vb index d2f9581a53729..3c7b4c3e60236 100644 --- a/src/VisualStudio/VisualBasic/Impl/Help/VisualBasicHelpContextService.Visitor.vb +++ b/src/VisualStudio/VisualBasic/Impl/Help/VisualBasicHelpContextService.Visitor.vb @@ -15,7 +15,7 @@ Namespace Microsoft.VisualStudio.LanguageServices.VisualBasic.Help Private Class Visitor Inherits VisualBasicSyntaxVisitor - Public result As String = Nothing + Public result As String Private ReadOnly _span As TextSpan Private ReadOnly _semanticModel As SemanticModel Private ReadOnly _service As VisualBasicHelpContextService @@ -30,11 +30,11 @@ Namespace Microsoft.VisualStudio.LanguageServices.VisualBasic.Help Me._cancellationToken = cancellationToken End Sub - Private Function Keyword(text As String) As String + Private Shared Function Keyword(text As String) As String Return "vb." + text End Function - Private Function Keyword(kind As SyntaxKind) As String + Private Shared Function Keyword(kind As SyntaxKind) As String Return Keyword(kind.GetText()) End Function @@ -245,11 +245,7 @@ Namespace Microsoft.VisualStudio.LanguageServices.VisualBasic.Help End Sub Public Overrides Sub VisitFieldDeclaration(node As FieldDeclarationSyntax) - Dim modifier = node.Modifiers.FirstOrDefault(Function(m) m.Span.IntersectsWith(_span)) - - If modifier <> Nothing Then - result = Keyword(modifier.Text) - End If + SelectModifier(node.Modifiers) End Sub Public Overrides Sub VisitForEachStatement(node As ForEachStatementSyntax) @@ -765,15 +761,48 @@ Namespace Microsoft.VisualStudio.LanguageServices.VisualBasic.Help End Sub Private Function SelectModifier(list As SyntaxTokenList) As Boolean - Dim modifier = list.FirstOrDefault(Function(t) t.Span.IntersectsWith(_span)) - If modifier <> Nothing Then - result = Keyword(modifier.Text) + For i As Integer = 0 To list.Count - 1 + Dim modifier = list(i) + If modifier.Span.IntersectsWith(_span) Then + + If SelectCombinationModifier(modifier, list) Then + Return True + End If + + ' Not a combination token, just normal keyword help + result = Keyword(modifier.Text) + Return True + End If + Next + + Return False + End Function + + Private Function SelectCombinationModifier(token As SyntaxToken, list As SyntaxTokenList) As Boolean + If SelectCombinationModifier(token, list, SyntaxKind.PrivateKeyword, SyntaxKind.ProtectedKeyword, HelpKeywords.PrivateProtected) Then + Return True + End If + + If SelectCombinationModifier(token, list, SyntaxKind.ProtectedKeyword, SyntaxKind.FriendKeyword, HelpKeywords.ProtectedFriend) Then Return True End If Return False End Function + Private Function SelectCombinationModifier(token As SyntaxToken, list As SyntaxTokenList, kind1 As SyntaxKind, kind2 As SyntaxKind, helpKeyword As String) As Boolean + If token.IsKind(kind1) AndAlso list.Any(Function(t) t.IsKind(kind2)) Then + result = helpKeyword + Return True + End If + + If token.IsKind(kind2) AndAlso list.Any(Function(t) t.IsKind(kind1)) Then + result = helpKeyword + Return True + End If + + Return False + End Function Public Overrides Sub VisitVariableDeclarator(node As VariableDeclaratorSyntax) Dim bestName = node.Names.FirstOrDefault(Function(n) n.Span.IntersectsWith(_span)) If bestName Is Nothing Then