More symbol info about collisions in error message when a link is ambiguous (#398)

* Use full declaration in ambiguous link error message

rdar://100436859

* Remove comment about a resolved issue

* Avoid percent encoding in the description of unresolved references

* Refine phrasing of symbol link collision error message

* Small style changes to map closure argument

Author: d-ronnqvist

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.

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
     }
 }
 

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)

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." }))