Use never_say_never hack to work around Rust 2024 regression for fn traits (#18804)

# Objective

After #17967, closures which always panic no longer satisfy various Bevy
traits. Principally, this affects observers, systems and commands.

While this may seem pointless (systems which always panic are kind of
useless), it is distinctly annoying when using the `todo!` macro, or
when writing tests that should panic.

Fixes #18778.

## Solution

- Add failing tests to demonstrate the problem
- Add the trick from
[`never_say_never`](https://docs.rs/never-say-never/latest/never_say_never/)
to name the `!` type on stable Rust
- Write looots of docs explaining what the heck is going on and why
we've done this terrible thing

## To do

Unfortunately I couldn't figure out how to avoid conflicting impls, and
I am out of time for today, the week and uh the week after that.
Vacation! If you feel like finishing this for me, please submit PRs to
my branch and I can review and press the button for it while I'm off.

Unless you're Cart, in which case you have write permissions to my
branch!

- [ ] fix for commands
- [ ] fix for systems
- [ ] fix for observers
- [ ] revert bevyengine/bevy-website#2092

## Testing

I've added a compile test for these failure cases and a few adjacent
non-failing cases (with explicit return types).

---------

Co-authored-by: Carter Anderson <[email protected]>

Author: alice-i-cecile

crates/bevy_ecs/src/error/command_handling.rs

@@ -2,6 +2,7 @@ use core::{any::type_name, fmt};
 
 use crate::{
     entity::Entity,
+    never::Never,
     system::{entity_command::EntityCommandError, Command, EntityCommand},
     world::{error::EntityMutableFetchError, World},
 };
@@ -42,6 +43,17 @@ where
     }
 }
 
+impl<C> HandleError<Never> for C
+where
+    C: Command<Never>,
+{
+    fn handle_error_with(self, _error_handler: fn(BevyError, ErrorContext)) -> impl Command {
+        move |world: &mut World| {
+            self.apply(world);
+        }
+    }
+}
+
 impl<C> HandleError for C
 where
     C: Command,

crates/bevy_ecs/src/lib.rs

@@ -43,6 +43,7 @@ pub mod identifier;
 pub mod intern;
 pub mod label;
 pub mod name;
+pub mod never;
 pub mod observer;
 pub mod query;
 #[cfg(feature = "bevy_reflect")]

crates/bevy_ecs/src/never.rs

@@ -0,0 +1,39 @@
+//! A workaround for the `!` type in stable Rust.
+//!
+//! This approach is taken from the [`never_say_never`] crate,
+//! reimplemented here to avoid adding a new dependency.
+//!
+//! This module exists due to a change in [never type fallback inference] in the Rust 2024 edition.
+//! This caused failures in `bevy_ecs`'s traits which are implemented for functions
+//! (like [`System`](crate::system::System)) when working with panicking closures.
+//!
+//! Note that using this hack is not recommended in general;
+//! by doing so you are knowingly opting out of rustc's stability guarantees.
+//! Code that compiles due to this hack may break in future versions of Rust.
+//!
+//! Please read [issue #18778](https://github.com/bevyengine/bevy/issues/18778) for an explanation of why
+//! Bevy has chosen to use this workaround.
+//!
+//! [`never_say_never`]: https://crates.io/crates/never_say_never
+//! [never type fallback inference]: https://doc.rust-lang.org/edition-guide/rust-2024/never-type-fallback.html
+
+mod fn_ret {
+    /// A helper trait for naming the ! type.
+    #[doc(hidden)]
+    pub trait FnRet {
+        /// The return type of the function.
+        type Output;
+    }
+
+    /// This blanket implementation allows us to name the never type,
+    /// by using the associated type of this trait for `fn() -> !`.
+    impl<R> FnRet for fn() -> R {
+        type Output = R;
+    }
+}
+
+/// A hacky type alias for the `!` (never) type.
+///
+/// This knowingly opts out of rustc's stability guarantees.
+/// Read the module documentation carefully before using this!
+pub type Never = <fn() -> ! as fn_ret::FnRet>::Output;

crates/bevy_ecs/src/schedule/config.rs

