improvement: add can_return_nil?/1 callback to Ash expressions

lib/ash/changeset/changeset.ex

@@ -1439,7 +1439,7 @@ defmodule Ash.Changeset do
         changeset
       else
         value =
-          if attribute.allow_nil? do
+          if attribute.allow_nil? || not Ash.Expr.can_return_nil?(value) do
             value
           else
             expr(

lib/ash/expr/expr.ex

@@ -240,6 +240,30 @@ defmodule Ash.Expr do
     end)
   end
 
+  def can_return_nil?(nil), do: true
+
+  def can_return_nil?(%Ash.Query.BooleanExpression{left: left, right: right}) do
+    can_return_nil?(left) || can_return_nil?(right)
+  end
+
+  def can_return_nil?(%Ash.Query.Not{expression: expression}) do
+    can_return_nil?(expression)
+  end
+
+  def can_return_nil?(%Ash.Query.Parent{expr: expr}) do
+    can_return_nil?(expr)
+  end
+
+  def can_return_nil?(%Ash.Query.Exists{}), do: false
+
+  def can_return_nil?(%mod{__predicate__?: _} = pred) do
+    mod.can_return_nil?(pred)
+  end
+
+  def can_return_nil?(%Ash.Query.Ref{attribute: %{allow_nil?: false}}), do: false
+
+  def can_return_nil?(_), do: true
+
   @doc "Whether or not a given template contains an actor reference"
   def template_references?(%BooleanExpression{op: :and, left: left, right: right}, pred) do
     template_references?(left, pred) || template_references?(right, pred)

lib/ash/query/function/ago.ex

@@ -66,4 +66,6 @@ defmodule Ash.Query.Function.Ago do
       result
     end
   end
+
+  def can_return_nil?(_), do: false
 end

lib/ash/query/function/at.ex

@@ -10,4 +10,8 @@ defmodule Ash.Query.Function.At do
   def evaluate(%{arguments: [list, at]}) do
     {:known, Enum.at(list, at)}
   end
+
+  def can_return_nil?(%{arguments: [value | _]}) do
+    Ash.Expr.can_return_nil?(value)
+  end
 end

lib/ash/query/function/composite_type.ex

@@ -37,4 +37,8 @@ defmodule Ash.Query.Function.CompositeType do
         {:error, error}
     end
   end
+
+  def can_return_nil?(%{arguments: [value | _]}) do
+    Ash.Expr.can_return_nil?(value)
+  end
 end

lib/ash/query/function/contains.ex

@@ -57,4 +57,8 @@ defmodule Ash.Query.Function.Contains do
   def evaluate(_other) do
     :unknown
   end
+
+  def can_return_nil?(%{arguments: arguments}) do
+    Enum.any?(arguments, &Ash.Expr.can_return_nil?/1)
+  end
 end

lib/ash/query/function/count_nils.ex

@@ -19,4 +19,8 @@ defmodule Ash.Query.Function.CountNils do
   def evaluate(_) do
     {:error, "Cannot use count_nils/1 on non-list inputs"}
   end
+
+  def can_return_nil?(%{arguments: [date | _]}) do
+    Ash.Expr.can_return_nil?(date)
+  end
 end

lib/ash/query/function/date_add.ex

@@ -21,4 +21,8 @@ defmodule Ash.Query.Function.DateAdd do
       {:known, truncated}
     end
   end
+
+  def can_return_nil?(%{arguments: [date | _]}) do
+    Ash.Expr.can_return_nil?(date)
+  end
 end

lib/ash/query/function/datetime_add.ex

@@ -16,4 +16,8 @@ defmodule Ash.Query.Function.DateTimeAdd do
     shifted = Ash.Query.Function.Ago.datetime_add(datetime, factor, interval)
     {:known, shifted}
   end
+
+  def can_return_nil?(%{arguments: [datetime | _]}) do
+    Ash.Expr.can_return_nil?(datetime)
+  end
 end

lib/ash/query/function/exists.ex

@@ -19,6 +19,8 @@ defmodule Ash.Query.Exists do
     %__MODULE__{path: path, expr: expr, at_path: at_path}
   end
 
+  def can_return_nil?(_), do: false
+
   defimpl Inspect do
     import Inspect.Algebra
 

