Repository: apple/swift-docc (swift)

Issue:
Show declarations when suggesting what disambiguation to add to symbol links
### Description

Currently the error messages from the path hierarchy based link resolver for when a path is ambiguous list the possible collisions and their names. However, the collision is based on the name, so all suggestions show the same name. 

For example, in the "MixedLanguageFrameworkWithLanguageRefinements" test content, the path "CollisionsWithDifferentFunctionArguments/something(argument:)" will result in an error message like:

```
Reference is ambiguous after '/MixedFramework/CollisionsWithDifferentFunctionArguments': Add '1cyvp' to refer to 'something(argument:)'. Add '2vke2' to refer to 'something(argument:)'.
```

(Note that the two suggestions have the same name)

To make it possible to distinguish the collisions and know which disambiguation to add we should display the declarations instead of just the name.

rdar://100436859

Pull request:
<!--
If you're opening a PR to cherry-pick a change for a release branch, use this template instead:
https://github.com/apple/swift-docc/blob/main/.github/PULL_REQUEST_TEMPLATE/CHERRY_PICK.md
-->

Bug/issue #, if applicable: https://github.com/apple/swift-docc/issues/397 rdar://100436859

## Summary

This shows the full symbol declarations in the error message for a symbol link that's ambiguous to help developers distinguish between the colliding symbols.

This also fixes a minor issue where the descriptions of unresolved references contained percent encoding, resulting in hard to read diagnostics. 

These changes together refines this error message
```
Topic reference 'emit(_%3A)' couldn't be resolved. Reference is ambiguous after '/SwiftDocC/DiagnosticEngine': Add '1xqdb' to refer to 'emit(_:)'. Add '5rcke' to refer to 'emit(_:)'. 
```
into this error message
```
Topic reference 'emit(_:)' couldn't be resolved. Reference is ambiguous after '/SwiftDocC/DiagnosticEngine': Append '-1xqdb' to refer to 'func emit(_ problem: Problem)'. Append '-5rcke' to refer to 'func emit(_ problems: [Problem])'. 
```

## Dependencies

n/a

## Testing

With `DOCC_USE_HIERARCHY_BASED_LINK_RESOLVER=YES` write an ambiguous link, for example by adding this code
```swift 
public enum CollisionsWithDifferentFunctionArguments {
    public func something(argument: Int) -> Int { 0 }
    public func something(argument: String) -> Int { 0 }
}
```
and linking to `CollisionsWithDifferentFunctionArguments/something(argument:)`

In the resulting error message it should be possible to distinguish the collisions. 

## Checklist

Make sure you check off the following items. If they cannot be completed, provide a reason.

- [x] Added tests
- [x] Ran the `./bin/test` script and it succeeded
- [x] ~Updated documentation if necessary~

Patch:
diff --git a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy.swift b/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy.swift
index 976f1e0557..656b7e9a17 100644
--- a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy.swift
+++ b/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy.swift
@@ -838,7 +838,7 @@ extension PathHierarchy.Error {
             return "Symbol links can only resolve symbols."
             
         case .lookupCollision(let partialResult, let collisions):
-            let collisionDescription = collisions.map { "Add \($0.disambiguation.singleQuoted) to refer to \($0.node.fullNameOfValue(context: context).singleQuoted)"}.sorted()
+            let collisionDescription = collisions.map { "Append '-\($0.disambiguation)' to refer to \($0.node.fullNameOfValue(context: context).singleQuoted)" }.sorted()
             return "Reference is ambiguous after \(partialResult.pathWithoutDisambiguation().singleQuoted): \(collisionDescription.joined(separator: ". "))."
         }
     }
