Skip to content

[5.10] Use extensionTo and declaredIn relationships to add extensions to path hierarchy #727

New issue

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

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

Already on GitHub? Sign in to your account

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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,27 @@ extension PathHierarchy {
if modules.count == 1 {
do {
return try searchForNode(descendingFrom: modules.first!.value, pathComponents: remaining, parsedPathForError: parsedPathForError, onlyFindSymbols: onlyFindSymbols)
} catch {
// Ignore this error and raise an error about not finding the module instead.
} catch let error as PathHierarchy.Error {
switch error {
case .notFound:
// Ignore this error and raise an error about not finding the module instead.
break
case .unknownName(let partialResult, remaining: _, availableChildren: _):
if partialResult.node.symbol?.kind.identifier == .module {
// Failed to find the first path component. Ignore this error and raise an error about not finding the module instead.
break
} else {
// Partially resolved the link. Raise the more specific error instead of a module-not-found error.
throw error
}

// These errors are all more specific than a module-not-found error would be.
case .unfindableMatch,
.nonSymbolMatchForSymbolLink,
.unknownDisambiguation,
.lookupCollision:
throw error
}
}
}
let topLevelNames = Set(modules.keys + [articlesContainer.name, tutorialContainer.name])
Expand Down Expand Up @@ -193,7 +212,8 @@ extension PathHierarchy {
try handleCollision(node: node, parsedPath: parsedPathForError, remaining: remaining, collisions: collisions, onlyFindSymbols: onlyFindSymbols)
}

// See if the collision can be resolved by looking ahead on level deeper.
// When there's a collision, use the remaining path components to try and narrow down the possible collisions.

guard let nextPathComponent = remaining.dropFirst().first else {
// This was the last path component so there's nothing to look ahead.
//
Expand All @@ -219,18 +239,38 @@ extension PathHierarchy {
// A wrapped error would have been raised while iterating over the collection.
return uniqueCollisions.first!.value
}
// Try resolving the rest of the path for each collision ...
let possibleMatches = collisions.compactMap {

// Look ahead one path component to narrow down the list of collisions.
// For each collision where the next path component can be found unambiguously, return that matching node one level down.
let possibleMatchesOneLevelDown = collisions.compactMap {
return try? $0.node.children[nextPathComponent.name]?.find(nextPathComponent.kind, nextPathComponent.hash)
}
// If only one collision matches, return that match.
if possibleMatches.count == 1 {
return possibleMatches.first!
let onlyPossibleMatch: Node?

if possibleMatchesOneLevelDown.count == 1 {
// Only one of the collisions found a match for the next path component
onlyPossibleMatch = possibleMatchesOneLevelDown.first!
} else if !possibleMatchesOneLevelDown.isEmpty, possibleMatchesOneLevelDown.dropFirst().allSatisfy({ $0.symbol?.identifier.precise == possibleMatchesOneLevelDown.first!.symbol?.identifier.precise }) {
// It's also possible that different language representations of the same symbols appear as different collisions.
// If _all_ collisions that can find the next path component are the same symbol, then we prefer the Swift version of that symbol.
onlyPossibleMatch = possibleMatchesOneLevelDown.first(where: { $0.symbol?.identifier.interfaceLanguage == "swift" }) ?? possibleMatchesOneLevelDown.first!
} else {
onlyPossibleMatch = nil
}
// If all matches are the same symbol, return the Swift version of that symbol
if !possibleMatches.isEmpty, possibleMatches.dropFirst().allSatisfy({ $0.symbol?.identifier.precise == possibleMatches.first!.symbol?.identifier.precise }) {
return possibleMatches.first(where: { $0.symbol?.identifier.interfaceLanguage == "swift" }) ?? possibleMatches.first!

if let onlyPossibleMatch = onlyPossibleMatch {
// If we found only a single match one level down then we've processed both this path component and the next.
remaining = remaining.dropFirst(2)
if remaining.isEmpty {
// If that was the end of the path we can simply return the result.
return onlyPossibleMatch
} else {
// Otherwise we continue looping over the remaining path components.
node = onlyPossibleMatch
continue
}
}

// Couldn't resolve the collision by look ahead.
return try handleCollision(node: node, parsedPath: parsedPathForError, remaining: remaining, collisions: collisions, onlyFindSymbols: onlyFindSymbols)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,19 @@ import SymbolKit
/// All known symbol kind identifiers.
///
/// This is used to identify parsed path components as kind information.
private let knownSymbolKinds = Set(SymbolGraph.Symbol.KindIdentifier.allCases.map { $0.identifier })
private let knownSymbolKinds: Set<String> = {
// There's nowhere else that registers these extended symbol kinds and we need to know them in this list.
SymbolGraph.Symbol.KindIdentifier.register(
.extendedProtocol,
.extendedStructure,
.extendedClass,
.extendedEnumeration,
.unknownExtendedType,
.extendedModule
)
return Set(SymbolGraph.Symbol.KindIdentifier.allCases.map(\.identifier))
}()

/// All known source language identifiers.
///
/// This is used to skip language prefixes from kind disambiguation information.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ struct PathHierarchy {
}

var topLevelCandidates = nodes
for relationship in graph.relationships where [.memberOf, .requirementOf, .optionalRequirementOf].contains(relationship.kind) {
for relationship in graph.relationships where relationship.kind.formsHierarchy {
guard let sourceNode = nodes[relationship.source] else {
continue
}
Expand Down Expand Up @@ -145,6 +145,11 @@ struct PathHierarchy {
// Disfavor the default implementation to favor the protocol requirement (or other symbol with the same path).
sourceNode.isDisfavoredInCollision = true

guard sourceNode.parent == nil else {
// This node already has a direct member-of parent. No need to go via the default-implementation-of relationship to find its location in the hierarchy.
continue
}

let targetNodes = nodes[relationship.target].map { [$0] } ?? allNodes[relationship.target] ?? []
guard !targetNodes.isEmpty else {
continue
Expand Down Expand Up @@ -204,7 +209,10 @@ struct PathHierarchy {

var lookup = [ResolvedIdentifier: Node]()
func descend(_ node: Node) {
assert(node.identifier == nil)
assert(
node.identifier == nil,
"Already encountered \(node.name). This is an indication that a symbol is the source of more than one memberOf relationship."
)
if node.symbol != nil {
node.identifier = ResolvedIdentifier()
lookup[node.identifier] = node
Expand Down Expand Up @@ -460,3 +468,15 @@ extension PathHierarchy.DisambiguationContainer {
}))
}
}

private extension SymbolGraph.Relationship.Kind {
/// Whether or not this relationship kind forms a hierarchical relationship between the source and the target.
var formsHierarchy: Bool {
switch self {
case .memberOf, .requirementOf, .optionalRequirementOf, .extensionTo, .declaredIn:
return true
default:
return false
}
}
}
43 changes: 43 additions & 0 deletions Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1225,6 +1225,49 @@ class PathHierarchyTests: XCTestCase {
XCTAssertEqual(try tree.findSymbol(path: "Something/second", parent: topLevelSymbolID).identifier.precise, "s:9SomethingAAV6secondSivp")
}

func testSymbolsWithSameNameAsExtendedModule() throws {
// ---- Inner
// public struct InnerStruct {}
// public class InnerClass {}
//
// ---- Outer
// // Shadow the Inner module with a local type
// public struct Inner {}
//
// public extension InnerStruct {
// func something() {}
// }
// public extension InnerClass {
// func something() {}
// }
let (_, context) = try testBundleAndContext(named: "ShadowExtendedModuleWithLocalSymbol")
let tree = try XCTUnwrap(context.hierarchyBasedLinkResolver?.pathHierarchy)

try assertPathCollision("Outer/Inner", in: tree, collisions: [
("s:m:s:e:s:5Inner0A5ClassC5OuterE9somethingyyF", "module.extension"),
("s:5Outer5InnerV", "struct"),
])
// If the first path component is ambiguous, it should have the same error as if that was a later path component.
try assertPathCollision("Inner", in: tree, collisions: [
("s:m:s:e:s:5Inner0A5ClassC5OuterE9somethingyyF", "module.extension"),
("s:5Outer5InnerV", "struct"),
])

try assertFindsPath("Inner-struct", in: tree, asSymbolID: "s:5Outer5InnerV")
try assertFindsPath("Inner-module.extension", in: tree, asSymbolID: "s:m:s:e:s:5Inner0A5ClassC5OuterE9somethingyyF")

try assertFindsPath("Inner-module.extension/InnerStruct", in: tree, asSymbolID: "s:e:s:5Inner0A6StructV5OuterE9somethingyyF")
try assertFindsPath("Inner-module.extension/InnerClass", in: tree, asSymbolID: "s:e:s:5Inner0A5ClassC5OuterE9somethingyyF")
try assertFindsPath("Inner-module.extension/InnerStruct/something()", in: tree, asSymbolID: "s:5Inner0A6StructV5OuterE9somethingyyF")
try assertFindsPath("Inner-module.extension/InnerClass/something()", in: tree, asSymbolID: "s:5Inner0A5ClassC5OuterE9somethingyyF")

// The "Inner" struct doesn't have "InnerStruct" or "InnerClass" descendants so the path is not ambiguous.
try assertFindsPath("Inner/InnerStruct", in: tree, asSymbolID: "s:e:s:5Inner0A6StructV5OuterE9somethingyyF")
try assertFindsPath("Inner/InnerClass", in: tree, asSymbolID: "s:e:s:5Inner0A5ClassC5OuterE9somethingyyF")
try assertFindsPath("Inner/InnerStruct/something()", in: tree, asSymbolID: "s:5Inner0A6StructV5OuterE9somethingyyF")
try assertFindsPath("Inner/InnerClass/something()", in: tree, asSymbolID: "s:5Inner0A5ClassC5OuterE9somethingyyF")
}

func testSnippets() throws {
let (_, context) = try testBundleAndContext(named: "Snippets")
let tree = try XCTUnwrap(context.hierarchyBasedLinkResolver?.pathHierarchy)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
{
"metadata": {
"formatVersion": {
"major": 0,
"minor": 6,
"patch": 0
},
"generator": "Apple Swift version 5.9 (swiftlang-5.9.0.128.108 clang-1500.0.40.1)"
},
"module": {
"name": "Outer",
"platform": {
"architecture": "arm64",
"operatingSystem": {
"minimumVersion": {
"major": 14,
"minor": 0
},
"name": "macosx"
},
"vendor": "apple"
}
},
"relationships": [],
"symbols": [
{
"accessLevel": "public",
"declarationFragments": [
{
"kind": "keyword",
"spelling": "struct"
},
{
"kind": "text",
"spelling": " "
},
{
"kind": "identifier",
"spelling": "Inner"
}
],
"identifier": {
"interfaceLanguage": "swift",
"precise": "s:5Outer5InnerV"
},
"kind": {
"displayName": "Structure",
"identifier": "swift.struct"
},
"location": {
"position": {
"character": 14,
"line": 11
},
"uri": "file:///Users/username/path/to/ShadowExtendedModuleWithLocalSymbol/Outer.swift"
},
"names": {
"navigator": [
{
"kind": "identifier",
"spelling": "Inner"
}
],
"subHeading": [
{
"kind": "keyword",
"spelling": "struct"
},
{
"kind": "text",
"spelling": " "
},
{
"kind": "identifier",
"spelling": "Inner"
}
],
"title": "Inner"
},
"pathComponents": [
"Inner"
]
}
]
}
Loading