Skip to content

Commit

Permalink
Use function argument declared types (or function declared type signa…
Browse files Browse the repository at this point in the history 
…ture when available) instead of resolved types (Azure#6592)

* Use function argument declared types (or function declared type signature when available) instead of resolved types

* Update E2E hover test

* Update hover tooltip on functions with no matched overload to show all available overloads.
  • Loading branch information
@jeskew
jeskew authored Apr 21, 2022
1 parent 6bcd201 commit bc62f3a
Show file tree
Hide file tree
Showing 3 changed files with 128 additions and 113 deletions.
137 changes: 79 additions & 58 deletions src/Bicep.LangServer.IntegrationTests/HoverTests.cs
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,6 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Text.RegularExpressions;
using System.Threading.Tasks;
using Bicep.Core.Extensions;
using Bicep.Core.FileSystem;
using Bicep.Core.Navigation;
Expand All @@ -28,6 +23,12 @@
using OmniSharp.Extensions.LanguageServer.Protocol.Client;
using OmniSharp.Extensions.LanguageServer.Protocol.Document;
using OmniSharp.Extensions.LanguageServer.Protocol.Models;
using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using System.Text.RegularExpressions;
using System.Threading.Tasks;
using SymbolKind = Bicep.Core.Semantics.SymbolKind;

namespace Bicep.LangServer.IntegrationTests
Expand Down Expand Up @@ -372,8 +373,8 @@ public async Task Function_hovers_include_descriptions_if_function_overload_has_
hovers.Should().SatisfyRespectively(
h => h!.Contents.MarkupContent!.Value.Should().Be("```bicep\nfunction resourceGroup(): resourceGroup\n```\nReturns the current resource group scope.\n"),
h => h!.Contents.MarkupContent!.Value.Should().Be("```bicep\nfunction resourceGroup(): resourceGroup\n```\nReturns the current resource group scope.\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"));
h => h!.Contents.MarkupContent!.Value.Should().Be("```bicep\nfunction concat(... : bool | int | string): 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(... : bool | int | string): string\n```\nCombines multiple string, integer, or boolean values and returns them as a concatenated string.\n"));
}

[TestMethod]
Expand Down Expand Up @@ -418,8 +419,12 @@ public async Task Function_hovers_display_without_descriptions_if_function_overl
");

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"));
h => h!.Contents.MarkedStrings.Should().ContainInOrder(
"```bicep\nfunction concat(... : array): array\n```\nCombines multiple arrays and returns the concatenated array.\n",
"```bicep\nfunction concat(... : bool | int | string): string\n```\nCombines multiple string, integer, or boolean values and returns them as a concatenated string.\n"),
h => h!.Contents.MarkedStrings.Should().ContainInOrder(
"```bicep\nfunction concat(... : array): array\n```\nCombines multiple arrays and returns the concatenated array.\n",
"```bicep\nfunction concat(... : bool | int | string): string\n```\nCombines multiple string, integer, or boolean values and returns them as a concatenated string.\n"));
}

[TestMethod]
Expand Down Expand Up @@ -511,57 +516,73 @@ private static void ValidateHover(Hover? hover, Symbol symbol)
hover!.Range!.Should().NotBeNull();
hover.Contents.Should().NotBeNull();

hover.Contents.HasMarkedStrings.Should().BeFalse();
hover.Contents.HasMarkupContent.Should().BeTrue();
hover.Contents.MarkedStrings.Should().BeNull();
hover.Contents.MarkupContent.Should().NotBeNull();

hover.Contents.MarkupContent!.Kind.Should().Be(MarkupKind.Markdown);
hover.Contents.MarkupContent.Value.Should().StartWith("```bicep\n");
Regex.Matches(hover.Contents.MarkupContent.Value, "```").Count.Should().Be(2);
List<string> tooltips = new();
if (hover.Contents.HasMarkedStrings)
{
tooltips.AddRange(hover.Contents.MarkedStrings!.Select(ms => ms.Value));
}
else
{
hover.Contents.MarkupContent!.Kind.Should().Be(MarkupKind.Markdown);
tooltips.Add(hover.Contents.MarkupContent!.Value);
}

