What is considered relevant when the formatter calculates line lengths?

[mariusbutuc]

It seems to me that mix format currently overlooks some characters when calculating line lengths. This difference became apparent in contrast with line lengths calculated by credo.

Current behaviour

Below is the noticed behaviour and my current attempt at reconciling the two:

         xxxxxxxxxxx: %{
           xxxxxxxxxxxxxxxx: %{
             xxxx: get_in(xxxxxxxxxxx, [:xxxxxxx, :xxxxxxxxxxxxxxxx, :xxxx]),
+            # credo:disable-for-lines:2 Credo.Check.Readability.MaxLineLength
             xxxxxxxxxxxxxx:
-              get_in(xxxxxxxxxxx,
-                [:xxxxxxx, :xxxxxxxxxxxxxxxx, :xxxxxxxxxxxxxx]),
+              get_in(xxxxxxxxxxx, [:xxxxxxx, :xxxxxxxxxxxxxxxx, :xxxxxxxxxxxxxx]),
             xxxxxxxx:
               get_in(xxxxxxxxxxx, [:xxxxxxx, :xxxxxxxxxxxxxxxx, :xxxxxxxx])
           },

The old code wrapped around the 80 character limit. credo was happy, but mix format wants everything on one line, as the closing parenthesis and comma seem to not be counted in the line length.

This behaviour is currently encountered in an umbrella project running

• mix format
  • Elixir 1.6.2
  • Erlang/OTP 20.2.4
  • line_length set to 80 in .formatter.exs
• credo --strict
  • credo 0.8.10
  • max_length set to 80 in .credo.exs

    {Credo.Check.Readability.MaxLineLength, max_length: 80, ignore_specs: true}

• macOS

  $ sw_vers
  ProductName:  Mac OS X
  ProductVersion: 10.13.3
  BuildVersion: 17D102

---

FWIW, this behaviour was first brought up on the forum, confirmed as not intended, then migrated here.

[mariusbutuc]

@josevalim if the https://github.com/elixir-lang/elixir/commit/f0a7f19ff4023adf253d26edf1e817e303abb4c9 fix was included in 1.6.3, it looks like it does not resolve this problem:

$ git diff
diff --git a/.credo.exs b/.credo.exs
--- a/.credo.exs
+++ b/.credo.exs
@@ -15,8 +15,7 @@
           Credo.Check.Readability.MaxLineLength,
           ignore_specs: true,
           priority: :low,
-          # increase from 80 while `mix format` counts line length differently
-          max_length: 85
+          max_length: 80
diff --git a/.tool-versions b/.tool-versions
--- a/.tool-versions
+++ b/.tool-versions
@@ -1,2 +1,2 @@
-elixir 1.6.2-otp-20
+elixir 1.6.3-otp-20
 erlang 20.2.4

# $ cat .formatter.exs
# Used by "mix format"
[
  inputs: ["mix.exs", "apps/*/mix.exs", "apps/*/{config,lib,test}/**/*.{ex,exs}"],
  line_length: 80
]

$ iex --version
Erlang/OTP 20 [erts-9.2.1] [source] [64-bit] [smp:8:8] [ds:8:8:10] [async-threads:10] [hipe] [kernel-poll:false]

IEx 1.6.3 (compiled with OTP 20)

$ mix format

$ mix credo
Checking 53 source files ...

  Code Readability
┃
┃ [R] ↓ Line is too long (max is 80, was 83).
┃       apps/….ex:13:81 #(…)
┃ [R] ↓ Line is too long (max is 80, was 82).
┃       apps/….ex:67:81 #(…)

[josevalim]

@mariusbutuc can you please provide those examples? We will need to handle each case individually, there is no blanket rule, unfortunately.

[mariusbutuc]

@josevalim Good call, let me see how I can make them safely reproducible. 👍 Sorry I didn't do it earlier.

[mariusbutuc]

This is a very contrived example, more like pseudocode, but I hope it helps reproduce the issue:

# some_client.ex
defmodule SomeClient do
  @moduledoc false

  alias HTTPoison.{Error, Response}
  use HTTPoison.Base

  def foo(id) do
    with {:ok, res} <- get("api/some_endpoint", [], params: %{id: id}),
         %Response{body: %{relevant_bit: relevant_bit}, status_code: 200} <- res do
      {:ok, relevant_bit}
    else
      {:error, %Error{reason: reason}} ->
        {:error, message: reason}
    end
  end

  def bar(bazz) do
    %{
      key: %{
        foo: %{
          bar:
            get_in(bazz, [:one_key, :two_key, :three_key, :yet_another_key_here]),
          baz: get_in(bazz, [:one_key, :two_key, :three_key])
        }
      }
    }
  end
end

$ mix format

$ mix credo
Checking 54 source files ...

  Code Readability
┃
┃ [R] ↓ Line is too long (max is 80, was 82).
┃       apps/web/lib/some_client.ex:22:81 #(SomeClient.bar)
┃ [R] ↓ Line is too long (max is 80, was 83).
┃       apps/web/lib/some_client.ex:9:81 #(SomeClient.foo)

Elixir 1.6.3 + .formatter.exs details

```sh $ iex --version Erlang/OTP 20 [erts-9.2.1] [source] [64-bit] [smp:8:8] [ds:8:8:10] [async-threads:10] [hipe] [kernel-poll:false] IEx 1.6.3 (compiled with OTP 20) ``` ```elixir # .formatter.exs # Used by "mix format" [ inputs: ["mix.exs", "apps/*/mix.exs", "apps/*/{config,lib,test}/**/*.{ex,exs}"], line_length: 80 ] ```

[josevalim]

Pseudo-code is perfect fine. My formatter examples are often filled with long variables composed only of the letter a. :)

[fertapric]

Here is another example (from #7604):

The following code exceeds the default line length (line length 100, with default maximum line length 98), but the formatter does not split the function definition into multiple lines:

defmodule LineLength do
  def a_really_really_really_long_function_name(first_argument, second_argument, third__argument) do
    :ok
  end
end

However, if the name of the last argument is changed to third____argument, then the formatter works as expected:

defmodule LineLength do
  def a_really_really_really_long_function_name(
        first_argument,
        second_argument,
        third____argument
      ) do
    :ok
  end
end

Environment (Mac OS X, Elixir v1.6.4)

``` # $ elixir --version Erlang/OTP 20 [erts-9.3] [source] [64-bit] [smp:8:8] [ds:8:8:10] [async-threads:10] [hipe] [kernel-poll:false] [dtrace] Elixir 1.6.4 (compiled with OTP 20) ```

[josevalim]

Thanks @fertapric and @mariusbutuc. The cases you have provided indeed won't break because there is no opportunity for break after the line length. So the formatter may have do and ), after the line break. There is a brief introduction on how the formatter works here and it is related to how groups are handled.

I have updated the docs to be a bit more explicit about this fact, that we can't enforce line length (for many reasons, those two being more of them).

[mariusbutuc]

❤️ 💚 💙 💛 💜

@josevalim thanks for clarifying this!