address review feedback

 - create TopicReferenceResolutionError with minimal public interface
 - replace TopicReferenceResolutionResult.failure's errorMessage with TopicReferenceResolutionError
 - add support for non-symbol reference syntax
 - add Solution for PathHierarchy.Error.nonSymbolMatchForSymbolLink
 - add meaningful information to Solution.summary
 - scope Replacements as small as possible (on disambiguation, path segment, or reference delimiters)
 - add note listing all available candidates even if near-miss suggestions are available
 - add Solutions if no near-miss is available, but there are only three candidates or less
 - make offsetWithRange mutating
 - remove XCTAssertElements test helper

Sources/SwiftDocC/Infrastructure/Diagnostics/Diagnostic.swift

@@ -87,17 +87,13 @@ public struct Diagnostic: DescribedError {
 
 public extension Diagnostic {
 
-    /// Returns a copy of the diagnostic but offset using a certain `SourceRange`.
+    /// Offsets the diagnostic using a certain SymbolKit `SourceRange`.
+    ///
     /// Useful when validating a doc comment that needs to be projected in its containing file "space".
-    func offsetedWithRange(_ docRange: SymbolGraph.LineList.SourceRange) -> Diagnostic {
-        guard let diagnosticRange = range else {
-            // No location information in the source diagnostic, might be removed for safety reasons.
-            return self
-        }
+    mutating func offsetWithRange(_ docRange: SymbolGraph.LineList.SourceRange) {
+        // If there is no location information in the source diagnostic, the diagnostic might be removed for safety reasons.
+        range?.offsetWithRange(docRange)
 
-        var result = self
-        result.range = diagnosticRange.offsetedWithRange(docRange)
-        return result
     }
 
     var localizedDescription: String {

Sources/SwiftDocC/Infrastructure/Diagnostics/Problem.swift

@@ -52,22 +52,17 @@ extension Problem {
 }
 
 extension Problem {
-
-    /// Returns a copy of the problem but offset using a certain `SourceRange`.
+    /// Offsets the problem using a certain SymbolKit `SourceRange`.
+    ///
     /// Useful when validating a doc comment that needs to be projected in its containing file "space".
-    func offsetedWithRange(_ docRange: SymbolGraph.LineList.SourceRange) -> Problem {
-        var result = self
-        result.diagnostic = diagnostic.offsetedWithRange(docRange)
+    mutating func offsetWithRange(_ docRange: SymbolGraph.LineList.SourceRange) {
+        diagnostic.offsetWithRange(docRange)
 
-        result.possibleSolutions = possibleSolutions.map {
-            var solution = $0
-            solution.replacements = solution.replacements.map { replacement in
-                replacement.offsetedWithRange(docRange)
+        for i in possibleSolutions.indices {
+            for j in possibleSolutions[i].replacements.indices {
+                possibleSolutions[i].replacements[j].offsetWithRange(docRange)
             }
-            return solution
         }
-
-        return result
     }
 }
 

Sources/SwiftDocC/Infrastructure/Diagnostics/Replacement.swift

@@ -28,22 +28,17 @@ public struct Replacement {
 }
 
 extension Replacement {
-
-    /// Returns a copy of the replacement but offset using a certain `SourceRange`.
+    /// Offsets the replacement using a certain `SourceRange`.
+    ///
     /// Useful when validating a doc comment that needs to be projected in its containing file "space".
-    func offsetedWithRange(_ docRange: SymbolGraph.LineList.SourceRange) -> Replacement {
-        var result = self
-        result.range = range.offsetedWithRange(docRange)
-        return result
+    mutating func offsetWithRange(_ range: SourceRange) {
+        self.range.offsetWithRange(range)
     }
-}
-
-extension SourceRange {
-    /// Returns a copy of the `SourceRange` offset using a certain  SymbolKit `SourceRange`.
-    func offsetedWithRange(_ docRange: SymbolGraph.LineList.SourceRange) -> SourceRange {
-        let start = SourceLocation(line: lowerBound.line + docRange.start.line, column: lowerBound.column + docRange.start.character, source: nil)
-        let end = SourceLocation(line: upperBound.line + docRange.start.line, column: upperBound.column + docRange.start.character, source: nil)
-
-        return start..<end
+
+    /// Offsets the replacement using a certain SymbolKit `SourceRange`.
+    /// 
+    /// Useful when validating a doc comment that needs to be projected in its containing file "space".
+    mutating func offsetWithRange(_ docRange: SymbolGraph.LineList.SourceRange) {
+        self.offsetWithRange(SourceRange(from: docRange))
     }
 }

Sources/SwiftDocC/Infrastructure/DocumentationContext.swift

@@ -611,9 +611,12 @@ public class DocumentationContext: DocumentationContextDataProviderDelegate {
                        let docs = documentationNode.symbol?.docComment {
                         // Offset all problem ranges by the start location of the
                         // source comment in the context of the complete file.
-                        problems = problems.compactMap { problem -> Problem? in
-                            guard let docRange = docs.lines.first?.range else { return nil }
-                            return problem.offsetedWithRange(docRange)
+                        if let docRange = docs.lines.first?.range {
+                            for i in problems.indices {
+                                problems[i].offsetWithRange(docRange)
+                            }
+                        } else {
+                            problems.removeAll()
                         }
                     }
 

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

@@ -123,7 +123,7 @@ public class OutOfProcessReferenceResolver: ExternalReferenceResolver, FallbackR
             do {
                 guard let unresolvedTopicURL = unresolvedReference.topicURL.components.url else {
                     // Return the unresolved reference if the underlying URL is not valid
-                    return .failure(unresolvedReference, errorMessage: "URL \(unresolvedReference.topicURL.absoluteString.singleQuoted) is not valid.")
+                    return .failure(unresolvedReference, TopicReferenceResolutionError("URL \(unresolvedReference.topicURL.absoluteString.singleQuoted) is not valid."))
                 }
                 let metadata = try resolveInformationForTopicURL(unresolvedTopicURL)
                 // Don't do anything with this URL. The external URL will be resolved during conversion to render nodes
@@ -136,7 +136,7 @@ public class OutOfProcessReferenceResolver: ExternalReferenceResolver, FallbackR
                     )
                 )
             } catch let error {
-                return .failure(unresolvedReference, errorMessage: error.localizedDescription)
+                return .failure(unresolvedReference, TopicReferenceResolutionError(error))
             }
         }
     }

Sources/SwiftDocC/Infrastructure/Link Resolution/DocumentationCacheBasedLinkResolver.swift

@@ -129,7 +129,7 @@ final class DocumentationCacheBasedLinkResolver {
         // Ensure we are resolving either relative links or "doc:" scheme links
         guard unresolvedReference.topicURL.url.scheme == nil || ResolvedTopicReference.urlHasResolvedTopicScheme(unresolvedReference.topicURL.url) else {
             // Not resolvable in the topic graph
-            return .failure(unresolvedReference, errorMessage: "Reference URL \(unresolvedReference.description.singleQuoted) doesn't have \"doc:\" scheme.")
+            return .failure(unresolvedReference, TopicReferenceResolutionError("Reference URL \(unresolvedReference.description.singleQuoted) doesn't have \"doc:\" scheme."))
         }
 
         // Fall back on the parent's bundle identifier for relative paths
@@ -267,7 +267,7 @@ final class DocumentationCacheBasedLinkResolver {
                 // Return the successful or failed externally resolved reference.
                 return resolvedExternalReference
             } else if !context.registeredBundles.contains(where: { $0.identifier == bundleID }) {
-                return .failure(unresolvedReference, errorMessage: "No external resolver registered for \(bundleID.singleQuoted).")
+                return .failure(unresolvedReference, TopicReferenceResolutionError("No external resolver registered for \(bundleID.singleQuoted)."))
             }
         }
 
@@ -295,7 +295,7 @@ final class DocumentationCacheBasedLinkResolver {
         // Give up: there is no local or external document for this reference.
 
         // External references which failed to resolve will already have returned a more specific error message.
-        return .failure(unresolvedReference, errorMessage: "No local documentation matches this reference.")
+        return .failure(unresolvedReference, TopicReferenceResolutionError("No local documentation matches this reference."))
     }
 
 

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

@@ -10,6 +10,7 @@
 
 import Foundation
 import SymbolKit
+import Markdown
 
 /// An opaque identifier that uniquely identifies a resolved entry in the path hierarchy,
 ///
@@ -829,63 +830,100 @@ private func availableChildNameIsBefore(_ lhs: String, _ rhs: String) -> Bool {
 }
 
 extension PathHierarchy.Error {
-    /// Formats the error into an error message suitable for presentation
-    func errorMessage(context: DocumentationContext) -> String {
+    /// Generate a ``TopicReferenceResolutionError`` from this error using the given `context` and `originalReference`.
+    ///
+    /// The resulting ``TopicReferenceResolutionError`` is human-readable and provides helpful solutions.
+    ///
+    /// - Parameters:
+    ///     - context: The ``DocumentationContext`` the `originalReference` was resolved in.
+    ///     - originalReference: The raw input string that represents the body of the reference that failed to resolve.
+    ///
+    /// - Note: `Replacement`s produced by this function use `SourceLocation`s relative to the `originalReference`, i.e. the beginning
+    /// of the _body_ of the original reference.
+    func asTopicReferenceResolutionError(context: DocumentationContext, originalReference: String) -> TopicReferenceResolutionError {
         switch self {
-        case .partialResult(let partialResult, let remaining, let available):
-            let nearMisses = NearMiss.bestMatches(for: available, against: remaining)
-            let suggestion: String
-            switch nearMisses.count {
-            case 0:
-                suggestion = "No similar pages. Available children: \(available.joined(separator: ", "))."
-            case 1:
-                suggestion = "Did you mean: \(nearMisses[0])?"
-            default:
-                suggestion = "Did you mean one of: \(nearMisses.joined(separator: ", "))?"
-            }
-            return "Reference at \(partialResult.pathWithoutDisambiguation().singleQuoted) can't resolve \(remaining.singleQuoted). \(suggestion)"
-        case .notFound, .unfindableMatch:
-            return "No local documentation matches this reference."
-
+        case .notFound(availableChildren: let availableChildren):
+            return TopicReferenceResolutionError("No local documentation matches this reference.", note: availabilityNote(category: "top-level elements", candidates: availableChildren))
+        case .unfindableMatch:
+            return TopicReferenceResolutionError("No local documentation matches this reference.")
         case .nonSymbolMatchForSymbolLink:
-            return "Symbol links can only resolve symbols."
+            return TopicReferenceResolutionError("Symbol links can only resolve symbols.", solutions: [
+                Solution(summary: "Use a '<doc:>' style reference.", replacements: [
+                    // the SourceRange points to the opening double-backtick
+                    Replacement(range: SourceLocation(line: 0, column: -2, source: nil)..<SourceLocation(line: 0, column: 0, source: nil), replacement: "<doc:"),
+                    // the SourceRange points to the closing double-backtick
+                    Replacement(range: SourceLocation(line: 0, column: originalReference.count, source: nil)..<SourceLocation(line: 0, column: originalReference.count+2, source: nil), replacement: ">"),
+                ])
+            ])
+        case .partialResult(partialResult: let partialResult, remainingSubpath: let remainingSubpath, availableChildren: let availableChildren):
+            let nearMisses = NearMiss.bestMatches(for: availableChildren, against: remainingSubpath)
 
-        case .lookupCollision(let partialResult, _, let collisions):
-            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: ". "))."
-        }
-    }
-
-    /// Generates replacements for a faulty `originalPath` that fix the first unresolvable path segment
-    /// by adding disambiguation suffixes or provding near-miss alternatives.
-    ///
-    /// The replacement options only fix the first faulty segment in a reference. They do not alter the valid prefix and keep
-    /// the unchecked suffix in place.
-    func replacements(for originalPath: String) -> [String] {
-        switch self {
-        case .partialResult(_, let remaining, let available):
-            var validPrefix = originalPath
-            validPrefix.removeLast(remaining.count)
+            let validPrefix = originalReference.dropLast(remainingSubpath.count)
+
+            let unprocessedSuffix = remainingSubpath.suffix(from: remainingSubpath.firstIndex(of: "/") ?? remainingSubpath.endIndex)
+
+            let replacementRange = SourceLocation(line: 0, column: validPrefix.count, source: nil)..<SourceLocation(line: 0, column: originalReference.count-unprocessedSuffix.count, source: nil)
 
-            let unprocessedSuffix = remaining.suffix(from: remaining.firstIndex(of: "/") ?? remaining.endIndex)
+            // we don't want to provide an endless amount of unrelated fixits, but if there are only three
+            // or less valid options, we still suggest them as fixits
+            let fixitCandidates = nearMisses.isEmpty && availableChildren.count <= 3 ? availableChildren : nearMisses
 
-            // We don't provide all available children as fixits here if we have no near miss. This
-            // is different from the `errorMessage(context:)`, but we shouldn't suggest a fix that in
-            // no way resembles the user's faulty input.
-            return NearMiss.bestMatches(for: available, against: remaining).map { suggestionSuffix in
-                validPrefix + suggestionSuffix + unprocessedSuffix
+            let solutions = fixitCandidates.map { candidate in
+                Solution(summary: "Correct reference to \(validPrefix + candidate + unprocessedSuffix).", replacements: [
+                    Replacement(range: replacementRange, replacement: candidate)
+                ])
             }
-        case .lookupCollision(_, let remaining, let collisions):
-            var validPrefix = originalPath
-            validPrefix.removeLast(remaining.count)
 
-            let unprocessedSuffix = remaining.suffix(from: remaining.firstIndex(of: "/") ?? remaining.endIndex)
 
-            return collisions.map { collision in
-                validPrefix + collision.node.name + "-\(collision.disambiguation)" + unprocessedSuffix
+            var message: String {
+                var suggestion: String {
+                    switch nearMisses.count {
+                    case 0:
+                        return "No similar pages."
+                    case 1:
+                        return "Did you mean \(nearMisses[0])?"
+                    default:
+                        return "Did you mean one of: \(nearMisses.joined(separator: ", "))?"
+                    }
+                }
+
+                return "Reference at \(partialResult.pathWithoutDisambiguation().singleQuoted) can't resolve \(remainingSubpath.singleQuoted). \(suggestion)"
             }
-        case .notFound, .unfindableMatch, .nonSymbolMatchForSymbolLink:
-            return []
+
+            return TopicReferenceResolutionError(message, note: availabilityNote(category: "children", candidates: availableChildren), solutions: solutions)
+
+        case .lookupCollision(partialResult: let partialResult, remainingSubpath: let remainingSubpath, collisions: let collisions):
+            let unprocessedSuffix = remainingSubpath.suffix(from: remainingSubpath.firstIndex(of: "/") ?? remainingSubpath.endIndex)
+
+            let prefixPotentiallyIncludingInvalidDisambiguation = originalReference.dropLast(unprocessedSuffix.count)
+            let lowerBoundOfLastSegment = prefixPotentiallyIncludingInvalidDisambiguation.lastIndex(of: "/")
+                ?? prefixPotentiallyIncludingInvalidDisambiguation.startIndex
+            let lowerBoundOfLastDisambiguation = prefixPotentiallyIncludingInvalidDisambiguation.lastIndex(of: "-")
+                ?? prefixPotentiallyIncludingInvalidDisambiguation.startIndex
+            let lowerBoundOfInvalidDisambiguation = lowerBoundOfLastDisambiguation > lowerBoundOfLastSegment ? lowerBoundOfLastDisambiguation : nil
+            let validPrefix = prefixPotentiallyIncludingInvalidDisambiguation[..<(lowerBoundOfInvalidDisambiguation ?? prefixPotentiallyIncludingInvalidDisambiguation.endIndex)]
+
+            let replacementRange = SourceLocation(line: 0, column: validPrefix.count, source: nil)..<SourceLocation(line: 0, column: originalReference.count-unprocessedSuffix.count, source: nil)
+
+            let solutions = collisions.sorted(by: {
+                $0.node.fullNameOfValue(context: context) + $0.disambiguation
+                    < $1.node.fullNameOfValue(context: context) + $1.disambiguation
+            }).map { collision in
+                Solution(summary: "Insert disambiguation suffix for \(collision.node.fullNameOfValue(context: context).singleQuoted)", replacements: [
+                    Replacement(range: replacementRange, replacement: "-" + collision.disambiguation)
+                ])
+            }
+
+            return TopicReferenceResolutionError("Reference is ambiguous after \(partialResult.pathWithoutDisambiguation().singleQuoted).", solutions: solutions)
+        }
+    }
+
+    private func availabilityNote(category: String, candidates: [String]) -> String {
+        switch candidates.count {
+        case 0:
+            return "No \(category) available."
+        default:
+            return "Available \(category): \(candidates.joined(separator: ", "))."
         }
     }
 }

Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchyBasedLinkResolver.swift

@@ -207,7 +207,7 @@ final class PathHierarchyBasedLinkResolver {
                 // Return the successful or failed externally resolved reference.
                 return resolvedExternalReference
             } else if !context.registeredBundles.contains(where: { $0.identifier == bundleID }) {
-                return .failure(unresolvedReference, errorMessage: "No external resolver registered for \(bundleID.singleQuoted).")
+                return .failure(unresolvedReference, TopicReferenceResolutionError("No external resolver registered for \(bundleID.singleQuoted)."))
             }
         }
 
@@ -227,7 +227,7 @@ final class PathHierarchyBasedLinkResolver {
                     originalReferenceString += "#" + fragment
                 }
 
-                return .failure(unresolvedReference.with(canidates: error.replacements(for: originalReferenceString)), errorMessage: error.errorMessage(context: context))
+                return .failure(unresolvedReference, error.asTopicReferenceResolutionError(context: context, originalReference: originalReferenceString))
             }
         } catch {
             fatalError("Only SymbolPathTree.Error errors are raised from the symbol link resolution code above.")

Sources/SwiftDocC/Model/DocumentationNode.swift

@@ -429,7 +429,7 @@ public struct DocumentationNode {
                     var problem = Problem(diagnostic: diagnostic, possibleSolutions: [])
 
                     if let offset = docComment.lines.first?.range {
-                        problem = problem.offsetedWithRange(offset)
+                        problem.offsetWithRange(offset)
                     }
 
                     engine.emit(problem)