switch (symbol)
foreach (var tooltip in tooltips)
{
case ParameterSymbol parameter:
hover.Contents.MarkupContent.Value.Should().Contain($"param {parameter.Name}: {parameter.Type}");
break;

case VariableSymbol variable:
// the hovers with errors don't appear in VS code and only occur in tests
hover.Contents.MarkupContent.Value.Should().ContainAny(new[] { $"var {variable.Name}: {variable.Type}", $"var {variable.Name}: error" });
break;

case ResourceSymbol resource:
hover.Contents.MarkupContent.Value.Should().Contain($"resource {resource.Name}");
hover.Contents.MarkupContent.Value.Should().Contain(resource.Type.Name);
break;

case ModuleSymbol module:
hover.Contents.MarkupContent.Value.Should().Contain($"module {module.Name}");
break;

case OutputSymbol output:
hover.Contents.MarkupContent.Value.Should().Contain($"output {output.Name}: {output.Type}");
break;

case FunctionSymbol function:
hover.Contents.MarkupContent.Value.Should().Contain($"function {function.Name}(");
break;

case LocalVariableSymbol local:
hover.Contents.MarkupContent.Value.Should().Contain($"{local.Name}: {local.Type}");
break;

case ImportedNamespaceSymbol import:
hover.Contents.MarkupContent.Value.Should().Contain($"{import.Name} namespace");
break;

case BuiltInNamespaceSymbol @namespace:
hover.Contents.MarkupContent.Value.Should().Contain($"{@namespace.Name} namespace");
break;

default:
throw new AssertFailedException($"Unexpected symbol type '{symbol.GetType().Name}'");
tooltip.Should().StartWith("```bicep\n");
Regex.Matches(tooltip, "```").Count.Should().Be(2);

switch (symbol)
{
case ParameterSymbol parameter:
tooltip.Should().Contain($"param {parameter.Name}: {parameter.Type}");
break;

case VariableSymbol variable:
// the hovers with errors don't appear in VS code and only occur in tests
tooltip.Should().ContainAny(new[] { $"var {variable.Name}: {variable.Type}", $"var {variable.Name}: error" });
break;

case ResourceSymbol resource:
tooltip.Should().Contain($"resource {resource.Name}");
tooltip.Should().Contain(resource.Type.Name);
break;

case ModuleSymbol module:
tooltip.Should().Contain($"module {module.Name}");
break;

case OutputSymbol output:
tooltip.Should().Contain($"output {output.Name}: {output.Type}");
break;

case FunctionSymbol function:
if (function.Overloads.All(fo => fo is FunctionWildcardOverload))
{
tooltip.Should().Contain($"function ");
tooltip.Should().Contain($"*(");
}
else
{
tooltip.Should().Contain($"function {function.Name}(");
}
break;

case LocalVariableSymbol local:
tooltip.Should().Contain($"{local.Name}: {local.Type}");
break;

case ImportedNamespaceSymbol import:
tooltip.Should().Contain($"{import.Name} namespace");
break;

case BuiltInNamespaceSymbol @namespace:
tooltip.Should().Contain($"{@namespace.Name} namespace");
break;

default:
throw new AssertFailedException($"Unexpected symbol type '{symbol.GetType().Name}'");
}
}
}

Expand Down
102 changes: 48 additions & 54 deletions src/Bicep.LangServer/Handlers/BicepHoverHandler.cs
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.
using System.Linq;
using System.Text;
using System.Threading;
using System.Threading.Tasks;
using Bicep.Core.Semantics;
using Bicep.Core.Semantics.Namespaces;
using Bicep.Core.Syntax;
Expand All @@ -13,6 +9,10 @@
using OmniSharp.Extensions.LanguageServer.Protocol.Client.Capabilities;
using OmniSharp.Extensions.LanguageServer.Protocol.Document;
using OmniSharp.Extensions.LanguageServer.Protocol.Models;
using System.Collections.Generic;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;

