Skip to content

Commit bfb6cdb

Browse files
committed
Emit struct comparison warning only if the intersection has structs
This avoids the false negatives while still emitting the warning when trying to compare two datetimes or similar.
1 parent b27b911 commit bfb6cdb

File tree

2 files changed

+85
-4
lines changed

2 files changed

+85
-4
lines changed

lib/elixir/lib/module/types/apply.ex

+36-4
Original file line numberDiff line numberDiff line change
@@ -436,11 +436,14 @@ defmodule Module.Types.Apply do
436436
number_type?(left) and number_type?(right) ->
437437
{:ok, result}
438438

439-
disjoint?(left, right) ->
440-
{:error, :mismatched_comparison}
441-
442439
true ->
443-
{:ok, result}
440+
common = intersection(left, right)
441+
442+
cond do
443+
empty?(common) -> {:error, :mismatched_comparison}
444+
match?({false, _}, map_fetch(common, :__struct__)) -> {:error, :struct_comparison}
445+
true -> {:ok, result}
446+
end
444447
end
445448
end
446449

@@ -979,6 +982,35 @@ defmodule Module.Types.Apply do
979982
}
980983
end
981984

985+
def format_diagnostic({:struct_comparison, [left, right], mfac, expr, context}) do
986+
{_, name, _, _} = mfac
987+
traces = collect_traces(expr, context)
988+
989+
%{
990+
details: %{typing_traces: traces},
991+
message:
992+
IO.iodata_to_binary([
993+
"""
994+
comparison with structs found:
995+
996+
#{expr_to_string(expr) |> indent(4)}
997+
998+
given types:
999+
1000+
#{type_comparison_to_string(name, left, right) |> indent(4)}
1001+
""",
1002+
format_traces(traces),
1003+
"""
1004+
1005+
Comparison operators (>, <, >=, <=, min, and max) perform structural \
1006+
and not semantic comparison. Comparing with a struct won't give meaningful \
1007+
results. Structs that can be compared typically define a compare/2 function \
1008+
within their modules that can be used for semantic comparison.
1009+
"""
1010+
])
1011+
}
1012+
end
1013+
9821014
def format_diagnostic({:undefined, :badmodule, module, fun, arity}) do
9831015
top =
9841016
if fun == :__struct__ and arity == 0 do

lib/elixir/test/elixir/module/types/expr_test.exs

+49
Original file line numberDiff line numberDiff line change
@@ -1067,6 +1067,55 @@ defmodule Module.Types.ExprTest do
10671067
which are always disjoint, and the result is either always true or always false
10681068
"""
10691069
end
1070+
1071+
test "warns on comparison with struct across dynamic call" do
1072+
assert typeerror!([x = %Point{}, y = %Point{}, mod = Kernel], mod.<=(x, y)) ==
1073+
~l"""
1074+
comparison with structs found:
1075+
1076+
mod.<=(x, y)
1077+
1078+
given types:
1079+
1080+
dynamic(%Point{}) <= dynamic(%Point{})
1081+
1082+
where "mod" was given the type:
1083+
1084+
# type: dynamic(Kernel)
1085+
# from: types_test.ex:LINE-1
1086+
mod = Kernel
1087+
1088+
where "x" was given the type:
1089+
1090+
# type: dynamic(%Point{})
1091+
# from: types_test.ex:LINE-1
1092+
x = %Point{}
1093+
1094+
where "y" was given the type:
1095+
1096+
# type: dynamic(%Point{})
1097+
# from: types_test.ex:LINE-1
1098+
y = %Point{}
1099+
1100+
Comparison operators (>, <, >=, <=, min, and max) perform structural and not semantic comparison. Comparing with a struct won't give meaningful results. Structs that can be compared typically define a compare/2 function within their modules that can be used for semantic comparison.
1101+
"""
1102+
1103+
assert typeerror!(
1104+
[x = %Point{}, mod = Kernel, condition],
1105+
(
1106+
y = if condition, do: 123, else: %Point{}
1107+
mod.<=(x, y)
1108+
)
1109+
) =~ "comparison with structs found:"
1110+
1111+
assert typecheck!(
1112+
[x = 456, mod = Kernel, condition],
1113+
(
1114+
y = if condition, do: 123, else: %Point{}
1115+
mod.<=(x, y)
1116+
)
1117+
) == boolean()
1118+
end
10701119
end
10711120

10721121
describe ":erlang rewrites" do

0 commit comments

Comments
 (0)