From 63301f4a181ed9aefb46dccef2dfb66466798341 Mon Sep 17 00:00:00 2001 From: Stephen Celis Date: Thu, 16 Nov 2023 12:42:31 -0800 Subject: [PATCH] Fix a few macro issues (#137) - Introduce better diagnostics when defaulting with a non-closure literal, like `unimplemented` - Support multiline default closures. --- .../DependencyClientMacro.swift | 10 +-- .../DependencyEndpointMacro.swift | 13 ++-- .../DependenciesMacrosPlugin/Support.swift | 33 ++++++++++ .../DependencyClientMacroTests.swift | 24 ++++++- .../DependencyEndpointMacroTests.swift | 64 ++++++++++++++++++- 5 files changed, 134 insertions(+), 10 deletions(-) diff --git a/Sources/DependenciesMacrosPlugin/DependencyClientMacro.swift b/Sources/DependenciesMacrosPlugin/DependencyClientMacro.swift index 32ea0597..9ca45c0c 100644 --- a/Sources/DependenciesMacrosPlugin/DependencyClientMacro.swift +++ b/Sources/DependenciesMacrosPlugin/DependencyClientMacro.swift @@ -21,9 +21,10 @@ public enum DependencyClientMacro: MemberAttributeMacro, MemberMacro { else { return [] } - // NB: Ideally `@DependencyEndpoint` would handle this for us, but there's a compiler crash. - if binding.initializer == nil, - functionType.effectSpecifiers?.throwsSpecifier == nil, + // NB: Ideally `@DependencyEndpoint` would handle this for us, but there are compiler crashes + if let initializer = binding.initializer { + try initializer.diagnose(node) + } else if functionType.effectSpecifiers?.throwsSpecifier == nil, !functionType.isVoid, !functionType.isOptional { @@ -156,12 +157,13 @@ public enum DependencyClientMacro: MemberAttributeMacro, MemberMacro { binding.pattern.trailingTrivia = "" binding.typeAnnotation = TypeAnnotationSyntax( colon: .colonToken(trailingTrivia: .space), - type: type + type: type.with(\.trailingTrivia, .space) ) } if isEndpoint { binding.initializer = nil } else if binding.initializer == nil, type.is(OptionalTypeSyntax.self) { + binding.typeAnnotation?.trailingTrivia = .space binding.initializer = InitializerClauseSyntax( equal: .equalToken(trailingTrivia: .space), value: NilLiteralExprSyntax() diff --git a/Sources/DependenciesMacrosPlugin/DependencyEndpointMacro.swift b/Sources/DependenciesMacrosPlugin/DependencyEndpointMacro.swift index 6c4c401c..7eb49add 100644 --- a/Sources/DependenciesMacrosPlugin/DependencyEndpointMacro.swift +++ b/Sources/DependenciesMacrosPlugin/DependencyEndpointMacro.swift @@ -21,7 +21,9 @@ public enum DependencyEndpointMacro: AccessorMacro, PeerMacro { else { return [] } - + if let initializer = binding.initializer { + try initializer.diagnose(node) + } return [ """ @storageRestrictions(initializes: _\(raw: identifier)) @@ -72,7 +74,6 @@ public enum DependencyEndpointMacro: AccessorMacro, PeerMacro { if let initializer = binding.initializer { guard var closure = initializer.value.as(ClosureExprSyntax.self) else { - // TODO: Diagnose? return [] } if !functionType.isVoid, @@ -83,7 +84,7 @@ public enum DependencyEndpointMacro: AccessorMacro, PeerMacro { statement.item = CodeBlockItemSyntax.Item( ReturnStmtSyntax( returnKeyword: .keyword(.return, trailingTrivia: .space), - expression: expression + expression: expression.trimmed ) ) closure.statements = closure.statements.with(\.[closure.statements.startIndex], statement) @@ -121,7 +122,11 @@ public enum DependencyEndpointMacro: AccessorMacro, PeerMacro { """, at: unimplementedDefault.statements.startIndex ) - + for index in unimplementedDefault.statements.indices { + unimplementedDefault.statements[index] = unimplementedDefault.statements[index] + .trimmed + .with(\.leadingTrivia, .newline) + } var effectSpecifiers = "" if functionType.effectSpecifiers?.throwsSpecifier != nil { effectSpecifiers.append("try ") diff --git a/Sources/DependenciesMacrosPlugin/Support.swift b/Sources/DependenciesMacrosPlugin/Support.swift index 07fa0fca..1f8b1836 100644 --- a/Sources/DependenciesMacrosPlugin/Support.swift +++ b/Sources/DependenciesMacrosPlugin/Support.swift @@ -70,6 +70,39 @@ extension FunctionTypeSyntax { } } +extension InitializerClauseSyntax { + func diagnose(_ attribute: AttributeSyntax) throws { + guard !self.value.is(ClosureExprSyntax.self) else { return } + var diagnostics: [Diagnostic] = [ + Diagnostic( + node: self.value, + message: MacroExpansionErrorMessage( + """ + '@\(attribute.attributeName)' default must be closure literal + """ + ) + ) + ] + if self.value.as(FunctionCallExprSyntax.self)? + .calledExpression.as(DeclReferenceExprSyntax.self)? + .baseName.tokenKind == .identifier("unimplemented") + { + diagnostics.append( + Diagnostic( + node: self.value, + message: MacroExpansionWarningMessage( + """ + Do not use 'unimplemented' with '@\(attribute.attributeName)'; it is a replacement and \ + implements the same runtime functionality as 'unimplemented' at compile time + """ + ) + ) + ) + } + throw DiagnosticsError(diagnostics: diagnostics) + } +} + extension VariableDeclSyntax { var asClosureType: FunctionTypeSyntax? { self.bindings.first?.typeAnnotation.flatMap { diff --git a/Tests/DependenciesMacrosPluginTests/DependencyClientMacroTests.swift b/Tests/DependenciesMacrosPluginTests/DependencyClientMacroTests.swift index 1fa4f23a..df6b293c 100644 --- a/Tests/DependenciesMacrosPluginTests/DependencyClientMacroTests.swift +++ b/Tests/DependenciesMacrosPluginTests/DependencyClientMacroTests.swift @@ -506,7 +506,7 @@ final class DependencyClientMacroTests: BaseTestCase { ✏️ Insert '= { <#Int#> }' } """ - }fixes: { + } fixes: { """ @DependencyClient struct Client: Sendable { @@ -720,4 +720,26 @@ final class DependencyClientMacroTests: BaseTestCase { """ } } + + func testNonClosureDefault() { + assertMacro { + """ + @DependencyClient + struct Foo { + var bar: () -> Int = unimplemented() + } + """ + } diagnostics: { + """ + @DependencyClient + struct Foo { + var bar: () -> Int = unimplemented() + ┬────────────── + ├─ 🛑 '@DependencyClient' default must be closure literal + ╰─ ⚠️ Do not use 'unimplemented' with '@DependencyClient'; it is a replacement and implements the same runtime functionality as 'unimplemented' at compile time + } + """ + } + } } + diff --git a/Tests/DependenciesMacrosPluginTests/DependencyEndpointMacroTests.swift b/Tests/DependenciesMacrosPluginTests/DependencyEndpointMacroTests.swift index a5603d68..ef7bb6e7 100644 --- a/Tests/DependenciesMacrosPluginTests/DependencyEndpointMacroTests.swift +++ b/Tests/DependenciesMacrosPluginTests/DependencyEndpointMacroTests.swift @@ -95,7 +95,7 @@ final class DependencyEndpointMacroTests: BaseTestCase { ✏️ Insert '= { <#Bool#> }' } """ - }fixes: { + } fixes: { """ struct Client { @DependencyEndpoint @@ -693,4 +693,66 @@ final class DependencyEndpointMacroTests: BaseTestCase { """ } } + + func testNonClosureDefault() { + assertMacro { + """ + struct Foo { + @DependencyEndpoint + var bar: () -> Int = unimplemented() + } + """ + } diagnostics: { + """ + struct Foo { + @DependencyEndpoint + var bar: () -> Int = unimplemented() + ┬────────────── + ├─ 🛑 '@DependencyEndpoint' default must be closure literal + ╰─ ⚠️ Do not use 'unimplemented' with '@DependencyEndpoint'; it is a replacement and implements the same runtime functionality as 'unimplemented' at compile time + } + """ + } + } + + func testMultilineClosure() { + assertMacro { + """ + struct Blah { + @DependencyEndpoint + public var doAThing: (_ value: Int) -> String = { _ in + "Hello, world" + } + } + """ + } expansion: { + """ + struct Blah { + public var doAThing: (_ value: Int) -> String = { _ in + "Hello, world" + } { + @storageRestrictions(initializes: _doAThing) + init(initialValue) { + _doAThing = initialValue + } + get { + _doAThing + } + set { + _doAThing = newValue + } + } + + public func doAThing(value p0: Int) -> String { + self.doAThing(p0) + } + + private var _doAThing: (_ value: Int) -> String = { _ in + XCTestDynamicOverlay.XCTFail("Unimplemented: 'doAThing'") + return "Hello, world" + } + } + """ + } + } }