Skip to content

Commit aa0a99e

Browse files
d-ronnqvistd-ronnqvist
authored
Continues searching if a non-symbol matches a symbol link (#901)
1 parent e6b8152 commit aa0a99e

File tree

3 files changed

+150
-91
lines changed

3 files changed

+150
-91
lines changed

‎Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+Error.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ extension PathHierarchy {
4646
///
4747
/// Includes information about:
4848
/// - The path to the non-symbol match.
49-
case nonSymbolMatchForSymbolLink(path: Substring)
49+
case nonSymbolMatchForSymbolLink(path: String)
5050

5151
/// Encountered an unknown disambiguation for a found node.
5252
///

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

Lines changed: 106 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,7 @@ extension PathHierarchy {
1818
/// - Returns: Returns the unique identifier for the found match or raises an error if no match can be found.
1919
/// - Throws: Raises a ``PathHierarchy/Error`` if no match can be found.
2020
func find(path rawPath: String, parent: ResolvedIdentifier? = nil, onlyFindSymbols: Bool) throws -> ResolvedIdentifier {
21-
let node = try findNode(path: rawPath, parentID: parent, onlyFindSymbols: onlyFindSymbols)
22-
if node.identifier == nil {
23-
throw Error.unfindableMatch(node)
24-
}
25-
if onlyFindSymbols, node.symbol == nil {
26-
throw Error.nonSymbolMatchForSymbolLink(path: rawPath[...])
27-
}
28-
return node.identifier
21+
return try findNode(path: rawPath, parentID: parent, onlyFindSymbols: onlyFindSymbols).identifier
2922
}
3023

3124
private func findNode(path rawPath: String, parentID: ResolvedIdentifier?, onlyFindSymbols: Bool) throws -> Node {
@@ -216,98 +209,121 @@ extension PathHierarchy {
216209
onlyFindSymbols: Bool,
217210
rawPathForError: String
218211
) throws -> Node {
219-
var node = startingPoint
220-
var remaining = pathComponents[...]
221-
222-
// Third, search for the match relative to the start node.
223-
if remaining.isEmpty {
224-
// If all path components were consumed, then the start of the search is the match.
225-
return node
226-
}
212+
// All code paths through this function wants to perform extra verification on the return value before returning it to the caller.
213+
// To accomplish that, the core implementation happens in `_innerImplementation`, which is called once, right below its definition.
227214

228-
// Search for the remaining components from the node
229-
while true {
230-
let (children, pathComponent) = try findChildContainer(node: &node, remaining: remaining, rawPathForError: rawPathForError)
215+
func _innerImplementation(
216+
descendingFrom startingPoint: Node,
217+
pathComponents: ArraySlice<PathComponent>,
218+
onlyFindSymbols: Bool,
219+
rawPathForError: String
220+
) throws -> Node {
221+
var node = startingPoint
222+
var remaining = pathComponents[...]
231223

232-
do {
233-
guard let child = try children.find(pathComponent.disambiguation) else {
234-
// The search has ended with a node that doesn't have a child matching the next path component.
235-
throw makePartialResultError(node: node, remaining: remaining, rawPathForError: rawPathForError)
236-
}
237-
node = child
238-
remaining = remaining.dropFirst()
239-
if remaining.isEmpty {
240-
// If all path components are consumed, then the match is found.
241-
return child
242-
}
243-
} catch DisambiguationContainer.Error.lookupCollision(let collisions) {
244-
func handleWrappedCollision() throws -> Node {
245-
try handleCollision(node: node, remaining: remaining, collisions: collisions, onlyFindSymbols: onlyFindSymbols, rawPathForError: rawPathForError)
246-
}
247-
248-
// When there's a collision, use the remaining path components to try and narrow down the possible collisions.
224+
// Search for the match relative to the start node.
225+
if remaining.isEmpty {
226+
// If all path components were consumed, then the start of the search is the match.
227+
return node
228+
}
229+
230+
// Search for the remaining components from the node
231+
while true {
232+
let (children, pathComponent) = try findChildContainer(node: &node, remaining: remaining, rawPathForError: rawPathForError)
249233

250-
guard let nextPathComponent = remaining.dropFirst().first else {
251-
// This was the last path component so there's nothing to look ahead.
252-
//
253-
// It's possible for a symbol that exist on multiple languages to collide with itself.
254-
// Check if the collision can be resolved by finding a unique symbol or an otherwise preferred match.
255-
var uniqueCollisions: [String: Node] = [:]
256-
for (node, _) in collisions {
257-
guard let symbol = node.symbol else {
258-
// Non-symbol collisions should have already been resolved
259-
return try handleWrappedCollision()
260-
}
261-
262-
let id = symbol.identifier.precise
263-
if symbol.identifier.interfaceLanguage == "swift" || !uniqueCollisions.keys.contains(id) {
264-
uniqueCollisions[id] = node
265-
}
266-
267-
guard uniqueCollisions.count < 2 else {
268-
// Encountered more than one unique symbol
269-
return try handleWrappedCollision()
270-
}
234+
do {
235+
guard let child = try children.find(pathComponent.disambiguation) else {
236+
// The search has ended with a node that doesn't have a child matching the next path component.
237+
throw makePartialResultError(node: node, remaining: remaining, rawPathForError: rawPathForError)
271238
}
272-
// A wrapped error would have been raised while iterating over the collection.
273-
return uniqueCollisions.first!.value
274-
}
275-
276-
// Look ahead one path component to narrow down the list of collisions.
277-
// For each collision where the next path component can be found unambiguously, return that matching node one level down.
278-
let possibleMatchesOneLevelDown = collisions.compactMap {
279-
return try? $0.node.children[String(nextPathComponent.name)]?.find(nextPathComponent.disambiguation)
280-
}
281-
let onlyPossibleMatch: Node?
282-
283-
if possibleMatchesOneLevelDown.count == 1 {
284-
// Only one of the collisions found a match for the next path component
285-
onlyPossibleMatch = possibleMatchesOneLevelDown.first!
286-
} else if !possibleMatchesOneLevelDown.isEmpty, possibleMatchesOneLevelDown.dropFirst().allSatisfy({ $0.symbol?.identifier.precise == possibleMatchesOneLevelDown.first!.symbol?.identifier.precise }) {
287-
// It's also possible that different language representations of the same symbols appear as different collisions.
288-
// If _all_ collisions that can find the next path component are the same symbol, then we prefer the Swift version of that symbol.
289-
onlyPossibleMatch = possibleMatchesOneLevelDown.first(where: { $0.symbol?.identifier.interfaceLanguage == "swift" }) ?? possibleMatchesOneLevelDown.first!
290-
} else {
291-
onlyPossibleMatch = nil
292-
}
293-
294-
if let onlyPossibleMatch {
295-
// If we found only a single match one level down then we've processed both this path component and the next.
296-
remaining = remaining.dropFirst(2)
239+
node = child
240+
remaining = remaining.dropFirst()
297241
if remaining.isEmpty {
298-
// If that was the end of the path we can simply return the result.
299-
return onlyPossibleMatch
242+
// If all path components are consumed, then the match is found.
243+
return child
244+
}
245+
} catch DisambiguationContainer.Error.lookupCollision(let collisions) {
246+
func handleWrappedCollision() throws -> Node {
247+
let match = try handleCollision(node: node, remaining: remaining, collisions: collisions, onlyFindSymbols: onlyFindSymbols, rawPathForError: rawPathForError)
248+
return match
249+
}
250+
251+
// When there's a collision, use the remaining path components to try and narrow down the possible collisions.
252+
253+
guard let nextPathComponent = remaining.dropFirst().first else {
254+
// This was the last path component so there's nothing to look ahead.
255+
//
256+
// It's possible for a symbol that exist on multiple languages to collide with itself.
257+
// Check if the collision can be resolved by finding a unique symbol or an otherwise preferred match.
258+
var uniqueCollisions: [String: Node] = [:]
259+
for (node, _) in collisions {
260+
guard let symbol = node.symbol else {
261+
// Non-symbol collisions should have already been resolved
262+
return try handleWrappedCollision()
263+
}
264+
265+
let id = symbol.identifier.precise
266+
if symbol.identifier.interfaceLanguage == "swift" || !uniqueCollisions.keys.contains(id) {
267+
uniqueCollisions[id] = node
268+
}
269+
270+
guard uniqueCollisions.count < 2 else {
271+
// Encountered more than one unique symbol
272+
return try handleWrappedCollision()
273+
}
274+
}
275+
// A wrapped error would have been raised while iterating over the collection.
276+
return uniqueCollisions.first!.value
277+
}
278+
279+
// Look ahead one path component to narrow down the list of collisions.
280+
// For each collision where the next path component can be found unambiguously, return that matching node one level down.
281+
let possibleMatchesOneLevelDown = collisions.compactMap {
282+
try? $0.node.children[String(nextPathComponent.name)]?.find(nextPathComponent.disambiguation)
283+
}
284+
let onlyPossibleMatch: Node?
285+
286+
if possibleMatchesOneLevelDown.count == 1 {
287+
// Only one of the collisions found a match for the next path component
288+
onlyPossibleMatch = possibleMatchesOneLevelDown.first!
289+
} else if !possibleMatchesOneLevelDown.isEmpty, possibleMatchesOneLevelDown.dropFirst().allSatisfy({ $0.symbol?.identifier.precise == possibleMatchesOneLevelDown.first!.symbol?.identifier.precise }) {
290+
// It's also possible that different language representations of the same symbols appear as different collisions.
291+
// If _all_ collisions that can find the next path component are the same symbol, then we prefer the Swift version of that symbol.
292+
onlyPossibleMatch = possibleMatchesOneLevelDown.first(where: { $0.symbol?.identifier.interfaceLanguage == "swift" }) ?? possibleMatchesOneLevelDown.first!
300293
} else {
301-
// Otherwise we continue looping over the remaining path components.
302-
node = onlyPossibleMatch
303-
continue
294+
onlyPossibleMatch = nil
295+
}
296+
297+
if let onlyPossibleMatch {
298+
// If we found only a single match one level down then we've processed both this path component and the next.
299+
remaining = remaining.dropFirst(2)
300+
if remaining.isEmpty {
301+
// If that was the end of the path we can simply return the result.
302+
return onlyPossibleMatch
303+
} else {
304+
// Otherwise we continue looping over the remaining path components.
305+
node = onlyPossibleMatch
306+
continue
307+
}
304308
}
309+
310+
// Couldn't resolve the collision by look ahead.
311+
return try handleWrappedCollision()
305312
}
306-
307-
// Couldn't resolve the collision by look ahead.
308-
return try handleCollision(node: node, remaining: remaining, collisions: collisions, onlyFindSymbols: onlyFindSymbols, rawPathForError: rawPathForError)
309313
}
310314
}
315+
316+
// Run the core implementation, defined above.
317+
let node = try _innerImplementation(descendingFrom: startingPoint, pathComponents: pathComponents, onlyFindSymbols: onlyFindSymbols, rawPathForError: rawPathForError)
318+
319+
// Perform extra validation on the return value before returning it to the caller.
320+
if node.identifier == nil {
321+
throw Error.unfindableMatch(node)
322+
}
323+
if onlyFindSymbols, node.symbol == nil {
324+
throw Error.nonSymbolMatchForSymbolLink(path: rawPathForError)
325+
}
326+
return node
311327
}
312328

313329
private func handleCollision(

‎Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1454,6 +1454,49 @@ class PathHierarchyTests: XCTestCase {
14541454
try assertFindsPath("Inner/InnerClass/something()", in: tree, asSymbolID: "s:5Inner0A5ClassC5OuterE9somethingyyF")
14551455
}
14561456

1457+
func testContinuesSearchingIfNonSymbolMatchesSymbolLink() throws {
1458+
let exampleDocumentation = Folder(name: "CatalogName.docc", content: [
1459+
JSONFile(name: "ModuleName.symbols.json", content: makeSymbolGraph(moduleName: "ModuleName", symbols: [
1460+
("some-class-id", .swift, ["SomeClass"], .class),
1461+
])),
1462+
1463+
TextFile(name: "Some-Article.md", utf8Content: """
1464+
# Some article
1465+
1466+
An article with a heading with the same name as a symbol and another heading.
1467+
1468+
### SomeClass
1469+
1470+
- ``SomeClass``
1471+
1472+
### OtherHeading
1473+
"""),
1474+
])
1475+
let catalogURL = try exampleDocumentation.write(inside: createTemporaryDirectory())
1476+
let (_, _, context) = try loadBundle(from: catalogURL)
1477+
let tree = context.linkResolver.localResolver.pathHierarchy
1478+
1479+
XCTAssert(context.problems.isEmpty, "Unexpected problems \(context.problems.map(\.diagnostic.summary))")
1480+
1481+
let articleID = try tree.find(path: "/CatalogName/Some-Article", onlyFindSymbols: false)
1482+
1483+
XCTAssertEqual(try tree.findNode(path: "SomeClass", onlyFindSymbols: false, parent: articleID).symbol?.identifier.precise, nil,
1484+
"A general documentation link will find the heading because its closer than the symbol.")
1485+
XCTAssertEqual(try tree.findNode(path: "SomeClass", onlyFindSymbols: true, parent: articleID).symbol?.identifier.precise, "some-class-id",
1486+
"A symbol link will skip the heading and continue searching until it finds the symbol.")
1487+
1488+
XCTAssertEqual(try tree.findNode(path: "OtherHeading", onlyFindSymbols: false, parent: articleID).symbol?.identifier.precise, nil,
1489+
"A general documentation link will find the other heading.")
1490+
XCTAssertThrowsError(
1491+
try tree.findNode(path: "OtherHeading", onlyFindSymbols: true, parent: articleID),
1492+
"A symbol link that find the header but doesn't find a symbol will raise the error about the heading not being a symbol"
1493+
) { untypedError in
1494+
let error = untypedError as! PathHierarchy.Error
1495+
let referenceError = error.makeTopicReferenceResolutionErrorInfo() { context.linkResolver.localResolver.fullName(of: $0, in: context) }
1496+
XCTAssertEqual(referenceError.message, "Symbol links can only resolve symbols")
1497+
}
1498+
}
1499+
14571500
func testSnippets() throws {
14581501
let (_, context) = try testBundleAndContext(named: "Snippets")
14591502
let tree = context.linkResolver.localResolver.pathHierarchy

0 commit comments

Comments
 (0)