Format codebase files with the upcoming code formatter

[josevalim]

Hi everyone!

Elixir master now includes a code formatter that automatically formats your code according to a consistent style.

We want to convert Elixir's codebase to use the formatter on all files. We understand this means a lot of code churn now but, on the positive side, it means we can automatically enforce and/or format future contributions, making it easier for everyone to contribute to Elixir.

We plan to migrate Elixir's source code to use the formatter gradually and we need your help! If you were looking forward to contribute to Elixir, this is a great opportunity to get started.

Please submit only one pull request per day with only one file formatted per PR, otherwise we are unable to give everyone feedback.

Helping with pull requests

You can contribute by picking a random file in Elixir's codebase, running the formatter on it, analysing it for improvements and then submitting a pull request.

The first step is to clone the Elixir repository and compile it:

$ git clone git@github.com:elixir-lang/elixir.git
$ cd elixir
$ make compile

If you have already cloned it, make sure you have the latest and run make clean compile again. You can read more about contributing in our README.

After you have latest Elixir on your machine, you need to pick a random file to format. We have added a script to do exactly that. From your Elixir checkout root:

$ bin/elixir scripts/random_file.exs

The script will tell you how to format a random file that has not been formatted yet. Run the command suggested by the script, check for style improvements and then submit a pull request for the changes on that single file. We will explain what are the possible improvements in the next section, so please look for them carefully. For more information on pull requests, read here.

Checking for style improvements

The formatter is guaranteed to spit out valid Elixir code. However, depending on how the original code was written, it may unecessarily span over multiple lines when formatted. In such cases, you may need to move some code around. Let's see some examples directly from the Elixir codebase.

Example 1: multi-line data structures

The example below uses Elixir old-style of indentation:

:ets.insert(table, {doc_tuple, line, kind,
                    merge_signatures(current_sign, signature, 1),
                    if(is_nil(doc), do: current_doc, else: doc)})

when formatted it looks like this:

:ets.insert(table, {
  doc_tuple,
  line,
  kind,
  merge_signatures(current_sign, signature, 1),
  if(is_nil(doc), do: current_doc, else: doc)
})

However, the code above only spawn multiple lines because we have a tuple full of expressions. If we move those expressions out, we will get better code altogether:

new_signature = merge_signatures(current_sign, signature, 1)
new_doc = if is_nil(doc), do: current_doc, else: doc
:ets.insert(table, {doc_tuple, line, kind, new_signature, new_doc})

After you do your changes, run the formatter again, see if the final code looks as you desire and ship it!

Example 2: multi-line function definitions

Sometimes a function definition may spawn over multiple lines:

def handle_call({:clean_up_process_data, parent_pid, child_pid}, _from,
                %{parent_pids: parent_pids, child_pids: child_pids} = state) do

When formatted, it will now look like:

def handle_call(
      {:clean_up_process_data, parent_pid, child_pid},
      _from,
      %{parent_pids: parent_pids, child_pids: child_pids} = state
    ) do

In some cases, the multi-line definition is necessary but, in this case, we could simply move the extraction of the state fields out of the function definition, since we are matching on any of them for flow-control:

def handle_call({:clean_up_process_data, parent_pid, child_pid}, _from, state) do
  %{parent_pids: parent_pids, child_pids: child_pids} = state

The code is now clearer and the function definition matches only on arguments that must be matched on the function head.

After you do your changes, run the formatter again, see if the final code looks as you desire and ship it!

Example 3: calls with many arguments

The example below is formatted by the formatter:

assert_raise EEx.SyntaxError,
             "nofile:2: unexpected end of string, expected a closing '<% end %>'",
             fn ->
               EEx.compile_string("foo\n<% if true do %>")
             end

This doesn't look ideal because of all the whitespace it leaves to the left of the code. In this case, prefer to extract arguments in variables so that they all fit on one line. This is especially easy in cases like the one above, because fn can be cleanly split at the end, so it's enough to extract the message into a variable (which is a common pattern in the Elixir codebase specifically regarding assert_raise/3):

message = "nofile:2: unexpected end of string, expected a closing '<% end %>'"

assert_raise EEx.SyntaxError, message, fn ->
  EEx.compile_string("foo\n<% if true do %>")
end

After you do your changes, run the formatter again, see if the final code looks as you desire and ship it!

Example 4: line splits in ugly places

The example below is formatted by the formatter:

"Finished in #{format_us(total_us)} seconds (#{format_us(load_us)}s on load, #{
  format_us(run_us)
}s on tests)"

As you can see, the line split happens on #{. This is not ideal, so in cases similar to this, the correct thing to do is to split the string into multiple strings and concatenate them through <>:

"Finished in #{format_us(total_us)} seconds (#{format_us(load_us)}s on load, " <>
  "#{format_us(run_us)}s on tests)"

After you do your changes, run the formatter again, see if the final code looks as you desire and ship it!

Summing up

Let us know if you have any questions and we are looking forward to your contributions!

[whatyouhide]

I performed the steps José suggested and came up with the first PR, #6652. If you're in doubt you can use it as an example :).

[LostKobrakai]

How about nested function calls to pipes? I've got elixir/lib/port.ex and there are a few line like that after formatting: nillify(:erlang.port_info(port, item)). It could be written as port |> :erlang.port_info(item) |> nillify()

[josevalim]

@LostKobrakai leave those as is especially because many modules in Elixir core cannot use pipes because of bootstrapping issues.

[gmile]

@josevalim running locally, I'm seeing the following code change introduced by formatter:

-    assert_raise RuntimeError, "the given function must return a two-element tuple or :pop, got: 1", fn ->
-      Keyword.get_and_update!([a: 1], :a, fn value -> value end)
-    end
+    assert_raise RuntimeError,
+                 "the given function must return a two-element tuple or :pop, got: 1",
+                 fn ->
+                   Keyword.get_and_update!([a: 1], :a, fn value -> value end)
+                 end

What would be a preferred way to fix this one? If I am to asked, I'd keep the original version as it looks fine?

[josevalim]

@gmile move the message out!

msg = "... long string ..."
assert_raise RuntimeError, msg, fn ->
  ...
end

[gmile]

@josevalim thanks! done in #6690

[josevalim]

Folks running the formatter multiple times, please don't forget to frequently pull from master. :) Thank you!

[LostKobrakai]

And do not forget to recompile after pulling :D