Skip to content

De-duplicate isBeta logic #1289

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 2 commits into
base: main
Choose a base branch
from
Open

De-duplicate isBeta logic #1289

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
eba20e7
Adds a shared utility for determining beta status
anferbui Sep 15, 2025
da305b3
Remove duplicate isBeta implementations
anferbui Sep 15, 2025
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
6 changes: 1 addition & 5 deletions Sources/SwiftDocC/Indexing/Navigator/RenderNode+NavigatorIndex.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,11 +125,7 @@ struct RenderNodeVariantView: NavigatorIndexableRenderNodeRepresentation {

extension NavigatorIndexableRenderMetadataRepresentation {
var isBeta: Bool {
guard let platforms, !platforms.isEmpty else {
return false
}

return platforms.allSatisfy { $0.isBeta == true }
return platforms?.isBeta ?? false
}
}

Expand Down
6 changes: 1 addition & 5 deletions Sources/SwiftDocC/Infrastructure/External Data/OutOfProcessReferenceResolver.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -590,11 +590,7 @@ extension OutOfProcessReferenceResolver {

/// A value that indicates whether this symbol is under development and likely to change.
var isBeta: Bool {
guard let platforms, !platforms.isEmpty else {
return false
}

return platforms.allSatisfy { $0.isBeta == true }
return platforms?.isBeta ?? false
}

/// Creates a new resolved information value with all its values.
Expand Down
8 changes: 2 additions & 6 deletions Sources/SwiftDocC/Infrastructure/Link Resolution/ExternalPathHierarchyResolver.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*
This source file is part of the Swift.org open source project

Copyright (c) 2023-2024 Apple Inc. and the Swift project authors
Copyright (c) 2023-2025 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
Expand Down Expand Up @@ -184,11 +184,7 @@ private extension Sequence<DeclarationRenderSection.Token> {
private extension LinkDestinationSummary {
/// A value that indicates whether this symbol is under development and likely to change.
var isBeta: Bool {
guard let platforms, !platforms.isEmpty else {
return false
}

return platforms.allSatisfy { $0.isBeta == true }
return platforms?.isBeta ?? false
}

/// Create a topic render render reference for this link summary and its content variants.
Expand Down
26 changes: 25 additions & 1 deletion Sources/SwiftDocC/Model/Rendering/Symbol/AvailabilityRenderMetadataItem.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*
This source file is part of the Swift.org open source project

Copyright (c) 2021-2023 Apple Inc. and the Swift project authors
Copyright (c) 2021-2025 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
Expand Down Expand Up @@ -163,3 +163,27 @@ public struct AvailabilityRenderItem: Codable, Hashable, Equatable {
self.isBeta = isBeta
}
}

extension Array where Array.Element == AvailabilityRenderItem {
Copy link
Contributor

Choose a reason for hiding this comment

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

FIY: this can be expressed more simply as

Suggested change
extension Array where Array.Element == AvailabilityRenderItem {
extension Array<AvailabilityRenderItem> {

/// Determines whether all platforms in the array are in beta.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a verb phrase ("determines") but properties is more commonly described by nouns.

We could either change this to a function (func allAreInBeta() -> Bool) to go better with the verb phrase or change this to a noun phrase. For example:

Suggested change
/// Determines whether all platforms in the array are in beta.
/// A Boolean value that determines whether all platforms in the array are in beta.

Copy link
Contributor

Choose a reason for hiding this comment

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

That said, with a simple and clean name like allAreInBeta I don't know if this needs any documentation at all as an internal API.

///
/// This property returns `true` if every availability item in the array has its `isBeta` property set to `true`,
/// indicating that the symbol is introduced in a beta version across all platforms. If the array is empty,
/// this property returns `false`, as an item with no platform availability information is not considered
/// to be in beta.
Comment on lines +170 to +173
Copy link
Contributor

Choose a reason for hiding this comment

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

If this behavior is clarified in the API then this documentation can be simplified to only covering the empty behavior.

Also, there's no need to wrap documentation to a fixed column width.

Suggested change
/// This property returns `true` if every availability item in the array has its `isBeta` property set to `true`,
/// indicating that the symbol is introduced in a beta version across all platforms. If the array is empty,
/// this property returns `false`, as an item with no platform availability information is not considered
/// to be in beta.
/// If the array is empty, this value is `false`. A symbol without any platform availability information isn't considered to be in beta.

///
/// This computed property centralizes the beta determination logic to avoid code duplication across multiple
/// components that need to check whether a symbol is in beta based on its platform availability.
/// `NavigatorIndexableRenderMetadataRepresentation`, `OutOfProcessReferenceResolver.ResolvedInformation` and `LinkDestinationSummary` all store an array of ``AvailabilityRenderItem`` and need to determine beta status based on platform availability,
/// so it is convenient to de-duplicate the shared logic here.
Comment on lines +175 to +178
Copy link
Contributor

Choose a reason for hiding this comment

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

This type of comment is about the implementation and isn't all that relevant to the user of the API.

Suggested change
/// This computed property centralizes the beta determination logic to avoid code duplication across multiple
/// components that need to check whether a symbol is in beta based on its platform availability.
/// `NavigatorIndexableRenderMetadataRepresentation`, `OutOfProcessReferenceResolver.ResolvedInformation` and `LinkDestinationSummary` all store an array of ``AvailabilityRenderItem`` and need to determine beta status based on platform availability,
/// so it is convenient to de-duplicate the shared logic here.

///
/// - Returns: `true` if all platforms in the array are in beta; `false` if the array is empty or if any
/// platform is not in beta.
Comment on lines +179 to +181
Copy link
Contributor

Choose a reason for hiding this comment

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

Simple properties like this one don't need a returns section.

Suggested change
///
/// - Returns: `true` if all platforms in the array are in beta; `false` if the array is empty or if any
/// platform is not in beta.

var isBeta: Bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this name looks a bit strange at the call site because the array isn't "in beta" rather it's the containing elements that are.

Also, this name has some ambiguity as to what it means for a collection of platforms to be in beta. This is explained in the documentation but if this behavior was clarified in the symbol name it would avoid this ambiguity all together.

Suggested change
var isBeta: Bool {
var allAreInBeta: Bool {

guard !self.isEmpty else {
return false
}

return self.allSatisfy { $0.isBeta == true }
Comment on lines +183 to +187
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI: you can write this in a single expression as

Suggested change
guard !self.isEmpty else {
return false
}
return self.allSatisfy { $0.isBeta == true }
contains(where: { $0.isBeta != true })

}
}