diff --git a/lib/ecto/query/builder.ex b/lib/ecto/query/builder.ex index 38c9944dc3..d2d78dfe52 100644 --- a/lib/ecto/query/builder.ex +++ b/lib/ecto/query/builder.ex @@ -360,7 +360,7 @@ defmodule Ecto.Query.Builder do if is_nil(left) or is_nil(right) do error!( - "comparison with nil 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 @@ -368,11 +368,17 @@ defmodule Ecto.Query.Builder do 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 - {{:{}, [], [comp_op, [], [left, right]]}, {params |> wrap_nil(left) |> wrap_nil(right), 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 @@ -585,18 +591,18 @@ defmodule Ecto.Query.Builder do defp validate_json_field!(unsupported_field), do: error!("`#{Macro.to_string(unsupported_field)}` is not a valid json field") - defp wrap_nil(params, {:{}, _, [:^, _, [ix]]}), - do: wrap_nil(params, length(params) - ix - 1, []) + defp wrap_nil(params, {:{}, _, [:^, _, [ix]]}, compare_str), + do: wrap_nil(params, length(params) - ix - 1, compare_str, []) - defp wrap_nil(params, _other), do: params + defp wrap_nil(params, _other, _compare_str), do: params - defp wrap_nil([{val, type} | params], 0, acc) do - val = quote do: Ecto.Query.Builder.not_nil!(unquote(val)) + defp wrap_nil([{val, type} | params], 0, compare_str, acc) do + val = quote do: Ecto.Query.Builder.not_nil!(unquote(val), unquote(compare_str)) Enum.reverse(acc, [{val, type} | params]) end - defp wrap_nil([pair | params], i, acc) do - wrap_nil(params, i - 1, [pair | acc]) + defp wrap_nil([pair | params], i, compare_str, acc) do + wrap_nil(params, i - 1, compare_str, [pair | acc]) end defp expand_and_split_fragment(query, env) do @@ -1184,13 +1190,13 @@ defmodule Ecto.Query.Builder do @doc """ Called by escaper at runtime to verify that a value is not nil. """ - def not_nil!(nil) do + def not_nil!(nil, compare_str) do raise ArgumentError, - "comparison with nil 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) do + def not_nil!(not_nil, _compare_str) do not_nil end diff --git a/test/ecto/query_test.exs b/test/ecto/query_test.exs index 4a6e43a4a4..e3b8307acb 100644 --- a/test/ecto/query_test.exs +++ b/test/ecto/query_test.exs @@ -56,17 +56,23 @@ defmodule Ecto.QueryTest do test "does not allow nils in comparison at compile time" do assert_raise Ecto.Query.CompileError, - ~r"comparison with nil is forbidden as it is unsafe", fn -> + ~r"comparison with nil in `p.id == nil` is forbidden as it is unsafe", fn -> quote_and_eval from p in "posts", where: p.id == nil end end test "does not allow interpolated nils at runtime" do + id = nil + assert_raise ArgumentError, - ~r"comparison with nil is forbidden as it is unsafe", fn -> - id = nil + ~r"nil given for `id`. comparison with nil is forbidden as it is unsafe", fn -> from p in "posts", where: [id: ^id] end + + assert_raise ArgumentError, + ~r"comparing `p.id` with `nil` is forbidden as it is unsafe", fn -> + from p in "posts", where: p.id == ^id + end end test "allows arbitrary parentheses in where" do