[NFC] Remove redundant DependencyResolver protocol (#7127)

### Motivation:

This protocol doesn't define any requirements and was used for
nemaspacing a single tuple typealias. Instead of typealiasing this
fairly complex and pervasively used tuple, we should define a separate
struct for it. Then the protocol itself can be removed as completely
unused.

### Modifications:

Defined new `struct DependencyResolverBinding` with cleanly named
fields, which now avoids expressions that look like `binding.binding`
and re-declaration of this tuple in multiple places.

One other `typealias` in `protocol DependencyResolver` is unused, thus
the whole protocol is unused and now can be removed.

Also used this as an opportunity to clean up file naming and directory
layout in the `PackageGraph` module.


### Result:

This confusing protocol is removed from the codebase. Readability is
improved with explicit struct field names instead of ad-hoc tuple use.
Directory layout makes it more obvious the purpose of specific files for
potential new contributors and for existing contributors when reviewing
code retrospectively.

Author: MaxDesiatov

Sources/PackageGraph/CMakeLists.txt

@@ -9,8 +9,6 @@
 add_library(PackageGraph
   BoundVersion.swift
   DependencyMirrors.swift
-  DependencyResolutionNode.swift
-  DependencyResolver.swift
   Diagnostics.swift
   GraphLoadingNode.swift
   ModuleAliasTracker.swift
@@ -21,17 +19,21 @@ add_library(PackageGraph
   PackageModel+Extensions.swift
   PackageRequirement.swift
   PinsStore.swift
-  PubGrub/Assignment.swift
-  PubGrub/ContainerProvider.swift
-  PubGrub/DiagnosticReportBuilder.swift
-  PubGrub/Incompatibility.swift
-  PubGrub/PartialSolution.swift
-  PubGrub/PubGrubDependencyResolver.swift
-  PubGrub/PubGrubPackageContainer.swift
-  PubGrub/Term.swift  
-  ResolvedPackage.swift
-  ResolvedProduct.swift
-  ResolvedTarget.swift
+  Resolution/PubGrub/Assignment.swift
+  Resolution/PubGrub/ContainerProvider.swift
+  Resolution/PubGrub/DiagnosticReportBuilder.swift
+  Resolution/PubGrub/Incompatibility.swift
+  Resolution/PubGrub/PartialSolution.swift
+  Resolution/PubGrub/PubGrubDependencyResolver.swift
+  Resolution/PubGrub/PubGrubPackageContainer.swift
+  Resolution/PubGrub/Term.swift
+  Resolution/DependencyResolutionNode.swift
+  Resolution/DependencyResolverBinding.swift
+  Resolution/DependencyResolverDelegate.swift
+  Resolution/DependencyResolverError.swift
+  Resolution/ResolvedPackage.swift
+  Resolution/ResolvedProduct.swift
+  Resolution/ResolvedTarget.swift
   Version+Extensions.swift
   VersionSetSpecifier.swift)
 target_link_libraries(PackageGraph PUBLIC

Sources/PackageGraph/GraphLoadingNode.swift

@@ -18,7 +18,7 @@ import PackageModel
 ///
 /// This node uses the product filter that was already finalized during resolution.
 ///
-/// - SeeAlso: DependencyResolutionNode
+/// - SeeAlso: ``DependencyResolutionNode``
 public struct GraphLoadingNode: Equatable, Hashable {
 
     /// The package identity.

Sources/PackageGraph/DependencyResolutionNode.swift renamed to Sources/PackageGraph/Resolution/DependencyResolutionNode.swift

@@ -18,7 +18,7 @@ import struct TSCUtility.Version
 ///
 /// See the documentation of each case for more detailed descriptions of each kind and how they interact.
 ///
-/// - SeeAlso: `GraphLoadingNode`
+/// - SeeAlso: ``GraphLoadingNode``
 public enum DependencyResolutionNode {
 
     /// An empty package node.

Sources/PackageGraph/Resolution/DependencyResolverBinding.swift

@@ -0,0 +1,20 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the Swift open source project
+//
+// Copyright (c) 2014-2023 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See http://swift.org/LICENSE.txt for license information
+// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+
+import enum PackageModel.ProductFilter
+import struct PackageModel.PackageReference
+
+public struct DependencyResolverBinding {
+    public let package: PackageReference
+    public let boundVersion: BoundVersion
+    public let products: ProductFilter
+}

Sources/PackageGraph/DependencyResolver.swift renamed to Sources/PackageGraph/Resolution/DependencyResolverDelegate.swift

@@ -2,7 +2,7 @@
 //
 // This source file is part of the Swift open source project
 //
-// Copyright (c) 2014-2017 Apple Inc. and the Swift project authors
+// Copyright (c) 2014-2023 Apple Inc. and the Swift project authors
 // Licensed under Apache License v2.0 with Runtime Library Exception
 //
 // See http://swift.org/LICENSE.txt for license information
@@ -16,25 +16,6 @@ import PackageModel
 
 import struct TSCUtility.Version
 
-public protocol DependencyResolver {
-    typealias Binding = (package: PackageReference, binding: BoundVersion, products: ProductFilter)
-    typealias Delegate = DependencyResolverDelegate
-}
-
-public enum DependencyResolverError: Error, Equatable {
-     /// A revision-based dependency contains a local package dependency.
-    case revisionDependencyContainsLocalPackage(dependency: String, localPackage: String)
-}
-
-extension DependencyResolverError: CustomStringConvertible {
-    public var description: String {
-        switch self {
-        case .revisionDependencyContainsLocalPackage(let dependency, let localPackage):
-            return "package '\(dependency)' is required using a revision-based requirement and it depends on local package '\(localPackage)', which is not supported"
-        }
-    }
-}
-
 public protocol DependencyResolverDelegate {
     func willResolve(term: Term)
     func didResolve(term: Term, version: Version, duration: DispatchTimeInterval)
@@ -44,7 +25,7 @@ public protocol DependencyResolverDelegate {
     func satisfied(term: Term, by assignment: Assignment, incompatibility: Incompatibility)
     func partiallySatisfied(term: Term, by assignment: Assignment, incompatibility: Incompatibility, difference: Term)
     func failedToResolve(incompatibility: Incompatibility)
-    func solved(result: [DependencyResolver.Binding])
+    func solved(result: [DependencyResolverBinding])
 }
 
 public struct ObservabilityDependencyResolverDelegate: DependencyResolverDelegate {
@@ -82,9 +63,9 @@ public struct ObservabilityDependencyResolverDelegate: DependencyResolverDelegat
         self.debug("\(term) is partially satisfied by '\(assignment)', which is caused by '\(assignment.cause?.description ?? "unknown cause")'. new incompatibility \(incompatibility)")
     }
 
-    public func solved(result: [DependencyResolver.Binding]) {
-        for (package, binding, _) in result {
-            self.debug("solved '\(package.identity)' (\(package.locationString)) at '\(binding)'")
+    public func solved(result: [DependencyResolverBinding]) {
+        for binding in result {
+            self.debug("solved '\(binding.package.identity)' (\(binding.package.locationString)) at '\(binding.boundVersion)'")
         }
         self.debug("dependency resolution complete!")
     }
@@ -129,7 +110,7 @@ public struct MultiplexResolverDelegate: DependencyResolverDelegate {
         underlying.forEach { $0.failedToResolve(incompatibility: incompatibility)  }
     }
 
-    public func solved(result: [(package: PackageReference, binding: BoundVersion, products: ProductFilter)]) {
+    public func solved(result: [DependencyResolverBinding]) {
         underlying.forEach { $0.solved(result: result)  }
     }
 

Sources/PackageGraph/Resolution/DependencyResolverError.swift

@@ -0,0 +1,27 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the Swift open source project
+//
+// Copyright (c) 2014-2023 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See http://swift.org/LICENSE.txt for license information
+// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+
+import Foundation
+
+public enum DependencyResolverError: Error, Equatable {
+     /// A revision-based dependency contains a local package dependency.
+    case revisionDependencyContainsLocalPackage(dependency: String, localPackage: String)
+}
+
+extension DependencyResolverError: CustomStringConvertible {
+    public var description: String {
+        switch self {
+        case .revisionDependencyContainsLocalPackage(let dependency, let localPackage):
+            return "package '\(dependency)' is required using a revision-based requirement and it depends on local package '\(localPackage)', which is not supported"
+        }
+    }
+}

Sources/PackageGraph/PubGrub/Assignment.swift renamed to Sources/PackageGraph/Resolution/PubGrub/Assignment.swift

@@ -140,7 +140,7 @@ public struct PubGrubDependencyResolver {
     }
 
     /// Execute the resolution algorithm to find a valid assignment of versions.
-    public func solve(constraints: [Constraint]) -> Result<[DependencyResolver.Binding], Error> {
+    public func solve(constraints: [Constraint]) -> Result<[DependencyResolverBinding], Error> {
         // the graph resolution root
         let root: DependencyResolutionNode
         if constraints.count == 1, let constraint = constraints.first, constraint.package.kind.isRoot {
@@ -180,7 +180,7 @@ public struct PubGrubDependencyResolver {
     /// Find a set of dependencies that fit the given constraints. If dependency
     /// resolution is unable to provide a result, an error is thrown.
     /// - Warning: It is expected that the root package reference has been set  before this is called.
-    internal func solve(root: DependencyResolutionNode, constraints: [Constraint]) throws -> (bindings: [DependencyResolver.Binding], state: State) {
+    internal func solve(root: DependencyResolutionNode, constraints: [Constraint]) throws -> (bindings: [DependencyResolverBinding], state: State) {
         // first process inputs
         let inputs = try self.processInputs(root: root, with: constraints)
 
@@ -241,18 +241,22 @@ public struct PubGrubDependencyResolver {
                 flattenedAssignments[updatePackage] = (binding: boundVersion, products: products)
             }
         }
-        var finalAssignments: [DependencyResolver.Binding]
+        var finalAssignments: [DependencyResolverBinding]
             = flattenedAssignments.keys.sorted(by: { $0.deprecatedName < $1.deprecatedName }).map { package in
                 let details = flattenedAssignments[package]!
-                return (package: package, binding: details.binding, products: details.products)
+                return .init(package: package, boundVersion: details.binding, products: details.products)
             }
 
         // Add overridden packages to the result.
         for (package, override) in state.overriddenPackages {
             // TODO: replace with async/await when available
             let container = try temp_await { provider.getContainer(for: package, completion: $0) }
             let updatePackage = try container.underlying.loadPackageReference(at: override.version)
-            finalAssignments.append((updatePackage, override.version, override.products))
+            finalAssignments.append(.init(
+                    package: updatePackage,
+                    boundVersion: override.version,
+                    products: override.products
+            ))
         }
 
         self.delegate?.solved(result: finalAssignments)

Sources/PackageGraph/PubGrub/ContainerProvider.swift renamed to Sources/PackageGraph/Resolution/PubGrub/ContainerProvider.swift

@@ -25,6 +25,7 @@ import struct PackageGraph.Assignment
 import enum PackageGraph.BoundVersion
 import enum PackageGraph.ContainerUpdateStrategy
 import protocol PackageGraph.CustomPackageContainer
+import struct PackageGraph.DependencyResolverBinding
 import protocol PackageGraph.DependencyResolverDelegate
 import struct PackageGraph.Incompatibility
 import struct PackageGraph.MultiplexResolverDelegate
@@ -631,7 +632,7 @@ extension Workspace {
     @discardableResult
     fileprivate func updateDependenciesCheckouts(
         root: PackageGraphRoot,
-        updateResults: [(PackageReference, BoundVersion, ProductFilter)],
+        updateResults: [DependencyResolverBinding],
         updateBranches: Bool = false,
         observabilityScope: ObservabilityScope
     ) -> [(PackageReference, PackageStateChange)] {
@@ -954,7 +955,7 @@ extension Workspace {
     /// Computes states of the packages based on last stored state.
     fileprivate func computePackageStateChanges(
         root: PackageGraphRoot,
-        resolvedDependencies: [(PackageReference, BoundVersion, ProductFilter)],
+        resolvedDependencies: [DependencyResolverBinding],
         updateBranches: Bool,
         observabilityScope: ObservabilityScope
     ) throws -> [(PackageReference, PackageStateChange)] {
@@ -963,59 +964,59 @@ extension Workspace {
         var packageStateChanges: [PackageIdentity: (PackageReference, PackageStateChange)] = [:]
 
         // Set the states from resolved dependencies results.
-        for (packageRef, binding, products) in resolvedDependencies {
+        for binding in resolvedDependencies {
             // Get the existing managed dependency for this package ref, if any.
 
             // first find by identity only since edit location may be different by design
-            var currentDependency = self.state.dependencies[packageRef.identity]
+            var currentDependency = self.state.dependencies[binding.package.identity]
             // Check if this is an edited dependency.
             if case .edited(let basedOn, _) = currentDependency?.state, let originalReference = basedOn?.packageRef {
                 packageStateChanges[originalReference.identity] = (originalReference, .unchanged)
             } else {
                 // if not edited, also compare by location since it may have changed
-                currentDependency = self.state.dependencies[comparingLocation: packageRef]
+                currentDependency = self.state.dependencies[comparingLocation: binding.package]
             }
 
-            switch binding {
+            switch binding.boundVersion {
             case .excluded:
                 throw InternalError("Unexpected excluded binding")
 
             case .unversioned:
                 // Ignore the root packages.
-                if root.packages.keys.contains(packageRef.identity) {
+                if root.packages.keys.contains(binding.package.identity) {
                     continue
                 }
 
                 if let currentDependency {
                     switch currentDependency.state {
                     case .fileSystem, .edited:
-                        packageStateChanges[packageRef.identity] = (packageRef, .unchanged)
+                        packageStateChanges[binding.package.identity] = (binding.package, .unchanged)
                     case .sourceControlCheckout:
-                        let newState = PackageStateChange.State(requirement: .unversioned, products: products)
-                        packageStateChanges[packageRef.identity] = (packageRef, .updated(newState))
+                        let newState = PackageStateChange.State(requirement: .unversioned, products: binding.products)
+                        packageStateChanges[binding.package.identity] = (binding.package, .updated(newState))
                     case .registryDownload:
                         throw InternalError("Unexpected unversioned binding for downloaded dependency")
                     case .custom:
                         throw InternalError("Unexpected unversioned binding for custom dependency")
                     }
                 } else {
-                    let newState = PackageStateChange.State(requirement: .unversioned, products: products)
-                    packageStateChanges[packageRef.identity] = (packageRef, .added(newState))
+                    let newState = PackageStateChange.State(requirement: .unversioned, products: binding.products)
+                    packageStateChanges[binding.package.identity] = (binding.package, .added(newState))
                 }
 
             case .revision(let identifier, let branch):
                 // Get the latest revision from the container.
                 // TODO: replace with async/await when available
                 guard let container = try (temp_await {
                     packageContainerProvider.getContainer(
-                        for: packageRef,
+                        for: binding.package,
                         updateStrategy: .never,
                         observabilityScope: observabilityScope,
                         on: .sharedConcurrent,
                         completion: $0
                     )
                 }) as? SourceControlPackageContainer else {
-                    throw InternalError("invalid container for \(packageRef) expected a SourceControlPackageContainer")
+                    throw InternalError("invalid container for \(binding.package) expected a SourceControlPackageContainer")
                 }
                 var revision = try container.getRevision(forIdentifier: identifier)
                 let branch = branch ?? (identifier == revision.identifier ? nil : identifier)
@@ -1024,7 +1025,7 @@ extension Workspace {
                 // branches, use the revision from pin instead (if present).
                 if branch != nil, !updateBranches {
                     if case .branch(branch, let pinRevision) = pinsStore.pins.values
-                        .first(where: { $0.packageRef == packageRef })?.state
+                        .first(where: { $0.packageRef == binding.package })?.state
                     {
                         revision = Revision(identifier: pinRevision)
                     }
@@ -1043,21 +1044,21 @@ extension Workspace {
                     if case .sourceControlCheckout(let checkoutState) = currentDependency.state,
                        checkoutState == newState
                     {
-                        packageStateChanges[packageRef.identity] = (packageRef, .unchanged)
+                        packageStateChanges[binding.package.identity] = (binding.package, .unchanged)
                     } else {
                         // Otherwise, we need to update this dependency to this revision.
                         let newState = PackageStateChange.State(
                             requirement: .revision(revision, branch: branch),
-                            products: products
+                            products: binding.products
                         )
-                        packageStateChanges[packageRef.identity] = (packageRef, .updated(newState))
+                        packageStateChanges[binding.package.identity] = (binding.package, .updated(newState))
                     }
                 } else {
                     let newState = PackageStateChange.State(
                         requirement: .revision(revision, branch: branch),
-                        products: products
+                        products: binding.products
                     )
-                    packageStateChanges[packageRef.identity] = (packageRef, .added(newState))
+                    packageStateChanges[binding.package.identity] = (binding.package, .added(newState))
                 }
 
             case .version(let version):
@@ -1066,11 +1067,11 @@ extension Workspace {
                 case .sourceControlCheckout(.version(version, _)), .registryDownload(version), .custom(version, _):
                     stateChange = .unchanged
                 case .edited, .fileSystem, .sourceControlCheckout, .registryDownload, .custom:
-                    stateChange = .updated(.init(requirement: .version(version), products: products))
+                    stateChange = .updated(.init(requirement: .version(version), products: binding.products))
                 case nil:
-                    stateChange = .added(.init(requirement: .version(version), products: products))
+                    stateChange = .added(.init(requirement: .version(version), products: binding.products))
                 }
-                packageStateChanges[packageRef.identity] = (packageRef, stateChange)
+                packageStateChanges[binding.package.identity] = (binding.package, stateChange)
             }
         }
         // Set the state of any old package that might have been removed.
@@ -1114,7 +1115,7 @@ extension Workspace {
         resolver: PubGrubDependencyResolver,
         constraints: [PackageContainerConstraint],
         observabilityScope: ObservabilityScope
-    ) -> [(package: PackageReference, binding: BoundVersion, products: ProductFilter)] {
+    ) -> [DependencyResolverBinding] {
         os_signpost(.begin, name: SignpostName.pubgrub)
         let result = resolver.solve(constraints: constraints)
         os_signpost(.end, name: SignpostName.pubgrub)
@@ -1179,7 +1180,7 @@ private struct WorkspaceDependencyResolverDelegate: DependencyResolverDelegate {
         difference: Term
     ) {}
     func failedToResolve(incompatibility: Incompatibility) {}
-    func solved(result: [(package: PackageReference, binding: BoundVersion, products: ProductFilter)]) {}
+    func solved(result: [DependencyResolverBinding]) {}
 }
 
 // FIXME: the manifest loading logic should be changed to use identity instead of location once identity is unique

Sources/PackageGraph/PubGrub/DiagnosticReportBuilder.swift renamed to Sources/PackageGraph/Resolution/PubGrub/DiagnosticReportBuilder.swift

@@ -69,7 +69,7 @@ class DependencyResolverRealWorldPerfTests: XCTestCasePerf {
                 switch resolver.solve(constraints: graph.constraints) {
                 case .success(let result):
                     let result: [(container: PackageReference, version: Version)] = result.compactMap {
-                        guard case .version(let version) = $0.binding else {
+                        guard case .version(let version) = $0.boundVersion else {
                             XCTFail("Unexpected result")
                             return nil
                         }