From bd6cfb634bcc04c3bdff1031bdfaa8371166e1db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20R=C3=B6nnqvist?= Date: Tue, 23 Apr 2024 11:44:42 +0200 Subject: [PATCH 01/13] Add functions to operate on directed graphs --- .../Utility/Graphs/DirectedGraph+Cycles.swift | 63 +++ .../Utility/Graphs/DirectedGraph+Paths.swift | 64 +++ .../Graphs/DirectedGraph+Traversal.swift | 105 +++++ .../Utility/Graphs/DirectedGraph.swift | 50 +++ .../Utility/DirectedGraphTests.swift | 377 ++++++++++++++++++ 5 files changed, 659 insertions(+) create mode 100644 Sources/SwiftDocC/Utility/Graphs/DirectedGraph+Cycles.swift create mode 100644 Sources/SwiftDocC/Utility/Graphs/DirectedGraph+Paths.swift create mode 100644 Sources/SwiftDocC/Utility/Graphs/DirectedGraph+Traversal.swift create mode 100644 Sources/SwiftDocC/Utility/Graphs/DirectedGraph.swift create mode 100644 Tests/SwiftDocCUtilitiesTests/Utility/DirectedGraphTests.swift diff --git a/Sources/SwiftDocC/Utility/Graphs/DirectedGraph+Cycles.swift b/Sources/SwiftDocC/Utility/Graphs/DirectedGraph+Cycles.swift new file mode 100644 index 0000000000..e4241310ba --- /dev/null +++ b/Sources/SwiftDocC/Utility/Graphs/DirectedGraph+Cycles.swift @@ -0,0 +1,63 @@ +/* + This source file is part of the Swift.org open source project + + Copyright (c) 2024 Apple Inc. and the Swift project authors + Licensed under Apache License v2.0 with Runtime Library Exception + + See https://swift.org/LICENSE.txt for license information + See https://swift.org/CONTRIBUTORS.txt for Swift project authors +*/ + +extension DirectedGraph { + /// Returns the first cycle in the graph encountered through breadth first traversal from a given starting point + /// + /// The cycle starts at the earliest repeated node and ends with the node that links back to the starting point. + /// + /// - Note: A cycle, if found, is guaranteed to contain at least one node. + func firstCycle(from startingPoint: Node) -> Path? { + for case let (path, _, cycleStartIndex?) in accumulatingPaths(from: startingPoint) { + return Array(path[cycleStartIndex...]) + } + return nil + } + + /// Returns a list of all the unique cycles in the graph encountered through breadth first traversal from a given starting point. + /// + /// Each cycle starts at the earliest repeated node and ends with the node that links back to the starting point. + /// + /// Two cycles are considered the same if they have either: + /// - The same start and end points, for example: `1,2,3` and `1,3`. + /// - A rotation of the same cycle, for example: `1,2,3`, `2,3,1`, and `3,1,2`. + func cycles(from startingPoint: Node) -> [Path] { + var cycles = [Path]() + + for case let (path, _, cycleStartIndex?) in accumulatingPaths(from: startingPoint) { + let cycle = path[cycleStartIndex...] + guard !cycles.contains(where: { areSameCycle(cycle, $0) }) else { + continue + } + cycles.append(Array(cycle)) + } + + return cycles + } + + private func areSameCycle(_ lhs: Path.SubSequence, _ rhs: Path) -> Bool { + // Check if the cycles have the same start and end points. + // A cycle has to contain at least one node, so it's always safe to unwrap 'first' and 'last'. + if lhs.first! == rhs.first!, lhs.last! == rhs.last! { + return true + } + + // Check if the cycles are rotations of each other + if lhs.count == rhs.count { + let rhsStart = rhs.first! + + return (lhs + lhs) // Repeat one of the cycles once + .drop(while: { $0 != rhsStart }) // Align it with the other cycle by removing its leading nodes + .starts(with: rhs) // See if the cycles match + } + // The two cycles are different. + return false + } +} diff --git a/Sources/SwiftDocC/Utility/Graphs/DirectedGraph+Paths.swift b/Sources/SwiftDocC/Utility/Graphs/DirectedGraph+Paths.swift new file mode 100644 index 0000000000..5258c7ffb3 --- /dev/null +++ b/Sources/SwiftDocC/Utility/Graphs/DirectedGraph+Paths.swift @@ -0,0 +1,64 @@ +/* + This source file is part of the Swift.org open source project + + Copyright (c) 2024 Apple Inc. and the Swift project authors + Licensed under Apache License v2.0 with Runtime Library Exception + + See https://swift.org/LICENSE.txt for license information + See https://swift.org/CONTRIBUTORS.txt for Swift project authors +*/ + +extension DirectedGraph { + /// Returns a list of all finite paths through the graph from a given starting point. + /// + /// The paths are found in breadth first order, so shorter paths are earlier in the returned list. + /// + /// - Note: Nodes that are reachable through multiple paths will be visited more than once. + /// + /// - Important: If all paths through the graph result in cycles, the returned list will be empty. + func allFinitePaths(from startingPoint: Node) -> [Path] { + var foundPaths = [Path]() + + for (path, isLeaf, _) in accumulatingPaths(from: startingPoint) where isLeaf { + foundPaths.append(path) + } + + return foundPaths + } + + /// Returns a list of the finite paths through the graph with the shortest length from a given starting point. + /// + /// The paths are found in breadth first order, so shorter paths are earlier in the returned list. + /// + /// - Note: Nodes that are reachable through multiple paths will be visited more than once. + /// + /// - Important: If all paths through the graph result in cycles, the returned list will be empty. + func shortestFinitePaths(from startingPoint: Node) -> [Path] { + var foundPaths = [Path]() + + for (path, isLeaf, _) in accumulatingPaths(from: startingPoint) where isLeaf { + if let lengthOfFoundPath = foundPaths.first?.count, lengthOfFoundPath < path.count { + // This path is longer than an already found path. + // All paths found from here on will be longer than what's already found. + break + } + + foundPaths.append(path) + } + + return foundPaths + } + + /// Returns a set of all the reachable leaf nodes by traversing the graph from a given starting point. + /// + /// - Important: If all paths through the graph result in cycles, the returned set will be empty. + func reachableLeafNodes(from startingPoint: Node) -> Set { + var foundLeafNodes: Set = [] + + for (path, isLeaf, _) in accumulatingPaths(from: startingPoint) where isLeaf { + foundLeafNodes.insert(path.last!) + } + + return foundLeafNodes + } +} diff --git a/Sources/SwiftDocC/Utility/Graphs/DirectedGraph+Traversal.swift b/Sources/SwiftDocC/Utility/Graphs/DirectedGraph+Traversal.swift new file mode 100644 index 0000000000..efd7373d41 --- /dev/null +++ b/Sources/SwiftDocC/Utility/Graphs/DirectedGraph+Traversal.swift @@ -0,0 +1,105 @@ +/* + This source file is part of the Swift.org open source project + + Copyright (c) 2024 Apple Inc. and the Swift project authors + Licensed under Apache License v2.0 with Runtime Library Exception + + See https://swift.org/LICENSE.txt for license information + See https://swift.org/CONTRIBUTORS.txt for Swift project authors +*/ + +extension DirectedGraph { + /// Returns a sequence that traverses the graph in breadth first order from a given element, without visiting the same node more than once. + func breadthFirstSearch(from startingPoint: Node) -> some Sequence { + IteratorSequence(GraphNodeIterator(from: startingPoint, traversal: .breadthFirst, in: self)) + } + + /// Returns a sequence that traverses the graph in depth first order from a given element, without visiting the same node more than once. + func depthFirstSearch(from startingPoint: Node) -> some Sequence { + IteratorSequence(GraphNodeIterator(from: startingPoint, traversal: .depthFirst, in: self)) + } +} + +extension DirectedGraph { + /// A path through the graph, including the start and end nodes. + typealias Path = [Node] + /// Information about the current accumulated path during iteration. + typealias PathIterationElement = (path: Path, isLeaf: Bool, cycleStartIndex: Int?) + + /// Returns a sequence of accumulated path information from traversing the graph in breadth first order. + func accumulatingPaths(from startingPoint: Node) -> some Sequence { + IteratorSequence(GraphBreadthFirstPathIterator(from: startingPoint, in: self)) + } +} + +// MARK: Node iterator + +/// An iterator that traverses a graph in either breadth first or depth first order depending on the buffer it uses to track nodes to traverse next. +private struct GraphNodeIterator: IteratorProtocol { + enum Traversal { + case breadthFirst, depthFirst + } + var traversal: Traversal + var graph: DirectedGraph + + private var nodesToTraverse: [Node] + private var seen: Set + + init(from startingPoint: Node, traversal: Traversal, in graph: DirectedGraph) { + self.traversal = traversal + self.graph = graph + self.nodesToTraverse = [startingPoint] + self.seen = [] + } + + private mutating func pop() -> Node? { + guard !nodesToTraverse.isEmpty else { return nil } + + switch traversal { + case .breadthFirst: + return nodesToTraverse.removeFirst() + case .depthFirst: + return nodesToTraverse.removeLast() + } + } + + mutating func next() -> Node? { + while let node = pop() { + guard !seen.contains(node) else { continue } + seen.insert(node) + + nodesToTraverse.append(contentsOf: graph.neighbors(of: node)) + + return node + } + return nil + } +} + +// MARK: Path iterator + +/// An iterator that traverses a graph in breadth first order and returns information about the accumulated path through the graph, up to the current node. +private struct GraphBreadthFirstPathIterator: IteratorProtocol { + var pathsToTraverse: [(Node, [Node])] + var graph: DirectedGraph + + init(from startingPoint: Node, in graph: DirectedGraph) { + self.pathsToTraverse = [(startingPoint, [])] + self.graph = graph + } + + mutating func next() -> DirectedGraph.PathIterationElement? { + guard !pathsToTraverse.isEmpty else { return nil } + let (node, path) = pathsToTraverse.removeFirst() + + if let cycleStartIndex = path.lastIndex(of: node) { + return (path, false, cycleStartIndex) + } + + let newPath = path + [node] + let neighbors = graph.neighbors(of: node) + pathsToTraverse.append(contentsOf: neighbors.map { ($0, newPath) }) + + return (newPath, neighbors.isEmpty, nil) + } +} diff --git a/Sources/SwiftDocC/Utility/Graphs/DirectedGraph.swift b/Sources/SwiftDocC/Utility/Graphs/DirectedGraph.swift new file mode 100644 index 0000000000..5ca8893418 --- /dev/null +++ b/Sources/SwiftDocC/Utility/Graphs/DirectedGraph.swift @@ -0,0 +1,50 @@ +/* + This source file is part of the Swift.org open source project + + Copyright (c) 2024 Apple Inc. and the Swift project authors + Licensed under Apache License v2.0 with Runtime Library Exception + + See https://swift.org/LICENSE.txt for license information + See https://swift.org/CONTRIBUTORS.txt for Swift project authors +*/ + +/// A directed, unweighted graph of nodes. +/// +/// Use a `DirectedGraph` to operate on data that describe the edges between nodes in a directed graph. +/// +/// ## Topics +/// +/// ### Search +/// +/// - ``breadthFirstSearch(from:)`` +/// - ``depthFirstSearch(from:)`` +/// +/// ### Paths +/// +/// - ``shortestFinitePaths(from:)`` +/// - ``allFinitePaths(from:)`` +/// - ``reachableLeafNodes(from:)`` +/// +/// ### Cycle detection +/// +/// - ``firstCycle(from:)`` +/// - ``cycles(from:)`` +/// +/// ### Low-level path traversal +/// +/// - ``accumulatingPaths(from:)`` +struct DirectedGraph { + // There are more generic ways to describe a graph that doesn't require that the elements are Hashable, + // but all our current usages of graph structures use dictionaries to track the neighboring nodes. + // + // This type is internal so we can change it's implementation later when there's new data that's structured differently. + private let _neighbors: [Node: [Node]] + init(neighbors: [Node: [Node]]) { + _neighbors = neighbors + } + + /// Returns the nodes that are reachable from the given node + func neighbors(of node: Node) -> [Node] { + _neighbors[node] ?? [] + } +} diff --git a/Tests/SwiftDocCUtilitiesTests/Utility/DirectedGraphTests.swift b/Tests/SwiftDocCUtilitiesTests/Utility/DirectedGraphTests.swift new file mode 100644 index 0000000000..e9fd8a1c66 --- /dev/null +++ b/Tests/SwiftDocCUtilitiesTests/Utility/DirectedGraphTests.swift @@ -0,0 +1,377 @@ +/* + This source file is part of the Swift.org open source project + + Copyright (c) 2024 Apple Inc. and the Swift project authors + Licensed under Apache License v2.0 with Runtime Library Exception + + See https://swift.org/LICENSE.txt for license information + See https://swift.org/CONTRIBUTORS.txt for Swift project authors +*/ + +import XCTest +@testable import SwiftDocC + +final class DirectedGraphTests: XCTestCase { + + func testGraphWithSingleAdjacency() throws { + // 1───▶2◀───3 + // │ + // ▼ + // 4───▶5 6 + // │ │ + // ▼ ▼ + // 7───▶8◀───9 + let graph = DirectedGraph(neighbors: [ + 1: [2], + 2: [5], + 3: [2], + 4: [5], + 5: [8], + 7: [8], + 6: [9], + 9: [8], + ]) + + // With only a single neighbor per node, breadth first and depth first perform the same traversal + assertEqual(graph.breadthFirstSearch(from: 1), [1,2,5,8]) + assertEqual(graph.breadthFirstSearch(from: 2), [2,5,8]) + assertEqual(graph.breadthFirstSearch(from: 3), [3,2,5,8]) + assertEqual(graph.breadthFirstSearch(from: 4), [4,5,8]) + assertEqual(graph.breadthFirstSearch(from: 5), [5,8]) + assertEqual(graph.breadthFirstSearch(from: 6), [6,9,8]) + assertEqual(graph.breadthFirstSearch(from: 7), [7,8]) + assertEqual(graph.breadthFirstSearch(from: 8), [8]) + assertEqual(graph.breadthFirstSearch(from: 9), [9,8]) + + assertEqual(graph.depthFirstSearch(from: 1), [1,2,5,8]) + assertEqual(graph.depthFirstSearch(from: 2), [2,5,8]) + assertEqual(graph.depthFirstSearch(from: 3), [3,2,5,8]) + assertEqual(graph.depthFirstSearch(from: 4), [4,5,8]) + assertEqual(graph.depthFirstSearch(from: 5), [5,8]) + assertEqual(graph.depthFirstSearch(from: 6), [6,9,8]) + assertEqual(graph.depthFirstSearch(from: 7), [7,8]) + assertEqual(graph.depthFirstSearch(from: 8), [8]) + assertEqual(graph.depthFirstSearch(from: 9), [9,8]) + + // With only a single neighbor per node, the path is the same as the traversal + XCTAssertEqual(graph.allFinitePaths(from: 1), [[1,2,5,8]]) + XCTAssertEqual(graph.allFinitePaths(from: 2), [[2,5,8]]) + XCTAssertEqual(graph.allFinitePaths(from: 3), [[3,2,5,8]]) + XCTAssertEqual(graph.allFinitePaths(from: 4), [[4,5,8]]) + XCTAssertEqual(graph.allFinitePaths(from: 5), [[5,8]]) + XCTAssertEqual(graph.allFinitePaths(from: 6), [[6,9,8]]) + XCTAssertEqual(graph.allFinitePaths(from: 7), [[7,8]]) + XCTAssertEqual(graph.allFinitePaths(from: 8), [[8]]) + XCTAssertEqual(graph.allFinitePaths(from: 9), [[9,8]]) + + for node in 1...9 { + XCTAssertNil(graph.firstCycle(from: node)) + XCTAssertEqual(graph.cycles(from: node), []) + } + } + + func testGraphWithTreeStructure() throws { + // ┌▶5 + // ┌─▶2─┤ + // │ └▶6 + // 1─┼─▶3 + // │ + // └─▶4──▶7──▶8 + let graph = DirectedGraph(neighbors: [ + 1: [2,3,4], + 2: [5,6], + 4: [7], + 7: [8], + ]) + + assertEqual(graph.breadthFirstSearch(from: 1), [1,2,3,4,5,6,7,8]) + + assertEqual(graph.depthFirstSearch(from: 1), [1,4,7,8,3,2,6,5]) + + XCTAssertEqual(graph.allFinitePaths(from: 1), [ + [1,3], + [1,2,5], + [1,2,6], + [1,4,7,8], + ]) + + XCTAssertEqual(graph.shortestFinitePaths(from: 1), [ + [1,3], + ]) + + for node in 1...8 { + XCTAssertNil(graph.firstCycle(from: node)) + } + } + + func testGraphWithTreeStructureAndMultipleAdjacency() throws { + // ┌─▶2─┐ + // │ │ + // 1─┼─▶3─┼▶5──▶6 + // │ │ + // └─▶4─┘ + let graph = DirectedGraph(neighbors: [ + 1: [2,3,4], + 2: [5], + 3: [5], + 4: [5], + 5: [6], + ]) + + assertEqual(graph.breadthFirstSearch(from: 1), [1,2,3,4,5,6]) + assertEqual(graph.depthFirstSearch(from: 1), [1,4,5,6,3,2]) + + XCTAssertEqual(graph.allFinitePaths(from: 1), [ + [1,2,5,6], + [1,3,5,6], + [1,4,5,6], + ]) + + XCTAssertEqual(graph.shortestFinitePaths(from: 1), [ + [1,2,5,6], + [1,3,5,6], + [1,4,5,6], + ]) + + for node in 1...6 { + XCTAssertNil(graph.firstCycle(from: node)) + } + } + + func testComplexGraphWithMultipleAdjacency() throws { + // 1 ┌──▶5 + // │ │ │ + // ▼ │ ▼ + // 2──▶4──┼──▶6──▶8 + // │ ▲ │ ▲ + // ▼ │ │ │ + // 3───┘ └──▶7───┘ + let graph = DirectedGraph(neighbors: [ + 1: [2], + 2: [3,4], + 3: [4], + 4: [5,6,7], + 5: [6], + 6: [8], + 7: [8], + ]) + + assertEqual(graph.breadthFirstSearch(from: 1), [1,2,3,4,5,6,7,8]) + assertEqual(graph.depthFirstSearch(from: 1), [1,2,4,7,8,6,5,3]) + + XCTAssertEqual(graph.allFinitePaths(from: 1), [ + [1,2,4,6,8], + [1,2,4,7,8], + [1,2,3,4,6,8], + [1,2,3,4,7,8], + [1,2,4,5,6,8], + [1,2,3,4,5,6,8], + ]) + + XCTAssertEqual(graph.shortestFinitePaths(from: 1), [ + [1,2,4,6,8], + [1,2,4,7,8], + ]) + + for node in 1...8 { + XCTAssertNil(graph.firstCycle(from: node)) + } + } + + func testGraphWithCycleAndSingleAdjacency() throws { + // 1───▶2◀───3 + // │ + // ▼ + // 4───▶5◀───6 + // │ ▲ + // ▼ │ + // 7───▶8───▶9 + let graph = DirectedGraph(neighbors: [ + 1: [2], + 2: [5], + 3: [2], + 4: [5], + 5: [8], + 6: [5], + 7: [8], + 8: [9], + 9: [6], + ]) + + // With only a single neighbor per node, breadth first and depth first perform the same traversal + assertEqual(graph.breadthFirstSearch(from: 1), [1,2,5,8,9,6]) + assertEqual(graph.depthFirstSearch(from: 1), [1,2,5,8,9,6]) + + XCTAssertEqual(graph.allFinitePaths(from: 1), [], "The only path from '1' is infinite (cyclic)") + XCTAssertEqual(graph.shortestFinitePaths(from: 1), [], "The only path from '1' is infinite (cyclic)") + XCTAssertEqual(graph.reachableLeafNodes(from: 1), [], "The only path from '1' is infinite (cyclic)") + + for node in [1,2,3,4,5] { + XCTAssertEqual(graph.firstCycle(from: node), [5,8,9,6]) + XCTAssertEqual(graph.cycles(from: node), [[5,8,9,6]]) + } + + for node in [7,8] { + XCTAssertEqual(graph.firstCycle(from: node), [8,9,6,5]) + XCTAssertEqual(graph.cycles(from: node), [[8,9,6,5]]) + } + XCTAssertEqual(graph.firstCycle(from: 6), [6,5,8,9]) + XCTAssertEqual(graph.cycles(from: 6), [[6,5,8,9]]) + + XCTAssertEqual(graph.firstCycle(from: 9), [9,6,5,8]) + XCTAssertEqual(graph.cycles(from: 9), [[9,6,5,8]]) + } + + func testGraphsWithCycleAndManyLeafNodes() throws { + do { + // 6 10 + // ▲ ▲ + // 1 3 │ │ + // ▲ ▲ ┌─▶4───▶7 + // │ │ │ │ ▲ + // 0───▶2──┤ │ ║ + // │ ▼ ▼ + // └─▶5───▶9 + // │ │ + // ▼ ▼ + // 8 11 + let graph = DirectedGraph(neighbors: [ + 0: [1,2], + 2: [3,4,5], + 4: [6,7], + 5: [8,9], + 7: [10,9], + 9: [11,7], + ]) + + XCTAssertEqual(graph.firstCycle(from: 0), [7,9]) + XCTAssertEqual(graph.firstCycle(from: 4), [7,9]) + XCTAssertEqual(graph.firstCycle(from: 5), [9,7]) + + XCTAssertEqual(graph.cycles(from: 0), [ + [7,9], // through breadth-first-traversal, 7 is reached before 9. + ]) + + XCTAssertEqual(graph.allFinitePaths(from: 0), [ + [0,1], + [0,2,3], + [0,2,4,6], + [0,2,5,8], + [0,2,4,7,10], + [0,2,5,9,11], + [0,2,4,7,9,11], + [0,2,5,9,7,10], + ]) + + XCTAssertEqual(graph.shortestFinitePaths(from: 0), [ + [0,1], + ]) + + XCTAssertEqual(graph.reachableLeafNodes(from: 0), [1,3,6,8,10,11]) + } + } + + func testGraphWithManyCycles() throws { + // ┌──┐ ┌───▶4────┐ + // │ │ │ │ │ + // │ │ │ ▼ ▼ + // └─▶1───▶2───▶5───▶7───▶10 + // ▲ ▲ │ ▲ + // ║ ║ │ ║ + // ║ ▼ ▼ ▼ + // ╚═══▶3 8───▶9───▶11 + let graph = DirectedGraph(neighbors: [ + 1: [1,2,3], + 2: [3,4,5], + 3: [1,2], + 4: [5,7], + 5: [8,7], + 7: [10,9], + 8: [9], + 9: [11,7], + ]) + + XCTAssertEqual(graph.firstCycle(from: 1), [1]) + XCTAssertEqual(graph.firstCycle(from: 2), [2,3]) + XCTAssertEqual(graph.firstCycle(from: 4), [7,9]) + XCTAssertEqual(graph.firstCycle(from: 8), [9,7]) + + XCTAssertEqual(graph.cycles(from: 1), [ + [1], + [1,3], + // There's also a [1,2,3] cycle but that can also be broken by removing the edge from 3 ──▶ 1. + [2,3], + [7,9] + ]) + + XCTAssertEqual(graph.allFinitePaths(from: 1), [ + [1, 2, 4, 7, 10], + [1, 2, 5, 7, 10], + [1, 2, 4, 5, 7, 10], + [1, 2, 4, 7, 9, 11], + [1, 2, 5, 8, 9, 11], + [1, 2, 5, 7, 9, 11], + [1, 3, 2, 4, 7, 10], + [1, 3, 2, 5, 7, 10], + [1, 2, 4, 5, 8, 9, 11], + [1, 2, 4, 5, 7, 9, 11], + [1, 2, 5, 8, 9, 7, 10], + [1, 3, 2, 4, 5, 7, 10], + [1, 3, 2, 4, 7, 9, 11], + [1, 3, 2, 5, 8, 9, 11], + [1, 3, 2, 5, 7, 9, 11], + [1, 2, 4, 5, 8, 9, 7, 10], + [1, 3, 2, 4, 5, 8, 9, 11], + [1, 3, 2, 4, 5, 7, 9, 11], + [1, 3, 2, 5, 8, 9, 7, 10], + [1, 3, 2, 4, 5, 8, 9, 7, 10] + ]) + + XCTAssertEqual(graph.shortestFinitePaths(from: 1), [ + [1, 2, 4, 7, 10], + [1, 2, 5, 7, 10], + ]) + + XCTAssertEqual(graph.reachableLeafNodes(from: 1), [10, 11]) + } + + func testGraphWithMultiplePathsToEnterCycle() throws { + // ┌─▶2◀─┐ + // │ │ │ + // │ ▼ │ + // 1──┼─▶3 5 + // │ │ ▲ + // │ ▼ │ + // └─▶4──┘ + let graph = DirectedGraph(neighbors: [ + 1: [2,3,4], + 2: [3], + 3: [4], + 4: [5], + 5: [2], + ]) + + // With only a single neighbor per node, breadth first and depth first perform the same traversal + assertEqual(graph.breadthFirstSearch(from: 1), [1,2,3,4,5]) + assertEqual(graph.depthFirstSearch(from: 1), [1,4,5,2,3]) + + XCTAssertEqual(graph.allFinitePaths(from: 1), [ + // The only path from 1 is cyclic + ]) + + XCTAssertEqual(graph.shortestFinitePaths(from: 1), [ + // The only path from 1 is cyclic + ]) + + XCTAssertEqual(graph.firstCycle(from: 1), [2,3,4,5]) + XCTAssertEqual(graph.cycles(from: 1), [ + [2,3,4,5] + // The other cycles are rotations of the first one. + ]) + } +} + +// A private helper to avoid needing to wrap the breadth first and depth first sequences into arrays to compare them. +private func assertEqual(_ lhs: some Sequence, _ rhs: some Sequence, file: StaticString = #filePath, line: UInt = #line) { + XCTAssertEqual(Array(lhs), Array(rhs), file: file, line: line) +} From dca31c1973883ef1e78e938b58a9c51640aa546d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20R=C3=B6nnqvist?= Date: Tue, 23 Apr 2024 12:21:43 +0200 Subject: [PATCH 02/13] Update topic graph to use sequences for graph traversal --- .../Infrastructure/DocumentationContext.swift | 12 +--- .../Topic Graph/TopicGraph.swift | 57 ++++--------------- .../Rendering/RenderNodeTranslator.swift | 28 ++++----- .../Infrastructure/TopicGraphTests.swift | 52 ++++------------- 4 files changed, 39 insertions(+), 110 deletions(-) diff --git a/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift b/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift index 8ae9b7cab3..014817b8aa 100644 --- a/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift +++ b/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift @@ -2707,15 +2707,9 @@ public class DocumentationContext: DocumentationContextDataProviderDelegate { return topicGraph.nodes[reference]?.title } - /** - Traverse the Topic Graph breadth-first, starting at the given reference. - */ - func traverseBreadthFirst(from reference: ResolvedTopicReference, _ observe: (TopicGraph.Node) -> TopicGraph.Traversal) { - guard let node = topicGraph.nodeWithReference(reference) else { - return - } - - topicGraph.traverseBreadthFirst(from: node, observe) + /// Returns a sequence that traverses the topic graph in breadth first order from a given reference, without visiting the same node more than once. + func breadthFirstSearch(from reference: ResolvedTopicReference) -> some Sequence { + topicGraph.breadthFirstSearch(from: reference) } /** diff --git a/Sources/SwiftDocC/Infrastructure/Topic Graph/TopicGraph.swift b/Sources/SwiftDocC/Infrastructure/Topic Graph/TopicGraph.swift index 7855c05063..9a40f9f151 100644 --- a/Sources/SwiftDocC/Infrastructure/Topic Graph/TopicGraph.swift +++ b/Sources/SwiftDocC/Infrastructure/Topic Graph/TopicGraph.swift @@ -23,15 +23,6 @@ import Markdown > If you need information about source language specific edged between nodes, you need to query another source of information. */ struct TopicGraph { - /// A decision about whether to continue a depth-first or breadth-first traversal after visiting a node. - enum Traversal { - /// Stop here, do not visit any more nodes. - case stop - - /// Continue to visit nodes. - case `continue` - } - /// A node in the graph. class Node: Hashable, CustomDebugStringConvertible { /// The location of the node's contents. @@ -284,44 +275,20 @@ struct TopicGraph { return edges[node.reference] ?? [] } - /// Traverses the graph depth-first and passes each node to `observe`. - func traverseDepthFirst(from startingNode: Node, _ observe: (Node) -> Traversal) { - var seen = Set() - var nodesToVisit = [startingNode] - while !nodesToVisit.isEmpty { - let node = nodesToVisit.removeLast() - guard !seen.contains(node) else { - continue - } - let children = self[node].map { - nodeWithReference($0)! - } - nodesToVisit.append(contentsOf: children) - guard case .continue = observe(node) else { - break - } - seen.insert(node) - } + /// Returns a sequence that traverses the topic graph in depth first order from a given reference, without visiting the same node more than once. + func depthFirstSearch(from reference: ResolvedTopicReference) -> some Sequence { + DirectedGraph(neighbors: edges) + .depthFirstSearch(from: reference) + .lazy + .map { nodeWithReference($0)! } } - /// Traverses the graph breadth-first and passes each node to `observe`. - func traverseBreadthFirst(from startingNode: Node, _ observe: (Node) -> Traversal) { - var seen = Set() - var nodesToVisit = [startingNode] - while !nodesToVisit.isEmpty { - let node = nodesToVisit.removeFirst() - guard !seen.contains(node) else { - continue - } - let children = self[node].map { - nodeWithReference($0)! - } - nodesToVisit.append(contentsOf: children) - guard case .continue = observe(node) else { - break - } - seen.insert(node) - } + /// Returns a sequence that traverses the topic graph in breadth first order from a given reference, without visiting the same node more than once. + func breadthFirstSearch(from reference: ResolvedTopicReference) -> some Sequence { + DirectedGraph(neighbors: edges) + .breadthFirstSearch(from: reference) + .lazy + .map { nodeWithReference($0)! } } /// Returns the children of this node that reference it as their overload group. diff --git a/Sources/SwiftDocC/Model/Rendering/RenderNodeTranslator.swift b/Sources/SwiftDocC/Model/Rendering/RenderNodeTranslator.swift index c1d9cdcab5..f3ae51a945 100644 --- a/Sources/SwiftDocC/Model/Rendering/RenderNodeTranslator.swift +++ b/Sources/SwiftDocC/Model/Rendering/RenderNodeTranslator.swift @@ -225,11 +225,8 @@ public struct RenderNodeTranslator: SemanticVisitor { // Get all the tutorials and tutorial articles in the learning path, ordered. var surroundingTopics = [(reference: ResolvedTopicReference, kind: DocumentationNode.Kind)]() - context.traverseBreadthFirst(from: volume) { node in - if node.kind == .tutorial || node.kind == .tutorialArticle { - surroundingTopics.append((node.reference, node.kind)) - } - return .continue + for node in context.breadthFirstSearch(from: volume) where node.kind == .tutorial || node.kind == .tutorialArticle { + surroundingTopics.append((node.reference, node.kind)) } // Find the tutorial or article that comes after the current page, if one exists. @@ -362,20 +359,19 @@ public struct RenderNodeTranslator: SemanticVisitor { private func totalEstimatedDuration(for technology: Technology) -> String? { var totalDurationMinutes: Int? = nil - context.traverseBreadthFirst(from: identifier) { node in - if let entity = try? context.entity(with: node.reference), - let durationMinutes = (entity.semantic as? Timed)?.durationMinutes - { - if totalDurationMinutes == nil { - totalDurationMinutes = 0 - } - totalDurationMinutes! += durationMinutes + for node in context.breadthFirstSearch(from: identifier) { + guard let entity = try? context.entity(with: node.reference), + let durationMinutes = (entity.semantic as? Timed)?.durationMinutes + else { + continue } - - return .continue + + if totalDurationMinutes == nil { + totalDurationMinutes = 0 + } + totalDurationMinutes! += durationMinutes } - return totalDurationMinutes.flatMap(contentRenderer.formatEstimatedDuration(minutes:)) } diff --git a/Tests/SwiftDocCTests/Infrastructure/TopicGraphTests.swift b/Tests/SwiftDocCTests/Infrastructure/TopicGraphTests.swift index 942bb611c0..1fef34094b 100644 --- a/Tests/SwiftDocCTests/Infrastructure/TopicGraphTests.swift +++ b/Tests/SwiftDocCTests/Infrastructure/TopicGraphTests.swift @@ -184,71 +184,43 @@ class TopicGraphTests: XCTestCase { func testBreadthFirstSearch() { let graph = TestGraphs.complex let A = TestGraphs.testNodeWithTitle("A") - var visited = [TopicGraph.Node]() - graph.traverseBreadthFirst(from: A) { - visited.append($0) - return .continue - } - XCTAssertEqual(["A", "B", "D", "C", "E"], - visited.map { $0.title }) + let visited = graph.breadthFirstSearch(from: A.reference).map(\.title) + XCTAssertEqual(["A", "B", "D", "C", "E"], visited) } func testBreadthFirstSearchWithCycle() { let graph = TestGraphs.withCycle let A = TestGraphs.testNodeWithTitle("A") - var visited = [TopicGraph.Node]() - graph.traverseBreadthFirst(from: A) { - visited.append($0) - return .continue - } - XCTAssertEqual(["A", "B", "C"], - visited.map { $0.title }) + let visited = graph.breadthFirstSearch(from: A.reference).map(\.title) + XCTAssertEqual(["A", "B", "C"], visited) } func testBreadthFirstSearchEarlyStop() { let graph = TestGraphs.complex let A = TestGraphs.testNodeWithTitle("A") - var visited = [TopicGraph.Node]() - graph.traverseBreadthFirst(from: A) { - visited.append($0) - return .stop - } - XCTAssertEqual(["A"], visited.map { $0.title }) + let visited = graph.breadthFirstSearch(from: A.reference).prefix(1).map(\.title) + XCTAssertEqual(["A"], visited) } func testDepthFirstSearch() { let graph = TestGraphs.complex let A = TestGraphs.testNodeWithTitle("A") - var visited = [TopicGraph.Node]() - graph.traverseDepthFirst(from: A) { - visited.append($0) - return .continue - } - XCTAssertEqual(["A", "D", "E", "B", "C"], - visited.map { $0.title }) + let visited = graph.depthFirstSearch(from: A.reference).map(\.title) + XCTAssertEqual(["A", "D", "E", "B", "C"], visited) } func testDepthFirstSearchWithCycle() { let graph = TestGraphs.withCycle let A = TestGraphs.testNodeWithTitle("A") - var visited = [TopicGraph.Node]() - graph.traverseDepthFirst(from: A) { - visited.append($0) - return .continue - } - XCTAssertEqual(["A", "B", "C"], - visited.map { $0.title }) + let visited = graph.breadthFirstSearch(from: A.reference).map(\.title) + XCTAssertEqual(["A", "B", "C"], visited) } func testDepthFirstSearchEarlyStop() { let graph = TestGraphs.complex let A = TestGraphs.testNodeWithTitle("A") - var visited = [TopicGraph.Node]() - graph.traverseDepthFirst(from: A) { - visited.append($0) - return .stop - } - XCTAssertEqual(["A"], visited.map { $0.title }) + let visited = graph.depthFirstSearch(from: A.reference).prefix(1).map(\.title) + XCTAssertEqual(["A"], visited) } func testEveryEdgeSourceHasNode() { From 07181ebd73bcdbd460c9bdd3e1641af63e2b747f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20R=C3=B6nnqvist?= Date: Tue, 23 Apr 2024 19:32:07 +0200 Subject: [PATCH 03/13] Avoid computing full paths to determine if a symbol is manually curated --- .../Infrastructure/DocumentationContext.swift | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift b/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift index 014817b8aa..3cca76122e 100644 --- a/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift +++ b/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift @@ -2279,15 +2279,7 @@ public class DocumentationContext: DocumentationContextDataProviderDelegate { /// /// - Parameter automaticallyCurated: A list of topics that have been automatically curated. func removeUnneededAutomaticCuration(_ automaticallyCurated: [(child: ResolvedTopicReference, parent: ResolvedTopicReference)]) { - for pair in automaticallyCurated { - let paths = pathsTo(pair.child) - - // Collect all current unique parents of the child. - let parents = Set(paths.map({ $0.last?.path })) - - // Check if the topic has multiple curation paths - guard parents.count > 1 else { continue } - + for pair in automaticallyCurated where parents(of: pair.child).count > 1 { // The topic has been manually curated, remove the automatic curation now. topicGraph.removeEdge(fromReference: pair.parent, toReference: pair.child) } From 4307c4e1dec1608b9a0bc1e3ea33ddc8e41e0295 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20R=C3=B6nnqvist?= Date: Wed, 24 Apr 2024 20:34:55 +0200 Subject: [PATCH 04/13] Fix an infinite recursion determining the paths to a node when there's cyclic curation rdar://126974763 --- .../DocumentationContext+Breadcrumbs.swift | 49 +++++ .../Infrastructure/DocumentationContext.swift | 49 ----- .../DocumentationContextTests.swift | 170 +++++++++++++++++- 3 files changed, 218 insertions(+), 50 deletions(-) create mode 100644 Sources/SwiftDocC/Infrastructure/DocumentationContext+Breadcrumbs.swift diff --git a/Sources/SwiftDocC/Infrastructure/DocumentationContext+Breadcrumbs.swift b/Sources/SwiftDocC/Infrastructure/DocumentationContext+Breadcrumbs.swift new file mode 100644 index 0000000000..eb7ac452b5 --- /dev/null +++ b/Sources/SwiftDocC/Infrastructure/DocumentationContext+Breadcrumbs.swift @@ -0,0 +1,49 @@ +/* + This source file is part of the Swift.org open source project + + Copyright (c) 2024 Apple Inc. and the Swift project authors + Licensed under Apache License v2.0 with Runtime Library Exception + + See https://swift.org/LICENSE.txt for license information + See https://swift.org/CONTRIBUTORS.txt for Swift project authors +*/ + +extension DocumentationContext { + /// Options to consider when producing node breadcrumbs. + struct PathOptions: OptionSet { + let rawValue: Int + + /// Prefer a technology as the canonical path over a shorter path. + static let preferTechnologyRoot = PathOptions(rawValue: 1 << 0) + } + + /// Finds all finite paths (breadcrumbs) to the given node reference. + /// + /// Each path is a list of references for the nodes traversed while walking the topic graph from a root node to, but not including, the given `reference`. + /// + /// The first path is the canonical path to the node. The other paths are sorted by increasing length (number of components). + /// + /// - Parameters: + /// - reference: The reference to build that paths to. + /// - options: Options to consider when producing node breadcrumbs. + /// - Returns: A list of finite paths to the given reference in the topic graph. + func pathsTo(_ reference: ResolvedTopicReference, options: PathOptions = []) -> [[ResolvedTopicReference]] { + return DirectedGraph(neighbors: topicGraph.reverseEdges) + .allFinitePaths(from: reference) + .map { $0.dropFirst().reversed() } + .sorted { (lhs, rhs) -> Bool in + // Order a path rooted in a technology as the canonical one. + if options.contains(.preferTechnologyRoot), let first = lhs.first { + return try! entity(with: first).semantic is Technology + } + + // If the breadcrumbs have equal amount of components + // sort alphabetically to produce stable paths order. + guard lhs.count != rhs.count else { + return lhs.map({ $0.path }).joined(separator: ",") < rhs.map({ $0.path }).joined(separator: ",") + } + // Order by the length of the breadcrumb. + return lhs.count < rhs.count + } + } +} diff --git a/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift b/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift index 3cca76122e..6fa55ef09b 100644 --- a/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift +++ b/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift @@ -2822,55 +2822,6 @@ public class DocumentationContext: DocumentationContextDataProviderDelegate { .map { $0.reference } } - /// Options to consider when producing node breadcrumbs. - struct PathOptions: OptionSet { - let rawValue: Int - - /// The node is a technology page; sort the path to a technology as canonical. - static let preferTechnologyRoot = PathOptions(rawValue: 1 << 0) - } - - /// Finds all paths (breadcrumbs) to the given node reference. - /// - /// Each path is an array of references to the symbols from the module symbol to the current one. - /// The first path in the array is always the canonical path to the symbol. - /// - /// - Parameters: - /// - reference: The reference to build that paths to. - /// - currentPathToNode: Used for recursion - an accumulated path to "continue" working on. - /// - Returns: A list of paths to the current reference in the topic graph. - func pathsTo(_ reference: ResolvedTopicReference, currentPathToNode: [ResolvedTopicReference] = [], options: PathOptions = []) -> [[ResolvedTopicReference]] { - let nodeParents = parents(of: reference) - guard !nodeParents.isEmpty else { - // The path ends with this node - return [currentPathToNode] - } - var results = [[ResolvedTopicReference]]() - for parentReference in nodeParents { - let parentPaths = pathsTo(parentReference, currentPathToNode: [parentReference] + currentPathToNode) - results.append(contentsOf: parentPaths) - } - - // We are sorting the breadcrumbs by the path distance to the documentation root - // so that the first element is the shortest path that we are using as canonical. - results.sort { (lhs, rhs) -> Bool in - // Order a path rooted in a technology as the canonical one. - if options.contains(.preferTechnologyRoot), let first = lhs.first { - return try! entity(with: first).semantic is Technology - } - - // If the breadcrumbs have equal amount of components - // sort alphabetically to produce stable paths order. - guard lhs.count != rhs.count else { - return lhs.map({ $0.path }).joined(separator: ",") < rhs.map({ $0.path }).joined(separator: ",") - } - // Order by the length of the breadcrumb. - return lhs.count < rhs.count - } - - return results - } - func dumpGraph() -> String { return topicGraph.nodes.values .filter { parents(of: $0.reference).isEmpty } diff --git a/Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContextTests.swift b/Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContextTests.swift index 0991134048..54beae9865 100644 --- a/Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContextTests.swift +++ b/Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContextTests.swift @@ -4486,6 +4486,173 @@ let expected = """ XCTAssertEqual(start.. SymbolGraph.Symbol { return SymbolGraph.Symbol( - identifier: .init(precise: identifier, interfaceLanguage: SourceLanguage.swift.id), + identifier: .init(precise: identifier, interfaceLanguage: language.id), names: .init(title: name, navigator: nil, subHeading: nil, prose: nil), pathComponents: pathComponents ?? [name], docComment: nil, From be1edf531371026a2da3fabb7f25d6354516a401 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20R=C3=B6nnqvist?= Date: Thu, 25 Apr 2024 09:08:58 +0200 Subject: [PATCH 05/13] Avoid computing all paths when the caller only needs the shortest path --- .../DocumentationContext+Breadcrumbs.swift | 38 +++++++++++++------ .../Topic Graph/AutomaticCuration.swift | 2 +- .../Model/Rendering/RenderContext.swift | 2 +- .../DocumentationContextTests.swift | 8 ++-- .../DocumentationCuratorTests.swift | 2 +- 5 files changed, 34 insertions(+), 18 deletions(-) diff --git a/Sources/SwiftDocC/Infrastructure/DocumentationContext+Breadcrumbs.swift b/Sources/SwiftDocC/Infrastructure/DocumentationContext+Breadcrumbs.swift index eb7ac452b5..d7ac4654f2 100644 --- a/Sources/SwiftDocC/Infrastructure/DocumentationContext+Breadcrumbs.swift +++ b/Sources/SwiftDocC/Infrastructure/DocumentationContext+Breadcrumbs.swift @@ -9,7 +9,7 @@ */ extension DocumentationContext { - /// Options to consider when producing node breadcrumbs. + /// Options that configure how the context produces node breadcrumbs. struct PathOptions: OptionSet { let rawValue: Int @@ -17,15 +17,15 @@ extension DocumentationContext { static let preferTechnologyRoot = PathOptions(rawValue: 1 << 0) } - /// Finds all finite paths (breadcrumbs) to the given node reference. + /// Finds all finite paths (breadcrumbs) to the given reference. /// /// Each path is a list of references for the nodes traversed while walking the topic graph from a root node to, but not including, the given `reference`. /// /// The first path is the canonical path to the node. The other paths are sorted by increasing length (number of components). /// /// - Parameters: - /// - reference: The reference to build that paths to. - /// - options: Options to consider when producing node breadcrumbs. + /// - reference: The reference to find paths to. + /// - options: Options for how the context produces node breadcrumbs. /// - Returns: A list of finite paths to the given reference in the topic graph. func pathsTo(_ reference: ResolvedTopicReference, options: PathOptions = []) -> [[ResolvedTopicReference]] { return DirectedGraph(neighbors: topicGraph.reverseEdges) @@ -37,13 +37,29 @@ extension DocumentationContext { return try! entity(with: first).semantic is Technology } - // If the breadcrumbs have equal amount of components - // sort alphabetically to produce stable paths order. - guard lhs.count != rhs.count else { - return lhs.map({ $0.path }).joined(separator: ",") < rhs.map({ $0.path }).joined(separator: ",") - } - // Order by the length of the breadcrumb. - return lhs.count < rhs.count + return breadcrumbsAreInIncreasingOrder(lhs, rhs) } } + + /// Finds the shortest finite path (breadcrumb) to the given reference. + /// + /// - Parameter reference: The reference to find the shortest path to. + /// - Returns: The shortest path to the given reference, or `nil` if all paths to the reference are infinite (contain cycles). + func shortestFinitePathTo(_ reference: ResolvedTopicReference) -> [ResolvedTopicReference]? { + return DirectedGraph(neighbors: topicGraph.reverseEdges) + .shortestFinitePaths(from: reference) + .map { $0.dropFirst().reversed() } + .min(by: breadcrumbsAreInIncreasingOrder) + } } + +/// Compares two breadcrumbs for sorting so that the breadcrumb with fewer components come first and breadcrumbs with the same number of components are sorter alphabetically. +private func breadcrumbsAreInIncreasingOrder(_ lhs: [ResolvedTopicReference], _ rhs: [ResolvedTopicReference]) -> Bool { + // If the breadcrumbs have the same number of components, sort alphabetically to produce stable results. + guard lhs.count != rhs.count else { + return lhs.map({ $0.path }).joined(separator: ",") < rhs.map({ $0.path }).joined(separator: ",") + } + // Otherwise, sort by the number of breadcrumb components. + return lhs.count < rhs.count +} + diff --git a/Sources/SwiftDocC/Infrastructure/Topic Graph/AutomaticCuration.swift b/Sources/SwiftDocC/Infrastructure/Topic Graph/AutomaticCuration.swift index 45628fd7cf..aab14197c3 100644 --- a/Sources/SwiftDocC/Infrastructure/Topic Graph/AutomaticCuration.swift +++ b/Sources/SwiftDocC/Infrastructure/Topic Graph/AutomaticCuration.swift @@ -152,7 +152,7 @@ public struct AutomaticCuration { } // First try getting the canonical path from a render context, default to the documentation context - guard let canonicalPath = renderContext?.store.content(for: node.reference)?.canonicalPath ?? context.pathsTo(node.reference).first, + guard let canonicalPath = renderContext?.store.content(for: node.reference)?.canonicalPath ?? context.shortestFinitePathTo(node.reference), let parentReference = canonicalPath.last else { // If the symbol is not curated or is a root symbol, no See Also please. diff --git a/Sources/SwiftDocC/Model/Rendering/RenderContext.swift b/Sources/SwiftDocC/Model/Rendering/RenderContext.swift index d7d957339f..bc110ba5e0 100644 --- a/Sources/SwiftDocC/Model/Rendering/RenderContext.swift +++ b/Sources/SwiftDocC/Model/Rendering/RenderContext.swift @@ -46,7 +46,7 @@ public struct RenderContext { let renderContentFor: (ResolvedTopicReference) -> RenderReferenceStore.TopicContent = { reference in var dependencies = RenderReferenceDependencies() let renderReference = renderer.renderReference(for: reference, dependencies: &dependencies) - let canonicalPath = documentationContext.pathsTo(reference).first.flatMap { $0.isEmpty ? nil : $0 } + let canonicalPath = documentationContext.shortestFinitePathTo(reference).flatMap { $0.isEmpty ? nil : $0 } let reverseLookup = renderer.taskGroups(for: reference) return RenderReferenceStore.TopicContent( diff --git a/Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContextTests.swift b/Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContextTests.swift index 54beae9865..ad2f0cd2df 100644 --- a/Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContextTests.swift +++ b/Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContextTests.swift @@ -1426,7 +1426,7 @@ let expected = """ let (cccNode, cccTgNode) = try createNode(in: context, bundle: bundle, parent: aaaNode.reference, name: "CCC") context.topicGraph.addEdge(from: bbbTgNode, to: cccTgNode) - let canonicalPathCCC = try XCTUnwrap(context.pathsTo(cccNode.reference).first) + let canonicalPathCCC = try XCTUnwrap(context.shortestFinitePathTo(cccNode.reference)) XCTAssertEqual(["/documentation/MyKit", "/documentation/MyKit/AAA"], canonicalPathCCC.map({ $0.path })) /// @@ -1440,7 +1440,7 @@ let expected = """ let (fffNode, fffTgNode) = try createNode(in: context, bundle: bundle, parent: eeeNode.reference, name: "FFF") context.topicGraph.addEdge(from: dddTgNode, to: fffTgNode) - let canonicalPathFFF = try XCTUnwrap(context.pathsTo(fffNode.reference).first) + let canonicalPathFFF = try XCTUnwrap(context.shortestFinitePathTo(fffNode.reference)) XCTAssertEqual(["/documentation/MyKit", "/documentation/MyKit/DDD"], canonicalPathFFF.map({ $0.path })) } @@ -1463,7 +1463,7 @@ let expected = """ context.topicGraph.addEdge(from: aaaTgNode, to: cccTgNode) context.topicGraph.addEdge(from: bbbTgNode, to: cccTgNode) - let canonicalPathCCC = try XCTUnwrap(context.pathsTo(cccNode.reference).first) + let canonicalPathCCC = try XCTUnwrap(context.shortestFinitePathTo(cccNode.reference)) XCTAssertEqual(["/documentation/MyKit"], canonicalPathCCC.map({ $0.path })) /// @@ -1479,7 +1479,7 @@ let expected = """ context.topicGraph.addEdge(from: dddTgNode, to: fffTgNode) context.topicGraph.addEdge(from: tgMykitNode, to: fffTgNode) - let canonicalPathFFF = try XCTUnwrap(context.pathsTo(fffNode.reference).first) + let canonicalPathFFF = try XCTUnwrap(context.shortestFinitePathTo(fffNode.reference)) XCTAssertEqual(["/documentation/MyKit"], canonicalPathFFF.map({ $0.path })) } diff --git a/Tests/SwiftDocCTests/Infrastructure/DocumentationCuratorTests.swift b/Tests/SwiftDocCTests/Infrastructure/DocumentationCuratorTests.swift index 3d33bed8cb..28f6cfeb78 100644 --- a/Tests/SwiftDocCTests/Infrastructure/DocumentationCuratorTests.swift +++ b/Tests/SwiftDocCTests/Infrastructure/DocumentationCuratorTests.swift @@ -211,7 +211,7 @@ class DocumentationCuratorTests: XCTestCase { XCTAssert(context.problems.isEmpty, "Expected no problems. Found: \(context.problems.map(\.diagnostic.summary))") guard let moduleNode = context.documentationCache["SourceLocations"], - let pathToRoot = context.pathsTo(moduleNode.reference).first, + let pathToRoot = context.shortestFinitePathTo(moduleNode.reference), let root = pathToRoot.first else { XCTFail("Module doesn't have technology root as a predecessor in its path") From 1bb242fef335206218a959821ba1e0f68c3298a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20R=C3=B6nnqvist?= Date: Thu, 25 Apr 2024 09:38:45 +0200 Subject: [PATCH 06/13] Avoid computing all paths when the caller only needs the roots/leaves --- .../DocumentationContext+Breadcrumbs.swift | 20 +++++++++++++++++-- .../Infrastructure/DocumentationCurator.swift | 5 +---- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/Sources/SwiftDocC/Infrastructure/DocumentationContext+Breadcrumbs.swift b/Sources/SwiftDocC/Infrastructure/DocumentationContext+Breadcrumbs.swift index d7ac4654f2..b0be43ad03 100644 --- a/Sources/SwiftDocC/Infrastructure/DocumentationContext+Breadcrumbs.swift +++ b/Sources/SwiftDocC/Infrastructure/DocumentationContext+Breadcrumbs.swift @@ -28,7 +28,7 @@ extension DocumentationContext { /// - options: Options for how the context produces node breadcrumbs. /// - Returns: A list of finite paths to the given reference in the topic graph. func pathsTo(_ reference: ResolvedTopicReference, options: PathOptions = []) -> [[ResolvedTopicReference]] { - return DirectedGraph(neighbors: topicGraph.reverseEdges) + reverseEdgesGraph .allFinitePaths(from: reference) .map { $0.dropFirst().reversed() } .sorted { (lhs, rhs) -> Bool in @@ -46,11 +46,27 @@ extension DocumentationContext { /// - Parameter reference: The reference to find the shortest path to. /// - Returns: The shortest path to the given reference, or `nil` if all paths to the reference are infinite (contain cycles). func shortestFinitePathTo(_ reference: ResolvedTopicReference) -> [ResolvedTopicReference]? { - return DirectedGraph(neighbors: topicGraph.reverseEdges) + reverseEdgesGraph .shortestFinitePaths(from: reference) .map { $0.dropFirst().reversed() } .min(by: breadcrumbsAreInIncreasingOrder) } + + /// Finds all the reachable root node references from the given reference. + /// + /// > Note: + /// If all paths from the given reference are infinite (contain cycles) then it can't reach any roots and will return an empty set. + /// + /// - Parameter reference: The reference to find reachable root node references from. + /// - Returns: The references of the root nodes that are reachable fro the given reference, or `[]` if all paths from the reference are infinite (contain cycles). + func reachableRoots(from reference: ResolvedTopicReference) -> Set { + reverseEdgesGraph.reachableLeafNodes(from: reference) + } + + /// The directed graph of reverse edges in the topic graph. + private var reverseEdgesGraph: DirectedGraph { + DirectedGraph(neighbors: topicGraph.reverseEdges) + } } /// Compares two breadcrumbs for sorting so that the breadcrumb with fewer components come first and breadcrumbs with the same number of components are sorter alphabetically. diff --git a/Sources/SwiftDocC/Infrastructure/DocumentationCurator.swift b/Sources/SwiftDocC/Infrastructure/DocumentationCurator.swift index dfee2676cb..3b88c8103b 100644 --- a/Sources/SwiftDocC/Infrastructure/DocumentationCurator.swift +++ b/Sources/SwiftDocC/Infrastructure/DocumentationCurator.swift @@ -237,10 +237,7 @@ struct DocumentationCurator { return node.kind == .module && documentationNode.kind.isSymbol == false } - let hasTechnologyRoot = isTechnologyRoot(nodeReference) || context.pathsTo(nodeReference).contains { path in - guard let root = path.first else { return false } - return isTechnologyRoot(root) - } + let hasTechnologyRoot = isTechnologyRoot(nodeReference) || context.reachableRoots(from: nodeReference).contains(where: isTechnologyRoot) if !hasTechnologyRoot { problems.append(Problem(diagnostic: Diagnostic(source: source(), severity: .warning, range: range(), identifier: "org.swift.docc.ModuleCuration", summary: "Linking to \((link.destination ?? "").singleQuoted) from a Topics group in \(nodeReference.absoluteString.singleQuoted) isn't allowed", explanation: "The former is a module, and modules only exist at the root"), possibleSolutions: [])) From 598abf3007c3d074bde8ff6f972b41ff9c3855ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20R=C3=B6nnqvist?= Date: Thu, 25 Apr 2024 09:43:55 +0200 Subject: [PATCH 07/13] Avoid computing all paths when the caller only needs know if a certain node is encountered --- Sources/SwiftDocC/Infrastructure/DocumentationCurator.swift | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Sources/SwiftDocC/Infrastructure/DocumentationCurator.swift b/Sources/SwiftDocC/Infrastructure/DocumentationCurator.swift index 3b88c8103b..5a2125becd 100644 --- a/Sources/SwiftDocC/Infrastructure/DocumentationCurator.swift +++ b/Sources/SwiftDocC/Infrastructure/DocumentationCurator.swift @@ -127,7 +127,9 @@ struct DocumentationCurator { } private func isReference(_ childReference: ResolvedTopicReference, anAncestorOf nodeReference: ResolvedTopicReference) -> Bool { - return context.pathsTo(nodeReference).contains { $0.contains(childReference) } + DirectedGraph(neighbors: context.topicGraph.reverseEdges) + .breadthFirstSearch(from: nodeReference) + .contains(childReference) } /// Crawls the topic graph starting at a given root node, curates articles during. From 4572d0fcc33403820a6df4f09be2926eb66d4d6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20R=C3=B6nnqvist?= Date: Thu, 25 Apr 2024 09:40:51 +0200 Subject: [PATCH 08/13] Avoid computing all paths when the caller only needs to visit each reachable node once --- .../Model/Rendering/RenderNodeTranslator.swift | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/Sources/SwiftDocC/Model/Rendering/RenderNodeTranslator.swift b/Sources/SwiftDocC/Model/Rendering/RenderNodeTranslator.swift index f3ae51a945..d6a444f981 100644 --- a/Sources/SwiftDocC/Model/Rendering/RenderNodeTranslator.swift +++ b/Sources/SwiftDocC/Model/Rendering/RenderNodeTranslator.swift @@ -604,13 +604,14 @@ public struct RenderNodeTranslator: SemanticVisitor { node.metadata.title = article.title!.plainText // Detect the article modules from its breadcrumbs. - let modules = context.pathsTo(identifier).compactMap({ path -> ResolvedTopicReference? in - return path.mapFirst(where: { ancestor in - guard let ancestorNode = try? context.entity(with: ancestor) else { return nil } - return (ancestorNode.semantic as? Symbol)?.moduleReference - }) - }) - let moduleNames = Set(modules).compactMap { reference -> String? in + var modules = Set() + for reference in DirectedGraph(neighbors: context.topicGraph.reverseEdges).breadthFirstSearch(from: identifier) { + if let moduleReference = (try? context.entity(with: reference).semantic as? Symbol)?.moduleReference { + modules.insert(moduleReference) + } + } + + let moduleNames = modules.compactMap { reference -> String? in guard let node = try? context.entity(with: reference) else { return nil } switch node.name { case .conceptual(let title): From 6bac02325bd5f12f5df1dda34f36ac86c2264f47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20R=C3=B6nnqvist?= Date: Thu, 25 Apr 2024 09:46:04 +0200 Subject: [PATCH 09/13] Rename 'pathsTo(_:options:)' to 'finitePaths(to:options:) --- .../GeneratedCurationWriter.swift | 2 +- .../DocumentationContext+Breadcrumbs.swift | 4 ++-- .../Topic Graph/AutomaticCuration.swift | 2 +- .../RenderHierarchyTranslator.swift | 4 ++-- .../Model/Rendering/RenderContext.swift | 2 +- .../Model/Rendering/RenderNodeTranslator.swift | 4 ++-- .../DocumentationContextTests.swift | 16 ++++++++-------- .../DocumentationCuratorTests.swift | 10 +++++----- 8 files changed, 22 insertions(+), 22 deletions(-) diff --git a/Sources/SwiftDocC/Catalog Processing/GeneratedCurationWriter.swift b/Sources/SwiftDocC/Catalog Processing/GeneratedCurationWriter.swift index fc0f826714..a2e296015b 100644 --- a/Sources/SwiftDocC/Catalog Processing/GeneratedCurationWriter.swift +++ b/Sources/SwiftDocC/Catalog Processing/GeneratedCurationWriter.swift @@ -133,7 +133,7 @@ public struct GeneratedCurationWriter { for (usr, reference) in context.documentationCache.referencesBySymbolID { // Filter out symbols that aren't in the specified sub hierarchy. if symbolLink != nil || depthLimit != nil { - guard reference == curationCrawlRoot || context.pathsTo(reference).contains(where: { path in path.suffix(depthLimit ?? .max).contains(curationCrawlRoot)}) else { + guard reference == curationCrawlRoot || context.finitePaths(to: reference).contains(where: { path in path.suffix(depthLimit ?? .max).contains(curationCrawlRoot)}) else { continue } } diff --git a/Sources/SwiftDocC/Infrastructure/DocumentationContext+Breadcrumbs.swift b/Sources/SwiftDocC/Infrastructure/DocumentationContext+Breadcrumbs.swift index b0be43ad03..f09ba97c2b 100644 --- a/Sources/SwiftDocC/Infrastructure/DocumentationContext+Breadcrumbs.swift +++ b/Sources/SwiftDocC/Infrastructure/DocumentationContext+Breadcrumbs.swift @@ -27,7 +27,7 @@ extension DocumentationContext { /// - reference: The reference to find paths to. /// - options: Options for how the context produces node breadcrumbs. /// - Returns: A list of finite paths to the given reference in the topic graph. - func pathsTo(_ reference: ResolvedTopicReference, options: PathOptions = []) -> [[ResolvedTopicReference]] { + func finitePaths(to reference: ResolvedTopicReference, options: PathOptions = []) -> [[ResolvedTopicReference]] { reverseEdgesGraph .allFinitePaths(from: reference) .map { $0.dropFirst().reversed() } @@ -45,7 +45,7 @@ extension DocumentationContext { /// /// - Parameter reference: The reference to find the shortest path to. /// - Returns: The shortest path to the given reference, or `nil` if all paths to the reference are infinite (contain cycles). - func shortestFinitePathTo(_ reference: ResolvedTopicReference) -> [ResolvedTopicReference]? { + func shortestFinitePath(to reference: ResolvedTopicReference) -> [ResolvedTopicReference]? { reverseEdgesGraph .shortestFinitePaths(from: reference) .map { $0.dropFirst().reversed() } diff --git a/Sources/SwiftDocC/Infrastructure/Topic Graph/AutomaticCuration.swift b/Sources/SwiftDocC/Infrastructure/Topic Graph/AutomaticCuration.swift index aab14197c3..07d613bf65 100644 --- a/Sources/SwiftDocC/Infrastructure/Topic Graph/AutomaticCuration.swift +++ b/Sources/SwiftDocC/Infrastructure/Topic Graph/AutomaticCuration.swift @@ -152,7 +152,7 @@ public struct AutomaticCuration { } // First try getting the canonical path from a render context, default to the documentation context - guard let canonicalPath = renderContext?.store.content(for: node.reference)?.canonicalPath ?? context.shortestFinitePathTo(node.reference), + guard let canonicalPath = renderContext?.store.content(for: node.reference)?.canonicalPath ?? context.shortestFinitePath(to: node.reference), let parentReference = canonicalPath.last else { // If the symbol is not curated or is a root symbol, no See Also please. diff --git a/Sources/SwiftDocC/Model/Rendering/Navigation Tree/RenderHierarchyTranslator.swift b/Sources/SwiftDocC/Model/Rendering/Navigation Tree/RenderHierarchyTranslator.swift index 25a9c28311..f86eaa0e73 100644 --- a/Sources/SwiftDocC/Model/Rendering/Navigation Tree/RenderHierarchyTranslator.swift +++ b/Sources/SwiftDocC/Model/Rendering/Navigation Tree/RenderHierarchyTranslator.swift @@ -37,7 +37,7 @@ struct RenderHierarchyTranslator { /// - omittingChapters: If `true`, don't include chapters in the returned hierarchy. /// - Returns: A tuple of 1) a tutorials hierarchy and 2) the root reference of the tutorials hierarchy. mutating func visitTechnologyNode(_ reference: ResolvedTopicReference, omittingChapters: Bool = false) -> (hierarchy: RenderHierarchy, technology: ResolvedTopicReference)? { - let paths = context.pathsTo(reference, options: [.preferTechnologyRoot]) + let paths = context.finitePaths(to: reference, options: [.preferTechnologyRoot]) // If the node is a technology return immediately without generating breadcrumbs if let _ = (try? context.entity(with: reference))?.semantic as? Technology { @@ -196,7 +196,7 @@ struct RenderHierarchyTranslator { /// multiple times under other API symbols, articles, or API collections. This method /// returns all the paths (breadcrumbs) between the framework landing page and the given symbol. mutating func visitSymbol(_ symbolReference: ResolvedTopicReference) -> RenderHierarchy { - let pathReferences = context.pathsTo(symbolReference) + let pathReferences = context.finitePaths(to: symbolReference) pathReferences.forEach({ collectedTopicReferences.formUnion($0) }) diff --git a/Sources/SwiftDocC/Model/Rendering/RenderContext.swift b/Sources/SwiftDocC/Model/Rendering/RenderContext.swift index bc110ba5e0..3dd3326f3f 100644 --- a/Sources/SwiftDocC/Model/Rendering/RenderContext.swift +++ b/Sources/SwiftDocC/Model/Rendering/RenderContext.swift @@ -46,7 +46,7 @@ public struct RenderContext { let renderContentFor: (ResolvedTopicReference) -> RenderReferenceStore.TopicContent = { reference in var dependencies = RenderReferenceDependencies() let renderReference = renderer.renderReference(for: reference, dependencies: &dependencies) - let canonicalPath = documentationContext.shortestFinitePathTo(reference).flatMap { $0.isEmpty ? nil : $0 } + let canonicalPath = documentationContext.shortestFinitePath(to: reference).flatMap { $0.isEmpty ? nil : $0 } let reverseLookup = renderer.taskGroups(for: reference) return RenderReferenceStore.TopicContent( diff --git a/Sources/SwiftDocC/Model/Rendering/RenderNodeTranslator.swift b/Sources/SwiftDocC/Model/Rendering/RenderNodeTranslator.swift index d6a444f981..04fabfb701 100644 --- a/Sources/SwiftDocC/Model/Rendering/RenderNodeTranslator.swift +++ b/Sources/SwiftDocC/Model/Rendering/RenderNodeTranslator.swift @@ -198,7 +198,7 @@ public struct RenderNodeTranslator: SemanticVisitor { // We guarantee there will be at least 1 path with at least 4 nodes in that path if the tutorial is curated. // The way to curate tutorials is to link them from a Technology page and that generates the following hierarchy: // technology -> volume -> chapter -> tutorial. - let technologyPath = context.pathsTo(identifier, options: [.preferTechnologyRoot])[0] + let technologyPath = context.finitePaths(to: identifier, options: [.preferTechnologyRoot])[0] if technologyPath.count >= 2 { let volume = technologyPath[technologyPath.count - 2] @@ -920,7 +920,7 @@ public struct RenderNodeTranslator: SemanticVisitor { } // Guaranteed to have at least one path - let technologyPath = context.pathsTo(identifier, options: [.preferTechnologyRoot])[0] + let technologyPath = context.finitePaths(to: identifier, options: [.preferTechnologyRoot])[0] node.sections.append(intro) diff --git a/Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContextTests.swift b/Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContextTests.swift index ad2f0cd2df..33ffa82d86 100644 --- a/Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContextTests.swift +++ b/Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContextTests.swift @@ -1390,10 +1390,10 @@ let expected = """ assertEqualDumps(context.dumpGraph(), expected) // Test correct symbol hierarchy in context - XCTAssertEqual(context.pathsTo(ResolvedTopicReference(bundleIdentifier: "org.swift.docc.example", path: "/documentation/MyKit/MyClass", sourceLanguage: .swift)).map { $0.map {$0.absoluteString} }, + XCTAssertEqual(context.finitePaths(to: ResolvedTopicReference(bundleIdentifier: "org.swift.docc.example", path: "/documentation/MyKit/MyClass", sourceLanguage: .swift)).map { $0.map {$0.absoluteString} }, [["doc://org.swift.docc.example/documentation/MyKit"], ["doc://org.swift.docc.example/documentation/MyKit", "doc://org.swift.docc.example/documentation/MyKit/MyProtocol"]]) - XCTAssertEqual(context.pathsTo(ResolvedTopicReference(bundleIdentifier: "org.swift.docc.example", path: "/documentation/MyKit/MyClass/init()-33vaw", sourceLanguage: .swift)).map { $0.map {$0.absoluteString} }, + XCTAssertEqual(context.finitePaths(to: ResolvedTopicReference(bundleIdentifier: "org.swift.docc.example", path: "/documentation/MyKit/MyClass/init()-33vaw", sourceLanguage: .swift)).map { $0.map {$0.absoluteString} }, [["doc://org.swift.docc.example/documentation/MyKit", "doc://org.swift.docc.example/documentation/MyKit/MyClass"], ["doc://org.swift.docc.example/documentation/MyKit", "doc://org.swift.docc.example/documentation/MyKit/MyProtocol", "doc://org.swift.docc.example/documentation/MyKit/MyClass"]]) } @@ -1426,7 +1426,7 @@ let expected = """ let (cccNode, cccTgNode) = try createNode(in: context, bundle: bundle, parent: aaaNode.reference, name: "CCC") context.topicGraph.addEdge(from: bbbTgNode, to: cccTgNode) - let canonicalPathCCC = try XCTUnwrap(context.shortestFinitePathTo(cccNode.reference)) + let canonicalPathCCC = try XCTUnwrap(context.shortestFinitePath(to: cccNode.reference)) XCTAssertEqual(["/documentation/MyKit", "/documentation/MyKit/AAA"], canonicalPathCCC.map({ $0.path })) /// @@ -1440,7 +1440,7 @@ let expected = """ let (fffNode, fffTgNode) = try createNode(in: context, bundle: bundle, parent: eeeNode.reference, name: "FFF") context.topicGraph.addEdge(from: dddTgNode, to: fffTgNode) - let canonicalPathFFF = try XCTUnwrap(context.shortestFinitePathTo(fffNode.reference)) + let canonicalPathFFF = try XCTUnwrap(context.shortestFinitePath(to: fffNode.reference)) XCTAssertEqual(["/documentation/MyKit", "/documentation/MyKit/DDD"], canonicalPathFFF.map({ $0.path })) } @@ -1463,7 +1463,7 @@ let expected = """ context.topicGraph.addEdge(from: aaaTgNode, to: cccTgNode) context.topicGraph.addEdge(from: bbbTgNode, to: cccTgNode) - let canonicalPathCCC = try XCTUnwrap(context.shortestFinitePathTo(cccNode.reference)) + let canonicalPathCCC = try XCTUnwrap(context.shortestFinitePath(to: cccNode.reference)) XCTAssertEqual(["/documentation/MyKit"], canonicalPathCCC.map({ $0.path })) /// @@ -1479,7 +1479,7 @@ let expected = """ context.topicGraph.addEdge(from: dddTgNode, to: fffTgNode) context.topicGraph.addEdge(from: tgMykitNode, to: fffTgNode) - let canonicalPathFFF = try XCTUnwrap(context.shortestFinitePathTo(fffNode.reference)) + let canonicalPathFFF = try XCTUnwrap(context.shortestFinitePath(to: fffNode.reference)) XCTAssertEqual(["/documentation/MyKit"], canonicalPathFFF.map({ $0.path })) } @@ -1519,7 +1519,7 @@ let expected = """ let node = try context.entity(with: ResolvedTopicReference(bundleIdentifier: "org.swift.docc.example", path: "/tutorials/Test-Bundle/TestTutorial", sourceLanguage: .swift)) // Get the breadcrumbs as paths - let paths = context.pathsTo(node.reference).sorted { (path1, path2) -> Bool in + let paths = context.finitePaths(to: node.reference).sorted { (path1, path2) -> Bool in return path1.count < path2.count } .map { return $0.map { $0.url.path } } @@ -4647,7 +4647,7 @@ let expected = """ ) XCTAssertEqual( - context.pathsTo(reference).map { $0.map(\.lastPathComponent) }, + context.finitePaths(to: reference).map { $0.map(\.lastPathComponent) }, [ ["ModuleName", "Code-swift.enum"] ], "There is only one _finite_ path from the 'someCase' enum case, through the reverse edges in the topic graph." ) diff --git a/Tests/SwiftDocCTests/Infrastructure/DocumentationCuratorTests.swift b/Tests/SwiftDocCTests/Infrastructure/DocumentationCuratorTests.swift index 28f6cfeb78..8d26f300a0 100644 --- a/Tests/SwiftDocCTests/Infrastructure/DocumentationCuratorTests.swift +++ b/Tests/SwiftDocCTests/Infrastructure/DocumentationCuratorTests.swift @@ -167,7 +167,7 @@ class DocumentationCuratorTests: XCTestCase { XCTAssert(context.problems.isEmpty, "Expected no problems. Found: \(context.problems.map(\.diagnostic.summary))") guard let moduleNode = context.documentationCache["SourceLocations"], - let pathToRoot = context.pathsTo(moduleNode.reference).first, + let pathToRoot = context.finitePaths(to: moduleNode.reference).first, let root = pathToRoot.first else { XCTFail("Module doesn't have technology root as a predecessor in its path") @@ -211,7 +211,7 @@ class DocumentationCuratorTests: XCTestCase { XCTAssert(context.problems.isEmpty, "Expected no problems. Found: \(context.problems.map(\.diagnostic.summary))") guard let moduleNode = context.documentationCache["SourceLocations"], - let pathToRoot = context.shortestFinitePathTo(moduleNode.reference), + let pathToRoot = context.shortestFinitePath(to: moduleNode.reference), let root = pathToRoot.first else { XCTFail("Module doesn't have technology root as a predecessor in its path") @@ -453,14 +453,14 @@ class DocumentationCuratorTests: XCTestCase { // Verify that the ONLY curation for `TopClass/name` is the manual curation under `MyArticle` // and the automatic curation under `TopClass` is not present. let nameReference = ResolvedTopicReference(bundleIdentifier: bundle.identifier, path: "/documentation/TestBed/TopClass/name", sourceLanguage: .swift) - XCTAssertEqual(context.pathsTo(nameReference).map({ $0.map(\.path) }), [ + XCTAssertEqual(context.finitePaths(to: nameReference).map({ $0.map(\.path) }), [ ["/documentation/TestBed", "/documentation/TestBed/TopClass", "/documentation/TestBed/TopClass/NestedEnum", "/documentation/TestBed/TopClass/NestedEnum/SecondLevelNesting", "/documentation/TestBed/MyArticle"], ]) // Verify that the BOTH manual curations for `TopClass/age` are preserved // even if one of the manual curations overlaps with the inheritance edge from the symbol graph. let ageReference = ResolvedTopicReference(bundleIdentifier: bundle.identifier, path: "/documentation/TestBed/TopClass/age", sourceLanguage: .swift) - XCTAssertEqual(context.pathsTo(ageReference).map({ $0.map(\.path) }), [ + XCTAssertEqual(context.finitePaths(to: ageReference).map({ $0.map(\.path) }), [ ["/documentation/TestBed", "/documentation/TestBed/TopClass"], ["/documentation/TestBed", "/documentation/TestBed/TopClass", "/documentation/TestBed/TopClass/NestedEnum", "/documentation/TestBed/TopClass/NestedEnum/SecondLevelNesting", "/documentation/TestBed/MyArticle"], ]) @@ -473,7 +473,7 @@ class DocumentationCuratorTests: XCTestCase { let reference = ResolvedTopicReference(bundleIdentifier: bundle.identifier, path: "/documentation/TestBed/DoublyManuallyCuratedClass/type()", sourceLanguage: .swift) - XCTAssertEqual(context.pathsTo(reference).map({ $0.map({ $0.path }) }), [ + XCTAssertEqual(context.finitePaths(to: reference).map({ $0.map({ $0.path }) }), [ [ "/documentation/TestBed", "/documentation/TestBed/TopClass", From 82cc80606b2881575d28fe16d15005bd545f877d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20R=C3=B6nnqvist?= Date: Thu, 18 Apr 2024 14:37:22 +0200 Subject: [PATCH 10/13] Fix another infinite recursion when pages are curated in cycles rdar://126974763 --- .../Indexing/Navigator/NavigatorIndex.swift | 3 +- .../Indexing/Navigator/NavigatorTree.swift | 50 ++++- .../Indexing/NavigatorIndexTests.swift | 177 ++++++++++++++++++ 3 files changed, 220 insertions(+), 10 deletions(-) diff --git a/Sources/SwiftDocC/Indexing/Navigator/NavigatorIndex.swift b/Sources/SwiftDocC/Indexing/Navigator/NavigatorIndex.swift index f7e7f8910b..076254cae0 100644 --- a/Sources/SwiftDocC/Indexing/Navigator/NavigatorIndex.swift +++ b/Sources/SwiftDocC/Indexing/Navigator/NavigatorIndex.swift @@ -255,8 +255,7 @@ public class NavigatorIndex { } /// Indicates the page type of a given item inside the tree. - /// - Note: This information is stored as UInt8 to decrease the required size to store it and make - /// the comparision faster between types. + /// - Note: This information is stored as `UInt8` to decrease the required size to store it and make the comparison faster between types. public enum PageType: UInt8 { case root = 0 case article = 1 diff --git a/Sources/SwiftDocC/Indexing/Navigator/NavigatorTree.swift b/Sources/SwiftDocC/Indexing/Navigator/NavigatorTree.swift index 118f61035c..95a5bd7a12 100644 --- a/Sources/SwiftDocC/Indexing/Navigator/NavigatorTree.swift +++ b/Sources/SwiftDocC/Indexing/Navigator/NavigatorTree.swift @@ -480,20 +480,54 @@ public class NavigatorTree { /// Copy the current node and children to new instances preserving the node item. public func copy() -> NavigatorTree.Node { + // DocC detects cyclic curation and avoids adding it to the topic graph. + // However, the navigator is based on the render node's topic sections which still has the cycle. + // + // Because the navigator nodes are shared instances, the cycle needs to be broken at each occurrence to correctly unroll it. + // For example, consider this hierarchy: + // + // 1 2 + // │ │ + // ▼ ▼ + // 3 ────▶ 4 + // ▲ │ + // └── 5 ◀─┘ + // + // When approaching the cycle from `1`, it unrolls as `1, 3, 4, 5` but when approaching it from `2` it unrolls as `2, 4, 5, 3`. + // This ensures a consistent structure for the navigation tree, no matter which render node was indexed first. + // + // Alternatively we could process the entire graph and break the cycle based on which node has the shortest path from the root. + // However, because DocC already warns about the cyclic curation, it seem sufficient to create a consistent navigation tree, + // even if it's not as small as it could be if we were using a more sophisticated graph algorithm. + return _copy([]) } - /// Private version of the logic to copy the current node and children to new instances preserving the node item. - /// - Parameter hierarchy: The set containing the parent items to avoid entering in an infinite loop. - private func _copy(_ hierarchy: Set) -> NavigatorTree.Node { + /// The private copy implementation which tracks the already encountered items to avoid an infinite recursion. + private func _copy(_ seen: Set) -> NavigatorTree.Node { let mirror = NavigatorTree.Node(item: item, bundleIdentifier: bundleIdentifier) - guard !hierarchy.contains(item) else { return mirror } // Avoid to enter in an infity loop. - var updatedHierarchy = hierarchy - if let parentItem = parent?.item { updatedHierarchy.insert(parentItem) } - children.forEach { (child) in - let childCopy = child._copy(updatedHierarchy) + guard !seen.contains(item) else { return mirror } // Avoid an infinite recursion. + var seen = seen + seen.insert(item) + + // Avoid repeating children that have already occurred in this path through the navigation hierarchy. + let childrenNotAlreadyEncountered = children.filter { !seen.contains($0.item) } + for (child, indexAfter) in zip(childrenNotAlreadyEncountered, 1...) { + // If the child is a group marker, ensure that the group is not empty by checking that it's followed by a non-group. + // If we didn't do this, we could end up with empty groups if all the children had already been encountered higher up in the hierarchy. + if child.item.pageType == NavigatorIndex.PageType.groupMarker.rawValue { + guard indexAfter < childrenNotAlreadyEncountered.endIndex, + childrenNotAlreadyEncountered[indexAfter].item.pageType != NavigatorIndex.PageType.groupMarker.rawValue + else { + // This group is empty. Skip copying it. + continue + } + // This group is not empty. Include a copy of it + } + let childCopy = child._copy(seen) mirror.add(child: childCopy) } + return mirror } diff --git a/Tests/SwiftDocCTests/Indexing/NavigatorIndexTests.swift b/Tests/SwiftDocCTests/Indexing/NavigatorIndexTests.swift index 6ca8a8bfeb..4685a7bb39 100644 --- a/Tests/SwiftDocCTests/Indexing/NavigatorIndexTests.swift +++ b/Tests/SwiftDocCTests/Indexing/NavigatorIndexTests.swift @@ -10,6 +10,7 @@ import XCTest @testable import SwiftDocC +import SwiftDocCTestUtilities typealias Node = NavigatorTree.Node typealias PageType = NavigatorIndex.PageType @@ -460,6 +461,182 @@ Root assertEqualDumps(results.first ?? "", try testTree(named: "testNavigatorIndexGeneration")) } + func testNavigatorIndexGenerationWithCyclicCuration() throws { + // This is a documentation hierarchy where every page exist in more than one place in the navigator, + // through a mix of automatic and manual curation, with a cycle between the two "leaf" nodes: + // + // ModuleName ─────┐ + // │ ▼ + // │ API Collection + // │ │ + // ┌─────┴────────┐ │ + // │┌─────────────┼┬┘ + // ▼▼ ▼▼ + // Container ──▶ OtherSymbol + // │ │ + // ├────────────┐│ + // ▼ ▼▼ + // first() ◀──▶ second() + let exampleDocumentation = Folder(name: "unit-test.docc", content: [ + InfoPlist(identifier: testBundleIdentifier), + + JSONFile(name: "ModuleName.symbols.json", content: makeSymbolGraph( + moduleName: "ModuleName", + symbols: [ + .init( + identifier: .init(precise: "some-container-symbol-id", interfaceLanguage: SourceLanguage.swift.id), + names: .init(title: "Container", navigator: [.init(kind: .identifier, spelling: "Container", preciseIdentifier: nil)], subHeading: nil, prose: nil), + pathComponents: ["Container"], + docComment: nil, + accessLevel: .public, + kind: .init(parsedIdentifier: .class, displayName: "Kind Display Name"), + mixins: [:] + ), + + .init( + identifier: .init(precise: "some-other-symbol-id", interfaceLanguage: SourceLanguage.swift.id), + names: .init(title: "OtherSymbol", navigator: [.init(kind: .identifier, spelling: "OtherSymbol", preciseIdentifier: nil)], subHeading: nil, prose: nil), + pathComponents: ["OtherSymbol"], + docComment: nil, + accessLevel: .public, + kind: .init(parsedIdentifier: .class, displayName: "Kind Display Name"), + mixins: [:] + ), + + .init( + identifier: .init(precise: "first-member-symbol-id", interfaceLanguage: SourceLanguage.swift.id), + names: .init(title: "first()", navigator: [.init(kind: .identifier, spelling: "first()", preciseIdentifier: nil)], subHeading: nil, prose: nil), + pathComponents: ["Container", "first()"], + docComment: nil, + accessLevel: .public, + kind: .init(parsedIdentifier: .method, displayName: "Kind Display Name"), + mixins: [:] + ), + + .init( + identifier: .init(precise: "second-member-symbol-id", interfaceLanguage: SourceLanguage.swift.id), + names: .init(title: "second()", navigator: [.init(kind: .identifier, spelling: "second()", preciseIdentifier: nil)], subHeading: nil, prose: nil), + pathComponents: ["Container", "second()"], + docComment: nil, + accessLevel: .public, + kind: .init(parsedIdentifier: .method, displayName: "Kind Display Name"), + mixins: [:] + ), + ], relationships: [ + .init(source: "some-container-symbol-id", target: "first-member-symbol-id", kind: .memberOf, targetFallback: nil), + .init(source: "some-container-symbol-id", target: "second-member-symbol-id", kind: .memberOf, targetFallback: nil), + ]) + ), + + TextFile(name: "Container.md", utf8Content: """ + # ``Container`` + + The container curates one of the members and the other symbol + + ## Topics + ### Manual curation + - ``first()`` + - ``OtherSymbol`` + """), + + TextFile(name: "OtherSymbol.md", utf8Content: """ + # ``OtherSymbol`` + + The other symbol curates the other member + + ## Topics + ### Manual curation + - ``Container/second()`` + """), + + + TextFile(name: "first.md", utf8Content: """ + # ``Container/first()`` + + Both members curate each other + + ## Topics + ### Manual curation + - ``second()`` + """), + + TextFile(name: "second.md", utf8Content: """ + # ``Container/second()`` + + Both members curate each other + + ## Topics + ### Manual curation + - ``first()`` + """), + + TextFile(name: "API-Collection.md", utf8Content: """ + # An API collection + + The API collection curates both top-level symbols + + ## Topics + ### Manual curation + - ``Container`` + - ``OtherSymbol`` + """), + + TextFile(name: "Module.md", utf8Content: """ + # ``ModuleName`` + + The module curates the API collection + + ## Topics + ### Manual curation + - + """), + ]) + + let tempURL = try createTempFolder(content: [exampleDocumentation]) + let (_, bundle, context) = try loadBundle(from: tempURL) + + let renderContext = RenderContext(documentationContext: context, bundle: bundle) + let converter = DocumentationContextConverter(bundle: bundle, context: context, renderContext: renderContext) + + let targetURL = try createTemporaryDirectory() + let builder = NavigatorIndex.Builder(outputURL: targetURL, bundleIdentifier: testBundleIdentifier) + builder.setup() + + for identifier in context.knownPages { + let source = context.documentURL(for: identifier) + let entity = try context.entity(with: identifier) + let renderNode = try XCTUnwrap(converter.renderNode(for: entity, at: source)) + try builder.index(renderNode: renderNode) + } + + builder.finalize() + + let navigatorIndex = try XCTUnwrap(builder.navigatorIndex) + + assertEqualDumps(navigatorIndex.navigatorTree.root.dumpTree(), """ + [Root] + ┗╸ModuleName + ┣╸Manual curation + ┗╸An API collection + ┣╸Manual curation + ┣╸Container + ┃ ┣╸Manual curation + ┃ ┣╸first() + ┃ ┃ ┣╸Manual curation + ┃ ┃ ┗╸second() + ┃ ┗╸OtherSymbol + ┃ ┣╸Manual curation + ┃ ┗╸second() + ┃ ┣╸Manual curation + ┃ ┗╸first() + ┗╸OtherSymbol + ┣╸Manual curation + ┗╸second() + ┣╸Manual curation + ┗╸first() + """) + } + func testNavigatorIndexGenerationVariantsPayload() throws { let jsonFile = Bundle.module.url(forResource: "Variant-render-node", withExtension: "json", subdirectory: "Test Resources")! let jsonData = try Data(contentsOf: jsonFile) From 7ecdc102c9473f02b7669833415e6a22bbfe7063 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20R=C3=B6nnqvist?= Date: Wed, 24 Apr 2024 09:16:16 +0200 Subject: [PATCH 11/13] Fix correctness issue where symbol has two auto curated parents --- .../Infrastructure/DocumentationContext.swift | 50 ++++++++++++++----- .../PathHierarchyBasedLinkResolver.swift | 18 ++++--- 2 files changed, 50 insertions(+), 18 deletions(-) diff --git a/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift b/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift index 6fa55ef09b..d8b4122a2d 100644 --- a/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift +++ b/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift @@ -2218,7 +2218,7 @@ public class DocumentationContext: DocumentationContextDataProviderDelegate { let automaticallyCurated = autoCurateSymbolsInTopicGraph() // Crawl the rest of the symbols that haven't been crawled so far in hierarchy pre-order. - allCuratedReferences = try crawlSymbolCuration(in: automaticallyCurated.map(\.child), bundle: bundle, initial: allCuratedReferences) + allCuratedReferences = try crawlSymbolCuration(in: automaticallyCurated.map(\.symbol), bundle: bundle, initial: allCuratedReferences) // Remove curation paths that have been created automatically above // but we've found manual curation for in the second crawl pass. @@ -2277,11 +2277,18 @@ public class DocumentationContext: DocumentationContextDataProviderDelegate { /// call `removeUnneededAutomaticCuration(_:)` which walks the list of automatic curations and removes /// the parent <-> child topic graph relationships that have been obsoleted. /// - /// - Parameter automaticallyCurated: A list of topics that have been automatically curated. - func removeUnneededAutomaticCuration(_ automaticallyCurated: [(child: ResolvedTopicReference, parent: ResolvedTopicReference)]) { - for pair in automaticallyCurated where parents(of: pair.child).count > 1 { - // The topic has been manually curated, remove the automatic curation now. - topicGraph.removeEdge(fromReference: pair.parent, toReference: pair.child) + /// - Parameter automaticallyCurated: A list of automatic curation records. + func removeUnneededAutomaticCuration(_ automaticallyCurated: [AutoCuratedSymbolRecord]) { + // It might look like it would be correct to check `topicGraph.nodes[symbol]?.isManuallyCurated` here, + // but that would incorrectly remove the only parent if the manual curation and the automatic curation was the same. + // + // Similarly, it might look like it would be correct to only check `parents(of: symbol).count > 1` here, + // but that would incorrectly remove the automatic curation for symbols with different language representations with different parents. + for (symbol, parent, counterpartParent) in automaticallyCurated where parents(of: symbol).count > (counterpartParent != nil ? 2 : 1) { + topicGraph.removeEdge(fromReference: parent, toReference: symbol) + if let counterpartParent { + topicGraph.removeEdge(fromReference: counterpartParent, toReference: symbol) + } } } @@ -2322,20 +2329,39 @@ public class DocumentationContext: DocumentationContextDataProviderDelegate { } } + typealias AutoCuratedSymbolRecord = (symbol: ResolvedTopicReference, parent: ResolvedTopicReference, counterpartParent: ResolvedTopicReference?) + /// Curate all remaining uncurated symbols under their natural parent from the symbol graph. /// /// This will include all symbols that were not manually curated by the documentation author. /// - Returns: An ordered list of symbol references that have been added to the topic graph automatically. - private func autoCurateSymbolsInTopicGraph() -> [(child: ResolvedTopicReference, parent: ResolvedTopicReference)] { - var automaticallyCuratedSymbols = [(ResolvedTopicReference, ResolvedTopicReference)]() - linkResolver.localResolver.traverseSymbolAndParentPairs { reference, parentReference in + private func autoCurateSymbolsInTopicGraph() -> [AutoCuratedSymbolRecord] { + var automaticallyCuratedSymbols = [AutoCuratedSymbolRecord]() + linkResolver.localResolver.traverseSymbolAndParents { reference, parentReference, counterpartParentReference in guard let topicGraphNode = topicGraph.nodeWithReference(reference), - let topicGraphParentNode = topicGraph.nodeWithReference(parentReference), - // Check that the node hasn't got any parents from manual curation + // Check that the node isn't already manually curated !topicGraphNode.isManuallyCurated else { return } + + // Check that the symbol doesn't already have parent's that aren't either language representation's hierarchical parent. + // This for example happens for default implementation and symbols that are requirements of protocol conformances. + guard parents(of: reference).allSatisfy({ $0 == parentReference || $0 == counterpartParentReference }) else { + return + } + + guard let topicGraphParentNode = topicGraph.nodeWithReference(parentReference) else { return } topicGraph.addEdge(from: topicGraphParentNode, to: topicGraphNode) - automaticallyCuratedSymbols.append((child: reference, parent: parentReference)) + + if let counterpartParentReference { + guard let topicGraphCounterpartParentNode = topicGraph.nodeWithReference(counterpartParentReference) else { + // If the counterpart parent doesn't exist. Return the auto curation record without the it. + automaticallyCuratedSymbols.append((reference, parentReference, nil)) + return + } + topicGraph.addEdge(from: topicGraphCounterpartParentNode, to: topicGraphNode) + } + // Collect a single automatic curation record for both language representation parents. + automaticallyCuratedSymbols.append((reference, parentReference, counterpartParentReference)) } return automaticallyCuratedSymbols } diff --git a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchyBasedLinkResolver.swift b/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchyBasedLinkResolver.swift index d112d2164b..be21e37237 100644 --- a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchyBasedLinkResolver.swift +++ b/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchyBasedLinkResolver.swift @@ -45,15 +45,21 @@ final class PathHierarchyBasedLinkResolver { return "\(unresolved.path)#\(urlReadableFragment(fragment))" } - /// Traverse all the pairs of symbols and their parents. - func traverseSymbolAndParentPairs(_ observe: (_ symbol: ResolvedTopicReference, _ parent: ResolvedTopicReference) -> Void) { + /// Traverse all the pairs of symbols and their parents and counterpart parents. + func traverseSymbolAndParents(_ observe: (_ symbol: ResolvedTopicReference, _ parent: ResolvedTopicReference, _ counterpartParent: ResolvedTopicReference?) -> Void) { + let swiftLanguageID = SourceLanguage.swift.id for (id, node) in pathHierarchy.lookup { - guard node.symbol != nil else { continue } - guard let parentID = node.parent?.identifier else { continue } - + guard let symbol = node.symbol, + let parentID = node.parent?.identifier, + // Symbols that exist in more than one source language may have more than one parent. + // If this symbol has language counterparts, only call `observe` for one of the counterparts. + node.counterpart == nil || symbol.identifier.interfaceLanguage == swiftLanguageID + else { continue } + // Only symbols in the symbol index are added to the reference map. guard let reference = resolvedReferenceMap[id], let parentReference = resolvedReferenceMap[parentID] else { continue } - observe(reference, parentReference) + + observe(reference, parentReference, node.counterpart?.parent?.identifier.flatMap { resolvedReferenceMap[$0] }) } } From 229c2ed5f61a9ffbd9cbab11d0eafbe1128b3a88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20R=C3=B6nnqvist?= Date: Mon, 29 Apr 2024 17:13:02 +0200 Subject: [PATCH 12/13] Address code review feedback: - Rename neighbors parameter to edges for DirectedGraph initializer - Rename GraphPathIterator and move it to DirectedGraph+Paths file - Add convenience properties for topic graph directed graph "views - Elaborate on breadcrumbs path documentation and implementation comments - Elaborate on graph cycle documentation with concrete examples - Fix missing edge in directed graph test data - Use preconditionFailure for topic graph node that should always exist - Add additional graph cycle tests --- .../DocumentationContext+Breadcrumbs.swift | 35 +++-- .../Infrastructure/DocumentationContext.swift | 10 +- .../Infrastructure/DocumentationCurator.swift | 2 +- .../Topic Graph/TopicGraph.swift | 14 +- .../Rendering/RenderNodeTranslator.swift | 2 +- .../Utility/Graphs/DirectedGraph+Cycles.swift | 139 +++++++++++++++++- .../Utility/Graphs/DirectedGraph+Paths.swift | 55 ++++++- .../Graphs/DirectedGraph+Traversal.swift | 42 +----- .../Utility/Graphs/DirectedGraph.swift | 8 +- .../DocumentationContextTests.swift | 2 +- .../Utility/DirectedGraphTests.swift | 75 ++++++++-- 11 files changed, 295 insertions(+), 89 deletions(-) diff --git a/Sources/SwiftDocC/Infrastructure/DocumentationContext+Breadcrumbs.swift b/Sources/SwiftDocC/Infrastructure/DocumentationContext+Breadcrumbs.swift index f09ba97c2b..b63bbc9e76 100644 --- a/Sources/SwiftDocC/Infrastructure/DocumentationContext+Breadcrumbs.swift +++ b/Sources/SwiftDocC/Infrastructure/DocumentationContext+Breadcrumbs.swift @@ -17,19 +17,25 @@ extension DocumentationContext { static let preferTechnologyRoot = PathOptions(rawValue: 1 << 0) } - /// Finds all finite paths (breadcrumbs) to the given reference. + /// Finds all finite (acyclic) paths, also called "breadcrumbs", to the given reference in the topic graph. /// - /// Each path is a list of references for the nodes traversed while walking the topic graph from a root node to, but not including, the given `reference`. + /// Each path is a list of references that describe a walk through the topic graph from a leaf node up to, but not including, the given `reference`. /// /// The first path is the canonical path to the node. The other paths are sorted by increasing length (number of components). /// + /// > Note: + /// If all paths from the given reference are infinite (cycle back on themselves) then this function will return an empty list, because there are no _finite_ paths in the topic graph from that reference. + /// /// - Parameters: /// - reference: The reference to find paths to. /// - options: Options for how the context produces node breadcrumbs. /// - Returns: A list of finite paths to the given reference in the topic graph. func finitePaths(to reference: ResolvedTopicReference, options: PathOptions = []) -> [[ResolvedTopicReference]] { - reverseEdgesGraph + topicGraph.reverseEdgesGraph .allFinitePaths(from: reference) + // Graph traversal typically happens from the starting point outwards, but the callers of `finitePaths(to:options:)` + // expect paths going inwards from the leaves to the starting point, excluding the starting point itself. + // To match the caller's expectations we remove the starting point and then flip the paths. .map { $0.dropFirst().reversed() } .sorted { (lhs, rhs) -> Bool in // Order a path rooted in a technology as the canonical one. @@ -41,13 +47,21 @@ extension DocumentationContext { } } - /// Finds the shortest finite path (breadcrumb) to the given reference. + /// Finds the shortest finite (acyclic) path, also called "breadcrumb", to the given reference in the topic graph. + /// + /// The path is a list of references that describe a walk through the topic graph from a leaf node up to, but not including, the given `reference`. + /// + /// > Note: + /// If all paths from the given reference are infinite (cycle back on themselves) then this function will return `nil`, because there are no _finite_ paths in the topic graph from that reference. /// /// - Parameter reference: The reference to find the shortest path to. /// - Returns: The shortest path to the given reference, or `nil` if all paths to the reference are infinite (contain cycles). func shortestFinitePath(to reference: ResolvedTopicReference) -> [ResolvedTopicReference]? { - reverseEdgesGraph + topicGraph.reverseEdgesGraph .shortestFinitePaths(from: reference) + // Graph traversal typically happens from the starting point outwards, but the callers of `shortestFinitePaths(to:)` + // expect paths going inwards from the leaves to the starting point, excluding the starting point itself. + // To match the caller's expectations we remove the starting point and then flip the paths. .map { $0.dropFirst().reversed() } .min(by: breadcrumbsAreInIncreasingOrder) } @@ -55,21 +69,16 @@ extension DocumentationContext { /// Finds all the reachable root node references from the given reference. /// /// > Note: - /// If all paths from the given reference are infinite (contain cycles) then it can't reach any roots and will return an empty set. + /// If all paths from the given reference are infinite (cycle back on themselves) then this function will return an empty set, because there are no reachable roots in the topic graph from that reference. /// /// - Parameter reference: The reference to find reachable root node references from. /// - Returns: The references of the root nodes that are reachable fro the given reference, or `[]` if all paths from the reference are infinite (contain cycles). func reachableRoots(from reference: ResolvedTopicReference) -> Set { - reverseEdgesGraph.reachableLeafNodes(from: reference) - } - - /// The directed graph of reverse edges in the topic graph. - private var reverseEdgesGraph: DirectedGraph { - DirectedGraph(neighbors: topicGraph.reverseEdges) + topicGraph.reverseEdgesGraph.reachableLeafNodes(from: reference) } } -/// Compares two breadcrumbs for sorting so that the breadcrumb with fewer components come first and breadcrumbs with the same number of components are sorter alphabetically. +/// Compares two breadcrumbs for sorting so that the breadcrumb with fewer components come first and breadcrumbs with the same number of components are sorted alphabetically. private func breadcrumbsAreInIncreasingOrder(_ lhs: [ResolvedTopicReference], _ rhs: [ResolvedTopicReference]) -> Bool { // If the breadcrumbs have the same number of components, sort alphabetically to produce stable results. guard lhs.count != rhs.count else { diff --git a/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift b/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift index 26026d6511..a99c36f0f9 100644 --- a/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift +++ b/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift @@ -2378,14 +2378,14 @@ public class DocumentationContext: DocumentationContextDataProviderDelegate { return } - guard let topicGraphParentNode = topicGraph.nodeWithReference(parentReference) else { return } + guard let topicGraphParentNode = topicGraph.nodeWithReference(parentReference) else { + preconditionFailure("Node with reference \(parentReference.absoluteString) exist in link resolver but not in topic graph.") + } topicGraph.addEdge(from: topicGraphParentNode, to: topicGraphNode) if let counterpartParentReference { - guard let topicGraphCounterpartParentNode = topicGraph.nodeWithReference(counterpartParentReference) else { - // If the counterpart parent doesn't exist. Return the auto curation record without the it. - automaticallyCuratedSymbols.append((reference, parentReference, nil)) - return + guard let topicGraphCounterpartParentNode = topicGraph.nodeWithReference(counterpartParentReference) else { + preconditionFailure("Node with reference \(counterpartParentReference.absoluteString) exist in link resolver but not in topic graph.") } topicGraph.addEdge(from: topicGraphCounterpartParentNode, to: topicGraphNode) } diff --git a/Sources/SwiftDocC/Infrastructure/DocumentationCurator.swift b/Sources/SwiftDocC/Infrastructure/DocumentationCurator.swift index 5a2125becd..9f7939259b 100644 --- a/Sources/SwiftDocC/Infrastructure/DocumentationCurator.swift +++ b/Sources/SwiftDocC/Infrastructure/DocumentationCurator.swift @@ -127,7 +127,7 @@ struct DocumentationCurator { } private func isReference(_ childReference: ResolvedTopicReference, anAncestorOf nodeReference: ResolvedTopicReference) -> Bool { - DirectedGraph(neighbors: context.topicGraph.reverseEdges) + context.topicGraph.reverseEdgesGraph .breadthFirstSearch(from: nodeReference) .contains(childReference) } diff --git a/Sources/SwiftDocC/Infrastructure/Topic Graph/TopicGraph.swift b/Sources/SwiftDocC/Infrastructure/Topic Graph/TopicGraph.swift index 9a40f9f151..2d414f7a57 100644 --- a/Sources/SwiftDocC/Infrastructure/Topic Graph/TopicGraph.swift +++ b/Sources/SwiftDocC/Infrastructure/Topic Graph/TopicGraph.swift @@ -277,7 +277,7 @@ struct TopicGraph { /// Returns a sequence that traverses the topic graph in depth first order from a given reference, without visiting the same node more than once. func depthFirstSearch(from reference: ResolvedTopicReference) -> some Sequence { - DirectedGraph(neighbors: edges) + edgesGraph .depthFirstSearch(from: reference) .lazy .map { nodeWithReference($0)! } @@ -285,11 +285,21 @@ struct TopicGraph { /// Returns a sequence that traverses the topic graph in breadth first order from a given reference, without visiting the same node more than once. func breadthFirstSearch(from reference: ResolvedTopicReference) -> some Sequence { - DirectedGraph(neighbors: edges) + edgesGraph .breadthFirstSearch(from: reference) .lazy .map { nodeWithReference($0)! } } + + /// A directed graph of the edges in the topic graph. + var edgesGraph: DirectedGraph { + DirectedGraph(edges: edges) + } + + /// A directed graph of the reverse edges in the topic graph. + var reverseEdgesGraph: DirectedGraph { + DirectedGraph(edges: reverseEdges) + } /// Returns the children of this node that reference it as their overload group. func overloads(of groupReference: ResolvedTopicReference) -> [ResolvedTopicReference]? { diff --git a/Sources/SwiftDocC/Model/Rendering/RenderNodeTranslator.swift b/Sources/SwiftDocC/Model/Rendering/RenderNodeTranslator.swift index 04fabfb701..b5037ad73c 100644 --- a/Sources/SwiftDocC/Model/Rendering/RenderNodeTranslator.swift +++ b/Sources/SwiftDocC/Model/Rendering/RenderNodeTranslator.swift @@ -605,7 +605,7 @@ public struct RenderNodeTranslator: SemanticVisitor { // Detect the article modules from its breadcrumbs. var modules = Set() - for reference in DirectedGraph(neighbors: context.topicGraph.reverseEdges).breadthFirstSearch(from: identifier) { + for reference in context.topicGraph.reverseEdgesGraph.breadthFirstSearch(from: identifier) { if let moduleReference = (try? context.entity(with: reference).semantic as? Symbol)?.moduleReference { modules.insert(moduleReference) } diff --git a/Sources/SwiftDocC/Utility/Graphs/DirectedGraph+Cycles.swift b/Sources/SwiftDocC/Utility/Graphs/DirectedGraph+Cycles.swift index e4241310ba..e7a32d7329 100644 --- a/Sources/SwiftDocC/Utility/Graphs/DirectedGraph+Cycles.swift +++ b/Sources/SwiftDocC/Utility/Graphs/DirectedGraph+Cycles.swift @@ -9,9 +9,43 @@ */ extension DirectedGraph { - /// Returns the first cycle in the graph encountered through breadth first traversal from a given starting point + /// Returns the first cycle in the graph encountered through breadth first traversal from a given starting point. /// - /// The cycle starts at the earliest repeated node and ends with the node that links back to the starting point. + /// The cycle starts at the earliest repeated node and ends with the node that links back to the repeated node. + /// + /// ## Example + /// + /// For example, consider the following subgraph, which can be entered from different directions: + /// ``` + /// ┌──────▶2◀── + /// │ │ + /// ──▶1───┐ │ + /// ▲ ▼ │ + /// └───3◀──┘ + /// ▲ + /// │ + /// ``` + /// + /// When entering the subgraph from "1", there are two cycles in this graph; `1,3` and `1,2,3`. + /// + /// ``` + /// ┌──────▶2 ┏━━━━━━▶2 + /// │ │ ┃ ┃ + /// 0──▶1━━━┓ │ 0──▶1───┐ ┃ + /// ▲ ▼ │ ▲ ▼ ┃ + /// ┗━━━3◀──┘ ┗━━━3◀━━┛ + /// ``` + /// + /// Breadth first traversal through the graph encounters the `1,3` cycle first, so it stops iterating and only returns that cycle. + /// + /// Entering the same subgraph from "2" encounters the same cycle, but with "3" as the earliest repeated node, so it returns `1,3` instead. + /// ``` + /// ┌──────▶2◀──0 + /// │ │ + /// 1━━━┓ │ + /// ▲ ▼ │ + /// ┗━━━3◀──┘ + /// ``` /// /// - Note: A cycle, if found, is guaranteed to contain at least one node. func firstCycle(from startingPoint: Node) -> Path? { @@ -21,13 +55,104 @@ extension DirectedGraph { return nil } - /// Returns a list of all the unique cycles in the graph encountered through breadth first traversal from a given starting point. + /// Returns a list of all the "unique" cycles in the graph encountered through breadth first traversal from a given starting point. + /// + /// Each cycle starts at the earliest repeated node and ends with the node that links back to the repeated node. + /// + /// Two cycles are considered the "same" if both cycles can be broken by removing the same edge in the graph. This happens when they either: + /// - Have the same start and end points, for example: `1,2,3` and `1,3`. + /// - Are rotations of each other, for example: `1,2,3` and `2,3,1` and `3,1,2`. + /// + /// > Important: + /// There graph may have different cycles that are reached from different starting points. + /// + /// ## Example: Single entry point to cycle + /// + /// For example, consider the following subgraph, which can be entered from different directions: + /// ``` + /// ┌──────▶2◀── + /// │ │ + /// ──▶1───┐ │ + /// ▲ ▼ │ + /// └───3◀──┘ + /// ▲ + /// │ + /// ``` + /// + /// When entering the subgraph from "1", there are two cycles in this graph; `1,3` and `1,2,3`. + /// These are considered the _same_ cycle because removing the `1─▶3` edge breaks both cycles. + /// ``` + /// ┏━━━━━━▶2 + /// ┃ ┃ + /// 0──▶1━━━┓ ┃ + /// ▼ ┃ + /// └ ─ 3◀━━┛ + /// ``` + /// + /// On the other hand, when entering the same subgraph from "2" there are two other cycles; `3,1` and `2,3,1`. + /// These are considered _different_ cycles because they each require removing a different edge to break that cycle; + /// `1─▶3` for the `3,1` cycle and `1─▶2` for the `2,3,1` cycle; + /// ``` + /// ┌ ─ ─ ─ 2◀──0 + /// ┃ + /// 1 ─ ┐ ┃ + /// ▲ ┃ + /// ┗━━━3◀━━┛ + /// ``` + /// + /// ## Example: Multiple entry points to cycle + /// + /// Consider the same subgraph as before, which can be entered from different directions, where the starting point "0" enters the subgraph more than once: + /// ``` + /// 0──────────┐ + /// │ ▼ + /// ┏━━━━━━▶2 │ ┏━━━━━━▶2 ┏━━━━━━▶2◀──┐ + /// ┃ ┃ │ ┃ ┃ ┃ ┃ │ + /// ┌──▶1━━━┓ ┃ └─▶1━━━┓ ┃ 1━━━┓ ┃ │ + /// │ ▲ ▼ ┃ ▲ ▼ ┃ ▲ ▼ ┃ │ + /// │ ┗━━━3◀━━┛ ┗━━━3◀━━┛ ┗━━━3◀━━┛ │ + /// │ ▲ ▲ │ + /// 0───────┘ └───────0 + /// ``` + /// + /// For each of these starting points: + /// - `1,3` and `3,1,2` are considered _different_ cycles because we need to remove the `3─▶1` edge to break the `1,3` cycle and remove the `2─▶3` to break the `3,1,2` cycle. + /// - `1,3` and `2,3,1` are considered _different_ cycles because we need to remove the `3─▶1` edge to break the `1,3` cycle and remove the `1─▶2` to break the `2,3,1` cycle. + /// - `3,1` and `2,3,1` are considered _different_ cycles because we need to remove the `1─▶3` edge to break the `3,1` cycle and remove the `1─▶2` to break the `2,3,1` cycle. + /// + /// ``` + /// 0──────────┐ + /// │ ▼ + /// ┏━━━━━━▶2 │ ┌ ─ ─ ─ 2 ┌ ─ ─ ─ 2◀──┐ + /// ┃ ╵ │ ╷ ┃ ╷ ┃ │ + /// ┌──▶1━━━┓ ╵ └─▶1━━━┓ ┃ 1 ─ ┐ ┃ │ + /// │ ╵ ▼ ╵ ╵ ▼ ┃ ▲ ╷ ┃ │ + /// │ └ ─ 3 ─ ┘ └ ─ 3◀━━┛ ┗━━━3◀━━┛ │ + /// │ ▲ ▲ │ + /// 0───────┘ └───────0 + /// ``` + /// + /// If you remove the edge from `cycle.last` to `cycle.first` for each cycle in the returned list, you'll break all the cycles from _that_ starting point in the graph. + /// This _doesn't_ guarantee that the graph is free of cycles from other starting points. + /// + /// ## Example: Rotation of cycle /// - /// Each cycle starts at the earliest repeated node and ends with the node that links back to the starting point. + /// Consider this cyclic subgraph which can be entered from different directions: + /// ``` + /// ┌────▶1━━━━▶2 + /// │ ▲ ┃ + /// │ ┃ ┃ + /// 0────▶3◀━━━━┛ + /// ``` /// - /// Two cycles are considered the same if they have either: - /// - The same start and end points, for example: `1,2,3` and `1,3`. - /// - A rotation of the same cycle, for example: `1,2,3`, `2,3,1`, and `3,1,2`. + /// With two ways to enter the cycle, it will encounter both the `1,2,3` and the `3,1,2` cycle. + /// These are considered the _same_ cycle because removing the `3─▶1` edge breaks both cycles. + /// ``` + /// ┌────▶1━━━━▶2 + /// │ ╷ ┃ + /// │ ╷ ┃ + /// 0────▶3◀━━━━┛ + /// ``` func cycles(from startingPoint: Node) -> [Path] { var cycles = [Path]() diff --git a/Sources/SwiftDocC/Utility/Graphs/DirectedGraph+Paths.swift b/Sources/SwiftDocC/Utility/Graphs/DirectedGraph+Paths.swift index 5258c7ffb3..d2507254ad 100644 --- a/Sources/SwiftDocC/Utility/Graphs/DirectedGraph+Paths.swift +++ b/Sources/SwiftDocC/Utility/Graphs/DirectedGraph+Paths.swift @@ -9,13 +9,13 @@ */ extension DirectedGraph { - /// Returns a list of all finite paths through the graph from a given starting point. + /// Returns a list of all finite (acyclic) paths through the graph from a given starting point. /// /// The paths are found in breadth first order, so shorter paths are earlier in the returned list. /// /// - Note: Nodes that are reachable through multiple paths will be visited more than once. /// - /// - Important: If all paths through the graph result in cycles, the returned list will be empty. + /// - Important: If all paths through the graph are infinite (cyclic), this function return an empty list. func allFinitePaths(from startingPoint: Node) -> [Path] { var foundPaths = [Path]() @@ -26,13 +26,13 @@ extension DirectedGraph { return foundPaths } - /// Returns a list of the finite paths through the graph with the shortest length from a given starting point. + /// Returns a list of the finite (acyclic) paths through the graph with the shortest length from a given starting point. /// /// The paths are found in breadth first order, so shorter paths are earlier in the returned list. /// /// - Note: Nodes that are reachable through multiple paths will be visited more than once. /// - /// - Important: If all paths through the graph result in cycles, the returned list will be empty. + /// - Important: If all paths through the graph are infinite (cyclic), this function return an empty list. func shortestFinitePaths(from startingPoint: Node) -> [Path] { var foundPaths = [Path]() @@ -51,7 +51,7 @@ extension DirectedGraph { /// Returns a set of all the reachable leaf nodes by traversing the graph from a given starting point. /// - /// - Important: If all paths through the graph result in cycles, the returned set will be empty. + /// - Important: If all paths through the graph are infinite (cyclic), this function return an empty set. func reachableLeafNodes(from startingPoint: Node) -> Set { var foundLeafNodes: Set = [] @@ -62,3 +62,48 @@ extension DirectedGraph { return foundLeafNodes } } + +// MARK: Path sequence + +extension DirectedGraph { + /// A path through the graph, including the start and end nodes. + typealias Path = [Node] + /// Information about the current accumulated path during iteration. + typealias PathIterationElement = (path: Path, isLeaf: Bool, cycleStartIndex: Int?) + + /// Returns a sequence of accumulated path information from traversing the graph in breadth first order. + func accumulatingPaths(from startingPoint: Node) -> some Sequence { + IteratorSequence(GraphPathIterator(from: startingPoint, in: self)) + } +} + +// MARK: Iterator + +/// An iterator that traverses a graph in breadth first order and returns information about the accumulated path through the graph, up to the current node. +private struct GraphPathIterator: IteratorProtocol { + var pathsToTraverse: [(Node, [Node])] + var graph: DirectedGraph + + init(from startingPoint: Node, in graph: DirectedGraph) { + self.pathsToTraverse = [(startingPoint, [])] + self.graph = graph + } + + mutating func next() -> DirectedGraph.PathIterationElement? { + guard !pathsToTraverse.isEmpty else { return nil } + // This is a breadth first search through the graph. + let (node, path) = pathsToTraverse.removeFirst() + + // Note: unlike `GraphNodeIterator`, the same node may be visited more than once. + + if let cycleStartIndex = path.firstIndex(of: node) { + return (path, false, cycleStartIndex) + } + + let newPath = path + [node] + let neighbors = graph.neighbors(of: node) + pathsToTraverse.append(contentsOf: neighbors.map { ($0, newPath) }) + + return (newPath, neighbors.isEmpty, nil) + } +} diff --git a/Sources/SwiftDocC/Utility/Graphs/DirectedGraph+Traversal.swift b/Sources/SwiftDocC/Utility/Graphs/DirectedGraph+Traversal.swift index efd7373d41..bbba9b27d1 100644 --- a/Sources/SwiftDocC/Utility/Graphs/DirectedGraph+Traversal.swift +++ b/Sources/SwiftDocC/Utility/Graphs/DirectedGraph+Traversal.swift @@ -20,19 +20,7 @@ extension DirectedGraph { } } -extension DirectedGraph { - /// A path through the graph, including the start and end nodes. - typealias Path = [Node] - /// Information about the current accumulated path during iteration. - typealias PathIterationElement = (path: Path, isLeaf: Bool, cycleStartIndex: Int?) - - /// Returns a sequence of accumulated path information from traversing the graph in breadth first order. - func accumulatingPaths(from startingPoint: Node) -> some Sequence { - IteratorSequence(GraphBreadthFirstPathIterator(from: startingPoint, in: self)) - } -} - -// MARK: Node iterator +// MARK: Iterator /// An iterator that traverses a graph in either breadth first or depth first order depending on the buffer it uses to track nodes to traverse next. private struct GraphNodeIterator: IteratorProtocol { @@ -75,31 +63,3 @@ private struct GraphNodeIterator: IteratorProtocol { return nil } } - -// MARK: Path iterator - -/// An iterator that traverses a graph in breadth first order and returns information about the accumulated path through the graph, up to the current node. -private struct GraphBreadthFirstPathIterator: IteratorProtocol { - var pathsToTraverse: [(Node, [Node])] - var graph: DirectedGraph - - init(from startingPoint: Node, in graph: DirectedGraph) { - self.pathsToTraverse = [(startingPoint, [])] - self.graph = graph - } - - mutating func next() -> DirectedGraph.PathIterationElement? { - guard !pathsToTraverse.isEmpty else { return nil } - let (node, path) = pathsToTraverse.removeFirst() - - if let cycleStartIndex = path.lastIndex(of: node) { - return (path, false, cycleStartIndex) - } - - let newPath = path + [node] - let neighbors = graph.neighbors(of: node) - pathsToTraverse.append(contentsOf: neighbors.map { ($0, newPath) }) - - return (newPath, neighbors.isEmpty, nil) - } -} diff --git a/Sources/SwiftDocC/Utility/Graphs/DirectedGraph.swift b/Sources/SwiftDocC/Utility/Graphs/DirectedGraph.swift index 5ca8893418..b1646c7102 100644 --- a/Sources/SwiftDocC/Utility/Graphs/DirectedGraph.swift +++ b/Sources/SwiftDocC/Utility/Graphs/DirectedGraph.swift @@ -38,13 +38,13 @@ struct DirectedGraph { // but all our current usages of graph structures use dictionaries to track the neighboring nodes. // // This type is internal so we can change it's implementation later when there's new data that's structured differently. - private let _neighbors: [Node: [Node]] - init(neighbors: [Node: [Node]]) { - _neighbors = neighbors + private let edges: [Node: [Node]] + init(edges: [Node: [Node]]) { + self.edges = edges } /// Returns the nodes that are reachable from the given node func neighbors(of node: Node) -> [Node] { - _neighbors[node] ?? [] + edges[node] ?? [] } } diff --git a/Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContextTests.swift b/Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContextTests.swift index 74ee7175a7..6f97f1f25e 100644 --- a/Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContextTests.swift +++ b/Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContextTests.swift @@ -4641,7 +4641,7 @@ let expected = """ let reference = ResolvedTopicReference(bundleIdentifier: bundle.identifier, path: "/documentation/ModuleName/SomeError/Code-swift.enum/someCase", sourceLanguage: .swift) XCTAssertEqual( - DirectedGraph(neighbors: context.topicGraph.reverseEdges).cycles(from: reference).map { $0.map(\.lastPathComponent) }, + context.topicGraph.reverseEdgesGraph.cycles(from: reference).map { $0.map(\.lastPathComponent) }, [ ["Code-swift.enum", "SomeError", "SomeClass"] ], "There is one cyclic path encountered while traversing the reverse edges from the 'someCase' enum case." ) diff --git a/Tests/SwiftDocCUtilitiesTests/Utility/DirectedGraphTests.swift b/Tests/SwiftDocCUtilitiesTests/Utility/DirectedGraphTests.swift index e9fd8a1c66..98c3b61201 100644 --- a/Tests/SwiftDocCUtilitiesTests/Utility/DirectedGraphTests.swift +++ b/Tests/SwiftDocCUtilitiesTests/Utility/DirectedGraphTests.swift @@ -21,7 +21,7 @@ final class DirectedGraphTests: XCTestCase { // │ │ // ▼ ▼ // 7───▶8◀───9 - let graph = DirectedGraph(neighbors: [ + let graph = DirectedGraph(edges: [ 1: [2], 2: [5], 3: [2], @@ -77,7 +77,7 @@ final class DirectedGraphTests: XCTestCase { // 1─┼─▶3 // │ // └─▶4──▶7──▶8 - let graph = DirectedGraph(neighbors: [ + let graph = DirectedGraph(edges: [ 1: [2,3,4], 2: [5,6], 4: [7], @@ -110,7 +110,7 @@ final class DirectedGraphTests: XCTestCase { // 1─┼─▶3─┼▶5──▶6 // │ │ // └─▶4─┘ - let graph = DirectedGraph(neighbors: [ + let graph = DirectedGraph(edges: [ 1: [2,3,4], 2: [5], 3: [5], @@ -146,7 +146,7 @@ final class DirectedGraphTests: XCTestCase { // │ ▲ │ ▲ // ▼ │ │ │ // 3───┘ └──▶7───┘ - let graph = DirectedGraph(neighbors: [ + let graph = DirectedGraph(edges: [ 1: [2], 2: [3,4], 3: [4], @@ -178,6 +178,60 @@ final class DirectedGraphTests: XCTestCase { } } + func testSimpleCycle() throws { + do { + // ┌──────▶2 + // │ │ + // 1───┐ │ + // ▲ ▼ │ + // └───3◀──┘ + let graph = DirectedGraph(edges: [ + 0: [2,3], + 1: [2,3], + 2: [3], + 3: [1], + ]) + + XCTAssertEqual(graph.cycles(from: 1), [ + [1,3], + ]) + XCTAssertEqual(graph.cycles(from: 2), [ + [2,3,1], + [3,1], + ]) + XCTAssertEqual(graph.cycles(from: 3), [ + [3,1], + [3,1,2], + ]) + + for id in [1,2,3] { + XCTAssertEqual(graph.allFinitePaths(from: id), [], "The only path from '\(id)' is infinite (cyclic)") + XCTAssertEqual(graph.shortestFinitePaths(from: id), [], "The only path from '\(id)' is infinite (cyclic)") + XCTAssertEqual(graph.reachableLeafNodes(from: id), [], "The only path from '\(id)' is infinite (cyclic)") + } + } + } + + func testSimpleCycleRotation() throws { + do { + // ┌───▶1───▶2 + // │ ▲ │ + // │ │ │ + // 0───▶3◀───┘ + let graph = DirectedGraph(edges: [ + 0: [1,3], + 1: [2,], + 2: [3], + 3: [1], + ]) + + XCTAssertEqual(graph.cycles(from: 0), [ + [1,2,3], + // '3,1,2' and '2,3,1' are both rotations of '1,2,3'. + ]) + } + } + func testGraphWithCycleAndSingleAdjacency() throws { // 1───▶2◀───3 // │ @@ -186,7 +240,7 @@ final class DirectedGraphTests: XCTestCase { // │ ▲ // ▼ │ // 7───▶8───▶9 - let graph = DirectedGraph(neighbors: [ + let graph = DirectedGraph(edges: [ 1: [2], 2: [5], 3: [2], @@ -235,10 +289,10 @@ final class DirectedGraphTests: XCTestCase { // │ │ // ▼ ▼ // 8 11 - let graph = DirectedGraph(neighbors: [ + let graph = DirectedGraph(edges: [ 0: [1,2], 2: [3,4,5], - 4: [6,7], + 4: [5,6,7], 5: [8,9], 7: [10,9], 9: [11,7], @@ -257,10 +311,13 @@ final class DirectedGraphTests: XCTestCase { [0,2,3], [0,2,4,6], [0,2,5,8], + [0,2,4,5,8], [0,2,4,7,10], [0,2,5,9,11], + [0,2,4,5,9,11], [0,2,4,7,9,11], [0,2,5,9,7,10], + [0,2,4,5,9,7,10] ]) XCTAssertEqual(graph.shortestFinitePaths(from: 0), [ @@ -280,7 +337,7 @@ final class DirectedGraphTests: XCTestCase { // ║ ║ │ ║ // ║ ▼ ▼ ▼ // ╚═══▶3 8───▶9───▶11 - let graph = DirectedGraph(neighbors: [ + let graph = DirectedGraph(edges: [ 1: [1,2,3], 2: [3,4,5], 3: [1,2], @@ -343,7 +400,7 @@ final class DirectedGraphTests: XCTestCase { // │ │ ▲ // │ ▼ │ // └─▶4──┘ - let graph = DirectedGraph(neighbors: [ + let graph = DirectedGraph(edges: [ 1: [2,3,4], 2: [3], 3: [4], From 421f9fe930f4f1c22cbfda34a9abee98875df17c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20R=C3=B6nnqvist?= Date: Mon, 29 Apr 2024 17:13:46 +0200 Subject: [PATCH 13/13] Explicitly exit (trap) if trying to dump a topic graph with cyclic paths --- .../SwiftDocC/Infrastructure/Topic Graph/TopicGraph.swift | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Sources/SwiftDocC/Infrastructure/Topic Graph/TopicGraph.swift b/Sources/SwiftDocC/Infrastructure/Topic Graph/TopicGraph.swift index 2d414f7a57..1efb09bf22 100644 --- a/Sources/SwiftDocC/Infrastructure/Topic Graph/TopicGraph.swift +++ b/Sources/SwiftDocC/Infrastructure/Topic Graph/TopicGraph.swift @@ -334,7 +334,14 @@ struct TopicGraph { /// │ ╰ doc://com.testbundle/documentation/MyFramework/MyClass/init() /// ... /// ``` + /// + /// - Precondition: All paths through the topic graph from the starting node are finite (acyclic). func dump(startingAt node: Node, keyPath: KeyPath = \.title, decorator: String = "") -> String { + if let cycle = edgesGraph.firstCycle(from: node.reference) { + let cycleDescription = cycle.map(\.absoluteString).joined(separator: " -> ") + preconditionFailure("Traversing the topic graph from \(node.reference.absoluteString) encounters an infinite cyclic path: \(cycleDescription) -cycle-> \(cycleDescription) ...") + } + var result = "" result.append("\(decorator) \(node[keyPath: keyPath])\r\n") if let childEdges = edges[node.reference]?.sorted(by: { $0.path < $1.path }) {