From b7fd998a2b3eb27789ce0a5c26e5502722ae54e9 Mon Sep 17 00:00:00 2001 From: makoto-developer <72484465+makoto-developer@users.noreply.github.com> Date: Sat, 8 Aug 2026 12:36:07 +0900 Subject: [PATCH 1/2] Only use do-end blocks when every trailing keyword is a block keyword Macro.to_string/1 rendered a trailing keyword list as do-end blocks as soon as its first key was do:, no matter what the other keys were. The output parses but means something different: Macro.to_string(quote do: for(cp <- gc, do: <>, into: "")) for cp <- gc do <> into "" end into is parsed back as a bare variable, so the comprehension no longer collects into a binary and the code does not compile. The pre-Code.Normalizer implementation required both that the list start with do: and that every key be a block keyword. It is still present in the deprecated Macro.to_string/2 and still gets this right: defp kw_blocks?([{:do, _} | _] = kw) do Enum.all?(kw, &match?({x, _} when x in unquote(kw_keywords), &1)) end Restore the second condition under the same name and guard both paths into normalize_kw_blocks/4 with it, including the one taken when the AST already carries do/end metadata. The metadata has to be dropped as well, since the formatter renders do-end from meta?(meta, :do) alone. Found by running every .ex and .exs file in this repository through parse |> Macro.to_string |> parse and comparing the resulting ASTs. Seven files that previously did not survive the round trip now do, including lib/elixir/lib/string.ex, and no file regressed. Closes #15724 Assisted-by: Claude Code:claude-opus-5 Signed-off-by: makoto-developer <72484465+makoto-developer@users.noreply.github.com> --- lib/elixir/lib/code/normalizer.ex | 19 +++++- .../code_normalizer/quoted_ast_test.exs | 60 +++++++++++++++++++ 2 files changed, 77 insertions(+), 2 deletions(-) diff --git a/lib/elixir/lib/code/normalizer.ex b/lib/elixir/lib/code/normalizer.ex index 76bc0712ac3..7656b79c505 100644 --- a/lib/elixir/lib/code/normalizer.ex +++ b/lib/elixir/lib/code/normalizer.ex @@ -4,6 +4,8 @@ defmodule Code.Normalizer do @moduledoc false + @do_end_keywords [:rescue, :catch, :else, :after] + defguard is_literal(x) when is_integer(x) or is_float(x) or @@ -347,18 +349,20 @@ defmodule Code.Normalizer do args = normalize_args(args, %{state | parent_meta: meta}) {form, meta, args} - Keyword.has_key?(meta, :do) -> + Keyword.has_key?(meta, :do) and kw_blocks?(last) -> # def foo do :ok end # def foo, do: :ok normalize_kw_blocks(form, meta, args, state) - match?([{:do, _} | _], last) and Keyword.keyword?(last) -> + match?([{:do, _} | _], last) and kw_blocks?(last) -> # Non normalized kw blocks line = state.parent_meta[:line] || meta[:line] meta = meta ++ [do: [line: line], end: [line: line]] normalize_kw_blocks(form, meta, args, state) true -> + # The formatter renders do-end blocks from the meta alone + meta = Keyword.drop(meta, [:do, :end]) args = normalize_args(args, %{state | parent_meta: meta}) {last_arg, leading_args} = List.pop_at(args, -1, []) @@ -397,6 +401,17 @@ defmodule Code.Normalizer do defp block_keyword?([]), do: true defp block_keyword?(_), do: false + # Anything after the do block that is not a block keyword makes it a keyword list + defp kw_blocks?([{:do, _} | rest] = kw) do + Keyword.keyword?(kw) and Enum.all?(rest, &match?({key, _} when key in @do_end_keywords, &1)) + end + + defp kw_blocks?([{{:__block__, _, [:do]}, _} | rest]) do + Enum.all?(rest, &match?({{:__block__, _, [key]}, _} when key in @do_end_keywords, &1)) + end + + defp kw_blocks?(_), do: false + defp allow_keyword?(:when, 2), do: true defp allow_keyword?(:{}, _), do: false defp allow_keyword?(op, arity), do: not is_atom(op) or not Macro.operator?(op, arity) diff --git a/lib/elixir/test/elixir/code_normalizer/quoted_ast_test.exs b/lib/elixir/test/elixir/code_normalizer/quoted_ast_test.exs index c10fdf041a2..2e46acb840f 100644 --- a/lib/elixir/test/elixir/code_normalizer/quoted_ast_test.exs +++ b/lib/elixir/test/elixir/code_normalizer/quoted_ast_test.exs @@ -636,6 +636,66 @@ defmodule Code.Normalizer.QuotedASTTest do assert quoted_to_string(quote(do: foo |> [bar: :baz])) == "foo |> [bar: :baz]" end + test "keyword args that start with do:" do + assert quoted_to_string(quote(do: foo(do: a, bar: b))) == "foo(do: a, bar: b)" + assert quoted_to_string(quote(do: foo(x, do: a, bar: b))) == "foo(x, do: a, bar: b)" + assert quoted_to_string(quote(do: Foo.bar(do: a, baz: b))) == "Foo.bar(do: a, baz: b)" + + assert quoted_to_string(quote(do: for(x <- y, do: x, into: ""))) == + ~S|for x <- y, do: x, into: ""| + + assert quoted_to_string({:foo, [], [[do: 1, do: 2]]}) == "foo(do: 1, do: 2)" + assert quoted_to_string(quote(do: foo(bar: b, do: a))) == "foo(bar: b, do: a)" + assert quoted_to_string({:foo, [], [[rescue: 1]]}) == "foo(rescue: 1)" + + assert quoted_to_string({:foo, [], [[do: {:__block__, [], [1, 2]}, bar: 3]]}) == + "foo(\n do:\n (\n 1\n 2\n ),\n bar: 3\n)" + + assert quoted_to_string({:case, [], [{:x, [], nil}, [do: [{:->, [], [[1], 2]}], other: 9]]}) == + "case x, do: (1 -> 2), other: 9" + + assert quoted_to_string(quote(do: foo(do: a))) == "foo do\n a\nend" + assert quoted_to_string(quote(do: foo(do: a, else: b))) == "foo do\n a\nelse\n b\nend" + assert quoted_to_string(quote(do: foo(do: a, catch: b))) == "foo do\n a\ncatch\n b\nend" + assert quoted_to_string(quote(do: foo(do: a, after: b))) == "foo do\n a\nafter\n b\nend" + + assert quoted_to_string(quote(do: foo(do: a, rescue: b, after: c))) == + "foo do\n a\nrescue\n b\nafter\n c\nend" + + assert quoted_to_string(quote(do: receive(do: (x -> x), after: (100 -> nil)))) == + "receive do\n x -> x\nafter\n 100 -> nil\nend" + end + + test "keyword args that start with do: with do/end in the metadata" do + meta = [do: [line: 1], end: [line: 1]] + + assert quoted_to_string({:foo, meta, [[do: 1, bar: 2]]}) == "foo(do: 1, bar: 2)" + assert quoted_to_string({:foo, meta, [[do: 1, else: 2]]}) == "foo do\n 1\nelse\n 2\nend" + end + + test "keyword args that start with do: are not forced into do-end blocks" do + assert quoted_to_string({:foo, [], [[do: 1, bar: 2]]}, force_do_end_blocks: true) == + "foo(do: 1, bar: 2)" + + assert quoted_to_string({:foo, [], [[do: 1, else: 2]]}, force_do_end_blocks: true) == + "foo do\n 1\nelse\n 2\nend" + end + + test "keyword args that start with do: round-trip through the parser" do + asts = [ + quote(do: foo(do: a, bar: b)), + quote(do: for(x <- y, do: x, into: "")), + quote(do: foo(do: a, else: b)), + {:foo, [], [[do: 1, do: 2]]}, + {:foo, [do: [line: 1], end: [line: 1]], [[do: 1, bar: 2]]} + ] + + for ast <- asts do + string = quoted_to_string(ast) + assert string |> Code.string_to_quoted!() |> quoted_to_string() == string + end + end + test "keyword arg with cursor" do input = "def foo, do: :bar, __cursor__()" expected = "def foo, [{:do, :bar}, __cursor__()]" From 1874fb7387e85c618bfe2e550b3c3683b85fe45c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Sat, 8 Aug 2026 08:38:12 +0200 Subject: [PATCH 2/2] Apply suggestion from @josevalim --- .../test/elixir/code_normalizer/quoted_ast_test.exs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/lib/elixir/test/elixir/code_normalizer/quoted_ast_test.exs b/lib/elixir/test/elixir/code_normalizer/quoted_ast_test.exs index 2e46acb840f..0d3370e0aae 100644 --- a/lib/elixir/test/elixir/code_normalizer/quoted_ast_test.exs +++ b/lib/elixir/test/elixir/code_normalizer/quoted_ast_test.exs @@ -639,21 +639,14 @@ defmodule Code.Normalizer.QuotedASTTest do test "keyword args that start with do:" do assert quoted_to_string(quote(do: foo(do: a, bar: b))) == "foo(do: a, bar: b)" assert quoted_to_string(quote(do: foo(x, do: a, bar: b))) == "foo(x, do: a, bar: b)" - assert quoted_to_string(quote(do: Foo.bar(do: a, baz: b))) == "Foo.bar(do: a, baz: b)" - - assert quoted_to_string(quote(do: for(x <- y, do: x, into: ""))) == - ~S|for x <- y, do: x, into: ""| + assert quoted_to_string(quote(do: foo(bar: b, do: a))) == "foo(bar: b, do: a)" assert quoted_to_string({:foo, [], [[do: 1, do: 2]]}) == "foo(do: 1, do: 2)" - assert quoted_to_string(quote(do: foo(bar: b, do: a))) == "foo(bar: b, do: a)" assert quoted_to_string({:foo, [], [[rescue: 1]]}) == "foo(rescue: 1)" assert quoted_to_string({:foo, [], [[do: {:__block__, [], [1, 2]}, bar: 3]]}) == "foo(\n do:\n (\n 1\n 2\n ),\n bar: 3\n)" - assert quoted_to_string({:case, [], [{:x, [], nil}, [do: [{:->, [], [[1], 2]}], other: 9]]}) == - "case x, do: (1 -> 2), other: 9" - assert quoted_to_string(quote(do: foo(do: a))) == "foo do\n a\nend" assert quoted_to_string(quote(do: foo(do: a, else: b))) == "foo do\n a\nelse\n b\nend" assert quoted_to_string(quote(do: foo(do: a, catch: b))) == "foo do\n a\ncatch\n b\nend" @@ -662,6 +655,9 @@ defmodule Code.Normalizer.QuotedASTTest do assert quoted_to_string(quote(do: foo(do: a, rescue: b, after: c))) == "foo do\n a\nrescue\n b\nafter\n c\nend" + assert quoted_to_string({:case, [], [{:x, [], nil}, [do: [{:->, [], [[1], 2]}], other: 9]]}) == + "case x, do: (1 -> 2), other: 9" + assert quoted_to_string(quote(do: receive(do: (x -> x), after: (100 -> nil)))) == "receive do\n x -> x\nafter\n 100 -> nil\nend" end