Skip to content

Commit

Permalink
Better support for relative links when multiple symbols in the hierar…
Browse files Browse the repository at this point in the history
…chy have the same name (#578)

* Walk up the path hierarchy if links fail to resolve in a specific scope

rdar://108672152

Also, check the module's scope if the link couldn't otherwise resolve

rdar://76252171

* Fix test linking to heading that doesn't exist

* Update expression that was very slow to type check

* Fix warning about mutating a captured sendable value

* Remove outdated comment about adding more test assertions

* Update test for old link resolver implementation
  • Loading branch information
d-ronnqvist authored May 5, 2023
1 parent de843c4 commit dd9c44c
Show file tree
Hide file tree
Showing 7 changed files with 1,033 additions and 163 deletions.
296 changes: 153 additions & 143 deletions Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy.swift

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,16 @@ public struct ConformanceSection: Codable, Equatable {
}

// Adds "," or ", and" to the requirements wherever necessary.
let merged = zip(rendered, separators).flatMap({ $0 + [$1] })
+ rendered[separators.count...].flatMap({ $0 })
var merged: [RenderInlineContent] = []
merged.reserveCapacity(rendered.count * 4) // 3 for each constraint and 1 for each separator
for (constraint, separator) in zip(rendered, separators) {
merged.append(contentsOf: constraint)
merged.append(separator)
}
merged.append(contentsOf: rendered.last!)
merged.append(.text("."))

self.constraints = merged + [RenderInlineContent.text(".")]
self.constraints = merged
}

