Omit the "doc://" and identifier prefix from V2 link(_:) requests (#1320)

* Omit the "doc://" and identifier prefix from V2 `link(_:)` requests

rdar://162694487

* Remove repeated "link" word in code comment

Co-authored-by: Andrea Fernandez Buitrago <15234535+anferbui@users.noreply.github.com>

---------

Co-authored-by: Andrea Fernandez Buitrago <15234535+anferbui@users.noreply.github.com>

Author: d-ronnqvist

Sources/SwiftDocC/Infrastructure/External Data/OutOfProcessReferenceResolver+Communication.swift

@@ -49,6 +49,10 @@ extension OutOfProcessReferenceResolver {
     public enum RequestV2: Codable {
         /// A request to resolve a link
         ///
+        /// DocC omits the "doc:\/\/" and identifier prefix from the link string because it would be the same for every link request.
+        /// For example: if your resolver registers itself for the `"your.resolver.id"` identifier---by sending it in the ``ResponseV2/identifierAndCapabilities(_:_:)`` handshake message---
+        /// and DocC encounters a `doc://your.resolver.id/path/to/some-page#some-fragment` link in any documentation content, DocC sends the `"/path/to/some-page#some-fragment"` link to your resolver.
+        ///
         /// Your external resolver should respond with either:
         /// - a ``ResponseV2/resolved(_:)`` message, with information about the requested link.
         /// - a ``ResponseV2/failure(_:)`` message, with human-readable information about the problem that the external link resolver encountered while resolving the link.

Sources/SwiftDocC/Infrastructure/External Data/OutOfProcessReferenceResolver.swift

@@ -406,24 +406,26 @@ extension OutOfProcessReferenceResolver {
         private var linkCache: [String /* either a USR or an absolute UnresolvedTopicReference */: LinkDestinationSummary] = [:]
 
         func resolve(unresolvedReference: UnresolvedTopicReference) throws -> TopicReferenceResolutionResult {
-            let linkString = unresolvedReference.topicURL.absoluteString
-            if let cachedSummary = linkCache[linkString] {
+            let unresolvedReferenceString = unresolvedReference.topicURL.absoluteString
+            if let cachedSummary = linkCache[unresolvedReferenceString] {
                 return .success( makeReference(for: cachedSummary) )
             }
 
+            let linkString = unresolvedReference.topicURL.url.withoutHostAndPortAndScheme().standardized.absoluteString
             let response: ResponseV2 = try longRunningProcess.sendAndWait(request: RequestV2.link(linkString))
 
             switch response {
                 case .identifierAndCapabilities:
                     throw Error.executableSentBundleIdentifierAgain
 
                 case .failure(let diagnosticMessage):
+                    let prefixLength = 2 /* for "//" */ + bundleID.rawValue.utf8.count
                     let solutions: [Solution] = (diagnosticMessage.solutions ?? []).map {
                         Solution(summary: $0.summary, replacements: $0.replacement.map { replacement in
                             [Replacement(
                                 // The replacement ranges are relative to the link itself.
-                                // To replace the entire link, we create a range from 0 to the original length, both offset by -4 (the "doc:" length)
-                                range: SourceLocation(line: 0, column: -4, source: nil) ..< SourceLocation(line: 0, column: linkString.utf8.count - 4, source: nil),
+                                // To replace only the path and fragment portion of the link, we create a range from 0 to the relative link string length, both offset by the bundle ID length
+                                range: SourceLocation(line: 0, column: prefixLength, source: nil) ..< SourceLocation(line: 0, column: linkString.utf8.count + prefixLength, source: nil),
                                 replacement: replacement
                             )]
                         } ?? [])
@@ -435,7 +437,7 @@ extension OutOfProcessReferenceResolver {
 
                 case .resolved(let linkSummary):
                     // Cache the information for the original authored link
-                    linkCache[linkString] = linkSummary
+                    linkCache[unresolvedReferenceString] = linkSummary
                     // Cache the information for the resolved reference. That's what's will be used when returning the entity later.
                     let reference = makeReference(for: linkSummary)
                     linkCache[reference.absoluteString] = linkSummary

Tests/SwiftDocCTests/OutOfProcessReferenceResolverV2Tests.swift

@@ -424,7 +424,7 @@ class OutOfProcessReferenceResolverV2Tests: XCTestCase {
             let diagnosticInfo = OutOfProcessReferenceResolver.ResponseV2.DiagnosticInformation(
                 summary: "Some external link issue summary",
                 solutions: [
-                    .init(summary: "Some external solution", replacement: "some-replacement")
+                    .init(summary: "Some external solution", replacement: "/some-replacement")
                 ]
             )
             let encodedDiagnostic = try String(decoding: JSONEncoder().encode(diagnosticInfo), as: UTF8.self)
@@ -473,7 +473,7 @@ class OutOfProcessReferenceResolverV2Tests: XCTestCase {
         let solution = try XCTUnwrap(problem.possibleSolutions.first)
         XCTAssertEqual(solution.summary, "Some external solution")
         XCTAssertEqual(solution.replacements.count, 1)
-        XCTAssertEqual(solution.replacements.first?.range.lowerBound, .init(line: 3, column: 65, source: nil))
+        XCTAssertEqual(solution.replacements.first?.range.lowerBound, .init(line: 3, column: 87, source: nil))
         XCTAssertEqual(solution.replacements.first?.range.upperBound, .init(line: 3, column: 97, source: nil))
 
         // Verify the warning presentation
@@ -493,7 +493,7 @@ class OutOfProcessReferenceResolverV2Tests: XCTestCase {
         1 | # My root page
         2 |
         3 + This page contains an external link that will fail to resolve: <doc:\(highlight)//com.example.test/some-link\(clear)>
-          |                                                                 ╰─\(suggestion)suggestion: Some external solution\(clear)
+          |                                                                                       ╰─\(suggestion)suggestion: Some external solution\(clear)
         
         """)
 
@@ -504,17 +504,81 @@ class OutOfProcessReferenceResolverV2Tests: XCTestCase {
         XCTAssertEqual(try solution.applyTo(original), """
         # My root page
 
-        This page contains an external link that will fail to resolve: <some-replacement>
+        This page contains an external link that will fail to resolve: <doc://com.example.test/some-replacement>
+        """)
+    }
+    
+    func testOnlySendsPathAndFragmentInLinkRequest() async throws {
+        let externalBundleID: DocumentationBundle.Identifier = "com.example.test"
+        
+        let resolver: OutOfProcessReferenceResolver
+        let savedRequestsFile: URL
+        do {
+            let temporaryFolder = try createTemporaryDirectory()
+            savedRequestsFile = temporaryFolder.appendingPathComponent("saved-requests.txt")
+            
+            let executableLocation = temporaryFolder.appendingPathComponent("link-resolver-executable")
+            try """
+            #!/bin/bash
+            echo '{"identifier":"\(externalBundleID)","capabilities": 0}'  # Write this resolver's identifier & capabilities
+            read                                                           # Wait for docc to send a request
+            echo $REPLY >> \(savedRequestsFile.path)                       # Save the raw request string
+            echo '{"failure":"ignored error message"}'                     # Respond with an error message
+            # Repeat the same read-save-respond steps 2 more times
+            read                                                           # Wait for 2nd request
+            echo $REPLY >> \(savedRequestsFile.path)                       # Save the raw request
+            echo '{"failure":"ignored error message"}'                     # Respond 
+            read                                                           # Wait for 3rd request
+            echo $REPLY >> \(savedRequestsFile.path)                       # Save the raw request
+            echo '{"failure":"ignored error message"}'                     # Respond
+            """.write(to: executableLocation, atomically: true, encoding: .utf8)
+            
+            // `0o0700` is `-rwx------` (read, write, & execute only for owner)
+            try FileManager.default.setAttributes([.posixPermissions: 0o0700], ofItemAtPath: executableLocation.path)
+            XCTAssert(FileManager.default.isExecutableFile(atPath: executableLocation.path))
+             
+            resolver = try OutOfProcessReferenceResolver(processLocation: executableLocation, errorOutputHandler: { _ in })
+        }
+        
+        let catalog = Folder(name: "unit-test.docc", content: [
+            TextFile(name: "Something.md", utf8Content: """
+            # My root page
+            
+            This page contains an 3 external links hat will fail to resolve: 
+            - <doc://\(externalBundleID.rawValue)/some-link>
+            - <doc://\(externalBundleID.rawValue)/path/to/some-link>
+            - <doc://\(externalBundleID.rawValue)/path/to/some-link#some-fragment>
+            """)
+        ])
+        let inputDirectory = Folder(name: "path", content: [Folder(name: "to", content: [catalog])])
+        
+        var configuration = DocumentationContext.Configuration()
+        configuration.externalDocumentationConfiguration.sources = [
+            externalBundleID: resolver
+        ]
+        // Create the context, just to process all the documentation and make the 3 external link requests
+        _ = try await loadBundle(catalog: inputDirectory, configuration: configuration)
+        
+        // The requests can come in any order so we sort the output lines for easier comparison
+        let readRequests = try String(contentsOf: savedRequestsFile, encoding: .utf8)
+            .components(separatedBy: .newlines)
+            .filter { !$0.isEmpty }
+            .sorted(by: \.count)
+            .joined(separator: "\n")
+        XCTAssertEqual(readRequests, """
+        {"link":"/some-link"}
+        {"link":"/path/to/some-link"}
+        {"link":"/path/to/some-link#some-fragment"}
         """)
     }
 
     func testEncodingAndDecodingRequests() throws {
         do {
-            let request = OutOfProcessReferenceResolver.RequestV2.link("doc://com.example/path/to/something")
+            let request = OutOfProcessReferenceResolver.RequestV2.link("/path/to/some-page#some-fragment")
 
             let data = try JSONEncoder().encode(request)
             if case .link(let link) = try JSONDecoder().decode(OutOfProcessReferenceResolver.RequestV2.self, from: data) {
-                XCTAssertEqual(link, "doc://com.example/path/to/something")
+                XCTAssertEqual(link, "/path/to/some-page#some-fragment")
             } else {
                 XCTFail("Decoded the wrong type of request")
             }