Detect received cycles (#34)

Sources/SafeDICore/Generators/ScopeGenerator.swift

@@ -18,8 +18,10 @@
 // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 // SOFTWARE.
 
+import Collections
+
 /// A model capable of generating code for a scope’s dependency tree.
-actor ScopeGenerator {
+actor ScopeGenerator: CustomStringConvertible, Hashable {
 
     // MARK: Initialization
 
@@ -39,8 +41,10 @@ actor ScopeGenerator {
                         .map(\.property)
                 )
             )
+            description = instantiable.concreteInstantiableType.asSource
         } else {
             scopeData = .root(instantiable: instantiable)
+            description = instantiable.concreteInstantiableType.asSource
         }
         self.property = property
         self.propertiesToGenerate = propertiesToGenerate
@@ -78,11 +82,29 @@ actor ScopeGenerator {
     ) {
         scopeData = .alias(property: property, fulfillingProperty: fulfillingProperty)
         requiredReceivedProperties = [fulfillingProperty]
+        description = property.asSource
         propertiesToGenerate = []
         propertiesToDeclare = []
         self.property = property
     }
 
+    // MARK: CustomStringConvertible
+
+    let description: String
+
+    // MARK: Equatable
+
+    static func == (lhs: ScopeGenerator, rhs: ScopeGenerator) -> Bool {
+        lhs.scopeData == rhs.scopeData
+    }
+
+    // MARK: Hashable
+
+    nonisolated
+    func hash(into hasher: inout Hasher) {
+        hasher.combine(scopeData)
+    }
+
     // MARK: Internal
 
     func generateCode(leadingWhitespace: String = "") async throws -> String {
@@ -202,7 +224,7 @@ actor ScopeGenerator {
 
     // MARK: Private
 
-    private enum ScopeData {
+    private enum ScopeData: Hashable {
         case root(instantiable: Instantiable)
         case property(
             instantiable: Instantiable,
@@ -236,42 +258,75 @@ actor ScopeGenerator {
     private var generateCodeTask: Task<String, Error>?
 
     private var orderedPropertiesToGenerate: [ScopeGenerator] {
-        guard var orderedPropertiesToGenerate = List(self.propertiesToGenerate) else { return [] }
-        for propertyToGenerate in orderedPropertiesToGenerate {
-            let hasDependenciesGeneratedByCurrentScope = !propertyToGenerate
-                .requiredReceivedProperties
-                .isDisjoint(with: propertiesToDeclare)
-            guard hasDependenciesGeneratedByCurrentScope else {
-                // This property does not have received dependencies generated by this scope, therefore its ordering is irrelevant.
-                continue
+        get throws {
+            // Step 1: Build dependency graph
+            var scopeToIncomingDependencyScopes = [ScopeGenerator: [ScopeGenerator]]()
+            var scopeToOutgoingUnresolvedDependenciesCount = OrderedDictionary<ScopeGenerator, Int>()
+
+            for scope in propertiesToGenerate {
+                // Find dependencies.
+                for otherScope in propertiesToGenerate {
+                    if
+                        let property = otherScope.property,
+                        scope.requiredReceivedProperties.contains(property)
+                    {
+                        scopeToOutgoingUnresolvedDependenciesCount[scope, default: 0] += 1
+                        scopeToIncomingDependencyScopes[otherScope, default: []] += [scope]
+                    }
+                }
             }
-            if let lastDependencyToGenerate = propertyToGenerate
-                // Ignore our own property.
-                .dropFirst()
-                // Reverse the list to find the last property we depend on first.
-                .reversed()
-                .first(where: { futurePropertyToGenerate in
-                    if let futureProperty = futurePropertyToGenerate.property,
-                       propertyToGenerate
-                        .requiredReceivedProperties
-                        .contains(futureProperty)
-                    { true } else { false }
-                })
-            {
-                // We depend on (at least) one item further ahead in the list!
-                // Make sure we are created after our dependencies.
-                lastDependencyToGenerate.insert(propertyToGenerate.value)
-                if let head = propertyToGenerate.remove() {
-                    orderedPropertiesToGenerate = head
+
+            // Step 2: Topological Sort
+            var nextPropertiesToGenerate = propertiesToGenerate
+                .filter { scopeToOutgoingUnresolvedDependenciesCount[$0] == nil }
+
+            var orderedPropertiesToGenerate = [ScopeGenerator]()
+            while !nextPropertiesToGenerate.isEmpty {
+                let nextProperty = nextPropertiesToGenerate.removeFirst()
+                orderedPropertiesToGenerate.append(nextProperty)
+
+                if let incomingDependencyScopes = scopeToIncomingDependencyScopes[nextProperty] {
+                    for dependantScope in incomingDependencyScopes {
+                        if let dependenciesCount = scopeToOutgoingUnresolvedDependenciesCount[dependantScope] {
+                            let newDependenciesCount = dependenciesCount - 1
+                            scopeToOutgoingUnresolvedDependenciesCount[dependantScope] = newDependenciesCount
+
+                            if newDependenciesCount == 0 {
+                                nextPropertiesToGenerate.append(dependantScope)
+                            }
+                        }
+                    }
+                }
+            }
+
+            // Step 3: Check for cycle
+            if !scopeToOutgoingUnresolvedDependenciesCount.isEmpty {
+                func detectCycle(from scope: ScopeGenerator, stack: OrderedSet<Property> = []) throws {
+                    var stack = stack
+                    if let property = scope.property {
+                        if stack.contains(property) {
+                            throw GenerationError.dependencyCycleDetected(
+                                stack.drop(while: { $0 != property }) + [property],
+                                scope: self
+                            )
+                        }
+                        stack.append(property)
+                        for nextScope in scopeToIncomingDependencyScopes[scope, default: []] {
+                            try detectCycle(from: nextScope, stack: stack)
+                        }
+                    }
+                }
+                for scope in scopeToOutgoingUnresolvedDependenciesCount.keys {
+                    try detectCycle(from: scope)
                 }
             }
+            return orderedPropertiesToGenerate
         }
-        return orderedPropertiesToGenerate.map(\.value)
     }
 
     private func generateProperties(leadingMemberWhitespace: String) async throws -> [String] {
         var generatedProperties = [String]()
-        for childGenerator in orderedPropertiesToGenerate {
+        for childGenerator in try orderedPropertiesToGenerate {
             generatedProperties.append(
                 try await childGenerator
                     .generateCode(leadingWhitespace: leadingMemberWhitespace)
@@ -284,11 +339,17 @@ actor ScopeGenerator {
 
     private enum GenerationError: Error, CustomStringConvertible {
         case forwardingInstantiatorGenericDoesNotMatch(property: Property, instantiable: Instantiable)
+        case dependencyCycleDetected(any Collection<Property>, scope: ScopeGenerator)
 
         var description: String {
             switch self {
             case let .forwardingInstantiatorGenericDoesNotMatch(property, instantiable):
                 "Property `\(property.asSource)` on \(instantiable.concreteInstantiableType.asSource) incorrectly configured. Property should instead be of type `\(Dependency.forwardingInstantiatorType)<\(instantiable.concreteInstantiableType.asSource).ForwardedArguments, \(property.typeDescription.asInstantiatedType.asSource)>`."
+            case let .dependencyCycleDetected(properties, scope):
+                """
+                Dependency cycle detected on \(scope)!
+                \(properties.map(\.asSource).joined(separator: " -> "))
+                """
             }
         }
     }