@@ -864,6 +864,9 @@ private extension PathHierarchy.Node {
     func fullNameOfValue(context: DocumentationContext) -> String {
         guard let identifier = identifier else { return name }
         if let symbol = symbol {
+            if let fragments = symbol[mixin: SymbolGraph.Symbol.DeclarationFragments.self]?.declarationFragments {
+                return fragments.map(\.spelling).joined().split(whereSeparator: { $0.isWhitespace || $0.isNewline }).joined(separator: " ")
+            }
             return context.symbolIndex[symbol.identifier.precise]!.name.description
         }
         // This only gets called for PathHierarchy error messages, so hierarchyBasedLinkResolver is never nil.
diff --git a/Sources/SwiftDocC/Model/Identifier.swift b/Sources/SwiftDocC/Model/Identifier.swift
index d4547e1b12..3180e15aba 100644
--- a/Sources/SwiftDocC/Model/Identifier.swift
+++ b/Sources/SwiftDocC/Model/Identifier.swift
@@ -518,7 +518,15 @@ public struct UnresolvedTopicReference: Hashable, CustomStringConvertible {
     }
     
     public var description: String {
-        return topicURL.components.string!
+        var result = topicURL.components.string!
+        // Replace that path and fragment parts of the description with the unescaped path and fragment values.
+        if let rangeOfFragment = topicURL.components.rangeOfFragment, let fragment = topicURL.components.fragment {
+            result.replaceSubrange(rangeOfFragment, with: fragment)
+        }
+        if let rangeOfPath = topicURL.components.rangeOfPath {
+            result.replaceSubrange(rangeOfPath, with: topicURL.components.path)
+        }
+        return result
     }
 }
 


Test patch:
diff --git a/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift b/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift
index 1cdc97aacd..01d3caee5a 100644
--- a/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift
+++ b/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift
@@ -75,8 +75,7 @@ class PathHierarchyTests: XCTestCase {
         try assertFindsPath("/MixedFramework/MyObjectiveCCompatibleProtocol/myProtocolMethod", in: tree, asSymbolID: "c:@M@MixedFramework@objc(pl)MyObjectiveCCompatibleProtocol(im)myProtocolMethod")
         try assertFindsPath("/MixedFramework/MyObjectiveCCompatibleProtocol/myProtocolMethod()", in: tree, asSymbolID: "c:@M@MixedFramework@objc(pl)MyObjectiveCCompatibleProtocol(im)myProtocolMethod")
         try assertFindsPath("/MixedFramework/MyObjectiveCCompatibleProtocol/myProtocolProperty", in: tree, asSymbolID: "c:@M@MixedFramework@objc(pl)MyObjectiveCCompatibleProtocol(py)myProtocolProperty")
-        // Objective-C class properties have a "property" kind instead of a "type.property" kind (rdar://92927788)
-        try assertFindsPath("/MixedFramework/MyObjectiveCCompatibleProtocol/myProtocolTypeProperty-type.property", in: tree, asSymbolID: "c:@M@MixedFramework@objc(pl)MyObjectiveCCompatibleProtocol(cpy)myProtocolTypeProperty")
+        try assertFindsPath("/MixedFramework/MyObjectiveCCompatibleProtocol/myProtocolTypeProperty", in: tree, asSymbolID: "c:@M@MixedFramework@objc(pl)MyObjectiveCCompatibleProtocol(cpy)myProtocolTypeProperty")
         try assertFindsPath("/MixedFramework/MyObjectiveCCompatibleProtocol/myPropertyOptionalMethod", in: tree, asSymbolID: "c:@M@MixedFramework@objc(pl)MyObjectiveCCompatibleProtocol(im)myPropertyOptionalMethod")
         try assertFindsPath("/MixedFramework/MyObjectiveCCompatibleProtocol/myPropertyOptionalMethod()", in: tree, asSymbolID: "c:@M@MixedFramework@objc(pl)MyObjectiveCCompatibleProtocol(im)myPropertyOptionalMethod")
         
@@ -295,6 +294,11 @@ class PathHierarchyTests: XCTestCase {
             (symbolID: "s:14MixedFramework28CollisionsWithDifferentKindsO9somethingyA2CmF", disambiguation: "enum.case"),
             (symbolID: "s:14MixedFramework28CollisionsWithDifferentKindsO9somethingSSvp", disambiguation: "property"),
         ])
+        try assertPathRaisesErrorMessage("/MixedFramework/CollisionsWithDifferentKinds/something", in: tree, context: context, expectedErrorMessage: """
+        Reference is ambiguous after '/MixedFramework/CollisionsWithDifferentKinds': \
+        Append '-enum.case' to refer to 'case something'. \
+        Append '-property' to refer to 'var something: String { get }'.
+        """)
         
         // public final class CollisionsWithEscapedKeywords {
         //     public subscript() -> Int { 0 }
@@ -310,12 +314,24 @@ class PathHierarchyTests: XCTestCase {
             (symbolID: "s:14MixedFramework29CollisionsWithEscapedKeywordsC4inityyF", disambiguation: "method"),
             (symbolID: "s:14MixedFramework29CollisionsWithEscapedKeywordsC4inityyFZ", disambiguation: "type.method"),
         ])
