Skip to content

Commit

Permalink
Fix checking of closed maps
Browse files Browse the repository at this point in the history
If a map has unknown keys, because we cannot specify
term() => term() with the necessary precision today,
we need to mark it as dynamic.

We also improve the pretty printing of differences
between composites types.
  • Loading branch information
josevalim committed Jan 20, 2025
1 parent b15de1c commit a1c1888
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 4 deletions.
17 changes: 16 additions & 1 deletion lib/elixir/lib/module/types/apply.ex
Original file line number Diff line number Diff line change
Expand Up @@ -1140,7 +1140,7 @@ defmodule Module.Types.Apply do
defp args_to_quoted_string(args_types, domain, converter) do
docs =
Enum.zip_with(args_types, domain, fn actual, expected ->
if compatible?(actual, expected) do
if compatible?(actual, expected) or not has_simple_difference?(actual, expected) do
actual |> to_quoted() |> Code.Formatter.to_algebra()
else
common = intersection(actual, expected)
Expand All @@ -1163,6 +1163,21 @@ defmodule Module.Types.Apply do
args_docs_to_quoted_string(converter.(docs))
end

@composite_types non_empty_list(term(), term())
|> union(tuple())
|> union(open_map())
|> union(fun())

# If actual/expected have a composite type, computing the
# `intersection(actual, expected) or difference(actual, expected)`
# can lead to an explosion of terms that actually make debugging
# harder. So we check that at least one of the two operations
# return none() (i.e. actual is a subtype or they are disjoint).
defp has_simple_difference?(actual, expected) do
composite_types = intersection(actual, @composite_types)
subtype?(composite_types, expected) or disjoint?(composite_types, expected)
end

defp ansi_red(doc) do
if IO.ANSI.enabled?() do
IA.concat(IA.color(doc, IO.ANSI.red()), IA.color(IA.empty(), IO.ANSI.reset()))
Expand Down
2 changes: 1 addition & 1 deletion lib/elixir/lib/module/types/expr.ex
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ defmodule Module.Types.Expr do
# TODO: Use the fallback type to actually indicate if open or closed.
# The fallback must be unioned with the result of map_values with all
# `keys` deleted.
open_map(pairs)
dynamic(open_map(pairs))
end
end)
catch
Expand Down
2 changes: 1 addition & 1 deletion lib/elixir/lib/module/types/of.ex
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ defmodule Module.Types.Of do
map =
permutate_map(pairs_types, stack, fn fallback, _keys, pairs ->
# TODO: Use the fallback type to actually indicate if open or closed.
if fallback == none(), do: closed_map(pairs), else: open_map(pairs)
if fallback == none(), do: closed_map(pairs), else: dynamic(open_map(pairs))
end)

{map, context}
Expand Down
2 changes: 1 addition & 1 deletion lib/elixir/test/elixir/module/types/expr_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -690,7 +690,7 @@ defmodule Module.Types.ExprTest do
end

test "creating open maps" do
assert typecheck!(%{123 => 456}) == open_map()
assert typecheck!(%{123 => 456}) == dynamic(open_map())
# Since key cannot override :foo, we preserve it
assert typecheck!([key], %{key => 456, foo: :bar}) == dynamic(open_map(foo: atom([:bar])))
# Since key can override :foo, we do not preserve it
Expand Down

0 comments on commit a1c1888

Please sign in to comment.