Skip to content

[6.2] clone path hierarchy nodes when their parent has a language-specific counterpart #1209

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,51 @@ struct PathHierarchy {
// would require that we redundantly create multiple nodes for the same symbol in many common cases and then merge them. To avoid doing that, we instead check
// the source symbol's path components to find the correct target symbol by matching its name.
if let targetNode = nodes[relationship.target], targetNode.name == expectedContainerName {
targetNode.add(symbolChild: sourceNode)
if sourceNode.parent == nil {
targetNode.add(symbolChild: sourceNode)
} else if sourceNode.parent !== targetNode {
// If the node we have for the child has an existing parent that doesn't
// match the parent from this symbol graph, we need to clone the child to
// ensure that the hierarchy remains consistent.
let clonedSourceNode = Node(
cloning: sourceNode,
symbol: graph.symbols[relationship.source],
children: [:],
languages: [language!]
)

// The original node no longer represents this symbol graph's language,
// so remove that data from there.
sourceNode.languages.remove(language!)

// Make sure that the clone's children can all line up with symbols from this symbol graph.
for (childName, children) in sourceNode.children {
for child in children.storage {
guard let childSymbol = child.node.symbol else {
// We shouldn't come across any non-symbol nodes here,
// but assume they can work as child of both variants.
clonedSourceNode.add(child: child.node, kind: child.kind, hash: child.hash)
continue
}
if nodes[childSymbol.identifier.precise] === child.node {
clonedSourceNode.add(symbolChild: child.node)
}
}
}

// Track the cloned node in the lists of nodes.
nodes[relationship.source] = clonedSourceNode
if let existingNodes = allNodes[relationship.source] {
clonedSourceNode.counterpart = existingNodes.first
for other in existingNodes {
other.counterpart = clonedSourceNode
}
}
allNodes[relationship.source, default: []].append(clonedSourceNode)

// Finally, add the cloned node as a child of its parent.
targetNode.add(symbolChild: clonedSourceNode)
}
topLevelCandidates.removeValue(forKey: relationship.source)
} else if var targetNodes = allNodes[relationship.target] {
// If the source was added in an extension symbol graph file, then its target won't be found in the same symbol graph file (in `nodes`).
Expand Down Expand Up @@ -532,7 +576,21 @@ extension PathHierarchy {
self.children = [:]
self.specialBehaviors = []
}


/// Initializes a node with a new identifier but the data from an existing node.
fileprivate init(
cloning source: Node,
symbol: SymbolGraph.Symbol?? = nil,
children: [String: DisambiguationContainer]? = nil,
languages: Set<SourceLanguage>? = nil
) {
self.symbol = symbol ?? source.symbol
self.name = source.name
self.children = children ?? source.children
self.specialBehaviors = source.specialBehaviors
self.languages = languages ?? source.languages
}

/// Adds a descendant to this node, providing disambiguation information from the node's symbol.
fileprivate func add(symbolChild: Node) {
precondition(symbolChild.symbol != nil)
Expand All @@ -558,6 +616,8 @@ extension PathHierarchy {
)
return
}

assert(child.parent == nil, "Nodes that already have a parent should not be added to a different parent.")
// If the name was passed explicitly, then the node could have spaces in its name
child.parent = self
children[child.name, default: .init()].add(child, kind: kind, hash: hash, parameterTypes: parameterTypes, returnTypes: returnTypes)
Expand Down
80 changes: 79 additions & 1 deletion Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2820,7 +2820,85 @@ class PathHierarchyTests: XCTestCase {
XCTAssertEqual(paths[containerID], "/ModuleName/ContainerName")
XCTAssertEqual(paths[memberID], "/ModuleName/ContainerName/memberName") // The Swift spelling is preferred
}


func testLanguageRepresentationsWithDifferentParentKinds() throws {
enableFeatureFlag(\.isExperimentalLinkHierarchySerializationEnabled)

let containerID = "some-container-symbol-id"
let memberID = "some-member-symbol-id"

let catalog = Folder(name: "unit-test.docc", content: [
Folder(name: "clang", content: [
JSONFile(name: "ModuleName.symbols.json", content: makeSymbolGraph(
moduleName: "ModuleName",
symbols: [
makeSymbol(id: containerID, language: .objectiveC, kind: .union, pathComponents: ["ContainerName"]),
makeSymbol(id: memberID, language: .objectiveC, kind: .property, pathComponents: ["ContainerName", "MemberName"]),
],
relationships: [
.init(source: memberID, target: containerID, kind: .memberOf, targetFallback: nil)
]
)),
]),

Folder(name: "swift", content: [
JSONFile(name: "ModuleName.symbols.json", content: makeSymbolGraph(
moduleName: "ModuleName",
symbols: [
makeSymbol(id: containerID, kind: .struct, pathComponents: ["ContainerName"]),
makeSymbol(id: memberID, kind: .property, pathComponents: ["ContainerName", "MemberName"]),
],
relationships: [
.init(source: memberID, target: containerID, kind: .memberOf, targetFallback: nil)
]
)),
])
])

let (_, context) = try loadBundle(catalog: catalog)
let tree = context.linkResolver.localResolver.pathHierarchy

let resolvedSwiftContainerID = try tree.find(path: "/ModuleName/ContainerName-struct", onlyFindSymbols: true)
let resolvedSwiftContainer = try XCTUnwrap(tree.lookup[resolvedSwiftContainerID])
XCTAssertEqual(resolvedSwiftContainer.name, "ContainerName")
XCTAssertEqual(resolvedSwiftContainer.symbol?.identifier.precise, containerID)
XCTAssertEqual(resolvedSwiftContainer.symbol?.kind.identifier, .struct)
XCTAssertEqual(resolvedSwiftContainer.languages, [.swift])

let resolvedObjcContainerID = try tree.find(path: "/ModuleName/ContainerName-union", onlyFindSymbols: true)
let resolvedObjcContainer = try XCTUnwrap(tree.lookup[resolvedObjcContainerID])
XCTAssertEqual(resolvedObjcContainer.name, "ContainerName")
XCTAssertEqual(resolvedObjcContainer.symbol?.identifier.precise, containerID)
XCTAssertEqual(resolvedObjcContainer.symbol?.kind.identifier, .union)
XCTAssertEqual(resolvedObjcContainer.languages, [.objectiveC])

let resolvedContainerID = try tree.find(path: "/ModuleName/ContainerName", onlyFindSymbols: true)
XCTAssertEqual(resolvedContainerID, resolvedSwiftContainerID)

let resolvedSwiftMemberID = try tree.find(path: "/ModuleName/ContainerName-struct/MemberName", onlyFindSymbols: true)
let resolvedSwiftMember = try XCTUnwrap(tree.lookup[resolvedSwiftMemberID])
XCTAssertEqual(resolvedSwiftMember.name, "MemberName")
XCTAssertEqual(resolvedSwiftMember.parent?.identifier, resolvedSwiftContainerID)
XCTAssertEqual(resolvedSwiftMember.symbol?.identifier.precise, memberID)
XCTAssertEqual(resolvedSwiftMember.symbol?.kind.identifier, .property)
XCTAssertEqual(resolvedSwiftMember.languages, [.swift])

let resolvedObjcMemberID = try tree.find(path: "/ModuleName/ContainerName-union/MemberName", onlyFindSymbols: true)
let resolvedObjcMember = try XCTUnwrap(tree.lookup[resolvedObjcMemberID])
XCTAssertEqual(resolvedObjcMember.name, "MemberName")
XCTAssertEqual(resolvedObjcMember.parent?.identifier, resolvedObjcContainerID)
XCTAssertEqual(resolvedObjcMember.symbol?.identifier.precise, memberID)
XCTAssertEqual(resolvedObjcMember.symbol?.kind.identifier, .property)
XCTAssertEqual(resolvedObjcMember.languages, [.objectiveC])

let resolvedMemberID = try tree.find(path: "/ModuleName/ContainerName/MemberName", onlyFindSymbols: true)
XCTAssertEqual(resolvedMemberID, resolvedSwiftMemberID)

let paths = tree.caseInsensitiveDisambiguatedPaths()
XCTAssertEqual(paths[containerID], "/ModuleName/ContainerName")
XCTAssertEqual(paths[memberID], "/ModuleName/ContainerName/MemberName")
}

func testMixedLanguageSymbolAndItsExtendingModuleWithDifferentContainerNames() throws {
let containerID = "some-container-symbol-id"
let memberID = "some-member-symbol-id"
Expand Down