Skip to content

Rework migrate options for the formatter #13841

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions .formatter.exs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,5 @@

# Float tests
float_assert: 1
],
normalize_bitstring_modifiers: false
]
]
36 changes: 27 additions & 9 deletions lib/elixir/lib/code.ex
Original file line number Diff line number Diff line change
Expand Up @@ -654,6 +654,8 @@ defmodule Code do

## Options

Regular options (do not change the AST):

* `:file` - the file which contains the string, used for error
reporting

Expand All @@ -677,28 +679,29 @@ defmodule Code do
If you set it to `false` later on, `do`-`end` blocks won't be
converted back to keywords.

* `:normalize_bitstring_modifiers` (since v1.14.0) - when `true`,
Migration options (change the AST), see the "Migration formatting" section below:

* `:migrate` (since v1.18.0) - when `true`, sets all other migration options
to `true` by default. Defaults to `false`.

* `:migrate_bitstring_modifiers` (since v1.18.0) - when `true`,
removes unnecessary parentheses in known bitstring
[modifiers](`<<>>/1`), for example `<<foo::binary()>>`
becomes `<<foo::binary>>`, or adds parentheses for custom
modifiers, where `<<foo::custom_type>>` becomes `<<foo::custom_type()>>`.
Defaults to `true`. This option changes the AST.
Defaults to the value of the `:migrate` option. This option changes the AST.

* `:normalize_charlists_as_sigils` (since v1.15.0) - when `true`,
* `:migrate_charlists_as_sigils` (since v1.18.0) - when `true`,
formats charlists as [`~c`](`Kernel.sigil_c/2`) sigils, for example
`'foo'` becomes `~c"foo"`.
Defaults to `true`. This option changes the AST.
Defaults to the value of the `:migrate` option. This option changes the AST.

## Design principles

The formatter was designed under three principles.

First, the formatter never changes the semantics of the code.
First, the formatter never changes the semantics of the code by default.
This means the input AST and the output AST are almost always equivalent.
The only cases where the formatter will change the AST is when the input AST
would cause *compiler warnings* and the output AST won't. The cases where
the formatter changes the AST can be disabled through formatting options
if desired.

The second principle is to provide as little configuration as possible.
This eases the formatter adoption by removing contention points while
Expand Down Expand Up @@ -986,6 +989,21 @@ defmodule Code do
## Newlines

The formatter converts all newlines in code from `\r\n` to `\n`.

## Migration formatting

As part of the Elixir release cycle, deprecations are being introduced,
emitting warnings which might require existing code to be changed.
In order to reduce the burden on developers when upgrading Elixir to the
next version, the formatter exposes some options, disabled by default,
in order to automate this process.

