Elixir: Add :force_do_end_blocks configuration to formatter

[josevalim]

The :force_do_end_blocks configuration will convert any inline keyword syntax to the block format. Therefore:

if something?, do: this, else: that

Will become:

if something? do
  this
else
  that
end

Rationale

Adding a new option may sound counter-intuitive to the formatter goals but the reason this option makes sense is because it makes the formatter more consistent and not less. When developers ask for formatter options, it is usually because they want to use different styles, while this option is about enforcing a particular style in a situation where we respect the user's choice today.

Furthermore, this option is convergent. Once you add it to the codebase and format it, removing the option will not make all of the code roll back to its previous state. So this option doesn't have the big downside in that flipping it on and off would cause large diffs in the codebase.

[fertapric]

I guess this would also affect functions, right?

def foo([h | t], acc), do: foo(t, [do_domething(h) | acc]
def foo([], acc), do: acc

# formatted as:

def foo([h | t], acc) do
  foo(t, [do_domething(h) | acc]
end

def foo([], acc) do
  acc
end

[josevalim]

Yup, all of them.

[NickNeck]

How about a config like code_block: :inline and code_block: :do_end?

[josevalim]

I think the word force here is important because flipping it back to false
wont revert the code to how it was before.

Plus I don’t see a scenario where you always want to inline everything.

Imagine all definitions, modules, conditionals, cases, etc being inclined.

José Valimwww.plataformatec.com.br
http://www.plataformatec.com.br/Founder and Director of R&D

[devonestes]

The Elixir formatter doesn't seem like the right place for this kind of configuration.

The way I see the formatter, it's job is to enforce a single community wide standard format. Basically, it decides what is and is not valid Elixir formatting.

This sort of configurable check seems to me like it is the responsibility of a different style checker, like Credo. This check's job would be to decide what is valid formatting _for a specific project_ and not for the language as a whole, and so in my eyes it wouldn't belong in the Elixir formatter.

[whatyouhide]

One thing to note is that we still need a mechanism to have Credo and similar tools be able to change AST in order to fix problems like this automatically. Otherwise, having Credo say that something is inconsistent without fixing it might become annoying and not get used that much.

[devonestes]

Everything that a tool like Credo would need to auto-correct is already available in Elixir. It doesn’t do so right now, but not because that’s an impossible problem to solve with the available APIs.

I don’t think the lack of that feature in some existing library is a compelling reason to add things like this to Elixir Core, but instead a good call to action for those looking to contribute to the Elixir ecosystem to add such a feature to Credo (if René would like to add it) or to create a new library that offers that auto-correct functionality.

[whatyouhide]

@devonestes there's no way right now (as in public APIs) that Credo can change the AST without losing user formatting.

[devonestes]

@whatyouhide Sorry to be a pest about the formalities here, but there absolutely _is_ a way to solve that problem today using only public APIs. It would just be unreasonably difficult to do so, and so people would most likely rely on the private API instead. There's a big difference between impossible and unreasonably difficult.

However, it would be several orders of magnitude easier - and therefore would make it much more likely that someone would add that kind of functionality to one of those tools - if Code.Formatter.to_algebra/2 was public.

[whatyouhide]

@devonestes what are the public APIs for doing that today? I'm not aware of a way to read code and convert it to quoted AST with formatter metadata.

[josevalim]

The issue here is that if the formatter enforces a single format, people
will complain their do/end as keywords have been replaced by blocks. So the
formatter was made to respect the user choice but some people would rather
enforce the blocks. So if the argument is that the formatter should be
strict, then it is an argument in favor of this option. As mentioned in the
description, this option is not about using or another, but about enforcing

something with no way back.

José Valimwww.plataformatec.com.br
http://www.plataformatec.com.br/Founder and Director of R&D

[devonestes]

@josevalim I think the question at the root of this is how you want the formatter to be used.

Right now, because user preferences are accepted in as many places as possible, the Elixir formatter only formats things that it decides are "incorrect". Presently, users have almost no say over that definition of "incorrect", so this gives us a "minimum acceptable formatting" for the language as a whole. Right now users can make that definition of "incorrect" more strict for their individual projects by using tools like Credo, but presently there is no tool (that I'm aware of) that will auto-format based on those preferences other than the formatter.

If you start adding the ability for users to add additional configuration to the formatter, then where is the line between something that should be in the Elixir formatter as a configurable option or handled by another library?

For example, what about an option called :force_no_parens_on_zero_arity_functions that rewrites def thing() do as def thing do, or :force_multi_alias that forced the alias MyApp.{Mod1, Mod2} syntax over multiple calls to alias, or :force_no_single_pipe to rewrite :atom |> Atom.to_string() as Atom.to_string(:atom)?

Other possible configuration options that I can think of off the top of my head that would offer this same behavior are:

:force_snake_case_function_names
:force_snake_case_variable_names
:force_snake_case_module_attributes
:force_pascal_case_module_names
:force_alphabetized_alias_calls
:force_unquoted_atoms
:force_string_sigils_with_escaped_quotes

Those would all have the same "No way back" behavior that you mention since both formats are currently valid with the Elixir formatter.

So, basically, do you want these sorts of options to be the responsibility of the formatter or the responsibility of other libraries maintained by the community? I personally don't think either choice is right or wrong - it's ultimately the core team's call - but I think the core team's intentions for what function the formatter serves in the community is what's really important in this decision.

However, if you do decide that you want this sort of per-project style/formatting to be handled by the community, I think making it easier for other libraries to implement auto-formatting by making public some more of the APIs used in the Elixir formatting process would be a very nice thing for the community so they don't need to implement that themselves or rely on private APIs. This consideration might be helpful to think about when making this choice.

@whatyouhide You are correct, there is no public API currently available that will convert an Elixir file to quoted AST with formatter metadata. However, someone could absolutely write that themselves if they wanted to, which is why it isn't impossible. In fact, it would be fairly easy to do - they'd just have to copy and paste all the code needed for parsing an Elixir file to their project and modify it slightly so they could do:

"path/to/file.ex"
|> File.read!()
|> MyParser.string_to_tokens()
|> MyParser.tokens_to_quoted(force_do_end_blocks: true)
|> MyFormatter.to_algebra()
|> Inspect.Algebra.format(98)

Of course that amount of duplication wouldn't be a smart thing to do since then they'd own a huge codebase that they needed to maintain, but it's absolutely possible today without using any private APIs in Elixir. It would certainly be much easier if some of the undocumented options for Code.string_to_quoted/2 that allow inclusion of formatter metadata and saving comments were made public, and if Code.Formatter.to_algebra/2 were public, but it's definitely not an impossible problem to solve.

[josevalim]

Right now, because user preferences are accepted in as many places as possible, the Elixir formatter only formats things that it decides are "incorrect".

I don't think this interpretation is correct. Ideally we would format everything as consistently as possible but that is not practical. Therefore it respects the user choice in some cases (not in as many as possible) but that's the exception, not the rule. So as I said, this option is all about bringing the formatter close to its ideal view.

For example, what about an option called :force_no_parens_on_zero_arity_functions that rewrites def thing() do as def thing do, or :force_multi_alias that forced the alias MyApp.{Mod1, Mod2} syntax over multiple calls to alias, or :force_no_single_pipe to rewrite :atom |> Atom.to_string() as Atom.to_string(:atom)?

None of the options above belong to the formatter because they change the AST of the code or they cannot be applied consistently - which is directly against the formatter goals. I understand your point though, which is that it can be a slippery slope, but the amount of "forcing" the formatter can do is very limited. We currently list in the docs only 6 cases where we keep the user's choice and some of those make no sense to be configurable (i.e. respecting the user choice is the only option).

To provide an example of how those scenarios are rare, of all of the force options you described, only the sigil one could potentially be made into the formatter.

In fact, it would be fairly easy to do

I wouldn't say it is "fairly easy". The specially annotated AST is counter intuitive due to the way it handles literals (but we need to do it to properly preserve double new lines, which would be impossible to reasonably automate without user input anyway) and those are more annoying to traverse.

Furthermore, as the author of the formatter, I would say writing those rules are far from trivial and, more importantly, I don't believe some of then cannot be done without introducing false positives. Even alphabetized alias calls, that look fairly straight-forward, needs to take into account that one alias can be defined in function of another and it cannot cross require/alias/import boundaries, and that will lead to a non straight-forward amount of code. :)

[devonestes]

@josevalim Oh, for sure adding new rules like as this to the formatter isn't easy at all! But copying & pasting the hard work that you and the rest of the core team have already done into a separate library so that library doesn't have to rely on private APIs is so simple that even I can do it (and have done so, several times). I wouldn't want to trivialize the hard work that went into building what we already have, so my apologies if that came across that way!

If the goal is to put as much in the formatter as possible as long as it does not modify the AST at all, then this check would further that goal and so it seems like it should be added.

Would it also make sense that users would be given the option for :force_no_do_end_blocks to force the if true, do: true, else: false or def func, do: true syntax in all possible cases? It would be much more difficult to implement for sure, but it would stand to reason that if one option is available that the other would also be available.

What about allowing folks to specify if they want to force do .. end blocks for if or unless macros but not for function definitions or other macros that can accept a do .. end block?

Also, the sigil examples do change the AST (although they're semantically equivalent), so that wouldn't fit the rules:

iex(3)> Code.string_to_quoted("\"a\"")       
{:ok, "a"}
iex(4)> Code.string_to_quoted("~s(a)")
{:ok, {:sigil_s, [line: 1], [{:<<>>, [line: 1], ["a"]}, []]}}

I think it would still be nice to make some of these APIs used in the formatting process public so folks could more easily write something that does auto-formatting that _does_ change the AST, like in the case of forcing the use of sigils or forcing parens (or no parens) on zero arity functions since those are semantically equivalent but do change the AST.

[josevalim]

I wouldn't want to trivialize the hard work that went into building what we already have, so my apologies if that came across that way!

Oh, it did not come across this way at all. My point is that, even after copy and pasting all of the code, writing the rules themselves are still hard.

Would it also make sense that users would be given the option for :force_no_do_end_blocks to force the if true, do: true, else: false or def func, do: true syntax in all possible cases?

What about allowing folks to specify if they want to force do .. end blocks for if or unless macros but not for function definitions or other macros that can accept a do .. end block?

This is counter intuitive to why we are adding the feature. We want to remove choice, not add more. If you look at any codebase, do/end is used way more frequently than the keywords one.

[OvermindDL1]

This is counter intuitive to why we are adding the feature. We want to remove choice, not add more. If you look at any codebase, do/end is used way more frequently than the keywords one.

I was curious, so just as a data point from my codebase:

Using , *do:: 733 occurrences

Using do: 738 occurrences

So a do...end is more common, but , do: is still very common as well. I consider it a success when I get a function to be a single expression so I can fit it on one easily readable line like that. ^.^

[OvermindDL1]

From my deps directory it is:

Using , *do:: 4559 occurrences

Using do: 16372 occurrences

The actual regex is more complex than this but this is the jist of it.

[josevalim]

I consider it a success when I get a function to be a single expression so I can fit it on one easily readable line like that. ^.^

Haha, I loved the way you phrased that as I feel the same. But some people find the duality confusing they were hoping the formatter would not allow both. I don't personally agree with the sentiment but I agree it is within the formatter goal to remove options. :)

[OvermindDL1]

I updated the values _right before_ you posted as my regex was a bit bad at first, but yeah, I'm surprised at my rate of , do:'s. :-)

Do note, I don't use the formatter on this project (it's my big work project, and a lot of its functionality is in libraries that I built and bring in) as the way the formatter formats a lot of the code in it is absolutely abysmally unreadable, so I actually have a lot of aligned , do:'s so things like:

def blah() when blorp,       do: blah(stuff)

to keep all heads aligned, which is far more readable for me. :-)

[josevalim]

as the way the formatter formats a lot of the code in it is absolutely abysmally unreadable

Yes, you mention this on pretty much every opportunity you get :P

[OvermindDL1]

Yes, you mention this on pretty much every opportunity you get :P

Sorry! I don't give examples anymore though! ^.^;

I think I just have a habit of writing more erlang'y/ocaml'y code than Elixir'y code, which definitely has a clash of styles and readability... ^.^;

EDIT: On the plus side, on the brand new files for the past couple of months at work I've been trying to have the formatter keep those auto-formatted, it's worked on ~90% of them when designed for it from the get-go. :-)

[josevalim]

Closing in favor of #8978.