lib/ash/query/function/from_now.ex

@@ -17,4 +17,6 @@ defmodule Ash.Query.Function.FromNow do
     shifted = Ash.Query.Function.Ago.datetime_add(now, factor, interval)
     {:known, shifted}
   end
+
+  def can_return_nil?(_), do: false
 end

lib/ash/query/function/function.ex

@@ -13,13 +13,22 @@ defmodule Ash.Query.Function do
   The number and types of arguments supported.
   """
   @callback args() :: [arg] | :var_args
+  @doc "The name of the function"
   @callback name() :: atom
+  @doc "Instantiate a new function with the provided arguments"
   @callback new(list(term)) :: {:ok, term} | {:error, String.t() | Exception.t()}
+  @doc "Evaluate a function when all arguments are known valid values"
   @callback evaluate(func :: map) :: :unknown | {:known, term} | {:error, term}
+  @doc "Evaluate a function when some or no arguments are known valid values"
   @callback partial_evaluate(func) :: {:ok, func} | {:error, term} when func: map
+  @doc "Whether or not the function can be evaluated eagerly. For example, `now()` cannot be."
   @callback eager_evaluate?() :: boolean()
+  @doc "Whether or not the function is a predicate (takes a reference as the first argument, a value as the second, and returns a boolean)"
   @callback predicate?() :: boolean()
+  @doc "Whether or not the function should be usable when parsing input."
   @callback private?() :: boolean
+  @doc "Whether or not the function return nil."
+  @callback can_return_nil?(func :: map) :: boolean()
 
   @doc """
   If `true`, will be allowed to evaluate `nil` inputs.
@@ -224,7 +233,10 @@ defmodule Ash.Query.Function do
       @impl Ash.Query.Function
       def private?, do: false
 
-      defoverridable new: 1, evaluate: 1, private?: 0, evaluate_nil_inputs?: 0
+      @impl Ash.Query.Function
+      def can_return_nil?(_), do: true
+
+      defoverridable new: 1, evaluate: 1, private?: 0, evaluate_nil_inputs?: 0, can_return_nil?: 1
 
       unless unquote(opts[:no_inspect?]) do
         defimpl Inspect do

lib/ash/query/function/if.ex

@@ -32,6 +32,10 @@ defmodule Ash.Query.Function.If do
     super([condition, block, else_block])
   end
 
+  def can_return_nil?(%{arguments: [_ | rest]}) do
+    Enum.any?(rest, &Ash.Expr.can_return_nil?/1)
+  end
+
   def evaluate(%{arguments: [true, when_true, _]}),
     do: {:known, when_true}
 

lib/ash/query/function/is_nil.ex

@@ -15,4 +15,6 @@ defmodule Ash.Query.Function.IsNil do
   def evaluate(%{arguments: [val]}) do
     {:known, is_nil(val)}
   end
+
+  def can_return_nil?(_), do: false
 end

lib/ash/query/function/length.ex

@@ -24,4 +24,8 @@ defmodule Ash.Query.Function.Length do
   def evaluate(_) do
     {:error, "Cannot use length/1 on non-list inputs"}
   end
+
+  def can_return_nil?(%{arguments: [val]}) do
+    Ash.Expr.can_return_nil?(val)
+  end
 end

lib/ash/query/function/minus.ex

@@ -10,4 +10,8 @@ defmodule Ash.Query.Function.Minus do
     {:ok, op} = Ash.Query.Operator.Basic.Times.new(val, -1)
     Ash.Query.Operator.evaluate(op)
   end
+
+  def can_return_nil?(%{arguments: [val]}) do
+    Ash.Expr.can_return_nil?(val)
+  end
 end

lib/ash/query/function/now.ex

@@ -7,4 +7,6 @@ defmodule Ash.Query.Function.Now do
   def args, do: [[]]
 
   def evaluate(_), do: {:known, DateTime.utc_now()}
+
+  def can_return_nil?(_), do: false
 end

lib/ash/query/function/round.ex

@@ -24,4 +24,8 @@ defmodule Ash.Query.Function.Round do
   def evaluate(%{arguments: [num, precision]}) do
     {:known, num |> Decimal.round(precision) |> Decimal.normalize()}
   end
