improvement: error at compile for bypasses that will have no effect

chore: remove unused `checks` field from `%Ash.Policy.Authorizer.Policy{}`

lib/ash/policy/info.ex

@@ -197,7 +197,6 @@ defmodule Ash.Policy.Info do
          %Ash.Policy.Policy{
            policies: policies,
            condition: conditions,
-           checks: checks,
            access_type: access_type
          } = policy,
          default
@@ -206,7 +205,6 @@ defmodule Ash.Policy.Info do
       policy
       | policies: set_access_type(policies, access_type || default),
         condition: set_access_type(conditions, access_type || default),
-        checks: set_access_type(checks, default),
         access_type: access_type || default
     }
   end

lib/ash/policy/policy.ex

@@ -7,7 +7,6 @@ defmodule Ash.Policy.Policy do
     :condition,
     :policies,
     :bypass?,
-    :checks,
     :description,
     :access_type
   ]
@@ -52,14 +51,28 @@ defmodule Ash.Policy.Policy do
 
   @doc false
   def transform(policy) do
-    if Enum.empty?(policy.policies) do
-      {:error, "Policies must have at least one check."}
-    else
-      if policy.condition in [nil, []] do
+    cond do
+      Enum.empty?(policy.policies) ->
+        {:error, "Policies must have at least one check."}
+
+      policy.bypass? &&
+          Enum.all?(List.wrap(policy.policies), &(&1.type in [:forbid_if, :forbid_unless])) ->
+        {:error,
+         """
+         Bypass policies that can only ever forbid have no effect.
+
+         When a bypass is authorized, it skips all remaining policies (including other bypasses)
+         and authorizes the request. If it fails, it is ignored and the remaining policies are checked.
+
+         This policy only contains `forbid_if` or `forbid_unless` check types therefore, it can
+         never have an effect.
+         """}
+
+      policy.condition in [nil, []] ->
         {:ok, %{policy | condition: [{Ash.Policy.Check.Static, result: true}]}}
-      else
+
+      true ->
         {:ok, policy}
-      end
     end
   end
 

test/resource/policies_test.exs

@@ -0,0 +1,27 @@
+defmodule Ash.Test.Resource.PoliciesTest do
+  @moduledoc false
+  use ExUnit.Case, async: true
+
+  test "records can belong to other resources" do
+    assert_raise Spark.Error.DslError,
+                 ~r/Bypass policies that can only ever forbid have no effect/,
+                 fn ->
+                   defmodule HasBadBypassPolicy do
+                     use Ash.Resource,
+                       domain: Ash.Test.Domain,
+                       authorizers: [Ash.Policy.Authorizer]
+
+                     attributes do
+                       uuid_primary_key :id
+                     end
+
+                     policies do
+                       bypass always() do
+                         forbid_if always()
+                         forbid_unless always()
+                       end
+                     end
+                   end
+                 end
+  end
+end