Merge pull request #1399 from hartbit/unused-dependencies-check

[SR-6064] Warn on unused dependencies

Author: hartbit

Sources/PackageGraph/PackageGraphLoader.swift

@@ -13,6 +13,18 @@ import PackageLoading
 import PackageModel
 import Utility
 
+struct UnusedDependencyDiagnostic: DiagnosticData {
+    static let id = DiagnosticID(
+        type: UnusedDependencyDiagnostic.self,
+        name: "org.swift.diags.unused-dependency",
+        defaultBehavior: .warning,
+        description: {
+            $0 <<< "dependency" <<< { "'\($0.dependencyName)'" } <<< "is not used by any target"
+        })
+
+    public let dependencyName: String
+}
+
 enum PackageGraphError: Swift.Error {
     /// Indicates a non-root package with no targets.
     case noModules(Package)
@@ -134,13 +146,46 @@ public struct PackageGraphLoader {
             diagnostics: diagnostics
         )
 
+        let rootPackages = resolvedPackages.filter({ rootManifestSet.contains($0.manifest) })
+
+        checkAllDependenciesAreUsed(rootPackages, diagnostics)
+
         return PackageGraph(
-            rootPackages: resolvedPackages.filter({ rootManifestSet.contains($0.manifest) }),
+            rootPackages: rootPackages,
             rootDependencies: resolvedPackages.filter({ rootDependencies.contains($0.manifest) })
         )
     }
 }
 
+private func checkAllDependenciesAreUsed(_ rootPackages: [ResolvedPackage], _ diagnostics: DiagnosticsEngine) {
+    for package in rootPackages {
+        // List all dependency products dependended on by the package targets.
+        let productDependencies: Set<ResolvedProduct> = Set(package.targets.flatMap({ target in
+            return target.dependencies.flatMap({ targetDependency in
+                switch targetDependency {
+                case .product(let product):
+                    return product
+                case .target:
+                    return nil
+                }
+            })
+        }))
+
+        for dependency in package.dependencies {
+            // We continue if the dependency contains executable products to make sure we don't
+            // warn on a valid use-case for a lone dependency: swift run dependency executables.
+            guard !dependency.products.contains(where: { $0.type == .executable }) else {
+                continue
+            }
+            
+            let dependencyIsUsed = dependency.products.contains(where: productDependencies.contains)
+            if !dependencyIsUsed {
+                diagnostics.emit(data: UnusedDependencyDiagnostic(dependencyName: dependency.name))
+            }
+        }
+    }
+}
+
 /// Create resolved packages from the loaded packages.
 private func createResolvedPackages(
     allManifests: [Manifest],

Tests/PackageGraphTests/PackageGraphTests.swift

@@ -170,12 +170,64 @@ class PackageGraphTests: XCTestCase {
         }
     }
 
+    func testUnusedDependency() throws {
+        let fs = InMemoryFileSystem(emptyFiles:
+            "/Foo/Sources/Foo/foo.swift",
+            "/Bar/Sources/Bar/bar.swift",
+            "/Baz/Sources/Baz/baz.swift",
+            "/Biz/Sources/Biz/main.swift"
+        )
+
+        let diagnostics = DiagnosticsEngine()
+        _ = loadMockPackageGraph4([
+            "/Bar": Package(
+                name: "Bar",
+                products: [
+                    .library(name: "BarLibrary", targets: ["Bar"]),
+                ],
+                targets: [
+                    .target(name: "Bar"),
+                ]),
+            "/Baz": Package(
+                name: "Baz",
+                products: [
+                    .library(name: "BazLibrary", targets: ["Baz"]),
+                ],
+                targets: [
+                    .target(name: "Baz"),
+                ]),
+            "/Biz": Package(
+                name: "Biz",
+                products: [
+                    .executable(name: "biz", targets: ["Biz"]),
+                ],
+                targets: [
+                    .target(name: "Biz"),
+                ]),
+            "/Foo": .init(
+                name: "Foo",
+                dependencies: [
+                    .package(url: "/Bar", from: "1.0.0"),
+                    .package(url: "/Baz", from: "1.0.0"),
+                    .package(url: "/Biz", from: "1.0.0"),
+                ],
+                targets: [
+                    .target(name: "Foo", dependencies: ["BarLibrary"]),
+                ]),
+            ], root: "/Foo", diagnostics: diagnostics, in: fs)
+
+        DiagnosticsEngineTester(diagnostics) { result in
+            result.check(diagnostic: "dependency 'Baz' is not used by any target", behavior: .warning)
+        }
+    }
+
     static var allTests = [
         ("testBasic", testBasic),
         ("testDuplicateModules", testDuplicateModules),
         ("testCycle", testCycle),
         ("testProductDependencies", testProductDependencies),
         ("testTestTargetDeclInExternalPackage", testTestTargetDeclInExternalPackage),
         ("testEmptyDependency", testEmptyDependency),
+        ("testUnusedDependency", testUnusedDependency),
     ]
 }

Tests/WorkspaceTests/WorkspaceTests.swift

@@ -1230,7 +1230,7 @@ final class WorkspaceTests: XCTestCase {
                 TestPackage(
                     name: "Root",
                     targets: [
-                        TestTarget(name: "Root", dependencies: ["Foo"]),
+                        TestTarget(name: "Root", dependencies: ["Foo", "Bar"]),
                     ],
                     products: [],
                     dependencies: [
@@ -1456,7 +1456,7 @@ final class WorkspaceTests: XCTestCase {
                 TestPackage(
                     name: "Root",
                     targets: [
-                        TestTarget(name: "Root", dependencies: ["Foo"]),
+                        TestTarget(name: "Root", dependencies: ["Foo", "Bar"]),
                     ],
                     products: [],
                     dependencies: [