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.

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

Fix availability logic refactor #1065

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Fix availability logic refactor #1065

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ec28dd5
Refactor symbol availability logic.
sofiaromorales Oct 24, 2024
d9915b0
Merge branch 'main' into fix-availability-logic-refactor
sofiaromorales Nov 11, 2024
e9152bb
Refactor unit tests for testing symbol availability.
sofiaromorales Nov 11, 2024
8b2cb42
Refactor fallback platforms data structure.
sofiaromorales Nov 12, 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
278 changes: 123 additions & 155 deletions Sources/SwiftDocC/Infrastructure/Symbol Graph/SymbolGraphLoader.swift
View file
Open in desktop

Large diffs are not rendered by default.

30 changes: 16 additions & 14 deletions Sources/SwiftDocC/Infrastructure/Workspace/DefaultAvailability.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,28 +131,30 @@ public struct DefaultAvailability: Codable, Equatable {

/// Fallback availability information for platforms we either don't emit SGFs for
/// or have the same availability information as another platform.
package static let fallbackPlatforms: [PlatformName: PlatformName] = [
.catalyst: .iOS,
.iPadOS: .iOS,
]
///
/// The key corresponds to the fallback platform and the value to the platform it's
/// fallbacking from.
package static let fallbackPlatforms: [PlatformName: [PlatformName]] = [.iOS: [.catalyst, .iPadOS]]

/// Creates a default availability module.
/// - Parameter modules: A map of modules and the default platform availability for symbols in that module.
public init(with modules: [String: [ModuleAvailability]]) {
self.modules = modules.mapValues { platformAvailabilities -> [DefaultAvailability.ModuleAvailability] in
// If a module doesn't contain default availability information for any of the fallback platforms,
// infer it from the corresponding mapped value.
platformAvailabilities + DefaultAvailability.fallbackPlatforms.compactMap { (platform, fallbackPlatform) in
if !platformAvailabilities.contains(where: { $0.platformName == platform }),
let fallbackAvailability = platformAvailabilities.first(where: { $0.platformName == fallbackPlatform }),
let fallbackIntroducedVersion = fallbackAvailability.introducedVersion
{
return DefaultAvailability.ModuleAvailability(
platformName: platform,
platformVersion: fallbackIntroducedVersion
)
platformAvailabilities + DefaultAvailability.fallbackPlatforms.flatMap { (fallbackPlatform, platform) in
platform.compactMap { pla in
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: This looks like an incomplete word.

Suggested change
platform.compactMap { pla in
platform.compactMap { platformName in

if !platformAvailabilities.contains(where: { $0.platformName == pla }),
let fallbackAvailability = platformAvailabilities.first(where: { $0.platformName == fallbackPlatform }),
let fallbackIntroducedVersion = fallbackAvailability.introducedVersion
{
return DefaultAvailability.ModuleAvailability(
platformName: pla,
platformVersion: fallbackIntroducedVersion
)
}
return nil
}
return nil
}
}
}
Expand Down
8 changes: 5 additions & 3 deletions Sources/SwiftDocC/Semantics/Symbol/Symbol.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -556,10 +556,12 @@ extension Symbol {
func mergeAvailabilities(unifiedSymbol: UnifiedSymbolGraph.Symbol) {
for (selector, mixins) in unifiedSymbol.mixins {
let trait = DocumentationDataVariantsTrait(for: selector)
if let unifiedSymbolAvailability = mixins[SymbolGraph.Symbol.Availability.mixinKey] as? SymbolGraph.Symbol.Availability {
guard let availabilityVariantTrait = availabilityVariants[trait] else {
return
}
if let unifiedSymbolAvailability = mixins.getValueIfPresent(for: SymbolGraph.Symbol.Availability.self) {
unifiedSymbolAvailability.availability.forEach { availabilityItem in
guard let availabilityVariantTrait = availabilityVariants[trait] else { return }
if (availabilityVariantTrait.availability.contains(where: { $0.domain?.rawValue == availabilityItem.domain?.rawValue })) {
guard availabilityVariantTrait.availability.firstIndex(where: { $0.domain?.rawValue == availabilityItem.domain?.rawValue }) == nil else {
return
}
availabilityVariants[trait]?.availability.append(availabilityItem)
Expand Down
2 changes: 1 addition & 1 deletion Sources/SwiftDocCTestUtilities/FilesAndFolders.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ public struct InfoPlist: File, DataRepresentable {
self.identifier = identifier
self.defaultAvailability = defaultAvailability
}

public init(from decoder: any Decoder) throws {
let container = try decoder.container(keyedBy: DocumentationBundle.Info.CodingKeys.self)
displayName = try container.decodeIfPresent(String.self, forKey: .displayName)
Expand Down
6 changes: 4 additions & 2 deletions Sources/SwiftDocCUtilities/Action/Actions/Convert/ConvertAction.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,10 @@ public struct ConvertAction: AsyncAction {
// Inject current platform versions if provided
if var currentPlatforms {
// Add missing platforms if their fallback platform is present.
for (platform, fallbackPlatform) in DefaultAvailability.fallbackPlatforms where currentPlatforms[platform.displayName] == nil {
currentPlatforms[platform.displayName] = currentPlatforms[fallbackPlatform.displayName]
for (fallbackPlatform, platforms) in DefaultAvailability.fallbackPlatforms {
for platform in platforms where currentPlatforms[platform.displayName] == nil {
currentPlatforms[platform.displayName] = currentPlatforms[fallbackPlatform.displayName]
}
}
configuration.externalMetadata.currentPlatforms = currentPlatforms
}
Expand Down
202 changes: 155 additions & 47 deletions Tests/SwiftDocCTests/Infrastructure/SymbolGraph/SymbolGraphLoaderTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ class SymbolGraphLoaderTests: XCTestCase {
// Update one symbol's availability to use as a verification if we're loading iOS or Catalyst symbol graph
catalystSymbolGraph.symbols["s:5MyKit0A5ClassC"]!.mixins[SymbolGraph.Symbol.Availability.mixinKey]! = SymbolGraph.Symbol.Availability(availability: [
.init(domain: SymbolGraph.Symbol.Availability.Domain(rawValue: "Mac Catalyst"), introducedVersion: .init(major: 1, minor: 0, patch: 0), deprecatedVersion: nil, obsoletedVersion: nil, message: nil, renamed: nil, isUnconditionallyDeprecated: false, isUnconditionallyUnavailable: false, willEventuallyBeDeprecated: false),
.init(domain: SymbolGraph.Symbol.Availability.Domain(rawValue: "iOS"), introducedVersion: .init(major: 7, minor: 0, patch: 0), deprecatedVersion: nil, obsoletedVersion: nil, message: nil, renamed: nil, isUnconditionallyDeprecated: false, isUnconditionallyUnavailable: false, willEventuallyBeDeprecated: false),
.init(domain: SymbolGraph.Symbol.Availability.Domain(rawValue: "iOS"), introducedVersion: nil, deprecatedVersion: nil, obsoletedVersion: nil, message: nil, renamed: nil, isUnconditionallyDeprecated: false, isUnconditionallyUnavailable: false, willEventuallyBeDeprecated: false),
])

let catalystSymbolGraphURL = bundleURL.appendingPathComponent(catalystSymbolGraphName)
Expand Down Expand Up @@ -671,13 +671,14 @@ class SymbolGraphLoaderTests: XCTestCase {
}
""",
platform: """
"environment" : "macabi",
"operatingSystem" : {
"minimumVersion" : {
"major" : 6,
"minor" : 5,
"patch" : 0
},
"name" : "macCatalyst"
"name" : "ios",
}
"""
)
Expand Down Expand Up @@ -775,12 +776,13 @@ class SymbolGraphLoaderTests: XCTestCase {
}
""",
platform: """
"environment" : "macabi",
"operatingSystem" : {
"minimumVersion" : {
"major" : 6,
"minor" : 5
},
"name" : "macCatalyst"
"name" : "ios"
}
"""
)
Expand Down Expand Up @@ -931,6 +933,7 @@ class SymbolGraphLoaderTests: XCTestCase {
XCTAssertNotNil(availability.first(where: { $0.domain?.rawValue == "iPadOS" }))
XCTAssertEqual(availability.first(where: { $0.domain?.rawValue == "iOS" })?.introducedVersion, SymbolGraph.SemanticVersion(major: 8, minor: 0, patch: 0))
XCTAssertEqual(availability.first(where: { $0.domain?.rawValue == "macCatalyst" })?.introducedVersion, SymbolGraph.SemanticVersion(major: 7, minor: 0, patch: 0))
XCTAssertEqual(availability.first(where: { $0.domain?.rawValue == "iPadOS" })?.introducedVersion, SymbolGraph.SemanticVersion(major: 6, minor: 0, patch: 0))
}

func testUnconditionallyunavailablePlatforms() throws {
Expand Down Expand Up @@ -995,7 +998,7 @@ class SymbolGraphLoaderTests: XCTestCase {
},
"accessLevel": "public",
"availability" : [{
"domain" : "maccatalyst",
"domain" : "macCatalyst",
"introduced" : {
"major" : 12,
"minor" : 0
Expand Down Expand Up @@ -1401,7 +1404,7 @@ class SymbolGraphLoaderTests: XCTestCase {
"accessLevel" : "public",
"availability" : [
{
"domain" : "maccatalyst",
"domain" : "macCatalyst",
"introduced" : {
"major" : 15,
"minor" : 2,
Expand Down Expand Up @@ -1453,7 +1456,7 @@ class SymbolGraphLoaderTests: XCTestCase {
// 'Mac Catalyst' (info.plist) and 'maccatalyst' (SGF).
XCTAssertTrue(availability.count == 2)
XCTAssertTrue(availability.filter({ $0.domain?.rawValue == "macCatalyst" }).count == 1)
XCTAssertTrue(availability.filter({ $0.domain?.rawValue == "maccatalyst" }).count == 0)
XCTAssertTrue(availability.filter({ $0.domain?.rawValue == "Mac Catalyst" }).count == 0)
}

func testFallbackOverrideDefaultAvailability() throws {
Expand Down Expand Up @@ -1518,17 +1521,7 @@ class SymbolGraphLoaderTests: XCTestCase {
"names": {
"title": "Foo",
},
"accessLevel": "public",
"availability" : [
{
"domain" : "iOS",
"introduced" : {
"major" : 12,
"minor" : 0,
"patch" : 0
}
}
]
"accessLevel": "public"
}
""",
platform: """
Expand Down Expand Up @@ -1595,8 +1588,7 @@ class SymbolGraphLoaderTests: XCTestCase {
"names": {
"title": "Foo",
},
"accessLevel": "public",
"availability" : []
"accessLevel": "public"
}
""",
platform: """
Expand Down Expand Up @@ -1652,6 +1644,122 @@ class SymbolGraphLoaderTests: XCTestCase {
XCTAssertEqual(availability.first(where: { $0.domain?.rawValue == "macCatalyst" })?.introducedVersion, SymbolGraph.SemanticVersion(major: 1, minor: 0, patch: 0))
}

func testNotAvailableSymbol() throws {
// Symbol from SG
let symbolGraphStringiOS = makeSymbolGraphString(
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: This variable says iOS but the data says macOS

moduleName: "MyModule",
symbols: """
{
"kind": {
"displayName" : "Instance Property",
"identifier" : "swift.property"
},
"identifier": {
"precise": "c:@F@A",
"interfaceLanguage": "swift"
},
"pathComponents": [
"Foo"
],
"names": {
"title": "Foo",
},
"accessLevel": "public",
"availability" : [
{
"domain" : "macOS",
"introduced" : {
"major" : 12,
"minor" : 0,
"patch" : 0
}
}
]
}
""",
platform: """
"operatingSystem" : {
"minimumVersion" : {
"major" : 12,
"minor" : 0,
"patch" : 0
},
"name" : "macosx"
}
"""
)
let symbolGraphStringMacOS = makeSymbolGraphString(
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: and this does the opposite

moduleName: "MyModule",
symbols: """
{
"kind": {
"displayName" : "Instance Property",
"identifier" : "swift.property"
},
"identifier": {
"precise": "c:@F@B",
"interfaceLanguage": "swift"
},
"pathComponents": [
"Foo"
],
"names": {
"title": "Bar",
},
"accessLevel": "public",
"availability" : [
{
"domain" : "iOS",
"introduced" : {
"major" : 12,
"minor" : 0,
"patch" : 0
}
}
]
}
""",
platform: """
"operatingSystem" : {
"minimumVersion" : {
"major" : 6,
"minor" : 5,
"patch" : 0
},
"name" : "ios"
}
"""
)
let infoPlist = """
<plist version="1.0">
<dict>
<key>CDAppleDefaultAvailability</key>
<dict>
<key>MyModule</key>
<array>
<dict>
<key>name</key>
<string>iOS</string>
<key>version</key>
<string>1.0</string>
</dict>
</array>
</dict>
</dict>
</plist>
"""
// Create an empty bundle
let targetURL = try createTemporaryDirectory(named: "test.docc")
// Store files
try symbolGraphStringiOS.write(to: targetURL.appendingPathComponent("MyModule-ios.symbols.json"), atomically: true, encoding: .utf8)
try symbolGraphStringMacOS.write(to: targetURL.appendingPathComponent("MyModule-macos.symbols.json"), atomically: true, encoding: .utf8)
try infoPlist.write(to: targetURL.appendingPathComponent("Info.plist"), atomically: true, encoding: .utf8)
// Load the bundle & reference resolve symbol graph docs
let (_, _, context) = try loadBundle(from: targetURL)
Copy link
Contributor

Choose a reason for hiding this comment

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

minor: Would it be possible to use the various File test helpers for this catalog instead of strings?

let availability = try XCTUnwrap((context.documentationCache["c:@F@A"]?.semantic as? Symbol)?.availability?.availability)
// Verify we don't fallback to iOS for 'Foo' even if there's default availability.
XCTAssertNil(availability.first(where: { $0.domain?.rawValue == "iOS" }))
}

func testDefaultAvailabilityWhenSymbolIsNotAvailableForThatPlatform() throws {
// Symbol from SGF
Expand Down Expand Up @@ -1728,35 +1836,35 @@ class SymbolGraphLoaderTests: XCTestCase {
moduleName: "MyModule",
symbols: symbolIOS,
platform: """
"operatingSystem" : {
"minimumVersion" : {
"major" : 12,
"minor" : 0,
"patch" : 0
},
"name" : "ios"
}
"""
)
let infoPlist = """
<plist version="1.0">
<dict>
<key>CDAppleDefaultAvailability</key>
<dict>
<key>MyModule</key>
<array>
<dict>
<key>name</key>
<string>iOS</string>
</dict>
<dict>
<key>name</key>
<string>tvOS</string>
</dict>
</array>
"operatingSystem" : {
"minimumVersion" : {
"major" : 12,
"minor" : 0,
"patch" : 0
},
"name" : "ios"
}
"""
)
let infoPlist = """
<plist version="1.0">
<dict>
<key>CDAppleDefaultAvailability</key>
<dict>
<key>MyModule</key>
<array>
<dict>
<key>name</key>
<string>iOS</string>
</dict>
<dict>
<key>name</key>
<string>tvOS</string>
</dict>
</array>
</dict>
</dict>
</dict>
</plist>
</plist>
"""
// Create an empty bundle
let targetURL = try createTemporaryDirectory(named: "test.docc")
Expand Down
6 changes: 4 additions & 2 deletions Tests/SwiftDocCTests/Model/SemaToRenderNodeTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1917,8 +1917,10 @@ Document
var configuration = DocumentationContext.Configuration()
// Add missing platforms if their fallback platform is present.
var currentPlatforms = currentPlatforms ?? [:]
for (platform, fallbackPlatform) in DefaultAvailability.fallbackPlatforms where currentPlatforms[platform.displayName] == nil {
currentPlatforms[platform.displayName] = currentPlatforms[fallbackPlatform.displayName]
for (fallbackPlatform, platforms) in DefaultAvailability.fallbackPlatforms {
for platform in platforms where currentPlatforms[platform.displayName] == nil {
currentPlatforms[platform.displayName] = currentPlatforms[fallbackPlatform.displayName]
}
}
configuration.externalMetadata.currentPlatforms = currentPlatforms

Expand Down
Loading