From 72ff199d30136c35823380dc3242a578ee189786 Mon Sep 17 00:00:00 2001 From: Dan Federman Date: Tue, 5 Dec 2023 18:00:19 -0800 Subject: [PATCH] Delete @Singleton macro. The value is too murky given the complexity of the system --- Sources/SafeDI/ExternalMacros.swift | 7 +- .../Errors/DependencyTreeGeneratorError.swift | 7 +- .../Errors/FixableInstantiableError.swift | 2 +- .../Generators/DependencyTreeGenerator.swift | 65 +------------------ Sources/SafeDICore/Models/Dependency.swift | 9 ++- Sources/SafeDICore/Models/Initializer.swift | 1 - Sources/SafeDICore/Models/Instantiable.swift | 5 -- Sources/SafeDICore/Models/Scope.swift | 12 ++-- Tests/SafeDICoreTests/InitializerTests.swift | 2 +- .../InjectableMacroTests.swift | 1 - .../InstantiableMacroTests.swift | 9 ++- 11 files changed, 20 insertions(+), 100 deletions(-) diff --git a/Sources/SafeDI/ExternalMacros.swift b/Sources/SafeDI/ExternalMacros.swift index d2a11b9c..63599382 100644 --- a/Sources/SafeDI/ExternalMacros.swift +++ b/Sources/SafeDI/ExternalMacros.swift @@ -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. @@ -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") diff --git a/Sources/SafeDICore/Errors/DependencyTreeGeneratorError.swift b/Sources/SafeDICore/Errors/DependencyTreeGeneratorError.swift index 126a7367..8eb5706f 100644 --- a/Sources/SafeDICore/Errors/DependencyTreeGeneratorError.swift +++ b/Sources/SafeDICore/Errors/DependencyTreeGeneratorError.swift @@ -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: diff --git a/Sources/SafeDICore/Errors/FixableInstantiableError.swift b/Sources/SafeDICore/Errors/FixableInstantiableError.swift index 016f09a6..f519b5e8 100644 --- a/Sources/SafeDICore/Errors/FixableInstantiableError.swift +++ b/Sources/SafeDICore/Errors/FixableInstantiableError.swift @@ -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: diff --git a/Sources/SafeDICore/Generators/DependencyTreeGenerator.swift b/Sources/SafeDICore/Generators/DependencyTreeGenerator.swift index 5d503a7b..a45f0b19 100644 --- a/Sources/SafeDICore/Generators/DependencyTreeGenerator.swift +++ b/Sources/SafeDICore/Generators/DependencyTreeGenerator.swift @@ -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 @@ -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 = Set( typeDescriptionToFulfillingInstantiableMap .values @@ -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]) { @@ -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 diff --git a/Sources/SafeDICore/Models/Dependency.swift b/Sources/SafeDICore/Models/Dependency.swift index 3af87401..d68c52ca 100644 --- a/Sources/SafeDICore/Models/Dependency.swift +++ b/Sources/SafeDICore/Models/Dependency.swift @@ -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 @@ -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 @@ -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 { @@ -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)" @@ -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 diff --git a/Sources/SafeDICore/Models/Initializer.swift b/Sources/SafeDICore/Models/Initializer.swift index a3102443..851b9c19 100644 --- a/Sources/SafeDICore/Models/Initializer.swift +++ b/Sources/SafeDICore/Models/Initializer.swift @@ -199,7 +199,6 @@ public struct Initializer: Codable, Equatable { switch dependency.source { case .instantiated, .inherited, - .singleton, .forwarded: CodeBlockItemSyntax( item: .expr(ExprSyntax(InfixOperatorExprSyntax( diff --git a/Sources/SafeDICore/Models/Instantiable.swift b/Sources/SafeDICore/Models/Instantiable.swift index 0bf7e5a5..746a6870 100644 --- a/Sources/SafeDICore/Models/Instantiable.swift +++ b/Sources/SafeDICore/Models/Instantiable.swift @@ -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 } diff --git a/Sources/SafeDICore/Models/Scope.swift b/Sources/SafeDICore/Models/Scope.swift index 3b63a52e..60b20725 100644 --- a/Sources/SafeDICore/Models/Scope.swift +++ b/Sources/SafeDICore/Models/Scope.swift @@ -73,9 +73,9 @@ final class Scope { } } - var instantiatedProperties = Set() - var lazyInstantiatedProperties = Set() - lazy var inheritedProperties = calculateInheritedProperties() + private(set) var instantiatedProperties = Set() + private(set) var lazyInstantiatedProperties = Set() + private(set) lazy var inheritedProperties = calculateInheritedProperties() var allInheritedProperties: Set { inheritedProperties.union(undeclaredInheritedProperties) @@ -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))) + ) } } diff --git a/Tests/SafeDICoreTests/InitializerTests.swift b/Tests/SafeDICoreTests/InitializerTests.swift index 02128c12..cbf9b965 100644 --- a/Tests/SafeDICoreTests/InitializerTests.swift +++ b/Tests/SafeDICoreTests/InitializerTests.swift @@ -544,7 +544,7 @@ final class InitializerTests: XCTestCase { label: "invariantC", typeDescription: .simple(name: "InvariantC") ), - source: .singleton + source: .instantiated ) ], typeIsClass: false, diff --git a/Tests/SafeDIMacrosTests/InjectableMacroTests.swift b/Tests/SafeDIMacrosTests/InjectableMacroTests.swift index eadcc5ec..1337cb0b 100644 --- a/Tests/SafeDIMacrosTests/InjectableMacroTests.swift +++ b/Tests/SafeDIMacrosTests/InjectableMacroTests.swift @@ -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, ] diff --git a/Tests/SafeDIMacrosTests/InstantiableMacroTests.swift b/Tests/SafeDIMacrosTests/InstantiableMacroTests.swift index 80037fa3..ee84b383 100644 --- a/Tests/SafeDIMacrosTests/InstantiableMacroTests.swift +++ b/Tests/SafeDIMacrosTests/InstantiableMacroTests.swift @@ -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, ] @@ -175,7 +174,7 @@ final class InstantiableMacroTests: XCTestCase { public let invariantA: InvariantA @Inherited let invariantB: InvariantB - @Singleton + @Instantiated private let invariantC: InvariantC } """, @@ -292,7 +291,7 @@ final class InstantiableMacroTests: XCTestCase { private let invariantA: invariantA @Inherited private let invariantB: InvariantB - @Singleton + @Instantiated private let invariantC: InvariantC } """, @@ -417,7 +416,7 @@ final class InstantiableMacroTests: XCTestCase { let variantB: VariantB @Instantiated private let invariantA: InvariantA - @Singleton + @Instantiated public let invariantB: InvariantB } """, @@ -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