Skip to content

Commit

Permalink
Add on_unimplemented Diagnostics to Most Public Traits (#13347) (#1…
Browse files Browse the repository at this point in the history
…3662)

# Objective

- #13414 did not have the intended effect.
- #13404 is still blocked

## Solution

- Re-adds #13347.

Co-authored-by: Zachary Harrold <[email protected]>
Co-authored-by: Jamie Ridding <[email protected]>
Co-authored-by: BD103 <[email protected]>
  • Loading branch information
4 people authored Jun 4, 2024
1 parent 37cc8ea commit ec7b349
Show file tree
Hide file tree
Showing 30 changed files with 129 additions and 12 deletions.
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ keywords = ["game", "engine", "gamedev", "graphics", "bevy"]
license = "MIT OR Apache-2.0"
repository = "https://github.com/bevyengine/bevy"
documentation = "https://docs.rs/bevy"
rust-version = "1.77.0"
rust-version = "1.78.0"

[workspace]
exclude = [
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_app/src/plugin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ use std::any::Any;
/// }
/// }
/// # fn damp_flickering() {}
/// ````
/// ```
pub trait Plugin: Downcast + Any + Send + Sync {
/// Configures the [`App`] to which this plugin is added.
fn build(&self, app: &mut App);
Expand Down
5 changes: 5 additions & 0 deletions crates/bevy_asset/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,11 @@ impl Plugin for AssetPlugin {
}
}

#[diagnostic::on_unimplemented(
message = "`{Self}` is not an `Asset`",
label = "invalid `Asset`",
note = "consider annotating `{Self}` with `#[derive(Asset)]`"
)]
pub trait Asset: VisitAssetDependencies + TypePath + Send + Sync + 'static {}

pub trait VisitAssetDependencies {
Expand Down
5 changes: 5 additions & 0 deletions crates/bevy_ecs/src/bundle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,11 @@ use std::ptr::NonNull;
// bundle, in the _exact_ order that [`DynamicBundle::get_components`] is called.
// - [`Bundle::from_components`] must call `func` exactly once for each [`ComponentId`] returned by
// [`Bundle::component_ids`].
#[diagnostic::on_unimplemented(
message = "`{Self}` is not a `Bundle`",
label = "invalid `Bundle`",
note = "consider annotating `{Self}` with `#[derive(Component)]` or `#[derive(Bundle)]`"
)]
pub unsafe trait Bundle: DynamicBundle + Send + Sync + 'static {
/// Gets this [`Bundle`]'s component ids, in the order of this bundle's [`Component`]s
#[doc(hidden)]
Expand Down
5 changes: 5 additions & 0 deletions crates/bevy_ecs/src/component.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,11 @@ use std::{
///
/// [`SyncCell`]: bevy_utils::synccell::SyncCell
/// [`Exclusive`]: https://doc.rust-lang.org/nightly/std/sync/struct.Exclusive.html
#[diagnostic::on_unimplemented(
message = "`{Self}` is not a `Component`",
label = "invalid `Component`",
note = "consider annotating `{Self}` with `#[derive(Component)]`"
)]
pub trait Component: Send + Sync + 'static {
/// A constant indicating the storage type used for this component.
const STORAGE_TYPE: StorageType;
Expand Down
1 change: 0 additions & 1 deletion crates/bevy_ecs/src/entity/map_entities.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ use super::EntityHashMap;
/// }
/// }
/// ```
///
pub trait MapEntities {
/// Updates all [`Entity`] references stored inside using `entity_mapper`.
///
Expand Down
5 changes: 5 additions & 0 deletions crates/bevy_ecs/src/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ use std::{
/// You can conveniently access events using the [`EventReader`] and [`EventWriter`] system parameter.
///
/// Events must be thread-safe.
#[diagnostic::on_unimplemented(
message = "`{Self}` is not an `Event`",
label = "invalid `Event`",
note = "consider annotating `{Self}` with `#[derive(Event]`"
)]
pub trait Event: Send + Sync + 'static {}

