Repository: apple/swift-syntax (swift)

Issue:
`SyntaxStringInterpolationError` should prepend `Internal macro error: ` if hit by a macro expansion
### Description

If a macro expansion hits a `SyntaxStringInterpolationError`, we should prepend some appropriate text to the description to mark it as an internal macro error, eg. `Internal macro error: `.

Pull request:
## Summary
This pull request prepends `SyntaxStringInterpolationError` message with `Internal macro error:`. Closes #2039.

## Changes
- In `MacroExpansionContext.addDiagnostics`, if the passed `error as? SyntaxStringInterpolationError` worked, we prepend it with the `Internal macro error` and grab it's description. Otherwise, we just use `String(describing:error)` as the diagnostic message.
- Adds a basic test for `addDiagnostics` for that behavior.

## Next steps
- @DougGregor, does this look like the right direction to you? I'm new to Macros, and I'm here to learn — no hard feelings if my attempt is naive / plain wrong.
- @Matejkob, sounds like your area of interest! Could you give me some hints to how I could test this end to end, and construct a test macro that would throw syntax string interpolation error?

Patch:
diff --git a/Sources/SwiftSyntaxBuilder/DeclSyntaxParseable.swift b/Sources/SwiftSyntaxBuilder/DeclSyntaxParseable.swift
index c4bb040b2fb..c52e70775fb 100644
--- a/Sources/SwiftSyntaxBuilder/DeclSyntaxParseable.swift
+++ b/Sources/SwiftSyntaxBuilder/DeclSyntaxParseable.swift
@@ -31,7 +31,7 @@ public extension DeclSyntaxParseable {
     if let castedDecl = node.as(Self.self) {
       self = castedDecl
     } else {
-      throw SyntaxStringInterpolationError.producedInvalidNodeType(expectedType: Self.self, actualNode: node)
+      throw SyntaxStringInterpolationInvalidNodeTypeError(expectedType: Self.self, actualNode: node)
     }
   }
 }
diff --git a/Sources/SwiftSyntaxBuilder/Syntax+StringInterpolation.swift b/Sources/SwiftSyntaxBuilder/Syntax+StringInterpolation.swift
index 7c3dddd83b1..d5eb04e9f63 100644
--- a/Sources/SwiftSyntaxBuilder/Syntax+StringInterpolation.swift
+++ b/Sources/SwiftSyntaxBuilder/Syntax+StringInterpolation.swift
@@ -161,23 +161,31 @@ where Self.StringInterpolation == SyntaxStringInterpolation {
   init(stringInterpolation: SyntaxStringInterpolation)
 }
 
