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

macros: improve error massages #1420

Merged
merged 2 commits into from
Aug 9, 2019
Merged

macros: improve error massages #1420

merged 2 commits into from
Aug 9, 2019

Conversation

taiki-e
Copy link
Member

@taiki-e taiki-e commented Aug 9, 2019

Motivation

dtolnay/syn#575

Solution

Using syn::Error::new_spanned.


return TokenStream::from(tokens);
let msg = "the main function cannot accept arguments";
return syn::Error::new_spanned(&input.decl.inputs, msg)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before:

error: the main function cannot accept arguments
  --> tokio\examples\echo.rs:34:1
   |
34 | async fn main(foo: Foo) -> Result<(), Box<dyn Error>> {
   | ^^^^^

After:

error: the main function cannot accept arguments
  --> tokio\examples\echo.rs:34:15
   |
34 | async fn main(foo: Foo) -> Result<(), Box<dyn Error>> {
   |               ^^^^^^^^

name => panic!("Unknown attribute {} is specified", name),
name => {
let msg = format!("Unknown attribute {} is specified", name);
return syn::Error::new_spanned(ident, msg)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before

error: custom attribute panicked
  --> tokio\examples\echo.rs:33:1
   |
33 | #[tokio::main(foo)]
   | ^^^^^^^^^^^^^^^^^^^
   |
   = help: message: Unknown attribute foo is specified

After:

error: Unknown attribute foo is specified
  --> tokio\examples\echo.rs:33:15
   |
33 | #[tokio::main(foo)]
   |               ^^^


return TokenStream::from(tokens);
let msg = "the async keyword is missing from the function declaration";
return syn::Error::new_spanned(input.decl.fn_token, msg)
Copy link
Member Author

@taiki-e taiki-e Aug 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no change (perhaps the clearest is to highlight fn.).

error: the async keyword is missing from the function declaration
  --> tokio\examples\echo.rs:34:1
   |
34 | fn main() -> Result<(), Box<dyn Error>> {
   | ^^

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that the original version highlights the entire function when procmacro2_semver_exempt is active.


return TokenStream::from(tokens);
let msg = "second test attribute is supplied";
return syn::Error::new_spanned(&attr, msg)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no change (this should be fine.).

error: second test attribute is supplied
  --> tokio\tests\buffered.rs:14:1
   |
14 | #[test]
   | ^^^^^^^

@taiki-e taiki-e requested a review from carllerche August 9, 2019 16:22
@taiki-e taiki-e changed the title macro: improve error massages macros: improve error massages Aug 9, 2019
@davidbarsky
Copy link
Member

This looks great! This could maybe be added in a followup PR, but how do you feel about using https://github.com/dtolnay/trybuild/ to test the error messages?

Copy link
Member

@carllerche carllerche left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks 👍 I added a line item in #1257 to track adding tests for macros.

@carllerche carllerche merged commit f7b41c9 into tokio-rs:master Aug 9, 2019
@taiki-e
Copy link
Member Author

taiki-e commented Aug 9, 2019

@davidbarsky

This could maybe be added in a followup PR, but how do you feel about using https://github.com/dtolnay/trybuild/ to test the error messages?

I think it's nice that the library that provides proc-macro does UI testing.
I've never used trybuild yet (I am using compiletest-rs), but it seems easier to use than compiletest-rs (compiletest-rs has many problems...).

@taiki-e taiki-e deleted the macro-spanned branch August 9, 2019 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants