Fix crash in Macro.to_string

[sabiwara]

This is the first issue described in #13895.

[sabiwara]

I renamed this because I found it confusing, but if anybody has another suggestion I'm happy to take it.

[josevalim]

I think the bug is actually in the normalizer. :( We only convert the sigil to proper syntax if it has a delimiter:

elixir/lib/elixir/lib/code/formatter.ex

Line 1421 in 3ce07dc

opening_delimiter when not is_nil(opening_delimiter) <- meta[:delimiter] do

I think the issue is that the normalizer is adding a delimiter, when it is not a proper AST. WDYT?

[sabiwara]

OK, so the idea is to treat sigil_r(<<"abc", 123>>, []) just as if it was not a sigil, same as foo(<<"abc", 123>>, [])?

Currently there are two differences:

• adding a delimiter
• wrapping literal args

iex> Code.Normalizer.normalize(quote do: foo(<<"abc", 123>>, []))
{:foo, [closing: [line: nil]],
 [
   {:<<>>, [], [{:__block__, [], ["abc"]}, {:__block__, [token: "123"], ~c"{"}]},
   {:__block__, [], [[]]}  # <=== [] is wrapped
 ]}
iex> Code.Normalizer.normalize(quote do: sigil_r(<<"abc", 123>>, []))
{:sigil_r, [delimiter: "\"", context: Elixir, imports: [{2, Kernel}]],
 [
   {:<<>>, [], [{:__block__, [], ["abc"]}, {:__block__, [token: "123"], ~c"{"}]},
   []                      # <=== [] is NOT wrapped
 ]}

[josevalim]

Exactly. We should not add a delimiter and do other sigil things if it is not a sigil.

[sabiwara]

@josevalim makes sense to me, re-pushed with the new approach adfe040.

I wonder though if it could be considered a regression, this felt intentional based on this commit 2d1c64c. Maybe when creating sigils through meta-programming, if that's a thing?

Oh, and we also need to re-evaluate the use of prune_metadata in assert, as the CI points out? Or keep delimiters here?

[josevalim]

@sabiwara we should definitely support sigils through meta-programming. I think we should not check for delimiters in Code.Normalizer. Instead, we should normalize that both arguments of the sigil macro are "valid" (from the point of view of the formatter) and only add the delimiter if we can normalize them.

[sabiwara]

OK I see what you mean now, sorry for the misunderstanding 🙇

BTW, unrelated but this feels like a good change, there is no need to lose the original delimiter in ExUnit no? I won't need it anymore for this PR though.

[josevalim]

Agreed it is a good change!

[sabiwara]

@josevalim forced-pushed take 3: 020025a.

I also caught a couple of crashes when the modifiers are invalid, covered these cases too.

[sabiwara]

Backported