namespace Bicep.LanguageServer.Handlers
{
Expand Down Expand Up @@ -44,77 +44,73 @@ public BicepHoverHandler(ISymbolResolver symbolResolver)

return Task.FromResult<Hover?>(new Hover
{
Contents = new MarkedStringsOrMarkupContent(new MarkupContent
{
Kind = MarkupKind.Markdown,
Value = markdown
}),
Contents = markdown,
Range = PositionHelper.GetNameRange(result.Context.LineStarts, result.Origin)
});
}

private static string? TryGetDescriptionMarkdown(SymbolResolutionResult result, DeclaredSymbol symbol)
{
if (symbol.DeclaringSyntax is StatementSyntax statementSyntax &&
SemanticModelHelper.TryGetDescription(result.Context.Compilation.GetEntrypointSemanticModel(), statementSyntax) is {} description)
SemanticModelHelper.TryGetDescription(result.Context.Compilation.GetEntrypointSemanticModel(), statementSyntax) is { } description)
{
return description;
}

return null;
}

private static string? GetMarkdown(HoverParams request, SymbolResolutionResult result)
private static MarkedStringsOrMarkupContent? GetMarkdown(HoverParams request, SymbolResolutionResult result)
{
// all of the generated markdown includes the language id to avoid VS code rendering
// with multiple borders
switch (result.Symbol)
{
case ImportedNamespaceSymbol import:
return CodeBlockWithDescription(
$"import {import.Name}", TryGetDescriptionMarkdown(result, import));
return WithMarkdown(CodeBlockWithDescription(
$"import {import.Name}", TryGetDescriptionMarkdown(result, import)));

case ParameterSymbol parameter:
return CodeBlockWithDescription(
$"param {parameter.Name}: {parameter.Type}", TryGetDescriptionMarkdown(result, parameter));
return WithMarkdown(CodeBlockWithDescription(
$"param {parameter.Name}: {parameter.Type}", TryGetDescriptionMarkdown(result, parameter)));

case VariableSymbol variable:
return CodeBlockWithDescription($"var {variable.Name}: {variable.Type}", TryGetDescriptionMarkdown(result, variable));
return WithMarkdown(CodeBlockWithDescription($"var {variable.Name}: {variable.Type}", TryGetDescriptionMarkdown(result, variable)));

case ResourceSymbol resource:
var docsSuffix = TryGetTypeDocumentationLink(resource) is {} typeDocsLink ? $"[View Type Documentation]({typeDocsLink})" : "";
var docsSuffix = TryGetTypeDocumentationLink(resource) is { } typeDocsLink ? $"[View Type Documentation]({typeDocsLink})" : "";
var description = TryGetDescriptionMarkdown(result, resource);

return CodeBlockWithDescription(
return WithMarkdown(CodeBlockWithDescription(
$"resource {resource.Name} {(resource.Type is ResourceType ? $"'{resource.Type}'" : resource.Type)}",
description is {} ? $"{description}\n{docsSuffix}" : docsSuffix);
description is { } ? $"{description}\n{docsSuffix}" : docsSuffix));

case ModuleSymbol module:
var filePath = SyntaxHelper.TryGetModulePath(module.DeclaringModule, out _);
if (filePath != null)
{
return CodeBlockWithDescription($"module {module.Name} '{filePath}'", TryGetDescriptionMarkdown(result, module));
return WithMarkdown(CodeBlockWithDescription($"module {module.Name} '{filePath}'", TryGetDescriptionMarkdown(result, module)));
}

return CodeBlockWithDescription($"module {module.Name}", TryGetDescriptionMarkdown(result, module));
return WithMarkdown(CodeBlockWithDescription($"module {module.Name}", TryGetDescriptionMarkdown(result, module)));

case OutputSymbol output:
return CodeBlockWithDescription(
$"output {output.Name}: {output.Type}", TryGetDescriptionMarkdown(result, output));
return WithMarkdown(CodeBlockWithDescription(
$"output {output.Name}: {output.Type}", TryGetDescriptionMarkdown(result, output)));

case BuiltInNamespaceSymbol builtInNamespace:
return CodeBlock($"{builtInNamespace.Name} namespace");
return WithMarkdown(CodeBlock($"{builtInNamespace.Name} namespace"));

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 GetFunctionMarkdown(function, functionCall, result.Context.Compilation.GetEntrypointSemanticModel());

case PropertySymbol property:
return CodeBlockWithDescription($"{property.Name}: {property.Type}", property.Description);
return WithMarkdown(CodeBlockWithDescription($"{property.Name}: {property.Type}", property.Description));

case LocalVariableSymbol local:
return CodeBlock($"{local.Name}: {local.Type}");
return WithMarkdown(CodeBlock($"{local.Name}: {local.Type}"));

default:
return null;
Expand All @@ -131,43 +127,33 @@ 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, FunctionCallSyntaxBase functionCall, SemanticModel model)
private static MarkedStringsOrMarkupContent 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 functionCall.Arguments)
{
var argumentType = model.GetTypeInfo(argumentSyntax);
buffer.Append(argumentType);

buffer.Append(argumentSeparator);
}