+
+  def can_return_nil?(%{arguments: [string]}) do
+    Ash.Expr.can_return_nil?(string)
+  end
 end

lib/ash/query/function/string_downcase.ex

@@ -22,4 +22,8 @@ defmodule Ash.Query.Function.StringDowncase do
   def evaluate(%{arguments: [value]}) do
     {:known, String.downcase(value)}
   end
+
+  def can_return_nil?(%{arguments: [string]}) do
+    Ash.Expr.can_return_nil?(string)
+  end
 end

lib/ash/query/function/string_join.ex

@@ -59,4 +59,8 @@ defmodule Ash.Query.Function.StringJoin do
       _ -> false
     end)
   end
+
+  def can_return_nil?(%{arguments: [string | _]}) do
+    Ash.Expr.can_return_nil?(string)
+  end
 end

lib/ash/query/function/string_length.ex

@@ -18,4 +18,8 @@ defmodule Ash.Query.Function.StringLength do
   def evaluate(%{arguments: [value]}) do
     {:known, String.length(value)}
   end
+
+  def can_return_nil?(%{arguments: [string]}) do
+    Ash.Expr.can_return_nil?(string)
+  end
 end

lib/ash/query/function/string_split.ex

@@ -82,4 +82,8 @@ defmodule Ash.Query.Function.StringSplit do
       {:known, String.split(value, delimiter, split_opts)}
     end
   end
+
+  def can_return_nil?(%{arguments: [string]}) do
+    Ash.Expr.can_return_nil?(string)
+  end
 end

lib/ash/query/function/string_trim.ex

@@ -20,4 +20,8 @@ defmodule Ash.Query.Function.StringTrim do
   def evaluate(%{arguments: [value]}) do
     {:known, String.trim(value)}
   end
+
+  def can_return_nil?(%{arguments: [string]}) do
+    Ash.Expr.can_return_nil?(string)
+  end
 end

lib/ash/query/function/today.ex

@@ -7,4 +7,6 @@ defmodule Ash.Query.Function.Today do
   def args, do: [[]]
 
   def evaluate(_), do: {:known, Date.utc_today()}
+
+  def can_return_nil?(_), do: false
 end

lib/ash/query/function/type.ex

@@ -20,6 +20,10 @@ defmodule Ash.Query.Function.Type do
     end
   end
 
+  def can_return_nil?(%{arguments: [value | _]}) do
+    Ash.Expr.can_return_nil?(value)
+  end
+
   def evaluate(%{arguments: [val, type, constraints]}) do
     case Ash.Type.cast_input(type, val, constraints) do
       {:ok, value} ->

lib/ash/query/operator/basic.ex

@@ -189,6 +189,14 @@ defmodule Ash.Query.Operator.Basic do
           end
         end
 
+        def can_return_nil?(%{operator: :||, right: right}) do
+          Ash.Expr.can_return_nil?(right)
+        end
+
+        def can_return_nil?(%{left: left, right: right} = op) do
+          Ash.Expr.can_return_nil?(left) or Ash.Expr.can_return_nil?(right)
+        end
+
         defp to_decimal(value) when is_float(value) do
           Decimal.from_float(value)
         end

lib/ash/query/operator/eq.ex

@@ -24,6 +24,9 @@ defmodule Ash.Query.Operator.Eq do
     {:known, Comp.equal?(left, right)}
   end
 
+  def can_return_nil?(%{left: left, right: right}),
+    do: Ash.Expr.can_return_nil?(left) or Ash.Expr.can_return_nil?(right)
+
   @impl Ash.Filter.Predicate
   def bulk_compare(predicates) do
     predicates

lib/ash/query/operator/greater_than.ex

@@ -17,6 +17,9 @@ defmodule Ash.Query.Operator.GreaterThan do
     {:known, Comp.greater_than?(left, right)}
   end
 
+  def can_return_nil?(%{left: left, right: right}),
+    do: Ash.Expr.can_return_nil?(left) or Ash.Expr.can_return_nil?(right)
+
   @impl Ash.Filter.Predicate
   def simplify(%__MODULE__{left: %Ref{} = ref, right: %Date{} = value}) do
     {:ok, op} = Ash.Query.Operator.new(Ash.Query.Operator.LessThan, ref, Date.add(value, 1))

