Implement DuplicateRegistrationDetector for whole graph Containers

Package.resolved

@@ -1,5 +1,5 @@
 {
-  "originHash" : "80b25f78b0b17934ec8338366df30b99e1b361299a3ebc0c9671bdb9b520b5b6",
+  "originHash" : "e7be7ae2f468d0465b934f5d628592d6882882d2b0d232d6fa1cce6407427a36",
   "pins" : [
     {
       "identity" : "swift-argument-parser",
@@ -22,10 +22,10 @@
     {
       "identity" : "swinject",
       "kind" : "remoteSourceControl",
-      "location" : "https://github.com/Swinject/Swinject.git",
+      "location" : "https://github.com/bradfol/Swinject.git",
       "state" : {
-        "revision" : "be9dbcc7b86811bc131539a20c6f9c2d3e56919f",
-        "version" : "2.9.1"
+        "branch" : "service-entry",
+        "revision" : "c4441192155a5e9a8206117132d708b1362dda3b"
       }
     },
     {

Package.swift

@@ -15,7 +15,7 @@ let package = Package(
         .executable(name: "knit-cli", targets: ["KnitCommand"]),
     ],
     dependencies: [
-        .package(url: "https://github.com/Swinject/Swinject.git", from: "2.9.1"),
+        .package(url: "https://github.com/bradfol/Swinject.git", branch: "service-entry"),
         .package(url: "https://github.com/Swinject/SwinjectAutoregistration.git", from: "2.9.1"),
         .package(url: "https://github.com/apple/swift-syntax.git", from: "510.0.2"),
         .package(url: "https://github.com/apple/swift-argument-parser", from: "1.4.0"),

README.md

@@ -63,7 +63,48 @@ The following commands are available:
 
 #### Default Setting
 
-By default Knit generates named getters for its type safety. 
+By default Knit generates named getters for its type safety.
+
+## Duplicate Registration Detection
+
+As an app scales, so do the number of DI registrations. It is important to detect duplicate registrations because by
+default Swinject will overwrite early registrations with any duplicates.
+While this capability is useful in test environments, at runtime it almost always indicates a programming/configuration
+error, which could lead to bugs.
+
+Knit includes two capabilities to detect duplicate registrations for additional DI graph safety:
+
+1. Compile-time detection within a single module.
+1. Runtime detection across an entire container's graph.
+
+### Compile-time Detection
+
+When Knit parses the assembly files within a single module it will automatically detect any duplicates
+found during parsing and immediately report an error with information about the duplicate.
+This is always on and automatic.
+
+### Runtime Detection
+
+Knit only parses the assemblies in a single module at a time so it is not able to detect duplicates across modules
+during parsing/compile time. 
+Instead Knit provides a `DuplicateRegistrationDetector` class, which is a Swinject Behavior.
+`DuplicateRegistrationDetector` allows for runtime detection of all registrations made to a single Container (DI graph),
+regardless of which modules those registrations came from. 
+This adds safety from duplicate registrations to your whole DI graph.
+
+An instance of DuplicateRegistrationDetector should be added to the Container to make use of this feature. Configuration steps:
+
+1. Create an instance of `DuplicateRegistrationDetector`.
+1. Provide that instance to your Container (ScopedModuleAssembler/ModuleAssembler/Assembler also allow behaviors to be
+    passed into their initializers).
+1. Perform the registration phase of your Container setup. If you are using ScopedModuleAssembler/ModuleAssembler then the registration phase will be complete after the initialer returns.
+1. Check the `detectedKeys` property of your `DuplicateRegistrationDetector` instance for duplicates.
+
+Note that there are also helpers on `DuplicateRegistrationDetector.Key` and `Array<DuplicateRegistrationDetector.Key>`
+to help with creating reports/error messages that are easier to read.
+
+`DuplicateRegistrationDetector` also provides a `duplicateWasDetected` closure hook if you would like to be informed of each
+duplicate at the moment that duplicate is registered.
 
 ---
 

Sources/Knit/DuplicateRegistrationDetector.swift

@@ -0,0 +1,117 @@
+//
+// Copyright © Block, Inc. All rights reserved.
+//
+
+import Swinject
+
+public final class DuplicateRegistrationDetector {
+
+    /// If a duplicate registration is detected, the `Key` describing that registration will be provided to this closure.
+    /// The closure can be called multiple times, once for each duplicate found.
+    public var duplicateWasDetected: ((Key) -> Void)?
+
+    /// An array of all the keys for duplicate registrations detected by this Behavior.
+    /// The array is updated before the `duplicateWasDetected` closure is invoked so will always contain all detected duplicates up to that point.
+    public private(set) var detectedKeys = [Key]()
+
+    /// Describes a single registration key.
+    /// If a duplicate is detected this key will be provided to the `duplicateWasDetected` closure.
+    public struct Key {
+        public let serviceType: Any.Type
+        public let argumentsType: Any.Type
+        public let name: String?
+    }
+
+    var existingRegistrations = Set<Key>()
+
+    public init(
+        duplicateWasDetected: ((Key) -> Void)? = nil
+    ) {
+        self.duplicateWasDetected = duplicateWasDetected
+    }
+
+}
+
+// MARK: -
+
+extension DuplicateRegistrationDetector: Behavior {
+
+    public func container<Type, Service>(
+        _ container: Container,
+        didRegisterType type: Type.Type,
+        toService entry: ServiceEntry<Service>,
+        withName name: String?
+    ) {
+        // Make sure to use `didRegisterType type: Type.Type` rather than `Service`
+        let key = Key(
+            serviceType: type,
+            argumentsType: entry.argumentsType,
+            name: name
+        )
+
+        let preInsertCount = existingRegistrations.count
+        existingRegistrations.insert(key)
+
+        if preInsertCount == existingRegistrations.count {
+            // The registration count did not increment, so the current service entry was a duplicate of an existing entry
+            detectedKeys.append(key)
+            duplicateWasDetected?(key)
+        }
+    }
+
+}
+
+// MARK: -
+
+extension DuplicateRegistrationDetector.Key: Hashable, Equatable {
+
+    public func hash(into hasher: inout Hasher) {
+        ObjectIdentifier(serviceType).hash(into: &hasher)
+        ObjectIdentifier(argumentsType).hash(into: &hasher)
+        name?.hash(into: &hasher)
+    }
+
+    public static func == (lhs: Self, rhs: Self) -> Bool {
+        return lhs.serviceType == rhs.serviceType
+            && lhs.argumentsType == rhs.argumentsType
+            && lhs.name == rhs.name
+    }
+
+}
+
+// MARK: -
+
+extension DuplicateRegistrationDetector.Key: CustomStringConvertible {
+
+    // Provide a more structured string description of the key, useful for logging error messages
+    public var description: String {
+        """
+        Duplicate Registration Key
+        Service type: \(serviceType)
+        Arguments type: \(argumentsType)
+        Name: \(name ?? "`nil`")
+        """
+    }
+
+
+}
+
+// MARK: -
+
+extension Array where Element == DuplicateRegistrationDetector.Key {
+
+    public var duplicatesDescription: String {
+        guard count > 0 else {
+            return "No duplicates in array"
+        }
+
+        let keyDescriptions = reduce("") { partialResult, key in
+            partialResult + "\(key)\n\n"
+        }
+        return """
+            Duplicate registrations were detected (count: \(count):
+            \(keyDescriptions)
+            """
+    }
+
+}

Sources/Knit/Module/ModuleAssembler.swift

@@ -72,6 +72,7 @@ public final class ModuleAssembler {
         overrideBehavior: OverrideBehavior = .defaultOverridesWhenTesting,
         assemblyValidation: ((any ModuleAssembly.Type) throws -> Void)? = nil,
         errorFormatter: ModuleAssemblerErrorFormatter = DefaultModuleAssemblerErrorFormatter(),
+        behaviors: [Behavior] = [],
         postAssemble: ((Container) -> Void)? = nil
     ) throws {
         self.builder = try DependencyBuilder(
@@ -84,7 +85,10 @@ public final class ModuleAssembler {
         )
 
         self.parent = parent
-        self._container = Container(parent: parent?._container)
+        self._container = Container(
+            parent: parent?._container,
+            behaviors: behaviors
+        )
         self.serviceCollector = .init(parent: parent?.serviceCollector)
         self._container.addBehavior(serviceCollector)
         let abstractRegistrations = self._container.registerAbstractContainer()

Sources/Knit/Module/ScopedModuleAssembler.swift

@@ -25,6 +25,7 @@ public final class ScopedModuleAssembler<ScopedResolver> {
         _ modules: [any ModuleAssembly],
         overrideBehavior: OverrideBehavior = .defaultOverridesWhenTesting,
         errorFormatter: ModuleAssemblerErrorFormatter = DefaultModuleAssemblerErrorFormatter(),
+        behaviors: [Behavior] = [],
         postAssemble: ((Container) -> Void)? = nil,
         file: StaticString = #fileID,
         line: UInt = #line
@@ -34,6 +35,7 @@ public final class ScopedModuleAssembler<ScopedResolver> {
                 parent: parent,
                 _modules: modules,
                 overrideBehavior: overrideBehavior,
+                behaviors: behaviors,
                 postAssemble: postAssemble
             )
         } catch {
@@ -53,6 +55,7 @@ public final class ScopedModuleAssembler<ScopedResolver> {
         _modules modules: [any ModuleAssembly],
         overrideBehavior: OverrideBehavior = .defaultOverridesWhenTesting,
         errorFormatter: ModuleAssemblerErrorFormatter = DefaultModuleAssemblerErrorFormatter(),
+        behaviors: [Behavior] = [],
         postAssemble: ((Container) -> Void)? = nil
     ) throws {
         // For provided modules, fail early if they are scoped incorrectly
@@ -80,6 +83,7 @@ public final class ScopedModuleAssembler<ScopedResolver> {
                 }
             },
             errorFormatter: errorFormatter,
+            behaviors: behaviors,
             postAssemble: postAssemble
         )
     }