From 245d03a78ad3988f23ef467ad0c49fe0c7349fa6 Mon Sep 17 00:00:00 2001 From: Gino Valente <49806985+MrGVSV@users.noreply.github.com> Date: Mon, 9 Sep 2024 09:26:17 -0700 Subject: [PATCH] bevy_reflect: Update `on_unimplemented` attributes (#15110) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Objective Some of the new compile error messages are a little unclear (at least to me). For example: ``` error[E0277]: `tests::foo::Bar` can not be created through reflection --> crates/bevy_reflect/src/lib.rs:679:18 | 679 | #[derive(Reflect)] | ^^^^^^^ the trait `from_reflect::FromReflect` is not implemented for `tests::foo::Bar` | = note: consider annotating `tests::foo::Bar` with `#[derive(Reflect)]` or `#[derive(FromReflect)]` ``` While the annotation makes it clear that `FromReflect` is missing, it's not very clear from the main error message. My IDE lists errors with only their message immediately present:

Image of said IDE listing errors with only their message immediately present. These errors are as follows: \

This makes it hard to tell at a glance why my code isn't compiling. ## Solution Updated all `on_unimplemented` attributes in `bevy_reflect` to mention the relevant trait—either the actual trait or the one users actually need to implement—as well as a small snippet of what not implementing them means. For example, failing to implement `TypePath` now mentions missing a `TypePath` implementation. And failing to implement `DynamicTypePath` now also mentions missing a `TypePath` implementation, since that's the actual trait users need to implement (i.e. they shouldn't implement `DynamicTypePath` directly). Lastly, I also added some missing `on_unimplemented` attributes for `MaybeTyped` and `RegisterForReflection` (which you can see in the image above). Here's how this looks in my IDE now:

Similar image as before showing the errors listed by the IDE. This time the errors read as follows: \

## Testing You can test by adding the following code and verifying the compile errors are correct: ```rust #[derive(Reflect)] struct Foo(Bar); struct Bar; ``` --- .../compile_fail/tests/reflect_derive/generics_fail.rs | 4 ++-- .../compile_fail/tests/reflect_remote/nested_fail.rs | 2 +- crates/bevy_reflect/src/from_reflect.rs | 4 ++-- crates/bevy_reflect/src/lib.rs | 4 ++++ crates/bevy_reflect/src/path/mod.rs | 2 +- crates/bevy_reflect/src/type_info.rs | 6 +++++- crates/bevy_reflect/src/type_path.rs | 4 ++-- crates/bevy_reflect/src/type_registry.rs | 2 +- 8 files changed, 18 insertions(+), 10 deletions(-) diff --git a/crates/bevy_reflect/compile_fail/tests/reflect_derive/generics_fail.rs b/crates/bevy_reflect/compile_fail/tests/reflect_derive/generics_fail.rs index 92b16288c407c..6f90097828b64 100644 --- a/crates/bevy_reflect/compile_fail/tests/reflect_derive/generics_fail.rs +++ b/crates/bevy_reflect/compile_fail/tests/reflect_derive/generics_fail.rs @@ -12,8 +12,8 @@ struct NoReflect(f32); fn main() { let mut foo: Box = Box::new(Foo:: { a: NoReflect(42.0) }); - //~^ ERROR: `NoReflect` does not provide type registration information - //~| ERROR: `NoReflect` can not provide type information through reflection + //~^ ERROR: `NoReflect` does not implement `GetTypeRegistration` so cannot provide type registration information + //~| ERROR: `NoReflect` does not implement `Typed` so cannot provide static type information // foo doesn't implement Reflect because NoReflect doesn't implement Reflect foo.get_field::("a").unwrap(); diff --git a/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_fail.rs b/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_fail.rs index c9f85c000187d..0f8ade8e234ca 100644 --- a/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_fail.rs +++ b/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_fail.rs @@ -25,7 +25,7 @@ mod incorrect_inner_type { //~^ ERROR: `TheirInner` does not implement `PartialReflect` so cannot be introspected //~| ERROR: `TheirInner` does not implement `PartialReflect` so cannot be introspected //~| ERROR: `TheirInner` does not implement `PartialReflect` so cannot be introspected - //~| ERROR: `TheirInner` can not be used as a dynamic type path + //~| ERROR: `TheirInner` does not implement `TypePath` so cannot provide dynamic type path information //~| ERROR: `?` operator has incompatible types struct MyOuter { // Reason: Should not use `MyInner` directly diff --git a/crates/bevy_reflect/src/from_reflect.rs b/crates/bevy_reflect/src/from_reflect.rs index b1ca70f52894d..7a8c3b289d010 100644 --- a/crates/bevy_reflect/src/from_reflect.rs +++ b/crates/bevy_reflect/src/from_reflect.rs @@ -22,8 +22,8 @@ use crate::{FromType, PartialReflect, Reflect}; /// [`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)]`" + message = "`{Self}` does not implement `FromReflect` so cannot be created through reflection", + note = "consider annotating `{Self}` with `#[derive(Reflect)]`" )] pub trait FromReflect: Reflect + Sized { /// Constructs a concrete instance of `Self` from a reflected value. diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs index 64216bfc09467..b226af4432402 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -620,6 +620,10 @@ pub mod __macro_exports { /// /// This trait has a blanket implementation for all types that implement `GetTypeRegistration` /// and manual implementations for all dynamic types (which simply do nothing). + #[diagnostic::on_unimplemented( + message = "`{Self}` does not implement `GetTypeRegistration` so cannot be registered for reflection", + note = "consider annotating `{Self}` with `#[derive(Reflect)]`" + )] pub trait RegisterForReflection { #[allow(unused_variables)] fn __register(registry: &mut TypeRegistry) {} diff --git a/crates/bevy_reflect/src/path/mod.rs b/crates/bevy_reflect/src/path/mod.rs index c3d8ccfa2de21..23918bed15dda 100644 --- a/crates/bevy_reflect/src/path/mod.rs +++ b/crates/bevy_reflect/src/path/mod.rs @@ -237,7 +237,7 @@ impl<'a> ReflectPath<'a> for &'a str { /// [`Array`]: crate::Array /// [`Enum`]: crate::Enum #[diagnostic::on_unimplemented( - message = "`{Self}` does not provide a reflection path", + message = "`{Self}` does not implement `GetPath` so cannot be accessed by reflection path", note = "consider annotating `{Self}` with `#[derive(Reflect)]`" )] pub trait GetPath: PartialReflect { diff --git a/crates/bevy_reflect/src/type_info.rs b/crates/bevy_reflect/src/type_info.rs index d3450239e6d3f..ca9963f3803aa 100644 --- a/crates/bevy_reflect/src/type_info.rs +++ b/crates/bevy_reflect/src/type_info.rs @@ -83,7 +83,7 @@ use thiserror::Error; /// /// [utility]: crate::utility #[diagnostic::on_unimplemented( - message = "`{Self}` can not provide type information through reflection", + message = "`{Self}` does not implement `Typed` so cannot provide static type information", note = "consider annotating `{Self}` with `#[derive(Reflect)]`" )] pub trait Typed: Reflect + TypePath { @@ -103,6 +103,10 @@ pub trait Typed: Reflect + TypePath { /// This trait has a blanket implementation for all types that implement `Typed` /// and manual implementations for all dynamic types (which simply return `None`). #[doc(hidden)] +#[diagnostic::on_unimplemented( + message = "`{Self}` does not implement `Typed` so cannot provide static type information", + note = "consider annotating `{Self}` with `#[derive(Reflect)]`" +)] pub trait MaybeTyped: PartialReflect { /// Returns the compile-time [info] for the underlying type, if it exists. /// diff --git a/crates/bevy_reflect/src/type_path.rs b/crates/bevy_reflect/src/type_path.rs index dd2e18cc12486..bc685eb9f95cf 100644 --- a/crates/bevy_reflect/src/type_path.rs +++ b/crates/bevy_reflect/src/type_path.rs @@ -80,7 +80,7 @@ use std::fmt; /// [`module_path`]: TypePath::module_path /// [`type_ident`]: TypePath::type_ident #[diagnostic::on_unimplemented( - message = "`{Self}` does not have a type path", + message = "`{Self}` does not implement `TypePath` so cannot provide static type path information", note = "consider annotating `{Self}` with `#[derive(Reflect)]` or `#[derive(TypePath)]`" )] pub trait TypePath: 'static { @@ -134,7 +134,7 @@ pub trait TypePath: 'static { /// /// [`Reflect`]: crate::Reflect #[diagnostic::on_unimplemented( - message = "`{Self}` can not be used as a dynamic type path", + message = "`{Self}` does not implement `TypePath` so cannot provide dynamic type path information", note = "consider annotating `{Self}` with `#[derive(Reflect)]` or `#[derive(TypePath)]`" )] pub trait DynamicTypePath { diff --git a/crates/bevy_reflect/src/type_registry.rs b/crates/bevy_reflect/src/type_registry.rs index 1263b0bbed523..1fe7208956ec5 100644 --- a/crates/bevy_reflect/src/type_registry.rs +++ b/crates/bevy_reflect/src/type_registry.rs @@ -57,7 +57,7 @@ impl Debug for TypeRegistryArc { /// /// [crate-level documentation]: crate #[diagnostic::on_unimplemented( - message = "`{Self}` does not provide type registration information", + message = "`{Self}` does not implement `GetTypeRegistration` so cannot provide type registration information", note = "consider annotating `{Self}` with `#[derive(Reflect)]`" )] pub trait GetTypeRegistration: 'static {