improvement: add %Ash.Expr{} wrapper struct

lib/ash/changeset/changeset.ex

@@ -1676,7 +1676,7 @@ defmodule Ash.Changeset do
   # ugly because it can end up reading like "not is not equal to" but
   # ultimately produces the correct results.
   @spec atomic_condition([{module(), keyword()}], Ash.Changeset.t(), map() | struct()) ::
-          {:atomic, Ash.Expr.t() | boolean()} | {:not_atomic, String.t()}
+          {:atomic, Ash.Expr.expression() | boolean()} | {:not_atomic, String.t()}
   def atomic_condition(where, changeset, context) do
     # Handle both map and struct contexts (e.g., Ash.Resource.Change.Context)
     context_map = if is_struct(context), do: Map.from_struct(context), else: context
@@ -2427,7 +2427,8 @@ defmodule Ash.Changeset do
   - `fully_atomic_changeset/4` for creating fully atomic changesets
   - `change_attribute/3` for regular (non-atomic) attribute changes
   """
-  @spec atomic_update(t(), atom(), {:atomic, Ash.Expr.t()} | Ash.Expr.t()) :: t()
+  @spec atomic_update(t(), atom(), {:atomic, Ash.Expr.expression()} | Ash.Expr.expression()) ::
+          t()
   def atomic_update(changeset, key, value, opts \\ [])
 
   def atomic_update(changeset, key, {:atomic, value}, _opts) do
@@ -2543,7 +2544,7 @@ defmodule Ash.Changeset do
     end)
   end
 
-  @spec atomic_set(t(), atom(), {:atomic, Ash.Expr.t()} | Ash.Expr.t()) :: t()
+  @spec atomic_set(t(), atom(), {:atomic, Ash.Expr.expression()} | Ash.Expr.expression()) :: t()
   def atomic_set(changeset, key, value, opts \\ [])
 
   def atomic_set(%{action_type: :update} = changeset, key, value, opts) do
@@ -2607,6 +2608,10 @@ defmodule Ash.Changeset do
     end
   end
 
+  defp check_for_exists_with_relationships(%Ash.Expr{expr: inner}) do
+    check_for_exists_with_relationships(inner)
+  end
+
   defp check_for_exists_with_relationships(%Ash.Query.Exists{path: path}) when path != [] do
     {:error, "atomic_set cannot use exists with relationships"}
   end
@@ -7561,7 +7566,7 @@ defmodule Ash.Changeset do
 
   Used by optimistic locking. See `Ash.Resource.Change.Builtins.optimistic_lock/1` for more.
   """
-  @spec filter(t(), Ash.Expr.t()) :: t()
+  @spec filter(t(), Ash.Expr.expression()) :: t()
   def filter(changeset, expr) when expr in [nil, %{}, []] do
     changeset
   end

lib/ash/custom_expression.ex

@@ -70,12 +70,12 @@ defmodule Ash.CustomExpression do
 
   @callback expression(
               data_layer :: Ash.DataLayer.t(),
-              arguments :: list(Ash.Expr.t())
+              arguments :: list(Ash.Expr.expression())
             ) ::
-              {:ok, Ash.Expr.t()} | :unknown
+              {:ok, Ash.Expr.expression()} | :unknown
 
   @doc false
-  @spec expression(module(), module(), list()) :: {:ok, Ash.Expr.t()} | :unknown
+  @spec expression(module(), module(), list()) :: {:ok, Ash.Expr.expression()} | :unknown
   def expression(module, data_layer, arguments) do
     result = apply(module, :expression, [data_layer, arguments])
 

lib/ash/data_layer/data_layer.ex

@@ -193,7 +193,7 @@ defmodule Ash.DataLayer do
           upsert?: boolean,
           action_select: list(atom),
           upsert_keys: nil | list(atom),
-          upsert_condition: Ash.Expr.t() | nil,
+          upsert_condition: Ash.Expr.expression() | nil,
           return_skipped_upsert?: boolean,
           identity: Ash.Resource.Identity.t() | nil,
           select: list(atom),
