swiftlang/swift-syntax

Rethinking Diagnostic Order in Macro Expansion Assertions

Opened this issue · 2 comments

Matejkob commented

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!

ahoppen commented

Tracked in Apple’s issue tracker as rdar://118335358

ahoppen commented

The order does not matter. The hard part is just trying to match the actual with the expected diagnostics without generating totally bogus output if e.g. the diagnostic message of a single diagnostic changes.

If you have any ideas of how to do that, I’m all ears. We just need to be very careful to not fall into the category of “this program tried to be a lot smarter than it actually is”. Ie. right now, it’s quite obvious what’s going on. If we try to be smart and essentially start writing a diff algorithm, things might turn out to be more confusing in the end than they are at the moment.

  • 👍1