[Traits] Disallow disabling default traits of a package without traits (#8326)

# Motivation

Traits are a great way for package authors to offer customization of the
functionality they provide. However, moving existing API behind a trait
is considered an API breaking change since packages that depend on them
might have disabled all default traits. This makes it almost impossible
for existing packages to adopt traits for existing code.

# Modifications

This PR disallows disabling the default traits for packages with no
traits at all. This allows package authors to move existing API behind
traits once since no consumer can disable the default traits before.

# Result

With this change we create a migration path for existing packages to
traits without them breaking their APIs.

Author: FranzBusch

Fixtures/Traits/DisablingEmptyDefaultsExample/Package.swift

@@ -0,0 +1,18 @@
+// swift-tools-version: 6.1
+
+import PackageDescription
+
+let package = Package(
+    name: "DisablingEmptyDefaultsExample",
+    dependencies: [
+        .package(
+            path: "../Package11",
+            traits: []
+        ),
+    ],
+    targets: [
+        .executableTarget(
+            name: "DisablingEmptyDefaultsExample"
+        ),
+    ]
+)

Fixtures/Traits/DisablingEmptyDefaultsExample/Sources/DisablingEmptyDefaultsExample/Example.swift

@@ -0,0 +1,6 @@
+@main
+struct Example {
+    static func main() {
+
+    }
+}

Fixtures/Traits/Package11/Package.swift

@@ -0,0 +1,18 @@
+// swift-tools-version: 6.1
+
+import PackageDescription
+
+let package = Package(
+    name: "Package11",
+    products: [
+        .library(
+            name: "Package11Library1",
+            targets: ["Package11Library1"]
+        ),
+    ],
+    targets: [
+        .target(
+            name: "Package11Library1"
+        ),
+    ]
+)

Fixtures/Traits/Package11/Sources/Package11Library1/Library.swift

@@ -0,0 +1,3 @@
+public func hello() {
+    print("Package11Library1")
+}

Sources/PackageGraph/ModulesGraph+Loading.swift

@@ -112,6 +112,7 @@ extension ModulesGraph {
                 manifest: package.manifest,
                 productFilter: .everything,
                 enabledTraits: calculateEnabledTraits(
+                    parentPackage: nil,
                     identity: identity,
                     manifest: package.manifest,
                     explictlyEnabledTraits: enabledTraits
@@ -154,6 +155,7 @@ extension ModulesGraph {
                                 manifest: manifest,
                                 productFilter: dependency.productFilter,
                                 enabledTraits: calculateEnabledTraits(
+                                    parentPackage: node.item.identity,
                                     identity: dependency.identity,
                                     manifest: manifest,
                                     explictlyEnabledTraits: explictlyEnabledTraits.flatMap { Set($0) }
@@ -941,6 +943,7 @@ private func emitDuplicateProductDiagnostic(
 }
 
 private func calculateEnabledTraits(
+    parentPackage: PackageIdentity?,
     identity: PackageIdentity,
     manifest: Manifest,
     explictlyEnabledTraits: Set<String>?
@@ -958,6 +961,15 @@ private func calculateEnabledTraits(
             throw ModuleError.invalidTrait(package: identity, trait: trait)
         }
     }
+    
+    if let parentPackage, !(explictlyEnabledTraits == nil || areDefaultsEnabled) && manifest.traits.isEmpty {
+        // We throw an error when default traits are disabled for a package without any traits
+        // This allows packages to initially move new API behind traits once.
+        throw ModuleError.disablingDefaultTraitsOnEmptyTraits(
+            parentPackage: parentPackage,
+            packageName: manifest.displayName
+        )
+    }
 
     // We have to enable all default traits if no traits are enabled or the defaults are explicitly enabled
     if explictlyEnabledTraits == nil || areDefaultsEnabled {

Sources/PackageLoading/PackageBuilder.swift

@@ -97,6 +97,11 @@ public enum ModuleError: Swift.Error {
         package: PackageIdentity,
         trait: String
     )
+    
+    case disablingDefaultTraitsOnEmptyTraits(
+        parentPackage: PackageIdentity,
+        packageName: String
+    )
 }
 
 extension ModuleError: CustomStringConvertible {
@@ -179,6 +184,10 @@ extension ModuleError: CustomStringConvertible {
             return """
             Trait '"\(trait)"' is not declared by package '\(package)'.
             """
+        case .disablingDefaultTraitsOnEmptyTraits(let parentPackage, let packageName):
+            return """
+            Disabled default traits by package '\(parentPackage)' on package '\(packageName)' that declares no traits. This is prohibited to allow packages to adopt traits initially without causing an API break.
+            """
         }
     }
 }

Sources/_InternalTestSupport/ManifestExtensions.swift

@@ -32,7 +32,7 @@ extension Manifest {
         dependencies: [PackageDependency] = [],
         products: [ProductDescription] = [],
         targets: [TargetDescription] = [],
-        traits: Set<TraitDescription> = []
+        traits: Set<TraitDescription> = [.init(name: "defaults")]
     ) -> Manifest {
         Self.createManifest(
             displayName: displayName,
@@ -70,7 +70,7 @@ extension Manifest {
         dependencies: [PackageDependency] = [],
         products: [ProductDescription] = [],
         targets: [TargetDescription] = [],
-        traits: Set<TraitDescription> = []
+        traits: Set<TraitDescription> = [.init(name: "defaults")]
     ) -> Manifest {
         Self.createManifest(
             displayName: displayName,
@@ -220,7 +220,7 @@ extension Manifest {
         dependencies: [PackageDependency] = [],
         products: [ProductDescription] = [],
         targets: [TargetDescription] = [],
-        traits: Set<TraitDescription> = []
+        traits: Set<TraitDescription> = [.init(name: "defaults")]
     ) -> Manifest {
         return Manifest(
             displayName: displayName,

Sources/_InternalTestSupport/MockPackage.swift

@@ -35,7 +35,7 @@ public struct MockPackage {
         targets: [MockTarget],
         products: [MockProduct] = [],
         dependencies: [MockDependency] = [],
-        traits: Set<TraitDescription> = [],
+        traits: Set<TraitDescription> = [.init(name: "defaults")],
         versions: [String?] = [],
         revisionProvider: ((String) -> String)? = nil,
         toolsVersion: ToolsVersion? = nil
@@ -60,7 +60,7 @@ public struct MockPackage {
         targets: [MockTarget],
         products: [MockProduct],
         dependencies: [MockDependency] = [],
-        traits: Set<TraitDescription> = [],
+        traits: Set<TraitDescription> = [.init(name: "defaults")],
         versions: [String?] = [],
         revisionProvider: ((String) -> String)? = nil,
         toolsVersion: ToolsVersion? = nil
@@ -86,7 +86,7 @@ public struct MockPackage {
         targets: [MockTarget],
         products: [MockProduct],
         dependencies: [MockDependency] = [],
-        traits: Set<TraitDescription> = [],
+        traits: Set<TraitDescription> = [.init(name: "defaults")],
         versions: [String?] = [],
         revisionProvider: ((String) -> String)? = nil,
         toolsVersion: ToolsVersion? = nil

Tests/FunctionalTests/TraitTests.swift

@@ -264,5 +264,24 @@ final class TraitTests: XCTestCase {
             XCTAssertTrue(symbolGraph.contains("TypeGatedByPackage10Trait2"))
         }
     }
+    
+    func testPackageDisablinDefaultsTrait_whenNoTraits() async throws {
+        try await fixture(name: "Traits") { fixturePath in
+            do {
+                let (_, _) = try await executeSwiftRun(fixturePath.appending("DisablingEmptyDefaultsExample"), "DisablingEmptyDefaultsExample")
+            } catch let error as SwiftPMError {
+                switch error {
+                case .packagePathNotFound:
+                    throw error
+                case .executionFailure(_, _, let stderr):
+                    let expectedErr = """
+                    error: Disabled default traits by package 'disablingemptydefaultsexample' on package 'Package11' that declares no traits. This is prohibited to allow packages to adopt traits initially without causing an API break.
+
+                    """
+                    XCTAssertTrue(stderr.contains(expectedErr))
+                }
+            }
+        }
+    }
 }
 #endif