Skip to content

Add a precondition for declaring a node as beta. #1055

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

Merged
merged 4 commits into from
Oct 14, 2024
Merged

Add a precondition for declaring a node as beta. #1055

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
086aae2
Added a precondition for declaring a node as beta.
sofiaromorales Oct 13, 2024
46fd65d
Update Sources/SwiftDocC/Model/Rendering/DocumentationContentRenderer…
sofiaromorales Oct 14, 2024
fa33727
code cleanup
sofiaromorales Oct 14, 2024
ea6ceac
Merge branch 'main' into 137762221/beta-tag-when-no-availability
sofiaromorales Oct 14, 2024
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
9 changes: 7 additions & 2 deletions Sources/SwiftDocC/Model/Rendering/DocumentationContentRenderer.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,11 +211,16 @@ public class DocumentationContentRenderer {
guard let symbol = node.semantic as? Symbol,
let currentPlatforms = documentationContext.configuration.externalMetadata.currentPlatforms,
!currentPlatforms.isEmpty,
let symbolAvailability = symbol.availability
let symbolAvailability = symbol.availability?.availability
else { return false }

// If the symbol does not have any availability item it can't be `beta`.
guard !symbolAvailability.isEmpty else {
return false
}

// Verify that if current platforms are in beta, they match the introduced version of the symbol
for availability in symbolAvailability.availability {
for availability in symbolAvailability {
// If not available on this platform, skip to next platform.
guard !availability.isUnconditionallyUnavailable, let introduced = availability.introducedVersion else {
continue
Expand Down
16 changes: 16 additions & 0 deletions Tests/SwiftDocCTests/Model/SemaToRenderNodeTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1922,6 +1922,22 @@ Document
XCTAssertEqual(renderNode.metadata.platforms?.first?.isBeta, false)
}

// Symbol with an empty set of availbility items.

do {

let (bundle, context, _) = try makeTestBundle(currentPlatforms: [
"Custom Name": PlatformVersion(VersionTriplet(100, 0, 0), beta: true)
])
let reference = ResolvedTopicReference(bundleIdentifier: bundle.identifier, path: "/documentation/MyKit/MyClass", sourceLanguage: .swift)
let node = try context.entity(with: reference)
(node.semantic as? Symbol)?.availability = SymbolGraph.Symbol.Availability(availability: [])
Copy link
Contributor

@d-ronnqvist d-ronnqvist Oct 14, 2024

Choose a reason for hiding this comment

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

This is not a blocking comment but something to keep in mind.

Modifying a page after the context has fully registered the data provider assumes that there are no derivatives of the original value elsewhere that was created while the context registered the data provider.

This means that the test could be exercising a different behavior than a real build would and could pass even though a real build would have a bug.

I think that there are no derivates of the availability information today, but there could be in the future in which case this test could hide a bug because it passes and there's no other test that exercises the real-build behavior.

Because of this, it's preferred to modify the inputs before they're registered with the context.

Copy link
Contributor Author

@sofiaromorales sofiaromorales Oct 14, 2024

Choose a reason for hiding this comment

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

It's true that data could be modified after the fact, but although this generally shouldn't be done, having this precondition adds an extra layer of security to the code.

let documentationContentRendered = DocumentationContentRenderer(documentationContext: context, bundle: bundle)
let isBeta = documentationContentRendered.isBeta(node)
// Verify that the symbol is not beta since it does not contains availability info.
XCTAssertFalse(isBeta)
}

// Different platform is beta
do {
let (bundle, context, reference) = try makeTestBundle(currentPlatforms: [
Expand Down