@@ -210,7 +210,7 @@ defmodule Ash.DataLayer do
   @type bulk_update_options :: %{
           return_records?: boolean,
           action_select: list(atom),
-          calculations: list({Ash.Query.Calculation.t(), Ash.Expr.t()}),
+          calculations: list({Ash.Query.Calculation.t(), Ash.Expr.expression()}),
           select: list(atom),
           tenant: term()
         }
@@ -308,7 +308,7 @@ defmodule Ash.DataLayer do
   @callback in_transaction?(Ash.Resource.t()) :: boolean
   @callback source(Ash.Resource.t()) :: String.t()
   @callback rollback(Ash.Resource.t(), term) :: no_return
-  @callback calculate(Ash.Resource.t(), list(Ash.Expr.t()), context :: map) ::
+  @callback calculate(Ash.Resource.t(), list(Ash.Expr.expression()), context :: map) ::
               {:ok, term} | {:error, term}
   @callback prefer_transaction?(Ash.Resource.t()) :: boolean
   @callback prefer_transaction_for_atomic_updates?(Ash.Resource.t()) :: boolean
@@ -397,14 +397,14 @@ defmodule Ash.DataLayer do
     Ash.DataLayer.functions(resource)
   end
 
-  @spec calculate(Ash.Resource.t(), list(Ash.Expr.t()), context :: map) ::
+  @spec calculate(Ash.Resource.t(), list(Ash.Expr.expression()), context :: map) ::
           {:ok, list(term)} | {:error, Ash.Error.t()}
   def calculate(resource, exprs, context) do
     calculate(Ash.DataLayer.data_layer(resource), resource, exprs, context)
   end
 
   @doc false
-  @spec calculate(module(), Ash.Resource.t(), list(Ash.Expr.t()), map()) ::
+  @spec calculate(module(), Ash.Resource.t(), list(Ash.Expr.expression()), map()) ::
           {:ok, list(term)} | {:error, Ash.Error.t()}
   def calculate(data_layer_module, resource, exprs, context) do
     Ash.BehaviourHelpers.call_and_validate_return(

lib/ash/expr/expr.ex

@@ -6,10 +6,20 @@ defmodule Ash.Expr do
   @moduledoc "Tools to build Ash expressions"
   alias Ash.Query.{BooleanExpression, Not}
 
+  defstruct [:expr]
+
+  @type t :: %__MODULE__{expr: any()}
+  @type expression :: t() | term()
+
+  def wrap(%__MODULE__{} = already), do: already
+  def wrap(nil), do: nil
+  def wrap(expr), do: %__MODULE__{expr: expr}
+
+  def unwrap(%__MODULE__{expr: inner}), do: inner
+  def unwrap(other), do: other
+
   @doc "Prepares a filter for comparison"
   defdelegate to_sat_expression(resource, expression), to: Ash.Expr.SAT
-
-  @type t :: any
   @pass_through_funcs [:where, :or_where, :expr, :@]
   @aggregate_kinds Ash.Query.Aggregate.kinds()
 
@@ -49,6 +59,7 @@ defmodule Ash.Expr do
 
   @doc "Returns true if the value is or contains an expression"
   @spec expr?(term) :: boolean()
+  def expr?(%__MODULE__{}), do: true
   def expr?({:_actor, _}), do: true
   def expr?({:_arg, _}), do: true
   def expr?({:_ref, _, _}), do: true
@@ -134,24 +145,32 @@ defmodule Ash.Expr do
     )
   end
 
-  @spec where(Macro.t(), Macro.t()) :: t
   defmacro where(left, right) do
+    left_expr = do_expr(left)
+    right_expr = do_expr(right)
+
     quote do
