Skip to content
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

Use only canonical paths for symbol breadcrumbs #1081

Merged
Merged
Show file tree
Hide file tree
Changes from 10 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
@@ -0,0 +1,62 @@
/*
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 Foundation
import SymbolKit

extension PathHierarchyBasedLinkResolver {

///
/// Finds the canonical path, also called "breadcrumbs", to the given symbol in the path hierarchy.
/// The path is a list of references that describe a walk through the path hierarchy descending from the module down to, but not including, the given `reference`.
///
/// - Parameters:
/// - reference: The symbol reference to find the canonical path to.
/// - sourceLanguage: The source language representation of the symbol to fin the canonical path for.
/// - Returns: The canonical path to the given symbol reference, or `nil` if the reference isn't a symbol or if the symbol doesn't have a representation in the given source language.
func breadcrumbs(of reference: ResolvedTopicReference, in sourceLanguage: SourceLanguage) -> [ResolvedTopicReference]? {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe use a singular name for this since it only returns a single breadcrumb, not a list of breadcrumbs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I consider a path to be a list of breadcrumbs and a single breadcrumb to only be one path component.

I see that I used singular "breadcrumb" in the documentation comment. I'll update that documentation to say "breadcrumbs"

Copy link
Contributor Author

@d-ronnqvist d-ronnqvist Nov 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed to plural "breadcrumbs" in the documentation comment in cc1e626

guard let nodeID = resolvedReferenceMap[reference] else { return nil }
var node = pathHierarchy.lookup[nodeID]! // Only the path hierarchy can create its IDs and a created ID always matches a node

func matchesRequestedLanguage(_ node: PathHierarchy.Node) -> Bool {
guard let symbol = node.symbol,
let language = SourceLanguage(knownLanguageIdentifier: symbol.identifier.interfaceLanguage)
else {
return false
}
return language == sourceLanguage
}

if !matchesRequestedLanguage(node) {
guard let counterpart = node.counterpart, matchesRequestedLanguage(counterpart) else {
// Neither this symbol, nor its counterpart matched the requested language
return nil
}
// Traverse from the counterpart instead because it matches the requested language
node = counterpart
}

// Traverse up the hierarchy and gather each reference
return sequence(first: node, next: \.parent)
// The hierarchy traversal happened from the starting point up, but the callers of `breadcrumbs(of:in:)`
// expect paths going from the root page, excluding the starting point itself (already dropped above).
// To match the caller's expectations we remove the starting point and then flip the paths.
.dropFirst().reversed()
.compactMap {
// Ignore any "unfindable" or "sparse" nodes resulting from a "partial" symbol graph.
//
// When the `ConvertService` builds documentation for a single symbol with multiple path components,
// the path hierarchy fills in unfindable nodes for the other path components to construct a connected hierarchy.
//
// These unfindable nodes can be traversed up and down, but are themselves considered to be "not found".
$0.identifier.flatMap { resolvedReferenceMap[$0] }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we assert on any nils? Or just remove them as you have done here? My guess is the PathHierarchy code is already well tested and has any asserts we would want.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, but I can add a comment that mentions why nil could be expected here.

The way that the ConvertService builds documentation for a single symbol may result in nodes in the hierarchy which doesn't correspond to any real symbols. The PathHierarchy internally refers to these as "sparse nodes". They only exist to construct a tree structure but links to them can't resolve. The ConvertService doesn't create any breadcrumbs, so handling nil here is possibly unnecessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a code comment in 8647adf

}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,13 @@ struct RenderHierarchyTranslator {
/// - reference: A reference to a tutorials-related topic.
/// - 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 visitTutorialTableOfContentsNode(_ reference: ResolvedTopicReference, omittingChapters: Bool = false) -> (hierarchy: RenderHierarchy, tutorialTableOfContents: ResolvedTopicReference)? {
mutating func visitTutorialTableOfContentsNode(_ reference: ResolvedTopicReference, omittingChapters: Bool = false) -> (hierarchyVariants: VariantCollection<RenderHierarchy?>, tutorialTableOfContents: ResolvedTopicReference)? {
let paths = context.finitePaths(to: reference, options: [.preferTutorialTableOfContentsRoot])

// If the node is a tutorial table-of-contents page, return immediately without generating breadcrumbs
if let _ = (try? context.entity(with: reference))?.semantic as? TutorialTableOfContents {
let hierarchy = visitTutorialTableOfContents(reference, omittingChapters: omittingChapters)
return (hierarchy: .tutorials(hierarchy), tutorialTableOfContents: reference)
return (hierarchyVariants: .init(defaultValue: .tutorials(hierarchy)), tutorialTableOfContents: reference)
}

guard let tutorialsPath = paths.mapFirst(where: { path -> [ResolvedTopicReference]? in
Expand All @@ -62,7 +62,7 @@ struct RenderHierarchyTranslator {
.sorted { lhs, _ in lhs == tutorialsPath }
.map { $0.map { $0.absoluteString } }

return (hierarchy: .tutorials(hierarchy), tutorialTableOfContents: tutorialTableOfContentsReference)
return (hierarchyVariants: .init(defaultValue: .tutorials(hierarchy)), tutorialTableOfContents: tutorialTableOfContentsReference)
}

/// Returns the hierarchy under a given tutorials table-of-contents page.
Expand Down Expand Up @@ -192,19 +192,42 @@ struct RenderHierarchyTranslator {
/// The documentation model is a graph (and not a tree) so you can curate API symbols
/// 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.finitePaths(to: symbolReference)
pathReferences.forEach({
collectedTopicReferences.formUnion($0)
})
let paths = pathReferences.map { $0.map { $0.absoluteString } }
return .reference(RenderReferenceHierarchy(paths: paths))
mutating func visitSymbol(_ symbolReference: ResolvedTopicReference) -> VariantCollection<RenderHierarchy?> {
// An inner function used to create a RenderHierarchy from a list of references and collect the unique references
func makeHierarchy(_ references: [ResolvedTopicReference]) -> RenderHierarchy {
collectedTopicReferences.formUnion(references)
let paths = references.map(\.absoluteString)
return .reference(.init(paths: [paths]))
}

let mainPathReferences = context.linkResolver.localResolver.breadcrumbs(of: symbolReference, in: symbolReference.sourceLanguage)

var hierarchyVariants = VariantCollection<RenderHierarchy?>(
defaultValue: mainPathReferences.map(makeHierarchy) // It's possible that the symbol only has a language representation in a variant language
)

for language in symbolReference.sourceLanguages where language != symbolReference.sourceLanguage {
guard let variantPathReferences = context.linkResolver.localResolver.breadcrumbs(of: symbolReference, in: language) else {
continue
}
hierarchyVariants.variants.append(.init(
traits: [.interfaceLanguage(language.id)],
patch: [.replace(value: makeHierarchy(variantPathReferences))]
))
}

return hierarchyVariants
}

/// Returns the hierarchy under a given article.
/// - Parameter symbolReference: The reference to the article.
/// - Parameter articleReference: The reference to the article.
/// - Returns: The framework hierarchy that describes all paths where the article is curated.
mutating func visitArticle(_ symbolReference: ResolvedTopicReference) -> RenderHierarchy {
return visitSymbol(symbolReference)
mutating func visitArticle(_ articleReference: ResolvedTopicReference) -> VariantCollection<RenderHierarchy?> {
let pathReferences = context.finitePaths(to: articleReference)
pathReferences.forEach({
collectedTopicReferences.formUnion($0)
})
let paths = pathReferences.map { $0.map { $0.absoluteString } }
return .init(defaultValue: .reference(RenderReferenceHierarchy(paths: paths)))
}
}
8 changes: 7 additions & 1 deletion Sources/SwiftDocC/Model/Rendering/RenderNode.swift
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,14 @@ public struct RenderNode: VariantContainer {
/// The key for each reference is the ``RenderReferenceIdentifier/identifier`` of the reference's ``RenderReference/identifier``.
public var references: [String: RenderReference] = [:]

@available(*, deprecated, message: "Use 'hierarchyVariants' instead. This deprecated API will be removed after 6.2 is released")
public var hierarchy: RenderHierarchy? {
get { hierarchyVariants.defaultValue }
set { hierarchyVariants.defaultValue = newValue }
}

/// Hierarchy information about the context in which this documentation node is placed.
public var hierarchy: RenderHierarchy?
public var hierarchyVariants: VariantCollection<RenderHierarchy?> = .init(defaultValue: nil)

/// Arbitrary metadata information about the render node.
public var metadata = RenderMetadata()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ extension RenderNode: Codable {
references = try (container.decodeIfPresent([String: CodableRenderReference].self, forKey: .references) ?? [:]).mapValues({$0.reference})
metadata = try container.decode(RenderMetadata.self, forKey: .metadata)
kind = try container.decode(Kind.self, forKey: .kind)
hierarchy = try container.decodeIfPresent(RenderHierarchy.self, forKey: .hierarchy)
hierarchyVariants = try container.decodeVariantCollection(ofValueType: RenderHierarchy?.self, forKey: .hierarchy)
topicSectionsStyle = try container.decodeIfPresent(TopicsSectionStyle.self, forKey: .topicSectionsStyle) ?? .list

primaryContentSectionsVariants = try container.decodeVariantCollectionArrayIfPresent(
Expand Down Expand Up @@ -79,7 +79,7 @@ extension RenderNode: Codable {

try container.encode(metadata, forKey: .metadata)
try container.encode(kind, forKey: .kind)
try container.encode(hierarchy, forKey: .hierarchy)
try container.encodeVariantCollection(hierarchyVariants, forKey: .hierarchy, encoder: encoder)
if topicSectionsStyle != .list {
try container.encode(topicSectionsStyle, forKey: .topicSectionsStyle)
}
Expand Down
21 changes: 10 additions & 11 deletions Sources/SwiftDocC/Model/Rendering/RenderNodeTranslator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ public struct RenderNodeTranslator: SemanticVisitor {

if let hierarchy = hierarchyTranslator.visitTutorialTableOfContentsNode(identifier) {
let tutorialTableOfContents = try! context.entity(with: hierarchy.tutorialTableOfContents).semantic as! TutorialTableOfContents
node.hierarchy = hierarchy.hierarchy
node.hierarchyVariants = hierarchy.hierarchyVariants
node.metadata.category = tutorialTableOfContents.name
node.metadata.categoryPathComponent = hierarchy.tutorialTableOfContents.url.lastPathComponent
} else if !context.configuration.convertServiceConfiguration.allowsRegisteringArticlesWithoutTechnologyRoot {
Expand Down Expand Up @@ -402,11 +402,10 @@ public struct RenderNodeTranslator: SemanticVisitor {
}

var hierarchyTranslator = RenderHierarchyTranslator(context: context, bundle: bundle)
node.hierarchy = hierarchyTranslator
.visitTutorialTableOfContentsNode(identifier, omittingChapters: true)!
.hierarchy

collectedTopicReferences.append(contentsOf: hierarchyTranslator.collectedTopicReferences)
if let (hierarchyVariants, _) = hierarchyTranslator.visitTutorialTableOfContentsNode(identifier, omittingChapters: true) {
node.hierarchyVariants = hierarchyVariants
collectedTopicReferences.append(contentsOf: hierarchyTranslator.collectedTopicReferences)
}

node.references = createTopicRenderReferences()

Expand Down Expand Up @@ -626,9 +625,9 @@ public struct RenderNodeTranslator: SemanticVisitor {
let documentationNode = try! context.entity(with: identifier)

var hierarchyTranslator = RenderHierarchyTranslator(context: context, bundle: bundle)
let hierarchy = hierarchyTranslator.visitArticle(identifier)
let hierarchyVariants = hierarchyTranslator.visitArticle(identifier)
collectedTopicReferences.append(contentsOf: hierarchyTranslator.collectedTopicReferences)
node.hierarchy = hierarchy
node.hierarchyVariants = hierarchyVariants

// Emit variants only if we're not compiling an article-only catalog to prevent renderers from
// advertising the page as "Swift", which is the language DocC assigns to pages in article only pages.
Expand Down Expand Up @@ -896,7 +895,7 @@ public struct RenderNodeTranslator: SemanticVisitor {

// Unlike for other pages, in here we use `RenderHierarchyTranslator` to crawl the technology
// and produce the list of modules for the render hierarchy to display in the tutorial local navigation.
node.hierarchy = hierarchy.hierarchy
node.hierarchyVariants = hierarchy.hierarchyVariants

let documentationNode = try! context.entity(with: identifier)
node.variants = variants(for: documentationNode)
Expand Down Expand Up @@ -1334,9 +1333,9 @@ public struct RenderNodeTranslator: SemanticVisitor {
node.metadata.tags = contentRenderer.tags(for: identifier)

var hierarchyTranslator = RenderHierarchyTranslator(context: context, bundle: bundle)
let hierarchy = hierarchyTranslator.visitSymbol(identifier)
let hierarchyVariants = hierarchyTranslator.visitSymbol(identifier)
collectedTopicReferences.append(contentsOf: hierarchyTranslator.collectedTopicReferences)
node.hierarchy = hierarchy
node.hierarchyVariants = hierarchyVariants

// In case `inheritDocs` is disabled and there is actually origin data for the symbol, then include origin information as abstract.
// Generate the placeholder abstract only in case there isn't an authored abstract coming from a doc extension.
Expand Down
83 changes: 83 additions & 0 deletions Tests/SwiftDocCTests/Infrastructure/SymbolBreadcrumbTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
/*
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

class SymbolBreadcrumbTests: XCTestCase {
func testLanguageSpecificBreadcrumbs() throws {
let (_, context) = try testBundleAndContext(named: "GeometricalShapes")
let resolver = try XCTUnwrap(context.linkResolver.localResolver)
let moduleReference = try XCTUnwrap(context.soleRootModuleReference)

// typedef struct {
// CGPoint center;
// CGFloat radius;
// } TLACircle NS_SWIFT_NAME(Circle);
do {
let reference = moduleReference.appendingPath("Circle/center")

XCTAssertEqual(resolver.breadcrumbs(of: reference, in: .swift)?.map(\.path), [
"/documentation/GeometricalShapes",
"/documentation/GeometricalShapes/Circle",
])
XCTAssertEqual(resolver.breadcrumbs(of: reference, in: .objectiveC)?.map(\.path), [
"/documentation/GeometricalShapes",
"/documentation/GeometricalShapes/Circle", // named TLACircle in Objective-C
])
}

// extern const TLACircle TLACircleZero NS_SWIFT_NAME(Circle.zero);
do {
let reference = moduleReference.appendingPath("Circle/zero")

XCTAssertEqual(resolver.breadcrumbs(of: reference, in: .swift)?.map(\.path), [
"/documentation/GeometricalShapes",
"/documentation/GeometricalShapes/Circle", // The Swift representation is a member
])
XCTAssertEqual(resolver.breadcrumbs(of: reference, in: .objectiveC)?.map(\.path), [
"/documentation/GeometricalShapes", // The Objective-C representation is a top-level function
])
}

// BOOL TLACircleIntersects(TLACircle circle, TLACircle otherCircle) NS_SWIFT_NAME(Circle.intersects(self:_:));
do {
let reference = moduleReference.appendingPath("Circle/intersects(_:)")

XCTAssertEqual(resolver.breadcrumbs(of: reference, in: .swift)?.map(\.path), [
"/documentation/GeometricalShapes",
"/documentation/GeometricalShapes/Circle", // The Swift representation is a member
])
XCTAssertEqual(resolver.breadcrumbs(of: reference, in: .objectiveC)?.map(\.path), [
"/documentation/GeometricalShapes", // The Objective-C representation is a top-level function
])
}

// TLACircle TLACircleMake(CGPoint center, CGFloat radius) NS_SWIFT_UNAVAILABLE("Use 'Circle.init(center:radius:)' instead.");
do {
let reference = moduleReference.appendingPath("TLACircleMake")

XCTAssertEqual(resolver.breadcrumbs(of: reference, in: .swift)?.map(\.path), nil) // There is no Swift representation
XCTAssertEqual(resolver.breadcrumbs(of: reference, in: .objectiveC)?.map(\.path), [
"/documentation/GeometricalShapes", // The Objective-C representation is a top-level function
])
}

do {
let reference = moduleReference.appendingPath("Circle/init(center:radius:)")

XCTAssertEqual(resolver.breadcrumbs(of: reference, in: .swift)?.map(\.path), [
"/documentation/GeometricalShapes",
"/documentation/GeometricalShapes/Circle", // The Swift representation is a member
])
XCTAssertEqual(resolver.breadcrumbs(of: reference, in: .objectiveC)?.map(\.path), nil) // There is no Objective-C representation
}
}
}
Loading