Display function overload description on hover (#4669)

* Display function overload description on hover

* Fix hover tests

* lint

src/Bicep.LangServer.IntegrationTests/HoverTests.cs

@@ -171,7 +171,7 @@ node is not ITopLevelNamedDeclarationSyntax &&
         }
 
 
-        [DataTestMethod]
+        [TestMethod]
         public async Task PropertyHovers_are_displayed_on_properties()
         {
             var (file, cursors) = ParserHelper.GetFileWithCursors(@"
@@ -202,7 +202,7 @@ public async Task PropertyHovers_are_displayed_on_properties()
         }
 
 
-        [DataTestMethod]
+        [TestMethod]
         public async Task PropertyHovers_are_displayed_on_properties_with_loops()
         {
             var (file, cursors) = ParserHelper.GetFileWithCursors(@"
@@ -233,7 +233,7 @@ public async Task PropertyHovers_are_displayed_on_properties_with_loops()
         }
 
 
-        [DataTestMethod]
+        [TestMethod]
         public async Task PropertyHovers_are_displayed_on_properties_with_conditions()
         {
             var (file, cursors) = ParserHelper.GetFileWithCursors(@"
@@ -263,7 +263,7 @@ public async Task PropertyHovers_are_displayed_on_properties_with_conditions()
                 h => h!.Contents.MarkupContent!.Value.Should().Be("```bicep\nreadonly: string\n```\nThis is a property which only supports reading.\n"));
         }
 
-        [DataTestMethod]
+        [TestMethod]
         public async Task Hovers_are_displayed_on_discription_decorator_objects()
         {
             var (file, cursors) = ParserHelper.GetFileWithCursors(@"
@@ -299,7 +299,7 @@ var test|Param string
                 h => h!.Contents.MarkupContent!.Value.Should().EndWith("```\nthis is my output\n"));
         }
 
-        [DataTestMethod]
+        [TestMethod]
         public async Task Hovers_are_displayed_on_discription_decorator_objects_across_bicep_modules()
         {
             var modFile = @"
@@ -352,7 +352,40 @@ param param1 string
                 h => h!.Contents.MarkupContent!.Value.Should().EndWith("```\nthis  \nis  \nout2\n"));
         }
 
-         [DataTestMethod]
+        [TestMethod]
+        public async Task Function_hovers_include_descriptions_if_function_overload_has_been_resolved()
+        {
+            var hovers = await RequestHoversAtCursorLocations(@"
+var rgFunc = resource|Group()
+var nsRgFunc = az.resourceGroup|()
+
+var concatFunc = conc|at('abc', 'def')
+var nsConcatFunc = sys.c|oncat('abc', 'def')
+");
+
+            hovers.Should().SatisfyRespectively(
+                h => h!.Contents.MarkupContent!.Value.Should().Be("```bicep\nfunction resourceGroup(): resourceGroup\n```\nReturns the current resource group scope. **This function can only be used in resourceGroup deployments.**\n"),
+                h => h!.Contents.MarkupContent!.Value.Should().Be("```bicep\nfunction resourceGroup(): resourceGroup\n```\nReturns the current resource group scope. **This function can only be used in resourceGroup deployments.**\n"),
+                h => h!.Contents.MarkupContent!.Value.Should().Be("```bicep\nfunction concat('abc', 'def'): string\n```\nCombines multiple string, integer, or boolean values and returns them as a concatenated string.\n"),
+                h => h!.Contents.MarkupContent!.Value.Should().Be("```bicep\nfunction concat('abc', 'def'): string\n```\nCombines multiple string, integer, or boolean values and returns them as a concatenated string.\n"));
+        }
+
+        [TestMethod]
+        public async Task Function_hovers_display_without_descriptions_if_function_overload_has_not_been_resolved()
+        {
+            // using the any type, we don't know which particular overload has been selected, so we cannot show an accurate description.
+            // this should be taken care of with https://github.com/Azure/bicep/issues/4588
+            var hovers = await RequestHoversAtCursorLocations(@"
+var concatFunc = conc|at(any('hello'))
+var nsConcatFunc = sys.conc|at(any('hello'))
+");
+
+            hovers.Should().SatisfyRespectively(
+                h => h!.Contents.MarkupContent!.Value.Should().Be("```bicep\nfunction concat(any): any\n```\n"),
+                h => h!.Contents.MarkupContent!.Value.Should().Be("```bicep\nfunction concat(any): any\n```\n"));
+        }
+
+        [TestMethod]
         public async Task Hovers_are_displayed_on_discription_decorator_objects_across_arm_modules()
         {
             var modFile = @"
@@ -416,7 +449,7 @@ param param1 string
                 h => h!.Contents.MarkupContent!.Value.Should().EndWith("```\nthis  \nis  \nout2\n"));
         }
 
-        [DataTestMethod]
+        [TestMethod]
         public async Task PropertyHovers_are_displayed_on_partial_discriminator_objects()
         {
             var (file, cursors) = ParserHelper.GetFileWithCursors(@"
@@ -514,5 +547,16 @@ private static IEnumerable<object[]> GetData()
 
             return hovers;
         }
+
+        public async Task<IEnumerable<Hover?>> RequestHoversAtCursorLocations(string fileWithCursors)
+        {
+            var (file, cursors) = ParserHelper.GetFileWithCursors(fileWithCursors);
+
+            var bicepFile = SourceFileFactory.CreateBicepFile(new Uri("file:///path/to/main.bicep"), file);
+            var client = await IntegrationTestHelper.StartServerWithTextAsync(this.TestContext, file, bicepFile.FileUri, creationOptions: new LanguageServer.Server.CreationOptions(NamespaceProvider: BuiltInTestTypes.Create()));
+
+
+            return await RequestHovers(client, bicepFile, cursors);
+        }
     }
 }

src/Bicep.LangServer/Handlers/BicepHoverHandler.cs

@@ -19,6 +19,7 @@
 using OmniSharp.Extensions.LanguageServer.Protocol.Document;
 using OmniSharp.Extensions.LanguageServer.Protocol.Models;
 using Bicep.Core.Semantics.Namespaces;
+using Bicep.LanguageServer.CompilationManager;
 
 namespace Bicep.LanguageServer.Handlers
 {
@@ -96,10 +97,10 @@ public BicepHoverHandler(ISymbolResolver symbolResolver)
                 case BuiltInNamespaceSymbol builtInNamespace:
                     return CodeBlock($"{builtInNamespace.Name} namespace");
 
-                case FunctionSymbol function when result.Origin is FunctionCallSyntax functionCall:
+                case FunctionSymbol function when result.Origin is FunctionCallSyntaxBase functionCall:
                     // it's not possible for a non-function call syntax to resolve to a function symbol
                     // but this simplifies the checks
-                    return CodeBlock(GetFunctionMarkdown(function, functionCall.Arguments, result.Origin, result.Context.Compilation.GetEntrypointSemanticModel()));
+                    return GetFunctionMarkdown(function, functionCall, result.Context.Compilation.GetEntrypointSemanticModel());
 
                 case PropertySymbol property:
                     if (GetModuleParameterOrOutputDescription(request, result, $"{property.Name}: {property.Type}", out var codeBlock))
@@ -108,10 +109,6 @@ public BicepHoverHandler(ISymbolResolver symbolResolver)
                     }
                     return CodeBlockWithDescription($"{property.Name}: {property.Type}", property.Description);
 
-                case FunctionSymbol function when result.Origin is InstanceFunctionCallSyntax functionCall:
-                    return CodeBlock(
-                        GetFunctionMarkdown(function, functionCall.Arguments, result.Origin, result.Context.Compilation.GetEntrypointSemanticModel()));
-
                 case LocalVariableSymbol local:
                     return CodeBlock($"{local.Name}: {local.Type}");
 
@@ -130,15 +127,15 @@ private static string CodeBlock(string content) =>
         // Markdown needs two leading whitespaces before newline to insert a line break
         private static string CodeBlockWithDescription(string content, string? description) => CodeBlock(content) + (description is not null ? $"{description.Replace("\n", "  \n")}\n" : string.Empty);
 
-        private static string GetFunctionMarkdown(FunctionSymbol function, ImmutableArray<FunctionArgumentSyntax> arguments, SyntaxBase functionCall, SemanticModel model)
+        private static string GetFunctionMarkdown(FunctionSymbol function, FunctionCallSyntaxBase functionCall, SemanticModel model)
         {
             var buffer = new StringBuilder();
             buffer.Append($"function ");
             buffer.Append(function.Name);
             buffer.Append('(');
 
             const string argumentSeparator = ", ";
-            foreach (FunctionArgumentSyntax argumentSyntax in arguments)
+            foreach (FunctionArgumentSyntax argumentSyntax in functionCall.Arguments)
             {
                 var argumentType = model.GetTypeInfo(argumentSyntax);
                 buffer.Append(argumentType);
@@ -147,15 +144,21 @@ private static string GetFunctionMarkdown(FunctionSymbol function, ImmutableArra
             }
 
             // remove trailing argument separator (if any)
-            if (arguments.Length > 0)
+            if (functionCall.Arguments.Length > 0)
             {
                 buffer.Remove(buffer.Length - argumentSeparator.Length, argumentSeparator.Length);
             }
 
             buffer.Append("): ");
             buffer.Append(model.GetTypeInfo(functionCall));
 
-            return buffer.ToString();
+            if (model.TypeManager.GetMatchedFunctionOverload(functionCall) is {} matchedOverload)
+            {
+                return CodeBlockWithDescription(buffer.ToString(), matchedOverload.Description);
+            }
+
+            // TODO fall back to displaying a more generic description if unable to resolve a particular overload, once https://github.com/Azure/bicep/issues/4588 has been implemented.
+            return CodeBlock(buffer.ToString());
         }
 
         private static bool GetModuleParameterOrOutputDescription(HoverParams request, SymbolResolutionResult result, string content, [NotNullWhen(true)] out string? codeBlock)

src/vscode-bicep/src/test/e2e/hover.test.ts

@@ -39,7 +39,7 @@ describe("hover", (): void => {
       startCharacter: 6,
       endLine: 1,
       endCharacter: 12,
-      contents: ["param vmName: string"],
+      contents: [codeblock("param vmName: string")],
     });
   });
 