These options should address most of the typical use cases, but given they
introduce changes to the AST, there is a non-zero risk for meta-programming
heavy projects that relied on a specific AST, or projects that are
re-defining functions from the `Kernel`. In such cases, migrations cannot
be applied blindly and some extra changes might be needed in order to
address the deprecation warnings.
"""
@doc since: "1.6.0"
@spec format_string!(binary, keyword) :: iodata
Expand Down
15 changes: 8 additions & 7 deletions lib/elixir/lib/code/formatter.ex
Original file line number Diff line number Diff line change
Expand Up @@ -189,8 +189,9 @@ defmodule Code.Formatter do
locals_without_parens = Keyword.get(opts, :locals_without_parens, [])
file = Keyword.get(opts, :file, nil)
sigils = Keyword.get(opts, :sigils, [])
normalize_bitstring_modifiers = Keyword.get(opts, :normalize_bitstring_modifiers, true)
normalize_charlists_as_sigils = Keyword.get(opts, :normalize_charlists_as_sigils, true)
migrate = Keyword.get(opts, :migrate, false)
migrate_bitstring_modifiers = Keyword.get(opts, :migrate_bitstring_modifiers, migrate)
migrate_charlists_as_sigils = Keyword.get(opts, :migrate_charlists_as_sigils, migrate)
syntax_colors = Keyword.get(opts, :syntax_colors, [])

sigils =
Expand All @@ -215,8 +216,8 @@ defmodule Code.Formatter do
comments: comments,
sigils: sigils,
file: file,
normalize_bitstring_modifiers: normalize_bitstring_modifiers,
normalize_charlists_as_sigils: normalize_charlists_as_sigils,
migrate_bitstring_modifiers: migrate_bitstring_modifiers,
migrate_charlists_as_sigils: migrate_charlists_as_sigils,
inspect_opts: %Inspect.Opts{syntax_colors: syntax_colors}
}
end
Expand Down Expand Up @@ -1433,7 +1434,7 @@ defmodule Code.Formatter do
{doc, state} = quoted_to_algebra(segment, :parens_arg, state)

{spec, state} =
bitstring_spec_to_algebra(spec, state, state.normalize_bitstring_modifiers, :"::")
bitstring_spec_to_algebra(spec, state, state.migrate_bitstring_modifiers, :"::")

spec = wrap_in_parens_if_inspected_atom(spec)
spec = if i == last, do: bitstring_wrap_parens(spec, i, last), else: spec
Expand Down Expand Up @@ -2431,7 +2432,7 @@ defmodule Code.Formatter do
end

defp get_charlist_quotes(:heredoc, state) do
if state.normalize_charlists_as_sigils do
if state.migrate_charlists_as_sigils do
{@sigil_c_heredoc, @double_heredoc}
else
{@single_heredoc, @single_heredoc}
Expand All @@ -2440,7 +2441,7 @@ defmodule Code.Formatter do

defp get_charlist_quotes({:regular, chunks}, state) do
cond do
!state.normalize_charlists_as_sigils -> {@single_quote, @single_quote}
!state.migrate_charlists_as_sigils -> {@single_quote, @single_quote}
Enum.any?(chunks, &has_double_quote?/1) -> {@sigil_c_single, @single_quote}
true -> {@sigil_c_double, @double_quote}
end
Expand Down
17 changes: 7 additions & 10 deletions lib/elixir/lib/code/normalizer.ex
Original file line number Diff line number Diff line change
Expand Up @@ -288,21 +288,18 @@ defmodule Code.Normalizer do
# Lists
defp normalize_literal(list, meta, state) when is_list(list) do
if list != [] and List.ascii_printable?(list) do
# It's a charlist
list =
# It's a charlist, we normalize it as a ~C sigil
string =
if state.escape do
{string, _} = Code.Identifier.escape(IO.chardata_to_string(list), nil)
IO.iodata_to_binary(string) |> to_charlist()
{iolist, _} = Code.Identifier.escape(IO.chardata_to_string(list), nil)
IO.iodata_to_binary(iolist)
else
list
List.to_string(list)
end

meta =
meta
|> Keyword.put_new(:delimiter, "'")
|> patch_meta_line(state.parent_meta)
meta = patch_meta_line([delimiter: "\""], state.parent_meta)

{:__block__, meta, [list]}
{:sigil_c, meta, [{:<<>>, [], [string]}, []]}
else
meta =
if line = state.parent_meta[:line] do
Expand Down
4 changes: 3 additions & 1 deletion lib/elixir/lib/macro.ex
Original file line number Diff line number Diff line change
Expand Up @@ -1133,7 +1133,9 @@ defmodule Macro do
"""
@spec to_string(t()) :: String.t()
def to_string(tree) do
doc = Inspect.Algebra.format(Code.quoted_to_algebra(tree), 98)
doc =
Inspect.Algebra.format(Code.quoted_to_algebra(tree, migrate_charlists_as_sigils: true), 98)

IO.iodata_to_binary(doc)
end

Expand Down
28 changes: 5 additions & 23 deletions lib/elixir/test/elixir/code_formatter/containers_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -293,33 +293,15 @@ defmodule Code.Formatter.ContainersTest do
assert_same "<<(<<y>> <- x)>>"
end

test "normalizes bitstring modifiers by default" do
assert_format "<<foo::binary()>>", "<<foo::binary>>"
test "keeps parentheses by default" do
assert_same "<<foo::binary()>>"
assert_same "<<foo::binary>>"

assert_format "<<foo::custom_type>>", "<<foo::custom_type()>>"
assert_same "<<foo::custom_type>>"
assert_same "<<foo::custom_type()>>"

assert_format "<<x::binary()-(13 * 6)-custom>>", "<<x::binary-(13 * 6)-custom()>>"
assert_same "<<x::binary-(13 * 6)-custom()>>"
assert_same "<<0::size*unit, bytes::binary>>"
assert_format "<<0::size*unit, bytes::custom>>", "<<0::size*unit, bytes::custom()>>"

assert_format "<<0, 1::2-integer() <- x>>", "<<0, 1::2-integer <- x>>"
assert_same "<<0, 1::2-integer <- x>>"
end

test "keeps parentheses when normalize_bitstring_modifiers is false" do
opts = [normalize_bitstring_modifiers: false]

assert_same "<<foo::binary()>>", opts
assert_same "<<foo::binary>>", opts

assert_same "<<foo::custom_type>>", opts
assert_same "<<foo::custom_type()>>", opts

assert_same "<<x::binary()-(13 * 6)-custom>>", opts
assert_same "<<0, 1::2-integer() <- x>>", opts
assert_same "<<x::binary()-(13 * 6)-custom>>"
assert_same "<<0, 1::2-integer() <- x>>"
end

test "is flex on line limits" do
Expand Down
Loading
Loading