lib/ash/query/operator/greater_than_or_equal.ex

@@ -16,6 +16,9 @@ defmodule Ash.Query.Operator.GreaterThanOrEqual do
   def evaluate(%{left: left, right: right}),
     do: {:known, Comp.greater_or_equal?(left, right)}
 
+  def can_return_nil?(%{left: left, right: right}),
+    do: Ash.Expr.can_return_nil?(left) or Ash.Expr.can_return_nil?(right)
+
   @impl Ash.Filter.Predicate
   def simplify(%__MODULE__{left: %Ref{} = ref, right: value}) do
     {:ok, op} = Ash.Query.Operator.new(Ash.Query.Operator.LessThan, ref, value)

lib/ash/query/operator/in.ex

@@ -21,6 +21,8 @@ defmodule Ash.Query.Operator.In do
 
   def new(left, right), do: {:ok, %__MODULE__{left: left, right: right}}
 
+  def can_return_nil?(%{left: left}), do: Ash.Expr.can_return_nil?(left)
+
   def evaluate(%{left: nil}), do: {:known, nil}
   def evaluate(%{right: nil}), do: {:known, nil}
 

lib/ash/query/operator/is_nil.ex

@@ -14,7 +14,11 @@ defmodule Ash.Query.Operator.IsNil do
   def new(nil, false), do: {:ok, false}
 
   def new(left, right) do
-    super(left, right)
+    if right == false and not Ash.Expr.can_return_nil?(left) do
+      {:known, false}
+    else
+      super(left, right)
+    end
   end
 
   @impl Ash.Query.Operator
@@ -55,6 +59,8 @@ defmodule Ash.Query.Operator.IsNil do
     end
   end
 
+  def can_return_nil?(%{right: right}), do: Ash.Expr.can_return_nil?(right)
+
   @impl Ash.Filter.Predicate
   def compare(%__MODULE__{left: %Ref{} = same_ref, right: true}, %Ash.Query.Operator.Eq{
         left: %Ref{} = same_ref,

lib/ash/query/operator/less_than.ex

@@ -120,4 +120,7 @@ defmodule Ash.Query.Operator.LessThan do
         end)
     end
   end
+
+  def can_return_nil?(%{left: left, right: right}),
+    do: Ash.Expr.can_return_nil?(left) or Ash.Expr.can_return_nil?(right)
 end

lib/ash/query/operator/less_than_or_equal.ex

@@ -31,4 +31,7 @@ defmodule Ash.Query.Operator.LessThanOrEqual do
   end
 
   def simplify(_), do: nil
+
+  def can_return_nil?(%{left: left, right: right}),
+    do: Ash.Expr.can_return_nil?(left) or Ash.Expr.can_return_nil?(right)
 end

lib/ash/query/operator/not_eq.ex

@@ -24,4 +24,7 @@ defmodule Ash.Query.Operator.NotEq do
   def simplify(%__MODULE__{left: left, right: right}) do
     %Not{expression: %Eq{left: left, right: right}}
   end
+
+  def can_return_nil?(%{left: left, right: right}),
+    do: Ash.Expr.can_return_nil?(left) or Ash.Expr.can_return_nil?(right)
 end

lib/ash/query/operator/operator.ex

@@ -47,6 +47,11 @@ defmodule Ash.Query.Operator do
 
   @callback predicate?() :: boolean()
 
+  @doc """
+  Whether or not the operator can evaluate to nil.
+  """
+  @callback can_return_nil?(func :: map) :: boolean()
+
   @doc "Evaluate the operator with provided inputs"
   def evaluate(%mod{left: left, right: right} = op) when is_nil(left) or is_nil(right) do
     if mod.evaluate_nil_inputs?() do
@@ -408,7 +413,10 @@ defmodule Ash.Query.Operator do
         ])
       end
 
-      defoverridable to_string: 2, new: 2, evaluate_nil_inputs?: 0
+      @impl Ash.Query.Operator
+      def can_return_nil?(_), do: true
+
+      defoverridable to_string: 2, new: 2, evaluate_nil_inputs?: 0, can_return_nil?: 1
 
       defimpl Inspect do
         def inspect(%mod{} = op, opts) do