From 5cf2a98bb61e78678272a7241abfbf80e1c678b8 Mon Sep 17 00:00:00 2001 From: Anthony Martin Date: Wed, 29 Sep 2021 22:28:55 -0400 Subject: [PATCH] Display function overload description on hover (#4669) * Display function overload description on hover * Fix hover tests * lint --- .../HoverTests.cs | 58 ++++++++++++++++--- .../Handlers/BicepHoverHandler.cs | 23 ++++---- src/vscode-bicep/src/test/e2e/hover.test.ts | 32 +++++++--- 3 files changed, 87 insertions(+), 26 deletions(-) diff --git a/src/Bicep.LangServer.IntegrationTests/HoverTests.cs b/src/Bicep.LangServer.IntegrationTests/HoverTests.cs index 23cd7c32b62..b8a2095cd06 100644 --- a/src/Bicep.LangServer.IntegrationTests/HoverTests.cs +++ b/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 GetData() return hovers; } + + public async Task> 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); + } } } diff --git a/src/Bicep.LangServer/Handlers/BicepHoverHandler.cs b/src/Bicep.LangServer/Handlers/BicepHoverHandler.cs index 6b0e437b5e9..f5e9f6a64d8 100644 --- a/src/Bicep.LangServer/Handlers/BicepHoverHandler.cs +++ b/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,7 +127,7 @@ 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 arguments, SyntaxBase functionCall, SemanticModel model) + private static string GetFunctionMarkdown(FunctionSymbol function, FunctionCallSyntaxBase functionCall, SemanticModel model) { var buffer = new StringBuilder(); buffer.Append($"function "); @@ -138,7 +135,7 @@ private static string GetFunctionMarkdown(FunctionSymbol function, ImmutableArra 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,7 +144,7 @@ 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); } @@ -155,7 +152,13 @@ private static string GetFunctionMarkdown(FunctionSymbol function, ImmutableArra 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) diff --git a/src/vscode-bicep/src/test/e2e/hover.test.ts b/src/vscode-bicep/src/test/e2e/hover.test.ts index 4215dc7287f..63c123d0291 100644 --- a/src/vscode-bicep/src/test/e2e/hover.test.ts +++ b/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`; + } });