@@ -54,7 +54,7 @@ describe("hover", (): void => {
       startCharacter: 4,
       endLine: 50,
       endCharacter: 22,
-      contents: ["var linuxConfiguration: object"],
+      contents: [codeblock("var linuxConfiguration: object")],
     });
   });
 
@@ -69,7 +69,11 @@ describe("hover", (): void => {
       startCharacter: 9,
       endLine: 108,
       endCharacter: 13,
-      contents: ["resource vnet\nMicrosoft.Network/virtualNetworks@2020-06-01"],
+      contents: [
+        codeblock(
+          "resource vnet\nMicrosoft.Network/virtualNetworks@2020-06-01"
+        ),
+      ],
     });
   });
 
@@ -84,7 +88,7 @@ describe("hover", (): void => {
       startCharacter: 7,
       endLine: 183,
       endCharacter: 28,
-      contents: ["output administratorUsername: string"],
+      contents: [codeblock("output administratorUsername: string")],
     });
   });
 
@@ -99,7 +103,12 @@ describe("hover", (): void => {
       startCharacter: 55,
       endLine: 18,
       endCharacter: 67,
-      contents: ["function uniqueString(string): string"],
+      contents: [
+        codeblockWithDescription(
+          "function uniqueString(string): string",
+          "Creates a deterministic hash string based on the values provided as parameters."
+        ),
+      ],
     });
   });
 
@@ -139,9 +148,7 @@ describe("hover", (): void => {
       );
       expect(hover.contents).toHaveLength(contents.length);
       hover.contents.forEach((content, contentIndex) => {
-        expect(normalizeMarkedString(content)).toBe(
-          marked(contents[contentIndex])
-        );
+        expect(normalizeMarkedString(content)).toBe(contents[contentIndex]);
       });
     });
   }
@@ -150,7 +157,14 @@ describe("hover", (): void => {
     return typeof markedString === "string" ? markedString : markedString.value;
   }
 
-  function marked(rawString: string): string {
+  function codeblock(rawString: string): string {
     return "```bicep\n" + rawString + "\n```\n";
   }
+
+  function codeblockWithDescription(
+    rawString: string,
+    description: string
+  ): string {
+    return `${codeblock(rawString)}${description}\n`;
+  }
 });