// remove trailing argument separator (if any)
if (functionCall.Arguments.Length > 0)
if (model.TypeManager.GetMatchedFunctionOverload(functionCall) is { } matchedOverload)
{
buffer.Remove(buffer.Length - argumentSeparator.Length, argumentSeparator.Length);
return WithMarkdown(GetFunctionOverloadMarkdown(matchedOverload, function.Overloads.Length - 1));
}

buffer.Append("): ");
buffer.Append(model.GetTypeInfo(functionCall));
var potentialMatches =
function.Overloads
.Select(overload => (overload, matchType:
overload.Match(functionCall.Arguments.Select(model.GetTypeInfo).ToList(), out _, out _)))
.Where(t => t.matchType == FunctionMatchResult.Match || t.matchType == FunctionMatchResult.PotentialMatch)
.Select(t => t.overload)
.ToList();

if (model.TypeManager.GetMatchedFunctionOverload(functionCall) is { } matchedOverload)
{
return CodeBlockWithDescription(buffer.ToString(), matchedOverload.Description);
}
// If there are no potential matches, just show all overloads
IEnumerable<FunctionOverload> toShow = potentialMatches.Count > 0 ? potentialMatches : function.Overloads;

// 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());
return WithMarkdown(toShow.Select(GetFunctionOverloadMarkdown));
}

private static string GetFunctionOverloadMarkdown(FunctionOverload overload, int functionOverloadCount)
=> CodeBlockWithDescription($"function {overload.Name}{overload.TypeSignature}", overload.Description);

private static string? TryGetTypeDocumentationLink(ResourceSymbol resource)
{
if (resource.TryGetResourceType() is {} resourceType &&
if (resource.TryGetResourceType() is { } resourceType &&
resourceType.DeclaringNamespace.ProviderNameEquals(AzNamespaceType.BuiltInName) &&
resourceType.DeclaringNamespace.ResourceTypeProvider.HasDefinedType(resourceType.TypeReference))
{
Expand All @@ -180,10 +166,18 @@ private static string GetFunctionMarkdown(FunctionSymbol function, FunctionCallS
return null;
}

private static MarkedStringsOrMarkupContent WithMarkdown(string markdown) => new MarkedStringsOrMarkupContent(new MarkupContent
{
Kind = MarkupKind.Markdown,
Value = markdown,
});

private static MarkedStringsOrMarkupContent WithMarkdown(IEnumerable<string> markdown)
=> new MarkedStringsOrMarkupContent(markdown.Select(md => new MarkedString(md)));

protected override HoverRegistrationOptions CreateRegistrationOptions(HoverCapability capability, ClientCapabilities clientCapabilities) => new()
{
DocumentSelector = DocumentSelectorFactory.Create()
};
}
}

Loading

0 comments on commit bc62f3a

Please sign in to comment.