+        try assertPathRaisesErrorMessage("/MixedFramework/CollisionsWithEscapedKeywords/init()", in: tree, context: context, expectedErrorMessage: """
+        Reference is ambiguous after '/MixedFramework/CollisionsWithEscapedKeywords': \
+        Append '-init' to refer to 'init()'. \
+        Append '-method' to refer to 'func `init`()'. \
+        Append '-type.method' to refer to 'static func `init`()'.
+        """)
         
         try assertPathCollision("/MixedFramework/CollisionsWithEscapedKeywords/subscript()", in: tree, collisions: [
             (symbolID: "s:14MixedFramework29CollisionsWithEscapedKeywordsC9subscriptyyF", disambiguation: "method"),
             (symbolID: "s:14MixedFramework29CollisionsWithEscapedKeywordsCSiycip", disambiguation: "subscript"),
             (symbolID: "s:14MixedFramework29CollisionsWithEscapedKeywordsC9subscriptyyFZ", disambiguation: "type.method"),
         ])
+        try assertPathRaisesErrorMessage("/MixedFramework/CollisionsWithEscapedKeywords/subscript()", in: tree, context: context, expectedErrorMessage: """
+        Reference is ambiguous after '/MixedFramework/CollisionsWithEscapedKeywords': \
+        Append '-method' to refer to 'func `subscript`()'. \
+        Append '-subscript' to refer to 'subscript() -> Int { get }'. \
+        Append '-type.method' to refer to 'static func `subscript`()'.
+        """)
         
         // public enum CollisionsWithDifferentFunctionArguments {
         //     public func something(argument: Int) -> Int { 0 }
@@ -325,6 +341,11 @@ class PathHierarchyTests: XCTestCase {
             (symbolID: "s:14MixedFramework40CollisionsWithDifferentFunctionArgumentsO9something8argumentS2i_tF", disambiguation: "1cyvp"),
             (symbolID: "s:14MixedFramework40CollisionsWithDifferentFunctionArgumentsO9something8argumentSiSS_tF", disambiguation: "2vke2"),
         ])
+        try assertPathRaisesErrorMessage("/MixedFramework/CollisionsWithDifferentFunctionArguments/something(argument:)", in: tree, context: context, expectedErrorMessage: """
+        Reference is ambiguous after '/MixedFramework/CollisionsWithDifferentFunctionArguments': \
+        Append '-1cyvp' to refer to 'func something(argument: Int) -> Int'. \
+        Append '-2vke2' to refer to 'func something(argument: String) -> Int'.
+        """)
         
         // public enum CollisionsWithDifferentSubscriptArguments {
         //     public subscript(something: Int) -> Int { 0 }
@@ -334,6 +355,11 @@ class PathHierarchyTests: XCTestCase {
             (symbolID: "s:14MixedFramework41CollisionsWithDifferentSubscriptArgumentsOyS2icip", disambiguation: "4fd0l"),
             (symbolID: "s:14MixedFramework41CollisionsWithDifferentSubscriptArgumentsOySiSScip", disambiguation: "757cj"),
         ])
+        try assertPathRaisesErrorMessage("/MixedFramework/CollisionsWithDifferentSubscriptArguments/subscript(_:)", in: tree, context: context, expectedErrorMessage: """
+        Reference is ambiguous after '/MixedFramework/CollisionsWithDifferentSubscriptArguments': \
+        Append '-4fd0l' to refer to 'subscript(something: Int) -> Int { get }'. \
+        Append '-757cj' to refer to 'subscript(somethingElse: String) -> Int { get }'.
+        """)
         
         // typedef NS_OPTIONS(NSInteger, MyObjectiveCOption) {
         //     MyObjectiveCOptionNone                                      = 0,
