From 5d85a076086a521d0d53c52e87de58c463bd9256 Mon Sep 17 00:00:00 2001 From: Arpit Jain Date: Tue, 7 Jul 2026 09:30:38 +0900 Subject: [PATCH 1/2] Warn on parentheses in query field access A field access written with empty parentheses such as q.title() is silently accepted and treated identically to q.title. The two only differ by the no_parens: true metadata on the outer call, so bind that metadata and emit a deprecation warning when it is absent, then proceed as before. Closes #4588. Signed-off-by: Arpit Jain --- lib/ecto/query/builder.ex | 13 ++++++++++++- test/ecto/query/builder_test.exs | 16 ++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/lib/ecto/query/builder.ex b/lib/ecto/query/builder.ex index 9b42956ba3..4c8d5c2f7b 100644 --- a/lib/ecto/query/builder.ex +++ b/lib/ecto/query/builder.ex @@ -84,8 +84,19 @@ defmodule Ecto.Query.Builder do def escape(expr, type, params_acc, vars, env) # var.x - where var is bound - def escape({{:., _, [callee, field]}, _, []}, _type, params_acc, vars, _env) + def escape({{:., _, [callee, field]}, call_meta, []}, _type, params_acc, vars, env) when is_atom(field) do + if call_meta[:no_parens] != true do + stacktrace = Macro.Env.stacktrace(get_env(env)) + + IO.warn( + "using parentheses after a field access, such as `#{Macro.to_string(callee)}.#{field}()`, " <> + "is deprecated and will raise in future Ecto versions. Remove the parentheses to keep " <> + "the current behaviour: `#{Macro.to_string(callee)}.#{field}`", + stacktrace + ) + end + {escape_field!(callee, field, vars), params_acc} end diff --git a/test/ecto/query/builder_test.exs b/test/ecto/query/builder_test.exs index b46db59696..497b659219 100644 --- a/test/ecto/query/builder_test.exs +++ b/test/ecto/query/builder_test.exs @@ -730,6 +730,22 @@ defmodule Ecto.Query.BuilderTest do end end + test "warns on parentheses in field access" do + import ExUnit.CaptureIO + + warning = + capture_io(:stderr, fn -> + escape(quote(do: x.y()), [x: 0], __ENV__) + end) + + assert warning =~ "using parentheses after a field access" + assert warning =~ "x.y()" + + assert capture_io(:stderr, fn -> + escape(quote(do: x.y), [x: 0], __ENV__) + end) == "" + end + test "doesn't escape interpolation" do import Kernel, except: [>: 2, ++: 2] From fdf7c82f5f9b8192e305b2c22cbd17c7d6d168e6 Mon Sep 17 00:00:00 2001 From: Arpit Jain Date: Tue, 7 Jul 2026 10:04:17 +0900 Subject: [PATCH 2/2] Remove parentheses from field access in tests The new deprecation warning for parenthesized query field access fired across the builder test suite, since many escape/1 cases wrote fields as x.x() instead of x.x on the input side. Drop the empty parentheses so the tests exercise the same expressions without emitting the warning. The escaped output is identical either way, so the assertions are unchanged. The intentional case in builder_test.exs that verifies the warning keeps its parentheses. Signed-off-by: Arpit Jain --- test/ecto/query/builder/distinct_test.exs | 6 ++-- test/ecto/query/builder/filter_test.exs | 2 +- test/ecto/query/builder/group_by_test.exs | 4 +-- test/ecto/query/builder/order_by_test.exs | 8 ++--- test/ecto/query/builder/select_test.exs | 6 ++-- test/ecto/query/builder/update_test.exs | 2 +- test/ecto/query/builder/windows_test.exs | 4 +-- test/ecto/query/builder_test.exs | 44 +++++++++++------------ 8 files changed, 38 insertions(+), 38 deletions(-) diff --git a/test/ecto/query/builder/distinct_test.exs b/test/ecto/query/builder/distinct_test.exs index 17d7b0822d..9a2b836f16 100644 --- a/test/ecto/query/builder/distinct_test.exs +++ b/test/ecto/query/builder/distinct_test.exs @@ -16,13 +16,13 @@ defmodule Ecto.Query.Builder.DistinctTest do escape(true, {[], %{}}, [x: 0], __ENV__) assert {Macro.escape(quote(do: [asc: &0.y()])), {[], %{}}} == - escape(quote(do: x.y()), {[], %{}}, [x: 0], __ENV__) + escape(quote(do: x.y), {[], %{}}, [x: 0], __ENV__) assert {Macro.escape(quote(do: [asc: &0.x(), asc: &1.y()])), {[], %{}}} == - escape(quote(do: [x.x(), y.y()]), {[], %{}}, [x: 0, y: 1], __ENV__) + escape(quote(do: [x.x, y.y]), {[], %{}}, [x: 0, y: 1], __ENV__) assert {Macro.escape(quote(do: [asc: &0.x(), desc: &1.y()])), {[], %{}}} == - escape(quote(do: [x.x(), desc: y.y()]), {[], %{}}, [x: 0, y: 1], __ENV__) + escape(quote(do: [x.x, desc: y.y]), {[], %{}}, [x: 0, y: 1], __ENV__) import Kernel, except: [>: 2] diff --git a/test/ecto/query/builder/filter_test.exs b/test/ecto/query/builder/filter_test.exs index f437dcd60e..9be7bfd603 100644 --- a/test/ecto/query/builder/filter_test.exs +++ b/test/ecto/query/builder/filter_test.exs @@ -13,7 +13,7 @@ defmodule Ecto.Query.Builder.FilterTest do assert escape(:where, quote(do: []), 0, [x: 0], __ENV__) === {true, {[], %{subqueries: []}}} - assert escape(:where, quote(do: {x.x()} == {^"foo"}), 0, [x: 0], __ENV__) === + assert escape(:where, quote(do: {x.x} == {^"foo"}), 0, [x: 0], __ENV__) === {Macro.escape(quote(do: {&0.x()} == {^0})), {[{"foo", {0, :x}}], %{subqueries: []}}} diff --git a/test/ecto/query/builder/group_by_test.exs b/test/ecto/query/builder/group_by_test.exs index a69fb55ee9..59bd424cdc 100644 --- a/test/ecto/query/builder/group_by_test.exs +++ b/test/ecto/query/builder/group_by_test.exs @@ -13,10 +13,10 @@ defmodule Ecto.Query.Builder.GroupByTest do describe "escape" do test "handles expressions and params" do assert {Macro.escape(quote(do: [&0.y()])), {[], %{}}} == - escape(:group_by, quote(do: x.y()), {[], %{}}, [x: 0], __ENV__) + escape(:group_by, quote(do: x.y), {[], %{}}, [x: 0], __ENV__) assert {Macro.escape(quote(do: [&0.x(), &1.y()])), {[], %{}}} == - escape(:group_by, quote(do: [x.x(), y.y()]), {[], %{}}, [x: 0, y: 1], __ENV__) + escape(:group_by, quote(do: [x.x, y.y]), {[], %{}}, [x: 0, y: 1], __ENV__) import Kernel, except: [>: 2] diff --git a/test/ecto/query/builder/order_by_test.exs b/test/ecto/query/builder/order_by_test.exs index a43f51006d..5389ce8728 100644 --- a/test/ecto/query/builder/order_by_test.exs +++ b/test/ecto/query/builder/order_by_test.exs @@ -33,15 +33,15 @@ defmodule Ecto.Query.Builder.OrderByTest do test "handles expressions and params" do assert {Macro.escape(quote(do: [asc: &0.y()])), {[], %{}}} == - escape(:order_by, quote(do: x.y()), {[], %{}}, [x: 0], __ENV__) + escape(:order_by, quote(do: x.y), {[], %{}}, [x: 0], __ENV__) assert {Macro.escape(quote(do: [asc: &0.x(), asc: &1.y()])), {[], %{}}} == - escape(:order_by, quote(do: [x.x(), y.y()]), {[], %{}}, [x: 0, y: 1], __ENV__) + escape(:order_by, quote(do: [x.x, y.y]), {[], %{}}, [x: 0, y: 1], __ENV__) assert {Macro.escape(quote(do: [asc: &0.x(), desc: &1.y()])), {[], %{}}} == escape( :order_by, - quote(do: [asc: x.x(), desc: y.y()]), + quote(do: [asc: x.x, desc: y.y]), {[], %{}}, [x: 0, y: 1], __ENV__ @@ -50,7 +50,7 @@ defmodule Ecto.Query.Builder.OrderByTest do assert {Macro.escape(quote(do: [asc: &0.x(), desc: &1.y()])), {[], %{}}} == escape( :order_by, - quote(do: [x.x(), desc: y.y()]), + quote(do: [x.x, desc: y.y]), {[], %{}}, [x: 0, y: 1], __ENV__ diff --git a/test/ecto/query/builder/select_test.exs b/test/ecto/query/builder/select_test.exs index 56687fd2ab..19c45b29e5 100644 --- a/test/ecto/query/builder/select_test.exs +++ b/test/ecto/query/builder/select_test.exs @@ -34,7 +34,7 @@ defmodule Ecto.Query.Builder.SelectTest do escape(quote(do: x), [x: 0], __ENV__) assert {Macro.escape(quote(do: &0.y())), params_acc()} == - escape(quote(do: x.y()), [x: 0], __ENV__) + escape(quote(do: x.y), [x: 0], __ENV__) assert {Macro.escape(quote(do: &0)), params_acc(take: %{0 => {:any, [:foo, :bar, baz: :bat]}})} == @@ -59,13 +59,13 @@ defmodule Ecto.Query.Builder.SelectTest do escape(quote(do: %{a => b}), [a: 0, b: 1], __ENV__) assert {[Macro.escape(quote(do: &0.y())), Macro.escape(quote(do: &0.z()))], params_acc()} == - escape(quote(do: [x.y(), x.z()]), [x: 0], __ENV__) + escape(quote(do: [x.y, x.z]), [x: 0], __ENV__) assert {[ {:{}, [], [{:{}, [], [:., [], [{:{}, [], [:&, [], [0]]}, :y]]}, [], []]}, {:{}, [], [:^, [], [0]]} ], params_acc(params: [{1, :any}])} == - escape(quote(do: [x.y(), ^1]), [x: 0], __ENV__) + escape(quote(do: [x.y, ^1]), [x: 0], __ENV__) assert {{:{}, [], [:%, [], [Foo, {:{}, [], [:%{}, [], [a: {:{}, [], [:&, [], [0]]}]]}]]}, params_acc()} == diff --git a/test/ecto/query/builder/update_test.exs b/test/ecto/query/builder/update_test.exs index 084f68430f..f1189feffe 100644 --- a/test/ecto/query/builder/update_test.exs +++ b/test/ecto/query/builder/update_test.exs @@ -14,7 +14,7 @@ defmodule Ecto.Query.Builder.UpdateTest do ] ] - assert escape(quote(do: [set: [foo: x.bar()]]), [x: 0], __ENV__) |> elem(0) == + assert escape(quote(do: [set: [foo: x.bar]]), [x: 0], __ENV__) |> elem(0) == Macro.escape(quote(do: [set: [foo: &0.bar()]])) end diff --git a/test/ecto/query/builder/windows_test.exs b/test/ecto/query/builder/windows_test.exs index c195823adb..71a445d67d 100644 --- a/test/ecto/query/builder/windows_test.exs +++ b/test/ecto/query/builder/windows_test.exs @@ -25,13 +25,13 @@ defmodule Ecto.Query.Builder.WindowsTest do describe "escape" do test "handles expressions and params" do assert {Macro.escape(quote(do: [partition_by: [&0.y()]])), [], {[], %{}}} == - escape(quote(do: [partition_by: x.y()]), {[], %{}}, [x: 0], __ENV__) + escape(quote(do: [partition_by: x.y]), {[], %{}}, [x: 0], __ENV__) assert {Macro.escape(quote(do: [partition_by: [&0.y()]])), [], {[], %{}}} == escape(quote(do: [partition_by: :y]), {[], %{}}, [x: 0], __ENV__) assert {Macro.escape(quote(do: [order_by: [asc: &0.y()]])), [], {[], %{}}} == - escape(quote(do: [order_by: x.y()]), {[], %{}}, [x: 0], __ENV__) + escape(quote(do: [order_by: x.y]), {[], %{}}, [x: 0], __ENV__) assert {Macro.escape(quote(do: [order_by: [asc: &0.y()]])), [], {[], %{}}} == escape(quote(do: [order_by: :y]), {[], %{}}, [x: 0], __ENV__) diff --git a/test/ecto/query/builder_test.exs b/test/ecto/query/builder_test.exs index 497b659219..bc4e81875d 100644 --- a/test/ecto/query/builder_test.exs +++ b/test/ecto/query/builder_test.exs @@ -19,7 +19,7 @@ defmodule Ecto.Query.BuilderTest do ), []} == escape( quote do - x.y() + x.y end, [x: 0], __ENV__ @@ -34,7 +34,7 @@ defmodule Ecto.Query.BuilderTest do ), []} == escape( quote do - x.y() > x.z() + x.y > x.z end, [x: 0], __ENV__ @@ -47,7 +47,7 @@ defmodule Ecto.Query.BuilderTest do ), []} == escape( quote do - x.y() > y.z() + x.y > y.z end, [x: 0, y: 1], __ENV__ @@ -62,7 +62,7 @@ defmodule Ecto.Query.BuilderTest do ), []} == escape( quote do - x.y() + y.z() + x.y + y.z end, [x: 0, y: 1], __ENV__ @@ -149,7 +149,7 @@ defmodule Ecto.Query.BuilderTest do ), []} == escape( quote do - -x.y() + -x.y end, [x: 0], __ENV__ @@ -344,7 +344,7 @@ defmodule Ecto.Query.BuilderTest do ), [{0, :any}]} == escape( quote do - fragment("date_add(?, ?)", p.created_at(), ^0) + fragment("date_add(?, ?)", p.created_at, ^0) end, [p: 0], __ENV__ @@ -370,7 +370,7 @@ defmodule Ecto.Query.BuilderTest do ), []} == escape( quote do - fragment("query\\?(?)", p.created_at()) + fragment("query\\?(?)", p.created_at) end, [p: 0], __ENV__ @@ -434,13 +434,13 @@ defmodule Ecto.Query.BuilderTest do test "escape over with window name" do assert {Macro.escape(quote(do: over(count(&0.id()), :w))), []} == - escape(quote(do: count(x.id()) |> over(:w)), [x: 0], __ENV__) + escape(quote(do: count(x.id) |> over(:w)), [x: 0], __ENV__) assert {Macro.escape(quote(do: over(nth_value(&0.id(), 1), :w))), []} == - escape(quote(do: nth_value(x.id(), 1) |> over(:w)), [x: 0], __ENV__) + escape(quote(do: nth_value(x.id, 1) |> over(:w)), [x: 0], __ENV__) assert {Macro.escape(quote(do: over(nth_value(&0.id(), 1), :w))), []} == - escape(quote(do: my_first_value(x.id()) |> over(:w)), [x: 0], __ENV__) + escape(quote(do: my_first_value(x.id) |> over(:w)), [x: 0], __ENV__) end defmacro my_custom_field(p) do @@ -460,10 +460,10 @@ defmodule Ecto.Query.BuilderTest do escape(quote(do: over(row_number())), [], __ENV__) assert {Macro.escape(quote(do: over(nth_value(&0.id(), 1), []))), []} == - escape(quote(do: over(my_first_value(x.id()))), [x: 0], __ENV__) + escape(quote(do: over(my_first_value(x.id))), [x: 0], __ENV__) assert {Macro.escape(quote(do: over(nth_value(&0.id(), 1), order_by: [asc: &0.id()]))), []} == - escape(quote(do: nth_value(x.id(), 1) |> over(order_by: x.id())), [x: 0], __ENV__) + escape(quote(do: nth_value(x.id, 1) |> over(order_by: x.id)), [x: 0], __ENV__) assert {Macro.escape( quote( @@ -478,14 +478,14 @@ defmodule Ecto.Query.BuilderTest do ) ), []} == escape( - quote(do: nth_value(x.id(), 1) |> over(order_by: my_complex_order(x))), + quote(do: nth_value(x.id, 1) |> over(order_by: my_complex_order(x))), [x: 0], __ENV__ ) assert {Macro.escape(quote(do: over(nth_value(&0.id(), 1), partition_by: [&0.id()]))), []} == escape( - quote(do: nth_value(x.id(), 1) |> over(partition_by: x.id())), + quote(do: nth_value(x.id, 1) |> over(partition_by: x.id)), [x: 0], __ENV__ ) @@ -493,7 +493,7 @@ defmodule Ecto.Query.BuilderTest do assert {Macro.escape(quote(do: over(nth_value(&0.id(), 1), frame: fragment({:raw, "ROWS"})))), []} == escape( - quote(do: nth_value(x.id(), 1) |> over(frame: fragment("ROWS"))), + quote(do: nth_value(x.id, 1) |> over(frame: fragment("ROWS"))), [x: 0], __ENV__ ) @@ -502,7 +502,7 @@ defmodule Ecto.Query.BuilderTest do ~r"windows definitions given to over/2 do not allow interpolations at the root", fn -> escape( - quote(do: nth_value(x.id(), 1) |> over(order_by: ^foo)), + quote(do: nth_value(x.id, 1) |> over(order_by: ^foo)), [x: 0], __ENV__ ) @@ -512,7 +512,7 @@ defmodule Ecto.Query.BuilderTest do assert {Macro.escape(quote(do: over(filter(avg(&0.value()), is_nil(&0.flag())), []))), []} == escape( - quote(do: avg(x.value()) |> filter(is_nil(x.flag())) |> over([])), + quote(do: avg(x.value) |> filter(is_nil(x.flag)) |> over([])), [x: 0], __ENV__ ) @@ -528,7 +528,7 @@ defmodule Ecto.Query.BuilderTest do ), []} == escape( quote do - type(x.y() + y.z(), :decimal) + type(x.y + y.z, :decimal) end, [x: 0, y: 1], __ENV__ @@ -580,7 +580,7 @@ defmodule Ecto.Query.BuilderTest do ), []} == escape( quote do - type(sum(x.y()), :decimal) + type(sum(x.y), :decimal) end, [x: 0], {__ENV__, %{}} @@ -608,7 +608,7 @@ defmodule Ecto.Query.BuilderTest do ), []} == escape( quote do - type(filter(sum(x.y()), x.y() > x.z()), :decimal) + type(filter(sum(x.y), x.y > x.z), :decimal) end, [x: 0], {__ENV__, %{}} @@ -624,7 +624,7 @@ defmodule Ecto.Query.BuilderTest do ), []} == escape( quote do - type(over(fragment("array_agg(?)", x.id()), :y), {:array, Ecto.UUID}) + type(over(fragment("array_agg(?)", x.id), :y), {:array, Ecto.UUID}) end, [x: 0], {__ENV__, %{}} @@ -661,7 +661,7 @@ defmodule Ecto.Query.BuilderTest do ), []} == escape( quote do - type(wrapped_sum(x.y()), :integer) + type(wrapped_sum(x.y), :integer) end, [x: 0], __ENV__