From 16abe40d109a2361bfb5313a5f73d1f9170c1348 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oddbj=C3=B8rn=20Gr=C3=B8dem?= <29732646+oddgrd@users.noreply.github.com> Date: Sun, 4 Dec 2022 18:01:29 +0100 Subject: [PATCH] Feat: parse shuttle::endpoint macro (#490) * feat: parse params * feat: parse app * feat: add test for missing endpoint attribute * test: invalid args, params and syntax * feat: check for and test no params * feat: don't emit error for missing annotation this way users can create separate functions for their endpoint logic * fix: remove missing attribute error, not param * refactor: reword comment Co-authored-by: Pieter * feat: duplicate param test, address review * feat: only strip endpoint attributes * feat: check for extra endpoint attributes * refactor: clearer error span for extra endpoints * feat: has_err variable to group param errors * refactor: remove optional hint Co-authored-by: Pieter * docs: add TODO for multiple endpoint attributes Co-authored-by: Pieter --- codegen/src/lib.rs | 16 + codegen/src/main/mod.rs | 2 +- codegen/src/next/mod.rs | 343 +++++++++++++++++- .../tests/ui/next/duplicate-endpoint-param.rs | 11 + .../ui/next/duplicate-endpoint-param.stderr | 41 +++ .../ui/next/extra-endpoint-attributes.rs | 7 + .../ui/next/extra-endpoint-attributes.stderr | 18 + .../tests/ui/next/invalid-endpoint-param.rs | 6 + .../ui/next/invalid-endpoint-param.stderr | 14 + .../tests/ui/next/invalid-endpoint-syntax.rs | 11 + .../ui/next/invalid-endpoint-syntax.stderr | 23 ++ .../tests/ui/next/missing-endpoint-param.rs | 16 + .../ui/next/missing-endpoint-param.stderr | 32 ++ 13 files changed, 536 insertions(+), 4 deletions(-) create mode 100644 codegen/tests/ui/next/duplicate-endpoint-param.rs create mode 100644 codegen/tests/ui/next/duplicate-endpoint-param.stderr create mode 100644 codegen/tests/ui/next/extra-endpoint-attributes.rs create mode 100644 codegen/tests/ui/next/extra-endpoint-attributes.stderr create mode 100644 codegen/tests/ui/next/invalid-endpoint-param.rs create mode 100644 codegen/tests/ui/next/invalid-endpoint-param.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/lib.rs b/codegen/src/lib.rs index c96d2f96c..384e19373 100644 --- a/codegen/src/lib.rs +++ b/codegen/src/lib.rs @@ -1,11 +1,27 @@ 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); + + let app = App::from_file(&mut file); + + 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 f996cfed3..27dc3a71f 100644 --- a/codegen/src/next/mod.rs +++ b/codegen/src/next/mod.rs @@ -1,13 +1,192 @@ use proc_macro_error::emit_error; use quote::{quote, ToTokens}; -use syn::{Ident, LitStr}; +use syn::{ + parenthesized, parse::Parse, parse2, punctuated::Punctuated, token::Paren, Expr, File, Ident, + Item, 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: &mut ItemFn) -> Option { + let function = item.sig.ident.clone(); + + let mut endpoint_index = None; + + // Find the index of an attribute that is an 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" { + // 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, + "extra endpoint attribute"; + hint = "There should only be one endpoint annotation per handler function." + ); + return None; + } + endpoint_index = Some(index); + } + } else { + return None; + } + } + + // 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; + }; + + // 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 + ); + return None; + } + }; + + // We'll use the paren span for errors later + 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; + } + + // 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; + + for Parameter { key, value, .. } in params.params { + 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." + ); + has_err = true; + } + 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." + ); + has_err = true; + } + if let Expr::Lit(literal) = value { + if let Some(Lit::Str(literal)) = Some(literal.lit) { + route = Some(literal); + } + } + } + _ => { + emit_error!( + key_ident, + "invalid endpoint argument"; + hint = "Only `method` and `route` are valid endpoint arguments." + ); + has_err = true; + } + } + } + + if route.is_none() { + emit_error!( + paren.span, + "no route provided"; + hint = "Add a route to your endpoint: `route = \"/hello\")`" + ); + has_err = true; + }; + + if method.is_none() { + emit_error!( + paren.span, + "no method provided"; + hint = "Add a method to your endpoint: `method = get`" + ); + has_err = true; + }; + + 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, + }) + } + } +} + impl ToTokens for Endpoint { fn to_tokens(&self, tokens: &mut proc_macro2::TokenStream) { let Self { @@ -33,10 +212,30 @@ impl ToTokens for Endpoint { } } +#[derive(Debug, Eq, PartialEq)] pub(crate) struct App { endpoints: Vec, } +impl App { + pub(crate) fn from_file(file: &mut File) -> Self { + let endpoints = file + .items + .iter_mut() + .filter_map(|item| { + if let Item::Fn(item_fn) = item { + Some(item_fn) + } else { + None + } + }) + .filter_map(Endpoint::from_item_fn) + .collect(); + + Self { endpoints } + } +} + impl ToTokens for App { fn to_tokens(&self, tokens: &mut proc_macro2::TokenStream) { let Self { endpoints } = self; @@ -62,6 +261,7 @@ impl ToTokens for App { } } +#[allow(dead_code)] pub(crate) fn wasi_bindings(app: App) -> proc_macro2::TokenStream { quote!( #app @@ -133,9 +333,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 +389,141 @@ mod tests { assert_eq!(actual.to_string(), expected.to_string()); } + + #[test] + fn parse_endpoint() { + let cases = 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! { + /// This attribute is not an endpoint so keep it + async fn say_hello() -> &'static str { + "Hello, World!" + } + }, + None, + 1, + ), + ]; + + for (mut input, expected, remaining_attributes) in cases { + let actual = Endpoint::from_item_fn(&mut input); + + assert_eq!(actual, expected); + + // Verify that only endpoint attributes have been stripped + assert_eq!(input.attrs.len(), remaining_attributes); + } + } + + #[test] + fn parse_parameter() { + // test method param + let cases: 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 cases { + 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); + } + + #[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); + 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"); + } } 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..25dd13f7e --- /dev/null +++ b/codegen/tests/ui/next/duplicate-endpoint-param.stderr @@ -0,0 +1,41 @@ +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: 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. + + --> tests/ui/next/duplicate-endpoint-param.rs:7:53 + | +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 + | +11 | } + | ^ consider adding a `main` function to `$DIR/tests/ui/next/duplicate-endpoint-param.rs` 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..bafa5b92b --- /dev/null +++ b/codegen/tests/ui/next/extra-endpoint-attributes.rs @@ -0,0 +1,7 @@ +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..4bcc17574 --- /dev/null +++ b/codegen/tests/ui/next/extra-endpoint-attributes.stderr @@ -0,0 +1,18 @@ +error: extra endpoint attribute + + = help: There should only be one endpoint annotation per handler function. + + --> tests/ui/next/extra-endpoint-attributes.rs:2:5 + | +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 | | } + | |_____^ + +error[E0601]: `main` function not found in crate `$CRATE` + --> tests/ui/next/extra-endpoint-attributes.rs:7:2 + | +7 | } + | ^ consider adding a `main` function to `$DIR/tests/ui/next/extra-endpoint-attributes.rs` diff --git a/codegen/tests/ui/next/invalid-endpoint-param.rs b/codegen/tests/ui/next/invalid-endpoint-param.rs new file mode 100644 index 000000000..e77b39f99 --- /dev/null +++ b/codegen/tests/ui/next/invalid-endpoint-param.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-param.stderr b/codegen/tests/ui/next/invalid-endpoint-param.stderr new file mode 100644 index 000000000..f52aa0858 --- /dev/null +++ b/codegen/tests/ui/next/invalid-endpoint-param.stderr @@ -0,0 +1,14 @@ +error: invalid endpoint argument + + = help: Only `method` and `route` are valid endpoint arguments. + + --> 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-param.rs:6:2 + | +6 | } + | ^ consider adding a `main` function to `$DIR/tests/ui/next/invalid-endpoint-param.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..478bd56aa --- /dev/null +++ b/codegen/tests/ui/next/invalid-endpoint-syntax.stderr @@ -0,0 +1,23 @@ +error: expected `,` + + = help: + + --> tests/ui/next/invalid-endpoint-syntax.rs:2:64 + | +2 | #[shuttle_codegen::endpoint(method = get, route = "/hello" extra = abundant)] + | ^^^^^ + +error: expected `=` + + = help: + + --> 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..a4fa91bf4 --- /dev/null +++ b/codegen/tests/ui/next/missing-endpoint-param.rs @@ -0,0 +1,16 @@ +shuttle_codegen::app! { + #[shuttle_codegen::endpoint(method = get)] + async fn only_method_param() -> &'static str { + "Hello, World!" + } + + #[shuttle_codegen::endpoint(route = "/goodbye")] + async fn only_route_param() -> &'static str { + "Goodbye, World!" + } + + #[shuttle_codegen::endpoint()] + async fn no_params() -> &'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..99150e32f --- /dev/null +++ b/codegen/tests/ui/next/missing-endpoint-param.stderr @@ -0,0 +1,32 @@ +error: no route provided + + = help: Add a route to your endpoint: `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: `method = get` + + --> tests/ui/next/missing-endpoint-param.rs:7:32 + | +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:16:2 + | +16 | } + | ^ consider adding a `main` function to `$DIR/tests/ui/next/missing-endpoint-param.rs`