private static let selfPrefix = "Self."
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3040,6 +3040,83 @@ let expected = """
}
}

func testMatchesDocumentationExtensionsRelativeToModule() throws {
try XCTSkipUnless(LinkResolutionMigrationConfiguration.shouldUseHierarchyBasedLinkResolver)

let (_, bundle, context) = try testBundleAndContext(copying: "MixedLanguageFrameworkWithLanguageRefinements") { url in
// Top level symbols, omitting the module name
try """
# ``MyStruct/myStructProperty``
@Metadata {
@DocumentationExtension(mergeBehavior: override)
}
my struct property
""".write(to: url.appendingPathComponent("struct-property.md"), atomically: true, encoding: .utf8)

try """
# ``MyTypeAlias``
@Metadata {
@DocumentationExtension(mergeBehavior: override)
}
my type alias
""".write(to: url.appendingPathComponent("alias.md"), atomically: true, encoding: .utf8)
}

do {
// The resolved reference needs more disambiguation than the documentation extension link did.
let reference = ResolvedTopicReference(bundleIdentifier: bundle.identifier, path: "/documentation/MixedFramework/MyStruct/myStructProperty", sourceLanguage: .swift)

let node = try context.entity(with: reference)
let symbol = try XCTUnwrap(node.semantic as? Symbol)
XCTAssertEqual(symbol.abstract?.plainText, "my struct property", "The abstract should be from the overriding documentation extension.")
}

do {
// The resolved reference needs more disambiguation than the documentation extension link did.
let reference = ResolvedTopicReference(bundleIdentifier: bundle.identifier, path: "/documentation/MixedFramework/MyTypeAlias", sourceLanguage: .swift)

let node = try context.entity(with: reference)
let symbol = try XCTUnwrap(node.semantic as? Symbol)
XCTAssertEqual(symbol.abstract?.plainText, "my type alias", "The abstract should be from the overriding documentation extension.")
}
}

func testCurationOfSymbolsWithSameNameAsModule() throws {
try XCTSkipUnless(LinkResolutionMigrationConfiguration.shouldUseHierarchyBasedLinkResolver)

let (_, bundle, context) = try testBundleAndContext(copying: "SymbolsWithSameNameAsModule") { url in
// Top level symbols, omitting the module name
try """
# ``Something``
This documentation extension covers the module symbol
## Topics
This link curates the top-level struct
- ``Something``
""".write(to: url.appendingPathComponent("something.md"), atomically: true, encoding: .utf8)
}

do {
// The resolved reference needs more disambiguation than the documentation extension link did.
let reference = ResolvedTopicReference(bundleIdentifier: bundle.identifier, path: "/documentation/Something", sourceLanguage: .swift)

let node = try context.entity(with: reference)
let symbol = try XCTUnwrap(node.semantic as? Symbol)
XCTAssertEqual(symbol.abstract?.plainText, "This documentation extension covers the module symbol", "The abstract should be from the overriding documentation extension.")

let topics = try XCTUnwrap(symbol.topics?.taskGroups.first)
XCTAssertEqual(topics.abstract?.paragraph.plainText, "This link curates the top-level struct")
XCTAssertEqual(topics.links.first?.destination, "doc://SymbolsWithSameNameAsModule/documentation/Something/Something")
}
}

func testMultipleDocumentationExtensionMatchDiagnostic() throws {
try XCTSkipUnless(LinkResolutionMigrationConfiguration.shouldUseHierarchyBasedLinkResolver)

Expand Down
48 changes: 48 additions & 0 deletions Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1149,6 +1149,54 @@ class PathHierarchyTests: XCTestCase {
"/ShapeKit/OverloadedEnum/firstTestMemberName(_:)-88rbf")
}

func testSymbolsWithSameNameAsModule() throws {
try XCTSkipUnless(LinkResolutionMigrationConfiguration.shouldUseHierarchyBasedLinkResolver)
let (_, context) = try testBundleAndContext(named: "SymbolsWithSameNameAsModule")
let tree = try XCTUnwrap(context.hierarchyBasedLinkResolver?.pathHierarchy)

// /* in a module named "Something "*/
// public struct Something {
// public enum Something {
// case first
// }
// public var second = 0
// }
// public struct Wrapper {
// public struct Something {
// public var third = 0
// }
// }
try assertFindsPath("Something", in: tree, asSymbolID: "Something")
try assertFindsPath("/Something", in: tree, asSymbolID: "Something")

let moduleID = try tree.find(path: "/Something", onlyFindSymbols: true)
XCTAssertEqual(try tree.findSymbol(path: "/Something", parent: moduleID).identifier.precise, "Something")
XCTAssertEqual(try tree.findSymbol(path: "Something-module", parent: moduleID).identifier.precise, "Something")
XCTAssertEqual(try tree.findSymbol(path: "Something", parent: moduleID).identifier.precise, "s:9SomethingAAV")
XCTAssertEqual(try tree.findSymbol(path: "/Something/Something", parent: moduleID).identifier.precise, "s:9SomethingAAV")
XCTAssertEqual(try tree.findSymbol(path: "Something/Something", parent: moduleID).identifier.precise, "s:9SomethingAAVAAO")
XCTAssertEqual(try tree.findSymbol(path: "Something/Something/Something", parent: moduleID).identifier.precise, "s:9SomethingAAVAAO")
XCTAssertEqual(try tree.findSymbol(path: "/Something/Something/Something", parent: moduleID).identifier.precise, "s:9SomethingAAVAAO")
XCTAssertEqual(try tree.findSymbol(path: "/Something/Something", parent: moduleID).identifier.precise, "s:9SomethingAAV")
XCTAssertEqual(try tree.findSymbol(path: "Something/second", parent: moduleID).identifier.precise, "s:9SomethingAAV6secondSivp")

let topLevelSymbolID = try tree.find(path: "/Something/Something", onlyFindSymbols: true)
XCTAssertEqual(try tree.findSymbol(path: "Something", parent: topLevelSymbolID).identifier.precise, "s:9SomethingAAVAAO")
XCTAssertEqual(try tree.findSymbol(path: "Something/Something", parent: topLevelSymbolID).identifier.precise, "s:9SomethingAAVAAO")
XCTAssertEqual(try tree.findSymbol(path: "Something/second", parent: topLevelSymbolID).identifier.precise, "s:9SomethingAAV6secondSivp")

let wrapperID = try tree.find(path: "/Something/Wrapper", onlyFindSymbols: true)
XCTAssertEqual(try tree.findSymbol(path: "Something/second", parent: wrapperID).identifier.precise, "s:9SomethingAAV6secondSivp")
XCTAssertEqual(try tree.findSymbol(path: "Something/third", parent: wrapperID).identifier.precise, "s:9Something7WrapperVAAV5thirdSivp")

let wrappedID = try tree.find(path: "/Something/Wrapper/Something", onlyFindSymbols: true)
XCTAssertEqual(try tree.findSymbol(path: "Something/second", parent: wrappedID).identifier.precise, "s:9SomethingAAV6secondSivp")
XCTAssertEqual(try tree.findSymbol(path: "Something/third", parent: wrappedID).identifier.precise, "s:9Something7WrapperVAAV5thirdSivp")

XCTAssertEqual(try tree.findSymbol(path: "Something/first", parent: topLevelSymbolID).identifier.precise, "s:9SomethingAAVAAO5firstyA2CmF")
XCTAssertEqual(try tree.findSymbol(path: "Something/second", parent: topLevelSymbolID).identifier.precise, "s:9SomethingAAV6secondSivp")
}

func testSnippets() throws {
try XCTSkipUnless(LinkResolutionMigrationConfiguration.shouldUseHierarchyBasedLinkResolver)
let (_, context) = try testBundleAndContext(named: "Snippets")
Expand Down
18 changes: 7 additions & 11 deletions Tests/SwiftDocCTests/Infrastructure/ReferenceResolverTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -471,21 +471,17 @@ class ReferenceResolverTests: XCTestCase {
try """
# ``ModuleWithSingleExtension``
This is a test module with an extension to ``Swift/Array#Array``.
This is a test module with an extension to ``Swift/Array``.
""".write(to: topLevelArticle, atomically: true, encoding: .utf8)
}

// Make sure that linking to `Swift/Array` raises a diagnostic about the page having been removed
if LinkResolutionMigrationConfiguration.shouldUseHierarchyBasedLinkResolver {
let diagnostic = try XCTUnwrap(context.problems.first(where: { $0.diagnostic.identifier == "org.swift.docc.removedExtensionLinkDestination" }))
XCTAssertEqual(diagnostic.possibleSolutions.count, 1)
let solution = try XCTUnwrap(diagnostic.possibleSolutions.first)
XCTAssertEqual(solution.replacements.count, 1)
let replacement = try XCTUnwrap(solution.replacements.first)
XCTAssertEqual(replacement.replacement, "`Swift/Array`")
} else {
XCTAssert(context.problems.contains(where: { $0.diagnostic.identifier == "org.swift.docc.unresolvedTopicReference" }))
}
let diagnostic = try XCTUnwrap(context.problems.first(where: { $0.diagnostic.identifier == "org.swift.docc.removedExtensionLinkDestination" }))
XCTAssertEqual(diagnostic.possibleSolutions.count, 1)
let solution = try XCTUnwrap(diagnostic.possibleSolutions.first)
XCTAssertEqual(solution.replacements.count, 1)
let replacement = try XCTUnwrap(solution.replacements.first)
XCTAssertEqual(replacement.replacement, "`Swift/Array`")

// Also make sure that the extension pages are still gone
let extendedModule = ResolvedTopicReference(bundleIdentifier: bundle.identifier, path: "/documentation/ModuleWithSingleExtension/Swift", sourceLanguage: .swift)
Expand Down
Loading

0 comments on commit dd9c44c

Please sign in to comment.