Skip to content

[Traits] Correctly diagnose unused dependency with traits #7702

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.

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

Merged
merged 1 commit into from
Jun 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
34 changes: 32 additions & 2 deletions Sources/PackageGraph/ModulesGraph+Loading.swift
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,22 @@ private func checkAllDependenciesAreUsed(
}
})

// List all dependencies of modules that are guarded by a trait.
let traitGuardedProductDependencies = Set(package.underlying.modules.flatMap { module in
module.dependencies.compactMap { moduleDependency in
switch moduleDependency {
case .product(let product, let conditions):
if conditions.contains(where: { $0.traitCondition != nil }) {
// This is a product dependency that was enabled by a trait
return product.name
}
return nil
case .module:
return nil
}
}
})

for dependencyId in package.dependencies {
guard let dependency = packages[dependencyId] else {
observabilityScope.emit(.error("Unknown package: \(dependencyId)"))
Expand All @@ -300,7 +316,16 @@ private func checkAllDependenciesAreUsed(
if dependency.products.contains(where: \.isCommandPlugin) {
continue
}


// Skip this check if traits are enabled since it is valid to add a dependency just
// to enable traits on it. This is useful if there is a transitive dependency in the graph
// that can be configured by enabling traits e.g. the depdency has a trait for its logging
// behaviour. This allows the root package to configure traits of transitive dependencies
// without emitting an unused dependency warning.
if !dependency.enabledTraits.isEmpty {
continue
}

// Make sure that any diagnostics we emit below are associated with the package.
let packageDiagnosticsScope = observabilityScope.makeChildScope(
description: "Package Dependency Validation",
Expand All @@ -311,7 +336,12 @@ private func checkAllDependenciesAreUsed(
let dependencyIsUsed = dependency.products.contains { product in
// Don't compare by product ID, but by product name to make sure both build triples as properties of
// `ResolvedProduct.ID` are allowed.
productDependencies.contains { $0.name == product.name }
let usedByPackage = productDependencies.contains { $0.name == product.name }
// We check if any of the products of this dependency is guarded by a trait.
let traitGuarded = traitGuardedProductDependencies.contains(product.name)

// If the product is either used directly or guarded by a trait we consider it as used
return usedByPackage || traitGuarded
}

if !dependencyIsUsed && !observabilityScope.errorsReportedInAnyScope {
Expand Down
32 changes: 24 additions & 8 deletions Tests/FunctionalTests/TraitTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ import XCTest
final class TraitTests: XCTestCase {
func testTraits_whenNoFlagPassed() async throws {
try await fixture(name: "Traits") { fixturePath in
let (stdout, _) = try await executeSwiftRun(fixturePath.appending("Example"), "Example")
let (stdout, stderr) = try await executeSwiftRun(fixturePath.appending("Example"), "Example")
// We expect no warnings to be produced. Specifically no unused dependency warnings.
XCTAssertFalse(stderr.contains("warning:"))
XCTAssertEqual(stdout, """
Package1Library1 trait1 enabled
Package2Library1 trait2 enabled
Expand All @@ -34,7 +36,9 @@ final class TraitTests: XCTestCase {

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

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

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

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

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

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

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