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/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..2d6e04dba 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(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 b9e9abc87..5945a3084 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 @@ -41,17 +42,38 @@ public override void WriteJson(JsonWriter writer, Uri value, JsonSerializer seri return; } - if (!(value is Uri uriValue)) + if (!value.IsAbsoluteUri) { - throw new JsonSerializationException("The value must be a URI."); + throw new JsonSerializationException("The URI value must be an absolute Uri. Relative URI instances are not allowed."); } - if (!uriValue.IsAbsoluteUri) + if (value.IsFile) { - throw new JsonSerializationException("The URI value must be an absolute Uri. Relative URI instances are not allowed."); - } + // First add the file scheme and :// + var builder = new StringBuilder(value.Scheme) + .Append("://"); + + // UNC file paths use the Host + if (value.HostNameType != UriHostNameType.Basic) + { + builder.Append(value.Host); + } - writer.WriteValue(uriValue.ToString()); + // Paths that start with a drive letter don't have a slash in the PathAndQuery + // but they need it in the final result. + if (value.PathAndQuery[0] != '/') + { + builder.Append('/'); + } + + // Lastly add the remaining parts of the URL + builder.Append(value.PathAndQuery); + writer.WriteValue(builder.ToString()); + } + else + { + writer.WriteValue(value.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/test/Lsp.Tests/Models/ApplyWorkspaceEditParamsTests.cs b/test/Lsp.Tests/Models/ApplyWorkspaceEditParamsTests.cs index 8ae497816..a44a14d71 100644 --- a/test/Lsp.Tests/Models/ApplyWorkspaceEditParamsTests.cs +++ b/test/Lsp.Tests/Models/ApplyWorkspaceEditParamsTests.cs @@ -43,6 +43,38 @@ 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>() { + { + // 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)) + }, + 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" + } + ] + } + } +} diff --git a/test/Lsp.Tests/Models/CodeActionParamsTests.cs b/test/Lsp.Tests/Models/CodeActionParamsTests.cs index 2b3300cd5..1a4c05e28 100644 --- a/test/Lsp.Tests/Models/CodeActionParamsTests.cs +++ b/test/Lsp.Tests/Models/CodeActionParamsTests.cs @@ -37,5 +37,33 @@ 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() { + // 树 - Chinese for tree + Uri = new Uri("file:///test/123/%E6%A0%91.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" + } + ] + } +} 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 + } + ] +}