Merge pull request #196 from TylerLeonhardt/workaround-unicode-charac…

…ters-uri-bug

Workaround Unicode characters in URIs .NET bug

src/JsonRpc/RequestRouterBase.cs

@@ -99,7 +99,10 @@ public virtual async Task<ErrorResponse> 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<T>.Default.
+                        // This respects IEquatable<T> (without boxing) as well as object.Equals, and handles all the Nullable<T> "lifted" nuances.
+                        // https://stackoverflow.com/a/864860
+                        if (EqualityComparer<TDescriptor>.Default.Equals(descriptor, default))
                         {
                             _logger.LogDebug("descriptor not found for Request ({Id}) {Method}", request.Id, request.Method);
                             return new MethodNotFound(request.Id, request.Method);

src/Protocol/Models/BooleanOr.cs

@@ -1,3 +1,5 @@
+using System.Collections.Generic;
+
 namespace OmniSharp.Extensions.LanguageServer.Protocol.Models
 {
     public class BooleanOr<T>
@@ -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<T>.Default.
+        // This respects IEquatable<T> (without boxing) as well as object.Equals, and handles all the Nullable<T> "lifted" nuances.
+        // https://stackoverflow.com/a/864860
+        public bool IsValue => !EqualityComparer<T>.Default.Equals(_value, default);
+
         public T Value
         {
             get { return this._value; }

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; }

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<DocumentLink>
         /// The uri this link points to. If missing a resolve request is sent later.
         /// </summary>
         [Optional]
+        [JsonConverter(typeof(AbsoluteUriConverter))]
         public Uri Target { get; set; }
 
         /// </summary>

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.
         /// </summary>
+        [JsonConverter(typeof(AbsoluteUriConverter))]
         public Uri RootUri { get; set; }
 
         /// <summary>

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
         /// <summary>
         /// The target resource identifier of this link.
         /// </summary>
+        [JsonConverter(typeof(AbsoluteUriConverter))]
         public Uri TargetUri { get; set; }
 
         /// <summary>
@@ -32,4 +35,4 @@ public class LocationLink
         /// </summary>
         public Range TargetSelectionRange { get; set; }
     }
-}
+}

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.
         /// </summary>
         [Optional]
+        [JsonConverter(typeof(AbsoluteUriKeyConverter<IEnumerable<TextEdit>>))]
         public IDictionary<Uri, IEnumerable<TextEdit>> Changes { get; set; }
         /// <summary>
         /// An array of `TextDocumentEdit`s to express changes to n different text documents

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);
+            }
         }
     }
 }

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<TValue> : JsonConverter<Dictionary<Uri, TValue>>
+    {
+        public override Dictionary<Uri, TValue> ReadJson(
+            JsonReader reader,
+            Type objectType,
+            Dictionary<Uri, TValue> existingValue,
+            bool hasExistingValue,
+            JsonSerializer serializer)
+        {
+            if (reader.TokenType != JsonToken.StartObject)
+            {
+                throw new JsonException();
+            }
+
+            var dictionary = new Dictionary<Uri, TValue>();
+
+            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<TValue>(reader);
+
+                // Add to dictionary.
+                dictionary.Add(key, value);
+            }
+
+            throw new JsonException();
+        }
+
+        public override void WriteJson(
+            JsonWriter writer,
+            Dictionary<Uri, TValue> 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();
+        }
+    }
+}

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<Uri, IEnumerable<TextEdit>>() {
+                        {
+                            // 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<ApplyWorkspaceEditParams>(expected);
+            deresult.Should().BeEquivalentTo(model);
+        }
+
         [Theory, JsonFixture]
         public void DocumentChangesTest(string expected)
         {

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"
+                }
+            ]
+        }
+    }
+}

test/Lsp.Tests/Models/CodeActionParamsTests.cs

@@ -37,5 +37,33 @@ public void SimpleTest(string expected)
             var deresult = new Serializer(ClientVersion.Lsp3).DeserializeObject<CodeActionParams>(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<CodeActionParams>(expected);
+            deresult.Should().BeEquivalentTo(model);
+        }
     }
 }