Fix a few macro issues

- Introduce better diagnostics when defaulting with a non-closure literal, like `unimplemented` - Support multiline default closures.
pointfreeco · Nov 16, 2023 · db0288f · db0288f
1 parent 1b0c4b5
commit db0288f
Show file tree

Hide file tree

Showing 5 changed files with 134 additions and 10 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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"
+          }
+      }
+      """
+    }
+  }
 }