Compute beta platform information for non-symbol documentation #959

anferbui · 2024-06-19T14:52:27Z

Bug/issue #, if applicable: rdar://129355087

Summary

When configuring availability information for an article using the @Available directive, beta tags are never shown/computed even when the platforms are configured as beta:

Markdown:

# Sample Code Article

@Metadata {
    @Available(iOS, introduced: "18.0")
    @Available(Xcode, introduced: "16.0")
    @Available(macOS, introduced: "15.0")
    @Available(visionOS, introduced: "2.0")
    @SupportedLanguage(swift)
}

DocC invocation:

docc preview SampleCode.docc \
--platform name=macOS,version=15.0.0,beta=true \
--platform name=iOS,version=18.0.0,beta=true \
--platform name=visionOS,version=2.0.0,beta=true \
--platform name=Xcode,version=16.0.0,beta=true

Preview:

I would expect the platforms to be marked with a beta badge, similarly to how that happens for symbols:

Fix

This bug was being caused by the AvailabilityRenderItem.isBeta property (which is used to render the beta badges) never being set for the case when availability information comes from the @Available directive.

It was being set for the symbol graph case correctly:
https://github.com/apple/swift-docc/blob/f53b6e03cfc99d4e684d54351a8cec7fe36caabc/Sources/SwiftDocC/Model/Rendering/Symbol/AvailabilityRenderMetadataItem.swift#L125-L141

But not for the metadata case:
https://github.com/apple/swift-docc/blob/f53b6e03cfc99d4e684d54351a8cec7fe36caabc/Sources/SwiftDocC/Model/Rendering/Symbol/AvailabilityRenderMetadataItem.swift#L143-L150

I've made it so that it's always set, which required a minor rework of the @Available directive.

Changes

The @Available directive now parses the introduced field as a VersionTriplet, instead of as a String. This is useful for:
- Validating the user input is in a valid format
- To be able to compare the availability from the @Available directive with the platform metadata from DocC's context, which is then used to determine whether a platform has been configured as being in beta.
.stringRepresentation(precisionUpToNonsignificant:) has been moved to VersionTriplet. This is a non-source-breaking change as this was internal API.
Sets AvailabilityRenderItem.isBeta when AvailabilityRenderItem.init?(_ :current:) is called.
Makes AvailabilityRenderItem.isBeta, AvailabilityRenderItem.introduced and AvailabilityRenderItem.name into let properties as they are integral to availability rendering. This is to avoid any future bugs, as the code will now no longer compile unless this property is set in an initializer.
Adds diagnostic explanation for non-convertible directive arguments, to be emitted as part of the diagnostic explanation whenever the author provides an argument which is not convertible to the expected type (198c827).

Dependencies

N/A

Testing

Steps:

Download the example documentation bundle:
SampleCode.docc.zip
Preview the documentation:

docc preview SampleCode.docc \
--platform name=macOS,version=15.0.0,beta=true \
--platform name=iOS,version=18.0.0,beta=true \
--platform name=visionOS,version=2.0.0,beta=true \
--platform name=Xcode,version=16.0.0,beta=true

Verify that all platforms as showing as being in beta, as well as the article itself.

More generally, this can be verified by adding availability information to any documentation markdown file (excepting a symbol extension file):

@Metadata {
    @Available(iOS, introduced: "2.0")
}

And defining that platform version as beta using docc's --platform flag.

Checklist

Make sure you check off the following items. If they cannot be completed, provide a reason.

Added tests
Ran the ./bin/test script and it succeeded
Updated documentation if necessary

Verification

Unit testing
Modified existing tests for VersionTriplet and SymbolGraph.SemanticVersion.
Added unit tests to MetadataAvailabilityTests and PlatformAvailabilityTests.

Site rendering

Before	After

Documentation

Updated documentation for @Available directive.

Error cases

When an invalid semantic version number is passed to the @Available directed, the following warning diagnostic is emitted:

warning: Cannot convert 'hello' to type 'VersionTriplet'
Available expects an argument for the 'introduced' parameter that's convertible to a semantic version number ('[0-9]+(.[0-9]+)?(.[0-9]+)?')

anferbui · 2024-06-19T14:56:39Z

Sources/SwiftDocC/Semantics/Metadata/Availability.swift

