Tidy 'uri!' proc-macro. Improve error reporting.

This commit is contained in:
Sergio Benitez 2018-10-04 02:00:04 -07:00
parent 7624aaf3e4
commit 3eb873d89d
6 changed files with 195 additions and 172 deletions

View File

@ -18,7 +18,7 @@ error: `simple` route uri expects 1 parameter but 2 were supplied
--> $DIR/typed-uris-bad-params.rs:20:18
|
20 | uri!(simple: "Hello", 23, );
| ^^^^^^^^^^^
| ^^^^^^^^^^^^
|
= note: expected parameter: id: i32
@ -73,7 +73,7 @@ error: invalid parameters for `simple` route uri
--> $DIR/typed-uris-bad-params.rs:26:18
|
26 | uri!(simple: id = 100, id = 100, );
| ^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^^^^^^
|
= note: uri parameters are: id: i32
help: duplicate parameter: `id`

View File

@ -10,11 +10,11 @@ error: named and unnamed parameters cannot be mixed
12 | uri!(simple: "Hello", id = 100);
| ^^^^^^^^^^^^^^^^^
error: expected one of `::`, `:`, or `<eof>`, found `,`
error: expected `:`
--> $DIR/typed-uris-invalid-syntax.rs:13:16
|
13 | uri!(simple,);
| ^ expected one of `::`, `:`, or `<eof>` here
| ^
error: expected argument list after `:`
--> $DIR/typed-uris-invalid-syntax.rs:14:16
@ -22,45 +22,41 @@ error: expected argument list after `:`
14 | uri!(simple:);
| ^
error: expected `,`, found `<eof>`
error: unexpected end of input: expected ',' followed by route path
--> $DIR/typed-uris-invalid-syntax.rs:15:10
|
15 | uri!("/mount");
| ^^^^^^^^ expected `,`
| ^^^^^^^^
error: expected identifier, found `<eof>`
--> $DIR/typed-uris-invalid-syntax.rs:16:18
error: unexpected end of input, expected identifier
--> $DIR/typed-uris-invalid-syntax.rs:16:5
|
16 | uri!("/mount",);
| ^ expected identifier
| ^^^^^^^^^^^^^^^^
error: invalid mount point
error: invalid mount point; mount points must be static, absolute URIs: `/example`
--> $DIR/typed-uris-invalid-syntax.rs:17:10
|
17 | uri!("mount", simple);
| ^^^^^^^
|
= help: mount points must be static, absolute URIs: `/example`
error: invalid mount point
error: invalid mount point; mount points must be static, absolute URIs: `/example`
--> $DIR/typed-uris-invalid-syntax.rs:18:10
|
18 | uri!("/mount/<id>", simple);
| ^^^^^^^^^^^^^
|
= help: mount points must be static, absolute URIs: `/example`
error: call to `uri!` cannot be empty
error: unexpected end of input, call to `uri!` cannot be empty
--> $DIR/typed-uris-invalid-syntax.rs:19:5
|
19 | uri!();
| ^^^^^^^
error: expected expression, found `<eof>`
--> $DIR/typed-uris-invalid-syntax.rs:20:21
error: unexpected end of input, expected expression
--> $DIR/typed-uris-invalid-syntax.rs:20:5
|
20 | uri!(simple: id = );
| ^ expected expression
| ^^^^^^^^^^^^^^^^^^^^
error: aborting due to 10 previous errors

View File