/// An `EventId` uniquely identifies an event stored in a specific [`World`].
Expand Down
4 changes: 4 additions & 0 deletions crates/bevy_ecs/src/query/fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,10 @@ use std::{cell::UnsafeCell, marker::PhantomData};
///
/// [`Query`]: crate::system::Query
/// [`ReadOnly`]: Self::ReadOnly
#[diagnostic::on_unimplemented(
message = "`{Self}` is not valid to request as data in a `Query`",
label = "invalid `Query` data"
)]
pub unsafe trait QueryData: WorldQuery {
/// The read-only variant of this [`QueryData`], which satisfies the [`ReadOnlyQueryData`] trait.
type ReadOnly: ReadOnlyQueryData<State = <Self as WorldQuery>::State>;
Expand Down
11 changes: 10 additions & 1 deletion crates/bevy_ecs/src/query/filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,11 @@ use std::{cell::UnsafeCell, marker::PhantomData};
/// [`matches_component_set`]: Self::matches_component_set
/// [`Query`]: crate::system::Query
/// [`State`]: Self::State
#[diagnostic::on_unimplemented(
message = "`{Self}` is not a valid `Query` filter",
label = "invalid `Query` filter",
note = "a `QueryFilter` typically uses a combination of `With<T>` and `Without<T>` statements"
)]
pub trait QueryFilter: WorldQuery {
/// Returns true if (and only if) this Filter relies strictly on archetypes to limit which
/// components are accessed by the Query.
Expand Down Expand Up @@ -942,6 +946,11 @@ impl<T: Component> QueryFilter for Changed<T> {
///
/// [`Added`] and [`Changed`] works with entities, and therefore are not archetypal. As such
/// they do not implement [`ArchetypeFilter`].
#[diagnostic::on_unimplemented(
message = "`{Self}` is not a valid `Query` filter based on archetype information",
label = "invalid `Query` filter",
note = "an `ArchetypeFilter` typically uses a combination of `With<T>` and `Without<T>` statements"
)]
pub trait ArchetypeFilter: QueryFilter {}

impl<T: Component> ArchetypeFilter for With<T> {}
Expand Down
8 changes: 8 additions & 0 deletions crates/bevy_ecs/src/schedule/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,10 @@ impl<T> NodeConfigs<T> {
/// )
/// );
/// ```
#[diagnostic::on_unimplemented(
message = "`{Self}` does not describe a valid system configuration",
label = "invalid system configuration"
)]
pub trait IntoSystemConfigs<Marker>
where
Self: Sized,
Expand Down Expand Up @@ -562,6 +566,10 @@ impl SystemSetConfig {
pub type SystemSetConfigs = NodeConfigs<InternedSystemSet>;

/// Types that can convert into a [`SystemSetConfigs`].
#[diagnostic::on_unimplemented(
message = "`{Self}` does not describe a valid system set configuration",
label = "invalid system set configuration"
)]
pub trait IntoSystemSetConfigs
where
Self: Sized,
Expand Down
4 changes: 4 additions & 0 deletions crates/bevy_ecs/src/schedule/set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,10 @@ impl SystemSet for AnonymousSet {
}

/// Types that can be converted into a [`SystemSet`].
#[diagnostic::on_unimplemented(
message = "`{Self}` is not a system set",
label = "invalid system set"
)]
pub trait IntoSystemSet<Marker>: Sized {
/// The type of [`SystemSet`] this instance converts into.
type Set: SystemSet;
Expand Down
4 changes: 4 additions & 0 deletions crates/bevy_ecs/src/system/adapter_system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ use crate::{schedule::InternedSystemSet, world::unsafe_world_cell::UnsafeWorldCe
/// # system.initialize(&mut world);
/// # assert!(system.run((), &mut world));
/// ```
#[diagnostic::on_unimplemented(
message = "`{Self}` can not adapt a system of type `{S}`",
label = "invalid system adapter"
)]
pub trait Adapt<S: System>: Send + Sync + 'static {
/// The [input](System::In) type for an [`AdapterSystem`].
type In;
Expand Down
5 changes: 5 additions & 0 deletions crates/bevy_ecs/src/system/combinator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,11 @@ use super::{ReadOnlySystem, System};
/// # assert!(world.resource::<RanFlag>().0);
/// # world.resource_mut::<RanFlag>().0 = false;
/// ```
#[diagnostic::on_unimplemented(
message = "`{Self}` can not combine systems `{A}` and `{B}`",
label = "invalid system combination",
note = "the inputs and outputs of `{A}` and `{B}` are not compatible with this combiner"
)]
pub trait Combine<A: System, B: System> {
/// The [input](System::In) type for a [`CombinatorSystem`].
type In;
Expand Down
6 changes: 5 additions & 1 deletion crates/bevy_ecs/src/system/exclusive_function_system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,14 +161,18 @@ where
///
/// This trait can be useful for making your own systems which accept other systems,
/// sometimes called higher order systems.
#[diagnostic::on_unimplemented(
message = "`{Self}` is not an exclusive system",
label = "invalid system"
)]
pub trait ExclusiveSystemParamFunction<Marker>: Send + Sync + 'static {
/// The input type to this system. See [`System::In`].
type In;

