Skip to content

Commit f2b31a6

Browse files
author
José Valim
committed
Ensure do blocks do not exceed line length on single arguments
Closes #7232
1 parent 3b41c9f commit f2b31a6

File tree

9 files changed

+51
-14
lines changed

9 files changed

+51
-14
lines changed

lib/elixir/lib/code/formatter.ex

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,8 @@ defmodule Code.Formatter do
276276
defp next_eol('\r\n' ++ rest, count), do: next_eol(rest, count + 1)
277277
defp next_eol(_, count), do: count
278278

279-
defp previous_eol([{token, {_, _, count}} | _]) when token in [:eol, :",", :";"] and count > 0 do
279+
defp previous_eol([{token, {_, _, count}} | _])
280+
when token in [:eol, :",", :";"] and count > 0 do
280281
count
281282
end
282283

@@ -453,7 +454,8 @@ defmodule Code.Formatter do
453454
{atom_to_algebra(atom), state}
454455
end
455456

456-
defp quoted_to_algebra({:__block__, meta, [integer]}, _context, state) when is_integer(integer) do
457+
defp quoted_to_algebra({:__block__, meta, [integer]}, _context, state)
458+
when is_integer(integer) do
457459
{integer_to_algebra(Keyword.fetch!(meta, :original)), state}
458460
end
459461

@@ -915,7 +917,8 @@ defmodule Code.Formatter do
915917
# Mod.function()
916918
# var.function
917919
# expression.function(arguments)
918-
defp remote_to_algebra({{:., _, [target, fun]}, meta, args}, context, state) when is_atom(fun) do
920+
defp remote_to_algebra({{:., _, [target, fun]}, meta, args}, context, state)
921+
when is_atom(fun) do
919922
{target_doc, state} = remote_target_to_algebra(target, state)
920923
fun = remote_fun_to_algebra(target, fun, length(args), state)
921924
remote_doc = target_doc |> concat(".") |> concat(string(fun))
@@ -1082,6 +1085,10 @@ defmodule Code.Formatter do
10821085
&quoted_to_algebra(&1, context, &2)
10831086
)
10841087

1088+
# If we have a single argument, then we won't have an option to break
1089+
# before the "extra" part, so we ungroup it and build it later.
1090+
args_doc = ungroup_if_group(args_doc)
1091+
10851092
doc =
10861093
if skip_parens? do
10871094
" "
@@ -1196,7 +1203,8 @@ defmodule Code.Formatter do
11961203
["\n" | entries]
11971204
end
11981205

1199-
defp interpolation_to_algebra([entry | entries], escape, state, acc, last) when is_binary(entry) do
1206+
defp interpolation_to_algebra([entry | entries], escape, state, acc, last)
1207+
when is_binary(entry) do
12001208
acc = concat(acc, escape_string(entry, escape))
12011209
interpolation_to_algebra(entries, escape, state, acc, last)
12021210
end
@@ -2041,6 +2049,9 @@ defmodule Code.Formatter do
20412049

20422050
## Algebra helpers
20432051

2052+
defp ungroup_if_group({:doc_group, group, _mode}), do: group
2053+
defp ungroup_if_group(other), do: other
2054+
20442055
defp format_to_string(doc) do
20452056
doc |> Inspect.Algebra.format(:infinity) |> IO.iodata_to_binary()
20462057
end

