Skip to content

Commit

Permalink
Deprecate struct update syntax
Browse files Browse the repository at this point in the history
The struct update syntax was added early in Elixir to help validate
at compile-time that the update keys were valid. However, for a couple
releases already, Elixir's static analysis can perform such validation
more reliably and find more error if you pattern match on the struct
when the variable is defined instead.

This deprecation simplifies the language and pushes developers to
better alternatives.

Closes #13974.
  • Loading branch information
josevalim committed Jan 18, 2025
1 parent 67b6f89 commit 83a70d7
Show file tree
Hide file tree
Showing 10 changed files with 50 additions and 196 deletions.
2 changes: 2 additions & 0 deletions lib/elixir/lib/exception.ex
Original file line number Diff line number Diff line change
Expand Up @@ -1387,6 +1387,8 @@ defmodule BadFunctionError do
end

defmodule BadStructError do
@moduledoc deprecated:
"This exception is deprecated alongside the struct update syntax that raises it"
defexception [:struct, :term]

@impl true
Expand Down
15 changes: 3 additions & 12 deletions lib/elixir/lib/kernel/special_forms.ex
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,8 @@ defmodule Kernel.SpecialForms do
%User{}
Underneath a struct is just a map with a `:__struct__` key
pointing to the `User` module:
Underneath a struct is a map with a `:__struct__` key pointing
to the `User` module, where the keys are validated at compile-time:
%User{} == %{__struct__: User, name: "john", age: 27}
Expand All @@ -118,16 +118,7 @@ defmodule Kernel.SpecialForms do
%User{full_name: "john doe"}
An update operation specific for structs is also available:
%User{user | age: 28}
Once again, the syntax above will guarantee the given keys
are valid at compilation time and it will guarantee at runtime
the given argument is a struct, failing with `BadStructError`
otherwise. The map update syntax can also be used for updating
structs, and it is useful when you want to update any struct,
regardless of their name, as long as they have matching fields:
The map update syntax can also be used for updating structs:
%{user | age: 28}
Expand Down
4 changes: 2 additions & 2 deletions lib/elixir/lib/map_set.ex
Original file line number Diff line number Diff line change
Expand Up @@ -440,8 +440,8 @@ defmodule MapSet do
defimpl Inspect do
import Inspect.Algebra

def inspect(map_set, opts) do
opts = %Inspect.Opts{opts | charlists: :as_lists}
def inspect(map_set, %Inspect.Opts{} = opts) do
opts = %{opts | charlists: :as_lists}
concat(["MapSet.new(", Inspect.List.inspect(MapSet.to_list(map_set), opts), ")"])
end
end
Expand Down
77 changes: 10 additions & 67 deletions lib/elixir/lib/module/types/expr.ex
Original file line number Diff line number Diff line change
Expand Up @@ -225,48 +225,23 @@ defmodule Module.Types.Expr do
end

# %Struct{map | ...}
# This syntax is deprecated, so we simply traverse.
def of_expr(
{:%, struct_meta, [module, {:%{}, _, [{:|, update_meta, [map, args]}]}]} = struct,
expected,
{:%, _, [_, {:%{}, _, [{:|, _, [map, args]}]}]} = struct,
_expected,
expr,
stack,
context
) do
if stack.mode == :traversal do
{_, context} = of_expr(map, term(), struct, stack, context)
{_, context} = of_expr(map, term(), struct, stack, context)

context =
Enum.reduce(args, context, fn {key, value}, context when is_atom(key) ->
{_, context} = of_expr(value, term(), expr, stack, context)
context
end)
context =
Enum.reduce(args, context, fn {key, value}, context when is_atom(key) ->
{_, context} = of_expr(value, term(), expr, stack, context)
context
end)

{dynamic(), context}
else
{info, context} = Of.struct_info(module, struct_meta, stack, context)
struct_type = Of.struct_type(module, info)
{map_type, context} = of_expr(map, struct_type, struct, stack, context)

if compatible?(map_type, struct_type) do
map_type = map_put!(map_type, :__struct__, atom([module]))

