-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Description
Elixir and Erlang/OTP versions
Erlang/OTP 28 [erts-16.1] [source] [64-bit] [smp:8:8] [ds:8:8:10] [async-threads:1] [jit]
Elixir 1.19.3 (compiled with Erlang/OTP 28)
Operating system
Ubuntu 22.04.3 LTS / Alpine 3.22 (erlang:28.1.0.0-alpine) (not urgent)
Current behavior
First thanks, the update to 1.19.3-otp-28 uncovered a bunch of type warnings that made sense and I was able to clear a decently sized app from any of those warnings easily, except for the tests (without doing the bare map update syntax), which is great.
The issue is more of an heads up and the TLDR is that, at least in test files, type violations show even when using explicit pattern matching.
There's 2 examples, both of them can be solved by using the bare map update syntax in the last assignment as suggested, but I still think, perhaps, it's worth looking into as it seems wrong. It occurred to me that this might be because these are in test files, because some issues I had with the codebase and the new type errors were solved using basically what I am trying to do here if I recall correctly (I didn't keep track as they were straightforward and made sense).
Example 1:
warning: a struct for Schemas.Scroll is expected on struct update:
base_tests-1 |
base_tests-1 | %Schemas.Scroll{scroll | gid: bearer_gid}
base_tests-1 |
base_tests-1 | but got type:
base_tests-1 |
base_tests-1 | dynamic()
base_tests-1 |
base_tests-1 | where "scroll" was given the type:
base_tests-1 |
base_tests-1 | # type: dynamic()
base_tests-1 | # from: test/server/unit/scrolls/light_test.exs:696:5
base_tests-1 | %{
base_tests-1 | duel: %Schemas.Duel{id: id, game: %Schemas.Duel.Game{id_seq: id_seq_0} = game} = duel,
base_tests-1 | active_ctx: %Schemas.Duel.Game.Context{id: active_id, player: active_player},
base_tests-1 | opponent_ctx: %Schemas.Duel.Game.Context{player: opponent_player},
base_tests-1 | cast_cost: %{"light_bearer" => {%Schemas.Scroll{} = scroll, payment}}
base_tests-1 | }
base_tests-1 |
base_tests-1 | when defining the variable "scroll", you must also pattern match on "%Schemas.Scroll{}".
base_tests-1 |
base_tests-1 | hint: given pattern matching is enough to catch typing errors, you may optionally convert the struct update into a map update. For example, instead of:
base_tests-1 |
base_tests-1 | user = some_function()
base_tests-1 | %User{user | name: "John Doe"}
base_tests-1 |
base_tests-1 | it is enough to write:
base_tests-1 |
base_tests-1 | %User{} = user = some_function()
base_tests-1 | %{user | name: "John Doe"}
base_tests-1 |
base_tests-1 | typing violation found at:
base_tests-1 | │
base_tests-1 | 907 │ %Scroll{scroll | gid: bearer_gid},
base_tests-1 | │ ~
base_tests-1 | │
base_tests-1 | └─ test/server/unit/scrolls/light_test.exs:907:16: Scrolls.Light.Test."test light_bearer"/1
The code in question is in a test file (ommitted irrelevant bits), the offending match is in the last key of the 2nd argument, cast_cost:
test(
"light_bearer",
%{
duel: %Duel{id: id, game: %Game{id_seq: id_seq_0} = game} = duel,
active_ctx: %Context{id: active_id, player: active_player},
opponent_ctx: %Context{player: opponent_player},
cast_cost: %{"light_bearer" => {%Scroll{} = scroll, payment}}
}
) do
#......
#......
assert %Engine{
game:
%Game{
status: %Status{
player_active: ^active_player,
player_turn: ^active_player,
stack: []
}
} = game_10
} =
TestH.execute_gift(
duel_3,
%Scroll{scroll | gid: bearer_gid},
[],
:creatures,
0,
active_ctx_2,
opponent_ctx_2
)
There's similar occurrences:
base_tests-1 | warning: a struct for Schemas.Scroll is expected on struct update:
base_tests-1 |
base_tests-1 | %Schemas.Scroll{scroll | gid: id_seq}
base_tests-1 |
base_tests-1 | but got type:
base_tests-1 |
base_tests-1 | dynamic()
base_tests-1 |
base_tests-1 | where "scroll" was given the type:
base_tests-1 |
base_tests-1 | # type: dynamic()
base_tests-1 | # from: test/support/helpers/duels.ex:476
base_tests-1 | right = scroll = Scrollrack.get_scroll(slug_or_id)
base_tests-1 |
base_tests-1 | when defining the variable "scroll", you must also pattern match on "%Schemas.Scroll{}".
base_tests-1 |
.................
base_tests-1 |
base_tests-1 | typing violation found at:
base_tests-1 | │
base_tests-1 | 477 │ scroll_w_gid = %Scroll{scroll | gid: id_seq}
base_tests-1 | │
base_tests-1 | │
base_tests-1 | └─ (server 0.1.0) test/support/helpers/duels.ex:477:20: Server.Tests.Helpers.Duels.add_hex/3
def add_hex(
%Game{id_seq: id_seq} = game,
player,
slug_or_id
) do
assert %Scroll{} = scroll = Scrollrack.get_scroll(slug_or_id)
scroll_w_gid = %Scroll{scroll | gid: id_seq}
# ....
Same if I add the %Scroll{} struct as an explicit match:
typing violation found at:
base_tests-1 | │
base_tests-1 | 477 │ %Scroll{} = scroll_w_gid = %Scroll{scroll | gid: id_seq}
base_tests-1 | │ ~
base_tests-1 | │
base_tests-1 | └─ (server 0.1.0) test/support/helpers/duels.ex:477:32: Server.Tests.Helpers.Duels.add_hex/3
Expected behavior
That there's no type violation given that it's explicitly matching inside the tuple assignment and in the second case explicitly.
I can indeed make the first issue go away with: assert scroll = %{scroll | gid: bearer_gid}:
assert scroll = %{scroll | gid: bearer_gid}
assert %Engine{
game:
%Game{
status: %Status{
player_active: ^active_player,
player_turn: ^active_player,
stack: []
}
} = game_10
} =
TestH.execute_gift(
duel_3,
scroll,
[],
:creatures,
0,
active_ctx_2,
opponent_ctx_2
)
Before and then using just scroll in the function call, but to be honest I would indeed prefer the ability to keep the struct name for readability while having less one line (besides thinking it's not intuitive or how I imagine it should work).
The same for the second one, turning into bare map update syntax the offending assignment works.
assert %Scroll{} = scroll = Scrollrack.get_scroll(slug_or_id)
scroll_w_gid = %{scroll | gid: id_seq}
In case it's helpful the setup block that populates that map is (this snippet is the full setup and below this one is the actual part) (I can't really share this as a fully working example, I imagine I can trigger it some way but since I saw other posts about the type system presenting some issues on the last releases it can be that you already know the issue and there's no need for me to write a working example - if that's the case or won't pursue, feel free to close this issue, thanks.
setup do
{
:ok,
%Duel{game: %Game{status: %Status{player_turn: active_player}} = game} = duel
} = TestH.generate_from_open()
assert %Context{} = active_ctx = Map.fetch!(game, active_player)
assert %Context{} = opponent_ctx = Map.fetch!(game, H.switch_player(active_player))
all_scrolls = :ets.select(:light, [{{:"$1", :_, :_}, [], [:"$1"]}])
{new_active_ctx, cast_map} =
Enum.reduce(
all_scrolls,
{active_ctx, %{}},
fn id, {ctx, c_map} ->
{
%Scroll{id: ^id, cost: cost_1, slug: slug} = scroll,
%Context{} = active_ctx_1
} = TestH.add_playable_scroll(ctx, id)
payment_cost_1 = String.replace(cost_1, "1", "A") |> String.split("", trim: true)
{active_ctx_1, Map.put(c_map, slug, {scroll, payment_cost_1})}
end
)
assert %Game{} = new_game = Map.put(game, active_player, new_active_ctx)
assert %Duel{} = new_duel = %Duel{duel | game: new_game}
{:ok,
%{
duel: new_duel,
active_ctx: new_active_ctx,
opponent_ctx: opponent_ctx,
cast_cost: cast_map
}}
end{
%Scroll{id: ^id, cost: cost_1, slug: slug} = scroll,
%Context{} = active_ctx_1
} = TestH.add_playable_scroll(ctx, id)
payment_cost_1 = String.replace(cost_1, "1", "A") |> String.split("", trim: true)
{active_ctx_1, Map.put(c_map, slug, {scroll, payment_cost_1})}This also seems to be appropriate, though the slug => {scroll, ...} map doesn't have any proper typing, the issue seems to be on the assignments themselves.
Anyway, thanks