@@ -839,6 +865,11 @@ class PathHierarchyTests: XCTestCase {
             ("s:5MyKit0A5MyProtocol0Afunc()DefaultImp", "2dxqn"),
             ("s:5MyKit0A5MyProtocol0Afunc()", "6ijsi"),
         ])
+        try assertPathRaisesErrorMessage("/SideKit/SideProtocol/func()", in: tree, context: context, expectedErrorMessage: """
+        Reference is ambiguous after '/SideKit/SideProtocol': \
+        Append '-2dxqn' to refer to 'func1()'. \
+        Append '-6ijsi' to refer to 'func1()'.
+        """) // This test data have the same declaration for both symbols.
         
         try assertFindsPath("/FillIntroduced/iOSOnlyDeprecated()", in: tree, asSymbolID: "s:14FillIntroduced17iOSOnlyDeprecatedyyF")
         try assertFindsPath("/FillIntroduced/macCatalystOnlyIntroduced()", in: tree, asSymbolID: "s:14FillIntroduced015macCatalystOnlyB0yyF")
@@ -879,6 +910,12 @@ class PathHierarchyTests: XCTestCase {
             ("c:@E@Foo", "struct"),
             ("c:MixedLanguageFramework.h@T@Foo", "typealias"),
         ])
+        try assertPathRaisesErrorMessage("MixedLanguageFramework/Foo", in: tree, context: context, expectedErrorMessage: """
+        Reference is ambiguous after '/MixedLanguageFramework': \
+        Append '-enum' to refer to 'typedef enum Foo : NSString { ... } Foo;'. \
+        Append '-struct' to refer to 'struct Foo'. \
+        Append '-typealias' to refer to 'typedef enum Foo : NSString { ... } Foo;'.
+        """) // The 'enum' and 'typealias' symbols have multi-line declarations that are presented on a single line
         
         try assertFindsPath("MixedLanguageFramework/Foo/first", in: tree, asSymbolID: "c:@E@Foo@first")
         
@@ -1233,6 +1270,14 @@ class PathHierarchyTests: XCTestCase {
         }
     }
     