@ -8,6 +8,12 @@ use syn_ext::{IdentExt, syn_to_diag};
mod uri;
mod uri_parsing;
crate fn prefix_last_segment(path: &mut Path, prefix: &str) {
let mut last_seg = path.segments.last_mut().expect("syn::Path has segments");
last_seg.value_mut().ident = last_seg.value().ident.prepend(prefix);
}
fn _prefixed_vec(prefix: &str, input: TokenStream, ty: &TokenStream2) -> Result<TokenStream2> {
// Parse a comma-separated list of paths.
let mut paths = <Punctuated<Path, Comma>>::parse_terminated
@ -15,10 +21,7 @@ fn _prefixed_vec(prefix: &str, input: TokenStream, ty: &TokenStream2) -> Result<
.map_err(syn_to_diag)?;
// Prefix the last segment in each path with `prefix`.
for path in paths.iter_mut() {
let mut last_seg = path.segments.last_mut().expect("last path segment");
last_seg.value_mut().ident = last_seg.value().ident.prepend(prefix);
}
paths.iter_mut().for_each(|p| prefix_last_segment(p, prefix));
// Return a `vec!` of the prefixed, mapped paths.
let prefixed_mapped_paths = paths.iter()

View File

@ -3,60 +3,43 @@ use proc_macro2::TokenStream as TokenStream2;
use std::fmt::Display;
use derive_utils::{syn, Result};
use quote::ToTokens;
use syn_ext::{IdentExt, syn_to_diag};
use self::syn::{Expr, Ident, Type};
use self::syn::spanned::Spanned as SynSpanned;
use super::uri_parsing::*;
use bang::{prefix_last_segment, uri_parsing::*};
use rocket_http::uri::Origin;
use rocket_http::ext::IntoOwned;
use rocket_http::{uri::Origin, ext::IntoOwned};
const URI_INFO_MACRO_PREFIX: &str = "rocket_uri_for_";
crate fn _uri_macro(input: TokenStream) -> Result<TokenStream> {
let args: TokenStream2 = input.clone().into();
let params = match syn::parse::<UriParams>(input) {
Ok(p) => p,
Err(e) => return Err(syn_to_diag(e)),
};
let mut path = params.route_path;
{
let mut last_seg = path.segments.last_mut().expect("last path segment");
last_seg.value_mut().ident = last_seg.value().ident.prepend(URI_INFO_MACRO_PREFIX);
}
// It's incredibly important we use this span as the Span for the generated
// code so that errors from the `internal` call show up on the user's code.
Ok(quote_spanned!(args.span().into() => {
#path!(#args)
}).into())
}
macro_rules! p {
("parameter", $num:expr) => (
if $num == 1 { "parameter" } else { "parameters" }
(@go $num:expr, $singular:expr, $plural:expr) => (
if $num == 1 { $singular.into() } else { $plural }
);
($num:expr, "was") => (
if $num == 1 { "1 was".into() } else { format!("{} were", $num) }
);
($num:expr, "parameter") => (
if $num == 1 { "1 parameter".into() } else { format!("{} parameters", $num) }
)
("parameter", $n:expr) => (p!(@go $n, "parameter", "parameters"));
($n:expr, "was") => (p!(@go $n, "1 was", format!("{} were", $n)));
($n:expr, "parameter") => (p!(@go $n, "1 parameter", format!("{} parameters", $n)));
}
fn extract_exprs(internal: &InternalUriParams) -> Result<Vec<Expr>> {
crate fn _uri_macro(input: TokenStream) -> Result<TokenStream> {
let input2: TokenStream2 = input.clone().into();
let mut params = syn::parse::<UriParams>(input).map_err(syn_to_diag)?;
prefix_last_segment(&mut params.route_path, URI_INFO_MACRO_PREFIX);
let path = &params.route_path;
Ok(quote!(#path!(#input2)).into())
}
fn extract_exprs(internal: &InternalUriParams) -> Result<Vec<&Expr>> {
let route_name = &internal.uri_params.route_path;
match internal.validate() {
Validation::Ok(exprs) => Ok(exprs),
Validation::Unnamed(expected, actual) => {
let mut diag = internal.uri_params.args_span().error(
format!("`{}` route uri expects {} but {} supplied",
route_name.clone().into_token_stream(), p!(expected, "parameter"), p!(actual, "was")));
format!("`{}` route uri expects {} but {} supplied", quote!(#route_name),
p!(expected, "parameter"), p!(actual, "was")));
if expected > 0 {
let ps = p!("parameter", expected);
@ -66,9 +49,9 @@ fn extract_exprs(internal: &InternalUriParams) -> Result<Vec<Expr>> {
Err(diag)
}
Validation::Named(missing, extra, dup) => {
let e = format!("invalid parameters for `{}` route uri", route_name.clone().into_token_stream());
let mut diag = internal.uri_params.args_span().error(e);
diag = diag.note(format!("uri parameters are: {}", internal.fn_args_str()));
let e = format!("invalid parameters for `{}` route uri", quote!(#route_name));
let mut diag = internal.uri_params.args_span().error(e)
.note(format!("uri parameters are: {}", internal.fn_args_str()));
fn join<S: Display, T: Iterator<Item = S>>(iter: T) -> (&'static str, String) {
let items: Vec<_> = iter.map(|i| format!("`{}`", i)).collect();
@ -114,8 +97,8 @@ fn extract_origin(internal: &InternalUriParams) -> Result<Origin<'static>> {
.map_err(|_| internal.uri.span().unstable().error("invalid route URI"))
}
fn explode<I>(route_str: &str, items: I) -> TokenStream2
where I: Iterator<Item = (Ident, Type, Expr)>
fn explode<'a, I>(route_str: &str, items: I) -> TokenStream2
where I: Iterator<Item = (&'a Ident, &'a Type, &'a Expr)>
{
// Generate the statements to typecheck each parameter.
// Building <$T as ::rocket::http::uri::FromUriParam<_>>::from_uri_param($e).
@ -123,16 +106,18 @@ fn explode<I>(route_str: &str, items: I) -> TokenStream2
let mut fmt_exprs = vec![];
for (mut ident, ty, expr) in items {
let (span, mut expr) = (expr.span(), expr.clone());
ident.set_span(span);
let ident_tmp = ident.prepend("tmp");
let (span, expr) = (expr.span(), expr);
let ident_tmp = ident.prepend("tmp_");
let_bindings.push(quote_spanned!(span =>
let #ident_tmp = #expr; let #ident = <#ty as ::rocket::http::uri::FromUriParam<_>>::from_uri_param(#ident_tmp);
let #ident_tmp = #expr;
let #ident = <#ty as ::rocket::http::uri::FromUriParam<_>>::from_uri_param(#ident_tmp);
));
// generating: arg tokens for format string
fmt_exprs.push(quote_spanned!(span => { &#ident as &::rocket::http::uri::UriDisplay }));
fmt_exprs.push(quote_spanned! { span =>
&#ident as &::rocket::http::uri::UriDisplay
});
}
// Convert all of the '<...>' into '{}'.
@ -164,13 +149,15 @@ crate fn _uri_internal_macro(input: TokenStream) -> Result<TokenStream> {
let path_param_count = origin.path().matches('<').count();
// Create an iterator over the `ident`, `ty`, and `expr` triple.
let mut arguments = internal.fn_args
.into_iter()
.zip(exprs.into_iter())
.map(|(FnArg { ident, ty }, expr)| (ident, ty, expr));
let mut arguments = internal.fn_args.iter()
.zip(exprs.iter())
.map(|(FnArg { ident, ty }, &expr)| (ident, ty, expr));
// Generate an expression for both the path and query.
let path = explode(origin.path(), arguments.by_ref().take(path_param_count));
// FIXME: Use Optional.
// let query = Optional(origin.query().map(|q| explode(q, arguments)));
let query = if let Some(expr) = origin.query().map(|q| explode(q, arguments)) {
quote!({ Some(#expr) })
} else {

View File

@ -1,26 +1,26 @@
use proc_macro::Span;
use derive_utils::syn;
use derive_utils::{syn, Spanned};
use derive_utils::proc_macro2::TokenStream as TokenStream2;
use derive_utils::ext::TypeExt;
use quote::ToTokens;
use self::syn::{Expr, Ident, LitStr, Path, Token, Type};
use self::syn::spanned::Spanned as SynSpanned;
use self::syn::parse::{self, Parse, ParseStream};
use self::syn::punctuated::Punctuated;
use indexmap::IndexMap;
#[derive(Debug)]
enum Arg {
pub enum Arg {
Unnamed(Expr),
Named(Ident, Expr),
Named(Ident, Token![=], Expr),
}
#[derive(Debug)]
pub enum Args {
Unnamed(Vec<Expr>),
Named(Vec<(Ident, Expr)>),
Unnamed(Punctuated<Arg, Token![,]>),
Named(Punctuated<Arg, Token![,]>),
}
// For an invocation that looks like:
@ -28,12 +28,11 @@ pub enum Args {
// ^-------------| ^----------| ^---------|
// uri_params.mount_point | uri_params.arguments
// uri_params.route_path
//
#[derive(Debug)]
pub struct UriParams {
pub mount_point: Option<LitStr>,
pub route_path: Path,
pub arguments: Option<Args>,
pub arguments: Args,
}
#[derive(Debug)]
@ -42,18 +41,29 @@ pub struct FnArg {
pub ty: Type,
}
pub enum Validation {
pub enum Validation<'a> {
// Number expected, what we actually got.
Unnamed(usize, usize),
// (Missing, Extra, Duplicate)
Named(Vec<Ident>, Vec<Ident>, Vec<Ident>),
// Everything is okay.
Ok(Vec<Expr>)
Named(Vec<&'a Ident>, Vec<&'a Ident>, Vec<&'a Ident>),
// Everything is okay; here are the expressions in the route decl order.
Ok(Vec<&'a Expr>)
}
// This is invoked by Rocket itself. The `uri!` macro expands to a call to a
// route-specific macro which in-turn expands to a call to `internal_uri!`,
// passing along the user's parameters from the original `uri!` call. This is
// necessary so that we can converge the type information in the route (from the
// route-specific macro) with the user's parameters (by forwarding them to the
// internal_uri! call).
//
// `fn_args` are the URI arguments (excluding guards) from the original route's
// handler in the order they were declared in the URI (`<first>/<second>`).
// `uri` is the full URI used in the origin route's attribute
// `uri` is the full URI used in the origin route's attribute.
//
// internal_uri!("/<first>/<second>", (first: ty, second: ty), $($tt)*);
// ^-----------------| ^-----------|---------| ^-----|
// uri fn_args uri_params
#[derive(Debug)]
pub struct InternalUriParams {
pub uri: String,
@ -61,63 +71,14 @@ pub struct InternalUriParams {
pub uri_params: UriParams,
}
impl Arg {
fn is_named(&self) -> bool {
match *self {
Arg::Named(..) => true,
Arg::Unnamed(_) => false,
}
}
fn unnamed(self) -> Expr {
match self {
Arg::Unnamed(expr) => expr,
_ => panic!("Called Arg::unnamed() on an Arg::named!"),
}
}
fn named(self) -> (Ident, Expr) {
match self {
Arg::Named(ident, expr) => (ident, expr),
_ => panic!("Called Arg::named() on an Arg::Unnamed!"),
}
}
}
impl UriParams {
/// The Span to use when referring to all of the arguments.
pub fn args_span(&self) -> Span {
match self.arguments {
Some(ref args) => {
let (first, last) = match args {
Args::Unnamed(ref exprs) => {
(
exprs.first().unwrap().span().unstable(),
exprs.last().unwrap().span().unstable()
)
},
Args::Named(ref pairs) => {
(
pairs.first().unwrap().0.span().unstable(),
pairs.last().unwrap().1.span().unstable()
)
},
};
first.join(last).expect("join spans")
},
None => self.route_path.span().unstable(),
}
}
}
impl Parse for Arg {
fn parse(input: ParseStream) -> parse::Result<Self> {
let has_key = input.peek2(Token![=]);
if has_key {
let ident = input.parse::<Ident>()?;
input.parse::<Token![=]>()?;
let eq_token = input.parse::<Token![=]>()?;
let expr = input.parse::<Expr>()?;
Ok(Arg::Named(ident, expr))
Ok(Arg::Named(ident, eq_token, expr))
} else {
let expr = input.parse::<Expr>()?;
Ok(Arg::Unnamed(expr))
@ -125,6 +86,10 @@ impl Parse for Arg {
}
}
fn err<T, S: AsRef<str>>(span: Span, s: S) -> parse::Result<T> {
Err(parse::Error::new(span.into(), s.as_ref()))
}
impl Parse for UriParams {
// Parses the mount point, if any, route identifier, and arguments.
fn parse(input: ParseStream) -> parse::Result<Self> {
@ -137,8 +102,16 @@ impl Parse for UriParams {
let string = input.parse::<LitStr>()?;
let value = string.value();
if value.contains('<') || !value.starts_with('/') {
return Err(parse::Error::new(string.span(), "invalid mount point; mount points must be static, absolute URIs: `/example`"));
// TODO(proc_macro): add example as a help, not in error
return err(string.span().unstable(), "invalid mount point; \
mount points must be static, absolute URIs: `/example`");
}
if !input.peek(Token![,]) && input.cursor().eof() {
return err(string.span().unstable(), "unexpected end of input: \
expected ',' followed by route path");
}
input.parse::<Token![,]>()?;
Some(string)
} else {
@ -149,20 +122,18 @@ impl Parse for UriParams {
let route_path = input.parse::<Path>()?;
// If there are no arguments, finish early.
if !input.peek(Token![:]) {
let arguments = None;
if !input.peek(Token![:]) && input.cursor().eof() {
let arguments = Args::Unnamed(Punctuated::new());
return Ok(Self { mount_point, route_path, arguments });
}
let colon = input.parse::<Token![:]>()?;
// Parse arguments
let args_start = input.cursor();
let colon = input.parse::<Token![:]>()?;
let arguments: Punctuated<Arg, Token![,]> = input.parse_terminated(Arg::parse)?;
// A 'colon' was used but there are no arguments.
if arguments.is_empty() {
return Err(parse::Error::new(colon.span(), "expected argument list after `:`"));
return err(colon.span(), "expected argument list after `:`");
}
// Ensure that both types of arguments were not used at once.
@ -175,18 +146,15 @@ impl Parse for UriParams {
}
if !homogeneous_args {
// TODO: This error isn't showing up with the right span.
return Err(parse::Error::new(args_start.token_stream().span(), "named and unnamed parameters cannot be mixed"));
return err(arguments.span(), "named and unnamed parameters cannot be mixed");
}
// Create the `Args` enum, which properly types one-kind-of-argument-ness.
let args = if prev_named.unwrap() {
Args::Named(arguments.into_iter().map(|arg| arg.named()).collect())
} else {
Args::Unnamed(arguments.into_iter().map(|arg| arg.unnamed()).collect())
// Create the `Args` enum, which properly record one-kind-of-argument-ness.
let arguments = match prev_named {
Some(true) => Args::Named(arguments),
_ => Args::Unnamed(arguments)
};
let arguments = Some(args);
Ok(Self { mount_point, route_path, arguments })
}
}
@ -204,7 +172,6 @@ impl Parse for FnArg {
impl Parse for InternalUriParams {
fn parse(input: ParseStream) -> parse::Result<InternalUriParams> {
let uri = input.parse::<LitStr>()?.value();
//let uri = parser.prev_span.wrap(uri_str);
input.parse::<Token![,]>()?;
let content;
@ -221,32 +188,30 @@ impl Parse for InternalUriParams {
impl InternalUriParams {
pub fn fn_args_str(&self) -> String {
self.fn_args.iter()
.map(|&FnArg { ref ident, ref ty }| format!("{}: {}", ident, ty.clone().into_token_stream().to_string().trim()))
.map(|FnArg { ident, ty }| format!("{}: {}", ident, quote!(#ty).to_string().trim()))
.collect::<Vec<_>>()
.join(", ")
}
pub fn validate(&self) -> Validation {
let unnamed = |args: &Vec<Expr>| -> Validation {
let (expected, actual) = (self.fn_args.len(), args.len());
let args = &self.uri_params.arguments;
match args {
Args::Unnamed(inner) => {
let (expected, actual) = (self.fn_args.len(), inner.len());
if expected != actual { Validation::Unnamed(expected, actual) }
else { Validation::Ok(args.clone()) }
};
match self.uri_params.arguments {
None => unnamed(&vec![]),
Some(Args::Unnamed(ref args)) => unnamed(args),
Some(Args::Named(ref args)) => {
let mut params: IndexMap<Ident, Option<Expr>> = self.fn_args.iter()
.map(|&FnArg { ref ident, .. }| (ident.clone(), None))
else { Validation::Ok(args.unnamed().unwrap().collect()) }
},
Args::Named(_) => {
let mut params: IndexMap<&Ident, Option<&Expr>> = self.fn_args.iter()
.map(|FnArg { ident, .. }| (ident, None))
.collect();
let (mut extra, mut dup) = (vec![], vec![]);
for &(ref ident, ref expr) in args {
for (ident, expr) in args.named().unwrap() {
match params.get_mut(ident) {
Some(ref entry) if entry.is_some() => dup.push(ident.clone()),
Some(entry) => *entry = Some(expr.clone()),
None => extra.push(ident.clone()),
Some(ref entry) if entry.is_some() => dup.push(ident),
Some(entry) => *entry = Some(expr),
None => extra.push(ident),
}
}
@ -268,3 +233,75 @@ impl InternalUriParams {
}
}
impl UriParams {
/// The Span to use when referring to all of the arguments.
pub fn args_span(&self) -> Span {
match self.arguments.num() {
0 => self.route_path.span(),
_ => self.arguments.span()
}
}
}
impl Arg {
fn is_named(&self) -> bool {
match *self {
Arg::Named(..) => true,
Arg::Unnamed(_) => false,
}
}
fn unnamed(&self) -> &Expr {
match self {
Arg::Unnamed(expr) => expr,
_ => panic!("Called Arg::unnamed() on an Arg::named!"),
}
}
fn named(&self) -> (&Ident, &Expr) {
match self {
Arg::Named(ident, _, expr) => (ident, expr),
_ => panic!("Called Arg::named() on an Arg::Unnamed!"),
}
}
}
impl Args {
fn num(&self) -> usize {
match self {
Args::Named(inner) | Args::Unnamed(inner) => inner.len(),
}
}
fn named(&self) -> Option<impl Iterator<Item = (&Ident, &Expr)>> {
match self {
Args::Named(args) => Some(args.iter().map(|arg| arg.named())),
_ => None
}
}
fn unnamed(&self) -> Option<impl Iterator<Item = &Expr>> {
match self {
Args::Unnamed(args) => Some(args.iter().map(|arg| arg.unnamed())),
_ => None
}
}
}
impl ToTokens for Arg {
fn to_tokens(&self, tokens: &mut TokenStream2) {
match self {
Arg::Unnamed(e) => e.to_tokens(tokens),
Arg::Named(ident, eq, expr) => tokens.extend(quote!(#ident #eq #expr))
}
}
}
impl ToTokens for Args {
fn to_tokens(&self, tokens: &mut TokenStream2) {
match self {
Args::Unnamed(e) | Args::Named(e) => e.to_tokens(tokens)
}
}
}

View File

@ -105,9 +105,9 @@ use self::priv_encode_set::PATH_ENCODE_SET;
/// dynamic parameter type.
///
/// ```rust
/// # #![feature(plugin, decl_macro)]
/// # #![feature(plugin, proc_macro_non_items, proc_macro_gen, decl_macro)]
/// # #![plugin(rocket_codegen)]
/// # extern crate rocket;
/// # #[macro_use] extern crate rocket;
/// # fn main() { }
/// use rocket::http::RawStr;
/// use rocket::request::FromParam;