Skip to content

Commit

Permalink
Improve + consolidate cycle detection (#100)
Browse files Browse the repository at this point in the history
  • Loading branch information
@dfed
dfed authored Jul 7, 2024
1 parent f7eaa04 commit c6da5d6
Show file tree
Hide file tree
Showing 3 changed files with 149 additions and 67 deletions.
70 changes: 61 additions & 9 deletions Sources/SafeDICore/Generators/DependencyTreeGenerator.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ public final class DependencyTreeGenerator {
case instantiableHasForwardedProperty(property: Property, instantiableWithForwardedProperty: Instantiable, parent: Instantiable)
case constantDependencyCycleDetected([TypeDescription])
case receivedInstantiatorDependencyCycleDetected(property: Property, directParent: TypeDescription, cycle: [TypeDescription])
case receivedConstantCycleDetected(instantiated: Property, receivedPropertyChain: [Property])

var description: String {
switch self {
Expand Down Expand Up @@ -124,6 +125,14 @@ public final class DependencyTreeGenerator {
.reversed()
.joined(separator: " -> "))
"""
case let .receivedConstantCycleDetected(instantiated, receivedPropertyChain):
"""
Dependency received in same chain it is instantiated!
\("@\(Dependency.Source.instantiatedRawValue) \(instantiated.asSource) -> "
+ receivedPropertyChain
.map { "@\(Dependency.Source.receivedRawValue) \($0.asSource)" }
.joined(separator: " -> "))
"""
}
}

Expand Down Expand Up @@ -151,7 +160,7 @@ public final class DependencyTreeGenerator {
try validateReachableTypeDescriptions()

let typeDescriptionToScopeMap = try createTypeDescriptionToScopeMapping()
try validateReceivedProperties(typeDescriptionToScopeMap: typeDescriptionToScopeMap)
try validatePropertiesAreFulfillable(typeDescriptionToScopeMap: typeDescriptionToScopeMap)
return try rootInstantiables
.sorted()
.compactMap {
Expand Down Expand Up @@ -314,9 +323,9 @@ public final class DependencyTreeGenerator {
return typeDescriptionToScopeMap
}

private func validateReceivedProperties(typeDescriptionToScopeMap: [TypeDescription: Scope]) throws {
private func validatePropertiesAreFulfillable(typeDescriptionToScopeMap: [TypeDescription: Scope]) throws {
var unfulfillableProperties = Set<DependencyTreeGeneratorError.UnfulfillableProperty>()
func validateReceivedProperties(
func validatePropertiesAreFulfillable(
on scope: Scope,
receivableProperties: Set<Property>,
property: Property?,
Expand All @@ -341,6 +350,38 @@ public final class DependencyTreeGenerator {
}
.map(\.property)
)
if let property {
func validateNoCycleInReceivedProperties(
scope: Scope,
receivedPropertyStack: OrderedSet<Property>
) throws {
for childProperty in scope.receivedProperties {
guard childProperty != property else {
throw DependencyTreeGeneratorError.receivedConstantCycleDetected(
instantiated: property,
receivedPropertyChain: receivedPropertyStack + [childProperty]
)
}
guard !receivedPropertyStack.contains(childProperty) else {
// We've found a cycle, but it's not our cycle. Bail and let a future loop find this.
return
}
if let receivedPropertyScope = typeDescriptionToScopeMap[childProperty.typeDescription] {
var childPropertyStack = receivedPropertyStack
childPropertyStack.append(childProperty)
try validateNoCycleInReceivedProperties(
scope: receivedPropertyScope,
receivedPropertyStack: childPropertyStack
)
}
}
}
try validateNoCycleInReceivedProperties(
scope: scope,
receivedPropertyStack: []
)
}

for receivedProperty in scope.receivedProperties {
let parentContainsProperty = receivableProperties.contains(receivedProperty)
let propertyIsCreatedAtThisScope = createdProperties.contains(receivedProperty)
Expand Down Expand Up @@ -389,16 +430,17 @@ public final class DependencyTreeGenerator {

for childPropertyToGenerate in scope.propertiesToGenerate {
switch childPropertyToGenerate {
case let .instantiated(property, childScope, _):
guard !childPropertyStack.contains(property) else {
case let .instantiated(childProperty, childScope, _):
guard !childPropertyStack.contains(childProperty) else {
// There is a cycle in our scope tree. Do not re-enter it.
continue
}
try validateReceivedProperties(
try validatePropertiesAreFulfillable(
on: childScope,
receivableProperties: receivableProperties
.union(scope.properties),
property: property,
.union(scope.properties)
.removing(childProperty),
property: childProperty,
propertyStack: childPropertyStack,
root: root
)
Expand All @@ -409,7 +451,7 @@ public final class DependencyTreeGenerator {
}

for rootScope in rootInstantiables.compactMap({ typeDescriptionToScopeMap[$0] }) {
try validateReceivedProperties(
try validatePropertiesAreFulfillable(
on: rootScope,
receivableProperties: Set(rootScope.properties),
property: nil,
Expand Down Expand Up @@ -468,3 +510,13 @@ extension Collection<Dependency> {
}) == nil
}
}

// MARK: - Set

extension Set {
fileprivate func removing(_ element: Element) -> Self {
var setWithoutElement = self
setWithoutElement.remove(element)
return setWithoutElement
}
}
79 changes: 29 additions & 50 deletions Sources/SafeDICore/Generators/ScopeGenerator.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ actor ScopeGenerator: CustomStringConvertible, Sendable {
}

func generateDOT() async throws -> String {
let orderedPropertiesToGenerate = try orderedPropertiesToGenerate
let orderedPropertiesToGenerate = orderedPropertiesToGenerate
let instantiatedProperties = orderedPropertiesToGenerate.map(\.scopeData.asDOTNode)
var childDOTs = [String]()
for orderedPropertyToGenerate in orderedPropertiesToGenerate {
Expand Down Expand Up @@ -351,58 +351,43 @@ actor ScopeGenerator: CustomStringConvertible, Sendable {
private var generateCodeTask: Task<String, Error>?

private var orderedPropertiesToGenerate: [ScopeGenerator] {
get throws {
var orderedPropertiesToGenerate = [ScopeGenerator]()
var propertyToUnfulfilledScopeMap = propertiesToGenerate
.reduce(into: OrderedDictionary<Property, ScopeGenerator>()) { partialResult, scope in
if let property = scope.property {
partialResult[property] = scope
}
}
func fulfill(_ scope: ScopeGenerator, stack: OrderedSet<Property> = []) throws {
guard
let property = scope.property,
propertyToUnfulfilledScopeMap[property] != nil
else {
return
}
guard !stack.contains(property) else {
if property.propertyType.isConstant {
throw GenerationError.dependencyCycleDetected(
stack.drop(while: { $0 != property }) + [property],
scope: self
)
} else {
return
}
var orderedPropertiesToGenerate = [ScopeGenerator]()
var propertyToUnfulfilledScopeMap = propertiesToGenerate
.reduce(into: OrderedDictionary<Property, ScopeGenerator>()) { partialResult, scope in
if let property = scope.property {
partialResult[property] = scope
}

let scopeDependencies = propertyToUnfulfilledScopeMap
.keys
.intersection(scope.requiredReceivedProperties)
.compactMap { propertyToUnfulfilledScopeMap[$0] }
// Fulfill the scopes we depend upon.
for dependentScope in scopeDependencies {
var stack = stack
stack.append(property)
try fulfill(dependentScope, stack: stack)
}
// We can now be marked as fulfilled!
orderedPropertiesToGenerate.append(scope)
propertyToUnfulfilledScopeMap[property] = nil
}

for scope in propertiesToGenerate {
try fulfill(scope)
func fulfill(_ scope: ScopeGenerator) {
guard
let property = scope.property,
propertyToUnfulfilledScopeMap[property] != nil
else {
return
}
let scopeDependencies = propertyToUnfulfilledScopeMap
.keys
.intersection(scope.requiredReceivedProperties)
.compactMap { propertyToUnfulfilledScopeMap[$0] }
// Fulfill the scopes we depend upon.
for dependentScope in scopeDependencies {
fulfill(dependentScope)
}
// We can now be marked as fulfilled!
orderedPropertiesToGenerate.append(scope)
propertyToUnfulfilledScopeMap[property] = nil
}

return orderedPropertiesToGenerate
for scope in propertiesToGenerate {
fulfill(scope)
}

return orderedPropertiesToGenerate
}

private func generateProperties(leadingMemberWhitespace: String) async throws -> [String] {
var generatedProperties = [String]()
for childGenerator in try orderedPropertiesToGenerate {
for childGenerator in orderedPropertiesToGenerate {
try await generatedProperties.append(
childGenerator
.generateCode(leadingWhitespace: leadingMemberWhitespace)
Expand All @@ -421,17 +406,11 @@ actor ScopeGenerator: CustomStringConvertible, Sendable {

private enum GenerationError: Error, CustomStringConvertible {
case erasedInstantiatorGenericDoesNotMatch(property: Property, instantiable: Instantiable)
case dependencyCycleDetected([Property], scope: ScopeGenerator)

var description: String {
switch self {
case let .erasedInstantiatorGenericDoesNotMatch(property, instantiable):
"Property `\(property.asSource)` on \(instantiable.concreteInstantiable.asSource) incorrectly configured. Property should instead be of type `\(Dependency.erasedInstantiatorType)<\(instantiable.concreteInstantiable.asSource).ForwardedProperties, \(property.typeDescription.asInstantiatedType.asSource)>`."
case let .dependencyCycleDetected(properties, scope):
"""
Dependency cycle detected on \(scope)!
\(properties.map(\.asSource).joined(separator: " -> "))
"""
}
}
}
Expand Down
67 changes: 59 additions & 8 deletions Tests/SafeDIToolTests/SafeDIToolCodeGenerationErrorTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -290,8 +290,8 @@ final class SafeDIToolCodeGenerationErrorTests: XCTestCase {
func test_run_onCodeWithReceivedPropertyThatRefersToCurrentInstantiable_throwsError() async throws {
await assertThrowsError(
"""
Dependency cycle detected!
AuthService -> AuthService
Dependency received in same chain it is instantiated!
@Instantiated authService: AuthService -> @Received authService: AuthService
"""
) {
try await executeSafeDIToolTest(
Expand Down Expand Up @@ -457,8 +457,8 @@ final class SafeDIToolCodeGenerationErrorTests: XCTestCase {
func test_run_onCodeWhereAliasedReceivedPropertyRefersToCurrentInstantiable_throwsError() async throws {
await assertThrowsError(
"""
Dependency cycle detected!
AuthService -> AuthService
Dependency received in same chain it is instantiated!
@Instantiated authService: AuthService -> @Received authService: AuthService
"""
) {
try await executeSafeDIToolTest(
Expand Down Expand Up @@ -900,8 +900,8 @@ final class SafeDIToolCodeGenerationErrorTests: XCTestCase {
func test_run_onCodeWithCircularReceivedDependencies_throwsError() async {
await assertThrowsError(
"""
Dependency cycle detected on Root!
a: A -> b: B -> c: C -> a: A
Dependency received in same chain it is instantiated!
@Instantiated a: A -> @Received b: B -> @Received c: C -> @Received a: A
"""
) {
try await executeSafeDIToolTest(
Expand Down Expand Up @@ -949,8 +949,59 @@ final class SafeDIToolCodeGenerationErrorTests: XCTestCase {
func test_run_onCodeWithCircularReceivedRenamedDependencies_throwsError() async {
await assertThrowsError(
"""
Dependency cycle detected on A!
b: B -> c: C -> renamedB: B -> b: B
Dependency received in same chain it is instantiated!
@Instantiated a: A -> @Received renamedB: B -> @Received c: C -> @Received a: A
"""
) {
try await executeSafeDIToolTest(
swiftFileContent: [
"""
@Instantiable
public struct Root {
@Instantiated
private let a: A
@Instantiated
private let b: B
@Received(fulfilledByDependencyNamed: "b", ofType: B.self)
private let renamedB: B
@Instantiated
private let c: C
}
""",
"""
@Instantiable
public struct A {
@Received
private let renamedB: B
}
""",
"""
@Instantiable
public struct B {
@Received
private let c: C
}
""",
"""
@Instantiable
public struct C {
@Received
private let a: A
}
""",
],
buildDependencyTreeOutput: true,
filesToDelete: &filesToDelete
)
}
}

@MainActor
func test_run_onCodeWithMultipleCircularReceivedRenamedDependencies_throwsError() async {
await assertThrowsError(
"""
Dependency received in same chain it is instantiated!
@Instantiated c: C -> @Received renamedB: B -> @Received c: C
"""
) {
try await executeSafeDIToolTest(
Expand Down

0 comments on commit c6da5d6

Please sign in to comment.