Skip to content

Commit d9b9206

Browse files
snprajwalsnprajwal
committed
Only curate external non-symbols in different lang
PR #757 enabled DocC to include manually curated non-symbol nodes in the topics section, irrespective of the node language. This allows curating articles across documentation in different languages, even though the article's language is considered to be "Swift". However, the logic was too permissive, and allowed curating non-symbol nodes within the same bundle but across different languages. This is incorrect, since the target page should be annotated with the `SupportedLanguage` directive to enable curating it in a different language. A bespoke mechanism to curate an internal reference correctly is not necessary. This patch tightens the logic to only allow curating external references in a different language than the source page language, and removes the existing test for the undesired behaviour of including local references to cross-language symbols. The changes in PR #757 were not propagated to the navigator, resulting in the references being present in the topics section but not in the sidebar. This patch also introduces the curation logic in the navigator to maintain parity with the topics section. The navigator tests for external render nodes have been modified accordingly to reflect this change. rdar://160284853
1 parent 88ee25b commit d9b9206

File tree

5 files changed

+46
-103
lines changed

5 files changed

+46
-103
lines changed

‎Sources/SwiftDocC/Indexing/Navigator/NavigatorIndex.swift‎

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -455,6 +455,17 @@ extension NavigatorIndex {
455455
self.fragment = fragment
456456
self.languageIdentifier = languageIdentifier
457457
}
458+
459+
/// Compare an identifier with another one, ignoring the identifier language.
460+
///
461+
/// Used when curating cross-language references in multi-language frameworks.
462+
///
463+
/// - Parameter other: The other identifier to compare with.
464+
func isEquivalentIgnoringLanguage(to other: Identifier) -> Bool {
465+
return self.bundleIdentifier == other.bundleIdentifier &&
466+
self.path == other.path &&
467+
self.fragment == other.fragment
468+
}
458469
}
459470

