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

Filter out language-specific topic sections when creating See Also sections #1082

Merged
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 @@ -163,6 +163,7 @@ public struct AutomaticCuration {
return nil
}

// FIXME: The shortest path to the reference may not be applicable to the given variants traits.
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we plan on addressing this soon? Should we open a GitHub issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I started working on this issue but it requires some replacement API that I introduced in a related PR so I put it on hold until those are merged. I opened this issue that refers to this code. I'll add a link in the code comment as well

// First try getting the canonical path from a render context, default to the documentation context
guard let canonicalPath = renderContext?.store.content(for: node.reference)?.canonicalPath ?? context.shortestFinitePath(to: node.reference),
let parentReference = canonicalPath.last
Expand All @@ -171,17 +172,32 @@ public struct AutomaticCuration {
return nil
}

let variantLanguages = Set(variantsTraits.compactMap { traits in
traits.interfaceLanguage.map { SourceLanguage(id: $0) }
})

func isRelevant(_ filteredGroup: DocumentationContentRenderer.ReferenceGroup) -> Bool {
// Check if the task group is filtered to a subset of languages
if let languageFilter = filteredGroup.languageFilter,
languageFilter.isDisjoint(with: variantLanguages)
{
// This group is only applicable to other languages than the given variant traits.
return false
}

// Otherwise, check that the group contains the this reference.
return filteredGroup.references.contains(node.reference)
}

