From cd6afc977e3ec67f80a749562ef1f11f70c9897b Mon Sep 17 00:00:00 2001 From: David Pedersen Date: Mon, 5 Apr 2021 21:45:40 +0200 Subject: [PATCH 1/5] macros: mention `start_paused` requires `test-util` --- tokio-macros/src/lib.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tokio-macros/src/lib.rs b/tokio-macros/src/lib.rs index 46879f822e1..18de77281c2 100644 --- a/tokio-macros/src/lib.rs +++ b/tokio-macros/src/lib.rs @@ -167,6 +167,8 @@ use proc_macro::TokenStream; /// } /// ``` /// +/// Note that `start_paused` requires the `test-util` feature to be enabled. +/// /// ### NOTE: /// /// If you rename the Tokio crate in your dependencies this macro will not work. @@ -257,6 +259,8 @@ pub fn main_rt(args: TokenStream, item: TokenStream) -> TokenStream { /// } /// ``` /// +/// Note that `start_paused` requires the `test-util` feature to be enabled. +/// /// ### NOTE: /// /// If you rename the Tokio crate in your dependencies this macro will not work. From 983cc43a928a816b77f1a67e31d682e096877a27 Mon Sep 17 00:00:00 2001 From: David Pedersen Date: Mon, 5 Apr 2021 21:47:05 +0200 Subject: [PATCH 2/5] macros: various error message improvements --- .../tests/fail/macros_invalid_input.rs | 15 +++++ .../tests/fail/macros_invalid_input.stderr | 38 +++++++++++-- tokio-macros/src/entry.rs | 56 +++++++++++-------- 3 files changed, 82 insertions(+), 27 deletions(-) diff --git a/tests-build/tests/fail/macros_invalid_input.rs b/tests-build/tests/fail/macros_invalid_input.rs index 5b1a8adc075..a47b8ac3d10 100644 --- a/tests-build/tests/fail/macros_invalid_input.rs +++ b/tests-build/tests/fail/macros_invalid_input.rs @@ -18,6 +18,21 @@ async fn test_fn_has_args(_x: u8) {} #[tokio::test(foo)] async fn test_attr_has_args() {} +#[tokio::test(foo = 123)] +async fn test_unexpected_attr() {} + +#[tokio::test(flavor = 123)] +async fn test_flavor_not_string() {} + +#[tokio::test(flavor = "multi_thread", start_paused = false)] +async fn test_multi_thread_with_start_paused() {} + +#[tokio::test(flavor = "multi_thread", worker_threads = "foo")] +async fn test_worker_threads_not_int() {} + +#[tokio::test(flavor = "current_thread", worker_threads = 4)] +async fn test_worker_threads_and_current_thread() {} + #[tokio::test] #[test] async fn test_has_second_test_attr() {} diff --git a/tests-build/tests/fail/macros_invalid_input.stderr b/tests-build/tests/fail/macros_invalid_input.stderr index bba2009352d..705404a3f66 100644 --- a/tests-build/tests/fail/macros_invalid_input.stderr +++ b/tests-build/tests/fail/macros_invalid_input.stderr @@ -1,4 +1,4 @@ -error: the async keyword is missing from the function declaration +error: the `async` keyword is missing from the function declaration --> $DIR/macros_invalid_input.rs:4:1 | 4 | fn main_is_not_async() {} @@ -16,7 +16,7 @@ error: Must have specified ident 9 | #[tokio::main(threadpool::bar)] | ^^^^^^^^^^^^^^^ -error: the async keyword is missing from the function declaration +error: the `async` keyword is missing from the function declaration --> $DIR/macros_invalid_input.rs:13:1 | 13 | fn test_is_not_async() {} @@ -34,8 +34,38 @@ error: Unknown attribute foo is specified; expected one of: `flavor`, `worker_th 18 | #[tokio::test(foo)] | ^^^ +error: Unknown attribute foo is specified; expected one of: `flavor`, `worker_threads`, `start_paused` + --> $DIR/macros_invalid_input.rs:21:15 + | +21 | #[tokio::test(foo = 123)] + | ^^^^^^^^^ + +error: Failed to parse value of `flavor` as string. + --> $DIR/macros_invalid_input.rs:24:24 + | +24 | #[tokio::test(flavor = 123)] + | ^^^ + +error: The `start_paused` option requires the `current_thread` runtime flavor. Use `#[tokio::test(flavor = "current_thread")]` + --> $DIR/macros_invalid_input.rs:27:55 + | +27 | #[tokio::test(flavor = "multi_thread", start_paused = false)] + | ^^^^^ + +error: Failed to parse value of `worker_threads` as integer. + --> $DIR/macros_invalid_input.rs:30:57 + | +30 | #[tokio::test(flavor = "multi_thread", worker_threads = "foo")] + | ^^^^^ + +error: The `worker_threads` option requires the `multi_thread` runtime flavor. Use `#[tokio::test(flavor = "multi_thread")]` + --> $DIR/macros_invalid_input.rs:33:59 + | +33 | #[tokio::test(flavor = "current_thread", worker_threads = 4)] + | ^ + error: second test attribute is supplied - --> $DIR/macros_invalid_input.rs:22:1 + --> $DIR/macros_invalid_input.rs:37:1 | -22 | #[test] +37 | #[test] | ^^^^^^^ diff --git a/tokio-macros/src/entry.rs b/tokio-macros/src/entry.rs index f82a329af16..9d56e6c530b 100644 --- a/tokio-macros/src/entry.rs +++ b/tokio-macros/src/entry.rs @@ -1,7 +1,6 @@ use proc_macro::TokenStream; use proc_macro2::Span; use quote::quote; -use syn::spanned::Spanned; #[derive(Clone, Copy, PartialEq)] enum RuntimeFlavor { @@ -34,6 +33,7 @@ struct Configuration { flavor: Option, worker_threads: Option<(usize, Span)>, start_paused: Option<(bool, Span)>, + is_test: bool, } impl Configuration { @@ -47,6 +47,7 @@ impl Configuration { flavor: None, worker_threads: None, start_paused: None, + is_test, } } @@ -92,16 +93,25 @@ impl Configuration { Ok(()) } + fn macro_name(&self) -> &'static str { + if self.is_test { + "tokio::test" + } else { + "tokio::main" + } + } + fn build(&self) -> Result { let flavor = self.flavor.unwrap_or(self.default_flavor); use RuntimeFlavor::*; let worker_threads = match (flavor, self.worker_threads) { (CurrentThread, Some((_, worker_threads_span))) => { - return Err(syn::Error::new( - worker_threads_span, - "The `worker_threads` option requires the `multi_thread` runtime flavor.", - )) + let msg = format!( + "The `worker_threads` option requires the `multi_thread` runtime flavor. Use `#[{}(flavor = \"multi_thread\")]`", + self.macro_name(), + ); + return Err(syn::Error::new(worker_threads_span, msg)); } (CurrentThread, None) => None, (Threaded, worker_threads) if self.rt_multi_thread_available => { @@ -119,10 +129,11 @@ impl Configuration { let start_paused = match (flavor, self.start_paused) { (Threaded, Some((_, start_paused_span))) => { - return Err(syn::Error::new( - start_paused_span, - "The `start_paused` option requires the `current_thread` runtime flavor.", - )); + let msg = format!( + "The `start_paused` option requires the `current_thread` runtime flavor. Use `#[{}(flavor = \"current_thread\")]`", + self.macro_name(), + ); + return Err(syn::Error::new(start_paused_span, msg)); } (CurrentThread, Some((start_paused, _))) => Some(start_paused), (_, None) => None, @@ -142,12 +153,12 @@ fn parse_int(int: syn::Lit, span: Span, field: &str) -> Result Ok(value), Err(e) => Err(syn::Error::new( span, - format!("Failed to parse {} as integer: {}", field, e), + format!("Failed to parse value of `{}` as integer: {}", field, e), )), }, _ => Err(syn::Error::new( span, - format!("Failed to parse {} as integer.", field), + format!("Failed to parse value of `{}` as integer.", field), )), } } @@ -158,7 +169,7 @@ fn parse_string(int: syn::Lit, span: Span, field: &str) -> Result Ok(s.to_string()), _ => Err(syn::Error::new( span, - format!("Failed to parse {} as string.", field), + format!("Failed to parse value of `{}` as string.", field), )), } } @@ -168,7 +179,7 @@ fn parse_bool(bool: syn::Lit, span: Span, field: &str) -> Result Ok(b.value), _ => Err(syn::Error::new( span, - format!("Failed to parse {} as bool.", field), + format!("Failed to parse value of `{}` as bool.", field), )), } } @@ -185,18 +196,14 @@ fn parse_knobs( let vis = input.vis; if sig.asyncness.is_none() { - let msg = "the async keyword is missing from the function declaration"; + let msg = "the `async` keyword is missing from the function declaration"; return Err(syn::Error::new_spanned(sig.fn_token, msg)); } sig.asyncness = None; - let macro_name = if is_test { - "tokio::test" - } else { - "tokio::main" - }; let mut config = Configuration::new(is_test, rt_multi_thread); + let macro_name = config.macro_name(); for arg in args { match arg { @@ -208,20 +215,23 @@ fn parse_knobs( } match ident.unwrap().to_string().to_lowercase().as_str() { "worker_threads" => { - config.set_worker_threads(namevalue.lit.clone(), namevalue.span())?; + config.set_worker_threads(namevalue.lit.clone(), namevalue.lit.span())?; } "flavor" => { - config.set_flavor(namevalue.lit.clone(), namevalue.span())?; + config.set_flavor(namevalue.lit.clone(), namevalue.lit.span())?; } "start_paused" => { - config.set_start_paused(namevalue.lit.clone(), namevalue.span())?; + config.set_start_paused(namevalue.lit.clone(), namevalue.lit.span())?; } "core_threads" => { let msg = "Attribute `core_threads` is renamed to `worker_threads`"; return Err(syn::Error::new_spanned(namevalue, msg)); } name => { - let msg = format!("Unknown attribute {} is specified; expected one of: `flavor`, `worker_threads`", name); + let msg = format!( + "Unknown attribute {} is specified; expected one of: `flavor`, `worker_threads`, `start_paused`", + name, + ); return Err(syn::Error::new_spanned(namevalue, msg)); } } From 6b68af2d57481e070dcf254ffac5d1805c1ad06c Mon Sep 17 00:00:00 2001 From: David Pedersen Date: Mon, 5 Apr 2021 21:58:12 +0200 Subject: [PATCH 3/5] Add test for invalid runtime flavor --- .../tests/fail/macros_invalid_input.rs | 3 +++ .../tests/fail/macros_invalid_input.stderr | 22 ++++++++++++------- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/tests-build/tests/fail/macros_invalid_input.rs b/tests-build/tests/fail/macros_invalid_input.rs index a47b8ac3d10..1700c8e742f 100644 --- a/tests-build/tests/fail/macros_invalid_input.rs +++ b/tests-build/tests/fail/macros_invalid_input.rs @@ -24,6 +24,9 @@ async fn test_unexpected_attr() {} #[tokio::test(flavor = 123)] async fn test_flavor_not_string() {} +#[tokio::test(flavor = "foo")] +async fn test_unknown_flavor() {} + #[tokio::test(flavor = "multi_thread", start_paused = false)] async fn test_multi_thread_with_start_paused() {} diff --git a/tests-build/tests/fail/macros_invalid_input.stderr b/tests-build/tests/fail/macros_invalid_input.stderr index 705404a3f66..35d98486905 100644 --- a/tests-build/tests/fail/macros_invalid_input.stderr +++ b/tests-build/tests/fail/macros_invalid_input.stderr @@ -46,26 +46,32 @@ error: Failed to parse value of `flavor` as string. 24 | #[tokio::test(flavor = 123)] | ^^^ +error: No such runtime flavor `foo`. The runtime flavors are `current_thread` and `multi_thread`. + --> $DIR/macros_invalid_input.rs:27:24 + | +27 | #[tokio::test(flavor = "foo")] + | ^^^^^ + error: The `start_paused` option requires the `current_thread` runtime flavor. Use `#[tokio::test(flavor = "current_thread")]` - --> $DIR/macros_invalid_input.rs:27:55 + --> $DIR/macros_invalid_input.rs:30:55 | -27 | #[tokio::test(flavor = "multi_thread", start_paused = false)] +30 | #[tokio::test(flavor = "multi_thread", start_paused = false)] | ^^^^^ error: Failed to parse value of `worker_threads` as integer. - --> $DIR/macros_invalid_input.rs:30:57 + --> $DIR/macros_invalid_input.rs:33:57 | -30 | #[tokio::test(flavor = "multi_thread", worker_threads = "foo")] +33 | #[tokio::test(flavor = "multi_thread", worker_threads = "foo")] | ^^^^^ error: The `worker_threads` option requires the `multi_thread` runtime flavor. Use `#[tokio::test(flavor = "multi_thread")]` - --> $DIR/macros_invalid_input.rs:33:59 + --> $DIR/macros_invalid_input.rs:36:59 | -33 | #[tokio::test(flavor = "current_thread", worker_threads = 4)] +36 | #[tokio::test(flavor = "current_thread", worker_threads = 4)] | ^ error: second test attribute is supplied - --> $DIR/macros_invalid_input.rs:37:1 + --> $DIR/macros_invalid_input.rs:40:1 | -37 | #[test] +40 | #[test] | ^^^^^^^ From 81488fdfa869c97cb4603503e6f5c14126225620 Mon Sep 17 00:00:00 2001 From: David Pedersen Date: Mon, 5 Apr 2021 22:02:23 +0200 Subject: [PATCH 4/5] Missing import Why does CI require this? --- tokio-macros/src/entry.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tokio-macros/src/entry.rs b/tokio-macros/src/entry.rs index 9d56e6c530b..d46ea8a1f1b 100644 --- a/tokio-macros/src/entry.rs +++ b/tokio-macros/src/entry.rs @@ -1,6 +1,7 @@ use proc_macro::TokenStream; use proc_macro2::Span; use quote::quote; +use syn::spanned::Spanned; #[derive(Clone, Copy, PartialEq)] enum RuntimeFlavor { From 8024cedcac7c7e9d5615d947ac9d8e5fd71d639b Mon Sep 17 00:00:00 2001 From: David Pedersen Date: Mon, 5 Apr 2021 22:06:39 +0200 Subject: [PATCH 5/5] try this --- tokio-macros/src/entry.rs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/tokio-macros/src/entry.rs b/tokio-macros/src/entry.rs index d46ea8a1f1b..a59447b9404 100644 --- a/tokio-macros/src/entry.rs +++ b/tokio-macros/src/entry.rs @@ -1,7 +1,6 @@ use proc_macro::TokenStream; use proc_macro2::Span; use quote::quote; -use syn::spanned::Spanned; #[derive(Clone, Copy, PartialEq)] enum RuntimeFlavor { @@ -216,13 +215,22 @@ fn parse_knobs( } match ident.unwrap().to_string().to_lowercase().as_str() { "worker_threads" => { - config.set_worker_threads(namevalue.lit.clone(), namevalue.lit.span())?; + config.set_worker_threads( + namevalue.lit.clone(), + syn::spanned::Spanned::span(&namevalue.lit), + )?; } "flavor" => { - config.set_flavor(namevalue.lit.clone(), namevalue.lit.span())?; + config.set_flavor( + namevalue.lit.clone(), + syn::spanned::Spanned::span(&namevalue.lit), + )?; } "start_paused" => { - config.set_start_paused(namevalue.lit.clone(), namevalue.lit.span())?; + config.set_start_paused( + namevalue.lit.clone(), + syn::spanned::Spanned::span(&namevalue.lit), + )?; } "core_threads" => { let msg = "Attribute `core_threads` is renamed to `worker_threads`";