-      Ash.Query.BooleanExpression.optimized_new(
-        :and,
-        Ash.Expr.expr(unquote(left)),
-        Ash.Expr.expr(unquote(right))
+      Ash.Expr.wrap(
+        Ash.Query.BooleanExpression.optimized_new(
+          :and,
+          unquote(left_expr),
+          unquote(right_expr)
+        )
       )
     end
   end
 
-  @spec or_where(Macro.t(), Macro.t()) :: t
   defmacro or_where(left, right) do
+    left_expr = do_expr(left)
+    right_expr = do_expr(right)
+
     quote do
-      Ash.Query.BooleanExpression.optimized_new(
-        :or,
-        Ash.Expr.expr(unquote(left)),
-        Ash.Expr.expr(unquote(right))
+      Ash.Expr.wrap(
+        Ash.Query.BooleanExpression.optimized_new(
+          :or,
+          unquote(left_expr),
+          unquote(right_expr)
+        )
       )
     end
   end
@@ -168,7 +187,6 @@ defmodule Ash.Expr do
   ])
   ```
   """
-  @spec calc(Macro.t(), opts :: Keyword.t()) :: t()
   defmacro calc(expression, opts \\ []) do
     quote generated: true do
       require Ash.Expr
@@ -193,7 +211,6 @@ defmodule Ash.Expr do
   @doc """
   Creates an expression. See the [Expressions guide](/documentation/topics/reference/expressions.md) for more.
   """
-  @spec expr(Macro.t()) :: t()
   defmacro expr(do: body) do
     quote location: :keep do
       Ash.Expr.expr(unquote(body))
@@ -204,7 +221,7 @@ defmodule Ash.Expr do
     expr = do_expr(body)
 
     quote location: :keep do
-      unquote(expr)
+      Ash.Expr.wrap(unquote(expr))
     end
   end
 
@@ -342,6 +359,10 @@ defmodule Ash.Expr do
 
   def can_return_nil?(nil), do: true
 
+  def can_return_nil?(%__MODULE__{expr: inner}) do
+    can_return_nil?(inner)
+  end
+
   def can_return_nil?(%Ash.Query.BooleanExpression{left: left, right: right}) do
     can_return_nil?(left) || can_return_nil?(right)
   end
@@ -373,6 +394,10 @@ defmodule Ash.Expr do
   end
 
   @doc "Whether or not a given template contains an actor reference"
+  def template_references?(%__MODULE__{expr: inner}, pred) do
+    template_references?(inner, pred)
+  end
+
   def template_references?(%{__struct__: Ash.Filter, expression: expression}, pred) do
     template_references?(expression, pred)
   end
@@ -430,6 +455,17 @@ defmodule Ash.Expr do
   def template_references?(thing, pred), do: pred.(thing)
 
   @doc false
+  def walk_template(%__MODULE__{expr: inner} = wrapper, mapper) do
+    case mapper.(wrapper) do
+      ^wrapper ->
+        walked = walk_template(inner, mapper)
+        wrap(walked)
+
+      other ->
+        walk_template(other, mapper)
+    end
+  end
+
   def walk_template(filter, mapper) when is_list(filter) do
     case mapper.(filter) do
       ^filter ->
@@ -2000,4 +2036,10 @@ defmodule Ash.Expr do
 
   defp remove_pin({:^, _, [value]}), do: value
   defp remove_pin(value), do: value
+
+  defimpl Inspect do
+    def inspect(%Ash.Expr{expr: inner}, opts) do
+      Inspect.inspect(inner, opts)
+    end
+  end
 end

lib/ash/expr/sat.ex

@@ -12,8 +12,8 @@ defmodule Ash.Expr.SAT do
   @dialyzer {:nowarn_function, overlap?: 2}
 
   @doc "Prepares a filter for comparison"
-  @spec to_sat_expression(Ash.Resource.t(), Ash.Expr.t()) ::
-          Crux.Expression.t(Ash.Expr.t())
+  @spec to_sat_expression(Ash.Resource.t(), Ash.Expr.expression()) ::
+          Crux.Expression.t(Ash.Expr.expression())
   def to_sat_expression(resource, expression) do
     expression
     |> consolidate_relationships(resource)
@@ -24,6 +24,7 @@ defmodule Ash.Expr.SAT do
   defp filter_to_expr(nil), do: nil
   defp filter_to_expr(false), do: false
   defp filter_to_expr(true), do: true
+  defp filter_to_expr(%Ash.Expr{expr: inner}), do: filter_to_expr(inner)
   defp filter_to_expr(%Filter{expression: expression}), do: filter_to_expr(expression)
   defp filter_to_expr(%{__predicate__?: _} = op_or_func), do: op_or_func
   defp filter_to_expr(%Ash.Query.Exists{} = exists), do: exists