Skip to content

Only use do-end blocks when every trailing keyword is a block keyword - #15725

Merged
josevalim merged 2 commits into
elixir-lang:mainfrom
makoto-developer:fix-do-block-keywords-v2
Aug 8, 2026
Merged

Only use do-end blocks when every trailing keyword is a block keyword#15725
josevalim merged 2 commits into
elixir-lang:mainfrom
makoto-developer:fix-do-block-keywords-v2

Conversation

@makoto-developer

Copy link
Copy Markdown
Contributor

Closes #15724

Macro.to_string/1 renders a trailing keyword list as do-end blocks as soon as its
first key is do:, regardless of what the remaining keys are. The output parses, but
it no longer means the same thing:

Macro.to_string(quote do: for(cp <- gc, do: <<cp::utf8>>, into: ""))
for cp <- gc do
  <<cp::utf8>>
into
  ""
end

into is parsed back as a bare variable, so the comprehension stops collecting into a
binary and the code does not compile.

The pre-Code.Normalizer implementation guarded on two conditions — the list starts
with do: and every key is a block keyword. It is still there, in the deprecated
Macro.to_string/2 (lib/elixir/lib/macro.ex:1534-1541), and it still gets this
right:

defp kw_blocks?([{:do, _} | _] = kw) do
  Enum.all?(kw, &match?({x, _} when x in unquote(kw_keywords), &1))
end

Only the first condition was carried over into Code.Normalizer. This restores the
second one under the same name, and applies it to both paths into
normalize_kw_blocks/4:

  • the Keyword.has_key?(meta, :do) branch, reached when the AST already carries
    do/end metadata (what Code.string_to_quoted(token_metadata: true) produces, so
    tools that parse code and then add an option to a call hit it);
  • the branch for keyword lists that are not yet normalized.

The metadata has to be dropped along with it. The formatter decides on do-end from
meta?(meta, :do) alone (code/formatter.ex:1353), so leaving :do/:end in place
would keep producing blocks no matter what the normalizer decided.

@do_end_keywords is the same list Code.Formatter uses for
can_force_do_end_blocks?/2 (code/formatter.ex:156), and macro.ex:1535 has a third
copy. Should these be shared, or is the duplication fine? elixir_compiler.erl:237
compiles code/formatter.ex before code/normalizer.ex and the normalizer already
depends on Code.Formatter, so sharing is possible — I did not want to add a public
function without asking.

Verification

  • New and extended tests in code_normalizer/quoted_ast_test.exs cover both
    directions and both paths: keyword lists with a non-block key stay keyword lists
    (including when do/end are in the metadata), and lists of only block keywords
    still render as do-end. They fail without the change.
  • Full suite green: make test — elixir 7503, ex_unit 465, logger 165, eex 118,
    iex 289, mix 941.
  • Running every .ex/.exs file under lib/*/{lib,test} (467 files, excluding mix test fixtures) through
    parse |> Macro.to_string |> parse and comparing ASTs: 63 files failed before,
    56 after
    . The 7 fixed are lib/elixir/lib/string.ex and six Mix internals
    (mix/state.ex, mix/project.ex, mix/dep.ex, mix/tasks/xref.ex,
    mix/compilers/protocol.ex, mix/compilers/test.ex). One further file,
    kernel/expansion_test.exs, goes from unparseable to parseable. No file
    regressed.
    The remaining 56 are unrelated round-trip differences.

Notes

  • mix format is unaffected — Code.format_string!/2 does not go through
    Code.Normalizer. The blast radius is Macro.to_string/1 and
    Code.quoted_to_algebra/2.
  • There is no regression in the other direction. elixir_tokenizer.erl:1681-1684
    tokenizes exactly after/else/catch/rescue as block keywords, so a list that
    should render as do-end cannot fall through to the keyword-list form.
  • The output is not reordered: for x <- y, do: x, into: "" keeps the original key
    order even though into: before do: is the more idiomatic spelling. Reordering
    keys is out of scope here.
  • With a multi-statement do body and a non-block key, the keyword-list form is
    verbose:
    foo(
      do:
        (
          a()
          b()
        ),
      bar: 1
    )
    It is still better than the previous output, which did not parse back to the same AST.
  • code.ex:759-764 documents :force_do_end_blocks as converting "all keywords" into
    do-end blocks. That was already inaccurate for Code.format_string!/2; after this
    change it is also inaccurate for Code.quoted_to_algebra/2
    (Code.quoted_to_algebra({:foo, [], [[do: 1, bar: 2]]}, force_do_end_blocks: true)
    stays a keyword list). Happy to fix the doc here or in a separate PR.
  • No CHANGELOG entry: the five previous normalizer fixes that changed
    Macro.to_string/1 output (09d43562e Fix edge case in normalizer for keyword args #13924, bc50d9494 Fix Code.Normalizer for keyword operand with :do key #13250, 673fe4a89
    Fix crash in Macro.to_string #13905, e54b87c18 Fix formatter adding extra escapes to remote call functions #13960, 5b8b8e358 Reintroduce escaped trailing newlines in heredocs in Macro.to_string/2 #15355) all shipped without one, and the old
    output here does not parse, so nothing could have depended on it.

Assisted-by: Claude Code:claude-opus-5

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: <<cp::utf8>>, into: ""))

    for cp <- gc do
      <<cp::utf8>>
    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 elixir-lang#15724

Assisted-by: Claude Code:claude-opus-5
Signed-off-by: makoto-developer <72484465+makoto-developer@users.noreply.github.com>
@makoto-developer
makoto-developer force-pushed the fix-do-block-keywords-v2 branch from 7ed8834 to b7fd998 Compare August 8, 2026 03:53
Comment thread lib/elixir/test/elixir/code_normalizer/quoted_ast_test.exs
@josevalim
josevalim merged commit 0bd52d4 into elixir-lang:main Aug 8, 2026
15 checks passed
@josevalim

Copy link
Copy Markdown
Member

💚 💙 💜 💛 ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Macro.to_string/1 turns trailing keywords into do-end block tags, changing their meaning

2 participants