Skip to content

Commit 587c879

Browse files
Add a new link resolver that finds documentation in a hierarchy of path components (#337)
* Add a new link resolver that searches a hierarchy of path components This resolver is off-by-default but can be enabled via configuration. Also, add some high level documentation of how link resolution and link disambiguation works. * Conditionally skip PathHierarchy tests * Handle collisions when all matches are the same symbol * Add opt-in ability to report mismatches between the two link resolvers. * Improve disambiguation of symbols that only collide with their counterpart in another language Also, improve error message when no part of the unresolved symbol link can be found. * Update link examples in DocC documentation about documentation links. Also, fix broken symbol link in DocC documentation. * Update gathering and reporting of link resolver implementation mismatches * Update for snippet changes and the new Snippets.docc test data * Fix minor pluralization in documentation article Co-authored-by: Franklin Schrans <[email protected]> * Run tests twice (once with each link resolver implementation) * Skip slow performance test that isn't compared to previous results Co-authored-by: Franklin Schrans <[email protected]>
1 parent d56e5a9 commit 587c879

File tree

30 files changed

+22189
-192
lines changed

30 files changed

+22189
-192
lines changed

Sources/SwiftDocC/DocumentationService/Convert/Symbol Link Resolution/DocCSymbolRepresentable.swift

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -154,12 +154,19 @@ public extension Collection where Element: DocCSymbolRepresentable {
154154
} else {
155155
// Disambiguate by kind
156156
return map { currentSymbol in
157-
(
158-
shouldAddIdHash: filter {
159-
$0.kindIdentifier == currentSymbol.kindIdentifier
160-
}.count > 1,
161-
shouldAddKind: true
162-
)
157+
let kindCount = filter { $0.kindIdentifier == currentSymbol.kindIdentifier }.count
158+
159+
if LinkResolutionMigrationConfiguration.shouldUseHierarchyBasedLinkResolver {
160+
return (
161+
shouldAddIdHash: kindCount > 1,
162+
shouldAddKind: kindCount == 1
163+
)
164+
} else {
165+
return (
166+
shouldAddIdHash: kindCount > 1,
167+
shouldAddKind: true
168+
)
169+
}
163170
}
164171
}
165172
}

Sources/SwiftDocC/Infrastructure/DocumentationContext.swift

Lines changed: 233 additions & 65 deletions
Large diffs are not rendered by default.