@@ -92,7 +102,7 @@ extension Metadata {

        /// The platform version that this page applies to.
        @DirectiveArgumentWrapped
-        public var introduced: String
+        public var introduced: VersionTriplet


Note: This change is source breaking.

I've tried to fix this so there's no longer a source breaking change: 859044b

Changes: - `@Available` directive now parses `introduced` field as a `VersionTriplet` - `.stringRepresentation(precisionUpToNonsignificant:)` has been moved to `VersionTriplet` - Modifies tests so that they compile given the above changes The availability directive supports an "introduced" field, which accepted any string. This commit modifies it to only accept strings which can be converted into a VersionTriplet, the type which DocC uses internally for representing version information. This is useful to be able to compare the availability from the `@Availability` directive with the platform metadata which DocC's context contains, for purposes such as determining whether a platform has been configured as being in beta. The `.stringRepresentation(precisionUpToNonsignificant:)` functionality has been moved to `VersionTriplet`, which is a non-breaking change as this was internal API. This is because we don't always have a `SymbolGraph.SemanticVersion` as the starting type, for example when using the `@Available` directive.

Changes: - Sets `AvailabilityRenderItem.isBeta` when `AvailabilityRenderItem.init?(_ :current:)` is called - Makes `AvailabilityRenderItem.isBeta`, `AvailabilityRenderItem. introduced` and `AvailabilityRenderItem.name` into `let` properties as they are integral to availability rendering. Unifies the behavior in `AvailabilityRenderItem` so that we always compute whether a platform is in beta, regardless of whether the availability information comes from the symbol graph or comes from the `@Available` directive. This fixes an issue where sample code articles defined with `@Available` directives would never display a beta badge regardless of whether the platform was configured to be a beta platform. Additionally, makes `isBeta` a `let` property to avoid any future bugs, as the code will now no longer compile unless this property is set in an initializer. Resolves rdar://129355087.

Documents the format which is expected for the "introduced" field of the `@Available` directive.

Reintroduces `Metadata.Availability.introduced` as a `String` to avoid any API breaking changes. Deprecates this field, in preparation for removal in a future release. Values for this field are derived from `Metadata.Availability.introducedVersion`. `Metadata.Availability.introducedVersion` is now driving argument `introduced` of the directive.

- Adds unit tests for verifying that the `introduced` field is parsed with the correct formatting rules. - Fixes a bug where a leading or trailing full stop would be considered a valid format. - Updates FIXMEs to contain the most up-to-date information - Refactors MetadataAvailabilityTests slightly Resolves rdar://129355087.

- Adds tests to verify that availability defined only in articles is correctly detected as beta depending on configuration. Resolves rdar://129355087.

When a directive argument is not convertible to a complex type (such as VersionTriplet), it is complicated for the author to know what the valid format is. Added a field `expectedFormat` in various places which can be used to specify a string which will describe what the expected format is. The changes are all non-breaking because they are either additive or they modify internal API. Added a custom expected format for `VersionTriplet` to improve the diagnostic information. Resolves rdar://129355087.

anferbui · 2024-06-27T15:41:07Z

@swift-ci please test

Sources/SwiftDocC/Model/Rendering/Symbol/AvailabilityRenderMetadataItem.swift

Sources/SwiftDocC/Semantics/General Purpose Analyses/HasArgumentOfType.swift

Tests/SwiftDocCTests/Rendering/PlatformAvailabilityTests.swift

Sources/SwiftDocC/Model/Rendering/Symbol/AvailabilityRenderMetadataItem.swift

d-ronnqvist · 2024-06-27T16:27:46Z

Sources/SwiftDocC/Model/Rendering/Symbol/AvailabilityRenderMetadataItem.swift

@@ -11,7 +11,7 @@
 import Foundation
 import SymbolKit

-extension SymbolGraph.SemanticVersion {
+extension VersionTriplet {


What's the reasons for changing this type? AFAICT there's no change in behavior from it.

The reason for it is that we don't always have a SymbolGraph.SemanticVersion type when we want to convert the semantic version to a string representation, for example here availability.introducedVersion is a VersionTriplet:
https://github.com/anferbui/swift-docc/blob/f5f320b75efe8da4af9198a0b11b381a6d6cc3c4/Sources/SwiftDocC/Model/Rendering/Symbol/AvailabilityRenderMetadataItem.swift#L143

I think it makes more sense to have the string representation function as part of VersionTriplet than SymbolGraph.SemanticVersion since VersionTriplet is the type that docc uses internally, WDYT?

VersionTriplet is also how we store the current platforms so it makes more sense to me to use that as the type for comparing semantic versions and deriving a string representation.

There's some overlap here with this comment but it seems we have many different version types in Swift DocC.

VersionTriplet is created in 2 places and used as a stored property in 1 place

Version is used in 3 places (2 of which uses it to create a VersionTriplet) and used as a stored property in 1 internal place

Platform.Version used as a stored property in 1 place

SemanticVersion is used as a stored property in 5 places.

Additionally, SymbolGraph.SemanticVersion is used a few places that operate on other types from SymbolKit.

Considering that SemanticVersion and SymbolGraph.SemanticVersion contain the same information and behaves the same, maybe it makes more sense to move towards using that type over VersionTriplet. What do you think?

d-ronnqvist · 2024-06-27T16:34:01Z

Sources/SwiftDocC/Semantics/Metadata/Availability.swift

-        @DirectiveArgumentWrapped
-        public var introduced: String
+        @DirectiveArgumentWrapped(name: .custom("introduced"))
+        public var introducedVersion: VersionTriplet


I wonder if it's worth making the source breaking change up-front here and name this property introduced.

Otherwise we'll likely have repeated deprecations to rename this property after 6.1 is released and remove the duplicate after 6.2 is release.

That's a good discussion point, thanks for bringing it up.

I think we could potentially leave this variable as introducedVersion without renaming it, so that we only need to remove introduced after 6.1 is released, WDYT?

If you feel strongly about not leaving this property as introducedVersion I'm happy to revert this back to a source-breaking change

I just saw we now have deprecated as well - d6e1217

Might we worth making the source breaking change in this PR for both of them to be VersionTriplets

I just saw we now have deprecated as well - d6e1217

Might we worth making the source breaking change in this PR for both of them to be VersionTriplets

Yes, I think both properties should be some form of version type. However, I'm not sure what version type that should be.

After looking at more of DocC's code I think find at least these types being used:

SemanticVersion which looks like a copy of SymbolKit.SymbolGraph.SemanticVersion

Version

VersionTriplet

Platform.Version

Long term I think we should strive to get rid of this duplication to one version type per repo, but for this PR we only need to pick which version type we want to use going forward.

Also, I think the deprecated property should continue to be called deprecated. This matches the "Omit needless words" guideline from the Swift API design guidelines:

In particular, omit words that merely repeat type information.

This leads to an inconsistency with the introducedVersion property (which for source compatibility reasons repeats type information in the name).

I personally think it's worth doing a second rename after the next release to have both properties be named consistently again.

Yes, I think both properties should be some form of version type. However, I'm not sure what version type that should be.

After looking at more of DocC's code I think find at least these types being used:

SemanticVersion which looks like a copy of SymbolKit.SymbolGraph.SemanticVersion

Version

VersionTriplet

Platform.Version

Long term I think we should strive to get rid of this duplication to one version type per repo, but for this PR we only need to pick which version type we want to use going forward.

I don't have a strong opinion on which type we settle on going forward, they all seem to be fulfilling a similar purpose. I have the most familiarity with VersionTriplet because that's the type which is used for determining whether a platform is in beta, but I'm not familiar with the others or how extensively they're used throughout the codebase.

Opened: #970

~~I propose that we don't try to decide which type we choose going forward at this point in time, as I think that requires evaluation, and we leave that decision to when we address #970~~

~~Either way converging on one semver type will require a lot of deprecation / source-breaking changes, so I don't think the risk is increased by a lot by not making a decision here.~~

EDIT: I'd missed #959 (comment), happy to go forward with this:

Considering that SemanticVersion and SymbolGraph.SemanticVersion contain the same information and behaves the same, maybe it makes more sense to move towards using that type over VersionTriplet. What do you think?

Based on a quick search, SemanticVersion seems to be the most widely used type in DocC so far.

I've made the change to SemanticVersion in commits 7efe5c1 and 7d729c0, WDYT?

Heads up that this PR is back to being a source-breaking change per our discussion to avoid having to do multiple renames in 6.1 and 6.2.

This is a source-breaking change, so removing it.

Assignments can be written succinctly using an if-expression as of Swift 5.9 (https://github.com/swiftlang/swift-evolution/blob/main/proposals/0380-if-switch-expressions.md)

Makes the test assertions clearer by rephrasing the logic. These assertions are tricky to read because `isBeta` can be one of three values: true, false and nil.

The function is only used in this file, and not needed anywhere else. We can leave it private.

This reverts commit 568318e.

Uses `SemanticVersion` type rather than `VersionTriplet`. We have 4 types in DocC to represent a semantic version, and want to converge towards using only SemanticVersion: swiftlang#970

Unifies the "deprecated" parameter behavior with the new "introduced" parameter behaviour, by making both of them be a `SemanticVersion` type.

Updates to mention that the `deprecated` argument is also meant to be a semantic version.

anferbui · 2024-07-01T09:04:41Z

@swift-ci please test

d-ronnqvist

LGTM

d-ronnqvist · 2024-07-04T13:20:03Z

Sources/SwiftDocC/Semantics/Metadata/Availability.swift

@@ -92,11 +103,11 @@ extension Metadata {

        /// The platform version that this page was introduced in.
        @DirectiveArgumentWrapped
-        public var introduced: String
+        public var introduced: SemanticVersion


We synced offline about this change and decided to go with changing the type now to avoid changing this property multiple times.

…lang#959) Fixes a bug where beta platform information was never being derived for non-symbol documentation, such as sample code articles. Sample code articles defined with `@Available` directives were never displaying a beta badge regardless of whether the platform was configured to be a beta platform. * Use `VersionTriplet` for `@Available` directive * Compute platform beta status when using `@Available` * Adds documentation for new `@Available` behavior * Add unit tests for new `@Available` behaviour * Add unit tests to `PlatformAvailabilityTests` for beta availability * Add diagnostic explanation for non-convertible directive arguments * Revert change to make `isBeta` a `let` property * Use if-expression to declare variables * Make assertions in `PlatformAvailabilityTests` clearer * Make `AvailabilityRenderItem.isBeta(introduced:current:)` private * Move to using `SemanticVersion` rather than `VersionTriplet` * Merge "deprecated" parameter changes with SemanticVersion changes * Update `@Available` documentation Resolves rdar://129355087.

…#972) Fixes a bug where beta platform information was never being derived for non-symbol documentation, such as sample code articles. Sample code articles defined with `@Available` directives were never displaying a beta badge regardless of whether the platform was configured to be a beta platform. * Use `VersionTriplet` for `@Available` directive * Compute platform beta status when using `@Available` * Adds documentation for new `@Available` behavior * Add unit tests for new `@Available` behaviour * Add unit tests to `PlatformAvailabilityTests` for beta availability * Add diagnostic explanation for non-convertible directive arguments * Revert change to make `isBeta` a `let` property * Use if-expression to declare variables * Make assertions in `PlatformAvailabilityTests` clearer * Make `AvailabilityRenderItem.isBeta(introduced:current:)` private * Move to using `SemanticVersion` rather than `VersionTriplet` * Merge "deprecated" parameter changes with SemanticVersion changes * Update `@Available` documentation Resolves rdar://129355087.

anferbui added the source breaking DocC's public API isn't source compatible with earlier versions label Jun 19, 2024

anferbui force-pushed the sample-code-beta-badges branch from 6336b44 to efbcd8a Compare June 19, 2024 14:53

anferbui commented Jun 19, 2024

View reviewed changes

anferbui force-pushed the sample-code-beta-badges branch from efbcd8a to ad00cf3 Compare June 19, 2024 15:28

This comment was marked as duplicate.

Sign in to view

anferbui force-pushed the sample-code-beta-badges branch from ad00cf3 to c2b2b9c Compare June 19, 2024 15:47

anferbui removed the source breaking DocC's public API isn't source compatible with earlier versions label Jun 19, 2024

This comment was marked as duplicate.

Sign in to view

anferbui force-pushed the sample-code-beta-badges branch 2 times, most recently from e0f5775 to 7b3db53 Compare June 21, 2024 14:33

This comment was marked as duplicate.

Sign in to view

anferbui marked this pull request as ready for review June 27, 2024 15:37

anferbui added 7 commits June 27, 2024 16:38

Adds documentation for new @Available behavior

7b5d185

Documents the format which is expected for the "introduced" field of the `@Available` directive.

Add unit tests to PlatformAvailabilityTests for beta availability

568e286

- Adds tests to verify that availability defined only in articles is correctly detected as beta depending on configuration. Resolves rdar://129355087.

anferbui force-pushed the sample-code-beta-badges branch from 198c827 to f5f320b Compare June 27, 2024 15:38

anferbui requested a review from d-ronnqvist June 27, 2024 15:42

d-ronnqvist reviewed Jun 27, 2024

View reviewed changes

anferbui added 6 commits June 28, 2024 13:09

Revert change to make isBeta a let property

5858ded

This is a source-breaking change, so removing it.

Use if-expression to declare variables

8da7bff

Assignments can be written succinctly using an if-expression as of Swift 5.9 (https://github.com/swiftlang/swift-evolution/blob/main/proposals/0380-if-switch-expressions.md)

Make assertions in PlatformAvailabilityTests clearer

d26b467

Makes the test assertions clearer by rephrasing the logic. These assertions are tricky to read because `isBeta` can be one of three values: true, false and nil.

Make AvailabilityRenderItem.isBeta(introduced:current:) private

89eb015

The function is only used in this file, and not needed anywhere else. We can leave it private.

Revert "Remove the source-breaking change to Metadata.Availability"

d67cc9f

This reverts commit 568318e.

Move to using SemanticVersion

7efe5c1

Uses `SemanticVersion` type rather than `VersionTriplet`. We have 4 types in DocC to represent a semantic version, and want to converge towards using only SemanticVersion: swiftlang#970

anferbui added 2 commits June 28, 2024 17:38

Merge branch 'main' into sample-code-beta-badges

8223994

Merge "deprecated" parameter changes with SemanticVersion changes

7d729c0

Unifies the "deprecated" parameter behavior with the new "introduced" parameter behaviour, by making both of them be a `SemanticVersion` type.

anferbui added the source breaking DocC's public API isn't source compatible with earlier versions label Jun 28, 2024

Revert unintentional changes to VersionTripletTest

98f9308

This comment was marked as duplicate.

Sign in to view

Update @Available documentation

1bfe9b4

Updates to mention that the `deprecated` argument is also meant to be a semantic version.

anferbui mentioned this pull request Jun 28, 2024

Remove duplicate semantic versioning types #970

Open

2 tasks

d-ronnqvist approved these changes Jul 4, 2024

View reviewed changes

anferbui merged commit 34a872d into swiftlang:main Jul 4, 2024
2 checks passed

anferbui deleted the sample-code-beta-badges branch July 4, 2024 13:27

anferbui mentioned this pull request Jul 4, 2024

[6.0] Compute beta platform information for non-symbol documentation #972

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Compute beta platform information for non-symbol documentation #959

Compute beta platform information for non-symbol documentation #959

anferbui commented Jun 19, 2024 •

edited

Loading

anferbui Jun 19, 2024 •

edited

Loading

anferbui Jun 19, 2024 •

edited

Loading

This comment was marked as duplicate.

This comment was marked as duplicate.

This comment was marked as duplicate.

This comment was marked as duplicate.

anferbui commented Jun 27, 2024

d-ronnqvist Jun 27, 2024

anferbui Jun 28, 2024

d-ronnqvist Jun 28, 2024

d-ronnqvist Jun 27, 2024

anferbui Jun 28, 2024 •

edited

Loading

anferbui Jun 28, 2024

d-ronnqvist Jun 28, 2024

d-ronnqvist Jun 28, 2024 •

edited

Loading

anferbui Jun 28, 2024

anferbui Jun 28, 2024 •

edited

Loading

d-ronnqvist Jun 28, 2024

anferbui Jun 28, 2024

This comment was marked as duplicate.

anferbui commented Jul 1, 2024

d-ronnqvist left a comment

d-ronnqvist Jul 4, 2024

Compute beta platform information for non-symbol documentation #959

Compute beta platform information for non-symbol documentation #959

Conversation

anferbui commented Jun 19, 2024 • edited Loading

Summary

Fix

Changes

Dependencies

Testing

Checklist

Verification

anferbui Jun 19, 2024 • edited Loading

Choose a reason for hiding this comment

anferbui Jun 19, 2024 • edited Loading

Choose a reason for hiding this comment

This comment was marked as duplicate.

This comment was marked as duplicate.

This comment was marked as duplicate.

This comment was marked as duplicate.

anferbui commented Jun 27, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anferbui Jun 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

d-ronnqvist Jun 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anferbui Jun 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as duplicate.

anferbui commented Jul 1, 2024

d-ronnqvist left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anferbui commented Jun 19, 2024 •

edited

Loading

anferbui Jun 19, 2024 •

edited

Loading

anferbui Jun 19, 2024 •

edited

Loading

anferbui Jun 28, 2024 •

edited

Loading

d-ronnqvist Jun 28, 2024 •

edited

Loading

anferbui Jun 28, 2024 •

edited

Loading