Skip to content

Commit

Permalink
Improve TypeUuid's derive macro error messages (bevyengine#9315)
Browse files Browse the repository at this point in the history
# 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.
  • Loading branch information
tguichaoua authored and Thomas Wilgenbus committed Oct 13, 2023
1 parent 41b9fdc commit 74a6efe
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 33 deletions.
9 changes: 5 additions & 4 deletions crates/bevy_reflect/bevy_reflect_derive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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";
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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()
}
58 changes: 29 additions & 29 deletions crates/bevy_reflect/bevy_reflect_derive/src/type_uuid.rs
Original file line number Diff line number Diff line change
@@ -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<TokenStream> {
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
Expand All @@ -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`].
Expand All @@ -90,7 +90,7 @@ pub(crate) struct TypeUuidDef {
}

impl Parse for TypeUuidDef {
fn parse(input: ParseStream) -> Result<Self> {
fn parse(input: ParseStream) -> syn::Result<Self> {
let type_ident = input.parse::<Ident>()?;
let generics = input.parse::<Generics>()?;
input.parse::<Comma>()?;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#[test]
fn test() {
let t = trybuild::TestCases::new();
t.compile_fail("tests/type_uuid_derive/*.rs");
}
Original file line number Diff line number Diff line change
@@ -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;
Original file line number Diff line number Diff line change
@@ -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"]
| ^^^^^

0 comments on commit 74a6efe

Please sign in to comment.