-enum SyntaxStringInterpolationError: Error, CustomStringConvertible {
-  case producedInvalidNodeType(expectedType: SyntaxProtocol.Type, actualType: SyntaxProtocol.Type)
-  case diagnostics([Diagnostic], tree: Syntax)
+/// Describes an error when building a syntax node with string interpolation resulted in an unexpected node type.
+public struct SyntaxStringInterpolationInvalidNodeTypeError: Error, CustomStringConvertible {
+  let expectedType: SyntaxProtocol.Type
+  let actualType: SyntaxProtocol.Type
+
+  /// Initialize the invalid node type error providing an expected type, and the actual node that resulted.
+  public init<S: SyntaxProtocol>(expectedType: SyntaxProtocol.Type, actualNode: S) {
+    self.expectedType = expectedType
+    self.actualType = type(of: actualNode)
+  }
 
-  static func producedInvalidNodeType<S: SyntaxProtocol>(expectedType: SyntaxProtocol.Type, actualNode: S) -> Self {
-    return .producedInvalidNodeType(expectedType: expectedType, actualType: type(of: actualNode))
+  public var description: String {
+    return "Parsing the code snippet was expected to produce a \(expectedType) but produced a \(actualType)"
   }
+}
+
+/// A string interpolation error based on a ``SwiftDiagnostics/Diagnostic``.
+struct SyntaxStringInterpolationDiagnosticError: Error, CustomStringConvertible {
+  let diagnostics: [Diagnostic]
+  let tree: Syntax
 
   var description: String {
-    switch self {
-    case .producedInvalidNodeType(expectedType: let expectedType, actualType: let actualType):
-      return "Parsing the code snippet was expected to produce a \(expectedType) but produced a \(actualType)"
-    case .diagnostics(let diagnostics, let tree):
-      // Start the diagnostic on a new line so it isn't prefixed with the file, which messes up the
-      // column-aligned message from ``DiagnosticsFormatter``.
-      return "\n" + DiagnosticsFormatter.annotatedSource(tree: tree, diags: diagnostics)
-    }
+    // Start the diagnostic on a new line so it isn't prefixed with the file, which messes up the
+    // column-aligned message from ``DiagnosticsFormatter``.
+    return "\n" + DiagnosticsFormatter.annotatedSource(tree: tree, diags: diagnostics)
   }
 }
 
diff --git a/Sources/SwiftSyntaxBuilder/SyntaxNodeWithBody.swift b/Sources/SwiftSyntaxBuilder/SyntaxNodeWithBody.swift
index 056fa5a4e37..0f2ac8c3f0b 100644
--- a/Sources/SwiftSyntaxBuilder/SyntaxNodeWithBody.swift
+++ b/Sources/SwiftSyntaxBuilder/SyntaxNodeWithBody.swift
@@ -71,7 +71,7 @@ public extension HasTrailingCodeBlock where Self: StmtSyntaxProtocol {
   init(_ header: SyntaxNodeString, @CodeBlockItemListBuilder bodyBuilder: () throws -> CodeBlockItemListSyntax) throws {
     let stmt = StmtSyntax("\(header) {}")
     guard let castedStmt = stmt.as(Self.self) else {
-      throw SyntaxStringInterpolationError.producedInvalidNodeType(expectedType: Self.self, actualNode: stmt)
+      throw SyntaxStringInterpolationInvalidNodeTypeError(expectedType: Self.self, actualNode: stmt)
     }
     self = castedStmt
     self.body = try CodeBlockSyntax(statements: bodyBuilder())
@@ -121,7 +121,7 @@ public extension HasTrailingOptionalCodeBlock where Self: DeclSyntaxProtocol {
   init(_ header: SyntaxNodeString, @CodeBlockItemListBuilder bodyBuilder: () throws -> CodeBlockItemListSyntax) throws {
     let decl = DeclSyntax("\(header) {}")
     guard let castedDecl = decl.as(Self.self) else {
-      throw SyntaxStringInterpolationError.producedInvalidNodeType(expectedType: Self.self, actualNode: decl)
+      throw SyntaxStringInterpolationInvalidNodeTypeError(expectedType: Self.self, actualNode: decl)
     }
     self = castedDecl
     self.body = try CodeBlockSyntax(statements: bodyBuilder())
@@ -166,7 +166,7 @@ public extension HasTrailingMemberDeclBlock where Self: DeclSyntaxProtocol {
   init(_ header: SyntaxNodeString, @MemberBlockItemListBuilder membersBuilder: () throws -> MemberBlockItemListSyntax) throws {
     let decl = DeclSyntax("\(header) {}")
     guard let castedDecl = decl.as(Self.self) else {
-      throw SyntaxStringInterpolationError.producedInvalidNodeType(expectedType: Self.self, actualNode: decl)
+      throw SyntaxStringInterpolationInvalidNodeTypeError(expectedType: Self.self, actualNode: decl)
     }
     self = castedDecl
     self.memberBlock = try MemberBlockSyntax(members: membersBuilder())
@@ -209,7 +209,7 @@ public extension IfExprSyntax {
   ) throws {
     let expr = ExprSyntax("\(header) {}")
     guard let ifExpr = expr.as(Self.self) else {
-      throw SyntaxStringInterpolationError.producedInvalidNodeType(expectedType: Self.self, actualNode: expr)
+      throw SyntaxStringInterpolationInvalidNodeTypeError(expectedType: Self.self, actualNode: expr)
     }
     self = ifExpr
     self.body = try CodeBlockSyntax(statements: bodyBuilder())
@@ -254,7 +254,7 @@ public extension IfExprSyntax {
   init(_ header: SyntaxNodeString, @CodeBlockItemListBuilder bodyBuilder: () throws -> CodeBlockItemListSyntax, elseIf: IfExprSyntax) throws {
     let expr = ExprSyntax("\(header) {}")
     guard let ifExpr = expr.as(Self.self) else {
-      throw SyntaxStringInterpolationError.producedInvalidNodeType(expectedType: Self.self, actualNode: expr)
+      throw SyntaxStringInterpolationInvalidNodeTypeError(expectedType: Self.self, actualNode: expr)
     }
     self = ifExpr
     self.body = CodeBlockSyntax(statements: try bodyBuilder())
@@ -321,7 +321,7 @@ public extension SwitchExprSyntax {
   init(_ header: SyntaxNodeString, @SwitchCaseListBuilder casesBuilder: () throws -> SwitchCaseListSyntax = { SwitchCaseListSyntax([]) }) throws {
     let expr = ExprSyntax("\(header) {}")
     guard let switchExpr = expr.as(Self.self) else {
-      throw SyntaxStringInterpolationError.producedInvalidNodeType(expectedType: Self.self, actualNode: expr)
+      throw SyntaxStringInterpolationInvalidNodeTypeError(expectedType: Self.self, actualNode: expr)
     }
     self = switchExpr
     self.cases = try casesBuilder()
@@ -355,7 +355,7 @@ public extension VariableDeclSyntax {
   init(_ header: SyntaxNodeString, @CodeBlockItemListBuilder accessor: () throws -> CodeBlockItemListSyntax) throws {
     let decl = DeclSyntax("\(header) {}")
     guard let castedDecl = decl.as(Self.self) else {
-      throw SyntaxStringInterpolationError.producedInvalidNodeType(expectedType: Self.self, actualNode: decl)
+      throw SyntaxStringInterpolationInvalidNodeTypeError(expectedType: Self.self, actualNode: decl)
     }
     self = castedDecl
     precondition(self.bindings.count == 1)
diff --git a/Sources/SwiftSyntaxBuilder/ValidatingSyntaxNodes.swift b/Sources/SwiftSyntaxBuilder/ValidatingSyntaxNodes.swift
index 15fe31d55d5..b42de2cacf1 100644
--- a/Sources/SwiftSyntaxBuilder/ValidatingSyntaxNodes.swift
+++ b/Sources/SwiftSyntaxBuilder/ValidatingSyntaxNodes.swift
@@ -25,7 +25,7 @@ extension SyntaxProtocol {
     if node.hasError {
       let diagnostics = ParseDiagnosticsGenerator.diagnostics(for: node)
       precondition(!diagnostics.isEmpty)
-      throw SyntaxStringInterpolationError.diagnostics(diagnostics, tree: Syntax(node))
+      throw SyntaxStringInterpolationDiagnosticError(diagnostics: diagnostics, tree: Syntax(node))
     }
     self = node
   }
@@ -52,7 +52,7 @@ extension Trivia {
         }
         offset += piece.sourceLength.utf8Length
       }
-      throw SyntaxStringInterpolationError.diagnostics(diagnostics, tree: Syntax(tree))
+      throw SyntaxStringInterpolationDiagnosticError(diagnostics: diagnostics, tree: Syntax(tree))
     }
   }
 }
diff --git a/Sources/SwiftSyntaxMacros/MacroExpansionContext.swift b/Sources/SwiftSyntaxMacros/MacroExpansionContext.swift
index 801c98569c0..9419dff760c 100644
--- a/Sources/SwiftSyntaxMacros/MacroExpansionContext.swift
+++ b/Sources/SwiftSyntaxMacros/MacroExpansionContext.swift
@@ -12,6 +12,7 @@
 
 import SwiftDiagnostics
 import SwiftSyntax
+import SwiftSyntaxBuilder
 
 /// Interface to extract information about the context in which a given
 /// macro is expanded.
@@ -77,7 +78,7 @@ private struct ThrownErrorDiagnostic: DiagnosticMessage {
 }
 
 extension MacroExpansionContext {
-  /// Add diagnostics from the error thrown during macro expansion.
+  /// Adds diagnostics from the error thrown during a macro expansion.
   public func addDiagnostics(from error: Error, node: some SyntaxProtocol) {
     // Inspect the error to form an appropriate set of diagnostics.
     var diagnostics: [Diagnostic]
@@ -85,6 +86,9 @@ extension MacroExpansionContext {
       diagnostics = diagnosticsError.diagnostics
     } else if let message = error as? DiagnosticMessage {
       diagnostics = [Diagnostic(node: Syntax(node), message: message)]
+    } else if let error = error as? SyntaxStringInterpolationInvalidNodeTypeError {
+      let diagnostic = Diagnostic(node: Syntax(node), message: ThrownErrorDiagnostic(message: "Internal macro error: \(error.description)"))
+      diagnostics = [diagnostic]
     } else {
       diagnostics = [Diagnostic(node: Syntax(node), message: ThrownErrorDiagnostic(message: String(describing: error)))]
     }


Test patch:
diff --git a/Tests/SwiftSyntaxMacroExpansionTest/StringInterpolationErrorTests.swift b/Tests/SwiftSyntaxMacroExpansionTest/StringInterpolationErrorTests.swift
new file mode 100644
index 00000000000..f797c7fa23e
--- /dev/null
+++ b/Tests/SwiftSyntaxMacroExpansionTest/StringInterpolationErrorTests.swift
@@ -0,0 +1,81 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2023 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+
+import SwiftDiagnostics
+import SwiftSyntax
+import SwiftSyntaxBuilder
+import SwiftSyntaxMacroExpansion
+import SwiftSyntaxMacros
+import SwiftSyntaxMacrosTestSupport
+import XCTest
+
+// An error that is not `SyntaxStringInterpolationError`, only used to verify
+// that other error types won't get prefixed with `Internal macro error:` when
+// passed to `MacroExpansionContext.addDiagnostics`.
+private struct DummyError: Error {
+  static let diagnosticTestError = DummyError()
+}
+
+// An extension macro that will fail with
+// `SyntaxStringInterpolationError.producedInvalidNodeType`
+private struct DummyMacro: ExtensionMacro {
+  static func expansion(
+    of node: AttributeSyntax,
+    attachedTo declaration: some DeclGroupSyntax,
+    providingExtensionsOf type: some TypeSyntaxProtocol,
+    conformingTo protocols: [TypeSyntax],
+    in context: some MacroExpansionContext
+  ) throws -> [ExtensionDeclSyntax] {
+    let ext = try ExtensionDeclSyntax("var x: Int")
+    return [ext]
+  }
+}
+
+final class StringInterpolationErrorTests: XCTestCase {
+
+  func testMacroExpansionContextAddDiagnosticsAddsSwiftSyntaxInterpolationErrorsWithWrappingMessage() throws {
+    let context = BasicMacroExpansionContext()
+    let error = SyntaxStringInterpolationInvalidNodeTypeError(expectedType: DeclSyntax.self, actualNode: ExprSyntax("test"))
+
+    // Since we only care about the error switch inside of addDagnostics, we don't care about the particular node we're passing in
+    context.addDiagnostics(from: error, node: ExprSyntax("1"))
+    XCTAssertEqual(context.diagnostics.count, 1)
+    let diagnostic = try XCTUnwrap(context.diagnostics.first)
+    XCTAssertTrue(diagnostic.message.starts(with: "Internal macro error:"))
+  }
+
+  // Verify that any other error messages do not get "Internal macro error:" prefix.
+  func testMacroExpansionContextAddDiagnosticsUsesErrorDescriptionForDiagMessage() throws {
+    let context = BasicMacroExpansionContext()
+    let error = DummyError.diagnosticTestError
+
+    context.addDiagnostics(from: error, node: ExprSyntax("1"))
+    XCTAssertEqual(context.diagnostics.count, 1)
+    let diagnostic = try XCTUnwrap(context.diagnostics.first)
+    XCTAssertEqual(diagnostic.message, String(describing: error))
+  }
+
+  func testMacroExpansionSyntaxInterpolationErrorGetsPrefixed() {
+    let expectedDiagnostic = DiagnosticSpec(
+      message: "Internal macro error: Parsing the code snippet was expected to produce a ExtensionDeclSyntax but produced a DeclSyntax",
+      line: 1,
+      column: 1
+    )
+
+    assertMacroExpansion(
+      "@dummy struct Foo {}",
+      expandedSource: "struct Foo {}",
+      diagnostics: [expectedDiagnostic],
+      macros: ["dummy": DummyMacro.self]
+    )
+  }
+}