/// The return type of this system. See [`System::Out`].
type Out;

/// The [`ExclusiveSystemParam`]/s defined by this system's `fn` parameters.
/// The [`ExclusiveSystemParam`]'s defined by this system's `fn` parameters.
type Param: ExclusiveSystemParam;

/// Executes this system once. See [`System::run`].
Expand Down
4 changes: 4 additions & 0 deletions crates/bevy_ecs/src/system/exclusive_system_param.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ use std::marker::PhantomData;

/// A parameter that can be used in an exclusive system (a system with an `&mut World` parameter).
/// Any parameters implementing this trait must come after the `&mut World` parameter.
#[diagnostic::on_unimplemented(
message = "`{Self}` can not be used as a parameter for an exclusive system",
label = "invalid system parameter"
)]
pub trait ExclusiveSystemParam: Sized {
/// Used to store data which persists across invocations of a system.
type State: Send + Sync + 'static;
Expand Down
4 changes: 4 additions & 0 deletions crates/bevy_ecs/src/system/function_system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -661,6 +661,10 @@ where
/// ```
/// [`PipeSystem`]: crate::system::PipeSystem
/// [`ParamSet`]: crate::system::ParamSet
#[diagnostic::on_unimplemented(
message = "`{Self}` is not a valid system",
label = "invalid system"
)]
pub trait SystemParamFunction<Marker>: Send + Sync + 'static {
/// The input type to this system. See [`System::In`].
type In;
Expand Down
4 changes: 4 additions & 0 deletions crates/bevy_ecs/src/system/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,10 @@ use crate::world::World;
// This trait has to be generic because we have potentially overlapping impls, in particular
// because Rust thinks a type could impl multiple different `FnMut` combinations
// even though none can currently
#[diagnostic::on_unimplemented(
message = "`{Self}` is not a valid system with input `{In}` and output `{Out}`",
label = "invalid system"
)]
pub trait IntoSystem<In, Out, Marker>: Sized {
/// The type of [`System`] that this instance converts into.
type System: System<In = In, Out = Out>;
Expand Down
1 change: 1 addition & 0 deletions crates/bevy_ecs/src/system/system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ use super::IntoSystem;
/// Systems are executed in parallel, in opportunistic order; data access is managed automatically.
/// It's possible to specify explicit execution order between specific systems,
/// see [`IntoSystemConfigs`](crate::schedule::IntoSystemConfigs).
#[diagnostic::on_unimplemented(message = "`{Self}` is not a system", label = "invalid system")]
pub trait System: Send + Sync + 'static {
/// The system's input. See [`In`](crate::system::In) for
/// [`FunctionSystem`](crate::system::FunctionSystem)s.
Expand Down
5 changes: 5 additions & 0 deletions crates/bevy_ecs/src/system/system_param.rs
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,11 @@ impl_param_set!();
/// ```
///
/// [`Exclusive`]: https://doc.rust-lang.org/nightly/std/sync/struct.Exclusive.html
#[diagnostic::on_unimplemented(
message = "`{Self}` is not a `Resource`",
label = "invalid `Resource`",
note = "consider annotating `{Self}` with `#[derive(Resource)]`"
)]
pub trait Resource: Send + Sync + 'static {}

// SAFETY: Res only reads a single World resource
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ struct NoReflect(f32);

