Skip to content

Include symbols without common lang in navigator #1269

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Include symbols without common lang in navigator #1269

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
66 changes: 51 additions & 15 deletions Sources/SwiftDocC/Indexing/Navigator/NavigatorIndex.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,12 +230,12 @@ public class NavigatorIndex {
}

/**
Initialize an `NavigatorIndex` from a given path with an empty tree.
Initialize a `NavigatorIndex` from a given path with an empty tree.

- Parameter url: The URL pointing to the path from which the index should be read.
- Parameter bundleIdentifier: The name of the bundle the index is referring to.

- Note: Don't exposed this initializer as it's used **ONLY** for building an index.
- Note: Don't expose this initializer as it's used **ONLY** for building an index.
*/
fileprivate init(withEmptyTree url: URL, bundleIdentifier: String) throws {
self.url = url
Expand Down Expand Up @@ -466,6 +466,17 @@ extension NavigatorIndex {
self.fragment = fragment
self.languageIdentifier = languageIdentifier
}

/// Compare an identifier with another one, ignoring the identifier language.
///
/// Used when curating cross-language references in multi-language frameworks.
///
/// - Parameter other: The other identifier to compare with.
func isEquivalentIgnoringLanguage(to other: Identifier) -> Bool {
return self.bundleIdentifier == other.bundleIdentifier &&
self.path == other.path &&
self.fragment == other.fragment
}
}

/**
Expand Down Expand Up @@ -916,7 +927,7 @@ extension NavigatorIndex {
/// - emitJSONRepresentation: Whether or not a JSON representation of the index should
/// be written to disk.
///
/// Defaults to `false`.
/// Defaults to `true`.
///
/// - emitLMDBRepresentation: Whether or not an LMDB representation of the index should
/// written to disk.
Expand Down Expand Up @@ -949,14 +960,28 @@ extension NavigatorIndex {
let (nodeID, parent) = nodesMultiCurated[index]
let placeholders = identifierToChildren[nodeID]!
for reference in placeholders {
if let child = identifierToNode[reference] {
var child = identifierToNode[reference]
var childReference = reference

// If no child node exists in this language, try to find one across all available languages.
if child == nil {
if let match = identifierToNode.first(where: { (identifier, node) in
identifier.isEquivalentIgnoringLanguage(to: reference) &&
PageType(rawValue: node.item.pageType)?.isSymbolKind == true
}) {
childReference = match.key
child = match.value
}
}

if let child = child {
parent.add(child: child)
pendingUncuratedReferences.remove(reference)
if !multiCurated.keys.contains(reference) && reference.fragment == nil {
pendingUncuratedReferences.remove(childReference)
if !multiCurated.keys.contains(childReference) && childReference.fragment == nil {
// As the children of a multi-curated node is itself curated multiple times
// we need to process it as well, ignoring items with fragments as those are sections.
nodesMultiCuratedChildren.append((reference, child))
multiCurated[reference] = child
nodesMultiCuratedChildren.append((childReference, child))
multiCurated[childReference] = child
}
}
}
Expand All @@ -970,10 +995,24 @@ extension NavigatorIndex {
for (nodeIdentifier, placeholders) in identifierToChildren {
for reference in placeholders {
let parent = identifierToNode[nodeIdentifier]!
if let child = identifierToNode[reference] {
let needsCopy = multiCurated[reference] != nil
var child = identifierToNode[reference]
var childReference = reference

// If no child node exists in this language, try to find one across all available languages.
if child == nil {
if let match = identifierToNode.first(where: { (identifier, node) in
identifier.isEquivalentIgnoringLanguage(to: reference) &&
PageType(rawValue: node.item.pageType)?.isSymbolKind == true
}) {
childReference = match.key
child = match.value
}
}

if let child = child {
let needsCopy = multiCurated[childReference] != nil
parent.add(child: (needsCopy) ? child.copy() : child)
pendingUncuratedReferences.remove(reference)
pendingUncuratedReferences.remove(childReference)
}
}
}
Expand Down Expand Up @@ -1005,10 +1044,7 @@ extension NavigatorIndex {

// If an uncurated page has been curated in another language, don't add it to the top-level.
if curatedReferences.contains(where: { curatedNodeID in
// Compare all the identifier's properties for equality, except for its language.
curatedNodeID.bundleIdentifier == nodeID.bundleIdentifier
&& curatedNodeID.path == nodeID.path
&& curatedNodeID.fragment == nodeID.fragment
curatedNodeID.isEquivalentIgnoringLanguage(to: nodeID)
}) {
continue
}
Expand Down
26 changes: 26 additions & 0 deletions Tests/SwiftDocCTests/Indexing/NavigatorIndexTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -894,6 +894,32 @@ Root
)
}

/// Test the curation of a symbol in a page where the page language is different from the symbol language.
/// This occurs in multi-language frameworks when a symbol in one language is referenced in another, e.g.
/// a Swift-only symbol includes a link to an Objective-C only symbol.
func testCurateSymbolsInPageWithNoCommonLanguage() async throws {
let navigatorIndex = try await generatedNavigatorIndex(
for: "MixedLanguageFrameworkSingleLanguageCuration",
bundleIdentifier: "org.swift.mixedlanguageframework"
)

XCTAssertEqual(
navigatorIndex.navigatorTree.root.children
.first { $0.item.title == "Swift" }?
.children
.first { $0.item.title == "MixedLanguageFramework" }?
.children
.first { $0.item.title == "SwiftOnlyStruct2" }?
.children
.contains { $0.item.title == "ObjectiveCOnlyClass" },
Comment on lines +911 to +914
Copy link
Contributor

Choose a reason for hiding this comment

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

This behavior doesn't seem correct to me.

If I was browsing the Swift version of this documentation I would expect to not see any symbols that don't have Swift representations.

Similarly, if I was browsing the Objective-C version of this documentation I would expect to not see any symbols that don't have an Objective-C representation. Curating a symbol under a page that it has no overlapping languages with, like this, feels like something we should raise a warning about if we don't already.

IMO if a symbol is only curated in a location that is has no overlapping languages with, we should treat it as uncrated and fall back to the automatic default curation for that symbol.

true,
"""
Expected the Objective-C-only node with title "ObjectiveCOnlyClass" to be curated in the Swift \
navigator tree.
"""
)
}

func testNavigatorIndexUsingPageTitleGeneration() async throws {
let (bundle, context) = try await testBundleAndContext(named: "LegacyBundle_DoNotUseInNewTests")
let renderContext = RenderContext(documentationContext: context, bundle: bundle)
Expand Down
1 change: 0 additions & 1 deletion ...les/MixedLanguageFrameworkSingleLanguageCuration.docc/MixedLanguageFramework.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

These symbols are Objective-C only and curated in multiple places in the catalog.

- ``MultiCuratedObjectiveCOnlyClass1``
- ``MultiCuratedObjectiveCOnlyClass1``
- ``MultiCuratedObjectiveCOnlyClass2``

Expand Down