Skip to content

Commit e610e0d

Browse files
authored
Consistently curate language refined symbols in the language specific places (#854)
* Add source language information to nodes in the path hierarchy rdar://122300247 * Explicitly track when topics are manually curated. The previous heuristic of checking if a node has more than 1 parent is incorrect when a symbol with multiple language representations exist in different places in different source languages. * Fix unrelated bug where topic references didn't encode their navigator title variants * Fix unrelated bug where `SourceLanguage(id:)` didn't recognize C++ IDs * Fix unrelated bug where handcrafted test data isn't recognized as default implementation * Use path hierarchy to source automatic curation for symbols * Update curation tests that were verifying behavior caused by bugs * Remove test that verifies an behavior that will never occur Symbols will never be automatically curated under an article. Manually adjusting topic graph edges makes the test unrealistic. * Fix hand crafted test data that incorrectly mixed source languages in a single symbol graph * Fix more hand crafted test data that incorrectly mixed source languages in a single symbol graph * Change isDisfavoredInCollision into a "special behaviors" option set * Bring over bug fixes from follow up PR * Unify check that a node matches a language filter * Minor grammar fix in comment
1 parent bf8bafb commit e610e0d

24 files changed

+2671
-250
lines changed

Sources/SwiftDocC/Infrastructure/DocumentationContext.swift

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,8 @@ public class DocumentationContext: DocumentationContextDataProviderDelegate {
117117
var dataProvider: DocumentationContextDataProvider
118118

119119
/// The graph of all the documentation content and their relationships to each other.
120+
///
121+
/// > Important: The topic graph has no awareness of source language specific edges.
120122
var topicGraph = TopicGraph()
121123

122124
/// User-provided global options for this documentation conversion.
@@ -2320,7 +2322,7 @@ public class DocumentationContext: DocumentationContextDataProviderDelegate {
23202322
guard let topicGraphNode = topicGraph.nodeWithReference(reference),
23212323
let topicGraphParentNode = topicGraph.nodeWithReference(parentReference),
23222324
// Check that the node hasn't got any parents from manual curation
2323-
topicGraph.reverseEdges[reference] == nil
2325+
!topicGraphNode.isManuallyCurated
23242326
else { return }
23252327
topicGraph.addEdge(from: topicGraphParentNode, to: topicGraphNode)
23262328
automaticallyCuratedSymbols.append((child: reference, parent: parentReference))
@@ -2386,6 +2388,7 @@ public class DocumentationContext: DocumentationContextDataProviderDelegate {
23862388
of: reference,
23872389
relateNodes: {
23882390
self.topicGraph.unsafelyAddEdge(source: $0, target: $1)
2391+
self.topicGraph.nodes[$1]?.isManuallyCurated = true
23892392
}
23902393
)
23912394
}
@@ -2573,6 +2576,8 @@ public class DocumentationContext: DocumentationContextDataProviderDelegate {
25732576

25742577
/// Fetch the child nodes of a documentation node with the given `reference`, optionally filtering to only children of the given `kind`.
25752578
///
2579+
/// > Important: The returned list can't be used to determine source language specific children.
2580+
///
25762581
/// - Parameters:
25772582
/// - reference: The reference of the node to fetch children for.
25782583
/// - kind: An optional documentation node kind to filter the children by.

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ private extension PathHierarchy.Node {
2222
func dumpableNode() -> DumpableNode {
2323
// Each node is printed as 3-layer hierarchy with the child names, their kind disambiguation, and their hash disambiguation.
2424
return DumpableNode(
25-
name: symbol.map { "{ \($0.identifier.precise) : \($0.identifier.interfaceLanguage).\($0.kind.identifier.identifier) }" } ?? "[ \(name) ]",
25+
name: symbol.map { "{ \($0.identifier.precise) : \($0.kind.identifier.identifier) [\(languages.map(\.name).joined(separator: ", "))] }" } ?? "[ \(name) ]",
2626
children: children.sorted(by: \.key).map { (key, disambiguationTree) -> DumpableNode in
2727
let grouped = [String: [PathHierarchy.DisambiguationContainer.Element]](grouping: disambiguationTree.storage, by: { $0.kind ?? "_" })
2828
return DumpableNode(

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,7 @@ extension PathHierarchy {
293293
onlyFindSymbols: Bool,
294294
rawPathForError: String
295295
) throws -> Node {
296-
if let favoredMatch = collisions.singleMatch({ $0.node.isDisfavoredInCollision == false }) {
296+
if let favoredMatch = collisions.singleMatch({ $0.node.specialBehaviors.contains(.disfavorInLinkCollision) == false }) {
297297
return favoredMatch.node
298298
}
299299
// If a module has the same name as the article root (which is named after the bundle display name) then its possible

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

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ extension PathHierarchy.FileRepresentation {
4242
at: identifierMap[node.identifier]!,
4343
to: Node(
4444
name: node.name,
45-
isDisfavoredInCollision: node.isDisfavoredInCollision,
45+
rawSpecialBehavior: node.specialBehaviors.rawValue,
4646
children: node.children.values.flatMap({ tree in
4747
var disambiguations = [Node.Disambiguation]()
4848
for element in tree.storage where element.node.identifier != nil { // nodes without identifiers can't be found in the tree
@@ -102,7 +102,7 @@ extension PathHierarchy {
102102
/// A node in the hierarchy.
103103
struct Node: Codable {
104104
var name: String
105-
var isDisfavoredInCollision: Bool = false
105+
var rawSpecialBehavior: Int = 0
106106
var children: [Disambiguation] = []
107107
var symbolID: SymbolGraph.Symbol.Identifier?
108108

@@ -118,7 +118,7 @@ extension PathHierarchy {
118118
extension PathHierarchy.FileRepresentation.Node {
119119
enum CodingKeys: String, CodingKey {
120120
case name
121-
case isDisfavoredInCollision = "disfavored"
121+
case rawSpecialBehavior = "disfavored"
122122
case children
123123
case symbolID
124124
}
@@ -127,7 +127,7 @@ extension PathHierarchy.FileRepresentation.Node {
127127
let container = try decoder.container(keyedBy: CodingKeys.self)
128128

129129
self.name = try container.decode(String.self, forKey: .name)
130-
self.isDisfavoredInCollision = try container.decodeIfPresent(Bool.self, forKey: .isDisfavoredInCollision) ?? false
130+
self.rawSpecialBehavior = try container.decodeIfPresent(Int.self, forKey: .rawSpecialBehavior) ?? 0
131131
self.children = try container.decodeIfPresent([Disambiguation].self, forKey: .children) ?? []
132132
self.symbolID = try container.decodeIfPresent(SymbolGraph.Symbol.Identifier.self, forKey: .symbolID)
133133
}
@@ -136,7 +136,9 @@ extension PathHierarchy.FileRepresentation.Node {
136136
var container: KeyedEncodingContainer = encoder.container(keyedBy: CodingKeys.self)
137137

138138
try container.encode(self.name, forKey: .name)
139-
try container.encodeIfTrue(self.isDisfavoredInCollision, forKey: .isDisfavoredInCollision)
139+
if rawSpecialBehavior > 0 {
140+
try container.encode(rawSpecialBehavior, forKey: .rawSpecialBehavior)
141+
}
140142
try container.encodeIfNotEmpty(self.children, forKey: .children)
141143
try container.encodeIfPresent(symbolID, forKey: .symbolID)
142144
}

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

Lines changed: 62 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -68,11 +68,15 @@ struct PathHierarchy {
6868
var allNodes: [String: [Node]] = [:]
6969

7070
let symbolGraphs = loader.symbolGraphs
71-
.sorted(by: { lhs, _ in
72-
return !lhs.key.lastPathComponent.contains("@")
71+
.map { url, graph in
72+
// Only compute the source language for each symbol graph once.
73+
(url: url, graph: graph, language: graph.symbols.values.mapFirst(where: { SourceLanguage(id: $0.identifier.interfaceLanguage) }))
74+
}
75+
.sorted(by: { lhs, rhs in
76+
return !lhs.url.lastPathComponent.contains("@")
7377
})
7478

75-
for (url, graph) in symbolGraphs {
79+
for (url, graph, language) in symbolGraphs {
7680
let moduleName = graph.module.name
7781
let moduleNode: Node
7882

@@ -84,11 +88,11 @@ struct PathHierarchy {
8488
} else if let existingModuleNode = roots[moduleName] {
8589
moduleNode = existingModuleNode
8690
} else {
87-
let moduleIdentifierLanguage = graph.symbols.values.first?.identifier.interfaceLanguage ?? SourceLanguage.swift.id
91+
let moduleIdentifierLanguage = language ?? .swift
8892
let moduleSymbol = SymbolGraph.Symbol(
89-
identifier: .init(precise: moduleName, interfaceLanguage: moduleIdentifierLanguage),
93+
identifier: .init(precise: moduleName, interfaceLanguage: moduleIdentifierLanguage.id),
9094
names: SymbolGraph.Symbol.Names(title: moduleName, navigator: nil, subHeading: nil, prose: nil),
91-
pathComponents: [moduleName],
95+
pathComponents: [], // Other symbols don't include the module name in their path components.
9296
docComment: nil,
9397
accessLevel: SymbolGraph.Symbol.AccessControl(rawValue: "public"),
9498
kind: SymbolGraph.Symbol.Kind(parsedIdentifier: .module, displayName: moduleKindDisplayName),
@@ -98,24 +102,37 @@ struct PathHierarchy {
98102
moduleNode = newModuleNode
99103
allNodes[moduleName] = [moduleNode]
100104
}
105+
if let language = language {
106+
moduleNode.languages.insert(language)
107+
}
101108

102109
var nodes: [String: Node] = [:]
103110
nodes.reserveCapacity(graph.symbols.count)
104111
for (id, symbol) in graph.symbols {
105112
if let existingNode = allNodes[id]?.first(where: {
106-
// If both identifiers are in the same language, they are the same symbol
113+
// If both identifiers are in the same language, they are the same symbol.
107114
$0.symbol!.identifier.interfaceLanguage == symbol.identifier.interfaceLanguage
108-
// Otherwise, if both have the same name and kind their differences doesn't matter for link resolution purposes
109-
|| ($0.name == symbol.pathComponents.last && $0.symbol!.kind.identifier == symbol.kind.identifier)
115+
// If both have the same path components and kind, their differences don't matter for link resolution purposes.
116+
|| ($0.symbol!.pathComponents == symbol.pathComponents && $0.symbol!.kind.identifier == symbol.kind.identifier)
110117
}) {
111118
nodes[id] = existingNode
119+
existingNode.languages.insert(language!) // If we have symbols in this graph we have a language as well
112120
} else {
113121
assert(!symbol.pathComponents.isEmpty, "A symbol should have at least its own name in its path components.")
114122
let node = Node(symbol: symbol, name: symbol.pathComponents.last!)
115123
// Disfavor synthesized symbols when they collide with other symbol with the same path.
116124
// FIXME: Get information about synthesized symbols from SymbolKit https://github.com/apple/swift-docc-symbolkit/issues/58
117-
node.isDisfavoredInCollision = symbol.identifier.precise.contains("::SYNTHESIZED::")
125+
if symbol.identifier.precise.contains("::SYNTHESIZED::") {
126+
node.specialBehaviors = [.disfavorInLinkCollision, .excludeFromAutomaticCuration]
127+
}
118128
nodes[id] = node
129+
130+
if let existing = allNodes[id] {
131+
node.counterpart = existing.first
132+
for other in existing {
133+
other.counterpart = node
134+
}
135+
}
119136
allNodes[id, default: []].append(node)
120137
}
121138
}
@@ -154,7 +171,7 @@ struct PathHierarchy {
154171
}
155172
// Default implementations collide with the protocol requirement that they implement.
156173
// Disfavor the default implementation to favor the protocol requirement (or other symbol with the same path).
157-
sourceNode.isDisfavoredInCollision = true
174+
sourceNode.specialBehaviors = [.disfavorInLinkCollision, .excludeFromAutomaticCuration]
158175

159176
guard sourceNode.parent == nil else {
160177
// This node already has a direct member-of parent. No need to go via the default-implementation-of relationship to find its location in the hierarchy.
@@ -213,15 +230,15 @@ struct PathHierarchy {
213230
guard knownDisambiguatedPathComponents != nil else {
214231
// If the path hierarchy wasn't passed any "known disambiguated path components" then the sparse/placeholder nodes won't contain any disambiguation.
215232
let nodeWithoutSymbol = Node(name: component)
216-
nodeWithoutSymbol.isDisfavoredInCollision = true
233+
nodeWithoutSymbol.specialBehaviors = [.disfavorInLinkCollision, .excludeFromAutomaticCuration]
217234
parent.add(child: nodeWithoutSymbol, kind: nil, hash: nil)
218235
parent = nodeWithoutSymbol
219236
continue
220237
}
221238
// If the path hierarchy was passed a lookup of "known disambiguation" path components", then it's possible that each path component could contain disambiguation that needs to be parsed.
222239
let component = PathParser.parse(pathComponent: component[...])
223240
let nodeWithoutSymbol = Node(name: String(component.name))
224-
nodeWithoutSymbol.isDisfavoredInCollision = true
241+
nodeWithoutSymbol.specialBehaviors = [.disfavorInLinkCollision, .excludeFromAutomaticCuration]
225242
// Create a spare/placeholder node with the parsed disambiguation for this path component.
226243
switch component.disambiguation {
227244
case .kindAndHash(kind: let kind, hash: let hash):
@@ -265,8 +282,8 @@ struct PathHierarchy {
265282
node.identifier = ResolvedIdentifier()
266283
lookup[node.identifier] = node
267284
}
268-
for tree in node.children.values {
269-
for element in tree.storage {
285+
for container in node.children.values {
286+
for element in container.storage {
270287
assert(element.node.parent === node, {
271288
func describe(_ node: Node?) -> String {
272289
guard let node = node else { return "<nil>" }
@@ -402,29 +419,45 @@ extension PathHierarchy {
402419
fileprivate(set) unowned var parent: Node?
403420
/// The symbol, if a node has one.
404421
fileprivate(set) var symbol: SymbolGraph.Symbol?
405-
406-
/// If the path hierarchy should disfavor this node in a link collision.
407-
///
408-
/// By default, nodes are not disfavored.
422+
/// The languages where this node's symbol is represented.
423+
fileprivate(set) var languages: Set<SourceLanguage> = []
424+
/// The other language representation of this symbol.
409425
///
410-
/// If a favored node collides with a disfavored node the link will resolve to the favored node without
411-
/// requiring any disambiguation. Referencing the disfavored node requires disambiguation.
412-
var isDisfavoredInCollision: Bool
426+
/// > Note: Swift currently only supports one other language representation (either Objective-C or C++ but not both).
427+
fileprivate(set) unowned var counterpart: Node?
428+
429+
/// A set of non-standard behaviors that apply to this node.
430+
fileprivate(set) var specialBehaviors: SpecialBehaviors
431+
432+
/// Options that specify non-standard behaviors of a node.
433+
struct SpecialBehaviors: OptionSet {
434+
let rawValue: Int
435+
436+
/// This node is disfavored in the the case of a link collision.
437+
///
438+
/// If a favored node collides with a disfavored node the link will resolve to the favored node without requiring any disambiguation.
439+
/// Referencing the disfavored node requires disambiguation unless it's the only match for that link.
440+
static let disfavorInLinkCollision = SpecialBehaviors(rawValue: 1 << 0)
441+
442+
/// This node is excluded from automatic curation.
443+
static let excludeFromAutomaticCuration = SpecialBehaviors(rawValue: 1 << 1)
444+
}
413445

414446
/// Initializes a symbol node.
415447
fileprivate init(symbol: SymbolGraph.Symbol!, name: String) {
416448
self.symbol = symbol
417449
self.name = name
418450
self.children = [:]
419-
self.isDisfavoredInCollision = false
451+
self.specialBehaviors = []
452+
self.languages = [SourceLanguage(id: symbol.identifier.interfaceLanguage)]
420453
}
421454

422455
/// Initializes a non-symbol node with a given name.
423456
fileprivate init(name: String) {
424457
self.symbol = nil
425458
self.name = name
426459
self.children = [:]
427-
self.isDisfavoredInCollision = false
460+
self.specialBehaviors = []
428461
}
429462

430463
/// Adds a descendant to this node, providing disambiguation information from the node's symbol.
@@ -463,6 +496,10 @@ extension PathHierarchy {
463496
element.node.parent = self
464497
}
465498
}
499+
500+
if let otherSymbol = other.symbol {
501+
languages.insert(SourceLanguage(id: otherSymbol.identifier.interfaceLanguage))
502+
}
466503
}
467504
}
468505
}
@@ -640,7 +677,7 @@ extension PathHierarchy {
640677
} else {
641678
node = Node(name: fileNode.name)
642679
}
643-
node.isDisfavoredInCollision = fileNode.isDisfavoredInCollision
680+
node.specialBehaviors = .init(rawValue: fileNode.rawSpecialBehavior)
644681
node.identifier = identifiers[index]
645682
lookup[node.identifier] = node
646683
}

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

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@ final class PathHierarchyBasedLinkResolver {
4949
func traverseSymbolAndParentPairs(_ observe: (_ symbol: ResolvedTopicReference, _ parent: ResolvedTopicReference) -> Void) {
5050
for (id, node) in pathHierarchy.lookup {
5151
guard node.symbol != nil else { continue }
52-
5352
guard let parentID = node.parent?.identifier else { continue }
5453

5554
// Only symbols in the symbol index are added to the reference map.
@@ -58,6 +57,42 @@ final class PathHierarchyBasedLinkResolver {
5857
}
5958
}
6059

60+
/// Returns the direct descendants of the given page that match the given source language filter.
61+
///
62+
/// A descendant is included if it has a language representation in at least one of the languages in the given language filter or if the language filter is empty.
63+
///
64+
/// - Parameters:
65+
/// - reference: The identifier of the page whose descendants to return.
66+
/// - languagesFilter: A set of source languages to filter descendants against.
67+
/// - Returns: The references of each direct descendant that has a language representation in at least one of the given languages.
68+
func directDescendants(of reference: ResolvedTopicReference, languagesFilter: Set<SourceLanguage>) -> Set<ResolvedTopicReference> {
69+
guard let id = resolvedReferenceMap[reference] else { return [] }
70+
let node = pathHierarchy.lookup[id]!
71+
72+
func directDescendants(of node: PathHierarchy.Node) -> [ResolvedTopicReference] {
73+
return node.children.flatMap { _, container in
74+
container.storage.compactMap { element in
75+
guard let childID = element.node.identifier, // Don't include sparse nodes
76+
!element.node.specialBehaviors.contains(.excludeFromAutomaticCuration),
77+
element.node.matches(languagesFilter: languagesFilter)
78+
else {
79+
return nil
80+
}
81+
return resolvedReferenceMap[childID]
82+
}
83+
}
84+
}
85+
86+
var results = Set<ResolvedTopicReference>()
87+
if node.matches(languagesFilter: languagesFilter) {
88+
results.formUnion(directDescendants(of: node))
89+
}
90+
if let counterpart = node.counterpart, counterpart.matches(languagesFilter: languagesFilter) {
91+
results.formUnion(directDescendants(of: counterpart))
92+
}
93+
return results
94+
}
95+
6196
/// Traverse all symbols of the same kind that have collisions.
6297
func traverseOverloadedSymbols(_ observe: (_ overloadedSymbols: [ResolvedTopicReference]) throws -> Void) rethrows {
6398
try pathHierarchy.traverseOverloadedSymbolGroups() { overloadedSymbols in
@@ -315,3 +350,9 @@ private func linkName<S: StringProtocol>(filename: S) -> String {
315350

316351
private let whitespaceAndDashes = CharacterSet.whitespaces
317352
.union(CharacterSet(charactersIn: "-–—")) // hyphen, en dash, em dash
353+
354+
private extension PathHierarchy.Node {
355+
func matches(languagesFilter: Set<SourceLanguage>) -> Bool {
356+
languagesFilter.isEmpty || !self.languages.isDisjoint(with: languagesFilter)
357+
}
358+
}

0 commit comments

Comments
 (0)