Formatter fix: next_break_fits respects line_length (#13792)

* Refactor fits to return :fit | :no_fit

* Formatter fix: next_break_fits respects line_length

Author: sabiwara

lib/elixir/lib/inspect/algebra.ex

@@ -973,20 +973,17 @@ defmodule Inspect.Algebra do
   #
   #   * flat - represents a document with breaks as flats (a break may fit, as it may break)
   #   * break - represents a document with breaks as breaks (a break always fits, since it breaks)
-  #
-  # The following modes are exclusive to fitting:
-  #
   #   * flat_no_break - represents a document with breaks as flat not allowed to enter in break mode
   #   * break_no_flat - represents a document with breaks as breaks not allowed to enter in flat mode
   #
   @typep mode :: :flat | :flat_no_break | :break | :break_no_flat
 
-  @spec fits?(
+  @spec fits(
           width :: non_neg_integer() | :infinity,
           column :: non_neg_integer(),
           break? :: boolean(),
           entries
-        ) :: boolean()
+        ) :: :fit | :no_fit | :break_next
         when entries:
                maybe_improper_list({integer(), mode(), t()}, {:tail, boolean(), entries} | [])
 
@@ -995,63 +992,68 @@ defmodule Inspect.Algebra do
   #
   # In case we have groups and the group fits, we need to consider the group
   # parent without the child breaks, hence {:tail, b?, t} below.
-  defp fits?(w, k, b?, _) when k > w and b?, do: false
-  defp fits?(_, _, _, []), do: true
-  defp fits?(w, k, _, {:tail, b?, t}), do: fits?(w, k, b?, t)
+  defp fits(w, k, b?, _) when k > w and b?, do: :no_fit
+  defp fits(_, _, _, []), do: :fit
+  defp fits(w, k, _, {:tail, b?, t}), do: fits(w, k, b?, t)
 
   ## Flat no break
 
-  defp fits?(w, k, b?, [{i, _, doc_fits(x, :disabled)} | t]),
-    do: fits?(w, k, b?, [{i, :flat_no_break, x} | t])
+  defp fits(w, k, b?, [{i, _, doc_fits(x, :disabled)} | t]),
+    do: fits(w, k, b?, [{i, :flat_no_break, x} | t])
 
-  defp fits?(w, k, b?, [{i, :flat_no_break, doc_fits(x, _)} | t]),
-    do: fits?(w, k, b?, [{i, :flat_no_break, x} | t])
+  defp fits(w, k, b?, [{i, :flat_no_break, doc_fits(x, _)} | t]),
+    do: fits(w, k, b?, [{i, :flat_no_break, x} | t])
 
   ## Breaks no flat
 
-  defp fits?(w, k, b?, [{i, _, doc_fits(x, :enabled)} | t]),
-    do: fits?(w, k, b?, [{i, :break_no_flat, x} | t])
+  defp fits(w, k, b?, [{i, _, doc_fits(x, :enabled)} | t]),
+    do: fits(w, k, b?, [{i, :break_no_flat, x} | t])
 
-  defp fits?(w, k, b?, [{i, :break_no_flat, doc_force(x)} | t]),
-    do: fits?(w, k, b?, [{i, :break_no_flat, x} | t])
+  defp fits(w, k, b?, [{i, :break_no_flat, doc_force(x)} | t]),
+    do: fits(w, k, b?, [{i, :break_no_flat, x} | t])
 
-  defp fits?(_, _, _, [{_, :break_no_flat, doc_break(_, _)} | _]), do: true
-  defp fits?(_, _, _, [{_, :break_no_flat, :doc_line} | _]), do: true
+  defp fits(w, k, b?, [{i, :break_no_flat, x} | t])
+       when x == :doc_line or (is_tuple(x) and elem(x, 0) == :doc_break) do
+    case fits(w, k, b?, [{i, :flat, x} | t]) do
+      :no_fit -> :break_next
+      fits -> fits
+    end
+  end
 
   ## Breaks
 
-  defp fits?(_, _, _, [{_, :break, doc_break(_, _)} | _]), do: true
-  defp fits?(_, _, _, [{_, :break, :doc_line} | _]), do: true
+  defp fits(_, _, _, [{_, :break, doc_break(_, _)} | _]), do: :fit
+  defp fits(_, _, _, [{_, :break, :doc_line} | _]), do: :fit
 
-  defp fits?(w, k, b?, [{i, :break, doc_group(x, _)} | t]),
-    do: fits?(w, k, b?, [{i, :flat, x} | {:tail, b?, t}])
+  defp fits(w, k, b?, [{i, :break, doc_group(x, _)} | t]),
+    do: fits(w, k, b?, [{i, :flat, x} | {:tail, b?, t}])
 
   ## Catch all
 
-  defp fits?(w, _, _, [{i, _, :doc_line} | t]), do: fits?(w, i, false, t)
-  defp fits?(w, k, b?, [{_, _, :doc_nil} | t]), do: fits?(w, k, b?, t)
-  defp fits?(w, _, b?, [{i, _, doc_collapse(_)} | t]), do: fits?(w, i, b?, t)
-  defp fits?(w, k, b?, [{i, m, doc_color(x, _)} | t]), do: fits?(w, k, b?, [{i, m, x} | t])
-  defp fits?(w, k, b?, [{_, _, doc_string(_, l)} | t]), do: fits?(w, k + l, b?, t)
-  defp fits?(w, k, b?, [{_, _, s} | t]) when is_binary(s), do: fits?(w, k + byte_size(s), b?, t)
-  defp fits?(_, _, _, [{_, _, doc_force(_)} | _]), do: false
-  defp fits?(w, k, _, [{_, _, doc_break(s, _)} | t]), do: fits?(w, k + byte_size(s), true, t)
-  defp fits?(w, k, b?, [{i, m, doc_nest(x, _, :break)} | t]), do: fits?(w, k, b?, [{i, m, x} | t])
+  defp fits(w, _, _, [{i, _, :doc_line} | t]), do: fits(w, i, false, t)
+  defp fits(w, k, b?, [{_, _, :doc_nil} | t]), do: fits(w, k, b?, t)
+  defp fits(w, _, b?, [{i, _, doc_collapse(_)} | t]), do: fits(w, i, b?, t)
+  defp fits(w, k, b?, [{i, m, doc_color(x, _)} | t]), do: fits(w, k, b?, [{i, m, x} | t])
+  defp fits(w, k, b?, [{_, _, doc_string(_, l)} | t]), do: fits(w, k + l, b?, t)
+  defp fits(w, k, b?, [{_, _, s} | t]) when is_binary(s), do: fits(w, k + byte_size(s), b?, t)
+  defp fits(_, _, _, [{_, _, doc_force(_)} | _]), do: :no_fit
+  defp fits(w, k, _, [{_, _, doc_break(s, _)} | t]), do: fits(w, k + byte_size(s), true, t)
+  defp fits(w, k, b?, [{i, m, doc_nest(x, _, :break)} | t]), do: fits(w, k, b?, [{i, m, x} | t])
 
-  defp fits?(w, k, b?, [{i, m, doc_nest(x, j, _)} | t]),
-    do: fits?(w, k, b?, [{apply_nesting(i, k, j), m, x} | t])
+  defp fits(w, k, b?, [{i, m, doc_nest(x, j, _)} | t]),
+    do: fits(w, k, b?, [{apply_nesting(i, k, j), m, x} | t])
 
-  defp fits?(w, k, b?, [{i, m, doc_cons(x, y)} | t]),
-    do: fits?(w, k, b?, [{i, m, x}, {i, m, y} | t])
+  defp fits(w, k, b?, [{i, m, doc_cons(x, y)} | t]),
+    do: fits(w, k, b?, [{i, m, x}, {i, m, y} | t])
 
-  defp fits?(w, k, b?, [{i, m, doc_group(x, _)} | t]),
-    do: fits?(w, k, b?, [{i, m, x} | {:tail, b?, t}])
+  defp fits(w, k, b?, [{i, m, doc_group(x, _)} | t]),
+    do: fits(w, k, b?, [{i, m, x} | {:tail, b?, t}])
 
-  defp fits?(w, k, b?, [{i, m, doc_limit(x, :infinity)} | t]) when w != :infinity,
-    do: fits?(:infinity, k, b?, [{i, :flat, x}, {i, m, doc_limit(empty(), w)} | t])
+  defp fits(w, k, b?, [{i, m, doc_limit(x, :infinity)} | t]) when w != :infinity,
+    do: fits(:infinity, k, b?, [{i, :flat, x}, {i, m, doc_limit(empty(), w)} | t])
 
-  defp fits?(_w, k, b?, [{i, m, doc_limit(x, w)} | t]),
-    do: fits?(w, k, b?, [{i, m, x} | t])
+  defp fits(_w, k, b?, [{i, m, doc_limit(x, w)} | t]),
+    do: fits(w, k, b?, [{i, m, x} | t])
 
   @spec format(
           width :: non_neg_integer() | :infinity,
@@ -1066,14 +1068,18 @@ defmodule Inspect.Algebra do
   defp format(w, k, [{_, _, doc_string(s, l)} | t]), do: [s | format(w, k + l, t)]
   defp format(w, k, [{_, _, s} | t]) when is_binary(s), do: [s | format(w, k + byte_size(s), t)]
   defp format(w, k, [{i, m, doc_force(x)} | t]), do: format(w, k, [{i, m, x} | t])
+
+  defp format(w, k, [{i, :flat_no_break, doc_fits(x, :enabled)} | t]),
+    do: format(w, k, [{i, :break_no_flat, x} | t])
+
   defp format(w, k, [{i, m, doc_fits(x, _)} | t]), do: format(w, k, [{i, m, x} | t])
   defp format(w, _, [{i, _, doc_collapse(max)} | t]), do: collapse(format(w, i, t), max, 0, i)
 
   # Flex breaks are not conditional to the mode
   defp format(w, k, [{i, m, doc_break(s, :flex)} | t]) do
     k = k + byte_size(s)
 
-    if w == :infinity or m == :flat or fits?(w, k, true, t) do
+    if w == :infinity or m == :flat or fits(w, k, true, t) != :no_fit do
       [s | format(w, k, t)]
     else
       [indent(i) | format(w, i, t)]
@@ -1103,11 +1109,17 @@ defmodule Inspect.Algebra do
     format(w, k, [{i, :break, x} | t])
   end
 
+  defp format(w, k, [{i, :break_no_flat, doc_group(x, _)} | t]) do
+    format(w, k, [{i, :break, x} | t])
+  end
+
   defp format(w, k, [{i, _, doc_group(x, _)} | t]) do
-    if w == :infinity or fits?(w, k, false, [{i, :flat, x}]) do
-      format(w, k, [{i, :flat, x} | t])
-    else
-      format(w, k, [{i, :break, x} | t])
+    fits = if w == :infinity, do: :fit, else: fits(w, k, false, [{i, :flat, x}])
+
+    case fits do
+      :fit -> format(w, k, [{i, :flat, x} | t])
+      :no_fit -> format(w, k, [{i, :break, x} | t])
+      :break_next -> format(w, k, [{i, :flat_no_break, x} | t])
     end
   end
 

lib/elixir/test/elixir/code_formatter/calls_test.exs

@@ -165,6 +165,11 @@ defmodule Code.Formatter.CallsTest do
 
       assert_format bad, good, @short_length
     end
+
+    test "for maps" do
+      assert_same "a(%{x: 1})", @short_length
+      assert_format "ab(%{x: 1})", "ab(%{\n  x: 1\n})", @short_length
+    end
   end
 
   describe "local calls" do

lib/elixir/test/elixir/config/provider_test.exs

@@ -41,8 +41,9 @@ defmodule Config.ProviderTest do
              {:elixir, [:unknown, :nested, :key], {:ok, :value}}
            ]) == :ok
 
-    assert Config.Provider.validate_compile_env([{:elixir, [:unknown, :nested, :unknown], :error}]) ==
-             :ok
+    assert Config.Provider.validate_compile_env([
+             {:elixir, [:unknown, :nested, :unknown], :error}
+           ]) == :ok
 
     assert {:error, msg} =
              Config.Provider.validate_compile_env([{:elixir, [:unknown, :nested], :error}])