lib/elixir/lib/gen_server.ex

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -885,7 +885,8 @@ defmodule GenServer do
885885
See `multi_call/4` for more information.
886886
"""
887887
@spec abcast([node], name :: atom, term) :: :abcast
888-
def abcast(nodes \\ [node() | Node.list()], name, request) when is_list(nodes) and is_atom(name) do
888+
def abcast(nodes \\ [node() | Node.list()], name, request)
889+
when is_list(nodes) and is_atom(name) do
889890
msg = cast_msg(request)
890891
_ = for node <- nodes, do: do_send({name, node}, msg)
891892
:abcast

lib/elixir/lib/inspect/algebra.ex

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -934,7 +934,7 @@ defmodule Inspect.Algebra do
934934
do: fits?(w, k, b?, [{apply_nesting(i, k, j), m, x} | t])
935935

936936
defp fits?(w, k, b?, [{i, m, doc_cons(x, y)} | t]),
937-
do: fits?(w, k, b?, [{i, m, x} | [{i, m, y} | t]])
937+
do: fits?(w, k, b?, [{i, m, x}, {i, m, y} | t])
938938

939939
defp fits?(w, k, b?, [{i, m, doc_group(x, _)} | t]),
940940
do: fits?(w, k, b?, [{i, m, x} | {:tail, b?, t}])
@@ -943,7 +943,7 @@ defmodule Inspect.Algebra do
943943
defp format(_, _, []), do: []
944944
defp format(w, k, [{_, _, :doc_nil} | t]), do: format(w, k, t)
945945
defp format(w, _, [{i, _, :doc_line} | t]), do: [indent(i) | format(w, i, t)]
946-
defp format(w, k, [{i, m, doc_cons(x, y)} | t]), do: format(w, k, [{i, m, x} | [{i, m, y} | t]])
946+
defp format(w, k, [{i, m, doc_cons(x, y)} | t]), do: format(w, k, [{i, m, x}, {i, m, y} | t])
947947
defp format(w, k, [{i, m, doc_color(x, c)} | t]), do: [ansi(c) | format(w, k, [{i, m, x} | t])]
948948
defp format(w, k, [{_, _, doc_string(s, l)} | t]), do: [s | format(w, k + l, t)]
949949
defp format(w, k, [{_, _, s} | t]) when is_binary(s), do: [s | format(w, k + byte_size(s), t)]

lib/elixir/lib/kernel.ex

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3330,7 +3330,8 @@ defmodule Kernel do
33303330
:lists.mapfoldl(fun, acc, list)
33313331
end
33323332

3333-
defp ensure_evaled_element(elem, acc) when is_number(elem) or is_atom(elem) or is_binary(elem) do
3333+
defp ensure_evaled_element(elem, acc)
3334+
when is_number(elem) or is_atom(elem) or is_binary(elem) do
33343335
{elem, acc}
33353336
end
33363337

lib/elixir/lib/map_set.ex

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,8 @@ defmodule MapSet do
150150
# If the first set is less than twice the size of the second map,
151151
# it is fastest to re-accumulate items in the first set that are not
152152
# present in the second set.
153-
def difference(%MapSet{map: map1}, %MapSet{map: map2}) when map_size(map1) < map_size(map2) * 2 do
153+
def difference(%MapSet{map: map1}, %MapSet{map: map2})
154+
when map_size(map1) < map_size(map2) * 2 do
154155
map =
155156
map1
156157
|> Map.keys()

lib/elixir/lib/module/locals_tracker.ex

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,8 @@ defmodule Module.LocalsTracker do
6161
@doc """
6262
Adds an import dispatch to the given target.
6363
"""
64-
def add_import(d, function, module, {name, arity}) when is_tuple(function) and is_atom(module) do
64+
def add_import(d, function, module, {name, arity})
65+
when is_tuple(function) and is_atom(module) do
6566
tuple = {:import, name, arity}
6667
put_edge(d, tuple, module)
6768
put_edge(d, function, tuple)

lib/elixir/lib/regex.ex

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -465,7 +465,8 @@ defmodule Regex do
465465
end
466466
end
467467

468-
def split(%Regex{re_pattern: compiled}, string, opts) when is_binary(string) and is_list(opts) do
468+
def split(%Regex{re_pattern: compiled}, string, opts)
469+
when is_binary(string) and is_list(opts) do
469470
on = Keyword.get(opts, :on, :first)
470471

471472
case :re.run(string, compiled, [:global, capture: on]) do
@@ -653,7 +654,8 @@ defmodule Regex do
653654
string
654655
end
655656

656-
defp apply_list(whole, string, pos, replacement, [[{mpos, _} | _] | _] = list) when mpos > pos do
657+
defp apply_list(whole, string, pos, replacement, [[{mpos, _} | _] | _] = list)
658+
when mpos > pos do
657659
length = mpos - pos
658660
<<untouched::binary-size(length), rest::binary>> = string
659661
[untouched | apply_list(whole, rest, mpos, replacement, list)]

lib/elixir/test/elixir/code_formatter/integration_test.exs

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -345,7 +345,7 @@ defmodule Code.Formatter.IntegrationTest do
345345
"""
346346
end
347347

348-
test "no parens keywords right on line limit" do
348+
test "no parens keywords at the end of the line" do
349349
bad = """
350350
defmodule Mod do
351351
defp token_list_downcase(<<char, rest::binary>>, acc) when is_whitespace(char) or is_comma(char), do: token_list_downcase(rest, acc)
@@ -382,6 +382,25 @@ defmodule Code.Formatter.IntegrationTest do
382382
assert_format bad, good, line_length: 18
383383
end
384384

385+
test "do at the end of the line with single argument" do
386+
bad = """
387+
defmodule Location do
388+
def new(line, column) when is_integer(line) and line >= 0 and is_integer(column) and column >= 0 do
389+
%{column: column, line: line}
390+
end
391+
end
392+
"""
393+
394+
assert_format bad, """
395+
defmodule Location do
396+
def new(line, column)
397+
when is_integer(line) and line >= 0 and is_integer(column) and column >= 0 do
398+
%{column: column, line: line}
399+
end
400+
end
401+
"""
402+
end
403+
385404
test "tuples as trees" do
386405
bad = """
387406
@document Parser.parse(

lib/logger/lib/logger.ex

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -605,7 +605,8 @@ defmodule Logger do
605605
"""
606606
@spec bare_log(level, message | (() -> message | {message, keyword}), keyword) ::
607607
:ok | {:error, :noproc} | {:error, term}
608-
def bare_log(level, chardata_or_fun, metadata \\ []) when level in @levels and is_list(metadata) do
608+
def bare_log(level, chardata_or_fun, metadata \\ [])
609+
when level in @levels and is_list(metadata) do
609610
case __metadata__() do
610611
{true, pdict} ->
611612
%{mode: mode, truncate: truncate, level: min_level, utc_log: utc_log?} =

0 commit comments

Comments
 (0)