+    private func assertPathRaisesErrorMessage(_ path: String, in tree: PathHierarchy, context: DocumentationContext, expectedErrorMessage: String, file: StaticString = #file, line: UInt = #line) throws {
+        XCTAssertThrowsError(try tree.findSymbol(path: path), "Finding path \(path) didn't raise an error.",file: file,line: line) { untypedError in
+            let error = untypedError as! PathHierarchy.Error
+            let errorMessage = error.errorMessage(context: context)
+            XCTAssertEqual(errorMessage, expectedErrorMessage, file: file, line: line)
+        }
+    }
+    
     private func assertParsedPathComponents(_ path: String, _ expected: [(String, String?, String?)], file: StaticString = #file, line: UInt = #line) {
         let (actual, _) = PathHierarchy.parse(path: path)
         XCTAssertEqual(actual.count, expected.count, file: file, line: line)
diff --git a/Tests/SwiftDocCTests/Semantics/SymbolTests.swift b/Tests/SwiftDocCTests/Semantics/SymbolTests.swift
index 1feabe8cbe..0412dbf641 100644
--- a/Tests/SwiftDocCTests/Semantics/SymbolTests.swift
+++ b/Tests/SwiftDocCTests/Semantics/SymbolTests.swift
@@ -525,7 +525,7 @@ class SymbolTests: XCTestCase {
 
             A cool API to call.
 
-            This overview has an ``UnresolvableSymbolLinkInMyClassOverview``.
+            This overview has an ``UnresolvableSymbolLinkInMyClassOverview<>(_:))``.
 
             - Parameters:
               - name: A parameter
@@ -554,14 +554,14 @@ class SymbolTests: XCTestCase {
         
         XCTAssertTrue(unresolvedTopicProblems.contains(where: { $0.diagnostic.localizedSummary == "Topic reference 'doc://com.test.external/ExternalPage' couldn't be resolved. No external resolver registered for 'com.test.external'." }))
         if LinkResolutionMigrationConfiguration.shouldUseHierarchyBasedLinkResolver {
-            XCTAssertTrue(unresolvedTopicProblems.contains(where: { $0.diagnostic.localizedSummary == "Topic reference 'UnresolvableSymbolLinkInMyClassOverview' couldn't be resolved. Reference at '/MyKit/MyClass' can't resolve 'UnresolvableSymbolLinkInMyClassOverview'. Available children: init(), myFunction()." }))
+            XCTAssertTrue(unresolvedTopicProblems.contains(where: { $0.diagnostic.localizedSummary == "Topic reference 'UnresolvableSymbolLinkInMyClassOverview<>(_:))' couldn't be resolved. Reference at '/MyKit/MyClass' can't resolve 'UnresolvableSymbolLinkInMyClassOverview<>(_:))'. Available children: init(), myFunction()." }))
             XCTAssertTrue(unresolvedTopicProblems.contains(where: { $0.diagnostic.localizedSummary == "Topic reference 'UnresolvableClassInMyClassTopicCuration' couldn't be resolved. Reference at '/MyKit/MyClass' can't resolve 'UnresolvableClassInMyClassTopicCuration'. Available children: init(), myFunction()." }))
 
             XCTAssertTrue(unresolvedTopicProblems.contains(where: { $0.diagnostic.localizedSummary == "Topic reference 'MyClass/unresolvablePropertyInMyClassTopicCuration' couldn't be resolved. Reference at '/MyKit/MyClass' can't resolve 'unresolvablePropertyInMyClassTopicCuration'. Available children: init(), myFunction()." }))
-            XCTAssertTrue(unresolvedTopicProblems.contains(where: { $0.diagnostic.localizedSummary == "Topic reference 'init()' couldn't be resolved. Reference is ambiguous after '/MyKit/MyClass': Add '33vaw' to refer to 'init()'. Add '3743d' to refer to 'init()'." }))
-            XCTAssertTrue(unresolvedTopicProblems.contains(where: { $0.diagnostic.localizedSummary == "Topic reference 'MyClass/init()-swift.init' couldn't be resolved. Reference is ambiguous after '/MyKit/MyClass': Add '33vaw' to refer to 'init()'. Add '3743d' to refer to 'init()'." }))
+            XCTAssertTrue(unresolvedTopicProblems.contains(where: { $0.diagnostic.localizedSummary == "Topic reference 'init()' couldn't be resolved. Reference is ambiguous after '/MyKit/MyClass': Append '-33vaw' to refer to 'init()'. Append '-3743d' to refer to 'init()'." }))
+            XCTAssertTrue(unresolvedTopicProblems.contains(where: { $0.diagnostic.localizedSummary == "Topic reference 'MyClass/init()-swift.init' couldn't be resolved. Reference is ambiguous after '/MyKit/MyClass': Append '-33vaw' to refer to 'init()'. Append '-3743d' to refer to 'init()'." }))
         } else {
-            XCTAssertTrue(unresolvedTopicProblems.contains(where: { $0.diagnostic.localizedSummary == "Topic reference 'UnresolvableSymbolLinkInMyClassOverview' couldn't be resolved. No local documentation matches this reference." }))
+            XCTAssertTrue(unresolvedTopicProblems.contains(where: { $0.diagnostic.localizedSummary == "Topic reference 'UnresolvableSymbolLinkInMyClassOverview<>(_:))' couldn't be resolved. No local documentation matches this reference." }))
             XCTAssertTrue(unresolvedTopicProblems.contains(where: { $0.diagnostic.localizedSummary == "Topic reference 'UnresolvableClassInMyClassTopicCuration' couldn't be resolved. No local documentation matches this reference." }))
             XCTAssertTrue(unresolvedTopicProblems.contains(where: { $0.diagnostic.localizedSummary == "Topic reference 'MyClass/unresolvablePropertyInMyClassTopicCuration' couldn't be resolved. No local documentation matches this reference." }))
             XCTAssertTrue(unresolvedTopicProblems.contains(where: { $0.diagnostic.localizedSummary == "Topic reference 'init()' couldn't be resolved. No local documentation matches this reference." }))