460471
/**
@@ -917,7 +928,7 @@ extension NavigatorIndex {
917928
let (nodeID, parent) = nodesMultiCurated[index]
918929
let placeholders = identifierToChildren[nodeID]!
919930
for reference in placeholders {
920-
if let child = identifierToNode[reference] {
931+
if let child = identifierToNode[reference] ?? externalNonSymbolNode(for: reference) {
921932
parent.add(child: child)
922933
pendingUncuratedReferences.remove(reference)
923934
if !multiCurated.keys.contains(reference) && reference.fragment == nil {
@@ -938,7 +949,7 @@ extension NavigatorIndex {
938949
for (nodeIdentifier, placeholders) in identifierToChildren {
939950
for reference in placeholders {
940951
let parent = identifierToNode[nodeIdentifier]!
941-
if let child = identifierToNode[reference] {
952+
if let child = identifierToNode[reference] ?? externalNonSymbolNode(for: reference) {
942953
let needsCopy = multiCurated[reference] != nil
943954
parent.add(child: (needsCopy) ? child.copy() : child)
944955
pendingUncuratedReferences.remove(reference)
@@ -969,14 +980,11 @@ extension NavigatorIndex {
969980
// page types as symbol nodes on the assumption that an unknown page type is a
970981
// symbol kind added in a future version of Swift-DocC.
971982
// Finally, don't add external references to the root; if they are not referenced within the navigation tree, they should be dropped altogether.
972-
if let node = identifierToNode[nodeID], PageType(rawValue: node.item.pageType)?.isSymbolKind == false , !node.item.isExternal {
983+
if let node = identifierToNode[nodeID], PageType(rawValue: node.item.pageType)?.isSymbolKind == false, !node.item.isExternal {
973984

974985
// If an uncurated page has been curated in another language, don't add it to the top-level.
975986
if curatedReferences.contains(where: { curatedNodeID in
976-
// Compare all the identifier's properties for equality, except for its language.
977-
curatedNodeID.bundleIdentifier == nodeID.bundleIdentifier
978-
&& curatedNodeID.path == nodeID.path
979-
&& curatedNodeID.fragment == nodeID.fragment
987+
curatedNodeID.isEquivalentIgnoringLanguage(to: nodeID)
980988
}) {
981989
continue
982990
}
@@ -1256,7 +1264,19 @@ extension NavigatorIndex {
12561264
problem = Problem(diagnostic: diagnostic, possibleSolutions: [])
12571265
problems.append(problem)
12581266
}
1259-
1267+
1268+
/// Find an external node for the reference that is not of a symbol kind. The source language
1269+
/// of the reference is ignored during this lookup since the reference assumes the target node
1270+
/// to be of the same language as the page that it is curated in. This may or may not be true
1271+
/// since non-symbol kinds (articles, tutorials, etc.) are not tied to a language.
1272+
func externalNonSymbolNode(for reference: NavigatorIndex.Identifier) -> NavigatorTree.Node? {
1273+
identifierToNode
1274+
.first { identifier, node in
1275+
identifier.isEquivalentIgnoringLanguage(to: reference)
1276+
&& PageType.init(rawValue: node.item.pageType)?.isSymbolKind == false
1277+
&& node.item.isExternal
1278+
}?.value
1279+
}
12601280

12611281
/// Build the index using the render nodes files in the provided documentation archive.
12621282
/// - Returns: A list containing all the errors encountered during indexing.

‎Sources/SwiftDocC/Infrastructure/DocumentationContext.swift‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2752,6 +2752,11 @@ public class DocumentationContext {
27522752
)
27532753
}
27542754

2755+
/// Returns whether the given reference resolves to an external entity.
2756+
func isExternal(reference: ResolvedTopicReference) -> Bool {
2757+
externalCache[reference] != nil
2758+
}
2759+
27552760
// MARK: - Relationship queries
27562761

27572762
/// Fetch the child nodes of a documentation node with the given `reference`, optionally filtering to only children of the given `kind`.

‎Sources/SwiftDocC/Model/Rendering/RenderNodeTranslator.swift‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1054,10 +1054,10 @@ public struct RenderNodeTranslator: SemanticVisitor {
10541054
return true
10551055
}
10561056

1057-
guard context.isSymbol(reference: reference) else {
1058-
// If the reference corresponds to any kind except Symbol
1059-
// (e.g., Article, Tutorial, SampleCode...), allow the topic
1060-
// to appear independently of the source language it belongs to.
1057+
// If this is a reference to a non-symbol kind (article, tutorial, sample code, etc.),
1058+
// and is external to the bundle, then curate the topic irrespective of the source
1059+
// language of the page or reference.
1060+
if !context.isSymbol(reference: reference) && context.isExternal(reference: reference) {
10611061
return true
10621062
}
10631063

‎Tests/SwiftDocCTests/Indexing/ExternalRenderNodeTests.swift‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -213,18 +213,18 @@ class ExternalRenderNodeTests: XCTestCase {
213213
// Verify that the curated external links are part of the index.
214214
let swiftExternalNodes = renderIndex.interfaceLanguages["swift"]?.first { $0.path == "/documentation/mixedlanguageframework" }?.children?.filter { $0.path?.contains("/path/to/external") ?? false } ?? []
215215
let occExternalNodes = renderIndex.interfaceLanguages["occ"]?.first { $0.path == "/documentation/mixedlanguageframework" }?.children?.filter { $0.path?.contains("/path/to/external") ?? false } ?? []
216-
XCTAssertEqual(swiftExternalNodes.count, 2)
217-
XCTAssertEqual(occExternalNodes.count, 2)
218-
XCTAssertEqual(swiftExternalNodes.map(\.title), ["SwiftArticle", "SwiftSymbol"])
219-
XCTAssertEqual(occExternalNodes.map(\.title), ["ObjCArticle", "ObjCSymbol"])
216+
XCTAssertEqual(swiftExternalNodes.count, 3)
217+
XCTAssertEqual(occExternalNodes.count, 3)
218+
XCTAssertEqual(swiftExternalNodes.map(\.title), ["SwiftArticle", "SwiftSymbol", "ObjCArticle"])
219+
XCTAssertEqual(occExternalNodes.map(\.title), ["SwiftArticle", "ObjCArticle", "ObjCSymbol"])
220220
XCTAssert(swiftExternalNodes.allSatisfy(\.isExternal))
221221
XCTAssert(occExternalNodes.allSatisfy(\.isExternal))
222222
XCTAssert(swiftExternalNodes.first { $0.title == "SwiftArticle" }?.isBeta == false)
223223
XCTAssert(swiftExternalNodes.first { $0.title == "SwiftSymbol" }?.isBeta == true)
224224
XCTAssert(occExternalNodes.first { $0.title == "ObjCArticle" }?.isBeta == true)
225225
XCTAssert(occExternalNodes.first { $0.title == "ObjCSymbol" }?.isBeta == false)
226-
XCTAssertEqual(swiftExternalNodes.map(\.type), ["article", "class"])
227-
XCTAssertEqual(occExternalNodes.map(\.type), ["article", "func"])
226+
XCTAssertEqual(swiftExternalNodes.map(\.type), ["article", "class", "article"])
227+
XCTAssertEqual(occExternalNodes.map(\.type), ["article", "article", "func"])
228228
}
229229

230230
func testNavigatorWithExternalNodesOnlyAddsCuratedNodesToNavigator() async throws {
@@ -280,13 +280,13 @@ class ExternalRenderNodeTests: XCTestCase {
280280
let swiftExternalNodes = renderIndex.interfaceLanguages["swift"]?.first { $0.path == "/documentation/mixedlanguageframework" }?.children?.filter { $0.path?.contains("/path/to/external") ?? false } ?? []
281281
let occExternalNodes = renderIndex.interfaceLanguages["occ"]?.first { $0.path == "/documentation/mixedlanguageframework" }?.children?.filter { $0.path?.contains("/path/to/external") ?? false } ?? []
282282
XCTAssertEqual(swiftExternalNodes.count, 1)
283-
XCTAssertEqual(occExternalNodes.count, 1)
283+
XCTAssertEqual(occExternalNodes.count, 2)
284284
XCTAssertEqual(swiftExternalNodes.map(\.title), ["SwiftArticle"])
285-
XCTAssertEqual(occExternalNodes.map(\.title), ["ObjCSymbol"])
285+
XCTAssertEqual(occExternalNodes.map(\.title), ["SwiftArticle", "ObjCSymbol"])
286286
XCTAssert(swiftExternalNodes.allSatisfy(\.isExternal))
287287
XCTAssert(occExternalNodes.allSatisfy(\.isExternal))
288288
XCTAssertEqual(swiftExternalNodes.map(\.type), ["article"])
289-
XCTAssertEqual(occExternalNodes.map(\.type), ["func"])
289+
XCTAssertEqual(occExternalNodes.map(\.type), ["article", "func"])
290290
}
291291

292292
func testExternalRenderNodeVariantRepresentationWhenIsBeta() throws {

‎Tests/SwiftDocCTests/Model/SemaToRenderNodeMultiLanguageTests.swift‎

Lines changed: 0 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -878,88 +878,6 @@ class SemaToRenderNodeMixedLanguageTests: XCTestCase {
878878
defaultLanguage: .swift
879879
)
880880
}
881-
882-
func testArticlesAreIncludedInAllVariantsTopicsSection() async throws {
883-
let outputConsumer = try await renderNodeConsumer(
884-
for: "MixedLanguageFramework",
885-
configureBundle: { bundleURL in
886-
try """
887-
# ObjCArticle
888-
889-
@Metadata {
890-
@SupportedLanguage(objc)
891-
}
892-
893-
This article has Objective-C as the source language.
894-
895-
## Topics
896-
""".write(to: bundleURL.appendingPathComponent("ObjCArticle.md"), atomically: true, encoding: .utf8)
897-
try """
898-
# SwiftArticle
899-
900-
@Metadata {
901-
@SupportedLanguage(swift)
902-
}
903-
904-
This article has Swift as the source language.
905-
""".write(to: bundleURL.appendingPathComponent("SwiftArticle.md"), atomically: true, encoding: .utf8)
906-
try """
907-
# ``MixedLanguageFramework``
908-
909-
This symbol has a Swift and Objective-C variant.
910-
911-
## Topics
912-
913-
- <doc:ObjCArticle>
914-
- <doc:SwiftArticle>
915-
- ``_MixedLanguageFrameworkVersionNumber``
916-
- ``SwiftOnlyStruct``
917-
918-
""".write(to: bundleURL.appendingPathComponent("MixedLanguageFramework.md"), atomically: true, encoding: .utf8)
919-
}
920-
)
921-
assertIsAvailableInLanguages(
922-
try outputConsumer.renderNode(
923-
withTitle: "ObjCArticle"
924-
),
925-
languages: ["occ"],
926-
defaultLanguage: .objectiveC
927-
)
928-
assertIsAvailableInLanguages(
929-
try outputConsumer.renderNode(
930-
withTitle: "_MixedLanguageFrameworkVersionNumber"
931-
),
932-
languages: ["occ"],
933-
defaultLanguage: .objectiveC
934-
)
935-
936-
let renderNode = try outputConsumer.renderNode(withIdentifier: "MixedLanguageFramework")
937-
938-
// Topic identifiers in the Swift variant of the `MixedLanguageFramework` symbol
939-
let swiftTopicIDs = renderNode.topicSections.flatMap(\.identifiers)
940-
941-
let data = try renderNode.encodeToJSON()
942-
let variantRenderNode = try RenderNodeVariantOverridesApplier()
943-
.applyVariantOverrides(in: data, for: [.interfaceLanguage("occ")])
944-
let objCRenderNode = try RenderJSONDecoder.makeDecoder().decode(RenderNode.self, from: variantRenderNode)
945-
// Topic identifiers in the ObjC variant of the `MixedLanguageFramework` symbol
946-
let objCTopicIDs = objCRenderNode.topicSections.flatMap(\.identifiers)
947-
948-
949-
// Verify that articles are included in the Topics section of both symbol
950-
// variants regardless of their perceived language.
951-
XCTAssertTrue(swiftTopicIDs.contains("doc://org.swift.MixedLanguageFramework/documentation/MixedLanguageFramework/ObjCArticle"))
952-
XCTAssertTrue(swiftTopicIDs.contains("doc://org.swift.MixedLanguageFramework/documentation/MixedLanguageFramework/SwiftArticle"))
953-
XCTAssertTrue(objCTopicIDs.contains("doc://org.swift.MixedLanguageFramework/documentation/MixedLanguageFramework/SwiftArticle"))
954-
XCTAssertTrue(objCTopicIDs.contains("doc://org.swift.MixedLanguageFramework/documentation/MixedLanguageFramework/ObjCArticle"))
955-
956-
// Verify that language specific symbols are dropped from the Topics section in the
957-
// variants for languages where the symbol isn't available.
958-
XCTAssertTrue(swiftTopicIDs.contains("doc://org.swift.MixedLanguageFramework/documentation/MixedLanguageFramework/SwiftOnlyStruct"))
959-
XCTAssertFalse(swiftTopicIDs.contains("doc://org.swift.MixedLanguageFramework/documentation/MixedLanguageFramework/_MixedLanguageFrameworkVersionNumber"))
960-
XCTAssertTrue(objCTopicIDs.contains("doc://org.swift.MixedLanguageFramework/documentation/MixedLanguageFramework/_MixedLanguageFrameworkVersionNumber"))
961-
XCTAssertFalse(objCTopicIDs.contains("doc://org.swift.MixedLanguageFramework/documentation/MixedLanguageFramework/SwiftOnlyStruct"))
962-
}
963881

964882
func testAutomaticSeeAlsoSectionElementLimit() async throws {
965883
let (bundle, context) = try await loadBundle(catalog:

0 commit comments

Comments
 (0)