From 41f887589309dfe8491c8ad30a7da746d502bee0 Mon Sep 17 00:00:00 2001 From: Dan Federman Date: Fri, 19 Jan 2024 09:21:36 -0800 Subject: [PATCH] Remove potential for assertion - sort dependencies and generate rather than re-validating during generation --- .../Generators/DependencyTreeGenerator.swift | 10 +-- .../Generators/ScopeGenerator.swift | 62 ++++--------------- Sources/SafeDICore/Models/Scope.swift | 61 +++++------------- 3 files changed, 33 insertions(+), 100 deletions(-) diff --git a/Sources/SafeDICore/Generators/DependencyTreeGenerator.swift b/Sources/SafeDICore/Generators/DependencyTreeGenerator.swift index 5dfa9e54..8be54e61 100644 --- 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,12 +238,12 @@ 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 )) @@ -251,7 +251,7 @@ public final class DependencyTreeGenerator { 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 index 488c1c7a..837587b0 100644 --- 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 + 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 + 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 private let propertiesMadeAvailableByChildren: Set - private let receivedProperties: Set private let propertiesToGenerate: [ScopeGenerator] private let property: Property? private let forwardedProperties: Set - private var resolvedProperties = Set() private var generateCodeTask: Task? 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 index 7fd9bf4c..d6be7691 100644 --- a/Sources/SafeDICore/Models/Scope.swift +++ b/Sources/SafeDICore/Models/Scope.swift @@ -21,7 +21,7 @@ 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 @@ -29,6 +29,19 @@ final class Scope { 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 @@ -39,24 +52,6 @@ final class Scope { enum PropertyToInstantiate { case instantiated(Property, Scope) case aliased(Property, fulfilledBy: Property) - - var property: Property { - switch self { - case - let .instantiated(property, _), - let .aliased(property, _): - property - } - } - - var scope: Scope? { - switch self { - case let .instantiated(_, scope): - scope - case .aliased: - nil - } - } } var properties: [Property] { @@ -76,7 +71,7 @@ final class Scope { fulfillingProperty case .forwarded, .instantiated: - return nil + nil } } } @@ -95,26 +90,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 +104,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.