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

src/VisualStudio/CSharp/Impl/LanguageService/CSharpHelpContextService.cs

@@ -106,7 +106,7 @@ public override async Task<string> 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<ISyntaxFactsService>();
             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;

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()
         {

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
 
+        <Fact, Trait(Traits.Feature, Traits.Features.F1Help)>
+        Public Async Function TestFriend() As Task
+            Dim text = <a>
+Fri[||]end Class G
+End Class</a>
+
+            Await TestAsync(text.Value, "vb.Friend")
+        End Function
+
+        <Fact, Trait(Traits.Feature, Traits.Features.F1Help)>
+        Public Async Function TestProtected() As Task
+            Dim text = <a>
+Public Class G
+    Protec[||]ted Sub M()
+    End Sub
+End Class</a>
+
+            Await TestAsync(text.Value, "vb.Protected")
+        End Function
+
+        <Fact, Trait(Traits.Feature, Traits.Features.F1Help)>
+        Public Async Function TestProtectedFriend1() As Task
+            Dim text = <a>
+Public Class G
+    Protec[||]ted Friend Sub M()
+    End Sub
+End Class</a>
+
+            Await TestAsync(text.Value, "vb.ProtectedFriend")
+        End Function
+
+        <Fact, Trait(Traits.Feature, Traits.Features.F1Help)>
+        Public Async Function TestProtectedFriend2() As Task
+            Dim text = <a>
+Public Class G
+    Friend Protec[||]ted Sub M()
+    End Sub
+End Class</a>
+
+            Await TestAsync(text.Value, "vb.ProtectedFriend")
+        End Function
+
+        <Fact, Trait(Traits.Feature, Traits.Features.F1Help)>
+        Public Async Function TestPrivateProtected1() As Task
+            Dim text = <a>
+Public Class G
+    Private Protec[||]ted Sub M()
+    End Sub
+End Class</a>
+
+            Await TestAsync(text.Value, "vb.PrivateProtected")
+        End Function
+
+        <Fact, Trait(Traits.Feature, Traits.Features.F1Help)>
+        Public Async Function TestPrivateProtected2() As Task
+            Dim text = <a>
+Public Class G
+    Priv[||]ate Protected Sub M()
+    End Sub
+End Class</a>
+
+            Await TestAsync(text.Value, "vb.PrivateProtected")
+        End Function
+
+        <Fact, Trait(Traits.Feature, Traits.Features.F1Help)>
+        Public Async Function TestPrivateProtected3() As Task
+            Dim text = <a>
+Public Class G
+    Protected Priv[||]ate Sub M()
+    End Sub
+End Class</a>
+
+            Await TestAsync(text.Value, "vb.PrivateProtected")
+        End Function
+
+        <Fact, Trait(Traits.Feature, Traits.Features.F1Help)>
+        Public Async Function TestPrivateProtected4() As Task
+            Dim text = <a>
+Public Class G
+    Protec[||]ted Private Sub M()
+    End Sub
+End Class</a>
+
+            Await TestAsync(text.Value, "vb.PrivateProtected")
+        End Function
+
+        <Fact, Trait(Traits.Feature, Traits.Features.F1Help)>
+        Public Async Function TestModifierSoup() As Task
+            Dim text = <a>
+Public Class G
+    Protec[||]ted Async Shared Private Sub M()
+    End Sub
+End Class</a>
+
+            Await TestAsync(text.Value, "vb.PrivateProtected")
+        End Function
+
+        <Fact, Trait(Traits.Feature, Traits.Features.F1Help)>
+        Public Async Function TestModifierSoupField() As Task
+            Dim text = <a>
+Public Class G
+    Private Shadows Shared Prot[||]ected foo as Boolean
+End Class</a>
+
+            Await TestAsync(text.Value, "vb.PrivateProtected")
+        End Function
+
         <Fact, Trait(Traits.Feature, Traits.Features.F1Help)>
         Public Async Function TestAddHandler1() As Task
             Dim text = <a>

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
 

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