From 60483e71d3f66197cdeee5074e42aa507a8e4539 Mon Sep 17 00:00:00 2001 From: David Driscoll Date: Wed, 27 Jan 2021 23:11:16 -0500 Subject: [PATCH 1/2] Added Container.From. do not serialize out the PrivateHandlerId if it's empty. StringOrMarkupContent implict conversion should be nullable. --- src/Dap.Protocol/Models/Container.cs | 33 ++++++++++++++++ src/Protocol/Models/Container.cs | 33 ++++++++++++++++ src/Protocol/Models/StringOrMarkupContent.cs | 6 +-- .../Pipelines/ResolveCommandPipeline.cs | 39 ++++++++++++++++--- test/Lsp.Tests/CompletionItemKindTests.cs | 1 + .../Matchers/ResolveCommandMatcherTests.cs | 16 ++++---- 6 files changed, 111 insertions(+), 17 deletions(-) diff --git a/src/Dap.Protocol/Models/Container.cs b/src/Dap.Protocol/Models/Container.cs index 21cdd5a01..1f89b4673 100644 --- a/src/Dap.Protocol/Models/Container.cs +++ b/src/Dap.Protocol/Models/Container.cs @@ -6,6 +6,39 @@ namespace OmniSharp.Extensions.DebugAdapter.Protocol.Models { + public static class Container + { + [return: NotNullIfNotNull("items")] + public static Container? From(IEnumerable? items) => items switch { + not null => new Container(items), + _ => null + }; + + [return: NotNullIfNotNull("items")] + public static Container? From(params T[] items) => items switch { + not null => new Container(items), + _ => null + }; + + [return: NotNullIfNotNull("items")] + public static Container? From(List? items) => items switch { + not null => new Container(items), + _ => null + }; + + [return: NotNullIfNotNull("items")] + public static Container? From(in ImmutableArray? items) => items switch { + not null => new Container(items), + _ => null + }; + + [return: NotNullIfNotNull("items")] + public static Container? From(ImmutableList? items) => items switch { + not null => new Container(items), + _ => null + }; + } + public class Container : ContainerBase { public Container() : this(Enumerable.Empty()) diff --git a/src/Protocol/Models/Container.cs b/src/Protocol/Models/Container.cs index 097abf5f3..0c22f382e 100644 --- a/src/Protocol/Models/Container.cs +++ b/src/Protocol/Models/Container.cs @@ -6,6 +6,39 @@ namespace OmniSharp.Extensions.LanguageServer.Protocol.Models { + public static class Container + { + [return: NotNullIfNotNull("items")] + public static Container? From(IEnumerable? items) => items switch { + not null => new Container(items), + _ => null + }; + + [return: NotNullIfNotNull("items")] + public static Container? From(params T[] items) => items switch { + not null => new Container(items), + _ => null + }; + + [return: NotNullIfNotNull("items")] + public static Container? From(List? items) => items switch { + not null => new Container(items), + _ => null + }; + + [return: NotNullIfNotNull("items")] + public static Container? From(in ImmutableArray? items) => items switch { + not null => new Container(items), + _ => null + }; + + [return: NotNullIfNotNull("items")] + public static Container? From(ImmutableList? items) => items switch { + not null => new Container(items), + _ => null + }; + } + public class Container : ContainerBase { public Container() : this(Enumerable.Empty()) diff --git a/src/Protocol/Models/StringOrMarkupContent.cs b/src/Protocol/Models/StringOrMarkupContent.cs index 94625d824..9b9ce5de7 100644 --- a/src/Protocol/Models/StringOrMarkupContent.cs +++ b/src/Protocol/Models/StringOrMarkupContent.cs @@ -4,7 +4,7 @@ namespace OmniSharp.Extensions.LanguageServer.Protocol.Models { - [JsonConverter(typeof(StringOrMarkupContentConverter))] + [JsonConverter(typeof( StringOrMarkupContentConverter))] [DebuggerDisplay("{" + nameof(DebuggerDisplay) + ",nq}")] public record StringOrMarkupContent { @@ -17,9 +17,9 @@ public record StringOrMarkupContent public MarkupContent? MarkupContent { get; } public bool HasMarkupContent => String == null; - public static implicit operator StringOrMarkupContent(string value) => new StringOrMarkupContent(value); + public static implicit operator StringOrMarkupContent?(string? value) => value is null ? null : new StringOrMarkupContent(value); - public static implicit operator StringOrMarkupContent(MarkupContent markupContent) => new StringOrMarkupContent(markupContent); + public static implicit operator StringOrMarkupContent?(MarkupContent? markupContent) => markupContent is null ? null : new StringOrMarkupContent(markupContent); private string DebuggerDisplay => $"{( HasString ? String : HasMarkupContent ? MarkupContent!.ToString() : string.Empty )}"; diff --git a/src/Server/Pipelines/ResolveCommandPipeline.cs b/src/Server/Pipelines/ResolveCommandPipeline.cs index e1869178c..af075aa89 100644 --- a/src/Server/Pipelines/ResolveCommandPipeline.cs +++ b/src/Server/Pipelines/ResolveCommandPipeline.cs @@ -21,7 +21,7 @@ public class ResolveCommandPipeline : IPipelineBehavior> logger) { _logger = logger; - _descriptor = (context.Descriptor as ILspHandlerDescriptor)!; + _descriptor = ( context.Descriptor as ILspHandlerDescriptor )!; } public async Task Handle(TRequest request, CancellationToken cancellationToken, RequestHandlerDelegate next) @@ -40,15 +40,42 @@ public async Task Handle(TRequest request, CancellationToken cancella ); foreach (var item in canBeResolvedItems) { - item.SetRawData(item.Data ?? new JObject()); - if (item.Data is JObject o) - { - o[Constants.PrivateHandlerId] = id; - } + UpdatePrivateHandlerId(item, id); } } + // Only pin the handler type, if we know the source handler (codelens) is also the resolver. + if (response is ICanBeResolved canBeResolvedItem) + { + var id = _descriptor.Handler is ICanBeIdentifiedHandler resolved ? resolved.Id : Guid.Empty; + _logger.LogTrace( + "Updating Resolve items with wrapped data for {Method}:{Handler}", + _descriptor.Method, + _descriptor.ImplementationType.FullName + ); + UpdatePrivateHandlerId(canBeResolvedItem, id); + } + return response; + + void UpdatePrivateHandlerId(ICanBeResolved item, Guid id) + { + item.SetRawData(item.Data ?? new JObject()); + if (item.Data is JObject o) + { + if (id == Guid.Empty) + { + if (o.ContainsKey(Constants.PrivateHandlerId)) + { + o.Remove(Constants.PrivateHandlerId); + } + + return; + } + + o[Constants.PrivateHandlerId] = id; + } + } } } } diff --git a/test/Lsp.Tests/CompletionItemKindTests.cs b/test/Lsp.Tests/CompletionItemKindTests.cs index 08bd91c48..27581d245 100644 --- a/test/Lsp.Tests/CompletionItemKindTests.cs +++ b/test/Lsp.Tests/CompletionItemKindTests.cs @@ -1,4 +1,5 @@ using FluentAssertions; +using Lsp.Tests.Integration.Fixtures; using OmniSharp.Extensions.LanguageServer.Protocol; using OmniSharp.Extensions.LanguageServer.Protocol.Client.Capabilities; using OmniSharp.Extensions.LanguageServer.Protocol.Models; diff --git a/test/Lsp.Tests/Matchers/ResolveCommandMatcherTests.cs b/test/Lsp.Tests/Matchers/ResolveCommandMatcherTests.cs index 9ffac1d23..b7313da9c 100644 --- a/test/Lsp.Tests/Matchers/ResolveCommandMatcherTests.cs +++ b/test/Lsp.Tests/Matchers/ResolveCommandMatcherTests.cs @@ -23,7 +23,7 @@ namespace Lsp.Tests.Matchers public class ResolveCommandMatcherTests : AutoTestBase { private readonly Guid _trueId = Guid.NewGuid(); - private readonly Guid _falseId = Guid.NewGuid(); + private readonly Guid _falseId = Guid.Empty; public ResolveCommandMatcherTests(ITestOutputHelper testOutputHelper) : base(testOutputHelper) { @@ -247,7 +247,7 @@ public async Task Should_Update_CompletionItems_With_HandlerType() 0, TextDocumentNames.Completion, "Key", - (resolveHandler as IJsonRpcHandler)!, + ( resolveHandler as IJsonRpcHandler )!, resolveHandler.GetType(), typeof(CompletionParams), null, @@ -298,7 +298,7 @@ public async Task Should_Update_CodeLensContainer_With_HandlerType() 0, TextDocumentNames.CodeLens, "Key", - (resolveHandler as IJsonRpcHandler)!, + ( resolveHandler as IJsonRpcHandler )!, resolveHandler.GetType(), typeof(CodeLensParams), null, @@ -344,12 +344,12 @@ public async Task Should_Update_CodeLens_Removing_HandlerType() typeof(ICanBeIdentifiedHandler) }, new object[0] ); - ( resolveHandler as ICanBeIdentifiedHandler )?.Id.Returns(_trueId); + ( resolveHandler as ICanBeIdentifiedHandler )?.Id.Returns(_falseId); var descriptor = new LspHandlerDescriptor( 0, TextDocumentNames.CodeLensResolve, "Key", - (resolveHandler as IJsonRpcHandler)!, + ( resolveHandler as IJsonRpcHandler )!, resolveHandler.GetType(), typeof(CodeLens), null, @@ -368,15 +368,15 @@ public async Task Should_Update_CodeLens_Removing_HandlerType() var item = new CodeLens { Data = JObject.FromObject(new { hello = "world" }) }; - item.Data[Constants.PrivateHandlerId] = Guid.Empty; // When var response = await handlerMatcher.Handle(item, CancellationToken.None, () => Task.FromResult(item)); // Then response.Should().BeEquivalentTo(item, x => x.UsingStructuralRecordEquality()); - item.Data?[Constants.PrivateHandlerId].Value().Should().BeEmpty(); - item.Data?["hello"].Value().Should().Be("world"); + var data = item.Data.Should().BeOfType().Subject; + data.Should().NotContainKey(Constants.PrivateHandlerId); + data["hello"].Value().Should().Be("world"); } } } From 97d474b1dcbca3376e874625a972b7e78ec0282f Mon Sep 17 00:00:00 2001 From: David Driscoll Date: Wed, 27 Jan 2021 23:32:13 -0500 Subject: [PATCH 2/2] try and fix #486 --- src/Dap.Client/DebugAdapterClient.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Dap.Client/DebugAdapterClient.cs b/src/Dap.Client/DebugAdapterClient.cs index 8c00a7631..5d8854b95 100644 --- a/src/Dap.Client/DebugAdapterClient.cs +++ b/src/Dap.Client/DebugAdapterClient.cs @@ -135,6 +135,8 @@ await DebugAdapterEventingHelper.Run( _connection.Open(); var serverParams = await this.RequestDebugAdapterInitialize(ClientSettings, token).ConfigureAwait(false); + _receiver.Initialized(); + ServerSettings = serverParams; await DebugAdapterEventingHelper.Run( @@ -146,8 +148,6 @@ await DebugAdapterEventingHelper.Run( token ).ConfigureAwait(false); - _receiver.Initialized(); - await _initializedComplete.ToTask(token); await DebugAdapterEventingHelper.Run(