From 0df36e74f6367f51aa1f3e0c33bb3df56556231c Mon Sep 17 00:00:00 2001 From: Natik Gadzhi Date: Sun, 10 Sep 2023 23:08:37 -0700 Subject: [PATCH 1/2] Mark syntax interpolation errors in macro expansion - This commit prepends `SyntaxStringInterpolationError` message with `Internal macro error:` when adding it as a diagnostic in `MacroExpansionContext.addDiagnostics`, so that macro expansion clarifies the error as internal to the macro, not to how it was applied. - Any unrecognized errors raised and sent to `addDiagnostics` will be added as is, without prepending them with a message. That's consistent with how it works now. - Adds a basic test for `addDiagnostics` for that behavior. - Adds end to end test with `assertMacroExpansion`, thanks @ahoppen for the hint! --- .../Syntax+StringInterpolation.swift | 7 +- .../MacroExpansionContext.swift | 6 +- .../MacroExpansionContextTests.swift | 79 +++++++++++++++++++ 3 files changed, 89 insertions(+), 3 deletions(-) create mode 100644 Tests/SwiftSyntaxMacroExpansionTest/MacroExpansionContextTests.swift diff --git a/Sources/SwiftSyntaxBuilder/Syntax+StringInterpolation.swift b/Sources/SwiftSyntaxBuilder/Syntax+StringInterpolation.swift index 7c3dddd83b1..8698ba5f9d3 100644 --- a/Sources/SwiftSyntaxBuilder/Syntax+StringInterpolation.swift +++ b/Sources/SwiftSyntaxBuilder/Syntax+StringInterpolation.swift @@ -161,7 +161,10 @@ where Self.StringInterpolation == SyntaxStringInterpolation { init(stringInterpolation: SyntaxStringInterpolation) } -enum SyntaxStringInterpolationError: Error, CustomStringConvertible { +// Used in `MacroExpansionContext` to make a nicer error message when a +// macro expansion fails with SwiftSyntaxBuilder making a syntax node +// with string interpolation. +@_spi(Diagnostics) public enum SyntaxStringInterpolationError: Error, CustomStringConvertible { case producedInvalidNodeType(expectedType: SyntaxProtocol.Type, actualType: SyntaxProtocol.Type) case diagnostics([Diagnostic], tree: Syntax) @@ -169,7 +172,7 @@ enum SyntaxStringInterpolationError: Error, CustomStringConvertible { return .producedInvalidNodeType(expectedType: expectedType, actualType: type(of: actualNode)) } - var description: String { + public 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)" diff --git a/Sources/SwiftSyntaxMacros/MacroExpansionContext.swift b/Sources/SwiftSyntaxMacros/MacroExpansionContext.swift index 801c98569c0..385e2b37b7a 100644 --- a/Sources/SwiftSyntaxMacros/MacroExpansionContext.swift +++ b/Sources/SwiftSyntaxMacros/MacroExpansionContext.swift @@ -12,6 +12,7 @@ import SwiftDiagnostics import SwiftSyntax +@_spi(Diagnostics) 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? SyntaxStringInterpolationError { + 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)))] } diff --git a/Tests/SwiftSyntaxMacroExpansionTest/MacroExpansionContextTests.swift b/Tests/SwiftSyntaxMacroExpansionTest/MacroExpansionContextTests.swift new file mode 100644 index 00000000000..43e02f92256 --- /dev/null +++ b/Tests/SwiftSyntaxMacroExpansionTest/MacroExpansionContextTests.swift @@ -0,0 +1,79 @@ +//===----------------------------------------------------------------------===// +// +// 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 +@_spi(Diagnostics) 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 MacroExpansionContextTests: XCTestCase { + + func testMacroExpansionContextAddDiagnosticsAddsSwiftSyntaxInterpolationErrorsWithWrappingMessage() { + let context = BasicMacroExpansionContext() + let error = SyntaxStringInterpolationError.producedInvalidNodeType(expectedType: DeclSyntax.self, actualType: ExprSyntax.self) + // 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) + XCTAssertTrue(context.diagnostics.first!.message.starts(with: "Internal macro error:")) + } + + // Verify that any other error messages do not get "Internal macro error:" prefix. + func testMacroExpansionContextAddDiagnosticsUsesErrorDescriptionForDiagMessage() { + let context = BasicMacroExpansionContext() + let error = DummyError.diagnosticTestError + + context.addDiagnostics(from: error, node: ExprSyntax("1")) + XCTAssertEqual(context.diagnostics.count, 1) + XCTAssertEqual(context.diagnostics.first!.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] + ) + } +} From ec8265e4312400e09b0cc035e5c28c18b8f04299 Mon Sep 17 00:00:00 2001 From: Natik Gadzhi Date: Mon, 11 Sep 2023 15:25:20 -0700 Subject: [PATCH 2/2] Make `SyntaxStringInterpolationInvalidTokenTypeError` public This commit splits `SyntaxStringInterpolationError` into two error types: - `SyntaxStringInterpolationInvalidTokenTypeError` for situations when interpolation resulted in a wrong token type, and - `SyntaxStringInterpolationDiagnosticError` when another error occured, and the user provides additional diagnostics with the error. Additionally: - The `SyntaxStringInterpolationInvalidTokenTypeError` is now `public` so other clients can catch it. - Both error types are structs instead of enums. - All invocation spots are updated to match the API. --- .../DeclSyntaxParseable.swift | 2 +- .../Syntax+StringInterpolation.swift | 39 +++++++++++-------- .../SyntaxNodeWithBody.swift | 14 +++---- .../ValidatingSyntaxNodes.swift | 4 +- .../MacroExpansionContext.swift | 4 +- ...ft => StringInterpolationErrorTests.swift} | 20 +++++----- 6 files changed, 45 insertions(+), 38 deletions(-) rename Tests/SwiftSyntaxMacroExpansionTest/{MacroExpansionContextTests.swift => StringInterpolationErrorTests.swift} (82%) 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 8698ba5f9d3..d5eb04e9f63 100644 --- a/Sources/SwiftSyntaxBuilder/Syntax+StringInterpolation.swift +++ b/Sources/SwiftSyntaxBuilder/Syntax+StringInterpolation.swift @@ -161,26 +161,31 @@ where Self.StringInterpolation == SyntaxStringInterpolation { init(stringInterpolation: SyntaxStringInterpolation) } -// Used in `MacroExpansionContext` to make a nicer error message when a -// macro expansion fails with SwiftSyntaxBuilder making a syntax node -// with string interpolation. -@_spi(Diagnostics) public enum SyntaxStringInterpolationError: Error, CustomStringConvertible { - case producedInvalidNodeType(expectedType: SyntaxProtocol.Type, actualType: SyntaxProtocol.Type) - case diagnostics([Diagnostic], tree: Syntax) - - static func producedInvalidNodeType(expectedType: SyntaxProtocol.Type, actualNode: S) -> Self { - return .producedInvalidNodeType(expectedType: expectedType, actualType: type(of: actualNode)) +/// 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(expectedType: SyntaxProtocol.Type, actualNode: S) { + self.expectedType = expectedType + self.actualType = type(of: actualNode) } public 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) - } + 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 { + // 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 385e2b37b7a..9419dff760c 100644 --- a/Sources/SwiftSyntaxMacros/MacroExpansionContext.swift +++ b/Sources/SwiftSyntaxMacros/MacroExpansionContext.swift @@ -12,7 +12,7 @@ import SwiftDiagnostics import SwiftSyntax -@_spi(Diagnostics) import SwiftSyntaxBuilder +import SwiftSyntaxBuilder /// Interface to extract information about the context in which a given /// macro is expanded. @@ -86,7 +86,7 @@ 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? SyntaxStringInterpolationError { + } else if let error = error as? SyntaxStringInterpolationInvalidNodeTypeError { let diagnostic = Diagnostic(node: Syntax(node), message: ThrownErrorDiagnostic(message: "Internal macro error: \(error.description)")) diagnostics = [diagnostic] } else { diff --git a/Tests/SwiftSyntaxMacroExpansionTest/MacroExpansionContextTests.swift b/Tests/SwiftSyntaxMacroExpansionTest/StringInterpolationErrorTests.swift similarity index 82% rename from Tests/SwiftSyntaxMacroExpansionTest/MacroExpansionContextTests.swift rename to Tests/SwiftSyntaxMacroExpansionTest/StringInterpolationErrorTests.swift index 43e02f92256..f797c7fa23e 100644 --- a/Tests/SwiftSyntaxMacroExpansionTest/MacroExpansionContextTests.swift +++ b/Tests/SwiftSyntaxMacroExpansionTest/StringInterpolationErrorTests.swift @@ -12,7 +12,7 @@ import SwiftDiagnostics import SwiftSyntax -@_spi(Diagnostics) import SwiftSyntaxBuilder +import SwiftSyntaxBuilder import SwiftSyntaxMacroExpansion import SwiftSyntaxMacros import SwiftSyntaxMacrosTestSupport @@ -40,26 +40,28 @@ private struct DummyMacro: ExtensionMacro { } } -final class MacroExpansionContextTests: XCTestCase { +final class StringInterpolationErrorTests: XCTestCase { - func testMacroExpansionContextAddDiagnosticsAddsSwiftSyntaxInterpolationErrorsWithWrappingMessage() { + func testMacroExpansionContextAddDiagnosticsAddsSwiftSyntaxInterpolationErrorsWithWrappingMessage() throws { let context = BasicMacroExpansionContext() - let error = SyntaxStringInterpolationError.producedInvalidNodeType(expectedType: DeclSyntax.self, actualType: ExprSyntax.self) - // 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")) + 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) - XCTAssertTrue(context.diagnostics.first!.message.starts(with: "Internal macro error:")) + 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() { + func testMacroExpansionContextAddDiagnosticsUsesErrorDescriptionForDiagMessage() throws { let context = BasicMacroExpansionContext() let error = DummyError.diagnosticTestError context.addDiagnostics(from: error, node: ExprSyntax("1")) XCTAssertEqual(context.diagnostics.count, 1) - XCTAssertEqual(context.diagnostics.first!.message, String(describing: error)) + let diagnostic = try XCTUnwrap(context.diagnostics.first) + XCTAssertEqual(diagnostic.message, String(describing: error)) } func testMacroExpansionSyntaxInterpolationErrorGetsPrefixed() {