Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code.string_to_quoted_with_comments withliteral_encoder changed results of with ... else (_ -> :ok) in 1.17 #14267

Closed
novaugust opened this issue Feb 14, 2025 · 12 comments

Comments

@novaugust
Copy link
Contributor

novaugust commented Feb 14, 2025

Elixir and Erlang/OTP versions

erlang 27.1.1
elixir 1.17.2

Operating system

macos

Current behavior

The following snippet has different output on 1.16 versus 1.17

code = "with :ok <- foo, do: :ok, else: (_ -> :error)"

Code.string_to_quoted_with_comments!(code,
  literal_encoder: fn literal, meta -> {:ok, {:__block__, meta, [literal]}} end,
  token_metadata: true,
  unescape: false
)

The discrepancy only appears when literal_encoder is specified, no change otherwise.

The difference arises in the encoding of the else keyword, where it seems 1.17 has an additional list literal being encoded as the parent of the arrow expression

1.17:

{{:__block__, [format: :keyword, line: 1], [:else]},
       {:__block__, [closing: [line: 1], line: 1],
        [
          [
            {:->, [line: 1],
             [[{:_, [line: 1], nil}], {:__block__, [line: 1], [:error]}]}
          ]
        ]}}

1.16:

{{:__block__, [format: :keyword, line: 1], [:else]},
       [
         {:->, [line: 1],
          [[{:_, [line: 1], nil}], {:__block__, [line: 1], [:error]}]}
       ]}

Found via adobe/elixir-styler#217

@josevalim
Copy link
Member

I am pretty sure it is intentional, as before we were missing the metadata around those parentheses. You can see it by using a smaller code sample:

code = "(_ -> :error)"

I will try to dig the commit that changed this.

@josevalim
Copy link
Member

Here is the commit: 80af632

Here is the issue: #13358

:)

@novaugust
Copy link
Contributor Author

apologies for my issue-search-fu failing me 🙏 tyty

@novaugust
Copy link
Contributor Author

@josevalim any advice for dealing with this expression being the first time that the literal encoding with a :__block__ expression not being reversible?

I understand the option is disclaimered as creating invalid ast, but it's been useful pretending otherwise ;)

ex:

"with :ok <- foo, do: :ok, else: (_ -> :error)"
|> Code.string_to_quoted!(
  literal_encoder: fn literal, meta -> {:ok, {:__block__, meta, [literal]}} end,
  token_metadata: true,
  unescape: false
)
|> Macro.to_string()

# => "with :ok <- foo, do: :ok, else: ->(_, :error)"

i thought the fix might be to just have literal_encoder return the stab itself, maintaining the 1.16 behaviour and keeping that same ast but alas, i still get the same output when transforming back to a string:

"with :ok <- foo, do: :ok, else: (_ -> :error)"
|> Code.string_to_quoted!(
  literal_encoder: fn 
    [{:->, _, _}] = stab, _meta -> {:ok, stab}
    literal, meta -> {:ok, {:__block__, meta, [literal]}} 
  end,
  token_metadata: true,
  unescape: false
)
|> Macro.to_string()

# => "with :ok <- foo, do: :ok, else: ->(_, :error)"

@josevalim
Copy link
Member

You can do a pass that unwraps it if you find {:__block__, _, [[{:->, _, _} | _]]} or similar.

But if you are getting # => "with :ok <- foo, do: :ok, else: ->(_, :error)", it is because the AST is invalid!

@novaugust
Copy link
Contributor Author

novaugust commented Feb 14, 2025

ah thank you, i didn't put together that i wasn't the one who was wrapping it in the block, elixir was =)

  literal_encoder: fn 
    {:__block__, _, [[{:->, _, _} | _] = stab]}, _meta -> {:ok, stab}
    literal, meta -> {:ok, {:__block__, meta, [literal]}} 
  end

this made the magic happen. appreciate your time

circling back to this, don't know what i saw but it wasn't a fix :) but i'll dig in on my own

@novaugust
Copy link
Contributor Author

@josevalim ultimately, I couldn't find a way to make this work while using the public Code.quoted_to_algebra/2. looking around at how other libraries weren't broken, it seems the solution is to call the private Code.Formatter.to_algebra/2 instead.

iex(67)> with = Code.string_to_quoted!(
...(67)>       "with x, do: :ok, else: (_ -> :error)",
...(67)>       literal_encoder: &{:ok, {:__block__, &2, [&1]}},
...(67)>       token_metadata: true,
...(67)>       unescape: false
...(67)> )
iex(68)> with |> Code.quoted_to_algebra(opts) |> Inspect.Algebra.format(:infinity) |> IO.iodata_to_binary()
"with x, do: :ok, else: ->(_, :error)"
iex(69)> with |> Code.Formatter.to_algebra([]) |> Inspect.Algebra.format(:infinity) |> IO.iodata_to_binary()
"with x, do: :ok, else: (_ -> :error)"

(in 1.16, these two expressions would both return the value shown on 69. in 1.17, quoted_to_algebra changed its results)

notably this doesn't occur for (_ -> :ok) alone; i'm only getting the repro when it's inside a keyword

So, I've got my fix at least, though I feel uncomfortable calling private elixir functions. Just curious if this is an intentional change in the language or a bug, so running it by you. As it stands, I couldn't find any way to use Code.quoted_to_algebra/2 to get valid AST while preserving the user's formatting (keyword do else)

@josevalim josevalim reopened this Feb 19, 2025
@josevalim
Copy link
Member

I will try to take a look later and provide an answer.

@josevalim
Copy link
Member

According to the docs of quoted_to_algebra, that should work, so we need to address this. Bug confirmed.

@novaugust
Copy link
Contributor Author

thanks for going on this journey with me ^.^

@josevalim
Copy link
Member

I pushed a test and it seems it works on v1.18 but v1.17 is indeed broken. I suggest updating Elixir then or not using this syntax during v1.17. :)

@novaugust
Copy link
Contributor Author

thanks jose, that's a solution i can get behind :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants