Skip to content

Commit 9fa51c0

Browse files
committed
[Traits] Correctly diagnose unused dependency with traits
# Motivation We are currently diagnosing unused dependencies by looking at the list of products used by the final modules and check if for every root dependency we are using at least one product of that package. This incorrectly diagnosis unused dependencies if the usage of the dependency is either: - Just declared to enable a trait if down the graph another package is depending on it - Used as an optional dependency and the trait is disabled # Modification This PR adds two more checks to the diagnostic pass to find unused dependencies. We first create a set of dependency products that are guarded by a trait. When checking each dependency we then check if we enabled any trait on the dependency since this means it is used for setting a trait. We also check if any product of the dependency is guarded by a trait to identify if it was an optional dependency. # Result No more incorrect unused dependency warnings.
1 parent 2e270a1 commit 9fa51c0

File tree

2 files changed

+53
-10
lines changed

2 files changed

+53
-10
lines changed

Sources/PackageGraph/ModulesGraph+Loading.swift

+29-2
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,22 @@ private func checkAllDependenciesAreUsed(
277277
}
278278
})
279279

280+
// List all dependencies of modules that are guarded by a trait.
281+
let traitGuardedProductDependencies = Set(package.underlying.modules.flatMap { module in
282+
module.dependencies.compactMap { moduleDependency in
283+
switch moduleDependency {
284+
case .product(let product, let conditions):
285+
if conditions.contains(where: { $0.traitCondition != nil }) {
286+
// This is a product dependency that was enabled by a trait
287+
return product.name
288+
}
289+
return nil
290+
case .module:
291+
return nil
292+
}
293+
}
294+
})
295+
280296
for dependencyId in package.dependencies {
281297
guard let dependency = packages[dependencyId] else {
282298
observabilityScope.emit(.error("Unknown package: \(dependencyId)"))
@@ -300,7 +316,13 @@ private func checkAllDependenciesAreUsed(
300316
if dependency.products.contains(where: \.isCommandPlugin) {
301317
continue
302318
}
303-
319+
320+
// Skip this check if traits are enabled since it is valid to add a dependency just
321+
// to enable traits on it.
322+
if !dependency.enabledTraits.isEmpty {
323+
continue
324+
}
325+
304326
// Make sure that any diagnostics we emit below are associated with the package.
305327
let packageDiagnosticsScope = observabilityScope.makeChildScope(
306328
description: "Package Dependency Validation",
@@ -311,7 +333,12 @@ private func checkAllDependenciesAreUsed(
311333
let dependencyIsUsed = dependency.products.contains { product in
312334
// Don't compare by product ID, but by product name to make sure both build triples as properties of
313335
// `ResolvedProduct.ID` are allowed.
314-
productDependencies.contains { $0.name == product.name }
336+
let usedByPackage = productDependencies.contains { $0.name == product.name }
337+
// We check if any of the products of this dependency is guarded by a trait.
338+
let traitGuarded = traitGuardedProductDependencies.contains(product.name)
339+
340+
// If the product is either used directly or guarded by a trait we consider it as used
341+
return usedByPackage || traitGuarded
315342
}
316343

317344
if !dependencyIsUsed && !observabilityScope.errorsReportedInAnyScope {

Tests/FunctionalTests/TraitTests.swift

+24-8
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@ import XCTest
1818
final class TraitTests: XCTestCase {
1919
func testTraits_whenNoFlagPassed() async throws {
2020
try await fixture(name: "Traits") { fixturePath in
21-
let (stdout, _) = try await executeSwiftRun(fixturePath.appending("Example"), "Example")
21+
let (stdout, stderr) = try await executeSwiftRun(fixturePath.appending("Example"), "Example")
22+
// We expect no warnings to be produced. Specifically no unused dependency warnings.
23+
XCTAssertFalse(stderr.contains("warning:"))
2224
XCTAssertEqual(stdout, """
2325
Package1Library1 trait1 enabled
2426
Package2Library1 trait2 enabled
@@ -34,7 +36,9 @@ final class TraitTests: XCTestCase {
3436

3537
func testTraits_whenTraitUnification() async throws {
3638
try await fixture(name: "Traits") { fixturePath in
37-
let (stdout, _) = try await executeSwiftRun(fixturePath.appending("Example"), "Example", extraArgs: ["--experimental-traits", "defaults,Package9,Package10"])
39+
let (stdout, stderr) = try await executeSwiftRun(fixturePath.appending("Example"), "Example", extraArgs: ["--experimental-traits", "defaults,Package9,Package10"])
40+
// We expect no warnings to be produced. Specifically no unused dependency warnings.
41+
XCTAssertFalse(stderr.contains("warning:"))
3842
XCTAssertEqual(stdout, """
3943
Package1Library1 trait1 enabled
4044
Package2Library1 trait2 enabled
@@ -54,7 +58,9 @@ final class TraitTests: XCTestCase {
5458

5559
func testTraits_whenTraitUnification_whenSecondTraitNotEnabled() async throws {
5660
try await fixture(name: "Traits") { fixturePath in
57-
let (stdout, _) = try await executeSwiftRun(fixturePath.appending("Example"), "Example", extraArgs: ["--experimental-traits", "defaults,Package9"])
61+
let (stdout, stderr) = try await executeSwiftRun(fixturePath.appending("Example"), "Example", extraArgs: ["--experimental-traits", "defaults,Package9"])
62+
// We expect no warnings to be produced. Specifically no unused dependency warnings.
63+
XCTAssertFalse(stderr.contains("warning:"))
5864
XCTAssertEqual(stdout, """
5965
Package1Library1 trait1 enabled
6066
Package2Library1 trait2 enabled
@@ -72,7 +78,9 @@ final class TraitTests: XCTestCase {
7278

7379
func testTraits_whenIndividualTraitsEnabled_andDefaultTraits() async throws {
7480
try await fixture(name: "Traits") { fixturePath in
75-
let (stdout, _) = try await executeSwiftRun(fixturePath.appending("Example"), "Example", extraArgs: ["--experimental-traits", "defaults,Package5,Package7,BuildCondition3"])
81+
let (stdout, stderr) = try await executeSwiftRun(fixturePath.appending("Example"), "Example", extraArgs: ["--experimental-traits", "defaults,Package5,Package7,BuildCondition3"])
82+
// We expect no warnings to be produced. Specifically no unused dependency warnings.
83+
XCTAssertFalse(stderr.contains("warning:"))
7684
XCTAssertEqual(stdout, """
7785
Package1Library1 trait1 enabled
7886
Package2Library1 trait2 enabled
@@ -91,7 +99,9 @@ final class TraitTests: XCTestCase {
9199

92100
func testTraits_whenDefaultTraitsDisabled() async throws {
93101
try await fixture(name: "Traits") { fixturePath in
94-
let (stdout, _) = try await executeSwiftRun(fixturePath.appending("Example"), "Example", extraArgs: ["--experimental-disable-default-traits"])
102+
let (stdout, stderr) = try await executeSwiftRun(fixturePath.appending("Example"), "Example", extraArgs: ["--experimental-disable-default-traits"])
103+
// We expect no warnings to be produced. Specifically no unused dependency warnings.
104+
XCTAssertFalse(stderr.contains("warning:"))
95105
XCTAssertEqual(stdout, """
96106
DEFINE1 disabled
97107
DEFINE2 disabled
@@ -103,7 +113,9 @@ final class TraitTests: XCTestCase {
103113

104114
func testTraits_whenIndividualTraitsEnabled_andDefaultTraitsDisabled() async throws {
105115
try await fixture(name: "Traits") { fixturePath in
106-
let (stdout, _) = try await executeSwiftRun(fixturePath.appending("Example"), "Example", extraArgs: ["--experimental-traits", "Package5,Package7"])
116+
let (stdout, stderr) = try await executeSwiftRun(fixturePath.appending("Example"), "Example", extraArgs: ["--experimental-traits", "Package5,Package7"])
117+
// We expect no warnings to be produced. Specifically no unused dependency warnings.
118+
XCTAssertFalse(stderr.contains("warning:"))
107119
XCTAssertEqual(stdout, """
108120
Package5Library1 trait1 enabled
109121
Package6Library1 trait1 enabled
@@ -118,7 +130,9 @@ final class TraitTests: XCTestCase {
118130

119131
func testTraits_whenAllTraitsEnabled() async throws {
120132
try await fixture(name: "Traits") { fixturePath in
121-
let (stdout, _) = try await executeSwiftRun(fixturePath.appending("Example"), "Example", extraArgs: ["--experimental-enable-all-traits"])
133+
let (stdout, stderr) = try await executeSwiftRun(fixturePath.appending("Example"), "Example", extraArgs: ["--experimental-enable-all-traits"])
134+
// We expect no warnings to be produced. Specifically no unused dependency warnings.
135+
XCTAssertFalse(stderr.contains("warning:"))
122136
XCTAssertEqual(stdout, """
123137
Package1Library1 trait1 enabled
124138
Package2Library1 trait2 enabled
@@ -141,7 +155,9 @@ final class TraitTests: XCTestCase {
141155

142156
func testTraits_whenAllTraitsEnabled_andDefaultTraitsDisabled() async throws {
143157
try await fixture(name: "Traits") { fixturePath in
144-
let (stdout, _) = try await executeSwiftRun(fixturePath.appending("Example"), "Example", extraArgs: ["--experimental-enable-all-traits", "--experimental-disable-default-traits"])
158+
let (stdout, stderr) = try await executeSwiftRun(fixturePath.appending("Example"), "Example", extraArgs: ["--experimental-enable-all-traits", "--experimental-disable-default-traits"])
159+
// We expect no warnings to be produced. Specifically no unused dependency warnings.
160+
XCTAssertFalse(stderr.contains("warning:"))
145161
XCTAssertEqual(stdout, """
146162
Package1Library1 trait1 enabled
147163
Package2Library1 trait2 enabled

0 commit comments

Comments
 (0)