Enum.reduce(args, {map_type, context}, fn
{key, value}, {map_type, context} when is_atom(key) ->
# TODO: Once we support typed structs, we need to type check them here.
expected_value_type =
case map_fetch(expected, key) do
{_, expected_value_type} -> expected_value_type
_ -> term()
end

{value_type, context} = of_expr(value, expected_value_type, expr, stack, context)
{map_put!(map_type, key, value_type), context}
end)
else
warning = {:badstruct, struct, struct_type, map_type, context}
{error_type(), error(__MODULE__, warning, update_meta, stack, context)}
end
end
{dynamic(), context}
end

# %{...}
Expand Down Expand Up @@ -792,13 +767,6 @@ defmodule Module.Types.Expr do
defp flatten_when({:when, _meta, [left, right]}), do: [left | flatten_when(right)]
defp flatten_when(other), do: [other]

defp map_put!(map_type, key, value_type) do
case map_put(map_type, key, value_type) do
{:ok, descr} -> descr
error -> raise "unexpected #{inspect(error)}"
end
end

defp repack_match(left_expr, {:=, meta, [new_left, new_right]}),
do: repack_match({:=, meta, [left_expr, new_left]}, new_right)

Expand All @@ -807,31 +775,6 @@ defmodule Module.Types.Expr do

## Warning formatting

def format_diagnostic({:badstruct, expr, expected_type, actual_type, context}) do
traces = collect_traces(expr, context)

%{
details: %{typing_traces: traces},
message:
IO.iodata_to_binary([
"""
incompatible types in struct update:
#{expr_to_string(expr) |> indent(4)}
expected type:
#{to_quoted_string(expected_type) |> indent(4)}
but got type:
#{to_quoted_string(actual_type) |> indent(4)}
""",
format_traces(traces)
])
}
end

def format_diagnostic({:badmap, type, expr, context}) do
traces = collect_traces(expr, context)

Expand Down
1 change: 1 addition & 0 deletions lib/elixir/src/elixir_erl_pass.erl
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,7 @@ translate_struct(Ann, Name, {'%{}', _, [{'|', _, [Update, Assocs]}]}, S) ->
Map = {map, Ann, [{map_field_exact, Ann, {atom, Ann, '__struct__'}, {atom, Ann, Name}}]},

Match = {match, Ann, Var, Map},
%% Once this is removed, we should remove badstruct handling from elixir_erl_try
Error = {tuple, Ann, [{atom, Ann, badstruct}, {atom, Ann, Name}, Var]},

{TUpdate, TU} = translate(Update, Ann, VS),
Expand Down
43 changes: 23 additions & 20 deletions lib/elixir/src/elixir_map.erl
Original file line number Diff line number Diff line change
Expand Up @@ -15,21 +15,29 @@ expand_map(Meta, Args, S, E) ->
{{'%{}', Meta, EArgs}, SE, EE}.

