Skip to content

Commit 8a52dcf

Browse files
[Overloads] Do not automatically curate overloaded symbols, in favor of their overload group (#864)
rdar://117768214
1 parent 37ed32d commit 8a52dcf

File tree

13 files changed

+403
-3
lines changed

13 files changed

+403
-3
lines changed

Sources/SwiftDocC/Infrastructure/DocumentationContext.swift

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1430,6 +1430,14 @@ public class DocumentationContext: DocumentationContextDataProviderDelegate {
14301430
localCache: documentationCache,
14311431
engine: diagnosticEngine
14321432
)
1433+
case .overloadOf:
1434+
// Build overload <-> overloadGroup relationships.
1435+
SymbolGraphRelationshipsBuilder.addOverloadRelationship(
1436+
edge: edge,
1437+
context: self,
1438+
localCache: documentationCache,
1439+
engine: diagnosticEngine
1440+
)
14331441
default:
14341442
break
14351443
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ struct PathHierarchy {
141141
// An 'overloadOf' relationship points from symbol -> group. We want to disfavor the
142142
// individual overload symbols in favor of resolving links to their overload group
143143
// symbol.
144-
nodes[relationship.source]?.specialBehaviors.insert(.disfavorInLinkCollision)
144+
nodes[relationship.source]?.specialBehaviors.formUnion([.disfavorInLinkCollision, .excludeFromAutomaticCuration])
145145
}
146146

147147
// If there are multiple symbol graphs (for example for different source languages or platforms) then the nodes may have already been added to the hierarchy.

Sources/SwiftDocC/Infrastructure/Symbol Graph/SymbolGraphRelationshipsBuilder.swift

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -430,4 +430,28 @@ struct SymbolGraphRelationshipsBuilder {
430430
constraint: newConstraint
431431
)
432432
}
433+
434+
static func addOverloadRelationship(
435+
edge: SymbolGraph.Relationship,
436+
context: DocumentationContext,
437+
localCache: DocumentationContext.LocalCache,
438+
engine: DiagnosticEngine
439+
) {
440+
guard let overloadNode = localCache[edge.source] else {
441+
engine.emit(NodeProblem.notFound(edge.source))
442+
return
443+
}
444+
guard let overloadGroupNode = localCache[edge.target] else {
445+
engine.emit(NodeProblem.notFound(edge.target))
446+
return
447+
}
448+
449+
guard let overloadTopicGraphNode = context.topicGraph.nodes[overloadNode.reference],
450+
let overloadGroupTopicGraphNode = context.topicGraph.nodes[overloadGroupNode.reference]
451+
else {
452+
return
453+
}
454+
overloadGroupTopicGraphNode.isOverloadGroup = true
455+
context.topicGraph.addEdge(from: overloadGroupTopicGraphNode, to: overloadTopicGraphNode)
456+
}
433457
}

