Skip to content

Commit

Permalink
Addressed some cr feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
bhsubra committed May 26, 2021
1 parent 5078ab9 commit ffbef4c
Show file tree
Hide file tree
Showing 8 changed files with 38 additions and 54 deletions.
25 changes: 14 additions & 11 deletions src/Bicep.LangServer.UnitTests/BicepCompletionProviderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@
using Bicep.Core;
using Bicep.Core.Extensions;
using Bicep.Core.FileSystem;
using Bicep.Core.Navigation;
using Bicep.Core.Parsing;
using Bicep.Core.Semantics;
using Bicep.Core.Semantics.Namespaces;
using Bicep.Core.Syntax;
Expand All @@ -20,22 +18,27 @@
using Bicep.LanguageServer.Telemetry;
using FluentAssertions;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using Moq;
using OmniSharp.Extensions.LanguageServer.Protocol.Models;
using OmniSharp.Extensions.LanguageServer.Protocol.Server;
using SymbolKind = Bicep.Core.Semantics.SymbolKind;

namespace Bicep.LangServer.UnitTests
{
[TestClass]
public class BicepCompletionProviderTests
{
private static readonly MockRepository Repository = new MockRepository(MockBehavior.Strict);
private static readonly ILanguageServerFacade Server = Repository.Create<ILanguageServerFacade>().Object;

[TestMethod]
public void DeclarationContextShouldReturnKeywordCompletions()
{
var grouping = SyntaxTreeGroupingFactory.CreateFromText(string.Empty);
var compilation = new Compilation(TestTypeHelper.CreateEmptyProvider(), grouping);
compilation.GetEntrypointSemanticModel().GetAllDiagnostics().Should().BeEmpty();

var provider = new BicepCompletionProvider(new FileResolver(), new SnippetsProvider(), new TelemetryProvider());
var provider = new BicepCompletionProvider(new FileResolver(), new SnippetsProvider(), new TelemetryProvider(Server));

var completions = provider.GetFilteredCompletions(compilation, BicepCompletionContext.Create(compilation, 0));

Expand Down Expand Up @@ -115,7 +118,7 @@ param p string
var offset = grouping.EntryPoint.ProgramSyntax.Declarations.OfType<VariableDeclarationSyntax>().Single().Value.Span.Position;
var compilation = new Compilation(TestTypeHelper.CreateEmptyProvider(), grouping);

var provider = new BicepCompletionProvider(new FileResolver(), new SnippetsProvider(), new TelemetryProvider());
var provider = new BicepCompletionProvider(new FileResolver(), new SnippetsProvider(), new TelemetryProvider(Server));
var context = BicepCompletionContext.Create(compilation, offset);
var completions = provider.GetFilteredCompletions(compilation, context).ToList();

Expand Down Expand Up @@ -154,7 +157,7 @@ public void CompletionsForOneLinerParameterDefaultValueShouldIncludeFunctionsVal

var offset = ((ParameterDefaultValueSyntax) grouping.EntryPoint.ProgramSyntax.Declarations.OfType<ParameterDeclarationSyntax>().Single().Modifier!).DefaultValue.Span.Position;

var provider = new BicepCompletionProvider(new FileResolver(), new SnippetsProvider(), new TelemetryProvider());
var provider = new BicepCompletionProvider(new FileResolver(), new SnippetsProvider(), new TelemetryProvider(Server));
var completions = provider.GetFilteredCompletions(
compilation,
BicepCompletionContext.Create(compilation, offset)).ToList();
Expand Down Expand Up @@ -185,7 +188,7 @@ public void CompletionsForModifierDefaultValuesShouldIncludeFunctionsValidInDefa
var compilation = new Compilation(TestTypeHelper.CreateEmptyProvider(), grouping);
var context = BicepCompletionContext.Create(compilation, offset);

var provider = new BicepCompletionProvider(new FileResolver(), new SnippetsProvider(), new TelemetryProvider());
var provider = new BicepCompletionProvider(new FileResolver(), new SnippetsProvider(), new TelemetryProvider(Server));
var completions = provider.GetFilteredCompletions(compilation, context).ToList();

AssertExpectedFunctions(completions, expectParamDefaultFunctions: true);
Expand Down Expand Up @@ -216,7 +219,7 @@ param concat string
var offset = grouping.EntryPoint.ProgramSyntax.Declarations.OfType<OutputDeclarationSyntax>().Single().Value.Span.Position;

var compilation = new Compilation(TestTypeHelper.CreateEmptyProvider(), grouping);
var provider = new BicepCompletionProvider(new FileResolver(), new SnippetsProvider(), new TelemetryProvider());
var provider = new BicepCompletionProvider(new FileResolver(), new SnippetsProvider(), new TelemetryProvider(Server));
var context = BicepCompletionContext.Create(compilation, offset);
var completions = provider.GetFilteredCompletions(compilation, context).ToList();

Expand Down Expand Up @@ -258,7 +261,7 @@ public void OutputTypeContextShouldReturnDeclarationTypeCompletions()
{
var grouping = SyntaxTreeGroupingFactory.CreateFromText("output test ");
var compilation = new Compilation(TestTypeHelper.CreateEmptyProvider(), grouping);
var provider = new BicepCompletionProvider(new FileResolver(), new SnippetsProvider(), new TelemetryProvider());
var provider = new BicepCompletionProvider(new FileResolver(), new SnippetsProvider(), new TelemetryProvider(Server));

var offset = grouping.EntryPoint.ProgramSyntax.Declarations.OfType<OutputDeclarationSyntax>().Single().Type.Span.Position;

Expand All @@ -275,7 +278,7 @@ public void ParameterTypeContextShouldReturnDeclarationTypeCompletions()
{
var grouping = SyntaxTreeGroupingFactory.CreateFromText("param foo ");
var compilation = new Compilation(TestTypeHelper.CreateEmptyProvider(), grouping);
var provider = new BicepCompletionProvider(new FileResolver(), new SnippetsProvider(), new TelemetryProvider());
var provider = new BicepCompletionProvider(new FileResolver(), new SnippetsProvider(), new TelemetryProvider(Server));

var offset = grouping.EntryPoint.ProgramSyntax.Declarations.OfType<ParameterDeclarationSyntax>().Single().Type.Span.Position;

Expand Down Expand Up @@ -322,7 +325,7 @@ public void VerifyParameterTypeCompletionWithPrecedingComment()
{
var grouping = SyntaxTreeGroupingFactory.CreateFromText("/*test*/param foo ");
var compilation = new Compilation(TestTypeHelper.CreateEmptyProvider(), grouping);
var provider = new BicepCompletionProvider(new FileResolver(), new SnippetsProvider(), new TelemetryProvider());
var provider = new BicepCompletionProvider(new FileResolver(), new SnippetsProvider(), new TelemetryProvider(Server));

var offset = grouping.EntryPoint.ProgramSyntax.Declarations.OfType<ParameterDeclarationSyntax>().Single().Type.Span.Position;

Expand Down Expand Up @@ -380,7 +383,7 @@ public void CommentShouldNotGiveAnyCompletions(string codeFragment)
{
var grouping = SyntaxTreeGroupingFactory.CreateFromText(codeFragment);
var compilation = new Compilation(TestTypeHelper.CreateEmptyProvider(), grouping);
var provider = new BicepCompletionProvider(new FileResolver(), new SnippetsProvider(), new TelemetryProvider());
var provider = new BicepCompletionProvider(new FileResolver(), new SnippetsProvider(), new TelemetryProvider(Server));

var offset = codeFragment.IndexOf('|');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ private IEnumerable<CompletionItem> GetDeclarationCompletions(BicepCompletionCon

foreach (Snippet resourceSnippet in SnippetsProvider.GetTopLevelNamedDeclarationSnippets())
{
TelemetryEvent telemetryEvent = TelemetryEvent.Create(TelemetryConstants.EventNames.TopLevelDeclarationSnippetInsertion);
BicepTelemetryEvent telemetryEvent = BicepTelemetryEvent.Create(TelemetryConstants.EventNames.TopLevelDeclarationSnippetInsertion);
telemetryEvent.Set("name", resourceSnippet.Prefix);
Command command = Command.Create(TelemetryConstants.CommandName, telemetryEvent);

Expand Down
27 changes: 6 additions & 21 deletions src/Bicep.LangServer/Handlers/BicepTelemetryHandler.cs
Original file line number Diff line number Diff line change
@@ -1,43 +1,28 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Bicep.LanguageServer.Telemetry;
using MediatR;
using Newtonsoft.Json.Linq;
using OmniSharp.Extensions.LanguageServer.Protocol.Models;
using OmniSharp.Extensions.JsonRpc;
using OmniSharp.Extensions.LanguageServer.Protocol.Workspace;

namespace Bicep.LanguageServer.Handlers
{
public class BicepTelemetryHandler : ExecuteCommandHandler
public class BicepTelemetryHandler : ExecuteCommandHandlerBase<BicepTelemetryEvent>
{
private readonly ITelemetryProvider TelemetryProvider;

public BicepTelemetryHandler(ITelemetryProvider telemetryProvider)
: base(GetExecuteCommandRegistrationOptions())
public BicepTelemetryHandler(ITelemetryProvider telemetryProvider, ISerializer serializer)
: base(TelemetryConstants.CommandName, serializer)
{
TelemetryProvider = telemetryProvider;
}

private static ExecuteCommandRegistrationOptions GetExecuteCommandRegistrationOptions()
=> new ExecuteCommandRegistrationOptions()
{
Commands = new Container<string>(TelemetryConstants.CommandName)
};

public override Task<Unit> Handle(ExecuteCommandParams request, CancellationToken cancellationToken)
public override Task<Unit> Handle(BicepTelemetryEvent bicepTelemetryEvent, CancellationToken cancellationToken)
{
JArray? arguments = request.Arguments;
if (arguments is not null && arguments.Any() &&
arguments[0] is JToken jToken &&
jToken.ToObject<TelemetryEvent>() is TelemetryEvent telemetryEvent)
{
TelemetryProvider.PostEvent(telemetryEvent);
}

TelemetryProvider.PostEvent(bicepTelemetryEvent);
return Unit.Task;
}
}
Expand Down
4 changes: 1 addition & 3 deletions src/Bicep.LangServer/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
using System.Threading.Tasks;
using Bicep.Core.FileSystem;
using Bicep.Core.TypeSystem.Az;
using Bicep.LanguageServer.Telemetry;

namespace Bicep.LanguageServer
{
Expand All @@ -22,8 +21,7 @@ public static async Task Main(string[] args)
new Server.CreationOptions
{
ResourceTypeProvider = AzResourceTypeProvider.CreateWithAzTypes(),
FileResolver = new FileResolver(),
TelemetryProvider = new TelemetryProvider()
FileResolver = new FileResolver()
});

await server.RunAsync(cancellationToken);
Expand Down
9 changes: 1 addition & 8 deletions src/Bicep.LangServer/Server.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@ public class CreationOptions
public IResourceTypeProvider? ResourceTypeProvider { get; set; }

public IFileResolver? FileResolver { get; set; }

public ITelemetryProvider? TelemetryProvider { get; set; }
}

private readonly OmnisharpLanguageServer server;
Expand Down Expand Up @@ -74,11 +72,6 @@ private Server(CreationOptions creationOptions, Action<LanguageServerOptions> on

onOptionsFunc(options);
});

if (creationOptions.TelemetryProvider is TelemetryProvider telemetryProvider)
{
telemetryProvider.LanguageServer = server;
}
}

public async Task RunAsync(CancellationToken cancellationToken)
Expand All @@ -95,7 +88,7 @@ private static void RegisterServices(CreationOptions creationOptions, IServiceCo
services.AddSingleton<IResourceTypeProvider>(services => creationOptions.ResourceTypeProvider ?? AzResourceTypeProvider.CreateWithAzTypes());
services.AddSingleton<ISnippetsProvider>(services => creationOptions.SnippetsProvider ?? new SnippetsProvider());
services.AddSingleton<IFileResolver>(services => creationOptions.FileResolver ?? new FileResolver());
services.AddSingleton<ITelemetryProvider>(services => creationOptions.TelemetryProvider ?? new TelemetryProvider());
services.AddSingleton<ITelemetryProvider, TelemetryProvider>();
services.AddSingleton<IWorkspace, Workspace>();
services.AddSingleton<ICompilationManager, BicepCompilationManager>();
services.AddSingleton<ICompilationProvider, BicepCompilationProvider>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,23 +6,23 @@

namespace Bicep.LanguageServer.Telemetry
{
public class TelemetryEvent : TelemetryEventParams
public class BicepTelemetryEvent : TelemetryEventParams
{
public Dictionary<string, string> Properties = new Dictionary<string, string>();
public Dictionary<string, string> Properties { get; set; } = new Dictionary<string, string>();

public string? EventName { get; set; }

public static TelemetryEvent Create(string eventName)
public static BicepTelemetryEvent Create(string eventName)
{
TelemetryEvent telemetryEvent = new TelemetryEvent();
BicepTelemetryEvent telemetryEvent = new BicepTelemetryEvent();
telemetryEvent.EventName = eventName;
return telemetryEvent;
}
}

public static class TelemetryExtensions
{
public static void Set(this TelemetryEvent telemetryEvent, string name, string value)
public static void Set(this BicepTelemetryEvent telemetryEvent, string name, string value)
{
if (!telemetryEvent.Properties.ContainsKey(name))
{
Expand Down
2 changes: 1 addition & 1 deletion src/Bicep.LangServer/Telemetry/ITelemetryProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@ namespace Bicep.LanguageServer.Telemetry
{
public interface ITelemetryProvider
{
void PostEvent(TelemetryEvent telemetryEvent);
void PostEvent(BicepTelemetryEvent telemetryEvent);
}
}
13 changes: 9 additions & 4 deletions src/Bicep.LangServer/Telemetry/TelemetryProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,23 @@ namespace Bicep.LanguageServer.Telemetry
{
public class TelemetryProvider : ITelemetryProvider
{
public ILanguageServer? LanguageServer { get; set; }
private readonly ILanguageServerFacade server;

public void PostEvent(TelemetryEvent telemetryEvent)
public TelemetryProvider(ILanguageServerFacade server)
{
this.server = server;
}

public void PostEvent(BicepTelemetryEvent telemetryEvent)
{
if (telemetryEvent is null ||
!string.IsNullOrWhiteSpace(telemetryEvent.EventName) ||
string.IsNullOrWhiteSpace(telemetryEvent.EventName) ||
telemetryEvent.Properties?.Count == 0)
{
return;
}

LanguageServer?.Window.SendTelemetryEvent(telemetryEvent);
server.Window.SendTelemetryEvent(telemetryEvent);
}
}
}

0 comments on commit ffbef4c

Please sign in to comment.