Elixir: Formatter: always omit parens

[jfrolich]

Current behavior

Currently locals_without_parens keeps the parentheses in place for instances where there already are parentheses in place. As the goal for a formatter is to have consistent code, this allows for inconsistencies in code formatting. I have this problem in my own project, where there are some parts of the code with parentheses and others not. For instance in schemas (especially since I formatted the project in an early version of the formatter that converted all function calls and macros to include parentheses.

Expected behavior

There should be configuration to force the omission of parentheses for certain keywords (if it's possible given the line-width). It would be even better if it would be able to be scoped, so we can only omit parentheses from a function call originating from a certain module (would be able to derive this from the AST?).

Actually I am for complete idempotency of the code formatter, so the AST should always yield the same code output (except for a double line break). So my preference would be to have a single configuration: which keywords are without parens, and apply this consistently. There are a few annoyances in the code formatter where if you make naming longer it will format over multiple lines but if you make naming shorter, it will not unwrap back in a single line (opposed to other formatters like Prettier), so you still have to think about formatting.

A compromise would be to include an option to force omit parentheses: force_locals_without_parens.

[josevalim]

Hi @jfrolich,

The issue is that in many situations removing the parens would be undesired, for example, imagine that you wrote this code elsewhere in your app:

def field(map, value) do
  ...
end

According to the feature proposal, it would become:

def field map, value do
  ...
end

Sure, the formatter could be a bit smart about this (i.e. only remove parens if the expression is being used directly in the module body) but it still means we would be unable to always apply such rules.

At the same time, I completely understand the situation some projects are, where parentheses were added in places they should have not, and they would like to revert that. But hopefully with the formatter being more widespread and adopted by upcoming Phoenix versions, this will be less of an issue. We will keep assessing the situation.

Actually I am for complete idempotency of the code formatter

Just a minor nitpick. The formatter is idempotent on its input (the textual representation). I don't think making a formatter fully idempotent on the AST would be desired for Elixir though. For example, you may write a function like this:

def my_function(a, b, c) do
  a
  |> other_fun(b)
  |> some_other_fun(c)
end

This pipeline fully fits a single line but if we were to rewrite that to a single line, I am confident the huge majority of the community would be actually displeased with this decision.

So the formatter will continue to take "hints" from its input file and all of them are documented in its API. All of the hints we added were based on feedback from the community and they are likely to stay as is. They are mostly about parens and newlines. I know this makes the formatter not a 100% automatic but I believe the hints give a bit of flexibility to the formatter, especially since Elixir's AST is generic and you may have similar AST constructs formatted differently.

[jfrolich]

Just a minor nitpick. The formatter is idempotent on its input (the textual representation). I don't think making a formatter fully idempotent on the AST would be desired for Elixir though. For example, you may write a function like this:

I think it's good to be opinionated in cases where the community has different opinions, because in the end it doesn't matter and we all benefit from not having to think about it anymore. I do think for those situations there is a 'right' way to format it that can be expressed in code. Same as with parens, there are obvious places where we want parens like in function definitions.

For instance in the example, wouldn't a rule that pipelines are always formatted on newlines be good enough? Maybe an exception if it's a pipeline of length 2.

Having idempotency from AST has some potential upsides like that people can use their own preferred formatting/linewidth, then before commit apply the canonical formatting.

I built some tests for a macro and used the code formatter to see if the generated code was correct. Generating the code and formatting, then comparing it to a snapshot really made the tests very readable. However it did strange things like add extra parens for function definitions. It's one example where idempotency from AST would be useful (only a small case I know).

[josevalim]

I think it's good to be opinionated in cases where the community has different opinions, because in the end it doesn't matter and we all benefit from not having to think about it anymore.

Other people find the formatter too opinionated and that we don't give them enough flexibility. This is a common thread in all of the formatter discussions, there are people arguing in favor of (or against) both sides. You could even say the current behaviour is rather too strict/opinionated (always keep parens) and you want more options around it.

Same as with parens, there are obvious places where we want parens like in function definitions.

Except that the formatter has no concept of function definitions. Anybody can define their own definition macros, such as defcall, so the formatter has to cope with it. The alternative is to tell the formatter what is considered a definition but that is a worse user experience IMO.

It's one example where idempotency from AST would be useful (only a small case I know).

Macro.to_string/2 provides that, as it is AST based, and not text based.

[jfrolich]

@josevalim, while I really appreciate you taking the time to explain some of the choices made here. I think it would be nicer to keep the issue open and keep an open mind about some of this, because I am pretty sure I have a point here. If we are an open community it would also be good to at least have the discussion before closing doors prematurely.

Other people find the formatter too opinionated and that we don't give them enough flexibility. This is a common thread in all of the formatter discussions, there are people arguing in favor of (or against) both sides. You could even say the current behaviour is rather too strict/opinionated (always keep parens) and you want more options around it.

A code formatter reason d'etre is to eliminate discussion about code formatting style, and automate the formatting as much as possible. For the people in favor against the code formatter making code style opinions, they can opt-out of the code formatter.

If there are two popular ways of formatting an expression, there can be configuration to satisfy both sides, but I think it's always better to have one canonical thing format. Ultimately formatting doesn't matter as long as it's readable! And consistent code style among projects improves the DX.

I think it's good to separate formatter configuration with code consistency.

I think code consistency is very important. If you take cues from how to source code is currently formatted to how it should be formatted, people are still in control of a percentage of how the source code is formatted. This means that if I have an open source project, and I ask people to run the formatter, there is still discussion around code style.

Your argument is that it's not possible to build good format rules in some scenarios. I'm of the opinion that it is possible to for each scenario come up with a good consistent formatting rule. Perhaps we can go through a number of these edge cases.

If you'd like people to control formatting. I thing it's waaaay better to have these affordances in configuration than in taking cues from current code so at least these preferences are applied consistently across the code base.

Except that the formatter has no concept of function definitions. Anybody can define their own definition macros, such as defcall, so the formatter has to cope with it. The alternative is to tell the formatter what is considered a definition but that is a worse user experience IMO.

We could have a default paren preference rule for def / defmacro / defcall. Then each library can export paren preferences for macros in the ecosystem. Perhaps we can even generalize for expressions in the defmodule body.

Macro.to_string/2 provides that, as it is AST based, and not text based.

Yes that is what I used, but if you quote > Macro.to_string/1 > Code.format_string!/1 the result is different (my point):

iex(5)> quote do 
  def bla do 
    bla 
  end 
end |> Macro.to_string() |> Code.format_string!() |> IO.puts
def(bla) do
  bla
end

[josevalim]

If you'd like people to control formatting. I thing it's waaaay better to have these affordances in configuration than in taking cues from current code

You prefer configuration but configuration is global and opens up the space for more style discussions than the small affordances we have. It seems we value those trade-offs differently so we arrive at different conclusions.

I agree that you have a point. The formatter could behave as you describe but we chose a different general direction to follow (allowances over configuration). Picking another direction would be a major change on how the formatter works and we don't plan to do so, partly because it would lead to a whole bunch of other issues on how the formatter is too inflexible (which some will prefer, others won't - and that's the formatter in a nutshell).

Going back to the topic of this specific bug report, we can't fix it without introducing false positives (i.e. we will remove parens when people would like to keep them). Alternatively we can introduce a new configuration option. Maybe we will add it in the future once we collect more feedback but we have been generally conservative about adding new formatter configs.

Therefore I respectfully disagree that the issue was closed prematurely. We don't intend to take actions on the issue for the reasons given. Note however that closing the issue is not a mechanism to shut down the discussion. We will be glad to keep it on going and to provide feedback.

Yes that is what I used, but if you quote > Macro.to_string/1 > Code.format_string!/1 the result is different (my point):

Yes, and they work differently by design. It is totally ok to disagree with the choice but don't expect it to change, as that would yield a very different formatter than the one we have today.

PS: I have properly tagged the issue because I forgot to do it the first time around.