Support disambiguating links with type signature information

Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+Dump.swift

@@ -11,6 +11,8 @@
 // This API isn't exposed anywhere and is only used from a debugger.
 #if DEBUG
 
+import SymbolKit
+
 /// A node in a tree structure that can be printed into a visual representation for debugging.
 private struct DumpableNode {
     var name: String
@@ -21,20 +23,25 @@ private extension PathHierarchy.Node {
     /// Maps the path hierarchy subtree into a representation that can be printed into a visual form for debugging.
     func dumpableNode() -> DumpableNode {
         // Each node is printed as 3-layer hierarchy with the child names, their kind disambiguation, and their hash disambiguation.
+
+        // One layer for the node itself that displays information about the symbol
         return DumpableNode(
-            name: symbol.map { "{ \($0.identifier.precise) : \($0.kind.identifier.identifier) [\(languages.map(\.name).joined(separator: ", "))] }" } ?? "[ \(name) ]",
-            children: children.sorted(by: \.key).map { (key, disambiguationTree) -> DumpableNode in
+            name: symbol.map(describe(_:)) ?? "[ \(name) ]",
+            children: children.sorted(by: \.key).map { (childName, disambiguationTree) -> DumpableNode in
+
+                // A second layer that displays the kind disambiguation
                 let grouped = [String: [PathHierarchy.DisambiguationContainer.Element]](grouping: disambiguationTree.storage, by: { $0.kind ?? "_" })
                 return DumpableNode(
-                    name: key,
+                    name: childName,
                     children: grouped.sorted(by: \.key).map { (kind, kindTree) -> DumpableNode in
+
+                        // A third layer that displays the hash disambiguation
                         DumpableNode(
                             name: kind,
                             children: kindTree.sorted(by: { lhs, rhs in (lhs.hash ?? "_") < (rhs.hash ?? "_") }).map { (element) -> DumpableNode in
-                                DumpableNode(
-                                    name: element.hash ?? "_",
-                                    children: [element.node.dumpableNode()]
-                                )
+
+                                // Recursively dump the subtree
+                                DumpableNode(name: element.hash ?? "_", children: [element.node.dumpableNode()])
                             }
                         )
                     }
@@ -44,6 +51,14 @@ private extension PathHierarchy.Node {
     }
 }
 
+private func describe(_ symbol: SymbolGraph.Symbol) -> String {
+    guard let (parameterTypes, returnValueTypes) = PathHierarchy.functionSignatureTypeNames(for: symbol) else {
+        return "{ \(symbol.identifier.precise) }"
+    }
+
+    return "{ \(symbol.identifier.precise) : (\(parameterTypes.joined(separator: ", "))) -> (\(returnValueTypes.joined(separator: ", "))) }"
+}
+
 extension PathHierarchy {
     /// Creates a visual representation or the path hierarchy for debugging.
     func dump() -> String {

Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+Error.swift

@@ -8,6 +8,7 @@
  See https://swift.org/CONTRIBUTORS.txt for Swift project authors
 */
 
+import Foundation
 import struct Markdown.SourceRange
 import struct Markdown.SourceLocation
 import SymbolKit
@@ -80,10 +81,38 @@ extension PathHierarchy.Error {
     ///   - fullNameOfNode: A closure that determines the full name of a node, to be displayed in collision diagnostics to precisely identify symbols and other pages.
     /// - Note: `Replacement`s produced by this function use `SourceLocation`s relative to the link text excluding its surrounding syntax.
     func makeTopicReferenceResolutionErrorInfo(fullNameOfNode: (PathHierarchy.Node) -> String) -> TopicReferenceResolutionErrorInfo {
+        // Both `.unknownDisambiguation(...)` and `.lookupCollisions(...)` create solutions on the same format from the same information.
         // This is defined inline because it captures `fullNameOfNode`.
-        func collisionIsBefore(_ lhs: (node: PathHierarchy.Node, disambiguation: String), _ rhs: (node: PathHierarchy.Node, disambiguation: String)) -> Bool {
-            return fullNameOfNode(lhs.node) + lhs.disambiguation
-                 < fullNameOfNode(rhs.node) + rhs.disambiguation
+        func makeCollisionSolutions(
+            from candidates: [(node: PathHierarchy.Node, disambiguation: String)],
+            nextPathComponent: PathHierarchy.PathComponent,
+            partialResultPrefix: Substring
+        ) -> (
+            pathPrefix: Substring,
+            foundDisambiguation: Substring,
+            solutions: [Solution]
+        ) {
+            let pathPrefix = partialResultPrefix + nextPathComponent.name
+            let foundDisambiguation = nextPathComponent.full.dropFirst(nextPathComponent.name.count)
+            let replacementRange = SourceRange.makeRelativeRange(startColumn: pathPrefix.count, length: foundDisambiguation.count)
+
+            let solutions: [Solution] = candidates
+                .map { (fullName: fullNameOfNode($0.node), disambiguation: $0.disambiguation) }
+                .sorted { lhs, rhs in
+                    // Sort by name first and disambiguation second
+                    if lhs.fullName == rhs.fullName {
+                        return lhs.disambiguation < rhs.disambiguation
+                    }
+                    return lhs.fullName < rhs.fullName
+                }
+                .map { (fullName: String, suggestedDisambiguation: String) -> Solution in
+                    // In contexts that display the solution message on a single line by removing newlines, this extra whitespace makes it look correct ─────────────╮
+                    //                                                                                                                                               ▼
+                    return Solution(summary: "\(Self.replacementOperationDescription(from: foundDisambiguation, to: suggestedDisambiguation, forCollision: true)) for \n\(fullName.singleQuoted)", replacements: [
+                        Replacement(range: replacementRange, replacement: suggestedDisambiguation)
+                    ])
+                }
+            return (pathPrefix, foundDisambiguation, solutions)
         }
 
         switch self {
@@ -127,7 +156,7 @@ extension PathHierarchy.Error {
 
         case .unfindableMatch(let node):
             return TopicReferenceResolutionErrorInfo("""
-                \(node.name.singleQuoted) can't be linked to in a partial documentation build
+            \(node.name.singleQuoted) can't be linked to in a partial documentation build
             """)
 
         case .nonSymbolMatchForSymbolLink(path: let path):
@@ -142,24 +171,13 @@ extension PathHierarchy.Error {
 
         case .unknownDisambiguation(partialResult: let partialResult, remaining: let remaining, candidates: let candidates):
             let nextPathComponent = remaining.first!
-            let validPrefix = partialResult.pathPrefix + nextPathComponent.name
-
-            let disambiguations = nextPathComponent.full.dropFirst(nextPathComponent.name.count)
-            let replacementRange = SourceRange.makeRelativeRange(startColumn: validPrefix.count, length: disambiguations.count)
-
-            let solutions: [Solution] = candidates
-                .sorted(by: collisionIsBefore)
-                .map { (node: PathHierarchy.Node, disambiguation: String) -> Solution in
-                    return Solution(summary: "\(Self.replacementOperationDescription(from: disambiguations, to: disambiguation)) for\n\(fullNameOfNode(node).singleQuoted)", replacements: [
-                        Replacement(range: replacementRange, replacement: disambiguation)
-                    ])
-                }
+            let (pathPrefix, foundDisambiguation, solutions) = makeCollisionSolutions(from: candidates, nextPathComponent: nextPathComponent, partialResultPrefix: partialResult.pathPrefix)
 
             return TopicReferenceResolutionErrorInfo("""
-                \(disambiguations.dropFirst().singleQuoted) isn't a disambiguation for \(nextPathComponent.name.singleQuoted) at \(partialResult.node.pathWithoutDisambiguation().singleQuoted)
+                \(foundDisambiguation.dropFirst().singleQuoted) isn't a disambiguation for \(nextPathComponent.name.singleQuoted) at \(partialResult.node.pathWithoutDisambiguation().singleQuoted)
                 """,
                 solutions: solutions,
-                rangeAdjustment: .makeRelativeRange(startColumn: validPrefix.count, length: disambiguations.count)
+                rangeAdjustment: .makeRelativeRange(startColumn: pathPrefix.count, length: foundDisambiguation.count)
             )
 
         case .unknownName(partialResult: let partialResult, remaining: let remaining, availableChildren: let availableChildren):
@@ -201,17 +219,7 @@ extension PathHierarchy.Error {
 
         case .lookupCollision(partialResult: let partialResult, remaining: let remaining, collisions: let collisions):
             let nextPathComponent = remaining.first!
-
-            let pathPrefix = partialResult.pathPrefix + nextPathComponent.name
-
-            let disambiguations = nextPathComponent.full.dropFirst(nextPathComponent.name.count)
-            let replacementRange = SourceRange.makeRelativeRange(startColumn: pathPrefix.count, length: disambiguations.count)
-
-            let solutions: [Solution] = collisions.sorted(by: collisionIsBefore).map { (node: PathHierarchy.Node, disambiguation: String) -> Solution in
-                return Solution(summary: "\(Self.replacementOperationDescription(from: disambiguations.dropFirst(), to: disambiguation)) for\n\(fullNameOfNode(node).singleQuoted)", replacements: [
-                    Replacement(range: replacementRange, replacement: "-" + disambiguation)
-                ])
-            }
+            let (pathPrefix, _, solutions) = makeCollisionSolutions(from: collisions, nextPathComponent: nextPathComponent, partialResultPrefix: partialResult.pathPrefix)
 
             return TopicReferenceResolutionErrorInfo("""
                 \(nextPathComponent.full.singleQuoted) is ambiguous at \(partialResult.node.pathWithoutDisambiguation().singleQuoted)
@@ -222,14 +230,25 @@ extension PathHierarchy.Error {
         }
     }
 
-    private static func replacementOperationDescription(from: some StringProtocol, to: some StringProtocol) -> String {
+    private static func replacementOperationDescription(from: some StringProtocol, to: some StringProtocol, forCollision: Bool = false) -> String {
         if from.isEmpty {
             return "Insert \(to.singleQuoted)"
         }
         if to.isEmpty {
             return "Remove \(from.singleQuoted)"
         }
-        return "Replace \(from.singleQuoted) with \(to.singleQuoted)"
+
+        guard forCollision else {
+            return "Replace \(from.singleQuoted) with \(to.singleQuoted)"
+        }
+
+        if to.hasPrefix("->") || from.hasPrefix("->") {
+            // If either the "to" or "from" descriptions are a return type disambiguation, include the full arrow for both.
+            // Only a ">" prefix doesn't read as an "arrow", and it looks incorrect when only of the descriptions have a "-" prefix.
+            return "Replace \(from.singleQuoted) with \(to.singleQuoted)"
+        }
+        // For other replacement descriptions, drop the leading "-" to focus on the text that's different.
+        return "Replace \(from.dropFirst().singleQuoted) with \(to.dropFirst().singleQuoted)"
     }
 }
 

Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+Find.swift

@@ -8,6 +8,9 @@
  See https://swift.org/CONTRIBUTORS.txt for Swift project authors
 */
 
+import Foundation
+import SymbolKit
+
 extension PathHierarchy {
     /// Attempts to find an element in the path hierarchy for a given path relative to another element.
     ///
@@ -250,6 +253,7 @@ extension PathHierarchy {
 
                     // When there's a collision, use the remaining path components to try and narrow down the possible collisions.
 
+                    // See if the collision can be resolved by looking ahead one level deeper.
                     guard let nextPathComponent = remaining.dropFirst().first else {
                         // This was the last path component so there's nothing to look ahead.
                         //
@@ -382,41 +386,18 @@ extension PathHierarchy {
             )
         }
 
-        // Use a empty disambiguation suffix for the preferred symbol, if there
-        // is one, which will trigger the warning to suggest removing the
-        // suffix entirely.
-        let candidates = disambiguationTree.disambiguatedValues()
-        let favoredSuffix = favoredSuffix(from: candidates)
-        let suffixes = candidates.map { $0.disambiguation.makeSuffix() }
-        let candidatesAndSuffixes = zip(candidates, suffixes).map { (candidate, suffix) in
-            if suffix == favoredSuffix {
-                return (node: candidate.value, disambiguation: "")
-            } else {
-                return (node: candidate.value, disambiguation: suffix)
-            }
-        }
         return Error.unknownDisambiguation(
             partialResult: (
                 node,
                 pathForError(of: rawPathForError, droppingLast: remaining.count)
             ),
             remaining: Array(remaining),
-            candidates: candidatesAndSuffixes
+            candidates: disambiguationTree.disambiguatedValues().map {
+                (node: $0.value, disambiguation: String($0.disambiguation.makeSuffix()))
+            }
         )
     }
 
-    /// Check if exactly one of the given candidate symbols is preferred, because it is not disfavored
-    /// for link resolution and all the other symbols are.
-    /// - Parameters:
-    ///   - from: An array of candidate node and disambiguation tuples.
-    /// - Returns: An optional string set to the disambiguation suffix string, without the hyphen separator e.g. "abc123",
-    ///            or nil if there is no preferred symbol.
-    private func favoredSuffix(from candidates: [(value: PathHierarchy.Node, disambiguation: PathHierarchy.DisambiguationContainer.Disambiguation)]) -> String? {
-        return candidates.singleMatch({
-            !$0.value.specialBehaviors.contains(PathHierarchy.Node.SpecialBehaviors.disfavorInLinkCollision)
-        })?.disambiguation.makeSuffix()
-    }
-
     private func pathForError(
         of rawPath: String,
         droppingLast trailingComponentsToDrop: Int
@@ -473,8 +454,12 @@ extension PathHierarchy.DisambiguationContainer {
     ///  - Exactly one match is found; indicated by a non-nil return value.
     ///  - More than one match is found; indicated by a raised error listing the matches and their missing disambiguation.
     func find(_ disambiguation: PathHierarchy.PathComponent.Disambiguation?) throws -> PathHierarchy.Node? {
-        if storage.count <= 1, disambiguation == nil {
-            return storage.first?.node
+        if disambiguation == nil {
+            if storage.count <= 1 {
+                return storage.first?.node
+            } else if let favoredMatch = storage.singleMatch({ !$0.node.specialBehaviors.contains(.disfavorInLinkCollision) }) {
+                return favoredMatch.node
+            }
         }
 
         switch disambiguation {
@@ -486,13 +471,33 @@ extension PathHierarchy.DisambiguationContainer {
                 let matches = storage.filter({ $0.kind == kind })
                 guard matches.count <= 1 else {
                     // Suggest not only hash disambiguation, but also type signature disambiguation.
-                    throw Error.lookupCollision(Self.disambiguatedValues(for: matches).map { ($0.value, $0.disambiguation.value()) })
+                    throw Error.lookupCollision(Self.disambiguatedValues(for: matches).map { ($0.value, $0.disambiguation.makeSuffix()) })
                 }
                 return matches.first?.node
             case (nil, let hash?):
                 let matches = storage.filter({ $0.hash == hash })
                 guard matches.count <= 1 else {
-                    throw Error.lookupCollision(matches.map { ($0.node, $0.kind!) }) // An element wouldn't match if it didn't have kind disambiguation.
+                    throw Error.lookupCollision(matches.map { ($0.node, "-" + $0.kind!) }) // An element wouldn't match if it didn't have kind disambiguation.
+                }
+                return matches.first?.node
+            case (nil, nil):
+                break
+            }
+        case .typeSignature(let parameterTypes, let returnTypes):
+            let storage = storage.filter { !$0.node.specialBehaviors.contains(.excludeFromAdvancedLinkDisambiguation )}
+            switch (parameterTypes, returnTypes) {
+            case (let parameterTypes?, let returnTypes?):
+                return storage.first(where: { typesMatch(provided: parameterTypes, actual: $0.parameterTypes) && typesMatch(provided: returnTypes, actual: $0.returnTypes) })?.node
+            case (let parameterTypes?, nil):
+                let matches = storage.filter({ typesMatch(provided: parameterTypes, actual: $0.parameterTypes) })
+                guard matches.count <= 1 else {
+                    throw Error.lookupCollision(matches.map { ($0.node, "->" + formattedTypes($0.parameterTypes)!) }) // An element wouldn't match if it didn't have parameter type disambiguation.
+                }
+                return matches.first?.node
+            case (nil, let returnTypes?):
+                let matches = storage.filter({ typesMatch(provided: returnTypes, actual: $0.returnTypes) })
+                guard matches.count <= 1 else {
+                    throw Error.lookupCollision(matches.map { ($0.node, "-" + formattedTypes($0.returnTypes)!) }) // An element wouldn't match if it didn't have return type disambiguation.
                 }
                 return matches.first?.node
             case (nil, nil):
@@ -501,13 +506,23 @@ extension PathHierarchy.DisambiguationContainer {
         case nil:
             break
         }
+
         // Disambiguate by a mix of kinds and USRs
-        throw Error.lookupCollision(self.disambiguatedValues().map { ($0.value, $0.disambiguation.value()) })
+        throw Error.lookupCollision(self.disambiguatedValues().map { ($0.value, $0.disambiguation.makeSuffix()) })
     }
 }
 
 // MARK: Private helper extensions
 
+private func formattedTypes(_ types: [String]?) -> String? {
+    guard let types = types else { return nil }
+    switch types.count {
+    case 0: return "()"
+    case 1: return types[0]
+    default: return "(\(types.joined(separator: ","))"
+    }
+}
+
 // Allow optional substrings to be compared to non-optional strings
 private func == (lhs: (some StringProtocol)?, rhs: some StringProtocol) -> Bool {
      guard let lhs else { return false }
@@ -546,6 +561,11 @@ private extension PathHierarchy.Node {
                 return name == component.name
                     && (kind == nil || kind! == symbol.kind.identifier.identifier)
                     && (hash == nil || hash! == symbol.identifier.precise.stableHashString)
+            case .typeSignature(let parameterTypes, let returnTypes):
+                let functionSignatureTypeNames = PathHierarchy.functionSignatureTypeNames(for: symbol)
+                return name == component.name
+                    && (parameterTypes == nil || typesMatch(provided: parameterTypes!, actual: functionSignatureTypeNames?.parameterTypeNames))
+                    && (returnTypes    == nil || typesMatch(provided: returnTypes!,    actual: functionSignatureTypeNames?.returnTypeNames))
             }
         }
 
@@ -558,3 +578,11 @@ private extension PathHierarchy.Node {
             || keys.contains(String(component.name))
     }
 }
+
+private func typesMatch(provided: [Substring], actual: [String]?) -> Bool {
+    guard let actual = actual, provided.count == actual.count else { return false }
+    for (providedType, actualType) in zip(provided, actual) where providedType != "_" && providedType != actualType {
+        return false
+    }
+    return true
+}