func filterReferences(_ references: [ResolvedTopicReference]) -> [ResolvedTopicReference] {
Array(
references
// Don't include the current node.
.filter { $0 != node.reference }

// Filter out nodes that aren't available in any of the given traits.
.filter { reference in
context.sourceLanguages(for: reference).contains(where: { language in
variantsTraits.contains(where: { $0.interfaceLanguage == language.id})
})
// Don't include the current node.
reference != node.reference
&&
// Don't include nodes that aren't available in any of the given traits.
!context.sourceLanguages(for: reference).isDisjoint(with: variantLanguages)
}
// Don't create too long See Also sections
.prefix(automaticSeeAlsoLimit)
Expand All @@ -190,7 +206,7 @@ public struct AutomaticCuration {

// Look up the render context first
if let taskGroups = renderContext?.store.content(for: parentReference)?.taskGroups,
let linkingGroup = taskGroups.first(where: { $0.references.contains(node.reference) })
let linkingGroup = taskGroups.first(where: isRelevant)
{
// Group match in render context, verify if there are any other references besides the current one.
guard linkingGroup.references.count > 1 else { return nil }
Expand All @@ -203,9 +219,7 @@ public struct AutomaticCuration {
}

// Find the group where the current symbol is curated
let linkingGroup = taskGroups.first { group -> Bool in
group.references.contains(node.reference)
}
let linkingGroup = taskGroups.first(where: isRelevant)

// Verify there is a matching linking group and more references than just the current one.
guard let group = linkingGroup, group.references.count > 1 else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -470,6 +470,7 @@ public class DocumentationContentRenderer {
public struct ReferenceGroup: Codable {
public let title: String?
public let references: [ResolvedTopicReference]
var languageFilter: Set<SourceLanguage>?
}

/// Returns the task groups for a given node reference.
Expand All @@ -489,10 +490,7 @@ public class DocumentationContentRenderer {

guard let taskGroups = groups, !taskGroups.isEmpty else { return nil }

// Find the linking group
var resolvedTaskGroups = [ReferenceGroup]()

for group in taskGroups {
return taskGroups.map { group in
let resolvedReferences = group.links.compactMap { link -> ResolvedTopicReference? in
guard let destination = link.destination.flatMap(URL.init(string:)),
destination.scheme != nil,
Expand All @@ -515,12 +513,16 @@ public class DocumentationContentRenderer {
)
}

resolvedTaskGroups.append(
ReferenceGroup(title: group.heading?.plainText, references: resolvedReferences)
let supportedLanguages = group.directives[SupportedLanguage.directiveName]?.compactMap {
SupportedLanguage(from: $0, source: nil, for: bundle, in: documentationContext)?.language
}

return ReferenceGroup(
title: group.heading?.plainText,
references: resolvedReferences,
languageFilter: supportedLanguages.map { Set($0) }
)
}

return resolvedTaskGroups
}
}

Expand Down
75 changes: 75 additions & 0 deletions Tests/SwiftDocCTests/Model/SemaToRenderNodeTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3349,6 +3349,81 @@ Document
])
}

func testLanguageSpecificTopicSectionDoesNotAppearInAutomaticSeeAlso() throws {
let catalog = Folder(name: "Something.docc", content: [
JSONFile(name: "Something-swift.symbols.json", content: makeSymbolGraph(moduleName: "Something", symbols: (1...4).map {
makeSymbol(id: "symbol-id-\($0)", language: .swift, kind: .class, pathComponents: ["SomeClass\($0)"])
})),

JSONFile(name: "Something-objc.symbols.json", content: makeSymbolGraph(moduleName: "Something", symbols: (1...4).map {
makeSymbol(id: "symbol-id-\($0)", language: .objectiveC, kind: .class, pathComponents: ["SomeClass\($0)"])
})),

TextFile(name: "ModuleExtension.md", utf8Content: """
# ``Something``

## Topics

### Something Swift only

@SupportedLanguage(swift)

- ``SomeClass1``
- ``SomeClass2``
- ``SomeClass3``

### Something Objective-C only

@SupportedLanguage(objc)

- ``SomeClass2``
- ``SomeClass3``
- ``SomeClass4``
"""),
])
let (bundle, context) = try loadBundle(catalog: catalog)
XCTAssert(context.problems.isEmpty, "\(context.problems.map(\.diagnostic.summary))")

let moduleReference = try XCTUnwrap(context.soleRootModuleReference)
let reference = moduleReference.appendingPath("SomeClass3")

let documentationNode = try context.entity(with: reference)
XCTAssertEqual(documentationNode.availableVariantTraits.count, 2, "This page has Swift and Objective-C variants")

// There's a behavioral difference between DocumentationContextConverter and DocumentationNodeConverter so we check both.
// DocumentationContextConverter may use pre-rendered content but the DocumentationNodeConverter computes task groups as-needed.

func assertExpectedTopicSections(_ renderNode: RenderNode, file: StaticString = #filePath, line: UInt = #line) {
let topicSectionsVariants = renderNode.seeAlsoSectionsVariants

let swiftSeeAlsoSection = topicSectionsVariants.defaultValue

XCTAssertEqual(swiftSeeAlsoSection.first?.title, "Something Swift only", file: file, line: line)
XCTAssertEqual(swiftSeeAlsoSection.first?.identifiers, [
"doc://Something/documentation/Something/SomeClass1",
"doc://Something/documentation/Something/SomeClass2",
], file: file, line: line)

let objcSeeAlsoSection = topicSectionsVariants.value(for: [.interfaceLanguage("occ")])

XCTAssertEqual(objcSeeAlsoSection.first?.title, "Something Objective-C only", file: file, line: line)
XCTAssertEqual(objcSeeAlsoSection.first?.identifiers, [
"doc://Something/documentation/Something/SomeClass2",
"doc://Something/documentation/Something/SomeClass4",
], file: file, line: line)
}

let nodeConverter = DocumentationNodeConverter(bundle: bundle, context: context)
try assertExpectedTopicSections(nodeConverter.convert(documentationNode))

let contextConverter = DocumentationContextConverter(
bundle: bundle,
context: context,
renderContext: RenderContext(documentationContext: context, bundle: bundle)
)
try assertExpectedTopicSections(XCTUnwrap(contextConverter.renderNode(for: documentationNode)))
}

func testTopicSectionWithUnsupportedDirectives() throws {
let exampleDocumentation = Folder(name: "unit-test.docc", content: [
TextFile(name: "root.md", utf8Content: """
Expand Down