Remove potential for assertion - sort dependencies and generate rathe…

…r than re-validating during generation
dfed · Jan 19, 2024 · 842e32d · 842e32d
1 parent c85d4e6
commit 842e32d
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 91 deletions.
diff --git a/Sources/SafeDICore/Generators/DependencyTreeGenerator.swift b/Sources/SafeDICore/Generators/DependencyTreeGenerator.swift
@@ -214,8 +214,8 @@ public final class DependencyTreeGenerator {
             })
 
         // Populate the propertiesToInstantiate on each scope.
-        for scope in typeDescriptionToScopeMap.values {
-            var additionalPropertiesToInstantiate = [Scope.PropertyToInstantiate]()
+        for scope in Set(typeDescriptionToScopeMap.values) {
+            var propertiesToInstantiate = [Scope.PropertyToInstantiate]()
             for instantiatedDependency in scope.instantiable.dependencies {
                 switch instantiatedDependency.source {
                 case .instantiated:
@@ -238,20 +238,20 @@ public final class DependencyTreeGenerator {
                     case .constant, .instantiator:
                         break
                     }
-                    additionalPropertiesToInstantiate.append(.instantiated(
+                    propertiesToInstantiate.append(.instantiated(
                         instantiatedDependency.property,
                         instantiatedScope
                     ))
                 case let .aliased(fulfillingProperty):
-                    additionalPropertiesToInstantiate.append(.aliased(
+                    propertiesToInstantiate.append(.aliased(
                         instantiatedDependency.property,
                         fulfilledBy: fulfillingProperty
                     ))
                 case .forwarded, .received:
                     continue
                 }
             }
-            scope.propertiesToInstantiate.append(contentsOf: additionalPropertiesToInstantiate)
+            scope.propertiesToInstantiate.append(contentsOf: propertiesToInstantiate)
         }
         return typeDescriptionToScopeMap
     }

diff --git a/Sources/SafeDICore/Generators/ScopeGenerator.swift b/Sources/SafeDICore/Generators/ScopeGenerator.swift
@@ -26,8 +26,7 @@ actor ScopeGenerator {
     init(
         instantiable: Instantiable,
         property: Property?,
-        propertiesToGenerate: [ScopeGenerator],
-        receivedProperties: Set<Property>
+        propertiesToGenerate: [ScopeGenerator]
     ) {
         if let property {
             scopeData = .property(
@@ -45,7 +44,6 @@ actor ScopeGenerator {
             scopeData = .root(instantiable: instantiable)
         }
         self.property = property
-        self.receivedProperties = receivedProperties
         self.propertiesToGenerate = propertiesToGenerate
         forwardedProperties = scopeData.forwardedProperties
         propertiesMadeAvailableByChildren = Set(
@@ -86,15 +84,13 @@ actor ScopeGenerator {
 
     init(
         property: Property,
-        fulfillingProperty: Property,
-        receivedProperties: Set<Property>
+        fulfillingProperty: Property
     ) {
         scopeData = .alias(property: property, fulfillingProperty: fulfillingProperty)
         requiredReceivedProperties = [fulfillingProperty]
         propertiesToGenerate = []
         forwardedProperties = []
         propertiesMadeAvailableByChildren = []
-        self.receivedProperties = receivedProperties
         self.property = property
     }
 
@@ -242,21 +238,23 @@ actor ScopeGenerator {
     private let scopeData: ScopeData
     private let requiredReceivedProperties: Set<Property>
     private let propertiesMadeAvailableByChildren: Set<Property>
-    private let receivedProperties: Set<Property>
     private let propertiesToGenerate: [ScopeGenerator]
     private let property: Property?
     private let forwardedProperties: Set<Property>
 
-    private var resolvedProperties = Set<Property>()
     private var generateCodeTask: Task<String, Error>?
 
     private func generateProperties(leadingMemberWhitespace: String) async throws -> [String] {
         var generatedProperties = [String]()
-        while
-            let childGenerator = nextSatisfiableProperty(),
-            let childProperty = childGenerator.property
-        {
-            resolvedProperties.insert(childProperty)
+        let orderedPropertiesToGenerate = propertiesToGenerate.sorted(by: { lhs, rhs in
+            guard let lhsProperty = lhs.property else {
+                return true
+            }
+            // We must generate properties that are required by other properties first
+            return rhs.requiredReceivedProperties.contains(lhsProperty)
+        })
+
+        for childGenerator in orderedPropertiesToGenerate {
             generatedProperties.append(
                 try await childGenerator
                     .generateCode(leadingWhitespace: leadingMemberWhitespace)
@@ -265,44 +263,6 @@ actor ScopeGenerator {
         return generatedProperties
     }
 
-    private func nextSatisfiableProperty() -> ScopeGenerator? {
-        let remainingProperties = propertiesToGenerate.filter {
-            if let property = $0.property {
-                !resolvedProperties.contains(property)
-            } else {
-                false
-            }
-        }
-        guard !remainingProperties.isEmpty else {
-            return nil
-        }
-
-        for propertyToGenerate in remainingProperties {
-            guard hasResolvedAllPropertiesRequired(for: propertyToGenerate) else {
-                continue
-            }
-            return propertyToGenerate
-        }
-
-        assertionFailure("Unexpected failure: unable to find next satisfiable property")
-        return nil
-    }
-
-    private func hasResolvedAllPropertiesRequired(for propertyToGenerate: ScopeGenerator) -> Bool {
-        !propertyToGenerate
-            .requiredReceivedProperties
-            .contains(where: {
-                !isPropertyResolved($0)
-                && !propertyToGenerate.forwardedProperties.contains($0)
-            })
-    }
-
-    private func isPropertyResolved(_ property: Property) -> Bool {
-        resolvedProperties.contains(property)
-        || receivedProperties.contains(property)
-        || forwardedProperties.contains(property)
-    }
-
     // MARK: GenerationError
 
     private enum GenerationError: Error, CustomStringConvertible {

diff --git a/Sources/SafeDICore/Models/Scope.swift b/Sources/SafeDICore/Models/Scope.swift
@@ -21,14 +21,27 @@
 import Collections
 
 /// A model of the scoped dependencies required for an `@Instantiable` in the reachable dependency tree.
-final class Scope {
+final class Scope: Hashable {
 
     // MARK: Initialization
 
     init(instantiable: Instantiable) {
         self.instantiable = instantiable
     }
 
+    // MARK: Equatable
+
+    static func == (lhs: Scope, rhs: Scope) -> Bool {
+        // Scopes are only identicial if they are the same object
+        lhs === rhs
+    }
+
+    // MARK: Hashable
+
+    func hash(into hasher: inout Hasher) {
+        hasher.combine(ObjectIdentifier(self))
+    }
+
     // MARK: Internal
 
     let instantiable: Instantiable
@@ -48,15 +61,6 @@ final class Scope {
                 property
             }
         }
-
-        var scope: Scope? {
-            switch self {
-            case let .instantiated(_, scope):
-                scope
-            case .aliased:
-                nil
-            }
-        }
     }
 
     var properties: [Property] {
@@ -76,7 +80,7 @@ final class Scope {
                     fulfillingProperty
                 case .forwarded,
                         .instantiated:
-                    return nil
+                    nil
                 }
             }
     }
@@ -95,26 +99,6 @@ final class Scope {
             if let property {
                 childPropertyStack.insert(property, at: 0)
             }
-            let receivedProperties = Set(
-                instantiableStack
-                    .flatMap(\.dependencies)
-                    .filter {
-                        switch $0.source {
-                        // The source has been injected into the dependency tree.
-                        case .instantiated,
-                                .forwarded,
-                            // This property has been re-injected into the dependency tree under a new alias.
-                                .aliased:
-                            return !propertyStack.contains($0.property) && $0.property != property
-                        case .received:
-                            return false
-                        }
-                    }
-                    .map(\.property)
-            ).subtracting(
-                // We want the local version of any instantiated property.
-                propertiesToInstantiate.map(\.property)
-            )
             let scopeGenerator = ScopeGenerator(
                 instantiable: instantiable,
                 property: property,
@@ -129,12 +113,10 @@ final class Scope {
                     case let .aliased(property, fulfilledBy: fulfillingProperty):
                         ScopeGenerator(
                             property: property,
-                            fulfillingProperty: fulfillingProperty,
-                            receivedProperties: receivedProperties
+                            fulfillingProperty: fulfillingProperty
                         )
                     }
-                },
-                receivedProperties: receivedProperties
+                }
             )
             Task.detached {
                 // Kick off code generation.