From 4829408bebb303d23439a8d49b073e7b0a3f826a Mon Sep 17 00:00:00 2001 From: Jack Rosen Date: Tue, 16 Jan 2024 11:42:32 -0500 Subject: [PATCH] PR Feedback --- Sources/WhoopDIKit/Injectable.swift | 2 + Sources/WhoopDIKit/Macros.swift | 4 + .../WhoopDIKitMacros/InjectableMacro.swift | 114 +----------------- .../Support/AccessControlType.swift | 16 +++ .../VariableDeclSyntax+Injectable.swift | 70 +++++++++++ .../Support/VariableDeclaration.swift | 50 ++++++++ 6 files changed, 143 insertions(+), 113 deletions(-) create mode 100644 Sources/WhoopDIKitMacros/Support/AccessControlType.swift create mode 100644 Sources/WhoopDIKitMacros/Support/VariableDeclSyntax+Injectable.swift create mode 100644 Sources/WhoopDIKitMacros/Support/VariableDeclaration.swift diff --git a/Sources/WhoopDIKit/Injectable.swift b/Sources/WhoopDIKit/Injectable.swift index 90f9881..7aba078 100644 --- a/Sources/WhoopDIKit/Injectable.swift +++ b/Sources/WhoopDIKit/Injectable.swift @@ -1,5 +1,7 @@ import Foundation +/// This protocol is used to create a detached injectable component without needing a dependency module. +/// This is most likely used with the `@Injectable` macro, which will create the inject function and define it for you public protocol Injectable { static func inject() throws -> Self } diff --git a/Sources/WhoopDIKit/Macros.swift b/Sources/WhoopDIKit/Macros.swift index e233099..9e63ab0 100644 --- a/Sources/WhoopDIKit/Macros.swift +++ b/Sources/WhoopDIKit/Macros.swift @@ -1,8 +1,12 @@ import Foundation +// These are the definition of the two macros, as explained here https://docs.swift.org/swift-book/documentation/the-swift-programming-language/macros/#Macro-Declarations + +/// The `@Injectable` macro is used to conform to `Injectable` and add a memberwise init and static default method @attached(extension, conformances: Injectable) @attached(member, names: named(inject), named(init)) public macro Injectable() = #externalMacro(module: "WhoopDIKitMacros", type: "InjectableMacro") +/// The `@InjectableName` macro is used as a marker for the `@Injectable` protocol to add a `WhoopDI.inject(name)` in the inject method @attached(peer) public macro InjectableName(name: String) = #externalMacro(module: "WhoopDIKitMacros", type: "InjectableNameMacro") diff --git a/Sources/WhoopDIKitMacros/InjectableMacro.swift b/Sources/WhoopDIKitMacros/InjectableMacro.swift index c833c00..40d2a9a 100644 --- a/Sources/WhoopDIKitMacros/InjectableMacro.swift +++ b/Sources/WhoopDIKitMacros/InjectableMacro.swift @@ -3,13 +3,6 @@ import SwiftSyntaxBuilder import SwiftSyntaxMacros import SwiftSyntaxMacroExpansion -private struct VariableDeclaration { - let name: String - let type: TypeSyntax - let defaultExpression: ExprSyntax? - let injectedName: String? -} - struct InjectableMacro: ExtensionMacro, MemberMacro { /// Adds the `inject` and `init` function that we use for the `Injectable` protocol static func expansion(of node: AttributeSyntax, providingMembersOf declaration: some DeclGroupSyntax, in context: some MacroExpansionContext) throws -> [DeclSyntax] { @@ -18,34 +11,7 @@ struct InjectableMacro: ExtensionMacro, MemberMacro { throw MacroExpansionErrorMessage("@Injectable needs to be declared on a concrete type, not a protocol") } - let allMembers = declaration.memberBlock.members - // Go through all members and return valid variable declarations when needed - let allVariables = allMembers.compactMap { (memberBlock) -> VariableDeclaration? in - // Only do this for stored properties that are not `let` with a value (since those are constant) - guard let declSyntax = memberBlock.decl.as(VariableDeclSyntax.self), - declSyntax.isStoredProperty, - !declSyntax.isLetWithValue, - !declSyntax.isStaticOrLazy, - let propertyName = declSyntax.variableName, - let typeName = declSyntax.typeName - else { return nil } - - // If the code has `InjectableName` on it, get the name to use - let injectedName = declSyntax.attributes.compactMap { (attribute) -> String? in - switch attribute { - case .attribute(let syntax): - // Check for `InjectableName` and then get the name from it - guard let name = syntax.attributeName.as(IdentifierTypeSyntax.self)?.name.text, - name == "InjectableName" else { return nil } - return syntax.arguments?.labeledContent - default: return nil - } - }.first - - // Use the equality expression in the initializer so that people do not need to put a real value - let equalityExpression = declSyntax.bindings.first?.initializer?.value - return VariableDeclaration(name: propertyName, type: typeName, defaultExpression: equalityExpression, injectedName: injectedName) - } + let allVariables = declaration.allMemberVariables // Create the initializer args in the form `name: type = default` let initializerArgs: String = allVariables.map { variable in @@ -112,22 +78,6 @@ struct InjectableMacro: ExtensionMacro, MemberMacro { } } - -enum AccessorType: String { - case `public` - case `private` - case `internal` - case `fileprivate` -} - -extension DeclModifierListSyntax { - var accessModifier: String? { - return compactMap { modifier in - AccessorType(rawValue: modifier.name.text)?.rawValue - }.first - } -} - extension AttributeSyntax.Arguments { // Get the first string literal in the argument list to the macro var labeledContent: String? { @@ -146,65 +96,3 @@ extension AttributeSyntax.Arguments { } } } - -extension VariableDeclSyntax { - /// Determine whether this variable has the syntax of a stored property. - /// - /// This syntactic check cannot account for semantic adjustments due to, - /// e.g., accessor macros or property wrappers. - /// taken from https://github.com/apple/swift-syntax/blob/main/Examples/Sources/MacroExamples/Implementation/MemberAttribute/WrapStoredPropertiesMacro.swift - var isStoredProperty: Bool { - if bindings.count != 1 { - return false - } - - let binding = bindings.first! - switch binding.accessorBlock?.accessors { - case .none: - return true - - case .accessors(let accessors): - for accessor in accessors { - switch accessor.accessorSpecifier.tokenKind { - case .keyword(.willSet), .keyword(.didSet): - // Observers can occur on a stored property. - break - - default: - // Other accessors make it a computed property. - return false - } - } - - return true - - case .getter: - return false - } - } - - // Check if the token is a let and if there is a value in the initializer - var isLetWithValue: Bool { - self.bindingSpecifier.tokenKind == .keyword(.let) && bindings.first?.initializer != nil - } - - // Check if the modifiers have lazy or static, in which case we wouldn't add it to the init - var isStaticOrLazy: Bool { - self.modifiers.contains { syntax in - syntax.name.tokenKind == .keyword(.static) || syntax.name.tokenKind == .keyword(.lazy) - } - } - - var variableName: String? { - self.bindings.first?.pattern.as(IdentifierPatternSyntax.self)?.identifier.text - } - - var typeName: TypeSyntax? { - guard let annotationType = self.bindings.first?.typeAnnotation?.type.trimmed else { return nil } - if (annotationType.is(FunctionTypeSyntax.self)) { - return "@escaping \(annotationType)" - } else { - return annotationType - } - } -} diff --git a/Sources/WhoopDIKitMacros/Support/AccessControlType.swift b/Sources/WhoopDIKitMacros/Support/AccessControlType.swift new file mode 100644 index 0000000..0b19423 --- /dev/null +++ b/Sources/WhoopDIKitMacros/Support/AccessControlType.swift @@ -0,0 +1,16 @@ +import SwiftSyntax + +enum AccessControlType: String { + case `public` + case `private` + case `internal` + case `fileprivate` +} + +extension DeclModifierListSyntax { + var accessModifier: String? { + return compactMap { modifier in + AccessControlType(rawValue: modifier.name.text)?.rawValue + }.first + } +} diff --git a/Sources/WhoopDIKitMacros/Support/VariableDeclSyntax+Injectable.swift b/Sources/WhoopDIKitMacros/Support/VariableDeclSyntax+Injectable.swift new file mode 100644 index 0000000..d7e3fb2 --- /dev/null +++ b/Sources/WhoopDIKitMacros/Support/VariableDeclSyntax+Injectable.swift @@ -0,0 +1,70 @@ +import SwiftSyntax +import SwiftSyntaxBuilder +import SwiftSyntaxMacros +import SwiftSyntaxMacroExpansion + +extension VariableDeclSyntax { + /// Determine whether this variable has the syntax of a stored property. + /// + /// This syntactic check cannot account for semantic adjustments due to, + /// e.g., accessor macros or property wrappers. + /// taken from https://github.com/apple/swift-syntax/blob/main/Examples/Sources/MacroExamples/Implementation/MemberAttribute/WrapStoredPropertiesMacro.swift + var isStoredProperty: Bool { + if bindings.count != 1 { + return false + } + + let binding = bindings.first! + switch binding.accessorBlock?.accessors { + case .none: + return true + + case .accessors(let accessors): + for accessor in accessors { + switch accessor.accessorSpecifier.tokenKind { + case .keyword(.willSet), .keyword(.didSet): + // Observers can occur on a stored property. + break + + default: + // Other accessors make it a computed property. + return false + } + } + + return true + + case .getter: + return false + } + } + + // Check if the token is a let and if there is a value in the initializer + var isLetWithValue: Bool { + self.bindingSpecifier.tokenKind == .keyword(.let) && bindings.first?.initializer != nil + } + + // Check if the modifiers have lazy or static, in which case we wouldn't add it to the init + var isStaticOrLazy: Bool { + self.modifiers.contains { syntax in + syntax.name.tokenKind == .keyword(.static) || syntax.name.tokenKind == .keyword(.lazy) + } + } + + var variableName: String? { + self.bindings.first?.pattern.as(IdentifierPatternSyntax.self)?.identifier.text + } + + var typeName: TypeSyntax? { + guard let annotationType = self.bindings.first?.typeAnnotation?.type.trimmed else { return nil } + if (annotationType.is(FunctionTypeSyntax.self)) { + return "@escaping \(annotationType)" + } else { + return annotationType + } + } + + var isInstanceAssignableVariable: Bool { + return !isStaticOrLazy && !isLetWithValue && isStoredProperty + } +} diff --git a/Sources/WhoopDIKitMacros/Support/VariableDeclaration.swift b/Sources/WhoopDIKitMacros/Support/VariableDeclaration.swift new file mode 100644 index 0000000..cb94195 --- /dev/null +++ b/Sources/WhoopDIKitMacros/Support/VariableDeclaration.swift @@ -0,0 +1,50 @@ +import SwiftSyntax +import SwiftSyntaxBuilder +import SwiftSyntaxMacros +import SwiftSyntaxMacroExpansion + +struct VariableDeclaration { + let name: String + let type: TypeSyntax + let defaultExpression: ExprSyntax? + let injectedName: String? +} + +extension DeclGroupSyntax { + var allMemberVariables: [VariableDeclaration] { + let allMembers = self.memberBlock.members + // Go through all members and return valid variable declarations when needed + return allMembers.compactMap { (memberBlock) -> VariableDeclaration? in + // Only do this for stored properties that are not `let` with a value (since those are constant) + guard let declSyntax = memberBlock.decl.as(VariableDeclSyntax.self), + let propertyName = declSyntax.variableName, + let typeName = declSyntax.typeName + else { return nil } + guard declSyntax.isInstanceAssignableVariable else { return nil } + + // If the code has `InjectableName` on it, get the name to use + let injectedName = injectableName(variableSyntax: declSyntax) + + /// Use the equality expression in the initializer as the default value (since that is how the memberwise init works) + /// Example: + /// var myValue: Int = 100 + /// Becomes + /// init(..., myValue: Int = 100) + let equalityExpression = declSyntax.bindings.first?.initializer?.value + return VariableDeclaration(name: propertyName, type: typeName, defaultExpression: equalityExpression, injectedName: injectedName) + } + } + + private func injectableName(variableSyntax: VariableDeclSyntax) -> String? { + variableSyntax.attributes.compactMap { (attribute) -> String? in + switch attribute { + case .attribute(let syntax): + // Check for `InjectableName` and then get the name from it + guard let name = syntax.attributeName.as(IdentifierTypeSyntax.self)?.name.text, + name == "InjectableName" else { return nil } + return syntax.arguments?.labeledContent + default: return nil + } + }.first + } +}