fn main() {
let mut foo: Box<dyn Struct> = Box::new(Foo::<NoReflect> { a: NoReflect(42.0) });
//~^ ERROR: not satisfied
//~^ ERROR: `NoReflect` does not provide type registration information

// foo doesn't implement Reflect because NoReflect doesn't implement Reflect
foo.get_field::<NoReflect>("a").unwrap();
//~^ ERROR: not satisfied
//~^ ERROR: `NoReflect` can not be reflected
}
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
error[E0277]: the trait bound `NoReflect: Reflect` is not satisfied
error[E0277]: `NoReflect` can not be reflected
--> tests/reflect_derive/generics_fail.rs:18:21
|
18 | foo.get_field::<NoReflect>("a").unwrap();
| --------- ^^^^^^^^^ the trait `Reflect` is not implemented for `NoReflect`
| |
| required by a bound introduced by this call
|
= note: Try using `#[derive(Reflect)]`
= help: the following other types implement trait `Reflect`:
bool
char
Expand All @@ -17,17 +18,19 @@ error[E0277]: the trait bound `NoReflect: Reflect` is not satisfied
i128
and 74 others
note: required by a bound in `bevy_reflect::GetField::get_field`
--> $BEVY_ROOT/crates/bevy_reflect/src/struct_trait.rs:242:21
--> $BEVY_ROOT/crates/bevy_reflect/src/struct_trait.rs:244:21
|
242 | fn get_field<T: Reflect>(&self, name: &str) -> Option<&T>;
244 | fn get_field<T: Reflect>(&self, name: &str) -> Option<&T>;
| ^^^^^^^ required by this bound in `GetField::get_field`

