From 5d1eb73a68c51bd89cb49be1f389f572c2c3c865 Mon Sep 17 00:00:00 2001
From: Alex Hoppen <ahoppen@apple.com>
Date: Tue, 10 Sep 2024 14:41:47 -0700
Subject: [PATCH] Revert "Add an extra percent encoding layer when encoding
 DocumentURIs to LSP requests"
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This reverts commit b2c17c748d32a3f7ed766e2a69f3ff0d132ab294.

The workaround isn’t needed anymore because we have a proper fix in the VS Code extension: https://github.com/swiftlang/vscode-swift/pull/1026
---
 .../SupportTypes/DocumentURI.swift            | 45 +------------------
 Sources/SKTestSupport/CheckCoding.swift       |  9 ++--
 .../CodingTests.swift                         | 23 ----------
 3 files changed, 5 insertions(+), 72 deletions(-)

diff --git a/Sources/LanguageServerProtocol/SupportTypes/DocumentURI.swift b/Sources/LanguageServerProtocol/SupportTypes/DocumentURI.swift
index 09f4d197..48ce2fb7 100644
--- a/Sources/LanguageServerProtocol/SupportTypes/DocumentURI.swift
+++ b/Sources/LanguageServerProtocol/SupportTypes/DocumentURI.swift
@@ -84,19 +84,7 @@ public struct DocumentURI: Codable, Hashable, Sendable {
   }
 
   public init(from decoder: Decoder) throws {
-    let string = try decoder.singleValueContainer().decode(String.self)
-    guard let url = URL(string: string) else {
-      throw FailedToConstructDocumentURIFromStringError(string: string)
-    }
-    if url.query() != nil, var urlComponents = URLComponents(string: url.absoluteString) {
-      // See comment in `encode(to:)`
-      urlComponents.percentEncodedQuery = urlComponents.percentEncodedQuery!.removingPercentEncoding
-      if let rewrittenQuery = urlComponents.url {
-        self.init(rewrittenQuery)
-        return
-      }
-    }
-    self.init(url)
+    try self.init(string: decoder.singleValueContainer().decode(String.self))
   }
 
   /// Equality check to handle escape sequences in file URLs.
@@ -109,36 +97,7 @@ public struct DocumentURI: Codable, Hashable, Sendable {
     hasher.combine(self.pseudoPath)
   }
 
-  private static let additionalQueryEncodingCharacterSet = CharacterSet(charactersIn: "?=&%").inverted
-
   public func encode(to encoder: Encoder) throws {
-    let urlToEncode: URL
-    if let query = storage.query(percentEncoded: true), var components = URLComponents(string: storage.absoluteString) {
-      // The URI standard RFC 3986 is ambiguous about whether percent encoding and their represented characters are
-      // considered equivalent. VS Code considers them equivalent and treats them the same:
-      //
-      // vscode.Uri.parse("x://a?b=xxxx%3Dyyyy").toString() -> 'x://a?b%3Dxxxx%3Dyyyy'
-      // vscode.Uri.parse("x://a?b=xxxx%3Dyyyy").toString(/*skipEncoding=*/true) -> 'x://a?b=xxxx=yyyy'
-      //
-      // This causes issues because SourceKit-LSP's macro expansion URLs encoded by URLComponents use `=` to denote the
-      // separation of a key and a value in the outer query. The value of the `parent` key may itself contain query
-      // items, which use the escaped form '%3D'. Simplified, such a URL may look like
-      // scheme://host?parent=scheme://host?line%3D2
-      // But after running this through VS Code's URI type `=` and `%3D` get canonicalized and are indistinguishable.
-      // To avoid this ambiguity, always percent escape the characters we use to distinguish URL query parameters,
-      // producing the following URL.
-      // scheme://host?parent%3Dscheme://host%3Fline%253D2
-      components.percentEncodedQuery =
-        query
-        .addingPercentEncoding(withAllowedCharacters: Self.additionalQueryEncodingCharacterSet)
-      if let componentsUrl = components.url {
-        urlToEncode = componentsUrl
-      } else {
-        urlToEncode = self.storage
-      }
-    } else {
-      urlToEncode = self.storage
-    }
-    try urlToEncode.absoluteString.encode(to: encoder)
+    try storage.absoluteString.encode(to: encoder)
   }
 }
diff --git a/Sources/SKTestSupport/CheckCoding.swift b/Sources/SKTestSupport/CheckCoding.swift
index 203841dd..358de770 100644
--- a/Sources/SKTestSupport/CheckCoding.swift
+++ b/Sources/SKTestSupport/CheckCoding.swift
@@ -43,12 +43,9 @@ package func checkCoding<T: Codable & Equatable>(
   XCTAssertEqual(json, str, file: file, line: line)
 
   let decoder = JSONDecoder()
-  XCTAssertNoThrow(
-    try {
-      let decodedValue = try decoder.decode(WrapFragment<T>.self, from: data).value
-      XCTAssertEqual(value, decodedValue, file: file, line: line)
-    }()
-  )
+  let decodedValue = try! decoder.decode(WrapFragment<T>.self, from: data).value
+
+  XCTAssertEqual(value, decodedValue, file: file, line: line)
 }
 
 /// JSONEncoder requires the top-level value to be encoded as a JSON container (array or object). Give it one.
diff --git a/Tests/LanguageServerProtocolTests/CodingTests.swift b/Tests/LanguageServerProtocolTests/CodingTests.swift
index 8297bde3..ee81ec2b 100644
--- a/Tests/LanguageServerProtocolTests/CodingTests.swift
+++ b/Tests/LanguageServerProtocolTests/CodingTests.swift
@@ -1343,29 +1343,6 @@ final class CodingTests: XCTestCase {
         """
     )
   }
-
-  func testDocumentUriQueryParameterCoding() throws {
-    checkCoding(
-      try DocumentURI(string: "scheme://host?parent=scheme://host?line%3D2"),
-      json: #"""
-        "scheme:\/\/host?parent%3Dscheme:\/\/host%3Fline%253D2"
-        """#
-    )
-
-    checkCoding(
-      try DocumentURI(string: "scheme://host?parent=scheme://host?parent%3Dscheme://host%3Fkey%253Dvalue"),
-      json: #"""
-        "scheme:\/\/host?parent%3Dscheme:\/\/host%3Fparent%253Dscheme:\/\/host%253Fkey%25253Dvalue"
-        """#
-    )
-
-    checkCoding(
-      try DocumentURI(string: "scheme://host?parent=with%23hash"),
-      json: #"""
-        "scheme:\/\/host?parent%3Dwith%2523hash"
-        """#
-    )
-  }
 }
 
 func with<T>(_ value: T, mutate: (inout T) -> Void) -> T {