From 066e6d2fcf71600061bbf4f295875cdded0c8063 Mon Sep 17 00:00:00 2001 From: Tristan Guichaoua <33934311+tguichaoua@users.noreply.github.com> Date: Mon, 2 Oct 2023 14:42:01 +0200 Subject: [PATCH] Improve TypeUuid's derive macro error messages (#9315) # Objective - Better error message - More idiomatic code ## Solution Refactorize `TypeUuid` macros to use `syn::Result` instead of panic. ## Before/After error messages ### Missing `#[uuid]` attribtue #### Before ``` error: proc-macro derive panicked --> src\main.rs:1:10 | 1 | #[derive(TypeUuid)] | ^^^^^^^^ | = help: message: No `#[uuid = "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx"` attribute found. ``` #### After ``` error: No `#[uuid = "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx"]` attribute found. --> src\main.rs:3:10 | 3 | #[derive(TypeUuid)] | ^^^^^^^^ | = note: this error originates in the derive macro `TypeUuid` (in Nightly builds, run with -Z macro-backtrace for more info) ``` ### Malformed attribute #### Before ``` error: proc-macro derive panicked --> src\main.rs:3:10 | 3 | #[derive(TypeUuid)] | ^^^^^^^^ | = help: message: `uuid` attribute must take the form `#[uuid = "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx"`. ``` #### After ``` error: `uuid` attribute must take the form `#[uuid = "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx"]`. --> src\main.rs:4:1 | 4 | #[uuid = 42] | ^^^^^^^^^^^^ ``` ### UUID parse fail #### Before ``` error: proc-macro derive panicked --> src\main.rs:3:10 | 3 | #[derive(TypeUuid)] | ^^^^^^^^ | = help: message: Value specified to `#[uuid]` attribute is not a valid UUID.: Error(SimpleLength { len: 3 }) ``` #### After ``` error: Invalid UUID: invalid length: expected length 32 for simple format, found 3 --> src\main.rs:4:10 | 4 | #[uuid = "000"] | ^^^^^ ``` ### With [Error Lens](https://marketplace.visualstudio.com/items?itemName=usernamehw.errorlens) #### Before ![image](https://github.com/bevyengine/bevy/assets/33934311/415247fa-ff5c-4513-8012-7a9ff77445fb) #### After ![image](https://github.com/bevyengine/bevy/assets/33934311/d124eeaa-9213-49e0-8860-539ad0218a40) --- ## Changelog - `#[derive(TypeUuid)]` provide better error messages. --- .../bevy_reflect_derive/src/lib.rs | 9 +-- .../bevy_reflect_derive/src/type_uuid.rs | 58 +++++++++---------- .../tests/type_uuid_derive.rs | 5 ++ .../type_uuid_derive/derive_type_uuid.rs | 17 ++++++ .../type_uuid_derive/derive_type_uuid.stderr | 19 ++++++ 5 files changed, 75 insertions(+), 33 deletions(-) create mode 100644 crates/bevy_reflect_compile_fail_tests/tests/type_uuid_derive.rs create mode 100644 crates/bevy_reflect_compile_fail_tests/tests/type_uuid_derive/derive_type_uuid.rs create mode 100644 crates/bevy_reflect_compile_fail_tests/tests/type_uuid_derive/derive_type_uuid.stderr diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/lib.rs b/crates/bevy_reflect/bevy_reflect_derive/src/lib.rs index f2251d3f8d0741..5474f143cda999 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/lib.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/lib.rs @@ -30,7 +30,6 @@ mod type_uuid; mod utility; use crate::derive_data::{ReflectDerive, ReflectMeta, ReflectStruct}; -use crate::type_uuid::gen_impl_type_uuid; use container_attributes::ReflectTraits; use derive_data::ReflectTypePath; use proc_macro::TokenStream; @@ -39,7 +38,6 @@ use reflect_value::ReflectValueDef; use syn::spanned::Spanned; use syn::{parse_macro_input, DeriveInput}; use type_path::NamedTypePathDef; -use type_uuid::TypeUuidDef; use utility::WhereClauseOptions; pub(crate) static REFLECT_ATTRIBUTE_NAME: &str = "reflect"; @@ -288,7 +286,10 @@ pub fn derive_type_path(input: TokenStream) -> TokenStream { // From https://github.com/randomPoison/type-uuid #[proc_macro_derive(TypeUuid, attributes(uuid))] pub fn derive_type_uuid(input: TokenStream) -> TokenStream { + let input = parse_macro_input!(input as DeriveInput); type_uuid::type_uuid_derive(input) + .unwrap_or_else(syn::Error::into_compile_error) + .into() } /// A macro that automatically generates type data for traits, which their implementors can then register. @@ -588,6 +589,6 @@ pub fn impl_type_path(input: TokenStream) -> TokenStream { /// Derives `TypeUuid` for the given type. This is used internally to implement `TypeUuid` on foreign types, such as those in the std. This macro should be used in the format of `<[Generic Params]> [Type (Path)], [Uuid (String Literal)]`. #[proc_macro] pub fn impl_type_uuid(input: TokenStream) -> TokenStream { - let def = parse_macro_input!(input as TypeUuidDef); - gen_impl_type_uuid(def) + let def = parse_macro_input!(input as type_uuid::TypeUuidDef); + type_uuid::gen_impl_type_uuid(def).into() } diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/type_uuid.rs b/crates/bevy_reflect/bevy_reflect_derive/src/type_uuid.rs index 6a32445439d53e..56372edd0ee2a8 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/type_uuid.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/type_uuid.rs @@ -1,53 +1,54 @@ -extern crate proc_macro; - use bevy_macro_utils::BevyManifest; +use proc_macro2::{Span, TokenStream}; use quote::quote; use syn::parse::{Parse, ParseStream}; use syn::token::Comma; -use syn::*; +use syn::{DeriveInput, Expr, ExprLit, Generics, Ident, Lit, LitInt, LitStr, Meta}; use uuid::Uuid; -/// Parses input from a derive of `TypeUuid`. -pub(crate) fn type_uuid_derive(input: proc_macro::TokenStream) -> proc_macro::TokenStream { - // Construct a representation of Rust code as a syntax tree - // that we can manipulate - let ast: DeriveInput = syn::parse(input).unwrap(); - // Build the trait implementation - let type_ident = ast.ident; - +pub(crate) fn type_uuid_derive(input: DeriveInput) -> syn::Result { let mut uuid = None; - for attribute in ast.attrs.iter().filter(|attr| attr.path().is_ident("uuid")) { + for attribute in input + .attrs + .iter() + .filter(|attr| attr.path().is_ident("uuid")) + { let Meta::NameValue(ref name_value) = attribute.meta else { continue; }; let uuid_str = match &name_value.value { Expr::Lit(ExprLit{lit: Lit::Str(lit_str), ..}) => lit_str, - _ => panic!("`uuid` attribute must take the form `#[uuid = \"xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx\"`."), + _ => return Err(syn::Error::new_spanned(attribute, "`uuid` attribute must take the form `#[uuid = \"xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx\"]`.")), }; - uuid = Some( - Uuid::parse_str(&uuid_str.value()) - .expect("Value specified to `#[uuid]` attribute is not a valid UUID."), - ); + uuid = + Some(Uuid::parse_str(&uuid_str.value()).map_err(|err| { + syn::Error::new_spanned(uuid_str, format!("Invalid UUID: {err}")) + })?); } - let uuid = - uuid.expect("No `#[uuid = \"xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx\"` attribute found."); - gen_impl_type_uuid(TypeUuidDef { - type_ident, - generics: ast.generics, + let uuid = uuid.ok_or_else(|| { + syn::Error::new( + Span::call_site(), + "No `#[uuid = \"xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx\"]` attribute found.", + ) + })?; + + Ok(gen_impl_type_uuid(TypeUuidDef { + type_ident: input.ident, + generics: input.generics, uuid, - }) + })) } /// Generates an implementation of `TypeUuid`. If there any generics, the `TYPE_UUID` will be a composite of the generic types' `TYPE_UUID`. -pub(crate) fn gen_impl_type_uuid(def: TypeUuidDef) -> proc_macro::TokenStream { +pub(crate) fn gen_impl_type_uuid(def: TypeUuidDef) -> TokenStream { let uuid = def.uuid; let mut generics = def.generics; let ty = def.type_ident; - let bevy_reflect_path: Path = BevyManifest::default().get_path("bevy_reflect"); + let bevy_reflect_path = BevyManifest::default().get_path("bevy_reflect"); generics.type_params_mut().for_each(|param| { param @@ -74,12 +75,11 @@ pub(crate) fn gen_impl_type_uuid(def: TypeUuidDef) -> proc_macro::TokenStream { } }); - let gen = quote! { + quote! { impl #impl_generics #bevy_reflect_path::TypeUuid for #ty #type_generics #where_clause { const TYPE_UUID: #bevy_reflect_path::Uuid = #type_uuid; } - }; - gen.into() + } } /// A struct containing the data required to generate an implementation of `TypeUuid`. This can be generated by either [`impl_type_uuid!`][crate::impl_type_uuid!] or [`type_uuid_derive`]. @@ -90,7 +90,7 @@ pub(crate) struct TypeUuidDef { } impl Parse for TypeUuidDef { - fn parse(input: ParseStream) -> Result { + fn parse(input: ParseStream) -> syn::Result { let type_ident = input.parse::()?; let generics = input.parse::()?; input.parse::()?; diff --git a/crates/bevy_reflect_compile_fail_tests/tests/type_uuid_derive.rs b/crates/bevy_reflect_compile_fail_tests/tests/type_uuid_derive.rs new file mode 100644 index 00000000000000..fe352d8d00fcf9 --- /dev/null +++ b/crates/bevy_reflect_compile_fail_tests/tests/type_uuid_derive.rs @@ -0,0 +1,5 @@ +#[test] +fn test() { + let t = trybuild::TestCases::new(); + t.compile_fail("tests/type_uuid_derive/*.rs"); +} diff --git a/crates/bevy_reflect_compile_fail_tests/tests/type_uuid_derive/derive_type_uuid.rs b/crates/bevy_reflect_compile_fail_tests/tests/type_uuid_derive/derive_type_uuid.rs new file mode 100644 index 00000000000000..da4ce924b757ab --- /dev/null +++ b/crates/bevy_reflect_compile_fail_tests/tests/type_uuid_derive/derive_type_uuid.rs @@ -0,0 +1,17 @@ +use bevy_reflect::TypeUuid; + +fn main() {} + +// Missing #[uuid] attribute +#[derive(TypeUuid)] +struct A; + +// Malformed attribute +#[derive(TypeUuid)] +#[uuid = 42] +struct B; + +// UUID parse fail +#[derive(TypeUuid)] +#[uuid = "000"] +struct C; diff --git a/crates/bevy_reflect_compile_fail_tests/tests/type_uuid_derive/derive_type_uuid.stderr b/crates/bevy_reflect_compile_fail_tests/tests/type_uuid_derive/derive_type_uuid.stderr new file mode 100644 index 00000000000000..95f4834eab081f --- /dev/null +++ b/crates/bevy_reflect_compile_fail_tests/tests/type_uuid_derive/derive_type_uuid.stderr @@ -0,0 +1,19 @@ +error: No `#[uuid = "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx"]` attribute found. + --> tests/type_uuid_derive/derive_type_uuid.rs:6:10 + | +6 | #[derive(TypeUuid)] + | ^^^^^^^^ + | + = note: this error originates in the derive macro `TypeUuid` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: `uuid` attribute must take the form `#[uuid = "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx"]`. + --> tests/type_uuid_derive/derive_type_uuid.rs:11:1 + | +11 | #[uuid = 42] + | ^^^^^^^^^^^^ + +error: Invalid UUID: invalid length: expected length 32 for simple format, found 3 + --> tests/type_uuid_derive/derive_type_uuid.rs:16:10 + | +16 | #[uuid = "000"] + | ^^^^^