Skip to content

Commit a2a669e

Browse files
josevalimjosevalim
committed
Improve error messages in many Kernel operations
1 parent b85d1c8 commit a2a669e

5 files changed

Lines changed: 233 additions & 129 deletions

File tree

‎lib/elixir/lib/kernel.ex‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2081,7 +2081,7 @@ defmodule Kernel do
20812081
end
20822082

20832083
annotate_case(
2084-
[optimize_boolean: true, type_check: :expr],
2084+
[optimize_boolean: true, type_check: {:case, operator}],
20852085
{:case, [], [check, [do: bools ++ error]]}
20862086
)
20872087
end
@@ -2109,7 +2109,7 @@ defmodule Kernel do
21092109
assert_no_match_or_guard_scope(__CALLER__.context, "!")
21102110

21112111
annotate_case(
2112-
[optimize_boolean: true, type_check: :expr],
2112+
[optimize_boolean: true, type_check: {:case, :!}],
21132113
quote do
21142114
case unquote(value) do
21152115
x when unquote(x_is_false_or_nil()) -> false
@@ -2123,7 +2123,7 @@ defmodule Kernel do
21232123
assert_no_match_or_guard_scope(__CALLER__.context, "!")
21242124

21252125
annotate_case(
2126-
[optimize_boolean: true, type_check: :expr],
2126+
[optimize_boolean: true, type_check: {:case, :!}],
21272127
quote do
21282128
case unquote(value) do
21292129
x when unquote(x_is_false_or_nil()) -> true
@@ -4054,7 +4054,7 @@ defmodule Kernel do
40544054

40554055
defp build_if(condition, do: do_clause, else: else_clause) do
40564056
annotate_case(
4057-
[optimize_boolean: true, type_check: :expr],
4057+
[optimize_boolean: true, type_check: {:case, :if}],
40584058
quote do
40594059
case unquote(condition) do
40604060
x when unquote(x_is_false_or_nil()) -> unquote(else_clause)
@@ -4105,9 +4105,15 @@ defmodule Kernel do
41054105
end
41064106

41074107
defp build_unless(condition, do: do_clause, else: else_clause) do
4108-
quote do
4109-
if(unquote(condition), do: unquote(else_clause), else: unquote(do_clause))
4110-
end
4108+
annotate_case(
4109+
[optimize_boolean: true, type_check: {:case, :unless}],
4110+
quote do
4111+
case unquote(condition) do
4112+
x when unquote(x_is_false_or_nil()) -> unquote(do_clause)
4113+
_ -> unquote(else_clause)
4114+
end
4115+
end
4116+
)
41114117
end
41124118

41134119
defp build_unless(_condition, _arguments) do
@@ -4375,7 +4381,7 @@ defmodule Kernel do
43754381
assert_no_match_or_guard_scope(__CALLER__.context, "&&")
43764382

43774383
annotate_case(
4378-
[type_check: :expr],
4384+
[type_check: {:case, :&&}],
43794385
quote do
43804386
case unquote(left) do
43814387
x when unquote(x_is_false_or_nil()) ->
@@ -4418,7 +4424,7 @@ defmodule Kernel do
44184424
assert_no_match_or_guard_scope(__CALLER__.context, "||")
44194425

44204426
annotate_case(
4421-
[type_check: :expr],
4427+
[type_check: {:case, :||}],
44224428
quote do
44234429
case unquote(left) do
44244430
x when unquote(x_is_false_or_nil()) ->

‎lib/elixir/lib/module/types/expr.ex‎

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -846,25 +846,6 @@ defmodule Module.Types.Expr do
846846

847847
## Warning formatting
848848

849-
def format_diagnostic({:badclause, {:case, meta, type, expr}, [head], context}) do
850-
{expr, message} =
851-
if meta[:type_check] == :expr do
852-
{expr,
853-
"""
854-
the following conditional expression will always evaluate to #{to_quoted_string(type)}:
855-
856-
#{expr_to_string(expr) |> indent(4)}
857-
"""}
858-
else
859-
{head, "the following clause has no effect because a previous clause will always match\n"}
860-
end
861-
862-
%{
863-
details: %{typing_traces: collect_traces(expr, context)},
864-
message: message
865-
}
866-
end
867-
868849
def format_diagnostic({:badupdate, type, expr, context}) do
869850
{:%, _, [module, {:%{}, _, [{:|, _, [map, _]}]}]} = expr
870851
traces = collect_traces(map, context)

‎lib/elixir/lib/module/types/helpers.ex‎

Lines changed: 90 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -355,99 +355,101 @@ defmodule Module.Types.Helpers do
355355
{:&, amp_meta, [{:/, slash_meta, [{{:., dot_meta, [mod, fun]}, call_meta, []}, arity]}]}
356356

357357
{:case, meta, [expr, [do: clauses]]} ->
358-
if meta[:type_check] == :expr do
359-
case clauses do
360-
[
361-
{:->, _,
362-
[
358+
case meta[:type_check] do
359+
{:case, _} ->
360+
case clauses do
361+
[
362+
{:->, _,
363363
[
364-
{:when, _,
365-
[
366-
{var, _, Kernel},
367-
{{:., _, [:erlang, :orelse]}, _,
368-
[
369-
{{:., _, [:erlang, :"=:="]}, _, [{var, _, Kernel}, false]},
370-
{{:., _, [:erlang, :"=:="]}, _, [{var, _, Kernel}, nil]}
371-
]}
372-
]}
373-
],
374-
true
375-
]},
376-
{:->, _, [[{:_, _, Kernel}], false]}
377-
] ->
378-
{:!, meta, [expr]}
379-
380-
[
381-
{:->, _,
382-
[
364+
[
365+
{:when, _,
366+
[
367+
{var, _, Kernel},
368+
{{:., _, [:erlang, :orelse]}, _,
369+
[
370+
{{:., _, [:erlang, :"=:="]}, _, [{var, _, Kernel}, false]},
371+
{{:., _, [:erlang, :"=:="]}, _, [{var, _, Kernel}, nil]}
372+
]}
373+
]}
374+
],
375+
true
376+
]},
377+
{:->, _, [[{:_, _, Kernel}], false]}
378+
] ->
379+
{:!, meta, [expr]}
380+
381+
[
382+
{:->, _,
383383
[
384-
{:when, _,
385-
[
386-
{var, _, Kernel},
387-
{{:., _, [:erlang, :orelse]}, _,
388-
[
389-
{{:., _, [:erlang, :"=:="]}, _, [{var, _, Kernel}, false]},
390-
{{:., _, [:erlang, :"=:="]}, _, [{var, _, Kernel}, nil]}
391-
]}
392-
]}
393-
],
394-
right_side
395-
]},
396-
{:->, _, [[{var, _, Kernel}], {var, _, Kernel}]}
397-
] ->
398-
{:||, meta, [expr, right_side]}
399-
400-
[
401-
{:->, _,
402-
[
384+
[
385+
{:when, _,
386+
[
387+
{var, _, Kernel},
388+
{{:., _, [:erlang, :orelse]}, _,
389+
[
390+
{{:., _, [:erlang, :"=:="]}, _, [{var, _, Kernel}, false]},
391+
{{:., _, [:erlang, :"=:="]}, _, [{var, _, Kernel}, nil]}
392+
]}
393+
]}
394+
],
395+
right_side
396+
]},
397+
{:->, _, [[{var, _, Kernel}], {var, _, Kernel}]}
398+
] ->
399+
{:||, meta, [expr, right_side]}
400+
401+
[
402+
{:->, _,
403403
[
404-
{:when, _,
405-
[
406-
{var, _, Kernel},
407-
{{:., _, [:erlang, :orelse]}, _,
408-
[
409-
{{:., _, [:erlang, :"=:="]}, _, [{var, _, Kernel}, false]},
410-
{{:., _, [:erlang, :"=:="]}, _, [{var, _, Kernel}, nil]}
411-
]}
412-
]}
413-
],
414-
{var, _, Kernel}
415-
]},
416-
{:->, _, [[{:_, _, Kernel}], right_side]}
417-
] ->
418-
{:&&, meta, [expr, right_side]}
419-
420-
[
421-
{:->, _,
422-
[
404+
[
405+
{:when, _,
406+
[
407+
{var, _, Kernel},
408+
{{:., _, [:erlang, :orelse]}, _,
409+
[
410+
{{:., _, [:erlang, :"=:="]}, _, [{var, _, Kernel}, false]},
411+
{{:., _, [:erlang, :"=:="]}, _, [{var, _, Kernel}, nil]}
412+
]}
413+
]}
414+
],
415+
{var, _, Kernel}
416+
]},
417+
{:->, _, [[{:_, _, Kernel}], right_side]}
418+
] ->
419+
{:&&, meta, [expr, right_side]}
420+
421+
[
422+
{:->, _,
423423
[
424-
{:when, _,
425-
[
426-
{var, _, Kernel},
427-
{{:., _, [:erlang, :orelse]}, _,
428-
[
429-
{{:., _, [:erlang, :"=:="]}, _, [{var, _, Kernel}, false]},
430-
{{:., _, [:erlang, :"=:="]}, _, [{var, _, Kernel}, nil]}
431-
]}
432-
]}
433-
],
434-
else_block
435-
]},
436-
{:->, _, [[{:_, _, Kernel}], do_block]}
437-
] ->
438-
{:if, meta, [expr, [do: do_block, else: else_block]]}
439-
440-
[
441-
{:->, _, [[false], else_block]},
442-
{:->, _, [[true], do_block]}
443-
] ->
444-
{:if, meta, [expr, [do: do_block, else: else_block]]}
424+
[
425+
{:when, _,
426+
[
427+
{var, _, Kernel},
428+
{{:., _, [:erlang, :orelse]}, _,
429+
[
430+
{{:., _, [:erlang, :"=:="]}, _, [{var, _, Kernel}, false]},
431+
{{:., _, [:erlang, :"=:="]}, _, [{var, _, Kernel}, nil]}
432+
]}
433+
]}
434+
],
435+
else_block
436+
]},
437+
{:->, _, [[{:_, _, Kernel}], do_block]}
438+
] ->
439+
{:if, meta, [expr, [do: do_block, else: else_block]]}
440+
441+
[
442+
{:->, _, [[false], else_block]},
443+
{:->, _, [[true], do_block]}
444+
] ->
445+
{:if, meta, [expr, [do: do_block, else: else_block]]}
446+
447+
_ ->
448+
{:case, meta, [expr, [do: {:..., [], []}]]}
449+
end
445450

446-
_ ->
447-
{:case, meta, [expr, [do: {:..., [], []}]]}
448-
end
449-
else
450-
{:case, meta, [expr, [do: {:..., [], []}]]}
451+
_ ->
452+
{:case, meta, [expr, [do: {:..., [], []}]]}
451453
end
452454

453455
{:try, meta, [[do: _] ++ _]} ->

‎lib/elixir/lib/module/types/pattern.ex‎

Lines changed: 57 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1393,18 +1393,67 @@ defmodule Module.Types.Pattern do
13931393
end
13941394

13951395
defp badpattern({{op, meta, expr, type}, args, previous}, _, _) when op in [:case, :try_else] do
1396+
type_check = meta[:type_check]
1397+
13961398
cond do
1397-
meta[:type_check] == :expr ->
1398-
{expr,
1399-
"""
1400-
the following conditional expression:
1399+
match?({:case, _}, type_check) ->
1400+
message =
1401+
case type_check do
1402+
{:case, op} when op in [:and, :or] ->
1403+
{first_message, second_message} =
1404+
case booleaness(type) do
1405+
{true, _} -> {" will always succeed", "because it evaluates to"}
1406+
{false, _} -> {" will never succeed", "because it evaluates to"}
1407+
:none -> {" will always fail", "because it evaluates to"}
1408+
_ -> {"", "will always evaluate to"}
1409+
end
14011410

1402-
#{expr_to_string(expr) |> indent(4)}
1411+
"""
1412+
the following conditional expression#{first_message}:
14031413
1404-
will always evaluate to:
1414+
#{expr_to_string(expr) |> indent(4)}
14051415
1406-
#{to_quoted_string(type) |> indent(4)}
1407-
"""}
1416+
#{second_message}:
1417+
1418+
#{to_quoted_string(type) |> indent(4)}
1419+
"""
1420+
1421+
{:case, :||} ->
1422+
if subtype?(type, atom([false, nil])) do
1423+
"""
1424+
the following conditional expression will never succeed:
1425+
1426+
#{expr_to_string(expr) |> indent(4)}
1427+
1428+
because it evaluates to:
1429+
1430+
#{to_quoted_string(type) |> indent(4)}
1431+
"""
1432+
else
1433+
"""
1434+
the right-hand side of || will never be executed:
1435+
1436+
#{expr_to_string({:||, [], [expr, {:..., [], []}]}) |> indent(4)}
1437+
1438+
because the left-hand side always evaluates to:
1439+
1440+
#{to_quoted_string(type) |> indent(4)}
1441+
"""
1442+
end
1443+
1444+
_ ->
1445+
"""
1446+
the following conditional expression:
1447+
1448+
#{expr_to_string(expr) |> indent(4)}
1449+
1450+
will always evaluate to:
1451+
1452+
#{to_quoted_string(type) |> indent(4)}
1453+
"""
1454+
end
1455+
1456+
{expr, message}
14081457

14091458
previous == [] ->
14101459
{args,

0 commit comments

Comments
 (0)