@@ -3,6 +3,7 @@ use variadics_please::all_tuples;
 
 use crate::{
     error::Result,
+    never::Never,
     schedule::{
         auto_insert_apply_deferred::IgnoreDeferred,
         condition::{BoxedCondition, Condition},
@@ -570,6 +571,16 @@ where
     }
 }
 
+impl<F, Marker> IntoScheduleConfigs<ScheduleSystem, (Never, Marker)> for F
+where
+    F: IntoSystem<(), Never, Marker>,
+{
+    fn into_configs(self) -> ScheduleConfigs<ScheduleSystem> {
+        let wrapper = InfallibleSystemWrapper::new(IntoSystem::into_system(self));
+        ScheduleConfigs::ScheduleConfig(ScheduleSystem::into_config(Box::new(wrapper)))
+    }
+}
+
 /// Marker component to allow for conflicting implementations of [`IntoScheduleConfigs`]
 #[doc(hidden)]
 pub struct Fallible;

crates/bevy_ecs/src/system/mod.rs

@@ -337,7 +337,8 @@ mod tests {
         component::{Component, Components},
         entity::{Entities, Entity},
         error::Result,
-        prelude::{AnyOf, EntityRef},
+        name::Name,
+        prelude::{AnyOf, EntityRef, Trigger},
         query::{Added, Changed, Or, With, Without},
         removal_detection::RemovedComponents,
         resource::Resource,
@@ -349,7 +350,7 @@ mod tests {
             Commands, In, IntoSystem, Local, NonSend, NonSendMut, ParamSet, Query, Res, ResMut,
             Single, StaticSystemParam, System, SystemState,
         },
-        world::{DeferredWorld, EntityMut, FromWorld, World},
+        world::{DeferredWorld, EntityMut, FromWorld, OnAdd, World},
     };
 
     use super::ScheduleSystem;
@@ -1823,4 +1824,59 @@ mod tests {
         let mut world = World::new();
         run_system(&mut world, sys);
     }
+
+    // Regression test for
+    // https://github.com/bevyengine/bevy/issues/18778
+    //
+    // Dear rustc team, please reach out if you encounter this
+    // in a crater run and we can work something out!
+    //
+    // These todo! macro calls should never be removed;
+    // they're intended to demonstrate real-world usage
+    // in a way that's clearer than simply calling `panic!`
+    //
+    // Because type inference behaves differently for functions and closures,
+    // we need to test both, in addition to explicitly annotating the return type
+    // to ensure that there are no upstream regressions there.
+    #[test]
+    fn nondiverging_never_trait_impls() {
+        // This test is a compilation test:
+        // no meaningful logic is ever actually evaluated.
+        // It is simply intended to check that the correct traits are implemented
+        // when todo! or similar nondiverging panics are used.
+        let mut world = World::new();
+        let mut schedule = Schedule::default();
+
+        fn sys(_query: Query<&Name>) {
+            todo!()
+        }
+
+        schedule.add_systems(sys);
+        schedule.add_systems(|_query: Query<&Name>| {});
+        schedule.add_systems(|_query: Query<&Name>| todo!());
+        #[expect(clippy::unused_unit, reason = "this forces the () return type")]
+        schedule.add_systems(|_query: Query<&Name>| -> () { todo!() });
+
+        fn obs(_trigger: Trigger<OnAdd, Name>) {
+            todo!()
+        }
+
+        world.add_observer(obs);
+        world.add_observer(|_trigger: Trigger<OnAdd, Name>| {});
+        world.add_observer(|_trigger: Trigger<OnAdd, Name>| todo!());
+        #[expect(clippy::unused_unit, reason = "this forces the () return type")]
+        world.add_observer(|_trigger: Trigger<OnAdd, Name>| -> () { todo!() });
+
+        fn my_command(_world: &mut World) {
+            todo!()
+        }
+
+        world.commands().queue(my_command);
+        world.commands().queue(|_world: &mut World| {});
+        world.commands().queue(|_world: &mut World| todo!());
+        #[expect(clippy::unused_unit, reason = "this forces the () return type")]
+        world
+            .commands()
+            .queue(|_world: &mut World| -> () { todo!() });
+    }
 }

