Skip to content

Commit

Permalink
review suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
greg-rychlewski committed Dec 25, 2024
1 parent c4ed2c3 commit 1e9502c
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 9 deletions.
19 changes: 11 additions & 8 deletions lib/ecto/query/builder.ex
Original file line number Diff line number Diff line change
Expand Up @@ -357,24 +357,28 @@ defmodule Ecto.Query.Builder do
def escape({comp_op, _, [left, right]} = expr, type, params_acc, vars, env)
when comp_op in ~w(== != < > <= >=)a do
assert_type!(expr, type, :boolean)
compare_str = Macro.to_string(expr)

if is_nil(left) or is_nil(right) do
error!(
"comparison with nil in `#{compare_str}` is forbidden as it is unsafe. " <>
"comparison with nil in `#{Macro.to_string(expr)}` is forbidden as it is unsafe. " <>
"If you want to check if a value is nil, use is_nil/1 instead"
)
end

ltype = quoted_type(right, vars)
rtype = quoted_type(left, vars)

{left, params_acc} = escape(left, ltype, params_acc, vars, env)
{right, params_acc} = escape(right, rtype, params_acc, vars, env)
{escaped_left, params_acc} = escape(left, ltype, params_acc, vars, env)
{escaped_right, params_acc} = escape(right, rtype, params_acc, vars, env)

{params, acc} = params_acc
params = params |> wrap_nil(left, compare_str) |> wrap_nil(right, compare_str)
{{:{}, [], [comp_op, [], [left, right]]}, {params, acc}}

params =
params
|> wrap_nil(escaped_left, Macro.to_string(right))
|> wrap_nil(escaped_right, Macro.to_string(left))

{{:{}, [], [comp_op, [], [escaped_left, escaped_right]]}, {params, acc}}
end

# mathematical operators
Expand Down Expand Up @@ -1188,9 +1192,8 @@ defmodule Ecto.Query.Builder do
"""
def not_nil!(nil, compare_str) do
raise ArgumentError,
"comparison with nil in `#{compare_str}` is forbidden as it is unsafe. " <>
"comparing `#{compare_str}` with `nil` is forbidden as it is unsafe. " <>
"If you want to check if a value is nil, use is_nil/1 instead"

end

def not_nil!(not_nil, _compare_str) do
Expand Down
2 changes: 1 addition & 1 deletion test/ecto/query_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ defmodule Ecto.QueryTest do
end

assert_raise ArgumentError,
~r"comparison with nil in `p.id == \^id` is forbidden as it is unsafe", fn ->
~r"comparing `p.id` with `nil` is forbidden as it is unsafe", fn ->
from p in "posts", where: p.id == ^id
end
end
Expand Down

0 comments on commit 1e9502c

Please sign in to comment.