From 1d67992ec3ee9e687142f89a183989b0b252a0da Mon Sep 17 00:00:00 2001 From: Tyler Leonhardt Date: Fri, 20 Dec 2019 11:07:50 -0800 Subject: [PATCH 1/4] Workaround Unicode characters in URIs .NET bug --- .../Converters/AbsoluteUriConverter.cs | 16 ++++++++- .../Models/ApplyWorkspaceEditParamsTests.cs | 31 +++++++++++++++++ ...aramsTests_$NonStandardCharactersTest.json | 34 +++++++++++++++++++ 3 files changed, 80 insertions(+), 1 deletion(-) create mode 100644 test/Lsp.Tests/Models/ApplyWorkspaceEditParamsTests_$NonStandardCharactersTest.json diff --git a/src/Protocol/Serialization/Converters/AbsoluteUriConverter.cs b/src/Protocol/Serialization/Converters/AbsoluteUriConverter.cs index b9e9abc87..769053bb3 100644 --- a/src/Protocol/Serialization/Converters/AbsoluteUriConverter.cs +++ b/src/Protocol/Serialization/Converters/AbsoluteUriConverter.cs @@ -51,7 +51,21 @@ public override void WriteJson(JsonWriter writer, Uri value, JsonSerializer seri throw new JsonSerializationException("The URI value must be an absolute Uri. Relative URI instances are not allowed."); } - writer.WriteValue(uriValue.ToString()); + if (uriValue.IsFile) + { + // Regular file paths + if (uriValue.HostNameType == UriHostNameType.Basic) + { + writer.WriteValue($"{uriValue.Scheme}://{uriValue.PathAndQuery}"); + return; + } + + // UNC file paths + writer.WriteValue($"{uriValue.Scheme}://{uriValue.Host}{uriValue.PathAndQuery}"); + return; + } + + writer.WriteValue(uriValue.AbsoluteUri); } } } diff --git a/test/Lsp.Tests/Models/ApplyWorkspaceEditParamsTests.cs b/test/Lsp.Tests/Models/ApplyWorkspaceEditParamsTests.cs index 8ae497816..2d8a4dafe 100644 --- a/test/Lsp.Tests/Models/ApplyWorkspaceEditParamsTests.cs +++ b/test/Lsp.Tests/Models/ApplyWorkspaceEditParamsTests.cs @@ -43,6 +43,37 @@ public void SimpleTest(string expected) deresult.Should().BeEquivalentTo(model); } + [Theory, JsonFixture] + public void NonStandardCharactersTest(string expected) + { + var model = new ApplyWorkspaceEditParams() + { + Edit = new WorkspaceEdit() + { + Changes = new Dictionary>() { + { + new Uri("/abc/123/Mörkö.cs"), new [] { + new TextEdit() { + NewText = "new text", + Range = new Range(new Position(1, 1), new Position(2,2)) + }, + new TextEdit() { + NewText = "new text2", + Range = new Range(new Position(3, 3), new Position(4,4)) + } + } + } + } + } + }; + var result = Fixture.SerializeObject(model); + + result.Should().Be(expected); + + var deresult = new Serializer(ClientVersion.Lsp3).DeserializeObject(expected); + deresult.Should().BeEquivalentTo(model); + } + [Theory, JsonFixture] public void DocumentChangesTest(string expected) { diff --git a/test/Lsp.Tests/Models/ApplyWorkspaceEditParamsTests_$NonStandardCharactersTest.json b/test/Lsp.Tests/Models/ApplyWorkspaceEditParamsTests_$NonStandardCharactersTest.json new file mode 100644 index 000000000..90bfd237b --- /dev/null +++ b/test/Lsp.Tests/Models/ApplyWorkspaceEditParamsTests_$NonStandardCharactersTest.json @@ -0,0 +1,34 @@ +{ + "edit": { + "changes": { + "file:///abc/123/M%C3%B6rk%C3%B6.cs": [ + { + "range": { + "start": { + "line": 1, + "character": 1 + }, + "end": { + "line": 2, + "character": 2 + } + }, + "newText": "new text" + }, + { + "range": { + "start": { + "line": 3, + "character": 3 + }, + "end": { + "line": 4, + "character": 4 + } + }, + "newText": "new text2" + } + ] + } + } +} From bea830ba84c74373bb6a096234b6037eec680550 Mon Sep 17 00:00:00 2001 From: Tyler Leonhardt Date: Fri, 20 Dec 2019 11:15:19 -0800 Subject: [PATCH 2/4] Chinese character test --- .../Lsp.Tests/Models/CodeActionParamsTests.cs | 27 ++++++++++++++ ...aramsTests_$NonStandardCharactersTest.json | 35 +++++++++++++++++++ 2 files changed, 62 insertions(+) create mode 100644 test/Lsp.Tests/Models/CodeActionParamsTests_$NonStandardCharactersTest.json diff --git a/test/Lsp.Tests/Models/CodeActionParamsTests.cs b/test/Lsp.Tests/Models/CodeActionParamsTests.cs index 2b3300cd5..e0869a570 100644 --- a/test/Lsp.Tests/Models/CodeActionParamsTests.cs +++ b/test/Lsp.Tests/Models/CodeActionParamsTests.cs @@ -37,5 +37,32 @@ public void SimpleTest(string expected) var deresult = new Serializer(ClientVersion.Lsp3).DeserializeObject(expected); deresult.Should().BeEquivalentTo(model); } + + [Theory, JsonFixture] + public void NonStandardCharactersTest(string expected) + { + var model = new CodeActionParams() { + Context = new CodeActionContext() { + Diagnostics = new[] { new Diagnostic() { + Code = new DiagnosticCode("abcd"), + Message = "message", + Range = new Range(new Position(1, 1), new Position(2,2)), + Severity = DiagnosticSeverity.Error, + Source = "csharp" + } } + + }, + Range = new Range(new Position(1, 1), new Position(2, 2)), + TextDocument = new TextDocumentIdentifier() { + Uri = new Uri("/test/123/树.cs") + } + }; + var result = Fixture.SerializeObject(model); + + result.Should().Be(expected); + + var deresult = new Serializer(ClientVersion.Lsp3).DeserializeObject(expected); + deresult.Should().BeEquivalentTo(model); + } } } diff --git a/test/Lsp.Tests/Models/CodeActionParamsTests_$NonStandardCharactersTest.json b/test/Lsp.Tests/Models/CodeActionParamsTests_$NonStandardCharactersTest.json new file mode 100644 index 000000000..5eb85d856 --- /dev/null +++ b/test/Lsp.Tests/Models/CodeActionParamsTests_$NonStandardCharactersTest.json @@ -0,0 +1,35 @@ +{ + "textDocument": { + "uri": "file:///test/123/%E6%A0%91.cs" + }, + "range": { + "start": { + "line": 1, + "character": 1 + }, + "end": { + "line": 2, + "character": 2 + } + }, + "context": { + "diagnostics": [ + { + "range": { + "start": { + "line": 1, + "character": 1 + }, + "end": { + "line": 2, + "character": 2 + } + }, + "severity": 1, + "code": "abcd", + "source": "csharp", + "message": "message" + } + ] + } +} From f29c0c5437fac644d82004cf23a82ab5e6d348fc Mon Sep 17 00:00:00 2001 From: Tyler Leonhardt Date: Fri, 20 Dec 2019 13:58:00 -0800 Subject: [PATCH 3/4] more tests and more models using the converter --- src/Protocol/Models/ConfigurationItem.cs | 3 + src/Protocol/Models/DocumentLink.cs | 3 +- src/Protocol/Models/InitializeParams.cs | 2 + src/Protocol/Models/LocationLink.cs | 5 +- src/Protocol/Models/WorkspaceEdit.cs | 3 + .../Converters/AbsoluteUriConverter.cs | 36 ++++++--- .../Converters/DictionaryUriConverter.cs | 77 +++++++++++++++++++ .../Models/ApplyWorkspaceEditParamsTests.cs | 3 +- .../Lsp.Tests/Models/CodeActionParamsTests.cs | 3 +- test/Lsp.Tests/Models/CodeLensParamsTests.cs | 15 ++++ ...aramsTests_$NonStandardCharactersTest.json | 5 ++ .../DidChangeTextDocumentParamsTests.cs | 23 ++++++ ...aramsTests_$NonStandardCharactersTest.json | 22 ++++++ .../DidChangeWatchedFilesParamsTests.cs | 20 +++++ ...aramsTests_$NonStandardCharactersTest.json | 8 ++ 15 files changed, 214 insertions(+), 14 deletions(-) create mode 100644 src/Protocol/Serialization/Converters/DictionaryUriConverter.cs create mode 100644 test/Lsp.Tests/Models/CodeLensParamsTests_$NonStandardCharactersTest.json create mode 100644 test/Lsp.Tests/Models/DidChangeTextDocumentParamsTests_$NonStandardCharactersTest.json create mode 100644 test/Lsp.Tests/Models/DidChangeWatchedFilesParamsTests_$NonStandardCharactersTest.json diff --git a/src/Protocol/Models/ConfigurationItem.cs b/src/Protocol/Models/ConfigurationItem.cs index 75df5c8ab..0abd1af6c 100644 --- a/src/Protocol/Models/ConfigurationItem.cs +++ b/src/Protocol/Models/ConfigurationItem.cs @@ -1,11 +1,14 @@ using System; +using Newtonsoft.Json; using OmniSharp.Extensions.LanguageServer.Protocol.Serialization; +using OmniSharp.Extensions.LanguageServer.Protocol.Serialization.Converters; namespace OmniSharp.Extensions.LanguageServer.Protocol.Models { public class ConfigurationItem { [Optional] + [JsonConverter(typeof(AbsoluteUriConverter))] public Uri ScopeUri { get; set; } [Optional] public string Section { get; set; } diff --git a/src/Protocol/Models/DocumentLink.cs b/src/Protocol/Models/DocumentLink.cs index 0705b10c2..9eb10383b 100644 --- a/src/Protocol/Models/DocumentLink.cs +++ b/src/Protocol/Models/DocumentLink.cs @@ -2,8 +2,8 @@ using MediatR; using Newtonsoft.Json; using Newtonsoft.Json.Linq; -using Newtonsoft.Json.Serialization; using OmniSharp.Extensions.LanguageServer.Protocol.Serialization; +using OmniSharp.Extensions.LanguageServer.Protocol.Serialization.Converters; namespace OmniSharp.Extensions.LanguageServer.Protocol.Models { @@ -23,6 +23,7 @@ public class DocumentLink : ICanBeResolved, IRequest /// The uri this link points to. If missing a resolve request is sent later. /// [Optional] + [JsonConverter(typeof(AbsoluteUriConverter))] public Uri Target { get; set; } /// diff --git a/src/Protocol/Models/InitializeParams.cs b/src/Protocol/Models/InitializeParams.cs index 1630a3e89..9a3ecc876 100644 --- a/src/Protocol/Models/InitializeParams.cs +++ b/src/Protocol/Models/InitializeParams.cs @@ -4,6 +4,7 @@ using Newtonsoft.Json.Serialization; using OmniSharp.Extensions.LanguageServer.Protocol.Client.Capabilities; using OmniSharp.Extensions.LanguageServer.Protocol.Serialization; +using OmniSharp.Extensions.LanguageServer.Protocol.Serialization.Converters; namespace OmniSharp.Extensions.LanguageServer.Protocol.Models { @@ -34,6 +35,7 @@ public string RootPath /// folder is open. If both `rootPath` and `rootUri` are set /// `rootUri` wins. /// + [JsonConverter(typeof(AbsoluteUriConverter))] public Uri RootUri { get; set; } /// diff --git a/src/Protocol/Models/LocationLink.cs b/src/Protocol/Models/LocationLink.cs index 60389252f..7b8725476 100644 --- a/src/Protocol/Models/LocationLink.cs +++ b/src/Protocol/Models/LocationLink.cs @@ -1,5 +1,7 @@ using System; +using Newtonsoft.Json; using OmniSharp.Extensions.LanguageServer.Protocol.Serialization; +using OmniSharp.Extensions.LanguageServer.Protocol.Serialization.Converters; namespace OmniSharp.Extensions.LanguageServer.Protocol.Models { @@ -17,6 +19,7 @@ public class LocationLink /// /// The target resource identifier of this link. /// + [JsonConverter(typeof(AbsoluteUriConverter))] public Uri TargetUri { get; set; } /// @@ -32,4 +35,4 @@ public class LocationLink /// public Range TargetSelectionRange { get; set; } } -} \ No newline at end of file +} diff --git a/src/Protocol/Models/WorkspaceEdit.cs b/src/Protocol/Models/WorkspaceEdit.cs index 4af14fb66..032ef4b33 100644 --- a/src/Protocol/Models/WorkspaceEdit.cs +++ b/src/Protocol/Models/WorkspaceEdit.cs @@ -1,7 +1,9 @@ using System; using System.Collections.Generic; +using Newtonsoft.Json; using Newtonsoft.Json.Serialization; using OmniSharp.Extensions.LanguageServer.Protocol.Serialization; +using OmniSharp.Extensions.LanguageServer.Protocol.Serialization.Converters; namespace OmniSharp.Extensions.LanguageServer.Protocol.Models { @@ -11,6 +13,7 @@ public class WorkspaceEdit /// Holds changes to existing resources. /// [Optional] + [JsonConverter(typeof(DictionaryUriConverter>))] public IDictionary> Changes { get; set; } /// /// An array of `TextDocumentEdit`s to express changes to n different text documents diff --git a/src/Protocol/Serialization/Converters/AbsoluteUriConverter.cs b/src/Protocol/Serialization/Converters/AbsoluteUriConverter.cs index 769053bb3..7a4a037dc 100644 --- a/src/Protocol/Serialization/Converters/AbsoluteUriConverter.cs +++ b/src/Protocol/Serialization/Converters/AbsoluteUriConverter.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. // #see https://github.com/NuGet/NuGet.Server using System; +using System.Text; using Newtonsoft.Json; namespace OmniSharp.Extensions.LanguageServer.Protocol.Serialization.Converters @@ -46,26 +47,41 @@ public override void WriteJson(JsonWriter writer, Uri value, JsonSerializer seri throw new JsonSerializationException("The value must be a URI."); } - if (!uriValue.IsAbsoluteUri) + writer.WriteValue(Convert(uriValue)); + } + + public static string Convert(Uri uri) + { + if (!uri.IsAbsoluteUri) { throw new JsonSerializationException("The URI value must be an absolute Uri. Relative URI instances are not allowed."); } - if (uriValue.IsFile) + if (uri.IsFile) { - // Regular file paths - if (uriValue.HostNameType == UriHostNameType.Basic) + // First add the file scheme and :// + var builder = new StringBuilder(uri.Scheme) + .Append("://"); + + // UNC file paths use the Host + if (uri.HostNameType != UriHostNameType.Basic) { - writer.WriteValue($"{uriValue.Scheme}://{uriValue.PathAndQuery}"); - return; + builder.Append(uri.Host); } - // UNC file paths - writer.WriteValue($"{uriValue.Scheme}://{uriValue.Host}{uriValue.PathAndQuery}"); - return; + // Paths that start with a drive letter don't have a slash in the PathAndQuery + // but they need it in the final result. + if (uri.PathAndQuery[0] != '/') + { + builder.Append('/'); + } + + // Lastly add the remaining parts of the URL + builder.Append(uri.PathAndQuery); + return builder.ToString(); } - writer.WriteValue(uriValue.AbsoluteUri); + return uri.AbsoluteUri; } } } diff --git a/src/Protocol/Serialization/Converters/DictionaryUriConverter.cs b/src/Protocol/Serialization/Converters/DictionaryUriConverter.cs new file mode 100644 index 000000000..5c8f11b2a --- /dev/null +++ b/src/Protocol/Serialization/Converters/DictionaryUriConverter.cs @@ -0,0 +1,77 @@ +using System; +using System.Collections.Generic; +using Newtonsoft.Json; + +namespace OmniSharp.Extensions.LanguageServer.Protocol.Serialization.Converters +{ + /// + /// This is necessary because Newtonsoft.Json creates instances with + /// which treats UNC paths as relative. NuGet.Core uses + /// which treats UNC paths as absolute. For more details, see: + /// https://github.com/JamesNK/Newtonsoft.Json/issues/2128 + /// < + class DictionaryUriConverter : + JsonConverter> where TKey : Uri + { + private readonly AbsoluteUriConverter _uriConverter = new AbsoluteUriConverter(); + + public override Dictionary ReadJson( + JsonReader reader, + Type objectType, + Dictionary existingValue, + bool hasExistingValue, + JsonSerializer serializer) + { + if (reader.TokenType != JsonToken.StartObject) + { + throw new JsonException(); + } + + Dictionary value = new Dictionary(); + + while (reader.Read()) + { + if (reader.TokenType == JsonToken.EndObject) + { + return value; + } + + // Get the key. + if (reader.TokenType != JsonToken.PropertyName) + { + throw new JsonException(); + } + + // Get the stringified Uri. + string propertyName = (string) reader.Value; + reader.Read(); + + Uri key = new Uri(propertyName, UriKind.Absolute); + + // Get the value. + TValue v = serializer.Deserialize(reader); + + // Add to dictionary. + value.Add((TKey) key, v); + } + + throw new JsonException(); + } + + public override void WriteJson( + JsonWriter writer, + Dictionary value, + JsonSerializer serializer) + { + writer.WriteStartObject(); + + foreach (KeyValuePair kvp in value) + { + writer.WritePropertyName(AbsoluteUriConverter.Convert(kvp.Key)); + serializer.Serialize(writer, kvp.Value); + } + + writer.WriteEndObject(); + } + } +} diff --git a/test/Lsp.Tests/Models/ApplyWorkspaceEditParamsTests.cs b/test/Lsp.Tests/Models/ApplyWorkspaceEditParamsTests.cs index 2d8a4dafe..a44a14d71 100644 --- a/test/Lsp.Tests/Models/ApplyWorkspaceEditParamsTests.cs +++ b/test/Lsp.Tests/Models/ApplyWorkspaceEditParamsTests.cs @@ -52,7 +52,8 @@ public void NonStandardCharactersTest(string expected) { Changes = new Dictionary>() { { - new Uri("/abc/123/Mörkö.cs"), new [] { + // Mörkö + new Uri("file:///abc/123/M%C3%B6rk%C3%B6.cs"), new [] { new TextEdit() { NewText = "new text", Range = new Range(new Position(1, 1), new Position(2,2)) diff --git a/test/Lsp.Tests/Models/CodeActionParamsTests.cs b/test/Lsp.Tests/Models/CodeActionParamsTests.cs index e0869a570..1a4c05e28 100644 --- a/test/Lsp.Tests/Models/CodeActionParamsTests.cs +++ b/test/Lsp.Tests/Models/CodeActionParamsTests.cs @@ -54,7 +54,8 @@ public void NonStandardCharactersTest(string expected) }, Range = new Range(new Position(1, 1), new Position(2, 2)), TextDocument = new TextDocumentIdentifier() { - Uri = new Uri("/test/123/树.cs") + // 树 - Chinese for tree + Uri = new Uri("file:///test/123/%E6%A0%91.cs") } }; var result = Fixture.SerializeObject(model); diff --git a/test/Lsp.Tests/Models/CodeLensParamsTests.cs b/test/Lsp.Tests/Models/CodeLensParamsTests.cs index 7c3b2596e..e5b1d2372 100644 --- a/test/Lsp.Tests/Models/CodeLensParamsTests.cs +++ b/test/Lsp.Tests/Models/CodeLensParamsTests.cs @@ -24,5 +24,20 @@ public void SimpleTest(string expected) var deresult = new Serializer(ClientVersion.Lsp3).DeserializeObject(expected); deresult.Should().BeEquivalentTo(model); } + + [Theory, JsonFixture] + public void NonStandardCharactersTest(string expected) + { + var model = new CodeLensParams() { + // UNC path with Chinese character for tree. + TextDocument = new TextDocumentIdentifier(new Uri("\\\\abc\\123\\树.cs")), + }; + var result = Fixture.SerializeObject(model); + + result.Should().Be(expected); + + var deresult = new Serializer(ClientVersion.Lsp3).DeserializeObject(expected); + deresult.Should().BeEquivalentTo(model); + } } } diff --git a/test/Lsp.Tests/Models/CodeLensParamsTests_$NonStandardCharactersTest.json b/test/Lsp.Tests/Models/CodeLensParamsTests_$NonStandardCharactersTest.json new file mode 100644 index 000000000..aec6cf272 --- /dev/null +++ b/test/Lsp.Tests/Models/CodeLensParamsTests_$NonStandardCharactersTest.json @@ -0,0 +1,5 @@ +{ + "textDocument": { + "uri": "file://abc/123/%E6%A0%91.cs" + } +} diff --git a/test/Lsp.Tests/Models/DidChangeTextDocumentParamsTests.cs b/test/Lsp.Tests/Models/DidChangeTextDocumentParamsTests.cs index 7e0e0e1ba..e0c8ca097 100644 --- a/test/Lsp.Tests/Models/DidChangeTextDocumentParamsTests.cs +++ b/test/Lsp.Tests/Models/DidChangeTextDocumentParamsTests.cs @@ -33,5 +33,28 @@ public void SimpleTest(string expected) var deresult = new Serializer(ClientVersion.Lsp3).DeserializeObject(expected); deresult.Should().BeEquivalentTo(model); } + + [Theory, JsonFixture] + public void NonStandardCharactersTest(string expected) + { + var model = new DidChangeTextDocumentParams() { + ContentChanges = new[] { + new TextDocumentContentChangeEvent() { + Range = new Range(new Position(1,1), new Position(2, 2)), + RangeLength = 12, + Text = "abc" + } + }, + TextDocument = new VersionedTextDocumentIdentifier() { + Uri = new Uri("C:\\abc\\Mörkö.cs") + } + }; + var result = Fixture.SerializeObject(model); + + result.Should().Be(expected); + + var deresult = new Serializer(ClientVersion.Lsp3).DeserializeObject(expected); + deresult.Should().BeEquivalentTo(model); + } } } diff --git a/test/Lsp.Tests/Models/DidChangeTextDocumentParamsTests_$NonStandardCharactersTest.json b/test/Lsp.Tests/Models/DidChangeTextDocumentParamsTests_$NonStandardCharactersTest.json new file mode 100644 index 000000000..81bb8294c --- /dev/null +++ b/test/Lsp.Tests/Models/DidChangeTextDocumentParamsTests_$NonStandardCharactersTest.json @@ -0,0 +1,22 @@ +{ + "textDocument": { + "version": 0, + "uri": "file:///C:/abc/M%C3%B6rk%C3%B6.cs" + }, + "contentChanges": [ + { + "range": { + "start": { + "line": 1, + "character": 1 + }, + "end": { + "line": 2, + "character": 2 + } + }, + "rangeLength": 12, + "text": "abc" + } + ] +} diff --git a/test/Lsp.Tests/Models/DidChangeWatchedFilesParamsTests.cs b/test/Lsp.Tests/Models/DidChangeWatchedFilesParamsTests.cs index 1acbdfe70..eda331235 100644 --- a/test/Lsp.Tests/Models/DidChangeWatchedFilesParamsTests.cs +++ b/test/Lsp.Tests/Models/DidChangeWatchedFilesParamsTests.cs @@ -29,5 +29,25 @@ public void SimpleTest(string expected) var deresult = new Serializer(ClientVersion.Lsp3).DeserializeObject(expected); deresult.Should().BeEquivalentTo(model); } + + [Theory, JsonFixture] + public void NonStandardCharactersTest(string expected) + { + var model = new DidChangeWatchedFilesParams() { + Changes = new[] { + new FileEvent() { + Type = FileChangeType.Created, + // Mörkö + Uri = new Uri("file:///M%C3%B6rk%C3%B6.cs") + } + } + }; + var result = Fixture.SerializeObject(model); + + result.Should().Be(expected); + + var deresult = new Serializer(ClientVersion.Lsp3).DeserializeObject(expected); + deresult.Should().BeEquivalentTo(model); + } } } diff --git a/test/Lsp.Tests/Models/DidChangeWatchedFilesParamsTests_$NonStandardCharactersTest.json b/test/Lsp.Tests/Models/DidChangeWatchedFilesParamsTests_$NonStandardCharactersTest.json new file mode 100644 index 000000000..abad85060 --- /dev/null +++ b/test/Lsp.Tests/Models/DidChangeWatchedFilesParamsTests_$NonStandardCharactersTest.json @@ -0,0 +1,8 @@ +{ + "changes": [ + { + "uri": "file:///M%C3%B6rk%C3%B6.cs", + "type": 1 + } + ] +} From 025a8d038d7090591472de242c034f838aa93401 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Bj=C3=B6rkstr=C3=B6m?= Date: Mon, 30 Dec 2019 23:48:48 +0200 Subject: [PATCH 4/4] Fix build errors on .NET SDK 3.1.100 and fixup some code after review. --- src/JsonRpc/RequestRouterBase.cs | 5 +- src/Protocol/Models/BooleanOr.cs | 8 +- src/Protocol/Models/WorkspaceEdit.cs | 2 +- .../Converters/AbsoluteUriConverter.cs | 32 ++---- .../Converters/AbsoluteUriKeyConverter.cs | 104 ++++++++++++++++++ .../Converters/DictionaryUriConverter.cs | 77 ------------- 6 files changed, 128 insertions(+), 100 deletions(-) create mode 100644 src/Protocol/Serialization/Converters/AbsoluteUriKeyConverter.cs delete mode 100644 src/Protocol/Serialization/Converters/DictionaryUriConverter.cs diff --git a/src/JsonRpc/RequestRouterBase.cs b/src/JsonRpc/RequestRouterBase.cs index 654e00913..944b838e5 100644 --- a/src/JsonRpc/RequestRouterBase.cs +++ b/src/JsonRpc/RequestRouterBase.cs @@ -99,7 +99,10 @@ public virtual async Task RouteRequest(TDescriptor descriptor, Re // TODO: Try / catch for Internal Error try { - if (descriptor == default) + // To avoid boxing, the best way to compare generics for equality is with EqualityComparer.Default. + // This respects IEquatable (without boxing) as well as object.Equals, and handles all the Nullable "lifted" nuances. + // https://stackoverflow.com/a/864860 + if (EqualityComparer.Default.Equals(descriptor, default)) { _logger.LogDebug("descriptor not found for Request ({Id}) {Method}", request.Id, request.Method); return new MethodNotFound(request.Id, request.Method); diff --git a/src/Protocol/Models/BooleanOr.cs b/src/Protocol/Models/BooleanOr.cs index 589ac51df..66e7e837b 100644 --- a/src/Protocol/Models/BooleanOr.cs +++ b/src/Protocol/Models/BooleanOr.cs @@ -1,3 +1,5 @@ +using System.Collections.Generic; + namespace OmniSharp.Extensions.LanguageServer.Protocol.Models { public class BooleanOr @@ -15,7 +17,11 @@ public BooleanOr(bool value) _bool = value; } - public bool IsValue => this._value != default; + // To avoid boxing, the best way to compare generics for equality is with EqualityComparer.Default. + // This respects IEquatable (without boxing) as well as object.Equals, and handles all the Nullable "lifted" nuances. + // https://stackoverflow.com/a/864860 + public bool IsValue => !EqualityComparer.Default.Equals(_value, default); + public T Value { get { return this._value; } diff --git a/src/Protocol/Models/WorkspaceEdit.cs b/src/Protocol/Models/WorkspaceEdit.cs index 032ef4b33..2d6e04dba 100644 --- a/src/Protocol/Models/WorkspaceEdit.cs +++ b/src/Protocol/Models/WorkspaceEdit.cs @@ -13,7 +13,7 @@ public class WorkspaceEdit /// Holds changes to existing resources. /// [Optional] - [JsonConverter(typeof(DictionaryUriConverter>))] + [JsonConverter(typeof(AbsoluteUriKeyConverter>))] public IDictionary> Changes { get; set; } /// /// An array of `TextDocumentEdit`s to express changes to n different text documents diff --git a/src/Protocol/Serialization/Converters/AbsoluteUriConverter.cs b/src/Protocol/Serialization/Converters/AbsoluteUriConverter.cs index 7a4a037dc..5945a3084 100644 --- a/src/Protocol/Serialization/Converters/AbsoluteUriConverter.cs +++ b/src/Protocol/Serialization/Converters/AbsoluteUriConverter.cs @@ -42,46 +42,38 @@ public override void WriteJson(JsonWriter writer, Uri value, JsonSerializer seri return; } - if (!(value is Uri uriValue)) - { - throw new JsonSerializationException("The value must be a URI."); - } - - writer.WriteValue(Convert(uriValue)); - } - - public static string Convert(Uri uri) - { - if (!uri.IsAbsoluteUri) + if (!value.IsAbsoluteUri) { throw new JsonSerializationException("The URI value must be an absolute Uri. Relative URI instances are not allowed."); } - if (uri.IsFile) + if (value.IsFile) { // First add the file scheme and :// - var builder = new StringBuilder(uri.Scheme) + var builder = new StringBuilder(value.Scheme) .Append("://"); // UNC file paths use the Host - if (uri.HostNameType != UriHostNameType.Basic) + if (value.HostNameType != UriHostNameType.Basic) { - builder.Append(uri.Host); + builder.Append(value.Host); } // Paths that start with a drive letter don't have a slash in the PathAndQuery // but they need it in the final result. - if (uri.PathAndQuery[0] != '/') + if (value.PathAndQuery[0] != '/') { builder.Append('/'); } // Lastly add the remaining parts of the URL - builder.Append(uri.PathAndQuery); - return builder.ToString(); + builder.Append(value.PathAndQuery); + writer.WriteValue(builder.ToString()); + } + else + { + writer.WriteValue(value.AbsoluteUri); } - - return uri.AbsoluteUri; } } } diff --git a/src/Protocol/Serialization/Converters/AbsoluteUriKeyConverter.cs b/src/Protocol/Serialization/Converters/AbsoluteUriKeyConverter.cs new file mode 100644 index 000000000..1df88a3f4 --- /dev/null +++ b/src/Protocol/Serialization/Converters/AbsoluteUriKeyConverter.cs @@ -0,0 +1,104 @@ +using System; +using System.Collections.Generic; +using System.Text; +using Newtonsoft.Json; + +namespace OmniSharp.Extensions.LanguageServer.Protocol.Serialization.Converters +{ + class AbsoluteUriKeyConverter : JsonConverter> + { + public override Dictionary ReadJson( + JsonReader reader, + Type objectType, + Dictionary existingValue, + bool hasExistingValue, + JsonSerializer serializer) + { + if (reader.TokenType != JsonToken.StartObject) + { + throw new JsonException(); + } + + var dictionary = new Dictionary(); + + while (reader.Read()) + { + if (reader.TokenType == JsonToken.EndObject) + { + return dictionary; + } + + // Get the key. + if (reader.TokenType != JsonToken.PropertyName) + { + throw new JsonSerializationException($"The token type must be a property name. Given {reader.TokenType.ToString()}"); + } + + // Get the stringified Uri. + var key = new Uri((string)reader.Value, UriKind.RelativeOrAbsolute); + if (!key.IsAbsoluteUri) + { + throw new JsonSerializationException($"The Uri must be absolute. Given: {reader.Value}"); + } + + // Get the value. + reader.Read(); + var value = serializer.Deserialize(reader); + + // Add to dictionary. + dictionary.Add(key, value); + } + + throw new JsonException(); + } + + public override void WriteJson( + JsonWriter writer, + Dictionary value, + JsonSerializer serializer) + { + writer.WriteStartObject(); + + foreach (var kvp in value) + { + var uri = kvp.Key; + if (!uri.IsAbsoluteUri) + { + throw new JsonSerializationException("The URI value must be an absolute Uri. Relative URI instances are not allowed."); + } + + if (uri.IsFile) + { + // First add the file scheme and :// + var builder = new StringBuilder(uri.Scheme) + .Append("://"); + + // UNC file paths use the Host + if (uri.HostNameType != UriHostNameType.Basic) + { + builder.Append(uri.Host); + } + + // Paths that start with a drive letter don't have a slash in the PathAndQuery + // but they need it in the final result. + if (uri.PathAndQuery[0] != '/') + { + builder.Append('/'); + } + + // Lastly add the remaining parts of the URL + builder.Append(uri.PathAndQuery); + writer.WritePropertyName(builder.ToString()); + } + else + { + writer.WritePropertyName(uri.AbsoluteUri); + } + + serializer.Serialize(writer, kvp.Value); + } + + writer.WriteEndObject(); + } + } +} diff --git a/src/Protocol/Serialization/Converters/DictionaryUriConverter.cs b/src/Protocol/Serialization/Converters/DictionaryUriConverter.cs deleted file mode 100644 index 5c8f11b2a..000000000 --- a/src/Protocol/Serialization/Converters/DictionaryUriConverter.cs +++ /dev/null @@ -1,77 +0,0 @@ -using System; -using System.Collections.Generic; -using Newtonsoft.Json; - -namespace OmniSharp.Extensions.LanguageServer.Protocol.Serialization.Converters -{ - /// - /// This is necessary because Newtonsoft.Json creates instances with - /// which treats UNC paths as relative. NuGet.Core uses - /// which treats UNC paths as absolute. For more details, see: - /// https://github.com/JamesNK/Newtonsoft.Json/issues/2128 - /// < - class DictionaryUriConverter : - JsonConverter> where TKey : Uri - { - private readonly AbsoluteUriConverter _uriConverter = new AbsoluteUriConverter(); - - public override Dictionary ReadJson( - JsonReader reader, - Type objectType, - Dictionary existingValue, - bool hasExistingValue, - JsonSerializer serializer) - { - if (reader.TokenType != JsonToken.StartObject) - { - throw new JsonException(); - } - - Dictionary value = new Dictionary(); - - while (reader.Read()) - { - if (reader.TokenType == JsonToken.EndObject) - { - return value; - } - - // Get the key. - if (reader.TokenType != JsonToken.PropertyName) - { - throw new JsonException(); - } - - // Get the stringified Uri. - string propertyName = (string) reader.Value; - reader.Read(); - - Uri key = new Uri(propertyName, UriKind.Absolute); - - // Get the value. - TValue v = serializer.Deserialize(reader); - - // Add to dictionary. - value.Add((TKey) key, v); - } - - throw new JsonException(); - } - - public override void WriteJson( - JsonWriter writer, - Dictionary value, - JsonSerializer serializer) - { - writer.WriteStartObject(); - - foreach (KeyValuePair kvp in value) - { - writer.WritePropertyName(AbsoluteUriConverter.Convert(kvp.Key)); - serializer.Serialize(writer, kvp.Value); - } - - writer.WriteEndObject(); - } - } -}