expand_struct(Meta, Left, {'%{}', MapMeta, MapArgs}, S, #{context := Context} = E) ->
CleanMapArgs = clean_struct_key_from_map_args(Meta, MapArgs, E),
CleanMapArgs = delete_struct_key(Meta, MapArgs, E),
{[ELeft, ERight], SE, EE} = elixir_expand:expand_args([Left, {'%{}', MapMeta, CleanMapArgs}], S, E),

case validate_struct(ELeft, Context) of
true when is_atom(ELeft) ->
case extract_struct_assocs(Meta, ERight, E) of
{expand, MapMeta, Assocs} when Context /= match -> %% Expand
case ERight of
{'%{}', MapMeta, [{'|', _, [_, Assocs]}]} ->
%% The update syntax for structs is deprecated,
%% so we return only the update syntax downstream.
%% TODO: Remove me on Elixir v2.0
file_warn(MapMeta, ?key(E, file), ?MODULE, {deprecated_update, ELeft, ERight}),
_ = load_struct_info(Meta, ELeft, Assocs, EE),
{{'%', Meta, [ELeft, ERight]}, SE, EE};

{'%{}', MapMeta, Assocs} when Context /= match ->
AssocKeys = [K || {K, _} <- Assocs],
Struct = load_struct(Meta, ELeft, Assocs, EE),
Keys = ['__struct__'] ++ AssocKeys,
WithoutKeys = lists:sort(maps:to_list(maps:without(Keys, Struct))),
StructAssocs = elixir_quote:escape(WithoutKeys, none, false),
{{'%', Meta, [ELeft, {'%{}', MapMeta, StructAssocs ++ Assocs}]}, SE, EE};

{_, _, Assocs} -> %% Update or match
{'%{}', MapMeta, Assocs} ->
_ = load_struct_info(Meta, ELeft, Assocs, EE),
{{'%', Meta, [ELeft, ERight]}, SE, EE}
end;
Expand All @@ -46,12 +54,12 @@ expand_struct(Meta, Left, {'%{}', MapMeta, MapArgs}, S, #{context := Context} =
expand_struct(Meta, _Left, Right, _S, E) ->
file_error(Meta, E, ?MODULE, {non_map_after_struct, Right}).

clean_struct_key_from_map_args(Meta, [{'|', PipeMeta, [Left, MapAssocs]}], E) ->
[{'|', PipeMeta, [Left, clean_struct_key_from_map_assocs(Meta, MapAssocs, E)]}];
clean_struct_key_from_map_args(Meta, MapAssocs, E) ->
clean_struct_key_from_map_assocs(Meta, MapAssocs, E).
delete_struct_key(Meta, [{'|', PipeMeta, [Left, MapAssocs]}], E) ->
[{'|', PipeMeta, [Left, delete_struct_key_assoc(Meta, MapAssocs, E)]}];
delete_struct_key(Meta, MapAssocs, E) ->
delete_struct_key_assoc(Meta, MapAssocs, E).

clean_struct_key_from_map_assocs(Meta, Assocs, E) ->
delete_struct_key_assoc(Meta, Assocs, E) ->
case lists:keytake('__struct__', 1, Assocs) of
{value, _, CleanAssocs} ->
file_warn(Meta, ?key(E, file), ?MODULE, ignored_struct_key_in_struct),
Expand Down Expand Up @@ -110,16 +118,6 @@ validate_kv(Meta, KV, Original, #{context := Context} = E) ->
file_error(Meta, E, ?MODULE, {not_kv_pair, lists:nth(Index, Original)})
end, {1, #{}}, KV).

extract_struct_assocs(_, {'%{}', Meta, [{'|', _, [_, Assocs]}]}, _) ->
{update, Meta, delete_struct_key(Assocs)};
extract_struct_assocs(_, {'%{}', Meta, Assocs}, _) ->
{expand, Meta, delete_struct_key(Assocs)};
extract_struct_assocs(Meta, Other, E) ->
file_error(Meta, E, ?MODULE, {non_map_after_struct, Other}).

delete_struct_key(Assocs) ->
lists:keydelete('__struct__', 1, Assocs).

validate_struct({'^', _, [{Var, _, Ctx}]}, match) when is_atom(Var), is_atom(Ctx) -> true;
validate_struct({Var, _Meta, Ctx}, match) when is_atom(Var), is_atom(Ctx) -> true;
validate_struct(Atom, _) when is_atom(Atom) -> true;
Expand Down Expand Up @@ -301,4 +299,9 @@ format_error({invalid_key_for_struct, Key}) ->
io_lib:format("invalid key ~ts for struct, struct keys must be atoms, got: ",
['Elixir.Macro':to_string(Key)]);
format_error(ignored_struct_key_in_struct) ->
"key :__struct__ is ignored when using structs".
"key :__struct__ is ignored when using structs";
format_error({deprecated_update, Struct, MapUpdate}) ->
io_lib:format("the struct update syntax is deprecated:\n\n~ts\n\n"
"Instead, prefer to pattern matching on structs when the variable is first defined and "
"use the regular map update syntax instead:\n\n~ts\n",
['Elixir.Macro':to_string({'%', [], [Struct, MapUpdate]}), 'Elixir.Macro':to_string(MapUpdate)]).
3 changes: 1 addition & 2 deletions lib/elixir/test/elixir/calendar/datetime_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -483,8 +483,7 @@ defmodule DateTimeTest do
assert DateTime.to_unix(gregorian_0) == -62_167_219_200
assert DateTime.to_unix(Map.from_struct(gregorian_0)) == -62_167_219_200

min_datetime = %DateTime{gregorian_0 | year: -9999}

min_datetime = %{gregorian_0 | year: -9999}
assert DateTime.to_unix(min_datetime) == -377_705_116_800
end

Expand Down
4 changes: 0 additions & 4 deletions lib/elixir/test/elixir/map_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -283,12 +283,8 @@ defmodule MapTest do

test "structs" do
assert %ExternalUser{} == %{__struct__: ExternalUser, name: "john", age: 27}

assert %ExternalUser{name: "meg"} == %{__struct__: ExternalUser, name: "meg", age: 27}

user = %ExternalUser{}
assert %ExternalUser{user | name: "meg"} == %{__struct__: ExternalUser, name: "meg", age: 27}

%ExternalUser{name: name} = %ExternalUser{}
assert name == "john"
end
Expand Down
81 changes: 0 additions & 81 deletions lib/elixir/test/elixir/module/types/expr_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -875,87 +875,6 @@ defmodule Module.Types.ExprTest do
"""
end

test "updating structs" do
assert typecheck!([x], %Point{x | x: :zero}) ==
dynamic(
closed_map(__struct__: atom([Point]), x: atom([:zero]), y: term(), z: term())
)

assert typecheck!([x], %Point{%Point{x | x: :zero} | y: :one}) ==
dynamic(
closed_map(
__struct__: atom([Point]),
x: atom([:zero]),
y: atom([:one]),
z: term()
)
)

assert typeerror!(
(
x = %{x: 0}
%Point{x | x: :zero}
)
) ==
~l"""
incompatible types in struct update:
%Point{x | x: :zero}
expected type:
dynamic(%Point{x: term(), y: term(), z: term()})
but got type:
%{x: integer()}
where "x" was given the type:
# type: %{x: integer()}
# from: types_test.ex:LINE-4
x = %{x: 0}
"""
end

test "inference on struct update" do
assert typecheck!(
[x],
(
%Point{x | x: :zero}
x
)
) ==
dynamic(closed_map(__struct__: atom([Point]), x: term(), y: term(), z: term()))

assert typeerror!(
[x],
(
x.w
%Point{x | x: :zero}
)
) ==
~l"""
incompatible types in struct update:
%Point{x | x: :zero}
expected type:
dynamic(%Point{x: term(), y: term(), z: term()})
but got type:
dynamic(%{..., w: term()})
where "x" was given the type:
# type: dynamic(%{..., w: term()})
# from: types_test.ex:LINE-4
x.w
"""
end

test "nested map" do
assert typecheck!([x = %{}], x.foo.bar) == dynamic()
end
Expand Down
16 changes: 8 additions & 8 deletions lib/ex_unit/lib/ex_unit/diff.ex
Original file line number Diff line number Diff line change
Expand Up @@ -218,19 +218,19 @@ defmodule ExUnit.Diff do
# Guards

defp diff_guard({:when, _, [expression, clause]}, right, env) do
{diff_expression, post_env} = diff_quoted(expression, right, nil, env)
{diff, post_env} = diff_quoted(expression, right, nil, env)

{guard_clause, guard_equivalent?} =
if diff_expression.equivalent? do
if diff.equivalent? do
bindings = Map.merge(post_env.pins, post_env.current_vars)
diff_guard_clause(clause, bindings)
else
{clause, false}
end

diff = %__MODULE__{
diff_expression
| left: {:when, [], [diff_expression.left, guard_clause]},
diff = %{
diff
| left: {:when, [], [diff.left, guard_clause]},
equivalent?: guard_equivalent?
}

Expand Down Expand Up @@ -825,10 +825,10 @@ defmodule ExUnit.Diff do
end

defp diff_string_concat(left, nil, indexes, _left_length, right, env) do
{parsed_diff, parsed_post_env} = diff_string(left, right, ?", env)
left_diff = rebuild_concat_string(parsed_diff.left, nil, indexes)
{diff, parsed_post_env} = diff_string(left, right, ?", env)
left_diff = rebuild_concat_string(diff.left, nil, indexes)

diff = %__MODULE__{parsed_diff | left: left_diff}
diff = %{diff | left: left_diff}
{diff, parsed_post_env}
end

Expand Down

0 comments on commit 83a70d7

Please sign in to comment.