elixir-lang / elixir

Elixir is a dynamic, functional language for building scalable and maintainable applications
https://elixir-lang.org/
Apache License 2.0
24.51k stars 3.38k forks source link

Code formatter breaks some not statements #6952

Closed johnwahba closed 7 years ago

johnwahba commented 7 years ago

Environment

Elixir & Erlang versions (elixir --version): Erlang 20.1 Elixir master Operating system: OSX

Current behavior

not x in list it converts it to x not in list. (implemented on https://github.com/elixir-lang/elixir/blob/master/lib/elixir/lib/code/formatter.ex#L493) This might be desirable as they do compile to a similar ast, but this does break ecto as where: ^u.id not in [2] is a syntax error while where: not ^u.id in [2] is how a where not is done. Whatever the case, the net impact is running mix format leading to a syntax error.

Expected behavior

Running mix format should not result in a syntax error

ericmj commented 7 years ago

not ^u.id in [2] and ^u.id not in [2] should be equivalent on the AST level which is what Ecto sees.

x not in y syntax requires Elixir 1.5.0+, what version is your Ecto project using?

johnwahba commented 7 years ago

That would explain it: I'm using master elixir to format an elixir 1.4.5 project. Thanks!

josevalim commented 7 years ago

Thanks for the report @johnwahba! Unfortunately I could not reproduce it. Are you running mix format on a codebase and then reverting to an earlier Elixir version?

Can you please post: which Elixir version failed to compile the code and the full snippet before and after? Here is my full IEx session on master:

iex(1)> quote do [where: ^u.id not in [2]]
...(1)> end
[
  where: {
    :not,
    [context: Elixir, import: Kernel],
    [
      {
        :in,
        [context: Elixir, import: Kernel],
        [{:^, [], [{{:., [], [{:u, [], Elixir}, :id]}, [], []}]}, [2]]
      }
    ]
  }
]
iex(2)> quote do [where: not ^u.id in [2]]
...(2)> end
[
  where: {
    :not,
    [context: Elixir, import: Kernel],
    [
      {
        :in,
        [context: Elixir, import: Kernel],
        [{:^, [], [{{:., [], [{:u, [], Elixir}, :id]}, [], []}]}, [2]]
      }
    ]
  }
]
josevalim commented 7 years ago

Oh, @ericmj was faster than me. :) Glad this is solved! ❤️

johnwahba commented 7 years ago

Thanks @ericmj and @josevalim. The formatter is amazing!