Skip to content

Commit e626518

Browse files
committed
[Traits] Replace isDefault with defaults trait
# Motivation The current spelling uses `isDefault` per trait to declare the default traits. This makes it hard to see which traits are actually the default traits. # Modification Instead of using an explicit `isDefault` per trait this PR now uses a `.defaults(enabledTraits:)` overload which just uses the `"default"` trait under the hood. This simplifies the logic tremendously to handle default traits. # Result Nicer spelling for default traits and easier implementation.
1 parent 2f3a2a5 commit e626518

15 files changed

+65
-86
lines changed

Fixtures/Traits/Example/Package.swift

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,24 @@
55
let package = Package(
66
name: "TraitsExample",
77
traits: [
8-
Trait(name: "Package1", isDefault: true),
9-
Trait(name: "Package2", isDefault: true),
10-
Trait(name: "Package3", isDefault: true),
11-
Trait(name: "Package4", isDefault: true),
8+
.default(
9+
enabledTraits: [
10+
"Package1",
11+
"Package2",
12+
"Package3",
13+
"Package4",
14+
"BuildCondition1",
15+
]
16+
),
17+
"Package1",
18+
"Package2",
19+
"Package3",
20+
"Package4",
1221
"Package5",
1322
"Package7",
1423
"Package9",
1524
"Package10",
16-
Trait(name: "BuildCondition1", isDefault: true),
25+
"BuildCondition1",
1726
"BuildCondition2",
1827
"BuildCondition3",
1928
],

Fixtures/Traits/Package3/Package.swift

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,10 @@ let package = Package(
1111
),
1212
],
1313
traits: [
14-
Trait(name: "Package3Trait1", enabledTraits: ["Package3Trait2"]),
15-
Trait(name: "Package3Trait2", enabledTraits: ["Package3Trait3"]),
16-
Trait(name: "Package3Trait3", isDefault: true),
14+
.default(enabledTraits: ["Package3Trait3"]),
15+
.trait(name: "Package3Trait1", enabledTraits: ["Package3Trait2"]),
16+
.trait(name: "Package3Trait2", enabledTraits: ["Package3Trait3"]),
17+
"Package3Trait3",
1718
],
1819
targets: [
1920
.target(

Fixtures/Traits/Package4/Package.swift

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@ let package = Package(
1111
),
1212
],
1313
traits: [
14-
Trait(name: "Package4Trait1", isDefault: true),
14+
.default(enabledTraits: ["Package4Trait1"]),
15+
"Package4Trait1",
1516
],
1617
targets: [
1718
.target(

Sources/PackageDescription/PackageDependencyTrait.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ extension Package.Dependency {
1616
@available(_PackageDescription, introduced: 999.0)
1717
public struct Trait: Hashable, Sendable, ExpressibleByStringLiteral {
1818
/// Enables all default traits of a package.
19-
public static let defaults = Self.init(name: "defaults")
19+
public static let defaults = Self.init(name: "default")
2020

2121
/// A condition that limits the application of a dependencies trait.
2222
public struct Condition: Hashable, Sendable {

Sources/PackageDescription/PackageDescriptionSerialization.swift

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,6 @@ enum Serialization {
270270
struct Trait: Hashable, Codable {
271271
let name: String
272272
let description: String?
273-
let isDefault: Bool
274273
let enabledTraits: Set<String>
275274
}
276275

Sources/PackageDescription/PackageDescriptionSerializationConversion.swift

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,6 @@ extension Serialization.Trait {
382382
init(_ trait: PackageDescription.Trait) {
383383
self.name = trait.name
384384
self.description = trait.description
385-
self.isDefault = trait.isDefault
386385
self.enabledTraits = trait.enabledTraits
387386
}
388387
}

Sources/PackageDescription/Trait.swift

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,15 @@
1818
@_spi(ExperimentalTraits)
1919
@available(_PackageDescription, introduced: 999.0)
2020
public struct Trait: Hashable, ExpressibleByStringLiteral {
21+
/// Declares the default traits for this package.
22+
public static func `default`(enabledTraits: Set<String>) -> Self {
23+
.init(
24+
name: "default",
25+
description: "The default traits of this package.",
26+
enabledTraits: enabledTraits
27+
)
28+
}
29+
2130
/// The trait's canonical name.
2231
///
2332
/// This is used when enabling the trait or when referring to it from other modifiers in the manifest.
@@ -35,9 +44,6 @@ public struct Trait: Hashable, ExpressibleByStringLiteral {
3544
/// Use this to explain what functionality this trait enables.
3645
public var description: String?
3746

38-
/// A boolean indicating wether the trait is enabled by default.
39-
public var isDefault: Bool
40-
4147
/// A set of other traits of this package that this trait enables.
4248
public var enabledTraits: Set<String>
4349

@@ -46,17 +52,14 @@ public struct Trait: Hashable, ExpressibleByStringLiteral {
4652
/// - Parameters:
4753
/// - name: The trait's canonical name.
4854
/// - description: The trait's description.
49-
/// - isDefault: A boolean indicating wether the trait is enabled by default.
5055
/// - enabledTraits: A set of other traits of this package that this trait enables.
5156
public init(
5257
name: String,
5358
description: String? = nil,
54-
isDefault: Bool = false,
5559
enabledTraits: Set<String> = []
5660
) {
5761
self.name = name
5862
self.description = description
59-
self.isDefault = isDefault
6063
self.enabledTraits = enabledTraits
6164
}
6265

@@ -69,18 +72,15 @@ public struct Trait: Hashable, ExpressibleByStringLiteral {
6972
/// - Parameters:
7073
/// - name: The trait's canonical name.
7174
/// - description: The trait's description.
72-
/// - isDefault: A boolean indicating wether the trait is enabled by default.
7375
/// - enabledTraits: A set of other traits of this package that this trait enables.
7476
public static func trait(
7577
name: String,
7678
description: String? = nil,
77-
isDefault: Bool = false,
7879
enabledTraits: Set<String> = []
7980
) -> Trait {
8081
.init(
8182
name: name,
8283
description: description,
83-
isDefault: isDefault,
8484
enabledTraits: enabledTraits
8585
)
8686
}

Sources/PackageGraph/ModulesGraph+Loading.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -799,7 +799,7 @@ private func calculateEnabledTraits(
799799
) throws -> Set<String> {
800800
// This the point where we flatten the enabled traits and resolve the recursive traits
801801
var recursiveEnabledTraits = explictlyEnabledTraits ?? []
802-
let areDefaultsEnabled = recursiveEnabledTraits.remove("defaults") != nil
802+
let areDefaultsEnabled = recursiveEnabledTraits.remove("default") != nil
803803

804804
// We are going to calculate which traits are actually enabled for a node here. To do this
805805
// we have to check if default traits should be used and then flatten all the enabled traits.
@@ -813,7 +813,7 @@ private func calculateEnabledTraits(
813813

814814
// We have to enable all default traits if no traits are enabled or the defaults are explicitly enabled
815815
if explictlyEnabledTraits == nil || areDefaultsEnabled {
816-
recursiveEnabledTraits.formUnion(manifest.traits.lazy.filter { $0.isDefault }.map { $0.name })
816+
recursiveEnabledTraits.formUnion(manifest.traits.first { $0.name == "default" }?.enabledTraits ?? [])
817817
}
818818

819819
while true {

Sources/PackageLoading/ManifestJSONParser.swift

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -605,7 +605,6 @@ extension TraitDescription {
605605
self.init(
606606
name: trait.name,
607607
description: trait.description,
608-
isDefault: trait.isDefault,
609608
enabledTraits: trait.enabledTraits
610609
)
611610
}

Sources/PackageLoading/ManifestLoader+Validation.swift

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -107,11 +107,6 @@ public struct ManifestValidator {
107107
continue
108108
}
109109

110-
guard traitName.lowercased() != "default" && traitName.lowercased() != "defaults" else {
111-
diagnostics.append(.defaultTraitName())
112-
continue
113-
}
114-
115110
for (index, unicodeScalar) in traitName.unicodeScalars.enumerated() {
116111
let properties = unicodeScalar.properties
117112

@@ -370,10 +365,6 @@ extension Basics.Diagnostic {
370365
.error("Empty strings are not allowed as trait names")
371366
}
372367

373-
static func defaultTraitName() -> Self {
374-
.error("Traits are not allowed to be named 'default' or 'defaults' to avoid confusion with default traits")
375-
}
376-
377368
static func invalidFirstCharacterInTrait(firstCharater: UnicodeScalar, trait: String) -> Self {
378369
.error("Invalid first character (\(firstCharater)) in trait \(trait). The first character must be a Unicode XID start character (most letters), a digit, or _.")
379370
}

Sources/PackageModel/Manifest/TraitDescription.swift

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,6 @@ public struct TraitDescription: Sendable, Hashable, Codable, ExpressibleByString
1919
/// The trait's description.
2020
public var description: String?
2121

22-
/// A boolean indicating wether the trait is enabled by default.
23-
public var isDefault: Bool
24-
2522
/// A set of other traits of this package that this trait enables.
2623
public var enabledTraits: Set<String>
2724

@@ -30,17 +27,14 @@ public struct TraitDescription: Sendable, Hashable, Codable, ExpressibleByString
3027
/// - Parameters:
3128
/// - name: The trait's canonical name.
3229
/// - description: The trait's description.
33-
/// - isDefault: A boolean indicating wether the trait is enabled by default.
3430
/// - enabledTraits: A set of other traits of this package that this trait enables.
3531
public init(
3632
name: String,
3733
description: String? = nil,
38-
isDefault: Bool = false,
3934
enabledTraits: Set<String> = []
4035
) {
4136
self.name = name
4237
self.description = description
43-
self.isDefault = isDefault
4438
self.enabledTraits = enabledTraits
4539
}
4640

Sources/SPMTestSupport/PackageDependencyDescriptionExtensions.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ package extension PackageDependency {
2222
deprecatedName: String? = nil,
2323
path: AbsolutePath,
2424
productFilter: ProductFilter = .everything,
25-
traits: Set<Trait> = [.init(name: "defaults")]
25+
traits: Set<Trait> = [.init(name: "default")]
2626
) -> Self {
2727
let identity = identity ?? PackageIdentity(path: path)
2828
return .fileSystem(
@@ -40,7 +40,7 @@ package extension PackageDependency {
4040
path: AbsolutePath,
4141
requirement: SourceControl.Requirement,
4242
productFilter: ProductFilter = .everything,
43-
traits: Set<Trait> = [.init(name: "defaults")]
43+
traits: Set<Trait> = [.init(name: "default")]
4444
) -> Self {
4545
let identity = identity ?? PackageIdentity(path: path)
4646
return .localSourceControl(
@@ -59,7 +59,7 @@ package extension PackageDependency {
5959
url: SourceControlURL,
6060
requirement: SourceControl.Requirement,
6161
productFilter: ProductFilter = .everything,
62-
traits: Set<Trait> = [.init(name: "defaults")]
62+
traits: Set<Trait> = [.init(name: "default")]
6363
) -> Self {
6464
let identity = identity ?? PackageIdentity(url: url)
6565
return .remoteSourceControl(
@@ -76,7 +76,7 @@ package extension PackageDependency {
7676
identity: String,
7777
requirement: Registry.Requirement,
7878
productFilter: ProductFilter = .everything,
79-
traits: Set<Trait> = [.init(name: "defaults")]
79+
traits: Set<Trait> = [.init(name: "default")]
8080
) -> Self {
8181
return .registry(
8282
identity: .plain(identity),

Tests/FunctionalTests/TraitTests.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ final class TraitTests: XCTestCase {
3535

3636
func testTraits_whenTraitUnification() async throws {
3737
try await fixture(name: "Traits") { fixturePath in
38-
let (stdout, _) = try await executeSwiftRun(fixturePath.appending("Example"), "Example", extraArgs: ["--experimental-traits", "defaults,Package9,Package10"])
38+
let (stdout, _) = try await executeSwiftRun(fixturePath.appending("Example"), "Example", extraArgs: ["--experimental-traits", "default,Package9,Package10"])
3939
XCTAssertEqual(stdout, """
4040
Package1Library1 trait1 enabled
4141
Package2Library1 trait2 enabled
@@ -55,7 +55,7 @@ final class TraitTests: XCTestCase {
5555

5656
func testTraits_whenTraitUnification_whenSecondTraitNotEnabled() async throws {
5757
try await fixture(name: "Traits") { fixturePath in
58-
let (stdout, _) = try await executeSwiftRun(fixturePath.appending("Example"), "Example", extraArgs: ["--experimental-traits", "defaults,Package9"])
58+
let (stdout, _) = try await executeSwiftRun(fixturePath.appending("Example"), "Example", extraArgs: ["--experimental-traits", "default,Package9"])
5959
XCTAssertEqual(stdout, """
6060
Package1Library1 trait1 enabled
6161
Package2Library1 trait2 enabled
@@ -73,7 +73,7 @@ final class TraitTests: XCTestCase {
7373

7474
func testTraits_whenIndividualTraitsEnabled_andDefaultTraits() async throws {
7575
try await fixture(name: "Traits") { fixturePath in
76-
let (stdout, _) = try await executeSwiftRun(fixturePath.appending("Example"), "Example", extraArgs: ["--experimental-traits", "defaults,Package5,Package7,BuildCondition3"])
76+
let (stdout, _) = try await executeSwiftRun(fixturePath.appending("Example"), "Example", extraArgs: ["--experimental-traits", "default,Package5,Package7,BuildCondition3"])
7777
XCTAssertEqual(stdout, """
7878
Package1Library1 trait1 enabled
7979
Package2Library1 trait2 enabled
@@ -170,7 +170,7 @@ final class TraitTests: XCTestCase {
170170
let json = try JSON(bytes: ByteString(encodingAsUTF8: dumpOutput))
171171
guard case let .dictionary(contents) = json else { XCTFail("unexpected result"); return }
172172
guard case let .array(traits)? = contents["experimentalTraits"] else { XCTFail("unexpected result"); return }
173-
XCTAssertEqual(traits.count, 11)
173+
XCTAssertEqual(traits.count, 12)
174174
}
175175
}
176176

Tests/PackageGraphTests/ModulesGraphTests.swift

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2919,7 +2919,7 @@ final class ModulesGraphTests: XCTestCase {
29192919
),
29202920
],
29212921
traits: [
2922-
.init(name: "Trait1", isDefault: true),
2922+
.init(name: "default", enabledTraits: ["Trait1"]),
29232923
"Trait1",
29242924
]
29252925
),
@@ -2955,7 +2955,8 @@ final class ModulesGraphTests: XCTestCase {
29552955
),
29562956
],
29572957
traits: [
2958-
.init(name: "Trait1", isDefault: true, enabledTraits: ["Trait2"]),
2958+
.init(name: "default", enabledTraits: ["Trait1"]),
2959+
.init(name: "Trait1", enabledTraits: ["Trait2"]),
29592960
.init(name: "Trait2", enabledTraits: ["Trait3", "Trait4"]),
29602961
"Trait3",
29612962
.init(name: "Trait4", enabledTraits: ["Trait5"]),
@@ -3004,7 +3005,7 @@ final class ModulesGraphTests: XCTestCase {
30043005
),
30053006
],
30063007
traits: [
3007-
.init(name: "Package1Trait1", isDefault: true),
3008+
.init(name: "default", enabledTraits: ["Package1Trait1"]),
30083009
"Package1Trait1",
30093010
]
30103011
),
@@ -3072,7 +3073,8 @@ final class ModulesGraphTests: XCTestCase {
30723073
),
30733074
],
30743075
traits: [
3075-
.init(name: "Package1Trait1", isDefault: true)
3076+
.init(name: "default", enabledTraits: ["Package1Trait1"]),
3077+
.init(name: "Package1Trait1")
30763078
]
30773079
),
30783080
Manifest.createFileSystemManifest(
@@ -3164,8 +3166,9 @@ final class ModulesGraphTests: XCTestCase {
31643166
),
31653167
],
31663168
traits: [
3167-
.init(name: "Package1Trait1", isDefault: true),
3168-
.init(name: "Package1Trait2", isDefault: true),
3169+
.init(name: "default", enabledTraits: ["Package1Trait1", "Package1Trait2"]),
3170+
.init(name: "Package1Trait1"),
3171+
.init(name: "Package1Trait2"),
31693172
]
31703173
),
31713174
Manifest.createFileSystemManifest(

0 commit comments

Comments
 (0)