swift-syntax icon indicating copy to clipboard operation
swift-syntax copied to clipboard

Rethinking Diagnostic Order in Macro Expansion Assertions

Open Matejkob opened this issue 1 year ago • 2 comments

Description

I've been tinkering with macro expansions in Swift and hit an interesting crossroad. It's all about how we handle the order of DiagnosticSpec in assertMacroExpansion. I'm curious to hear your thoughts on this.

Right now, when we assert macro expansions, we're pretty strict about the order of expected diagnostics. Here’s a quick example to show you what I mean:

fileprivate struct TestMacro: DeclarationMacro {
  static func expansion(
    of node: some FreestandingMacroExpansionSyntax,
    in context: some MacroExpansionContext
  ) throws -> [DeclSyntax] {
    let firstErrorMessage = MacroExpansionErrorMessage("First message")
    context.diagnose(Diagnostic(node: node.macroName, message: firstErrorMessage))
    let secondErrorMessage = MacroExpansionErrorMessage("Second message")
    context.diagnose(Diagnostic(node: node.pound, message: secondErrorMessage))
    return []
  }
}

final class MacroDiagnosticsTests: XCTestCase {
  func test_1() {
    assertMacroExpansion(
      "#test",
      expandedSource: "",
      diagnostics: [
        DiagnosticSpec(message: "First message", line: 1, column: 2),
        DiagnosticSpec(message: "Second message", line: 1, column: 1)
      ],
      macros: ["test": TestMacro.self]
    )
  }

  func test_2() {
    assertMacroExpansion(
      "#test",
      expandedSource: "",
      diagnostics: [
        /// failed - message does not match
        /// –Second message
        /// +First message
        ///
        /// XCTAssertEqual failed: ("2") is not equal to ("1") - column does not match
        DiagnosticSpec(message: "Second message", line: 1, column: 1),
        /// failed - message does not match
        /// –First message
        /// +Second message
        ///
        /// XCTAssertEqual failed: ("2") is not equal to ("1") - column does not match
        DiagnosticSpec(message: "First message", line: 1, column: 2)
      ],
      macros: ["test": TestMacro.self]
    )
  }
}

This got me thinking – how much does the order really matter? I mean, would it be better if we tried to match the expected diagnostics to their corresponding emitted equivalents, even if it means complicating the assertion code a bit?

I'm torn between keeping things simple and going for more precision. What's your take? Do you think keeping a strict order is the way to go, or should we be a bit more flexible, especially in more complex codebases?

Would love to hear your experiences or any ideas you might have. Maybe there's an angle I haven't considered yet!

Looking forward to your thoughts!

Matejkob avatar Nov 13 '23 06:11 Matejkob