From a1a239f846b070ce3147d62c9eb9a831fa3ae862 Mon Sep 17 00:00:00 2001 From: oddgrd <29732646+oddgrd@users.noreply.github.com> Date: Thu, 24 Nov 2022 13:06:04 +0100 Subject: [PATCH 01/15] feat: parse params --- codegen/src/next/mod.rs | 156 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 153 insertions(+), 3 deletions(-) diff --git a/codegen/src/next/mod.rs b/codegen/src/next/mod.rs index f996cfed3..55faf6c01 100644 --- a/codegen/src/next/mod.rs +++ b/codegen/src/next/mod.rs @@ -1,13 +1,98 @@ use proc_macro_error::emit_error; use quote::{quote, ToTokens}; -use syn::{Ident, LitStr}; +use syn::{ + braced, bracketed, parenthesized, parse::Parse, parse2, parse_macro_input, parse_quote, + punctuated::Punctuated, token::Paren, Expr, Ident, ItemFn, Lit, LitStr, Token, +}; +#[derive(Debug, Eq, PartialEq)] struct Endpoint { route: LitStr, method: Ident, function: Ident, } +#[derive(Debug, Eq, PartialEq)] +struct Parameter { + key: Ident, + equals: Token![=], + value: Expr, +} + +#[derive(Debug, Eq, PartialEq)] +struct Params { + params: Punctuated, + paren_token: Paren, +} + +impl Parse for Parameter { + fn parse(input: syn::parse::ParseStream) -> syn::Result { + Ok(Self { + key: input.parse()?, + equals: input.parse()?, + value: input.parse()?, + }) + } +} + +impl Parse for Params { + fn parse(input: syn::parse::ParseStream) -> syn::Result { + let content; + Ok(Self { + paren_token: parenthesized!(content in input), + params: content.parse_terminated(Parameter::parse)?, + }) + } +} + +impl Endpoint { + fn from_item_fn(item: ItemFn) -> syn::Result { + let function = item.sig.ident; + + let params = item.attrs[0].tokens.clone(); + let params: Params = parse2(params)?; + + let mut route = None; + let mut method = None; + + for Parameter { key, value, .. } in params.params { + match key.to_string().as_str() { + "method" => { + if let Expr::Path(path) = value { + method = Some(path.path.segments[0].ident.clone()); + }; + } + "route" => { + if let Expr::Lit(literal) = value { + if let Some(Lit::Str(literal)) = Some(literal.lit) { + route = Some(literal); + } + } + } + _ => todo!(), + } + } + + let route = if let Some(route) = route { + route + } else { + todo!() + }; + + let method = if let Some(method) = method { + method + } else { + todo!() + }; + + Ok(Endpoint { + route, + method, + function, + }) + } +} + impl ToTokens for Endpoint { fn to_tokens(&self, tokens: &mut proc_macro2::TokenStream) { let Self { @@ -133,9 +218,9 @@ mod tests { use quote::quote; use syn::parse_quote; - use crate::next::App; + use crate::next::{App, Parameter}; - use super::Endpoint; + use super::{Endpoint, Params}; #[test] fn endpoint_to_token() { @@ -189,4 +274,69 @@ mod tests { assert_eq!(actual.to_string(), expected.to_string()); } + + #[test] + fn parse_endpoint() { + let input = parse_quote! { + #[shuttle_codegen::endpoint(method = get, route = "/hello")] + async fn hello() -> &'static str { + "Hello, World!" + } + }; + + let actual = Endpoint::from_item_fn(input).unwrap(); + let expected = Endpoint { + route: parse_quote!("/hello"), + method: parse_quote!(get), + function: parse_quote!(hello), + }; + + assert_eq!(actual, expected); + } + + #[test] + fn parse_parameter() { + // test method param + let tests: Vec<(Parameter, Parameter)> = vec![ + ( + // parsing an identifier + parse_quote! { + method = get + }, + Parameter { + key: parse_quote!(method), + equals: parse_quote!(=), + value: parse_quote!(get), + }, + ), + ( + // parsing a string literal + parse_quote! { + route = "/hello" + }, + Parameter { + key: parse_quote!(route), + equals: parse_quote!(=), + value: parse_quote!("/hello"), + }, + ), + ]; + for (actual, expected) in tests { + assert_eq!(actual, expected); + } + } + + #[test] + fn parse_params() { + let actual: Params = parse_quote![(method = get, route = "/hello")]; + + let mut expected = Params { + params: Default::default(), + paren_token: Default::default(), + }; + expected.params.push(parse_quote!(method = get)); + expected.params.push(parse_quote!(route = "/hello")); + + assert_eq!(actual, expected); + } } From bf6aa58a7e92e16354c5f8b84cc93396ae66e11e Mon Sep 17 00:00:00 2001 From: oddgrd <29732646+oddgrd@users.noreply.github.com> Date: Thu, 24 Nov 2022 13:54:46 +0100 Subject: [PATCH 02/15] feat: parse app --- codegen/src/lib.rs | 15 ++++++++ codegen/src/main/mod.rs | 2 +- codegen/src/next/mod.rs | 79 +++++++++++++++++++++++++++++++++++++---- 3 files changed, 89 insertions(+), 7 deletions(-) diff --git a/codegen/src/lib.rs b/codegen/src/lib.rs index c96d2f96c..4ea87b16c 100644 --- a/codegen/src/lib.rs +++ b/codegen/src/lib.rs @@ -1,11 +1,26 @@ mod main; mod next; +use next::App; use proc_macro::TokenStream; use proc_macro_error::proc_macro_error; +use syn::{parse_macro_input, File}; #[proc_macro_error] #[proc_macro_attribute] pub fn main(attr: TokenStream, item: TokenStream) -> TokenStream { main::r#impl(attr, item) } + +#[proc_macro_error] +#[proc_macro] +pub fn app(item: TokenStream) -> TokenStream { + let mut file = parse_macro_input!(item as File); + // todo: handle error + let app = App::from_file(&mut file).unwrap(); + quote::quote!( + #file + #app + ) + .into() +} diff --git a/codegen/src/main/mod.rs b/codegen/src/main/mod.rs index a3ec3200a..be3939920 100644 --- a/codegen/src/main/mod.rs +++ b/codegen/src/main/mod.rs @@ -652,6 +652,6 @@ mod tests { #[test] fn ui() { let t = trybuild::TestCases::new(); - t.compile_fail("tests/ui/*.rs"); + t.compile_fail("tests/ui/main/*.rs"); } } diff --git a/codegen/src/next/mod.rs b/codegen/src/next/mod.rs index 55faf6c01..73207c68d 100644 --- a/codegen/src/next/mod.rs +++ b/codegen/src/next/mod.rs @@ -1,8 +1,8 @@ use proc_macro_error::emit_error; use quote::{quote, ToTokens}; use syn::{ - braced, bracketed, parenthesized, parse::Parse, parse2, parse_macro_input, parse_quote, - punctuated::Punctuated, token::Paren, Expr, Ident, ItemFn, Lit, LitStr, Token, + parenthesized, parse::Parse, parse2, punctuated::Punctuated, token::Paren, Expr, File, Ident, + Item, ItemFn, Lit, LitStr, Token, }; #[derive(Debug, Eq, PartialEq)] @@ -46,10 +46,13 @@ impl Parse for Params { } impl Endpoint { - fn from_item_fn(item: ItemFn) -> syn::Result { - let function = item.sig.ident; + fn from_item_fn(item: &mut ItemFn) -> syn::Result { + let function = item.sig.ident.clone(); let params = item.attrs[0].tokens.clone(); + + item.attrs.clear(); + let params: Params = parse2(params)?; let mut route = None; @@ -118,10 +121,29 @@ impl ToTokens for Endpoint { } } +#[derive(Debug, Eq, PartialEq)] pub(crate) struct App { endpoints: Vec, } +impl App { + pub(crate) fn from_file(file: &mut File) -> syn::Result { + let endpoints = file + .items + .iter_mut() + .filter_map(|item| { + if let Item::Fn(item_fn) = item { + Some(item_fn) + } else { + None + } + }) + .map(Endpoint::from_item_fn) + .collect::>>()?; + Ok(Self { endpoints }) + } +} + impl ToTokens for App { fn to_tokens(&self, tokens: &mut proc_macro2::TokenStream) { let Self { endpoints } = self; @@ -277,14 +299,15 @@ mod tests { #[test] fn parse_endpoint() { - let input = parse_quote! { + let mut input = parse_quote! { #[shuttle_codegen::endpoint(method = get, route = "/hello")] async fn hello() -> &'static str { "Hello, World!" } + }; - let actual = Endpoint::from_item_fn(input).unwrap(); + let actual = Endpoint::from_item_fn(&mut input).unwrap(); let expected = Endpoint { route: parse_quote!("/hello"), method: parse_quote!(get), @@ -292,6 +315,11 @@ mod tests { }; assert_eq!(actual, expected); + + assert!( + input.attrs.is_empty(), + "expected attributes to be stripped since there is no macro for them" + ); } #[test] @@ -339,4 +367,43 @@ mod tests { assert_eq!(actual, expected); } + + #[test] + fn parse_app() { + let mut input = parse_quote! { + #[shuttle_codegen::endpoint(method = get, route = "/hello")] + async fn hello() -> &'static str { + "Hello, World!" + } + + #[shuttle_codegen::endpoint(method = post, route = "/goodbye")] + async fn goodbye() -> &'static str { + "Goodbye, World!" + } + }; + + let actual = App::from_file(&mut input).unwrap(); + let expected = App { + endpoints: vec![ + Endpoint { + route: parse_quote!("/hello"), + method: parse_quote!(get), + function: parse_quote!(hello), + }, + Endpoint { + route: parse_quote!("/goodbye"), + method: parse_quote!(post), + function: parse_quote!(goodbye), + }, + ], + }; + + assert_eq!(actual, expected); + } + + #[test] + fn ui() { + let t = trybuild::TestCases::new(); + t.compile_fail("tests/ui/next/*.rs"); + } } From a90945a6704bb53496b9143fffb80852847416b2 Mon Sep 17 00:00:00 2001 From: oddgrd <29732646+oddgrd@users.noreply.github.com> Date: Thu, 24 Nov 2022 15:40:26 +0100 Subject: [PATCH 03/15] feat: add test for missing endpoint attribute --- codegen/src/lib.rs | 2 +- codegen/src/next/mod.rs | 28 +++++++++++++------ .../ui/next/missing-endpoint-attribute.rs | 9 ++++++ .../ui/next/missing-endpoint-attribute.stderr | 23 +++++++++++++++ 4 files changed, 52 insertions(+), 10 deletions(-) create mode 100644 codegen/tests/ui/next/missing-endpoint-attribute.rs create mode 100644 codegen/tests/ui/next/missing-endpoint-attribute.stderr diff --git a/codegen/src/lib.rs b/codegen/src/lib.rs index 4ea87b16c..6cb8b0ab9 100644 --- a/codegen/src/lib.rs +++ b/codegen/src/lib.rs @@ -17,7 +17,7 @@ pub fn main(attr: TokenStream, item: TokenStream) -> TokenStream { pub fn app(item: TokenStream) -> TokenStream { let mut file = parse_macro_input!(item as File); // todo: handle error - let app = App::from_file(&mut file).unwrap(); + let app = App::from_file(&mut file); quote::quote!( #file #app diff --git a/codegen/src/next/mod.rs b/codegen/src/next/mod.rs index 73207c68d..629a5522d 100644 --- a/codegen/src/next/mod.rs +++ b/codegen/src/next/mod.rs @@ -46,14 +46,23 @@ impl Parse for Params { } impl Endpoint { - fn from_item_fn(item: &mut ItemFn) -> syn::Result { + fn from_item_fn(item: &mut ItemFn) -> Option { let function = item.sig.ident.clone(); - let params = item.attrs[0].tokens.clone(); + let params = if let Some(attribute) = item.attrs.get(0) { + attribute.tokens.clone() + } else { + emit_error!( + function, + "no endpoint attribute given"; + hint = "Try adding `#[shuttle_codegen::endpoint(method = get, route = \"/hello\")]`" + ); + return None; + }; item.attrs.clear(); - let params: Params = parse2(params)?; + let params: Params = parse2(params).unwrap(); let mut route = None; let mut method = None; @@ -88,7 +97,7 @@ impl Endpoint { todo!() }; - Ok(Endpoint { + Some(Endpoint { route, method, function, @@ -127,7 +136,7 @@ pub(crate) struct App { } impl App { - pub(crate) fn from_file(file: &mut File) -> syn::Result { + pub(crate) fn from_file(file: &mut File) -> Self { let endpoints = file .items .iter_mut() @@ -138,9 +147,10 @@ impl App { None } }) - .map(Endpoint::from_item_fn) - .collect::>>()?; - Ok(Self { endpoints }) + .filter_map(Endpoint::from_item_fn) + .collect(); + + Self { endpoints } } } @@ -382,7 +392,7 @@ mod tests { } }; - let actual = App::from_file(&mut input).unwrap(); + let actual = App::from_file(&mut input); let expected = App { endpoints: vec![ Endpoint { diff --git a/codegen/tests/ui/next/missing-endpoint-attribute.rs b/codegen/tests/ui/next/missing-endpoint-attribute.rs new file mode 100644 index 000000000..f49a52a4b --- /dev/null +++ b/codegen/tests/ui/next/missing-endpoint-attribute.rs @@ -0,0 +1,9 @@ +shuttle_codegen::app! { + async fn hello() -> &'static str { + "Hello, World!" + } + + async fn goodbye() -> &'static str { + "Goodbye, World!" + } +} diff --git a/codegen/tests/ui/next/missing-endpoint-attribute.stderr b/codegen/tests/ui/next/missing-endpoint-attribute.stderr new file mode 100644 index 000000000..32ef67513 --- /dev/null +++ b/codegen/tests/ui/next/missing-endpoint-attribute.stderr @@ -0,0 +1,23 @@ +error: missing endpoint attribute + + = help: Try adding `#[shuttle_codegen::endpoint(method = get, route = "/hello")]` + + --> tests/ui/next/missing-endpoint-attribute.rs:2:14 + | +2 | async fn hello() -> &'static str { + | ^^^^^ + +error: missing endpoint attribute + + = help: Try adding `#[shuttle_codegen::endpoint(method = get, route = "/hello")]` + + --> tests/ui/next/missing-endpoint-attribute.rs:6:14 + | +6 | async fn goodbye() -> &'static str { + | ^^^^^^^ + +error[E0601]: `main` function not found in crate `$CRATE` + --> tests/ui/next/missing-endpoint-attribute.rs:9:2 + | +9 | } + | ^ consider adding a `main` function to `$DIR/tests/ui/next/missing-endpoint-attribute.rs` From 2fd4125662b85cce158f9673ff5d76d163091557 Mon Sep 17 00:00:00 2001 From: oddgrd <29732646+oddgrd@users.noreply.github.com> Date: Thu, 24 Nov 2022 19:13:02 +0100 Subject: [PATCH 04/15] test: invalid args, params and syntax --- codegen/src/next/mod.rs | 41 ++++++++++++++++--- .../ui/next/invalid-endpoint-argument.rs | 6 +++ .../ui/next/invalid-endpoint-argument.stderr | 14 +++++++ .../tests/ui/next/invalid-endpoint-syntax.rs | 11 +++++ .../ui/next/invalid-endpoint-syntax.stderr | 23 +++++++++++ .../tests/ui/next/missing-endpoint-param.rs | 11 +++++ .../ui/next/missing-endpoint-param.stderr | 23 +++++++++++ 7 files changed, 124 insertions(+), 5 deletions(-) create mode 100644 codegen/tests/ui/next/invalid-endpoint-argument.rs create mode 100644 codegen/tests/ui/next/invalid-endpoint-argument.stderr create mode 100644 codegen/tests/ui/next/invalid-endpoint-syntax.rs create mode 100644 codegen/tests/ui/next/invalid-endpoint-syntax.stderr create mode 100644 codegen/tests/ui/next/missing-endpoint-param.rs create mode 100644 codegen/tests/ui/next/missing-endpoint-param.stderr diff --git a/codegen/src/next/mod.rs b/codegen/src/next/mod.rs index 629a5522d..311a513e0 100644 --- a/codegen/src/next/mod.rs +++ b/codegen/src/next/mod.rs @@ -54,7 +54,7 @@ impl Endpoint { } else { emit_error!( function, - "no endpoint attribute given"; + "missing endpoint attribute"; hint = "Try adding `#[shuttle_codegen::endpoint(method = get, route = \"/hello\")]`" ); return None; @@ -62,12 +62,23 @@ impl Endpoint { item.attrs.clear(); - let params: Params = parse2(params).unwrap(); + let params: Params = match parse2(params) { + Ok(params) => params, + Err(err) => { + emit_error!( + err.span(), + err; + hint = "The endpoint takes a comma-separated list of keys and values: `endpoint(method = get, route = \"/hello\")`" + ); + return None; + } + }; let mut route = None; let mut method = None; for Parameter { key, value, .. } in params.params { + let key_ident = key.clone(); match key.to_string().as_str() { "method" => { if let Expr::Path(path) = value { @@ -81,20 +92,40 @@ impl Endpoint { } } } - _ => todo!(), + _ => { + emit_error!( + key_ident, + "invalid endpoint argument"; + hint = "Only `method` and `route` are valid endpoint arguments." + ); + return None; + } } } + // use paren span for missing argument errors + let paren = params.paren_token; + let route = if let Some(route) = route { route } else { - todo!() + emit_error!( + paren.span, + "no route provided"; + hint = "Add a route to your endpoint: `#[shuttle_codegen::endpoint(method = get, route = \"/hello\")]`" + ); + return None; }; let method = if let Some(method) = method { method } else { - todo!() + emit_error!( + paren.span, + "no method provided"; + hint = "Add a method to your endpoint: `#[shuttle_codegen::endpoint(method = get, route = \"/hello\")]`" + ); + return None; }; Some(Endpoint { diff --git a/codegen/tests/ui/next/invalid-endpoint-argument.rs b/codegen/tests/ui/next/invalid-endpoint-argument.rs new file mode 100644 index 000000000..e77b39f99 --- /dev/null +++ b/codegen/tests/ui/next/invalid-endpoint-argument.rs @@ -0,0 +1,6 @@ +shuttle_codegen::app! { + #[shuttle_codegen::endpoint(method = get, route = "/goodbye", invalid = bad)] + async fn goodbye() -> &'static str { + "Goodbye, World!" + } +} diff --git a/codegen/tests/ui/next/invalid-endpoint-argument.stderr b/codegen/tests/ui/next/invalid-endpoint-argument.stderr new file mode 100644 index 000000000..779c84421 --- /dev/null +++ b/codegen/tests/ui/next/invalid-endpoint-argument.stderr @@ -0,0 +1,14 @@ +error: invalid endpoint argument + + = help: Only `method` and `route` are valid endpoint arguments. + + --> tests/ui/next/invalid-endpoint-argument.rs:2:67 + | +2 | #[shuttle_codegen::endpoint(method = get, route = "/goodbye", invalid = bad)] + | ^^^^^^^ + +error[E0601]: `main` function not found in crate `$CRATE` + --> tests/ui/next/invalid-endpoint-argument.rs:6:2 + | +6 | } + | ^ consider adding a `main` function to `$DIR/tests/ui/next/invalid-endpoint-argument.rs` diff --git a/codegen/tests/ui/next/invalid-endpoint-syntax.rs b/codegen/tests/ui/next/invalid-endpoint-syntax.rs new file mode 100644 index 000000000..9bf7a2d8c --- /dev/null +++ b/codegen/tests/ui/next/invalid-endpoint-syntax.rs @@ -0,0 +1,11 @@ +shuttle_codegen::app! { + #[shuttle_codegen::endpoint(method = get, route = "/hello" extra = abundant)] + async fn hello() -> &'static str { + "Hello, World!" + } + + #[shuttle_codegen::endpoint(method = get, route = "/goodbye", invalid)] + async fn goodbye() -> &'static str { + "Goodbye, World!" + } +} diff --git a/codegen/tests/ui/next/invalid-endpoint-syntax.stderr b/codegen/tests/ui/next/invalid-endpoint-syntax.stderr new file mode 100644 index 000000000..dc72ede1b --- /dev/null +++ b/codegen/tests/ui/next/invalid-endpoint-syntax.stderr @@ -0,0 +1,23 @@ +error: expected `,` + + = help: The endpoint takes a comma-separated list of keys and values: `endpoint(method = get, route = "/hello")` + + --> tests/ui/next/invalid-endpoint-syntax.rs:2:64 + | +2 | #[shuttle_codegen::endpoint(method = get, route = "/hello" extra = abundant)] + | ^^^^^ + +error: expected `=` + + = help: The endpoint takes a comma-separated list of keys and values: `endpoint(method = get, route = "/hello")` + + --> tests/ui/next/invalid-endpoint-syntax.rs:7:74 + | +7 | #[shuttle_codegen::endpoint(method = get, route = "/goodbye", invalid)] + | ^ + +error[E0601]: `main` function not found in crate `$CRATE` + --> tests/ui/next/invalid-endpoint-syntax.rs:11:2 + | +11 | } + | ^ consider adding a `main` function to `$DIR/tests/ui/next/invalid-endpoint-syntax.rs` diff --git a/codegen/tests/ui/next/missing-endpoint-param.rs b/codegen/tests/ui/next/missing-endpoint-param.rs new file mode 100644 index 000000000..8c53d4d6e --- /dev/null +++ b/codegen/tests/ui/next/missing-endpoint-param.rs @@ -0,0 +1,11 @@ +shuttle_codegen::app! { + #[shuttle_codegen::endpoint(method = get)] + async fn hello() -> &'static str { + "Hello, World!" + } + + #[shuttle_codegen::endpoint(route = "/goodbye")] + async fn goodbye() -> &'static str { + "Goodbye, World!" + } +} diff --git a/codegen/tests/ui/next/missing-endpoint-param.stderr b/codegen/tests/ui/next/missing-endpoint-param.stderr new file mode 100644 index 000000000..b6582ad80 --- /dev/null +++ b/codegen/tests/ui/next/missing-endpoint-param.stderr @@ -0,0 +1,23 @@ +error: no route provided + + = help: Add a route to your endpoint: `#[shuttle_codegen::endpoint(method = get, route = "/hello")]` + + --> tests/ui/next/missing-endpoint-param.rs:2:32 + | +2 | #[shuttle_codegen::endpoint(method = get)] + | ^^^^^^^^^^^^^^ + +error: no method provided + + = help: Add a method to your endpoint: `#[shuttle_codegen::endpoint(method = get, route = "/hello")]` + + --> tests/ui/next/missing-endpoint-param.rs:7:32 + | +7 | #[shuttle_codegen::endpoint(route = "/goodbye")] + | ^^^^^^^^^^^^^^^^^^^^ + +error[E0601]: `main` function not found in crate `$CRATE` + --> tests/ui/next/missing-endpoint-param.rs:11:2 + | +11 | } + | ^ consider adding a `main` function to `$DIR/tests/ui/next/missing-endpoint-param.rs` From 25716167f2172355d515e836770020dd39962b3f Mon Sep 17 00:00:00 2001 From: oddgrd <29732646+oddgrd@users.noreply.github.com> Date: Thu, 24 Nov 2022 19:26:55 +0100 Subject: [PATCH 05/15] feat: check for and test no params --- codegen/src/next/mod.rs | 15 ++++++++++++--- ...oint-argument.rs => invalid-endpoint-param.rs} | 0 ...ument.stderr => invalid-endpoint-param.stderr} | 6 +++--- codegen/tests/ui/next/missing-endpoint-param.rs | 5 +++++ .../tests/ui/next/missing-endpoint-param.stderr | 13 +++++++++++-- 5 files changed, 31 insertions(+), 8 deletions(-) rename codegen/tests/ui/next/{invalid-endpoint-argument.rs => invalid-endpoint-param.rs} (100%) rename codegen/tests/ui/next/{invalid-endpoint-argument.stderr => invalid-endpoint-param.stderr} (75%) diff --git a/codegen/src/next/mod.rs b/codegen/src/next/mod.rs index 311a513e0..57ad0d667 100644 --- a/codegen/src/next/mod.rs +++ b/codegen/src/next/mod.rs @@ -74,6 +74,18 @@ impl Endpoint { } }; + // use paren span for missing argument errors + let paren = params.paren_token; + + if params.params.is_empty() { + emit_error!( + paren.span, + "missing endpoint arguments"; + hint = "The endpoint takes two arguments: `endpoint(method = get, route = \"/hello\")`" + ); + return None; + } + let mut route = None; let mut method = None; @@ -103,9 +115,6 @@ impl Endpoint { } } - // use paren span for missing argument errors - let paren = params.paren_token; - let route = if let Some(route) = route { route } else { diff --git a/codegen/tests/ui/next/invalid-endpoint-argument.rs b/codegen/tests/ui/next/invalid-endpoint-param.rs similarity index 100% rename from codegen/tests/ui/next/invalid-endpoint-argument.rs rename to codegen/tests/ui/next/invalid-endpoint-param.rs diff --git a/codegen/tests/ui/next/invalid-endpoint-argument.stderr b/codegen/tests/ui/next/invalid-endpoint-param.stderr similarity index 75% rename from codegen/tests/ui/next/invalid-endpoint-argument.stderr rename to codegen/tests/ui/next/invalid-endpoint-param.stderr index 779c84421..f52aa0858 100644 --- a/codegen/tests/ui/next/invalid-endpoint-argument.stderr +++ b/codegen/tests/ui/next/invalid-endpoint-param.stderr @@ -2,13 +2,13 @@ error: invalid endpoint argument = help: Only `method` and `route` are valid endpoint arguments. - --> tests/ui/next/invalid-endpoint-argument.rs:2:67 + --> tests/ui/next/invalid-endpoint-param.rs:2:67 | 2 | #[shuttle_codegen::endpoint(method = get, route = "/goodbye", invalid = bad)] | ^^^^^^^ error[E0601]: `main` function not found in crate `$CRATE` - --> tests/ui/next/invalid-endpoint-argument.rs:6:2 + --> tests/ui/next/invalid-endpoint-param.rs:6:2 | 6 | } - | ^ consider adding a `main` function to `$DIR/tests/ui/next/invalid-endpoint-argument.rs` + | ^ consider adding a `main` function to `$DIR/tests/ui/next/invalid-endpoint-param.rs` diff --git a/codegen/tests/ui/next/missing-endpoint-param.rs b/codegen/tests/ui/next/missing-endpoint-param.rs index 8c53d4d6e..abeeaa591 100644 --- a/codegen/tests/ui/next/missing-endpoint-param.rs +++ b/codegen/tests/ui/next/missing-endpoint-param.rs @@ -8,4 +8,9 @@ shuttle_codegen::app! { async fn goodbye() -> &'static str { "Goodbye, World!" } + + #[shuttle_codegen::endpoint()] + async fn goodbye() -> &'static str { + "Goodbye, World!" + } } diff --git a/codegen/tests/ui/next/missing-endpoint-param.stderr b/codegen/tests/ui/next/missing-endpoint-param.stderr index b6582ad80..45254dbaa 100644 --- a/codegen/tests/ui/next/missing-endpoint-param.stderr +++ b/codegen/tests/ui/next/missing-endpoint-param.stderr @@ -16,8 +16,17 @@ error: no method provided 7 | #[shuttle_codegen::endpoint(route = "/goodbye")] | ^^^^^^^^^^^^^^^^^^^^ +error: missing endpoint arguments + + = help: The endpoint takes two arguments: `endpoint(method = get, route = "/hello")` + + --> tests/ui/next/missing-endpoint-param.rs:12:32 + | +12 | #[shuttle_codegen::endpoint()] + | ^^ + error[E0601]: `main` function not found in crate `$CRATE` - --> tests/ui/next/missing-endpoint-param.rs:11:2 + --> tests/ui/next/missing-endpoint-param.rs:16:2 | -11 | } +16 | } | ^ consider adding a `main` function to `$DIR/tests/ui/next/missing-endpoint-param.rs` From 96732b9e552cc7f4fa63bb135f8478ea0ca9631f Mon Sep 17 00:00:00 2001 From: oddgrd <29732646+oddgrd@users.noreply.github.com> Date: Fri, 25 Nov 2022 11:19:45 +0100 Subject: [PATCH 06/15] feat: don't emit error for missing annotation this way users can create separate functions for their endpoint logic --- codegen/src/next/mod.rs | 5 ---- .../ui/next/missing-endpoint-attribute.rs | 9 -------- .../ui/next/missing-endpoint-attribute.stderr | 23 ------------------- 3 files changed, 37 deletions(-) delete mode 100644 codegen/tests/ui/next/missing-endpoint-attribute.rs delete mode 100644 codegen/tests/ui/next/missing-endpoint-attribute.stderr diff --git a/codegen/src/next/mod.rs b/codegen/src/next/mod.rs index 57ad0d667..6d8536ef1 100644 --- a/codegen/src/next/mod.rs +++ b/codegen/src/next/mod.rs @@ -78,11 +78,6 @@ impl Endpoint { let paren = params.paren_token; if params.params.is_empty() { - emit_error!( - paren.span, - "missing endpoint arguments"; - hint = "The endpoint takes two arguments: `endpoint(method = get, route = \"/hello\")`" - ); return None; } diff --git a/codegen/tests/ui/next/missing-endpoint-attribute.rs b/codegen/tests/ui/next/missing-endpoint-attribute.rs deleted file mode 100644 index f49a52a4b..000000000 --- a/codegen/tests/ui/next/missing-endpoint-attribute.rs +++ /dev/null @@ -1,9 +0,0 @@ -shuttle_codegen::app! { - async fn hello() -> &'static str { - "Hello, World!" - } - - async fn goodbye() -> &'static str { - "Goodbye, World!" - } -} diff --git a/codegen/tests/ui/next/missing-endpoint-attribute.stderr b/codegen/tests/ui/next/missing-endpoint-attribute.stderr deleted file mode 100644 index 32ef67513..000000000 --- a/codegen/tests/ui/next/missing-endpoint-attribute.stderr +++ /dev/null @@ -1,23 +0,0 @@ -error: missing endpoint attribute - - = help: Try adding `#[shuttle_codegen::endpoint(method = get, route = "/hello")]` - - --> tests/ui/next/missing-endpoint-attribute.rs:2:14 - | -2 | async fn hello() -> &'static str { - | ^^^^^ - -error: missing endpoint attribute - - = help: Try adding `#[shuttle_codegen::endpoint(method = get, route = "/hello")]` - - --> tests/ui/next/missing-endpoint-attribute.rs:6:14 - | -6 | async fn goodbye() -> &'static str { - | ^^^^^^^ - -error[E0601]: `main` function not found in crate `$CRATE` - --> tests/ui/next/missing-endpoint-attribute.rs:9:2 - | -9 | } - | ^ consider adding a `main` function to `$DIR/tests/ui/next/missing-endpoint-attribute.rs` From 58c216eded0fff0c2cb8679e299288c4e3039601 Mon Sep 17 00:00:00 2001 From: oddgrd <29732646+oddgrd@users.noreply.github.com> Date: Fri, 25 Nov 2022 11:42:55 +0100 Subject: [PATCH 07/15] fix: remove missing attribute error, not param --- codegen/src/next/mod.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/codegen/src/next/mod.rs b/codegen/src/next/mod.rs index 6d8536ef1..6add92607 100644 --- a/codegen/src/next/mod.rs +++ b/codegen/src/next/mod.rs @@ -52,11 +52,6 @@ impl Endpoint { let params = if let Some(attribute) = item.attrs.get(0) { attribute.tokens.clone() } else { - emit_error!( - function, - "missing endpoint attribute"; - hint = "Try adding `#[shuttle_codegen::endpoint(method = get, route = \"/hello\")]`" - ); return None; }; @@ -78,6 +73,11 @@ impl Endpoint { let paren = params.paren_token; if params.params.is_empty() { + emit_error!( + paren.span, + "missing endpoint arguments"; + hint = "The endpoint takes two arguments: `endpoint(method = get, route = \"/hello\")`" + ); return None; } From e65f2334de4d932c5e2b1f4174ff2dec7deae1ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oddbj=C3=B8rn=20Gr=C3=B8dem?= <29732646+oddgrd@users.noreply.github.com> Date: Fri, 25 Nov 2022 14:06:33 +0100 Subject: [PATCH 08/15] refactor: reword comment Co-authored-by: Pieter --- codegen/src/next/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codegen/src/next/mod.rs b/codegen/src/next/mod.rs index 6add92607..7feece77e 100644 --- a/codegen/src/next/mod.rs +++ b/codegen/src/next/mod.rs @@ -69,7 +69,7 @@ impl Endpoint { } }; - // use paren span for missing argument errors + // We'll use the paren span for errors later let paren = params.paren_token; if params.params.is_empty() { From 1179f3e4d15342f03d4310cf9dca8da447682311 Mon Sep 17 00:00:00 2001 From: oddgrd <29732646+oddgrd@users.noreply.github.com> Date: Fri, 25 Nov 2022 14:35:10 +0100 Subject: [PATCH 09/15] feat: duplicate param test, address review --- codegen/src/lib.rs | 3 ++- codegen/src/next/mod.rs | 22 +++++++++++++++--- .../tests/ui/next/duplicate-endpoint-param.rs | 11 +++++++++ .../ui/next/duplicate-endpoint-param.stderr | 23 +++++++++++++++++++ .../ui/next/invalid-endpoint-syntax.stderr | 4 ++-- .../ui/next/missing-endpoint-param.stderr | 4 ++-- 6 files changed, 59 insertions(+), 8 deletions(-) create mode 100644 codegen/tests/ui/next/duplicate-endpoint-param.rs create mode 100644 codegen/tests/ui/next/duplicate-endpoint-param.stderr diff --git a/codegen/src/lib.rs b/codegen/src/lib.rs index 6cb8b0ab9..384e19373 100644 --- a/codegen/src/lib.rs +++ b/codegen/src/lib.rs @@ -16,8 +16,9 @@ pub fn main(attr: TokenStream, item: TokenStream) -> TokenStream { #[proc_macro] pub fn app(item: TokenStream) -> TokenStream { let mut file = parse_macro_input!(item as File); - // todo: handle error + let app = App::from_file(&mut file); + quote::quote!( #file #app diff --git a/codegen/src/next/mod.rs b/codegen/src/next/mod.rs index 7feece77e..52a33c288 100644 --- a/codegen/src/next/mod.rs +++ b/codegen/src/next/mod.rs @@ -63,7 +63,7 @@ impl Endpoint { emit_error!( err.span(), err; - hint = "The endpoint takes a comma-separated list of keys and values: `endpoint(method = get, route = \"/hello\")`" + hint = "The endpoint takes two arguments: `endpoint(method = get, route = \"/hello\")`" ); return None; } @@ -88,11 +88,27 @@ impl Endpoint { let key_ident = key.clone(); match key.to_string().as_str() { "method" => { + if method.is_some() { + emit_error!( + key_ident, + "duplicate endpoint method"; + hint = "The endpoint `method` should only be set once." + ); + return None; + } if let Expr::Path(path) = value { method = Some(path.path.segments[0].ident.clone()); }; } "route" => { + if route.is_some() { + emit_error!( + key_ident, + "duplicate endpoint route"; + hint = "The endpoint `route` should only be set once." + ); + return None; + } if let Expr::Lit(literal) = value { if let Some(Lit::Str(literal)) = Some(literal.lit) { route = Some(literal); @@ -116,7 +132,7 @@ impl Endpoint { emit_error!( paren.span, "no route provided"; - hint = "Add a route to your endpoint: `#[shuttle_codegen::endpoint(method = get, route = \"/hello\")]`" + hint = "Add a route to your endpoint: `route = \"/hello\")`" ); return None; }; @@ -127,7 +143,7 @@ impl Endpoint { emit_error!( paren.span, "no method provided"; - hint = "Add a method to your endpoint: `#[shuttle_codegen::endpoint(method = get, route = \"/hello\")]`" + hint = "Add a method to your endpoint: `method = get`" ); return None; }; diff --git a/codegen/tests/ui/next/duplicate-endpoint-param.rs b/codegen/tests/ui/next/duplicate-endpoint-param.rs new file mode 100644 index 000000000..7830c9ad3 --- /dev/null +++ b/codegen/tests/ui/next/duplicate-endpoint-param.rs @@ -0,0 +1,11 @@ +shuttle_codegen::app! { + #[shuttle_codegen::endpoint(method = get, method = get)] + async fn hello() -> &'static str { + "Hello, World!" + } + + #[shuttle_codegen::endpoint(route = "/goodbye", route = "/goodbye")] + async fn goodbye() -> &'static str { + "Goodbye, World!" + } +} diff --git a/codegen/tests/ui/next/duplicate-endpoint-param.stderr b/codegen/tests/ui/next/duplicate-endpoint-param.stderr new file mode 100644 index 000000000..ffaff59b9 --- /dev/null +++ b/codegen/tests/ui/next/duplicate-endpoint-param.stderr @@ -0,0 +1,23 @@ +error: duplicate endpoint method + + = help: The endpoint `method` should only be set once. + + --> tests/ui/next/duplicate-endpoint-param.rs:2:47 + | +2 | #[shuttle_codegen::endpoint(method = get, method = get)] + | ^^^^^^ + +error: duplicate endpoint route + + = help: The endpoint `route` should only be set once. + + --> tests/ui/next/duplicate-endpoint-param.rs:7:53 + | +7 | #[shuttle_codegen::endpoint(route = "/goodbye", route = "/goodbye")] + | ^^^^^ + +error[E0601]: `main` function not found in crate `$CRATE` + --> tests/ui/next/duplicate-endpoint-param.rs:11:2 + | +11 | } + | ^ consider adding a `main` function to `$DIR/tests/ui/next/duplicate-endpoint-param.rs` diff --git a/codegen/tests/ui/next/invalid-endpoint-syntax.stderr b/codegen/tests/ui/next/invalid-endpoint-syntax.stderr index dc72ede1b..61994bc59 100644 --- a/codegen/tests/ui/next/invalid-endpoint-syntax.stderr +++ b/codegen/tests/ui/next/invalid-endpoint-syntax.stderr @@ -1,6 +1,6 @@ error: expected `,` - = help: The endpoint takes a comma-separated list of keys and values: `endpoint(method = get, route = "/hello")` + = help: The endpoint takes two arguments: `endpoint(method = get, route = "/hello")` --> tests/ui/next/invalid-endpoint-syntax.rs:2:64 | @@ -9,7 +9,7 @@ error: expected `,` error: expected `=` - = help: The endpoint takes a comma-separated list of keys and values: `endpoint(method = get, route = "/hello")` + = help: The endpoint takes two arguments: `endpoint(method = get, route = "/hello")` --> tests/ui/next/invalid-endpoint-syntax.rs:7:74 | diff --git a/codegen/tests/ui/next/missing-endpoint-param.stderr b/codegen/tests/ui/next/missing-endpoint-param.stderr index 45254dbaa..99150e32f 100644 --- a/codegen/tests/ui/next/missing-endpoint-param.stderr +++ b/codegen/tests/ui/next/missing-endpoint-param.stderr @@ -1,6 +1,6 @@ error: no route provided - = help: Add a route to your endpoint: `#[shuttle_codegen::endpoint(method = get, route = "/hello")]` + = help: Add a route to your endpoint: `route = "/hello")` --> tests/ui/next/missing-endpoint-param.rs:2:32 | @@ -9,7 +9,7 @@ error: no route provided error: no method provided - = help: Add a method to your endpoint: `#[shuttle_codegen::endpoint(method = get, route = "/hello")]` + = help: Add a method to your endpoint: `method = get` --> tests/ui/next/missing-endpoint-param.rs:7:32 | From 55da1446ddafea46b2b4d32fb823ce36ed457f32 Mon Sep 17 00:00:00 2001 From: oddgrd <29732646+oddgrd@users.noreply.github.com> Date: Fri, 25 Nov 2022 23:11:31 +0100 Subject: [PATCH 10/15] feat: only strip endpoint attributes --- codegen/src/next/mod.rs | 91 ++++++++++++++----- .../ui/next/invalid-endpoint-syntax.stderr | 4 +- .../tests/ui/next/missing-endpoint-param.rs | 6 +- 3 files changed, 73 insertions(+), 28 deletions(-) diff --git a/codegen/src/next/mod.rs b/codegen/src/next/mod.rs index 52a33c288..d9e56c9fc 100644 --- a/codegen/src/next/mod.rs +++ b/codegen/src/next/mod.rs @@ -49,21 +49,38 @@ impl Endpoint { fn from_item_fn(item: &mut ItemFn) -> Option { let function = item.sig.ident.clone(); - let params = if let Some(attribute) = item.attrs.get(0) { - attribute.tokens.clone() + let endpoint_index; + // Find the index of an attribute that is an endpoint + if let Some(index) = item.attrs.iter().position(|attr| { + if let Some(segment) = attr.path.segments.last() { + segment.ident.to_string().as_str() == "endpoint" + } else { + false + } + }) { + endpoint_index = Some(index); } else { + // This item does not have an endpoint attribute return None; }; - item.attrs.clear(); + // Strip the endpoint attribute if it exists + let endpoint = if let Some(index) = endpoint_index { + item.attrs.remove(index) + } else { + // This item does not have an endpoint attribute + return None; + }; - let params: Params = match parse2(params) { + // Parse the endpoint's parameters + let params: Params = match parse2(endpoint.tokens) { Ok(params) => params, Err(err) => { + // This will error on invalid parameter syntax emit_error!( err.span(), err; - hint = "The endpoint takes two arguments: `endpoint(method = get, route = \"/hello\")`" + hint = "" ); return None; } @@ -360,27 +377,55 @@ mod tests { #[test] fn parse_endpoint() { - let mut input = parse_quote! { - #[shuttle_codegen::endpoint(method = get, route = "/hello")] - async fn hello() -> &'static str { - "Hello, World!" - } - - }; + let inputs = vec![ + ( + parse_quote! { + #[shuttle_codegen::endpoint(method = get, route = "/hello")] + async fn hello() -> &'static str { + "Hello, World!" + }}, + Some(Endpoint { + route: parse_quote!("/hello"), + method: parse_quote!(get), + function: parse_quote!(hello), + }), + 0, + ), + ( + parse_quote! { + #[doc = r" This attribute is not an endpoint so keep it"] + #[shuttle_codegen::endpoint(method = get, route = "/hello")] + async fn hello() -> &'static str { + "Hello, World!" + }}, + Some(Endpoint { + route: parse_quote!("/hello"), + method: parse_quote!(get), + function: parse_quote!(hello), + }), + 1, + ), + ( + parse_quote! { + #[doc = r" This attribute is not an endpoint so keep it"] + async fn say_hello() -> &'static str { + "Hello, World!" + } + }, + None, + 1, + ), + ]; - let actual = Endpoint::from_item_fn(&mut input).unwrap(); - let expected = Endpoint { - route: parse_quote!("/hello"), - method: parse_quote!(get), - function: parse_quote!(hello), - }; + for (mut input, expected, remaining_attributes) in inputs { + let actual = Endpoint::from_item_fn(&mut input); - assert_eq!(actual, expected); + assert_eq!(actual, expected); - assert!( - input.attrs.is_empty(), - "expected attributes to be stripped since there is no macro for them" - ); + // If input was an endpoint, its attribute should be stripped, + // if not an endpoint the attribute should remain + assert_eq!(input.attrs.len(), remaining_attributes); + } } #[test] diff --git a/codegen/tests/ui/next/invalid-endpoint-syntax.stderr b/codegen/tests/ui/next/invalid-endpoint-syntax.stderr index 61994bc59..478bd56aa 100644 --- a/codegen/tests/ui/next/invalid-endpoint-syntax.stderr +++ b/codegen/tests/ui/next/invalid-endpoint-syntax.stderr @@ -1,6 +1,6 @@ error: expected `,` - = help: The endpoint takes two arguments: `endpoint(method = get, route = "/hello")` + = help: --> tests/ui/next/invalid-endpoint-syntax.rs:2:64 | @@ -9,7 +9,7 @@ error: expected `,` error: expected `=` - = help: The endpoint takes two arguments: `endpoint(method = get, route = "/hello")` + = help: --> tests/ui/next/invalid-endpoint-syntax.rs:7:74 | diff --git a/codegen/tests/ui/next/missing-endpoint-param.rs b/codegen/tests/ui/next/missing-endpoint-param.rs index abeeaa591..a4fa91bf4 100644 --- a/codegen/tests/ui/next/missing-endpoint-param.rs +++ b/codegen/tests/ui/next/missing-endpoint-param.rs @@ -1,16 +1,16 @@ shuttle_codegen::app! { #[shuttle_codegen::endpoint(method = get)] - async fn hello() -> &'static str { + async fn only_method_param() -> &'static str { "Hello, World!" } #[shuttle_codegen::endpoint(route = "/goodbye")] - async fn goodbye() -> &'static str { + async fn only_route_param() -> &'static str { "Goodbye, World!" } #[shuttle_codegen::endpoint()] - async fn goodbye() -> &'static str { + async fn no_params() -> &'static str { "Goodbye, World!" } } From 617e939608b72562dbca085fff17437286289459 Mon Sep 17 00:00:00 2001 From: oddgrd <29732646+oddgrd@users.noreply.github.com> Date: Sat, 26 Nov 2022 15:04:07 +0100 Subject: [PATCH 11/15] feat: check for extra endpoint attributes --- codegen/src/next/mod.rs | 44 +++++++++++-------- .../ui/next/extra-endpoint-attributes.rs | 6 +++ .../ui/next/extra-endpoint-attributes.stderr | 21 +++++++++ 3 files changed, 52 insertions(+), 19 deletions(-) create mode 100644 codegen/tests/ui/next/extra-endpoint-attributes.rs create mode 100644 codegen/tests/ui/next/extra-endpoint-attributes.stderr diff --git a/codegen/src/next/mod.rs b/codegen/src/next/mod.rs index d9e56c9fc..4855bc43d 100644 --- a/codegen/src/next/mod.rs +++ b/codegen/src/next/mod.rs @@ -1,4 +1,4 @@ -use proc_macro_error::emit_error; +use proc_macro_error::{emit_call_site_error, emit_error}; use quote::{quote, ToTokens}; use syn::{ parenthesized, parse::Parse, parse2, punctuated::Punctuated, token::Paren, Expr, File, Ident, @@ -49,20 +49,26 @@ impl Endpoint { fn from_item_fn(item: &mut ItemFn) -> Option { let function = item.sig.ident.clone(); - let endpoint_index; + let mut endpoint_index = None; + // Find the index of an attribute that is an endpoint - if let Some(index) = item.attrs.iter().position(|attr| { - if let Some(segment) = attr.path.segments.last() { - segment.ident.to_string().as_str() == "endpoint" + for index in 0..item.attrs.len() { + // The endpoint ident should be the last segment in the path + if let Some(segment) = item.attrs[index].path.segments.last() { + if segment.ident.to_string().as_str() == "endpoint" { + if endpoint_index.is_some() { + emit_call_site_error!( + "more than one endpoint attribute provided"; + hint = "There should only be one endpoint annotation per handler function." + ); + return None; + } + endpoint_index = Some(index); + } } else { - false + return None; } - }) { - endpoint_index = Some(index); - } else { - // This item does not have an endpoint attribute - return None; - }; + } // Strip the endpoint attribute if it exists let endpoint = if let Some(index) = endpoint_index { @@ -247,6 +253,7 @@ impl ToTokens for App { } } +#[allow(dead_code)] pub(crate) fn wasi_bindings(app: App) -> proc_macro2::TokenStream { quote!( #app @@ -377,7 +384,7 @@ mod tests { #[test] fn parse_endpoint() { - let inputs = vec![ + let cases = vec![ ( parse_quote! { #[shuttle_codegen::endpoint(method = get, route = "/hello")] @@ -407,7 +414,7 @@ mod tests { ), ( parse_quote! { - #[doc = r" This attribute is not an endpoint so keep it"] + /// This attribute is not an endpoint so keep it async fn say_hello() -> &'static str { "Hello, World!" } @@ -417,13 +424,12 @@ mod tests { ), ]; - for (mut input, expected, remaining_attributes) in inputs { + for (mut input, expected, remaining_attributes) in cases { let actual = Endpoint::from_item_fn(&mut input); assert_eq!(actual, expected); - // If input was an endpoint, its attribute should be stripped, - // if not an endpoint the attribute should remain + // Verify that only endpoint attributes have been stripped assert_eq!(input.attrs.len(), remaining_attributes); } } @@ -431,7 +437,7 @@ mod tests { #[test] fn parse_parameter() { // test method param - let tests: Vec<(Parameter, Parameter)> = vec![ + let cases: Vec<(Parameter, Parameter)> = vec![ ( // parsing an identifier parse_quote! { @@ -455,7 +461,7 @@ mod tests { }, ), ]; - for (actual, expected) in tests { + for (actual, expected) in cases { assert_eq!(actual, expected); } } diff --git a/codegen/tests/ui/next/extra-endpoint-attributes.rs b/codegen/tests/ui/next/extra-endpoint-attributes.rs new file mode 100644 index 000000000..966b36c3b --- /dev/null +++ b/codegen/tests/ui/next/extra-endpoint-attributes.rs @@ -0,0 +1,6 @@ +shuttle_codegen::app! { + #[shuttle_codegen::endpoint(method = get, route = "/hello")] + #[shuttle_codegen::endpoint(method = post, route = "/hello")] + async fn hello() -> &'static str { + "Hello, World!" +}} diff --git a/codegen/tests/ui/next/extra-endpoint-attributes.stderr b/codegen/tests/ui/next/extra-endpoint-attributes.stderr new file mode 100644 index 000000000..5dbe42522 --- /dev/null +++ b/codegen/tests/ui/next/extra-endpoint-attributes.stderr @@ -0,0 +1,21 @@ +error: more than one endpoint attribute provided + + = help: There should only be one endpoint annotation per handler function. + + --> tests/ui/next/extra-endpoint-attributes.rs:1:1 + | +1 | / shuttle_codegen::app! { +2 | | #[shuttle_codegen::endpoint(method = get, route = "/hello")] +3 | | #[shuttle_codegen::endpoint(method = post, route = "/hello")] +4 | | async fn hello() -> &'static str { +5 | | "Hello, World!" +6 | | }} + | |__^ + | + = note: this error originates in the macro `shuttle_codegen::app` (in Nightly builds, run with -Z macro-backtrace for more info) + +error[E0601]: `main` function not found in crate `$CRATE` + --> tests/ui/next/extra-endpoint-attributes.rs:6:3 + | +6 | }} + | ^ consider adding a `main` function to `$DIR/tests/ui/next/extra-endpoint-attributes.rs` From e6a99dfe141e786ad4df5db1af314e10abc663ad Mon Sep 17 00:00:00 2001 From: oddgrd <29732646+oddgrd@users.noreply.github.com> Date: Sat, 26 Nov 2022 15:16:03 +0100 Subject: [PATCH 12/15] refactor: clearer error span for extra endpoints --- codegen/src/next/mod.rs | 7 ++++--- .../ui/next/extra-endpoint-attributes.rs | 3 ++- .../ui/next/extra-endpoint-attributes.stderr | 19 ++++++++----------- 3 files changed, 14 insertions(+), 15 deletions(-) diff --git a/codegen/src/next/mod.rs b/codegen/src/next/mod.rs index 4855bc43d..91b84176e 100644 --- a/codegen/src/next/mod.rs +++ b/codegen/src/next/mod.rs @@ -1,4 +1,4 @@ -use proc_macro_error::{emit_call_site_error, emit_error}; +use proc_macro_error::emit_error; use quote::{quote, ToTokens}; use syn::{ parenthesized, parse::Parse, parse2, punctuated::Punctuated, token::Paren, Expr, File, Ident, @@ -57,8 +57,9 @@ impl Endpoint { if let Some(segment) = item.attrs[index].path.segments.last() { if segment.ident.to_string().as_str() == "endpoint" { if endpoint_index.is_some() { - emit_call_site_error!( - "more than one endpoint attribute provided"; + emit_error!( + item, + "extra endpoint attribute"; hint = "There should only be one endpoint annotation per handler function." ); return None; diff --git a/codegen/tests/ui/next/extra-endpoint-attributes.rs b/codegen/tests/ui/next/extra-endpoint-attributes.rs index 966b36c3b..bafa5b92b 100644 --- a/codegen/tests/ui/next/extra-endpoint-attributes.rs +++ b/codegen/tests/ui/next/extra-endpoint-attributes.rs @@ -3,4 +3,5 @@ shuttle_codegen::app! { #[shuttle_codegen::endpoint(method = post, route = "/hello")] async fn hello() -> &'static str { "Hello, World!" -}} + } +} diff --git a/codegen/tests/ui/next/extra-endpoint-attributes.stderr b/codegen/tests/ui/next/extra-endpoint-attributes.stderr index 5dbe42522..4bcc17574 100644 --- a/codegen/tests/ui/next/extra-endpoint-attributes.stderr +++ b/codegen/tests/ui/next/extra-endpoint-attributes.stderr @@ -1,21 +1,18 @@ -error: more than one endpoint attribute provided +error: extra endpoint attribute = help: There should only be one endpoint annotation per handler function. - --> tests/ui/next/extra-endpoint-attributes.rs:1:1 + --> tests/ui/next/extra-endpoint-attributes.rs:2:5 | -1 | / shuttle_codegen::app! { -2 | | #[shuttle_codegen::endpoint(method = get, route = "/hello")] +2 | / #[shuttle_codegen::endpoint(method = get, route = "/hello")] 3 | | #[shuttle_codegen::endpoint(method = post, route = "/hello")] 4 | | async fn hello() -> &'static str { 5 | | "Hello, World!" -6 | | }} - | |__^ - | - = note: this error originates in the macro `shuttle_codegen::app` (in Nightly builds, run with -Z macro-backtrace for more info) +6 | | } + | |_____^ error[E0601]: `main` function not found in crate `$CRATE` - --> tests/ui/next/extra-endpoint-attributes.rs:6:3 + --> tests/ui/next/extra-endpoint-attributes.rs:7:2 | -6 | }} - | ^ consider adding a `main` function to `$DIR/tests/ui/next/extra-endpoint-attributes.rs` +7 | } + | ^ consider adding a `main` function to `$DIR/tests/ui/next/extra-endpoint-attributes.rs` From 0fb44fcd2f7a12a0969113473099ce2f3a3e431f Mon Sep 17 00:00:00 2001 From: oddgrd <29732646+oddgrd@users.noreply.github.com> Date: Sat, 26 Nov 2022 15:52:53 +0100 Subject: [PATCH 13/15] feat: has_err variable to group param errors --- codegen/src/next/mod.rs | 37 +++++++++++-------- .../ui/next/duplicate-endpoint-param.stderr | 18 +++++++++ 2 files changed, 39 insertions(+), 16 deletions(-) diff --git a/codegen/src/next/mod.rs b/codegen/src/next/mod.rs index 91b84176e..11bf7bcdf 100644 --- a/codegen/src/next/mod.rs +++ b/codegen/src/next/mod.rs @@ -105,6 +105,10 @@ impl Endpoint { return None; } + // At this point an endpoint with params and valid syntax exists, so we will check for + // all errors before returning + let mut has_err = false; + let mut route = None; let mut method = None; @@ -118,7 +122,7 @@ impl Endpoint { "duplicate endpoint method"; hint = "The endpoint `method` should only be set once." ); - return None; + has_err = true; } if let Expr::Path(path) = value { method = Some(path.path.segments[0].ident.clone()); @@ -131,7 +135,7 @@ impl Endpoint { "duplicate endpoint route"; hint = "The endpoint `route` should only be set once." ); - return None; + has_err = true; } if let Expr::Lit(literal) = value { if let Some(Lit::Str(literal)) = Some(literal.lit) { @@ -145,38 +149,39 @@ impl Endpoint { "invalid endpoint argument"; hint = "Only `method` and `route` are valid endpoint arguments." ); - return None; + has_err = true; } } } - let route = if let Some(route) = route { - route - } else { + if route.is_none() { emit_error!( paren.span, "no route provided"; hint = "Add a route to your endpoint: `route = \"/hello\")`" ); - return None; + has_err = true; }; - let method = if let Some(method) = method { - method - } else { + if method.is_none() { emit_error!( paren.span, "no method provided"; hint = "Add a method to your endpoint: `method = get`" ); - return None; + has_err = true; }; - Some(Endpoint { - route, - method, - function, - }) + if has_err { + None + } else { + // Safe to unwrap because `has_err` is true if `route` or `method` is `None` + Some(Endpoint { + route: route.unwrap(), + method: method.unwrap(), + function, + }) + } } } diff --git a/codegen/tests/ui/next/duplicate-endpoint-param.stderr b/codegen/tests/ui/next/duplicate-endpoint-param.stderr index ffaff59b9..25dd13f7e 100644 --- a/codegen/tests/ui/next/duplicate-endpoint-param.stderr +++ b/codegen/tests/ui/next/duplicate-endpoint-param.stderr @@ -7,6 +7,15 @@ error: duplicate endpoint method 2 | #[shuttle_codegen::endpoint(method = get, method = get)] | ^^^^^^ +error: no route provided + + = help: Add a route to your endpoint: `route = "/hello")` + + --> tests/ui/next/duplicate-endpoint-param.rs:2:32 + | +2 | #[shuttle_codegen::endpoint(method = get, method = get)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + error: duplicate endpoint route = help: The endpoint `route` should only be set once. @@ -16,6 +25,15 @@ error: duplicate endpoint route 7 | #[shuttle_codegen::endpoint(route = "/goodbye", route = "/goodbye")] | ^^^^^ +error: no method provided + + = help: Add a method to your endpoint: `method = get` + + --> tests/ui/next/duplicate-endpoint-param.rs:7:32 + | +7 | #[shuttle_codegen::endpoint(route = "/goodbye", route = "/goodbye")] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + error[E0601]: `main` function not found in crate `$CRATE` --> tests/ui/next/duplicate-endpoint-param.rs:11:2 | From 4c1f1ecf6c33625594cf20b62b30e070c3e84a29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oddbj=C3=B8rn=20Gr=C3=B8dem?= <29732646+oddgrd@users.noreply.github.com> Date: Mon, 28 Nov 2022 10:18:16 +0100 Subject: [PATCH 14/15] refactor: remove optional hint Co-authored-by: Pieter --- codegen/src/next/mod.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/codegen/src/next/mod.rs b/codegen/src/next/mod.rs index 11bf7bcdf..78b51f1cc 100644 --- a/codegen/src/next/mod.rs +++ b/codegen/src/next/mod.rs @@ -86,8 +86,7 @@ impl Endpoint { // This will error on invalid parameter syntax emit_error!( err.span(), - err; - hint = "" + err ); return None; } From 32ab17cf55df78a6decb315b7e850be5a5780bff Mon Sep 17 00:00:00 2001 From: oddgrd <29732646+oddgrd@users.noreply.github.com> Date: Sun, 4 Dec 2022 18:00:48 +0100 Subject: [PATCH 15/15] docs: add TODO for multiple endpoint attributes --- codegen/src/next/mod.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/codegen/src/next/mod.rs b/codegen/src/next/mod.rs index 78b51f1cc..27dc3a71f 100644 --- a/codegen/src/next/mod.rs +++ b/codegen/src/next/mod.rs @@ -56,6 +56,9 @@ impl Endpoint { // The endpoint ident should be the last segment in the path if let Some(segment) = item.attrs[index].path.segments.last() { if segment.ident.to_string().as_str() == "endpoint" { + // TODO: we should allow multiple endpoint attributes per handler. + // We could refactor this to return a Vec and then check + // that the combination of endpoints is valid. if endpoint_index.is_some() { emit_error!( item,