error[E0277]: the trait bound `NoReflect: GetTypeRegistration` is not satisfied
error[E0277]: `NoReflect` does not provide type registration information
--> tests/reflect_derive/generics_fail.rs:14:36
|
14 | let mut foo: Box<dyn Struct> = Box::new(Foo::<NoReflect> { a: NoReflect(42.0) });
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `GetTypeRegistration` is not implemented for `NoReflect`, which is required by `Foo<NoReflect>: bevy_reflect::Struct`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Invalid Type
|
= help: the trait `GetTypeRegistration` is not implemented for `NoReflect`, which is required by `Foo<NoReflect>: bevy_reflect::Struct`
= note: Try using `#[derive(Reflect)]`
= help: the following other types implement trait `GetTypeRegistration`:
bool
char
Expand Down
4 changes: 4 additions & 0 deletions crates/bevy_reflect/src/from_reflect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ use crate::{FromType, Reflect};
/// [derive macro]: bevy_reflect_derive::FromReflect
/// [`DynamicStruct`]: crate::DynamicStruct
/// [crate-level documentation]: crate
#[diagnostic::on_unimplemented(
message = "`{Self}` can not be created through reflection",
note = "consider annotating `{Self}` with `#[derive(FromReflect)]`"
)]
pub trait FromReflect: Reflect + Sized {
/// Constructs a concrete instance of `Self` from a reflected value.
fn from_reflect(reflect: &dyn Reflect) -> Option<Self>;
Expand Down
4 changes: 4 additions & 0 deletions crates/bevy_reflect/src/path/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,10 @@ impl<'a> ReflectPath<'a> for &'a str {
/// [`List`]: crate::List
/// [`Array`]: crate::Array
/// [`Enum`]: crate::Enum
#[diagnostic::on_unimplemented(
message = "`{Self}` does not provide a reflection path",
note = "consider annotating `{Self}` with `#[derive(Reflect)]`"
)]
pub trait GetPath: Reflect {
/// Returns a reference to the value specified by `path`.
///
Expand Down
4 changes: 4 additions & 0 deletions crates/bevy_reflect/src/reflect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,10 @@ impl std::fmt::Display for ReflectKind {
/// [`bevy_reflect`]: crate
/// [derive macro]: bevy_reflect_derive::Reflect
/// [crate-level documentation]: crate
#[diagnostic::on_unimplemented(
message = "`{Self}` can not be reflected",
note = "consider annotating `{Self}` with `#[derive(Reflect)]`"
)]
pub trait Reflect: DynamicTypePath + Any + Send + Sync {
/// Returns the [`TypeInfo`] of the type _represented_ by this value.
///
Expand Down
4 changes: 4 additions & 0 deletions crates/bevy_reflect/src/type_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,10 @@ use std::fmt::Debug;
/// ```
///
/// [utility]: crate::utility
#[diagnostic::on_unimplemented(
message = "`{Self}` can not provide type information through reflection",
note = "consider annotating `{Self}` with `#[derive(Reflect)]`"
)]
pub trait Typed: Reflect + TypePath {
/// Returns the compile-time [info] for the underlying type.
///
Expand Down
8 changes: 8 additions & 0 deletions crates/bevy_reflect/src/type_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,10 @@ use std::fmt;
/// [`crate_name`]: TypePath::crate_name
/// [`module_path`]: TypePath::module_path
/// [`type_ident`]: TypePath::type_ident
#[diagnostic::on_unimplemented(
message = "`{Self}` does not have a type path",
note = "consider annotating `{Self}` with `#[derive(Reflect)]` or `#[derive(TypePath)]`"
)]
pub trait TypePath: 'static {
/// Returns the fully qualified path of the underlying type.
///
Expand Down Expand Up @@ -129,6 +133,10 @@ pub trait TypePath: 'static {
/// Since this is a supertrait of [`Reflect`] its methods can be called on a `dyn Reflect`.
///
/// [`Reflect`]: crate::Reflect
#[diagnostic::on_unimplemented(
message = "`{Self}` can not be used as a dynamic type path",
note = "consider annotating `{Self}` with `#[derive(Reflect)]` or `#[derive(TypePath)]`"
)]
pub trait DynamicTypePath {
/// See [`TypePath::type_path`].
fn reflect_type_path(&self) -> &str;
Expand Down
4 changes: 4 additions & 0 deletions crates/bevy_reflect/src/type_registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,10 @@ impl Debug for TypeRegistryArc {
/// See the [crate-level documentation] for more information on type registration.
///
/// [crate-level documentation]: crate
#[diagnostic::on_unimplemented(
message = "`{Self}` does not provide type registration information",
note = "consider annotating `{Self}` with `#[derive(Reflect)]`"
)]
pub trait GetTypeRegistration: 'static {
/// Returns the default [`TypeRegistration`] for this type.
fn get_type_registration() -> TypeRegistration;
Expand Down
1 change: 1 addition & 0 deletions crates/bevy_state/src/state/freely_mutable_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use super::{take_next_state, transitions::*};
///
/// While ordinary states are freely mutable (and implement this trait as part of their derive macro),
/// computed states are not: instead, they can *only* change when the states that drive them do.
#[diagnostic::on_unimplemented(note = "consider annotating `{Self}` with `#[derive(States)]`")]
pub trait FreelyMutableState: States {
/// This function registers all the necessary systems to apply state changes and run transition schedules
fn register_state(schedule: &mut Schedule) {
Expand Down
5 changes: 5 additions & 0 deletions crates/bevy_state/src/state/states.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,11 @@ use std::hash::Hash;
/// app.add_systems(Update, handle_escape_pressed.run_if(in_state(GameState::MainMenu)));
/// app.add_systems(OnEnter(GameState::SettingsMenu), open_settings_menu);
/// ```
#[diagnostic::on_unimplemented(
message = "`{Self}` can not be used as a state",
label = "invalid state",
note = "consider annotating `{Self}` with `#[derive(States)]`"
)]
pub trait States: 'static + Send + Sync + Clone + PartialEq + Eq + Hash + Debug {
/// How many other states this state depends on.
/// Used to help order transitions and de-duplicate [`ComputedStates`](crate::state::ComputedStates), as well as prevent cyclical
Expand Down
5 changes: 5 additions & 0 deletions crates/bevy_state/src/state/sub_states.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,11 @@ pub use bevy_state_macros::SubStates;
///
/// impl FreelyMutableState for GamePhase {}
/// ```
#[diagnostic::on_unimplemented(
message = "`{Self}` can not be used as a sub-state",
label = "invalid sub-state",
note = "consider annotating `{Self}` with `#[derive(SubStates)]`"
)]
pub trait SubStates: States + FreelyMutableState {
/// The set of states from which the [`Self`] is derived.
///
Expand Down

0 comments on commit ec7b349

Please sign in to comment.