Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Impove derive macro error message #62

Closed
constfold opened this issue Jul 19, 2020 · 4 comments
Closed

Impove derive macro error message #62

constfold opened this issue Jul 19, 2020 · 4 comments
Labels
enhancement New feature or request

Comments

@constfold
Copy link
Contributor

Currently when an error happend in deive macro it will just panic, we should use syn::Error::to_compile_error() instead.
For example, an invalid attribute give this message:

error: proc-macro derive panicked
 --> src\main.rs:3:10
  |
3 | #[derive(DekuRead)]
  |          ^^^^^^^^
  |
  = help: message: called `Result::unwrap()` on an `Err` value: Error { kind: UnknownField(ErrorUnknownField { name: "id", did_you_mean: None }), locations: ["b"], span: Some(#0 bytes(79..86)) }

A better message could be:

error: unknown deku field attribute `id`
 --> src\main.rs:3:10
|
7| #[deku(id = "")]
|         ^^
@sharksforarms sharksforarms added the enhancement New feature or request label Jul 20, 2020
@sharksforarms
Copy link
Owner

Made a commit to master: d7d5f7d

Let me know if that's what you were thinking! Changes the unwrap panic to compile error

@constfold
Copy link
Contributor Author

@sharksforarms looks good! and FYI there still are a few panics in Deku*Receiver like

panic!("`id_*` attributes only supported on enum")

panic!("`id_type` must be specified with `id_bits` or `id_bytes`");

Maybe hard to convert?

@sharksforarms
Copy link
Owner

@sharksforarms looks good! and FYI there still are a few panics in Deku*Receiver like

panic!("`id_*` attributes only supported on enum")

panic!("`id_type` must be specified with `id_bits` or `id_bytes`");

Maybe hard to convert?

Yes. Not exactly sure how to convert these. I'm using darling's map = in order to validate... maybe there's a better way.

@sharksforarms
Copy link
Owner

Addressed in #61

wcampbell0x2a pushed a commit to wcampbell0x2a/deku that referenced this issue Jun 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants