Skip to content

Commit

Permalink
Delete @singleton macro. The value is too murky given the complexity …
Browse files Browse the repository at this point in the history
…of the system
  • Loading branch information
dfed committed Dec 6, 2023
1 parent 76b51ed commit 72ff199
Show file tree
Hide file tree
Showing 11 changed files with 20 additions and 100 deletions.
7 changes: 2 additions & 5 deletions Sources/SafeDI/ExternalMacros.swift
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@

#else

/// Marks a `class`, `struct`, or `actor` as capable of having properties that conform to this type decorated with `@Instantiated` or `@Singleton`.
/// Marks a `class`, `struct`, or `actor` as capable of having properties that conform to this type decorated with `@Instantiated`
///
/// - Parameter fulfillingAdditionalTypes: The types (in addition to the type decorated with this macro) that can be decorated with `@Instantiated` or `@Singleton` and yield a result of this type. The types provided *must* be either superclasses of this type or protocols to which this type conforms.
/// - Parameter fulfillingAdditionalTypes: The types (in addition to the type decorated with this macro) that can be decorated with `@Instantiated` and yield a result of this type. The types provided *must* be either superclasses of this type or protocols to which this type conforms.
@attached(member, names: arbitrary) public macro Instantiable(fulfillingAdditionalTypes: [Any.Type] = []) = #externalMacro(module: "SafeDIMacros", type: "InstantiableMacro")

/// Marks a SafeDI dependency that is instantiated when its parent object is instantiated.
Expand All @@ -34,9 +34,6 @@
/// Marks a SafeDI dependency that is instantiated by an object higher up in the dependency tree.
@attached(peer) public macro Inherited() = #externalMacro(module: "SafeDIMacros", type: "InjectableMacro")

/// Marks a SafeDI dependency that will only ever have one instance instantiated at a given time. Singleton dependencies may deallocate when all of the objects that use it deallocate. Singleton dependencies can not be marked with @Instantiated.
@attached(peer) public macro Singleton() = #externalMacro(module: "SafeDIMacros", type: "InjectableMacro")

/// Marks a SafeDI dependency that is injected into the parent object's initializer and forwarded to objects further down in the dependency tree.
@attached(peer) public macro Forwarded() = #externalMacro(module: "SafeDIMacros", type: "InjectableMacro")

Expand Down
7 changes: 2 additions & 5 deletions Sources/SafeDICore/Errors/DependencyTreeGeneratorError.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,14 @@ enum DependencyTreeGeneratorError: Error, CustomStringConvertible {

case noInstantiableFound(TypeDescription)
case noRootInstantiableFound
case unsatisfiableSingletons([Property], roots: [TypeDescription])
case unfulfillableProperties([UnfulfillableProperty])

var description: String {
switch self {
case let .noInstantiableFound(typeDescription):
"No `@\(InstantiableVisitor.macroName)`-decorated type found to fulfill `@\(Dependency.Source.instantiated.rawValue)`, `@\(Dependency.Source.lazyInstantiated.rawValue)`, or `@\(Dependency.Source.singleton.rawValue)`-decorated property with type '\(typeDescription.asSource)'"
"No `@\(InstantiableVisitor.macroName)`-decorated type found to fulfill `@\(Dependency.Source.instantiated.rawValue)` or `@\(Dependency.Source.lazyInstantiated.rawValue)`-decorated property with type '\(typeDescription.asSource)'"
case .noRootInstantiableFound:
"All `@\(InstantiableVisitor.macroName)`-decorated types were found on a @\(Dependency.Source.instantiated.rawValue)`, `@\(Dependency.Source.lazyInstantiated.rawValue)`, or `@\(Dependency.Source.singleton.rawValue)`-decorated property. There must be at least one `@\(InstantiableVisitor.macroName)`-decorated types that is not instantiated by another `@\(InstantiableVisitor.macroName)`-decorated type"
case let .unsatisfiableSingletons(properties, roots):
"All `@\(Dependency.Source.singleton.rawValue)`-decorated properties must be in the same dependency tree to ensure they are singletons. Found multiple root `@\(InstantiableVisitor.macroName)`-decorated types: \(roots.map(\.asSource).joined(separator: ", ")). The following `@\(Dependency.Source.singleton.rawValue)`-decorated properties were found across multiple roots: \(properties.map(\.asSource).joined(separator: ", "))."
"All `@\(InstantiableVisitor.macroName)`-decorated types were found on a @\(Dependency.Source.instantiated.rawValue)` or `@\(Dependency.Source.lazyInstantiated.rawValue)`-decorated property. There must be at least one `@\(InstantiableVisitor.macroName)`-decorated types that is not instantiated by another `@\(InstantiableVisitor.macroName)`-decorated type"
case let .unfulfillableProperties(unfulfillableProperties):
"""
The following inherited properties were never instantiated:
Expand Down
2 changes: 1 addition & 1 deletion Sources/SafeDICore/Errors/FixableInstantiableError.swift
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public enum FixableInstantiableError: DiagnosticError {
public var description: String {
switch self {
case .dependencyHasTooManyAttributes:
return "Dependency can have at most one of @\(Dependency.Source.instantiated), @\(Dependency.Source.inherited), @\(Dependency.Source.singleton), or @\(Dependency.Source.forwarded) attached macro"
return "Dependency can have at most one of @\(Dependency.Source.instantiated), @\(Dependency.Source.inherited), or @\(Dependency.Source.forwarded) attached macro"
case .dependencyHasInitializer:
return "Dependency must not have hand-written initializer"
case .missingPublicOrOpenAttribute:
Expand Down
65 changes: 1 addition & 64 deletions Sources/SafeDICore/Generators/DependencyTreeGenerator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,8 @@ public final class DependencyTreeGenerator {
public func generate() async throws -> String {
try validateReachableTypeDescriptions()
try validateAtLeastOneRootFound()
// showstopper TODO: Validate that all @Singleton properties are never @Constructed (or @Inherited??)

let typeDescriptionToScopeMap = try createTypeDescriptionToScopeMapping()
try assignSingletonsToScopes(typeDescriptionToScopeMap: typeDescriptionToScopeMap)
try propagateUndeclaredInheritedProperties(typeDescriptionToScopeMap: typeDescriptionToScopeMap)
let rootScopes = rootInstantiableTypes.compactMap({ typeDescriptionToScopeMap[$0] })
_ = rootScopes
Expand Down Expand Up @@ -69,7 +67,6 @@ public final class DependencyTreeGenerator {

/// A collection of `@Instantiable`-decorated types that do not explicitly inherit dependencies.
/// - Note: These are not necessarily roots in the build graph, since these types may be instantiated by another `@Instantiable`.
/// These types may also inherit (rather than construct) a `@Singleton` dependency.
private lazy var possibleRootInstantiableTypes: Set<TypeDescription> = Set(
typeDescriptionToFulfillingInstantiableMap
.values
Expand Down Expand Up @@ -180,69 +177,9 @@ public final class DependencyTreeGenerator {
}
scope.propertiesToInstantiate.append(contentsOf: additionalPropertiesToInstantiate)
}
// Note: Singletons have not been assigned to scopes yet!
return typeDescriptionToScopeMap
}

private func assignSingletonsToScopes(typeDescriptionToScopeMap: [TypeDescription: Scope]) throws {
/// A mapping of singleton properties to the scopes that require this property.
var singletonPropertyToScopesCountMap: [Property: Int] = typeDescriptionToScopeMap
.values
.reduce(into: [Property: Int]()) { partialResult, scope in
for property in scope.instantiable.singletonProperties {
partialResult[property, default: 0] += 1
}
}
var scopeIdentifierToSingletonPropertyToScopesInTreeCount = [ObjectIdentifier: [Property: Int]]()
func recordSingletonPropertiesOnScope(_ scope: Scope, parentScopes: [Scope]) {
let scopes = [scope] + parentScopes // parentScopes has root scope as last.
for singletonProperty in scope.instantiable.singletonProperties {
for (index, scope) in scopes.enumerated() {
let scopesInTreeUtilizingSingletonProperty = (scopeIdentifierToSingletonPropertyToScopesInTreeCount[ObjectIdentifier(scope)]?[singletonProperty] ?? 0) + 1
defer {
scopeIdentifierToSingletonPropertyToScopesInTreeCount[ObjectIdentifier(scope), default: [Property: Int]()][singletonProperty] = scopesInTreeUtilizingSingletonProperty
}
if
singletonPropertyToScopesCountMap[singletonProperty] == scopesInTreeUtilizingSingletonProperty,
let singletonPropertyScope = typeDescriptionToScopeMap[singletonProperty.typeDescription.asInstantiatedType]
{
scope.propertiesToInstantiate.append(Scope.PropertyToInstantiate(
property: singletonProperty,
instantiable: singletonPropertyScope.instantiable,
scope: singletonPropertyScope,
type: singletonProperty.nonLazyPropertyType // Singletons can not be lazy
))
// Remove the singleton property from our tracker so we can find orphaned singletons later.
singletonPropertyToScopesCountMap[singletonProperty] = nil
// Visit the scope we just placed.
recordSingletonPropertiesOnScope(singletonPropertyScope, parentScopes: Array(scopes.dropFirst(index)))
// We don't need to keep traversing up the tree because we found what we're looking for.
break
}
}
}
for childScope in scope.propertiesToInstantiate.map(\.scope) {
guard !parentScopes.contains(where: { $0 === childScope }) else {
// We've previously visited this child scope.
// There is a cycle in our scope tree. Do not re-enter it.
continue
}
recordSingletonPropertiesOnScope(childScope, parentScopes: scopes)
}
}

let rootScopes = rootInstantiableTypes.compactMap({ typeDescriptionToScopeMap[$0] })
for rootScope in rootScopes {
recordSingletonPropertiesOnScope(rootScope, parentScopes: [])
}
if !singletonPropertyToScopesCountMap.isEmpty {
throw DependencyTreeGeneratorError.unsatisfiableSingletons(
Array(singletonPropertyToScopesCountMap.keys),
roots: rootScopes.map(\.instantiable.concreteInstantiableType)
)
}
}

private func propagateUndeclaredInheritedProperties(typeDescriptionToScopeMap: [TypeDescription: Scope]) throws {
var unfulfillableProperties = [DependencyTreeGeneratorError.UnfulfillableProperty]()
func propagateUndeclaredInheritedProperties(on scope: Scope, parentScopes: [Scope]) {
Expand Down Expand Up @@ -320,7 +257,7 @@ public final class DependencyTreeGenerator {
extension Dependency {
fileprivate var isInstantiated: Bool {
switch source {
case .instantiated, .lazyInstantiated, .singleton:
case .instantiated, .lazyInstantiated:
return true
case .forwarded, .inherited:
return false
Expand Down
9 changes: 4 additions & 5 deletions Sources/SafeDICore/Models/Dependency.swift
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
import SwiftSyntax

/// A representation of a dependency.
/// e.g. `@Singleton let mySingleton: MySingleton`
/// e.g. `@Instantiated let myService: MyService`
public struct Dependency: Codable, Hashable {

// MARK: Public
Expand All @@ -31,7 +31,7 @@ public struct Dependency: Codable, Hashable {

public var isForwarded: Bool {
switch source {
case .instantiated, .lazyInstantiated, .inherited, .singleton:
case .instantiated, .lazyInstantiated, .inherited:
return false
case .forwarded:
return true
Expand All @@ -42,7 +42,6 @@ public struct Dependency: Codable, Hashable {
case instantiated = "Instantiated"
case lazyInstantiated = "LazyInstantiated"
case inherited = "Inherited"
case singleton = "Singleton"
case forwarded = "Forwarded"

public var description: String {
Expand All @@ -62,7 +61,7 @@ public struct Dependency: Codable, Hashable {
/// The label by which this property is referenced in an initializer.
var initializerArgumentLabel: String {
switch source {
case .instantiated, .inherited, .singleton, .forwarded:
case .instantiated, .inherited, .forwarded:
return property.label
case .lazyInstantiated:
return "\(property.label)\(Self.instantiatorType)"
Expand All @@ -72,7 +71,7 @@ public struct Dependency: Codable, Hashable {
/// The type description by which this property is referenced in an initializer.
var initializerArgumentTypeDescription: TypeDescription {
switch source {
case .instantiated, .inherited, .singleton, .forwarded:
case .instantiated, .inherited, .forwarded:
return property.typeDescription
case .lazyInstantiated:
// TODO: fully qualify this type with `SafeDI.` member prefix
Expand Down
1 change: 0 additions & 1 deletion Sources/SafeDICore/Models/Initializer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,6 @@ public struct Initializer: Codable, Equatable {
switch dependency.source {
case .instantiated,
.inherited,
.singleton,
.forwarded:
CodeBlockItemSyntax(
item: .expr(ExprSyntax(InfixOperatorExprSyntax(
Expand Down
5 changes: 0 additions & 5 deletions Sources/SafeDICore/Models/Instantiable.swift
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,6 @@ public struct Instantiable: Codable, Hashable {
.filter { $0.source == .lazyInstantiated }
.map(\.property)
}
var singletonProperties: [Property] {
dependencies
.filter { $0.source == .singleton }
.map(\.property)
}
var forwardedProperties: [Property] {
dependencies
.filter { $0.source == .forwarded }
Expand Down
12 changes: 5 additions & 7 deletions Sources/SafeDICore/Models/Scope.swift
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,9 @@ final class Scope {
}
}

var instantiatedProperties = Set<Property>()
var lazyInstantiatedProperties = Set<Property>()
lazy var inheritedProperties = calculateInheritedProperties()
private(set) var instantiatedProperties = Set<Property>()
private(set) var lazyInstantiatedProperties = Set<Property>()
private(set) lazy var inheritedProperties = calculateInheritedProperties()

var allInheritedProperties: Set<Property> {
inheritedProperties.union(undeclaredInheritedProperties)
Expand All @@ -93,13 +93,11 @@ final class Scope {
.instantiated,
.lazyInstantiated:
return false
case .inherited,
// We will subtract out singletons created on this scope next.
.singleton:
case .inherited:
return true
}
}
.map(\.property)
).subtracting(Set(propertiesToInstantiate.map(\.property)))
)
}
}
2 changes: 1 addition & 1 deletion Tests/SafeDICoreTests/InitializerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -544,7 +544,7 @@ final class InitializerTests: XCTestCase {
label: "invariantC",
typeDescription: .simple(name: "InvariantC")
),
source: .singleton
source: .instantiated
)
],
typeIsClass: false,
Expand Down
1 change: 0 additions & 1 deletion Tests/SafeDIMacrosTests/InjectableMacroTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ final class InjectableMacroTests: XCTestCase {
let testMacros: [String: Macro.Type] = [
Dependency.Source.instantiated.rawValue: InjectableMacro.self,
Dependency.Source.inherited.rawValue: InjectableMacro.self,
Dependency.Source.singleton.rawValue: InjectableMacro.self,
Dependency.Source.forwarded.rawValue: InjectableMacro.self,
]

Expand Down
9 changes: 4 additions & 5 deletions Tests/SafeDIMacrosTests/InstantiableMacroTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ final class InstantiableMacroTests: XCTestCase {
InstantiableVisitor.macroName: InstantiableMacro.self,
Dependency.Source.instantiated.rawValue: InjectableMacro.self,
Dependency.Source.inherited.rawValue: InjectableMacro.self,
Dependency.Source.singleton.rawValue: InjectableMacro.self,
Dependency.Source.forwarded.rawValue: InjectableMacro.self,
]

Expand Down Expand Up @@ -175,7 +174,7 @@ final class InstantiableMacroTests: XCTestCase {
public let invariantA: InvariantA
@Inherited
let invariantB: InvariantB
@Singleton
@Instantiated
private let invariantC: InvariantC
}
""",
Expand Down Expand Up @@ -292,7 +291,7 @@ final class InstantiableMacroTests: XCTestCase {
private let invariantA: invariantA
@Inherited
private let invariantB: InvariantB
@Singleton
@Instantiated
private let invariantC: InvariantC
}
""",
Expand Down Expand Up @@ -417,7 +416,7 @@ final class InstantiableMacroTests: XCTestCase {
let variantB: VariantB
@Instantiated
private let invariantA: InvariantA
@Singleton
@Instantiated
public let invariantB: InvariantB
}
""",
Expand Down Expand Up @@ -487,7 +486,7 @@ final class InstantiableMacroTests: XCTestCase {
}
@Inherited
╰─ 🛑 Dependency can have at most one of @Instantiated, @Inherited, @Singleton, or @Forwarded attached macro
╰─ 🛑 Dependency can have at most one of @Instantiated, @Inherited, or @Forwarded attached macro
✏️ Remove excessive attached macros
@Instantiated
let invariantA: InvariantA
Expand Down

0 comments on commit 72ff199

Please sign in to comment.