Adding guard to macro that just calls another function

Honestly this looks fine to me—it’s nice and explicit. It’s not just the when clause that’s different but payload: unquote(pattern) v payload: unquote(payload). What you have is nice and explicit and I think DRYing it up would probably be more confusing (and hence a great case of “when DRY is too DRY”).

Although I see Chris K. is replying and he a notorious committer of macro crimes, so I’m interested to see what he’ll suggest

Consider the (only semi-public but very stable) :elixir_utils.extract_guards/1 function from Elixir’s compiler. It takes in Macro AST of an expression, and returns a two-tuple of that expression with top-level guards removed, and a list of the guards extracted. Its implementation is similar to yours but handles multiple guards correctly (a rarely used feature in Elixir code, but syntactically valid).

Ex:

payload = quote(do: foo)
{pattern, guards} = :elixir_utils.extract_guards(payload)
#=> {{:foo, [], Elixir}, []}
payload = quote(do: foo when fizz)
{pattern, guards} = :elixir_utils.extract_guards(payload)
#=> {{:foo, [], Elixir}, [{:fizz, [], Elixir}]}
payload = quote(do: foo when fizz when buzz)
{pattern, guards} = :elixir_utils.extract_guards(payload)
#=> {{:foo, [], Elixir}, [{:fizz, [], Elixir}, {:buzz, [], Elixir}]}

You can then rebuild the guard AST around a new expression for insertion where desired without thinking about if there are zero or more by reducing over them:

new_guarded_expression = quote(do: bar)
Enum.reduce(guards, new_guarded_expression, fn guard, expression ->
  {:when, [], [expression, guard]}
end)
|> Macro.to_string
#=> "(bar when fizz) when buzz"

He did not disappoint, but also made me more confident in my resolve TIL about :elixir_utils.extract_guards which is very cool. But for what it’s worth, in a tense debugging session (or even just coming across it randomly in while trying to understand a system) I would much rather see your code.

I’m inclined to agree if this macro is intended for an application. You control the version of Elixir at all points in time, control the calling code and can proceed with confidence that guards are not nested, and the simpler literal AST manipulation is more intention-revealing.

If it’s meant for a library there’s a small benefit in using these compiler functions (aside from handling nested guards): you’ll also know whatever version of Elixir the end user is running, your code will work for that version, whatever “correct” semantics of extracting guards are like for that version. (Not that the semantics of guards have ever changed or are likely to throughout Elixir ~> 1.0.)

Yep, I’m glossing over a lot here, especially since OP’s code is clearly a test assertion. There is absolutely a huge difference in what’s acceptable in library v. application code and I was assuming this was for the latter (even if it is library-esque code within application code).

@christhekeele @sodapopcan this is actually from Phoenix, so it is library code. I wanted to add the functionality as stated in my post for testing.