assertMacroExpansion doing string compare or syntax tree compare?

Using Swift
1

I'm working my way through implementing my first macros, and I ran into something with assertMacroExpansion() that surprised me. It seems to think these two are different:

func bar() { }

and

func bar() {
}

My test:

func
testRequestMappingMacro()
{
	assertMacroExpansion(
		"""
		struct Foo {
			@RequestMapping()
			func bar() { }
		}
		""",
		expandedSource:
		"""
		struct Foo {
			func bar() { }
		}
		""",
		macros: testMacros
	)
}

The error:

error: -[VaporMacrosTests.VaporMacrosTests testRequestMappingMacro] : failed - Actual output (+) differed from expected output (-):
 struct Foo {
–	func bar() { }
+	func bar() {
+	}
 }

I would have thought those two have the same syntax tree and are thus equivalent, but this seems to be doing a string comparison.

2 Likes
2

Yes, assertMacroExpansion performs a string comparison. The implementation of assertMacroExpansion ultimately calls assertStringsEqualWithDiff (source).

I think this makes sense because source fidelity is one of SwiftSyntax's design philosophies, i.e. the syntax tree not only represents the syntax nodes, but also whitespace, comments etc.

3

Except it's not really preserving source fidelity in this case. I wrote the method all on one line, but it put a newline between the braces. Rather, I should say, the macro expansion is not preserving the source. It's implemented like this:

public struct RequestMappingMacro : PeerMacro
{
	public static func
	expansion(of node: AttributeSyntax,
				providingPeersOf declaration: some DeclSyntaxProtocol,
				in context: some MacroExpansionContext)
		throws -> [DeclSyntax]
	{
		return []
	}

Is that a bug in macro expansion?

2 Likes
4

I’ve also ran into this same issue, with my tests failing simply because of strange white space issues that seem to be a bug. @Douglas_Gregor is this behavior that you have seen before?

1 Like
5

Yes, the overall behavior is intended. The macro expansion operation does some "light" formatting of the resulting syntax, and the assertion does consider whitespace significant when comparing the results. @bnbarham can probably explain the idea here better than I.

Doug

6

The problem I see with this is that my tests become dependent on the rendering of the syntax tree, rather than the validity of the program code generated. Should macros change in the future to render the source text differently, my tests will break.

Now, I can see when I would want to test the rendering, for example if I’m writing a code formatter. But I'm not sure that testing of macro expansion should consider format. I'd really like to see a test assertion suite that ignores formatting.

7 Likes
7

To expand on this, BasicFormat is what ends up being run on the macro expansion. To copy from the docs I just added:

... implementation is primarily aimed at adding whitespace where
required such that re-parsing the tree's description results in the same
tree. But it also makes an attempt at adding in formatting, eg. splitting
lines where obvious and some basic indentation at nesting levels.

This is generally less of an issue when using string interpolations to build up the syntax tree, but for trees created manually it means there's no need to worry about inserting the correct trivia (eg. a space between the struct keyword and the identifier naming that struct).

Note that you can disable the automatic formatting using formatMode in your macro implementation:

  static var formatMode: FormatMode {
    return .disabled
  }

This is a reasonable point IMO. There are actually already tree matching assertions, so we could consider having those available for macro expansions as well.

It depends on what those strange whitespace issues were. There's a known issue where unexpected nodes were being formatted by BasicFormat and this lead to some odd behaviour, eg. a space being added at the start of a string literal. There's two fixes here:

  1. assertMacroExpansion now asserts when there's syntax errors in the expanded tree
  2. BasicFormat should not format any unexpected nodes

@ahoppen also has another PR to log when creation of a syntax node using string interpolation failed, though it would be interesting to see what other ideas we can come up with. We did originally fatal error in this case, but that went against swift-syntax's core principle of graceful recovery.

2 Likes
8

Thanks everyone for raising and addressing this.

As an experience report, I spent more time futzing with assertMacroExpansion() than implementing macros.

I fell back to a local fork of assertStringsEqualWithDiff() that collapses series of newlines or of spaces and tabs. That works when testing different indent styles and nesting levels. I can't think of a whitespace variant this logic wouldn't properly compare, and recommend that logic be available via a (pass-through) parameter.

As for indenting alternatives, I went down the road of inspecting the declaration leading trivia, but that didn't seem reliable in practice or theory for indenting generated members of a type.

I tried to disable formatting in the macro:

That might work on the macro expansion, but the test assertion always formats (L293 below [1]) using its indent parameter. This assertion could check the macros and not format if some macro had .disable and no macro had .auto, but that's still not broadly usable and still unreliable wrt the expected string:

let haveDisable = nil != macros.first { $0.value.formatMode == .disabled}
let haveAuto = nil != macros.first { $0.value.formatMode == .auto}
let doFormat = haveAuto || !haveDisable
let formattedSourceFile = !doFormat ? expandedSourceFile
      : expandedSourceFile.formatted(using: BasicFormat(indentationWidth: indentationWidth))

I might head to the more reliable tree-matching assertions when I find them or they become available.

However, the simplicity of String is very appealing and accurate enough, if the assertion can opt into being as flexible as the compiler is wrt whitespace.

[1] Assertion always formats:

9

An old thread, but this has turned out to be an issue still in 2025. I understand the intent was to ensure that Macros spit out legible source code, but assertMacroExpansion as implemented is very frustrating.

Consider this:

Screenshot 2025-01-15 at 15.08.19
Screenshot 2025-01-15 at 15.08.191920×1487 115 KB
v

I've been fighting, trying to get the blank lines to pass. It doesn't like \n in the "expected output". But if I leave those lines blank in "expected output", it tells me that it didn't expect any blank lines. I don't control the line spacing between init() and func(); I'm just passing two declSyntaxes out of the construction method.

Maybe you could consider a slightly less pedantic assertion to complement this one. Something like assertMacroExpansion(ignoreInsigificantWhitespace: true ...

As it stands currently, I've also abandoned it for a less pedantic comparison. If people aren't using the API, maybe that's a sign that the API needs work?

4 Likes
10

Unfortunately the state of things make writing tests for my Macros almost useless at the moment.

Typically the test failure provides a diff view of the differences, but in my case there's 0 differences highlighted.

I've tried adjusting whitespace and disabling formatting and both don't seem make a difference.

I even tried copying the expanded macro and pasting it in the expanded source and it fails to match.