From a7ba52cfe147a2cb80f4146f699de259d9466986 Mon Sep 17 00:00:00 2001 From: Dan Federman Date: Sat, 9 Dec 2023 06:54:03 -0800 Subject: [PATCH] At most one @Forwarded property per @Instantiable --- .../ForwardingInstantiator.swift | 14 +- .../SafeDICore/Models/ScopeGenerator.swift | 23 +- .../Macros/InstantiableMacro.swift | 9 +- .../InstantiableMacroTests.swift | 278 +++++++++++------- 4 files changed, 188 insertions(+), 136 deletions(-) diff --git a/Sources/SafeDI/DelayedInstantiation/ForwardingInstantiator.swift b/Sources/SafeDI/DelayedInstantiation/ForwardingInstantiator.swift index e480b0b4..02df40b1 100644 --- a/Sources/SafeDI/DelayedInstantiation/ForwardingInstantiator.swift +++ b/Sources/SafeDI/DelayedInstantiation/ForwardingInstantiator.swift @@ -25,13 +25,13 @@ /// Instantiation is thread-safe. /// /// - SeeAlso: `Instantiator` -/// - Note: This class is the sole means for instantiating an `@Instantiable` type with `@Forwarded` -/// properties within the SafeDI framework. -public final class ForwardingInstantiator { +/// - Note: This class is the sole means for instantiating an `@Instantiable` type with a `@Forwarded` +/// property within the SafeDI framework. +public final class ForwardingInstantiator { /// Initializes a new forwarding instantiator with the provided instantiation closure. /// /// - Parameter instantiator: A closure that takes `ArgumentsToForward` and returns an instance of `InstantiableType`. - public init(_ instantiator: @escaping (ArgumentsToForward) -> InstantiableType) { + public init(_ instantiator: @escaping (ArgumentToForward) -> InstantiableType) { self.instantiator = instantiator } @@ -39,9 +39,9 @@ public final class ForwardingInstantiator /// /// - Parameter arguments: Arguments required for instantiation. /// - Returns: An `InstantiableType` instance. - public func instantiate(_ arguments: ArgumentsToForward) -> InstantiableType { - instantiator(arguments) + public func instantiate(_ argument: ArgumentToForward) -> InstantiableType { + instantiator(argument) } - private let instantiator: (ArgumentsToForward) -> InstantiableType + private let instantiator: (ArgumentToForward) -> InstantiableType } diff --git a/Sources/SafeDICore/Models/ScopeGenerator.swift b/Sources/SafeDICore/Models/ScopeGenerator.swift index 75815614..4da6732c 100644 --- a/Sources/SafeDICore/Models/ScopeGenerator.swift +++ b/Sources/SafeDICore/Models/ScopeGenerator.swift @@ -34,11 +34,13 @@ actor ScopeGenerator { self.propertiesToGenerate = propertiesToGenerate self.receivedProperties = receivedProperties - forwardedProperties = instantiable + forwardedProperty = instantiable .dependencies // Instantiated properties will self-resolve. .filter { $0.source == .forwarded } .map(\.property) + // Our @Instantiable macro enforces that we have at most one forwarded property. + .first propertiesMadeAvailableByChildren = Set( instantiable @@ -78,20 +80,12 @@ actor ScopeGenerator { let isConstant: Bool let propertyDeclaration: String let leadingConcreteTypeName: String - let closureArguments: String + let closureArguments = if let forwardedProperty { " \(forwardedProperty.label) in" } else { "" } switch property.propertyType { - case .forwardingInstantiator: + case .instantiator, .lazy, .forwardingInstantiator: isConstant = false propertyDeclaration = "let \(property.label)" leadingConcreteTypeName = property.typeDescription.asSource - // TODO: Would be better to match types rather than assuming property order for the forwarded properties. - // TODO: Throw error if forwardedProperties has multiple of the same type. - closureArguments = " \(forwardedProperties.map(\.label).joined(separator: ", ")) in" - case .instantiator, .lazy: - isConstant = false - propertyDeclaration = "let \(property.label)" - leadingConcreteTypeName = property.typeDescription.asSource - closureArguments = "" case .constant: isConstant = true if concreteTypeName == property.typeDescription.asSource { @@ -100,7 +94,6 @@ actor ScopeGenerator { propertyDeclaration = "let \(property.label): \(property.typeDescription.asSource)" } leadingConcreteTypeName = "" - closureArguments = "" } let leadingMemberWhitespace = " " @@ -153,7 +146,7 @@ actor ScopeGenerator { private let property: Property? private let receivedProperties: Set private let propertiesToGenerate: [ScopeGenerator] - private let forwardedProperties: [Property] + private let forwardedProperty: Property? private let requiredReceivedProperties: Set private let propertiesMadeAvailableByChildren: Set @@ -203,13 +196,13 @@ actor ScopeGenerator { .requiredReceivedProperties .contains(where: { !isPropertyResolved($0) - && !propertyToGenerate.forwardedProperties.contains($0) + && propertyToGenerate.forwardedProperty != $0 }) } private func isPropertyResolved(_ property: Property) -> Bool { resolvedProperties.contains(property) || receivedProperties.contains(property) - || forwardedProperties.contains(property) + || forwardedProperty == property } } diff --git a/Sources/SafeDIMacros/Macros/InstantiableMacro.swift b/Sources/SafeDIMacros/Macros/InstantiableMacro.swift index f2c64b4b..2e26d0bc 100644 --- a/Sources/SafeDIMacros/Macros/InstantiableMacro.swift +++ b/Sources/SafeDIMacros/Macros/InstantiableMacro.swift @@ -45,7 +45,9 @@ public struct InstantiableMacro: MemberMacro { context.diagnose(diagnostic) } - // TODO: Do not allow having multiple @Forwarded properties of the same type. + guard visitor.dependencies.filter({ $0.source == .forwarded }).count <= 1 else { + throw InstantiableError.tooManyForwardedProperties + } let hasMemberwiseInitializerForInjectableProperties = visitor .initializers @@ -80,11 +82,14 @@ public struct InstantiableMacro: MemberMacro { private enum InstantiableError: Error, CustomStringConvertible { case decoratingIncompatibleType + case tooManyForwardedProperties var description: String { switch self { case .decoratingIncompatibleType: - return "@\(InstantiableVisitor.macroName) must decorate a class, struct, or actor" + "@\(InstantiableVisitor.macroName) must decorate a class, struct, or actor" + case .tooManyForwardedProperties: + "An @\(InstantiableVisitor.macroName) type must have at most one @\(Dependency.Source.forwarded.rawValue) property" } } } diff --git a/Tests/SafeDIMacrosTests/InstantiableMacroTests.swift b/Tests/SafeDIMacrosTests/InstantiableMacroTests.swift index f5fe0e8a..9d818eee 100644 --- a/Tests/SafeDIMacrosTests/InstantiableMacroTests.swift +++ b/Tests/SafeDIMacrosTests/InstantiableMacroTests.swift @@ -50,14 +50,68 @@ final class InstantiableMacroTests: XCTestCase { assertMacro { """ @Instantiable - protocol ExampleService {} + public protocol ExampleService {} """ } diagnostics: { """ @Instantiable ┬──────────── ╰─ 🛑 @Instantiable must decorate a class, struct, or actor - protocol ExampleService {} + public protocol ExampleService {} + """ + } + } + + func test_throwsErrorWhenOnEnum() { + assertMacro { + """ + @Instantiable + public enum ExampleService {} + """ + } diagnostics: { + """ + @Instantiable + ┬──────────── + ╰─ 🛑 @Instantiable must decorate a class, struct, or actor + public enum ExampleService {} + """ + } + } + + func test_throwsErrorWhenMoreThanOneForwardedProperty() { + assertMacro { + """ + @Instantiable + public final class UserService { + public init(userID: String, userName: String) { + self.userID = userID + self.userName = userName + } + + @Forwarded + let userID: String + + @Forwarded + let userName: String + } + """ + } diagnostics: { + """ + @Instantiable + ┬──────────── + ╰─ 🛑 An @Instantiable type must have at most one @Forwarded property + public final class UserService { + public init(userID: String, userName: String) { + self.userID = userID + self.userName = userName + } + + @Forwarded + let userID: String + + @Forwarded + let userName: String + } """ } } @@ -69,13 +123,13 @@ final class InstantiableMacroTests: XCTestCase { """ @Instantiable public struct ExampleService { - init(invariantA: InvariantA) { - self.invariantA = invariantA + init(receivedA: ReceivedA) { + self.receivedA = receivedA } @Received @Instantiated - let invariantA: InvariantA + let receivedA: ReceivedA } """ } diagnostics: { @@ -84,15 +138,15 @@ final class InstantiableMacroTests: XCTestCase { public struct ExampleService { ╰─ 🛑 @Instantiable-decorated type must have `public` or `open` initializer comprising all injected parameters ✏️ Add required initializer - init(invariantA: InvariantA) { - self.invariantA = invariantA + init(receivedA: ReceivedA) { + self.receivedA = receivedA } @Received ╰─ 🛑 Dependency can have at most one of @Instantiated, @Received, or @Forwarded attached macro ✏️ Remove excessive attached macros @Instantiated - let invariantA: InvariantA + let receivedA: ReceivedA } """ } fixes: { @@ -101,13 +155,13 @@ final class InstantiableMacroTests: XCTestCase { public struct ExampleService { public init() {} - init(invariantA: InvariantA) { - self.invariantA = invariantA + init(receivedA: ReceivedA) { + self.receivedA = receivedA } @Received @Instantiated - let invariantA: InvariantA + let receivedA: ReceivedA } """ } @@ -118,47 +172,47 @@ final class InstantiableMacroTests: XCTestCase { """ @Instantiable public struct ExampleService { - public init(invariantA: InvariantA) { - self.invariantA = invariantA + public init(receivedA: ReceivedA) { + self.receivedA = receivedA } @Instantiated - let invariantA: InvariantA = .init() + let receivedA: ReceivedA = .init() } """ } diagnostics: { """ @Instantiable public struct ExampleService { - public init(invariantA: InvariantA) { - self.invariantA = invariantA + public init(receivedA: ReceivedA) { + self.receivedA = receivedA } @Instantiated ╰─ 🛑 Dependency must not have hand-written initializer ✏️ Remove initializer - let invariantA: InvariantA = .init() + let receivedA: ReceivedA = .init() } """ } fixes: { """ @Instantiable public struct ExampleService { - public init(invariantA: InvariantA) { - self.invariantA = invariantA + public init(receivedA: ReceivedA) { + self.receivedA = receivedA } @Instantiated - let invariantA: InvariantA + let receivedA: ReceivedA } """ } expansion: { """ public struct ExampleService { - public init(invariantA: InvariantA) { - self.invariantA = invariantA + public init(receivedA: ReceivedA) { + self.receivedA = receivedA } - let invariantA: InvariantA + let receivedA: ReceivedA } """ } @@ -169,12 +223,12 @@ final class InstantiableMacroTests: XCTestCase { """ @Instantiable struct ExampleService { - public init(invariantA: InvariantA) { - self.invariantA = invariantA + public init(receivedA: ReceivedA) { + self.receivedA = receivedA } @Instantiated - let invariantA: InvariantA + let receivedA: ReceivedA } """ } diagnostics: { @@ -183,12 +237,12 @@ final class InstantiableMacroTests: XCTestCase { ╰─ 🛑 @Instantiable-decorated type must be `public` or `open` ✏️ Add `public` modifier struct ExampleService { - public init(invariantA: InvariantA) { - self.invariantA = invariantA + public init(receivedA: ReceivedA) { + self.receivedA = receivedA } @Instantiated - let invariantA: InvariantA + let receivedA: ReceivedA } """ } fixes: { @@ -196,12 +250,12 @@ final class InstantiableMacroTests: XCTestCase { @Instantiable public public ExampleService { - public init(invariantA: InvariantA) { - self.invariantA = invariantA + public init(receivedA: ReceivedA) { + self.receivedA = receivedA } @Instantiated - let invariantA: InvariantA + let receivedA: ReceivedA } """ // this fixes are wrong (we aren't deleting 'struct'), but also the whitespace is wrong in Xcode. // TODO: fix Xcode spacing of this replacement. @@ -210,12 +264,12 @@ final class InstantiableMacroTests: XCTestCase { @Instantiable public public ExampleService { - public init(invariantA: InvariantA) { - self.invariantA = invariantA + public init(receivedA: ReceivedA) { + self.receivedA = receivedA } @Instantiated - let invariantA: InvariantA + let receivedA: ReceivedA } """ } @@ -260,7 +314,7 @@ final class InstantiableMacroTests: XCTestCase { @Instantiable public struct ExampleService { @Instantiated - let invariantA: InvariantA + let receivedA: ReceivedA } """ } diagnostics: { @@ -270,28 +324,28 @@ final class InstantiableMacroTests: XCTestCase { ╰─ 🛑 @Instantiable-decorated type must have `public` or `open` initializer comprising all injected parameters ✏️ Add required initializer @Instantiated - let invariantA: InvariantA + let receivedA: ReceivedA } """ } fixes: { """ @Instantiable public struct ExampleService { - public init(invariantA: InvariantA) { - self.invariantA = invariantA + public init(receivedA: ReceivedA) { + self.receivedA = receivedA } @Instantiated - let invariantA: InvariantA + let receivedA: ReceivedA } """ // Whitespace is correct in Xcode, but not here. } expansion: { """ public struct ExampleService { - public init(invariantA: InvariantA) { - self.invariantA = invariantA + public init(receivedA: ReceivedA) { + self.receivedA = receivedA } - let invariantA: InvariantA + let receivedA: ReceivedA } """ } @@ -302,18 +356,18 @@ final class InstantiableMacroTests: XCTestCase { """ @Instantiable public struct ExampleService { - public init(variantA: VariantA, variantB: VariantB) { - self.variantA = variantA - self.variantB = variantB - invariantA = InvariantA() + public init(forwardedA: ForwardedA, forwardedB: ForwardedB) { + self.forwardedA = forwardedA + self.forwardedB = forwardedB + receivedA = ReceivedA() } @Forwarded - let variantA: VariantA - @Forwarded - let variantB: VariantB + let forwardedA: ForwardedA @Received - let invariantA: InvariantA + let forwardedB: ForwardedB + @Received + let receivedA: ReceivedA } """ } diagnostics: { @@ -322,61 +376,61 @@ final class InstantiableMacroTests: XCTestCase { public struct ExampleService { ╰─ 🛑 @Instantiable-decorated type must have `public` or `open` initializer comprising all injected parameters ✏️ Add required initializer - public init(variantA: VariantA, variantB: VariantB) { - self.variantA = variantA - self.variantB = variantB - invariantA = InvariantA() + public init(forwardedA: ForwardedA, forwardedB: ForwardedB) { + self.forwardedA = forwardedA + self.forwardedB = forwardedB + receivedA = ReceivedA() } @Forwarded - let variantA: VariantA - @Forwarded - let variantB: VariantB + let forwardedA: ForwardedA + @Received + let forwardedB: ForwardedB @Received - let invariantA: InvariantA + let receivedA: ReceivedA } """ } fixes: { """ @Instantiable public struct ExampleService { - public init(variantA: VariantA, variantB: VariantB, invariantA: InvariantA) { - self.variantA = variantA - self.variantB = variantB - self.invariantA = invariantA + public init(forwardedA: ForwardedA, forwardedB: ForwardedB, receivedA: ReceivedA) { + self.forwardedA = forwardedA + self.forwardedB = forwardedB + self.receivedA = receivedA } - public init(variantA: VariantA, variantB: VariantB) { - self.variantA = variantA - self.variantB = variantB - invariantA = InvariantA() + public init(forwardedA: ForwardedA, forwardedB: ForwardedB) { + self.forwardedA = forwardedA + self.forwardedB = forwardedB + receivedA = ReceivedA() } @Forwarded - let variantA: VariantA - @Forwarded - let variantB: VariantB + let forwardedA: ForwardedA + @Received + let forwardedB: ForwardedB @Received - let invariantA: InvariantA + let receivedA: ReceivedA } """ } expansion: { """ public struct ExampleService { - public init(variantA: VariantA, variantB: VariantB, invariantA: InvariantA) { - self.variantA = variantA - self.variantB = variantB - self.invariantA = invariantA + public init(forwardedA: ForwardedA, forwardedB: ForwardedB, receivedA: ReceivedA) { + self.forwardedA = forwardedA + self.forwardedB = forwardedB + self.receivedA = receivedA } - public init(variantA: VariantA, variantB: VariantB) { - self.variantA = variantA - self.variantB = variantB - invariantA = InvariantA() + public init(forwardedA: ForwardedA, forwardedB: ForwardedB) { + self.forwardedA = forwardedA + self.forwardedB = forwardedB + receivedA = ReceivedA() } - let variantA: VariantA - let variantB: VariantB - let invariantA: InvariantA + let forwardedA: ForwardedA + let forwardedB: ForwardedB + let receivedA: ReceivedA } """ } @@ -388,7 +442,7 @@ final class InstantiableMacroTests: XCTestCase { @Instantiable public struct ExampleService { @Instantiated - private let invariantAInstantiator: Instantiator + private let instantiatableAInstantiator: Instantiator } """ } diagnostics: { @@ -398,28 +452,28 @@ final class InstantiableMacroTests: XCTestCase { ╰─ 🛑 @Instantiable-decorated type must have `public` or `open` initializer comprising all injected parameters ✏️ Add required initializer @Instantiated - private let invariantAInstantiator: Instantiator + private let instantiatableAInstantiator: Instantiator } """ } fixes: { """ @Instantiable public struct ExampleService { - public init(invariantAInstantiator: Instantiator) { - self.invariantAInstantiator = invariantAInstantiator + public init(instantiatableAInstantiator: Instantiator) { + self.instantiatableAInstantiator = instantiatableAInstantiator } @Instantiated - private let invariantAInstantiator: Instantiator + private let instantiatableAInstantiator: Instantiator } """ } expansion: { """ public struct ExampleService { - public init(invariantAInstantiator: Instantiator) { - self.invariantAInstantiator = invariantAInstantiator + public init(instantiatableAInstantiator: Instantiator) { + self.instantiatableAInstantiator = instantiatableAInstantiator } - private let invariantAInstantiator: Instantiator + private let instantiatableAInstantiator: Instantiator } """ } @@ -432,7 +486,7 @@ final class InstantiableMacroTests: XCTestCase { @Instantiable public struct ExampleService { @LazyInstantiated - private var invariantA: InvariantA + private var instantiatableA: ReceivedA } """ } diagnostics: { @@ -442,30 +496,30 @@ final class InstantiableMacroTests: XCTestCase { ╰─ 🛑 @Instantiable-decorated type must have `public` or `open` initializer comprising all injected parameters ✏️ Add required initializer @LazyInstantiated - private var invariantA: InvariantA + private var instantiatableA: ReceivedA } """ } fixes: { """ @Instantiable public struct ExampleService { - public init(invariantAInstantiator: Instantiator) { - _invariantA = LazyInstantiated(invariantAInstantiator) + public init(instantiatableAInstantiator: Instantiator) { + _instantiatableA = LazyInstantiated(instantiatableAInstantiator) } @LazyInstantiated - private var invariantA: InvariantA + private var instantiatableA: ReceivedA } """ } expansion: { """ public struct ExampleService { - public init(invariantAInstantiator: Instantiator) { - _invariantA = LazyInstantiated(invariantAInstantiator) + public init(instantiatableAInstantiator: Instantiator) { + _instantiatableA = LazyInstantiated(instantiatableAInstantiator) } @LazyInstantiated - private var invariantA: InvariantA + private var instantiatableA: ReceivedA } """ } @@ -477,9 +531,9 @@ final class InstantiableMacroTests: XCTestCase { @Instantiable public struct ExampleService { @LazyInstantiated - private var invariantA: InvariantA + private var instantiatableA: InstantiatableA @Instantiated - let invariantAInstantiator: Instantiator + let instantiatableAInstantiator: Instantiator } """ } diagnostics: { @@ -489,37 +543,37 @@ final class InstantiableMacroTests: XCTestCase { ╰─ 🛑 @Instantiable-decorated type must have `public` or `open` initializer comprising all injected parameters ✏️ Add required initializer @LazyInstantiated - private var invariantA: InvariantA + private var instantiatableA: InstantiatableA @Instantiated - let invariantAInstantiator: Instantiator + let instantiatableAInstantiator: Instantiator } """ } fixes: { """ @Instantiable public struct ExampleService { - public init(invariantAInstantiator: Instantiator) { - _invariantA = LazyInstantiated(invariantAInstantiator) - self.invariantAInstantiator = invariantAInstantiator + public init(instantiatableAInstantiator: Instantiator) { + _instantiatableA = LazyInstantiated(instantiatableAInstantiator) + self.instantiatableAInstantiator = instantiatableAInstantiator } @LazyInstantiated - private var invariantA: InvariantA + private var instantiatableA: InstantiatableA @Instantiated - let invariantAInstantiator: Instantiator + let instantiatableAInstantiator: Instantiator } """ } expansion: { """ public struct ExampleService { - public init(invariantAInstantiator: Instantiator) { - _invariantA = LazyInstantiated(invariantAInstantiator) - self.invariantAInstantiator = invariantAInstantiator + public init(instantiatableAInstantiator: Instantiator) { + _instantiatableA = LazyInstantiated(instantiatableAInstantiator) + self.instantiatableAInstantiator = instantiatableAInstantiator } @LazyInstantiated - private var invariantA: InvariantA - let invariantAInstantiator: Instantiator + private var instantiatableA: InstantiatableA + let instantiatableAInstantiator: Instantiator } """ }