From a6ae6712555c3a2c63f892955fce5de1666cffa0 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 9 Jan 2026 21:53:35 +0100 Subject: [PATCH 1/4] Simplify DocumentUri implementation --- .../Protocol/Protocol/DocumentUri.cs | 79 ++++++------------- 1 file changed, 26 insertions(+), 53 deletions(-) diff --git a/src/LanguageServer/Protocol/Protocol/DocumentUri.cs b/src/LanguageServer/Protocol/Protocol/DocumentUri.cs index d1442f6f8db6a..cf1e3c63e9f2c 100644 --- a/src/LanguageServer/Protocol/Protocol/DocumentUri.cs +++ b/src/LanguageServer/Protocol/Protocol/DocumentUri.cs @@ -3,7 +3,7 @@ // See the LICENSE file in the project root for more information. using System; -using System.Diagnostics.CodeAnalysis; +using Microsoft.CodeAnalysis; namespace Roslyn.LanguageServer.Protocol; @@ -20,24 +20,14 @@ namespace Roslyn.LanguageServer.Protocol; /// In order to gracefully handle these issues, we defer the parsing of the URI until someone /// actually asks for it (and can handle the failure). /// -internal sealed class DocumentUri : IEquatable +internal sealed record class DocumentUri(string UriString) { - private readonly Lazy _parsedUriLazy; + private Optional _parsedUri; - public DocumentUri(string uriString) + public DocumentUri(Uri parsedUri) : this(parsedUri.AbsoluteUri) { - UriString = uriString; - _parsedUriLazy = new(() => ParseUri(UriString)); } - public DocumentUri(Uri parsedUri) - { - UriString = parsedUri.AbsoluteUri; - _parsedUriLazy = new(() => parsedUri); - } - - public string UriString { get; } - /// /// Gets the parsed System.Uri for the URI string. /// @@ -51,7 +41,14 @@ public DocumentUri(Uri parsedUri) /// For example, any URI containing a 'sub-delims' character in the host name /// is a valid RFC spec URI, but will fail with System.Uri /// - public Uri? ParsedUri => _parsedUriLazy.Value; + public Uri? ParsedUri + { + get + { + _parsedUri = _parsedUri.HasValue ? _parsedUri : ParseUri(UriString); + return _parsedUri.Value; + } + } private static Uri? ParseUri(string uriString) { @@ -68,8 +65,6 @@ public DocumentUri(Uri parsedUri) public override string ToString() => UriString; - public override bool Equals([NotNullWhen(true)] object? obj) => obj is DocumentUri other && this.Equals(other); - public bool Equals(DocumentUri otherUri) { // 99% of the time the equivalent URIs will have equivalent URI strings, as the client is expected to be consistent in how it sends the URIs to the server, @@ -100,14 +95,8 @@ public bool Equals(DocumentUri otherUri) // the URI untouched, even if they generally send unencoded URIs. In such cases we need to consider the encoded and unencoded URI equivalent. // // To handle that, we first compare the AbsoluteUri properties on both, which are always percent encoded. - if (this.ParsedUri.IsAbsoluteUri && otherUri.ParsedUri.IsAbsoluteUri && this.ParsedUri.AbsoluteUri == otherUri.ParsedUri.AbsoluteUri) - { - return true; - } - else - { - return Uri.Equals(this.ParsedUri, otherUri.ParsedUri); - } + return (this.ParsedUri.IsAbsoluteUri && otherUri.ParsedUri.IsAbsoluteUri && this.ParsedUri.AbsoluteUri == otherUri.ParsedUri.AbsoluteUri) || + Equals(this.ParsedUri, otherUri.ParsedUri); } public override int GetHashCode() @@ -118,33 +107,17 @@ public override int GetHashCode() return this.UriString.GetHashCode(); } - if (this.ParsedUri.IsAbsoluteUri) - { - // Since the Uri type does not consider an encoded Uri equal to an unencoded Uri, we need to handle this ourselves. - // The AbsoluteUri property is always encoded, so we can use this to compare the URIs (see Equals above). - // - // However, depending on the kind of URI, case sensitivity in AbsoluteUri should be ignored. - // Uri.GetHashCode normally handles this internally, but the parameters it uses to determine which comparison to use are not exposed. - // - // Instead, we will always create the hash code ignoring case, and will rely on the Equals implementation - // to handle collisions (between two Uris with different casing). This should be very rare in practice. - // Collisions can happen for non UNC URIs (e.g. `git:/blah` vs `git:/Blah`). - return StringComparer.OrdinalIgnoreCase.GetHashCode(this.ParsedUri.AbsoluteUri); - } - else - { - return this.ParsedUri.GetHashCode(); - } + // Since the Uri type does not consider an encoded Uri equal to an unencoded Uri, we need to handle this ourselves. + // The AbsoluteUri property is always encoded, so we can use this to compare the URIs (see Equals above). + // + // However, depending on the kind of URI, case sensitivity in AbsoluteUri should be ignored. + // Uri.GetHashCode normally handles this internally, but the parameters it uses to determine which comparison to use are not exposed. + // + // Instead, we will always create the hash code ignoring case, and will rely on the Equals implementation + // to handle collisions (between two Uris with different casing). This should be very rare in practice. + // Collisions can happen for non UNC URIs (e.g. `git:/blah` vs `git:/Blah`). + return this.ParsedUri.IsAbsoluteUri + ? StringComparer.OrdinalIgnoreCase.GetHashCode(this.ParsedUri.AbsoluteUri) + : this.ParsedUri.GetHashCode(); } - - public static bool operator ==(DocumentUri? uri1, DocumentUri? uri2) - => (uri1, uri2) switch - { - (null, null) => true, - (null, _) or (_, null) => false, - _ => uri1.Equals(uri2) - }; - - public static bool operator !=(DocumentUri? uri1, DocumentUri? uri2) - => !(uri1 == uri2); } From 181b08dc03279ac799098304ddb88c6ac570f70c Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 9 Jan 2026 22:23:34 +0100 Subject: [PATCH 2/4] Fix --- src/LanguageServer/Protocol/Protocol/DocumentUri.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/LanguageServer/Protocol/Protocol/DocumentUri.cs b/src/LanguageServer/Protocol/Protocol/DocumentUri.cs index cf1e3c63e9f2c..d24f8b9db3cdb 100644 --- a/src/LanguageServer/Protocol/Protocol/DocumentUri.cs +++ b/src/LanguageServer/Protocol/Protocol/DocumentUri.cs @@ -26,6 +26,7 @@ internal sealed record class DocumentUri(string UriString) public DocumentUri(Uri parsedUri) : this(parsedUri.AbsoluteUri) { + _parsedUri = parsedUri; } /// From ee594d199ca5f2fe2b21bbb8ac489a36183690e2 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 9 Jan 2026 22:24:09 +0100 Subject: [PATCH 3/4] Simplify --- src/LanguageServer/Protocol/Protocol/DocumentUri.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/LanguageServer/Protocol/Protocol/DocumentUri.cs b/src/LanguageServer/Protocol/Protocol/DocumentUri.cs index d24f8b9db3cdb..46f5fd6f0f3f6 100644 --- a/src/LanguageServer/Protocol/Protocol/DocumentUri.cs +++ b/src/LanguageServer/Protocol/Protocol/DocumentUri.cs @@ -25,9 +25,7 @@ internal sealed record class DocumentUri(string UriString) private Optional _parsedUri; public DocumentUri(Uri parsedUri) : this(parsedUri.AbsoluteUri) - { - _parsedUri = parsedUri; - } + => _parsedUri = parsedUri; /// /// Gets the parsed System.Uri for the URI string. From d680ff670d5e33cd212f6133592e5ae8ca432003 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 9 Jan 2026 23:42:51 +0100 Subject: [PATCH 4/4] Docs --- .../Protocol/Protocol/DocumentUri.cs | 75 +++++++++---------- 1 file changed, 37 insertions(+), 38 deletions(-) diff --git a/src/LanguageServer/Protocol/Protocol/DocumentUri.cs b/src/LanguageServer/Protocol/Protocol/DocumentUri.cs index 46f5fd6f0f3f6..dbc63a85c4b2a 100644 --- a/src/LanguageServer/Protocol/Protocol/DocumentUri.cs +++ b/src/LanguageServer/Protocol/Protocol/DocumentUri.cs @@ -12,13 +12,12 @@ namespace Roslyn.LanguageServer.Protocol; /// see https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#uri /// /// -/// While .NET has a type represent URIs (System.Uri), we do not want to use this type directly in -/// serialization and deserialization. System.Uri does full parsing and validation on the URI upfront, so -/// any issues in the uri format will cause deserialization to fail and bypass any of our error recovery. +/// While .NET has a type represent URIs (System.Uri), we do not want to use this type directly in serialization and +/// deserialization. System.Uri does full parsing and validation on the URI upfront, so any issues in the uri format +/// will cause deserialization to fail and bypass any of our error recovery. /// -/// Compounding this problem, System.Uri will fail to parse various RFC spec valid URIs. -/// In order to gracefully handle these issues, we defer the parsing of the URI until someone -/// actually asks for it (and can handle the failure). +/// Compounding this problem, System.Uri will fail to parse various RFC spec valid URIs. In order to gracefully handle +/// these issues, we defer the parsing of the URI until someone actually asks for it (and can handle the failure). /// internal sealed record class DocumentUri(string UriString) { @@ -34,11 +33,11 @@ public DocumentUri(Uri parsedUri) : this(parsedUri.AbsoluteUri) /// Null if the URI string is not parse-able with System.Uri. /// /// - /// Invalid RFC spec URI strings are not parse-able as so will return null here. - /// However, System.Uri can also fail to parse certain valid RFC spec URI strings. + /// Invalid RFC spec URI strings are not parse-able as so will return null here. However, System.Uri can also fail + /// to parse certain valid RFC spec URI strings. /// - /// For example, any URI containing a 'sub-delims' character in the host name - /// is a valid RFC spec URI, but will fail with System.Uri + /// For example, any URI containing a 'sub-delims' character in the host name is a valid RFC spec URI, but will fail + /// with System.Uri /// public Uri? ParsedUri { @@ -66,32 +65,32 @@ public Uri? ParsedUri public bool Equals(DocumentUri otherUri) { - // 99% of the time the equivalent URIs will have equivalent URI strings, as the client is expected to be consistent in how it sends the URIs to the server, - // either always encoded or always unencoded. - // See https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#uri + if (otherUri is null) + return false; + + // 99% of the time the equivalent URIs will have equivalent URI strings, as the client is expected to be + // consistent in how it sends the URIs to the server, either always encoded or always unencoded. See + // https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#uri if (this.UriString == otherUri.UriString) - { return true; - } - // If either of the URIs cannot be parsed + // Bail if we cannot parse either of the URIs. We already determined the URI strings are not equal and we need + // to be able to parse the URIs to do deeper equivalency checks. if (otherUri.ParsedUri is null || this.ParsedUri is null) - { - // Bail if we cannot parse either of the URIs. We already determined the URI strings are not equal - // and we need to be able to parse the URIs to do deeper equivalency checks. return false; - } - // Next we compare the parsed URIs to handle various casing and encoding scenarios (for example - different schemes may handle casing differently). + // Next we compare the parsed URIs to handle various casing and encoding scenarios (for example - different + // schemes may handle casing differently). - // Uri.Equals will not always consider a percent encoded URI equal to an unencoded URI, even if they point to the same resource. - // As above, the client is supposed to be consistent in which kind of URI they send. + // Uri.Equals will not always consider a percent encoded URI equal to an unencoded URI, even if they point to + // the same resource. As above, the client is supposed to be consistent in which kind of URI they send. // - // However, there are rare cases where we are comparing an unencoded URI to an encoded URI and should consider them - // equivalent if they point to the same file path. - // For example - say the client generally sends us the unencoded URI. When we serialize URIs back to the client, we always serialize the AbsoluteUri property (see FromUri). - // The AbsoluteUri property is *always* percent encoded - if this URI gets sent back to us as part of a data object on a request (e.g. codelens/resolve), the client will leave - // the URI untouched, even if they generally send unencoded URIs. In such cases we need to consider the encoded and unencoded URI equivalent. + // However, there are rare cases where we are comparing an unencoded URI to an encoded URI and should consider + // them equivalent if they point to the same file path. For example - say the client generally sends us the + // unencoded URI. When we serialize URIs back to the client, we always serialize the AbsoluteUri property (see + // FromUri). The AbsoluteUri property is *always* percent encoded - if this URI gets sent back to us as part of + // a data object on a request (e.g. codelens/resolve), the client will leave the URI untouched, even if they + // generally send unencoded URIs. In such cases we need to consider the encoded and unencoded URI equivalent. // // To handle that, we first compare the AbsoluteUri properties on both, which are always percent encoded. return (this.ParsedUri.IsAbsoluteUri && otherUri.ParsedUri.IsAbsoluteUri && this.ParsedUri.AbsoluteUri == otherUri.ParsedUri.AbsoluteUri) || @@ -100,21 +99,21 @@ public bool Equals(DocumentUri otherUri) public override int GetHashCode() { + // We can't do anything better than the uri string hash code if we cannot parse the URI. if (this.ParsedUri is null) - { - // We can't do anything better than the uri string hash code if we cannot parse the URI. return this.UriString.GetHashCode(); - } - // Since the Uri type does not consider an encoded Uri equal to an unencoded Uri, we need to handle this ourselves. - // The AbsoluteUri property is always encoded, so we can use this to compare the URIs (see Equals above). + // Since the Uri type does not consider an encoded Uri equal to an unencoded Uri, we need to handle this + // ourselves. The AbsoluteUri property is always encoded, so we can use this to compare the URIs (see Equals + // above). // - // However, depending on the kind of URI, case sensitivity in AbsoluteUri should be ignored. - // Uri.GetHashCode normally handles this internally, but the parameters it uses to determine which comparison to use are not exposed. + // However, depending on the kind of URI, case sensitivity in AbsoluteUri should be ignored. Uri.GetHashCode + // normally handles this internally, but the parameters it uses to determine which comparison to use are not + // exposed. // - // Instead, we will always create the hash code ignoring case, and will rely on the Equals implementation - // to handle collisions (between two Uris with different casing). This should be very rare in practice. - // Collisions can happen for non UNC URIs (e.g. `git:/blah` vs `git:/Blah`). + // Instead, we will always create the hash code ignoring case, and will rely on the Equals implementation to + // handle collisions (between two Uris with different casing). This should be very rare in practice. Collisions + // can happen for non UNC URIs (e.g. `git:/blah` vs `git:/Blah`). return this.ParsedUri.IsAbsoluteUri ? StringComparer.OrdinalIgnoreCase.GetHashCode(this.ParsedUri.AbsoluteUri) : this.ParsedUri.GetHashCode();