Fix rendering issue for links with special characters (#532)

* Fast path to look up resolved references by their absolute string

rdar://85531439

* Fix bug where links with special characters used the link as title

rdar://85531439

* Skip new test when running old link resolver implementation

* Add more type annotations in test to accommodate other compiler versions

* Expand the fallback parsing of authored documentation links

Author: d-ronnqvist

Sources/SwiftDocC/Infrastructure/DocumentationContext.swift

@@ -1009,6 +1009,9 @@ public class DocumentationContext: DocumentationContextDataProviderDelegate {
         return documentationCache[reference]
     }
 
+    /// A lookup of resolved references based on the reference's absolute string.
+    private(set) var referenceIndex = [String: ResolvedTopicReference]()
+    
     private func nodeWithInitializedContent(reference: ResolvedTopicReference, matches: [DocumentationContext.SemanticResult<Article>]?) -> DocumentationNode {
         precondition(documentationCache.keys.contains(reference))
 
@@ -2324,6 +2327,14 @@ public class DocumentationContext: DocumentationContextDataProviderDelegate {
         topicGraphGlobalAnalysis()
 
         preResolveModuleNames()
+        
+        referenceIndex.reserveCapacity(knownIdentifiers.count + nodeAnchorSections.count)
+        for reference in knownIdentifiers {
+            referenceIndex[reference.absoluteString] = reference
+        }
+        for reference in nodeAnchorSections.keys {
+            referenceIndex[reference.absoluteString] = reference
+        }
     }
 
     /// Given a list of topics that have been automatically curated, checks if a topic has been additionally manually curated

Sources/SwiftDocC/Infrastructure/DocumentationCurator.swift

@@ -38,7 +38,7 @@ struct DocumentationCurator {
         }
 
         // Optimization for absolute links.
-        if let cached = context.documentationCacheBasedLinkResolver.referenceFor(absoluteSymbolPath: destination, parent: resolved) {
+        if let cached = context.referenceIndex[destination] {
             return cached
         }
 

Sources/SwiftDocC/Model/Identifier.swift

@@ -623,7 +623,7 @@ public struct ResourceReference: Hashable {
 /// If this step is not performed, the disallowed characters are instead percent escape encoded instead which is less readable.
 /// For example, a path like `"hello world/example project"` is converted to `"hello-world/example-project"`
 /// instead of `"hello%20world/example%20project"`.
-func urlReadablePath(_ path: String) -> String {
+func urlReadablePath<S: StringProtocol>(_ path: S) -> String {
     return path.components(separatedBy: .urlPathNotAllowed).joined(separator: "-")
 }
 
@@ -639,7 +639,7 @@ private extension CharacterSet {
 ///
 /// If this step is not performed, the disallowed characters are instead percent escape encoded, which is less readable.
 /// For example, a fragment like `"#hello world"` is converted to `"#hello-world"` instead of `"#hello%20world"`.
-func urlReadableFragment(_ fragment: String) -> String {
+func urlReadableFragment<S: StringProtocol>(_ fragment: S) -> String {
     var fragment = fragment
         // Trim leading/trailing whitespace
         .trimmingCharacters(in: .whitespaces)

Sources/SwiftDocC/Model/Rendering/RenderContentCompiler.swift

@@ -175,9 +175,23 @@ struct RenderContentCompiler: MarkupVisitor {
             useOverriding = false
         } else if let overridingTitle = overridingTitle,
                   overridingTitle.hasPrefix(ResolvedTopicReference.urlScheme + ":"),
-                  destination.hasPrefix(ResolvedTopicReference.urlScheme + "://"),
-                  destination.hasSuffix(overridingTitle.dropFirst((ResolvedTopicReference.urlScheme + ":").count)) { // If the link is a transformed doc link, we don't use overriding info
-            useOverriding = false
+                  destination.hasPrefix(ResolvedTopicReference.urlScheme + "://")
+        {
+            // The overriding title looks like a documentation link. Escape it like a resolved reference string to compare it with the destination.
+            let withoutScheme = overridingTitle.dropFirst((ResolvedTopicReference.urlScheme + ":").count)
+            if destination.hasSuffix(withoutScheme) {
+                useOverriding = false
+            } else {
+                let escapedTitle: String
+                if let fragmentIndex = withoutScheme.firstIndex(of: "#") {
+                    let escapedFragment = withoutScheme[fragmentIndex...].dropFirst().addingPercentEncoding(withAllowedCharacters: .urlFragmentAllowed) ?? ""
+                    escapedTitle = "\(urlReadablePath(withoutScheme[..<fragmentIndex]))#\(escapedFragment)"
+                } else {
+                    escapedTitle = urlReadablePath(withoutScheme)
+                }
+                
+                useOverriding = !destination.hasSuffix(escapedTitle) // If the link is a transformed doc link, we don't use overriding info
+            }
         } else {
             useOverriding = true
         }
@@ -192,6 +206,11 @@ struct RenderContentCompiler: MarkupVisitor {
     }
 
     mutating func resolveTopicReference(_ destination: String) -> ResolvedTopicReference? {
+        if let cached = context.referenceIndex[destination] {
+            collectedTopicReferences.append(cached)
+            return cached
+        }
+        
         guard let validatedURL = ValidatedURL(parsingAuthoredLink: destination) else {
             return nil
         }
@@ -215,7 +234,7 @@ struct RenderContentCompiler: MarkupVisitor {
     }
 
     func resolveSymbolReference(destination: String) -> ResolvedTopicReference? {
-        if let cached = context.documentationCacheBasedLinkResolver.referenceFor(absoluteSymbolPath: destination, parent: identifier) {
+        if let cached = context.referenceIndex[destination] {
             return cached
         }
 

Sources/SwiftDocC/Model/Rendering/RenderNodeTranslator.swift

@@ -534,8 +534,7 @@ public struct RenderNodeTranslator: SemanticVisitor {
 
         let action: RenderInlineContent
         // We expect, at this point of the rendering, this API to be called with valid URLs, otherwise crash.
-        let unresolved = UnresolvedTopicReference(topicURL: ValidatedURL(link)!)
-        if case let .success(resolved) = context.resolve(.unresolved(unresolved), in: bundle.rootReference) {
+        if let resolved = context.referenceIndex[link.absoluteString] {
             action = RenderInlineContent.reference(identifier: RenderReferenceIdentifier(resolved.absoluteString),
                                                    isActive: true,
                                                    overridingTitle: overridingTitle,
@@ -545,7 +544,7 @@ public struct RenderNodeTranslator: SemanticVisitor {
             // This is an external link
             let externalLinkIdentifier = RenderReferenceIdentifier(forExternalLink: link.absoluteString)
             if linkReferences.keys.contains(externalLinkIdentifier.identifier) {
-                // If we've already seen this link, return the existing reference with an overriden title.
+                // If we've already seen this link, return the existing reference with an overridden title.
                 action = RenderInlineContent.reference(identifier: externalLinkIdentifier,
                                                        isActive: true,
                                                        overridingTitle: overridingTitle,

Sources/SwiftDocC/Semantics/MarkupReferenceResolver.swift

@@ -113,12 +113,17 @@ struct MarkupReferenceResolver: MarkupRewriter {
             return link
         }
         var link = link
+        let wasAutoLink = link.isAutolink
         link.destination = resolvedURL.absoluteString
+        if wasAutoLink {
+            link.replaceChildrenInRange(0..<link.childCount, with: [Text(resolvedURL.absoluteString)])
+            assert(link.isAutolink)
+        }
         return link
     }
 
     mutating func resolveAbsoluteSymbolLink(unresolvedDestination: String, elementRange range: SourceRange?) -> ResolvedTopicReference? {
-        if let cached = context.documentationCacheBasedLinkResolver.referenceFor(absoluteSymbolPath: unresolvedDestination, parent: rootReference) {
+        if let cached = context.referenceIndex[unresolvedDestination] {
             guard context.topicGraph.isLinkable(cached) == true else {
                 problems.append(disabledLinkDestinationProblem(reference: cached, source: source, range: range, severity: .warning))
                 return nil

Sources/SwiftDocC/Utility/ValidatedURL.swift

@@ -62,12 +62,57 @@ public struct ValidatedURL: Hashable, Equatable {
             return
         }
 
-        // If the string doesn't contain a fragment and the string couldn't be parsed with `ValidatedURL(parsing:)` above, then consider it invalid.
-        guard let fragmentSeparatorIndex = string.firstIndex(of: "#"), var components = URLComponents(string: String(string[..<fragmentSeparatorIndex])) else {
-            return nil
+        // If the `URLComponents(string:)` parsing in `init(parsingExact:)` failed try a fallback that attempts to individually
+        // percent encode each component.
+        //
+        // This fallback parsing tries to determine the substrings of the authored link that correspond to the scheme, bundle
+        // identifier, path, and fragment of a documentation link or symbol link. It is not meant to work with general links.
+        //
+        // By identifying the subranges they can each be individually percent encoded with the characters that are allowed for
+        // that component. This allows authored links to contain characters that wouldn't otherwise be valid in a general URL.
+        //
+        // Assigning the percent encoded values to `URLComponents/percentEncodedHost`, URLComponents/percentEncodedPath`, and
+        // URLComponents/percentEncodedFragment` allow for the creation of a `URLComponents` value with special characters.
+        var components = URLComponents()
+        var remainder = string[...]
+        
+        // See if the link is a documentation link and try to split out the scheme and bundle identifier. If the link isn't a
+        // documentation link it's assumed that it's a symbol link that start with the path component.
+        // Other general URLs should have been successfully parsed with `URLComponents(string:)` in `init(parsingExact:)` above.
+        if remainder.hasPrefix("\(ResolvedTopicReference.urlScheme):") {
+            // The authored link is a doc link
+            components.scheme = ResolvedTopicReference.urlScheme
+            remainder = remainder.dropFirst("\(ResolvedTopicReference.urlScheme):".count)
+            
+            if remainder.hasPrefix("//") {
+                // The authored link includes a bundle ID
+                guard let startOfPath = remainder.dropFirst(2).firstIndex(of: "/") else {
+                    // The link started with "doc://" but didn't contain another "/" to start of the path.
+                    return nil
+                }
+                components.percentEncodedHost = String(remainder[..<startOfPath]).addingPercentEncoding(withAllowedCharacters: .urlHostAllowed)
+                remainder = remainder[startOfPath...]
+            }
+        }
+        
+        // This either is the start of a symbol link or the remainder of a doc link after the scheme and bundle ID was parsed.
+        // This means that the remainder of the string is a path with an optional fragment. No other URL components are supported
+        // by documentation links and symbol links.
+        if let fragmentSeparatorIndex = remainder.firstIndex(of: "#") {
+            // Encode the path substring and fragment substring separately
+            guard let path = String(remainder[..<fragmentSeparatorIndex]).addingPercentEncoding(withAllowedCharacters: .urlPathAllowed) else {
+                return nil
+            }
+            components.percentEncodedPath = path
+            components.percentEncodedFragment = String(remainder[fragmentSeparatorIndex...].dropFirst()).addingPercentEncoding(withAllowedCharacters: .urlFragmentAllowed)
+        } else {
+            // Since the link didn't include a fragment, the rest of the string is the path.
+            guard let path = String(remainder).addingPercentEncoding(withAllowedCharacters: .urlPathAllowed) else {
+                return nil
+            }
+            components.percentEncodedPath = path
         }
 
-        components.percentEncodedFragment = String(string[fragmentSeparatorIndex...].dropFirst()).addingPercentEncoding(withAllowedCharacters: .urlFragmentAllowed)
         self.components = components
     }