Sources/SwiftDocC/Infrastructure/Topic Graph/TopicGraph.swift

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,10 @@ struct TopicGraph {
9898

9999
/// If true, the topic has been manually organized into a topic section on some other page.
100100
var isManuallyCurated: Bool = false
101-
101+
102+
/// If true, this topic is a generated "overload group" symbol page.
103+
var isOverloadGroup: Bool = false
104+
102105
init(reference: ResolvedTopicReference, kind: DocumentationNode.Kind, source: ContentLocation, title: String, isResolvable: Bool = true, isVirtual: Bool = false, isEmptyExtension: Bool = false, isManuallyCurated: Bool = false) {
103106
self.reference = reference
104107
self.kind = kind
@@ -321,6 +324,16 @@ struct TopicGraph {
321324
}
322325
}
323326

327+
/// Returns the children of this node that reference it as their overload group.
328+
func overloads(of groupReference: ResolvedTopicReference) -> [ResolvedTopicReference]? {
329+
guard nodes[groupReference]?.isOverloadGroup == true else {
330+
return nil
331+
}
332+
return edges[groupReference, default: []].filter({ childReference in
333+
nodes[childReference]?.isManuallyCurated == false
334+
})
335+
}
336+
324337
/// Returns true if a node exists with the given reference and it's set as linkable.
325338
func isLinkable(_ reference: ResolvedTopicReference) -> Bool {
326339
// Sections (represented by the node path + fragment with the section name)

Sources/SwiftDocC/LinkTargets/LinkDestinationSummary.swift

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,13 @@ public extension DocumentationNode {
333333
case .tutorial, .tutorialArticle, .technology, .technologyOverview, .chapter, .volume, .onPageLandmark:
334334
taskGroups = [.init(title: nil, identifiers: context.children(of: reference).map { $0.reference.absoluteString })]
335335
default:
336-
taskGroups = renderNode.topicSections.map { group in .init(title: group.title, identifiers: group.identifiers) }
336+
var topicSectionGroups: [LinkDestinationSummary.TaskGroup] = renderNode.topicSections.map { group in .init(title: group.title, identifiers: group.identifiers) }
337+
338+
if let overloadChildren = context.topicGraph.overloads(of: self.reference), !overloadChildren.isEmpty {
339+
topicSectionGroups.append(.init(title: "Overloads", identifiers: overloadChildren.map(\.absoluteString)))
340+
}
341+
342+
taskGroups = topicSectionGroups
337343
for variant in renderNode.topicSectionsVariants.variants {
338344
taskGroupVariants[variant.traits] = variant.applyingPatchTo(renderNode.topicSections).map { group in .init(title: group.title, identifiers: group.identifiers) }
339345
}

Tests/SwiftDocCTests/Indexing/NavigatorIndexTests.swift

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1621,6 +1621,25 @@ Root
16211621
)
16221622
}
16231623

1624+
func testNavigatorDoesNotContainOverloads() throws {
1625+
enableFeatureFlag(\.isExperimentalOverloadedSymbolPresentationEnabled)
1626+
1627+
let navigatorIndex = try generatedNavigatorIndex(
1628+
for: "OverloadedSymbols",
1629+
bundleIdentifier: "com.shapes.ShapeKit")
1630+
1631+
let protocolID = try XCTUnwrap(navigatorIndex.id(for: "/documentation/shapekit/overloadedprotocol", with: .swift))
1632+
let protocolNode = try XCTUnwrap(search(node: navigatorIndex.navigatorTree.root) { $0.id == protocolID })
1633+
XCTAssertEqual(protocolNode.children.map(\.item.title), [
1634+
"Instance Methods",
1635+
"func fourthTestMemberName(test:)",
1636+
])
1637+
1638+
let overloadGroupID = try XCTUnwrap(navigatorIndex.id(for: "/documentation/shapekit/overloadedprotocol/fourthtestmembername(test:)-9b6be", with: .swift))
1639+
let overloadGroupNode = try XCTUnwrap(search(node: navigatorIndex.navigatorTree.root) { $0.id == overloadGroupID })
1640+
XCTAssert(overloadGroupNode.children.isEmpty)
1641+
}
1642+
16241643
func generatedNavigatorIndex(for testBundleName: String, bundleIdentifier: String) throws -> NavigatorIndex {
16251644
let (bundle, context) = try testBundleAndContext(named: testBundleName)
16261645
let renderContext = RenderContext(documentationContext: context, bundle: bundle)

Tests/SwiftDocCTests/Infrastructure/AutomaticCurationTests.swift

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -669,6 +669,74 @@ class AutomaticCurationTests: XCTestCase {
669669
]
670670
)
671671
}
672+
673+
func testOverloadedSymbolsAreCuratedUnderGroup() throws {
674+
enableFeatureFlag(\.isExperimentalOverloadedSymbolPresentationEnabled)
675+
676+
let protocolRenderNode = try renderNode(
677+
atPath: "/documentation/ShapeKit/OverloadedProtocol",
678+
fromTestBundleNamed: "OverloadedSymbols")
679+
680+
guard protocolRenderNode.topicSections.count == 1, let protocolTopicSection = protocolRenderNode.topicSections.first else {
681+
XCTFail("Expected to find 1 topic section, found \(protocolRenderNode.topicSections.count): \(protocolRenderNode.topicSections.map(\.title?.singleQuoted))")
682+
return
683+
}
684+
685+
XCTAssertEqual(protocolTopicSection.title, "Instance Methods")
686+
XCTAssertEqual(protocolTopicSection.identifiers, [
687+
"doc://com.shapes.ShapeKit/documentation/ShapeKit/OverloadedProtocol/fourthTestMemberName(test:)-9b6be"
688+
])
689+
690+
let overloadGroupRenderNode = try renderNode(
691+
atPath: "/documentation/ShapeKit/OverloadedProtocol/fourthTestMemberName(test:)-9b6be",
692+
fromTestBundleNamed: "OverloadedSymbols")
693+
694+
XCTAssertEqual(
695+
overloadGroupRenderNode.topicSections.count, 0,
696+
"Expected no topic sections, found \(overloadGroupRenderNode.topicSections.map(\.title?.singleQuoted))"
697+
)
698+
}
699+
700+
func testAutomaticCurationHandlesOverloadsWithLanguageFilters() throws {
701+
enableFeatureFlag(\.isExperimentalOverloadedSymbolPresentationEnabled)
702+
703+
let (bundle, context) = try testBundleAndContext(named: "OverloadedSymbols")
704+
705+
let protocolDocumentationNode = try context.entity(
706+
with: .init(
707+
bundleIdentifier: bundle.identifier,
708+
path: "/documentation/ShapeKit/OverloadedProtocol",
709+
sourceLanguage: .swift))
710+
711+
func assertAutomaticCuration(
712+
variants: Set<DocumentationDataVariantsTrait>,
713+
file: StaticString = #file,
714+
line: UInt = #line
715+
) throws {
716+
let topics = try AutomaticCuration.topics(
717+
for: protocolDocumentationNode,
718+
withTraits: variants,
719+
context: context)
720+
721+
guard topics.count == 1, let overloadTopic = topics.first else {
722+
XCTFail(
723+
"Expected one automatic curation topic, found \(topics.count): \(topics.map(\.title?.singleQuoted))",
724+
file: file, line: line)
725+
return
726+
}
727+
728+
XCTAssertEqual(overloadTopic.title, "Instance Methods", file: file, line: line)
729+
XCTAssertEqual(overloadTopic.references.map(\.absoluteString), [
730+
"doc://com.shapes.ShapeKit/documentation/ShapeKit/OverloadedProtocol/fourthTestMemberName(test:)-9b6be"
731+
], file: file, line: line)
732+
}
733+
734+
// AutomaticCuration uses a different method for collecting child nodes when the variant
735+
// traits set is empty and when it's not. Ensure that in both cases, we only see the
736+
// overload group symbol curated under the protocol symbol.
737+
try assertAutomaticCuration(variants: [])
738+
try assertAutomaticCuration(variants: [.swift])
739+
}
672740
}
673741