Sources/SwiftDocC/Infrastructure/DocumentationConverter.swift

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ public struct DocumentationConverter: DocumentationConverterProtocol {
103103
/// - currentPlatforms: The current version and beta information for platforms that may be encountered while processing symbol graph files.
104104
/// that may be encountered while processing symbol graph files.
105105
/// - workspace: A provided documentation workspace. Creates a new empty workspace if value is `nil`.
106-
/// - context: A provided documentation context. Creates a new empty context in the workspace if value is `nil`.
106+
/// - context: A provided documentation context.
107107
/// - dataProvider: A data provider to use when registering bundles.
108108
/// - Parameter fileManager: A file persistence manager
109109
/// - Parameter externalIDsToConvert: The external IDs of the documentation nodes to convert.
@@ -377,6 +377,8 @@ public struct DocumentationConverter: DocumentationConverterProtocol {
377377
// Log the peak memory.
378378
benchmark(add: Benchmark.PeakMemory())
379379

380+
context.linkResolutionMismatches.reportGatheredMismatchesIfEnabled()
381+
380382
return (analysisProblems: context.problems, conversionProblems: conversionProblems)
381383
}
382384

Sources/SwiftDocC/Infrastructure/DocumentationCurator.swift

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,9 @@ struct DocumentationCurator {
4242
return cached
4343
}
4444

45-
let unresolved = UnresolvedTopicReference(topicURL: ValidatedURL(symbolPath: destination))
46-
let maybeResolved = context.resolve(.unresolved(unresolved), in: resolved, fromSymbolLink: true)
47-
48-
if case let .success(resolved) = maybeResolved {
45+
// The symbol link may be written with a scheme and bundle identifier.
46+
let url = ValidatedURL(parsingExact: destination)?.requiring(scheme: ResolvedTopicReference.urlScheme) ?? ValidatedURL(symbolPath: destination)
47+
if case let .success(resolved) = context.resolve(.unresolved(.init(topicURL: url)), in: resolved, fromSymbolLink: true) {
4948
return resolved
5049
}
5150
return nil
@@ -116,6 +115,11 @@ struct DocumentationCurator {
116115
context.topicGraph.addNode(curatedNode)
117116

118117
// Move the article from the article cache to the documentation
118+
119+
if let hierarchyBasedLinkResolver = context.hierarchyBasedLinkResolver {
120+
hierarchyBasedLinkResolver.addArticle(filename: articleFilename, reference: reference, anchorSections: documentationNode.anchorSections)
121+
}
122+
119123
context.documentationCache[reference] = documentationNode
120124
for anchor in documentationNode.anchorSections {
121125
context.nodeAnchorSections[anchor.reference] = anchor
Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
1+
/*
2+
This source file is part of the Swift.org open source project
3+
4+
Copyright (c) 2022 Apple Inc. and the Swift project authors
5+
Licensed under Apache License v2.0 with Runtime Library Exception
6+
7+
See https://swift.org/LICENSE.txt for license information
8+
See https://swift.org/CONTRIBUTORS.txt for Swift project authors
9+
*/
10+
11+
import Foundation
12+
13+
/// A scope of configurations for how the documentation context should resolve links while migrating from one implementation to another.
14+
///
15+
/// If any reporting is enabled, the documentation context will setup both link resolver to compare them.
16+
///
17+
///> Note: This is a temporary configuration that will go away along with the ``DocumentationCacheBasedLinkResolver`` at some point in the future.
18+
enum LinkResolutionMigrationConfiguration {
19+
20+
// MARK: Configuration
21+
22+
/// Whether or not the context should the a ``PathHierarchyBasedLinkResolver`` to resolve links.
23+
static var shouldUseHierarchyBasedLinkResolver: Bool = {
24+
return UserDefaults.standard.bool(forKey: "DocCUseHierarchyBasedLinkResolver")
25+
|| ProcessInfo.processInfo.environment["DOCC_USE_HIERARCHY_BASED_LINK_RESOLVER"] == "YES"
26+
}()
27+
28+
/// Whether or not the context should report differences between the disambiguated paths created by ``PathHierarchyBasedLinkResolver`` and ``DocumentationCacheBasedLinkResolver``.
29+
///
30+
/// What mismatches can be reported depend on the value of ``shouldUseHierarchyBasedLinkResolver``:
31+
/// - When the cache based resolved is used to resolve links both mismatched symbol paths and mismatched link resolution reports will be reported.
32+
/// - When the path hierarchy based resolved is used to resolve links only mismatched symbol paths will be reported.
33+
static var shouldReportLinkResolutionMismatches: Bool = {
34+
return UserDefaults.standard.bool(forKey: "DocCReportLinkResolutionMismatches")
35+
|| ProcessInfo.processInfo.environment["DOCC_REPORT_LINK_RESOLUTION_MISMATCHES"] == "YES"
36+
}()
37+
38+
// MARK: Derived conditions
39+
40+
/// Whether or not the context should set up a ``PathHierarchyBasedLinkResolver``.
41+
///
42+
/// > Node: Check ``shouldUseHierarchyBasedLinkResolver`` to determine which implementation to use to resolve links.
43+
static var shouldSetUpHierarchyBasedLinkResolver: Bool {
44+
return shouldUseHierarchyBasedLinkResolver || shouldReportLinkResolutionMismatches
45+
}
46+
47+
/// Whether or not to report mismatches in link resolution results between the two implementations.
48+
static var shouldReportLinkResolutionResultMismatches: Bool {
49+
return shouldReportLinkResolutionMismatches && !shouldUseHierarchyBasedLinkResolver
50+
}
51+
52+
/// Whether or not to report mismatches in symbol path disambiguation between the two implementations.
53+
static var shouldReportLinkResolutionPathMismatches: Bool {
54+
return shouldReportLinkResolutionMismatches
55+
}
56+
}
57+
58+
// MARK: Gathering mismatches
59+
60+
/// A type that gathers differences between the two link resolution implementations.
61+
///
62+
/// > Note: This is a temporary report that will go away along with the ``DocumentationCacheBasedLinkResolver`` at some point in the future.
63+
final class LinkResolutionMismatches {
64+
/// Gathered resolved reference paths that have different disambiguation in the two implementations.
65+
var pathsWithMismatchedDisambiguation: [String: String] = [:]
66+
67+
/// Gathered resolved reference paths that are missing from the path hierarchy-based implementation.
68+
var missingPathsInHierarchyBasedLinkResolver: [String] = []
69+
/// Gathered resolved reference paths that are missing from the documentation cache-based implementation.
70+
var missingPathsInCacheBasedLinkResolver: [String] = []
71+
72+
/// Information about the inputs for a link that resolved in one implementation but not the other.
73+
struct FailedLinkInfo: Hashable {
74+
/// The path, and optional fragment, of the unresolved reference.
75+
let path: String
76+
/// The path, and optional fragment, of the parent reference that the link was resolved relative to.
77+
let parent: String
78+
/// Whether or not the link was resolved as a symbol link.
79+
let asSymbolLink: Bool
80+
}
81+
82+
/// Links that resolved in the cache-based implementation but not the path hierarchy-based implementation
83+
var mismatchedLinksThatHierarchyBasedLinkResolverFailedToResolve = Set<FailedLinkInfo>()
84+
85+
/// Links that resolved in the path hierarchy-based implementation but not the cache-based implementation.
86+
var mismatchedLinksThatCacheBasedLinkResolverFailedToResolve = Set<FailedLinkInfo>()
87+
}
88+
89+
// MARK: Reporting mismatches
90+
91+
extension LinkResolutionMismatches {
92+
/// Prints the gathered mismatches
93+
///
94+
/// > Note: If ``LinkResolutionMigrationConfiguration/shouldReportLinkResolutionPathMismatches`` is ``false`` this won't print anything.
95+
func reportGatheredMismatchesIfEnabled() {
96+
guard LinkResolutionMigrationConfiguration.shouldReportLinkResolutionPathMismatches else { return }
97+
let prefix = "[HierarchyBasedLinkResolutionDiff]"
98+
99+
if pathsWithMismatchedDisambiguation.isEmpty {
100+
print("\(prefix) All symbol paths have the same disambiguation suffixes in both link resolver implementations.")
101+
} else {
102+
print("\(prefix) The following symbol paths have the different disambiguation across the two link resolver implementations:")
103+
let columnWidth = max(40, (pathsWithMismatchedDisambiguation.keys.map { $0.count } + ["path hierarchy implementation".count]).max()!)
104+
print("\("Path hierarchy implementation".padding(toLength: columnWidth, withPad: " ", startingAt: 0)) | Documentation cache implementation")
105+
print("\(String(repeating: "-", count: columnWidth))-+-\(String(repeating: "-", count: columnWidth))")
106+
for (hierarchyBasedPath, mainCacheBasedPath) in pathsWithMismatchedDisambiguation.sorted(by: \.key) {
107+
print("\(hierarchyBasedPath.padding(toLength: columnWidth, withPad: " ", startingAt: 0)) | \(mainCacheBasedPath)")
108+
}
109+
}
110+
111+
if !missingPathsInHierarchyBasedLinkResolver.isEmpty {
112+
let missingPaths = missingPathsInHierarchyBasedLinkResolver.sorted()
113+
print("\(prefix) The following symbol paths exist in the cache-based link resolver but is missing in the path hierarchy-based link resolver:\n\(missingPaths.joined(separator: "\n"))")
114+
}
115+
if !missingPathsInCacheBasedLinkResolver.isEmpty {
116+
let missingPaths = missingPathsInCacheBasedLinkResolver.sorted()
117+
print("\(prefix) The following symbol paths exist in the path hierarchy-based link resolver but is missing in the cache-based link resolver:\n\(missingPaths.joined(separator: "\n"))")
118+
}
119+
120+
guard !LinkResolutionMigrationConfiguration.shouldUseHierarchyBasedLinkResolver else {
121+
// Results can only be reported when the documentation cache based implementation is used to resolve links
122+
return
123+
}
124+
125+
let mismatchedFailedCacheResults = mismatchedLinksThatCacheBasedLinkResolverFailedToResolve
126+
let mismatchedFailedHierarchyResults = mismatchedLinksThatHierarchyBasedLinkResolverFailedToResolve
127+
128+
if mismatchedFailedCacheResults.isEmpty && mismatchedFailedHierarchyResults.isEmpty {
129+
print("\(prefix) Both link resolver implementations succeeded and failed to resolve the same links.")
130+
} else {
131+
if !mismatchedFailedCacheResults.isEmpty {
132+
print("\(prefix) The following links failed to resolve in the documentation cache implementation but succeeded in the path hierarchy implementation:")
133+
134+
let firstColumnWidth = mismatchedFailedCacheResults.map { $0.path.count }.max()! + 2 // 2 extra for the quotes
135+
for result in mismatchedFailedCacheResults {
136+
print("\(result.path.singleQuoted.padding(toLength: firstColumnWidth, withPad: " ", startingAt: 0)) relative to \(result.parent.singleQuoted) \(result.asSymbolLink ? "(symbol link)" : "")")
137+
}
138+
}
139+
140+
if !mismatchedFailedHierarchyResults.isEmpty {
141+
print("\(prefix) The following links failed to resolve in the path hierarchy implementation but succeeded in the documentation cache implementation:")
142+
143+
let firstColumnWidth = mismatchedFailedHierarchyResults.map { $0.path.count }.max()! + 2 // 2 extra for the quotes
144+
for result in mismatchedFailedHierarchyResults {
145+
print("\(result.path.singleQuoted.padding(toLength: firstColumnWidth, withPad: " ", startingAt: 0)) relative to \(result.parent.singleQuoted) \(result.asSymbolLink ? "(symbol link)" : "")")
146+
}
147+
}
148+
}
149+
}
150+
}

0 commit comments

Comments
 (0)