crates/bevy_ecs/src/system/observer_system.rs

@@ -5,6 +5,7 @@ use crate::{
     archetype::ArchetypeComponentId,
     component::{ComponentId, Tick},
     error::Result,
+    never::Never,
     prelude::{Bundle, Trigger},
     query::Access,
     schedule::{Fallible, Infallible},
@@ -45,7 +46,7 @@ pub trait IntoObserverSystem<E: 'static, B: Bundle, M, Out = Result>: Send + 'st
     fn into_system(this: Self) -> Self::System;
 }
 
-impl<E, B, M, Out, S> IntoObserverSystem<E, B, (Fallible, M), Out> for S
+impl<E, B, M, S, Out> IntoObserverSystem<E, B, (Fallible, M), Out> for S
 where
     S: IntoSystem<Trigger<'static, E, B>, Out, M> + Send + 'static,
     S::System: ObserverSystem<E, B, Out>,
@@ -66,20 +67,32 @@ where
     E: Send + Sync + 'static,
     B: Bundle,
 {
-    type System = InfallibleObserverWrapper<E, B, S::System>;
+    type System = InfallibleObserverWrapper<E, B, S::System, ()>;
+
+    fn into_system(this: Self) -> Self::System {
+        InfallibleObserverWrapper::new(IntoSystem::into_system(this))
+    }
+}
+impl<E, B, M, S> IntoObserverSystem<E, B, (Never, M), Result> for S
+where
+    S: IntoSystem<Trigger<'static, E, B>, Never, M> + Send + 'static,
+    E: Send + Sync + 'static,
+    B: Bundle,
+{
+    type System = InfallibleObserverWrapper<E, B, S::System, Never>;
 
     fn into_system(this: Self) -> Self::System {
         InfallibleObserverWrapper::new(IntoSystem::into_system(this))
     }
 }
 
 /// A wrapper that converts an observer system that returns `()` into one that returns `Ok(())`.
-pub struct InfallibleObserverWrapper<E, B, S> {
+pub struct InfallibleObserverWrapper<E, B, S, Out> {
     observer: S,
-    _marker: PhantomData<(E, B)>,
+    _marker: PhantomData<(E, B, Out)>,
 }
 
-impl<E, B, S> InfallibleObserverWrapper<E, B, S> {
+impl<E, B, S, Out> InfallibleObserverWrapper<E, B, S, Out> {
     /// Create a new `InfallibleObserverWrapper`.
     pub fn new(observer: S) -> Self {
         Self {
@@ -89,11 +102,12 @@ impl<E, B, S> InfallibleObserverWrapper<E, B, S> {
     }
 }
 
-impl<E, B, S> System for InfallibleObserverWrapper<E, B, S>
+impl<E, B, S, Out> System for InfallibleObserverWrapper<E, B, S, Out>
 where
-    S: ObserverSystem<E, B, ()>,
+    S: ObserverSystem<E, B, Out>,
     E: Send + Sync + 'static,
     B: Bundle,
+    Out: Send + Sync + 'static,
 {
     type In = Trigger<'static, E, B>;
     type Out = Result;

crates/bevy_ecs/src/system/schedule_system.rs

@@ -12,16 +12,16 @@ use crate::{
 use super::{IntoSystem, SystemParamValidationError};
 
 /// A wrapper system to change a system that returns `()` to return `Ok(())` to make it into a [`ScheduleSystem`]
-pub struct InfallibleSystemWrapper<S: System<In = (), Out = ()>>(S);
+pub struct InfallibleSystemWrapper<S: System<In = ()>>(S);
 
-impl<S: System<In = (), Out = ()>> InfallibleSystemWrapper<S> {
+impl<S: System<In = ()>> InfallibleSystemWrapper<S> {
     /// Create a new `OkWrapperSystem`
     pub fn new(system: S) -> Self {
         Self(IntoSystem::into_system(system))
     }
 }
 
-impl<S: System<In = (), Out = ()>> System for InfallibleSystemWrapper<S> {
+impl<S: System<In = ()>> System for InfallibleSystemWrapper<S> {
     type In = ();
     type Out = Result;