674742
private func makeSymbol(

Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1248,6 +1248,41 @@ class PathHierarchyTests: XCTestCase {
12481248
"/ShapeKit/OverloadedEnum/firstTestMemberName(_:)-88rbf")
12491249
}
12501250

1251+
func testOverloadedSymbolsWithOverloadGroups() throws {
1252+
enableFeatureFlag(\.isExperimentalOverloadedSymbolPresentationEnabled)
1253+
1254+
let (_, context) = try testBundleAndContext(named: "OverloadedSymbols")
1255+
let tree = context.linkResolver.localResolver.pathHierarchy
1256+
1257+
let paths = tree.caseInsensitiveDisambiguatedPaths()
1258+
1259+
XCTAssertEqual(paths["s:8ShapeKit22OverloadedParentStructV"],
1260+
"/ShapeKit/OverloadedParentStruct-1jr3p")
1261+
XCTAssertEqual(paths["s:8ShapeKit22overloadedparentstructV"],
1262+
"/ShapeKit/overloadedparentstruct-6a7lx")
1263+
1264+
// These need to be disambiguated in two path components
1265+
XCTAssertEqual(paths["s:8ShapeKit22OverloadedParentStructV15fifthTestMemberSivpZ"],
1266+
"/ShapeKit/OverloadedParentStruct-1jr3p/fifthTestMember")
1267+
XCTAssertEqual(paths["s:8ShapeKit22overloadedparentstructV15fifthTestMemberSivp"],
1268+
"/ShapeKit/overloadedparentstruct-6a7lx/fifthTestMember")
1269+
1270+
// This is the only enum case and can be disambiguated as such
1271+
XCTAssertEqual(paths["s:8ShapeKit14OverloadedEnumO19firstTestMemberNameyACSScACmF"],
1272+
"/ShapeKit/OverloadedEnum/firstTestMemberName(_:)-enum.case")
1273+
// These are all methods and can only be disambiguated with the USR hash
1274+
XCTAssertEqual(paths["s:8ShapeKit14OverloadedEnumO19firstTestMemberNameySdSiF"],
1275+
"/ShapeKit/OverloadedEnum/firstTestMemberName(_:)-14g8s")
1276+
XCTAssertEqual(paths["s:8ShapeKit14OverloadedEnumO19firstTestMemberNameySdSfF"],
1277+
"/ShapeKit/OverloadedEnum/firstTestMemberName(_:)-14ife")
1278+
XCTAssertEqual(paths["s:8ShapeKit14OverloadedEnumO19firstTestMemberNameySdSSF"],
1279+
"/ShapeKit/OverloadedEnum/firstTestMemberName(_:)-14ob0")
1280+
XCTAssertEqual(paths["s:8ShapeKit14OverloadedEnumO19firstTestMemberNameyS2dF"],
1281+
"/ShapeKit/OverloadedEnum/firstTestMemberName(_:)-4ja8m")
1282+
XCTAssertEqual(paths["s:8ShapeKit14OverloadedEnumO19firstTestMemberNameySdSaySdGF"],
1283+
"/ShapeKit/OverloadedEnum/firstTestMemberName(_:)-88rbf")
1284+
}
1285+
12511286
func testOverloadGroupSymbolsResolveLinksWithoutHash() throws {
12521287
enableFeatureFlag(\.isExperimentalOverloadedSymbolPresentationEnabled)
12531288

Tests/SwiftDocCTests/Infrastructure/TopicGraphTests.swift

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,36 @@ class TopicGraphTests: XCTestCase {
5757
graph.addEdge(from: testNodeWithTitle("C"), to: testNodeWithTitle("A"))
5858
return graph
5959
}
60+
61+
/// Return a graph with overload group information:
62+
///
63+
/// ```
64+
/// Parent
65+
/// -> A
66+
/// -> B
67+
/// -> Overload Group
68+
/// -> A
69+
/// -> B
70+
/// ```
71+
static var withOverloadGroup: TopicGraph {
72+
var graph = TopicGraph()
73+
74+
let parent = testNodeWithTitle("Parent")
75+
let group = testNodeWithTitle("Overload Group")
76+
let a = testNodeWithTitle("A")
77+
let b = testNodeWithTitle("B")
78+
79+
graph.addEdge(from: parent, to: a)
80+
graph.addEdge(from: parent, to: b)
81+
graph.addEdge(from: parent, to: group)
82+
83+
graph.addEdge(from: group, to: a)
84+
graph.addEdge(from: group, to: b)
85+
86+
graph.nodes[group.reference]?.isOverloadGroup = true
87+
88+
return graph
89+
}
6090
}
6191
func testNodes() {
6292
XCTAssertEqual(1, TestGraphs.withOneNode.nodes.count)
@@ -274,4 +304,14 @@ class TopicGraphTests: XCTestCase {
274304
}
275305
}
276306
}
307+
308+
func testCollectOverloads() {
309+
let graph = TestGraphs.withOverloadGroup
310+
let overloadGroup = TestGraphs.testNodeWithTitle("Overload Group")
311+
312+
XCTAssertEqual(graph.overloads(of: overloadGroup.reference), [
313+
TestGraphs.testNodeWithTitle("A").reference,
314+
TestGraphs.testNodeWithTitle("B").reference,
315+
])
316+
}
277317
}

0 commit comments

Comments
 (0)