From f14dec0728743e2d137761522e6a4aff62fc8e8a Mon Sep 17 00:00:00 2001 From: Sergio Benitez Date: Thu, 8 Sep 2016 20:25:07 -0700 Subject: [PATCH] Use proper ident definition. Add param parse tests. --- macros/src/decorators/route.rs | 14 ++++---- macros/src/lib.rs | 2 +- macros/src/parser/param.rs | 27 +++++++++------ macros/src/utils/arg_ext.rs | 4 +-- macros/src/utils/mod.rs | 33 +++++++++++++++++++ .../tests/compile-fail/bad-ident-argument.rs | 9 +++++ macros/tests/compile-fail/ignored_params.rs | 2 +- .../compile-fail/malformed-param-list.rs | 25 ++++++++++++++ 8 files changed, 93 insertions(+), 23 deletions(-) create mode 100644 macros/tests/compile-fail/bad-ident-argument.rs create mode 100644 macros/tests/compile-fail/malformed-param-list.rs diff --git a/macros/src/decorators/route.rs b/macros/src/decorators/route.rs index 1adbf1a8..9a90224e 100644 --- a/macros/src/decorators/route.rs +++ b/macros/src/decorators/route.rs @@ -78,12 +78,7 @@ impl RouteGenerateExt for RouteParams { } let arg = arg.unwrap(); - if arg.ident().is_none() { - ecx.span_err(arg.pat.span, "argument names must be identifiers"); - return None; - }; - - let name = arg.ident().unwrap().prepend(PARAM_PREFIX); + let name = arg.ident().expect("form param identifier").prepend(PARAM_PREFIX); let ty = strip_ty_lifetimes(arg.ty.clone()); Some(quote_stmt!(ecx, let $name: $ty = @@ -152,14 +147,17 @@ impl RouteGenerateExt for RouteParams { // A from_request parameter is one that isn't declared, form, or query. let from_request = |a: &&Arg| { - a.name().map_or(false, |name| { + if let Some(name) = a.name() { !declared_set.contains(name) && self.form_param.as_ref().map_or(true, |p| { !a.named(&p.value().name) }) && self.query_param.as_ref().map_or(true, |p| { !a.named(&p.node.name) }) - }) + } else { + ecx.span_err(a.pat.span, "argument names must be identifiers"); + false + } }; // Generate the code for `form_request` parameters. diff --git a/macros/src/lib.rs b/macros/src/lib.rs index e573e8a1..d6dca3a6 100644 --- a/macros/src/lib.rs +++ b/macros/src/lib.rs @@ -1,5 +1,5 @@ #![crate_type = "dylib"] -#![feature(quote, concat_idents, plugin_registrar, rustc_private)] +#![feature(quote, concat_idents, plugin_registrar, rustc_private, unicode)] #![feature(custom_attribute)] #![feature(dotdot_in_tuple_patterns)] #![allow(unused_attributes)] diff --git a/macros/src/parser/param.rs b/macros/src/parser/param.rs index 59092bc1..ebc7342d 100644 --- a/macros/src/parser/param.rs +++ b/macros/src/parser/param.rs @@ -3,7 +3,7 @@ use syntax::ext::base::ExtCtxt; use syntax::codemap::{Span, Spanned, BytePos}; use syntax::parse::token::str_to_ident; -use utils::{span, SpanExt}; +use utils::{span, SpanExt, is_valid_ident}; #[derive(Debug)] pub enum Param { @@ -45,16 +45,23 @@ impl<'s, 'a, 'c> Iterator for ParamIter<'s, 'a, 'c> { type Item = Param; fn next(&mut self) -> Option { + let err = |ecx: &ExtCtxt, sp: Span, msg: &str| { + ecx.span_err(sp, msg); + return None; + }; + // Find the start and end indexes for the next parameter, if any. - let (start, end) = match (self.string.find('<'), self.string.find('>')) { - (Some(i), Some(j)) => (i, j), + let (start, end) = match self.string.find('<') { + Some(i) => match self.string.find('>') { + Some(j) => (i, j), + None => return err(self.ctxt, self.span, "malformed parameter list") + }, _ => return None, }; // Ensure we found a valid parameter. if end <= start { - self.ctxt.span_err(self.span, "Parameter list is malformed."); - return None; + return err(self.ctxt, self.span, "malformed parameter list"); } // Calculate the parameter's ident. @@ -74,11 +81,11 @@ impl<'s, 'a, 'c> Iterator for ParamIter<'s, 'a, 'c> { // Check for nonemptiness, that the characters are correct, and return. if param.is_empty() { - self.ctxt.span_err(param_span, "parameter names cannot be empty"); - None - } else if param.contains(|c: char| !c.is_alphanumeric()) { - self.ctxt.span_err(param_span, "parameter names must be alphanumeric"); - None + err(self.ctxt, param_span, "parameter names cannot be empty") + } else if !is_valid_ident(param) { + err(self.ctxt, param_span, "parameter names must be valid identifiers") + } else if param.starts_with("_") { + err(self.ctxt, param_span, "parameters cannot be ignored") } else if is_many && !self.string.is_empty() { let sp = self.span.shorten_to(self.string.len() as u32); self.ctxt.struct_span_err(sp, "text after a trailing '..' param") diff --git a/macros/src/utils/arg_ext.rs b/macros/src/utils/arg_ext.rs index d3277a45..6ff0fce1 100644 --- a/macros/src/utils/arg_ext.rs +++ b/macros/src/utils/arg_ext.rs @@ -4,9 +4,7 @@ pub trait ArgExt { fn ident(&self) -> Option<&Ident>; fn name(&self) -> Option<&Name> { - self.ident().map(|ident| { - &ident.name - }) + self.ident().map(|ident| &ident.name) } fn named(&self, name: &Name) -> bool { diff --git a/macros/src/utils/mod.rs b/macros/src/utils/mod.rs index b6ba0318..b89c3bd2 100644 --- a/macros/src/utils/mod.rs +++ b/macros/src/utils/mod.rs @@ -10,6 +10,8 @@ pub use self::parser_ext::ParserExt; pub use self::ident_ext::IdentExt; pub use self::span_ext::SpanExt; +use std::convert::AsRef; + use syntax::parse::token::Token; use syntax::tokenstream::TokenTree; use syntax::ast::{Item, Expr}; @@ -90,3 +92,34 @@ impl Folder for TyLifetimeRemover { pub fn strip_ty_lifetimes(ty: P) -> P { TyLifetimeRemover.fold_ty(ty) } + +// Lifted from Rust's lexer, except this takes a `char`, not an `Option`. +fn ident_start(c: char) -> bool { + (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || c == '_' || + (c > '\x7f' && c.is_xid_start()) +} + +// Lifted from Rust's lexer, except this takes a `char`, not an `Option`. +fn ident_continue(c: char) -> bool { + (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9') || + c == '_' || (c > '\x7f' && c.is_xid_continue()) +} + +pub fn is_valid_ident>(s: S) -> bool { + let string = s.as_ref(); + if string.is_empty() { + return false; + } + + for (i, c) in string.chars().enumerate() { + if i == 0 { + if !ident_start(c) { + return false; + } + } else if !ident_continue(c) { + return false; + } + } + + true +} diff --git a/macros/tests/compile-fail/bad-ident-argument.rs b/macros/tests/compile-fail/bad-ident-argument.rs new file mode 100644 index 00000000..8dd3a19e --- /dev/null +++ b/macros/tests/compile-fail/bad-ident-argument.rs @@ -0,0 +1,9 @@ +#![feature(plugin)] +#![plugin(rocket_macros)] + +extern crate rocket; + +#[get("/")] +fn get(_: &str) -> &'static str { "hi" } //~ ERROR argument + +fn main() { } diff --git a/macros/tests/compile-fail/ignored_params.rs b/macros/tests/compile-fail/ignored_params.rs index 59b487c4..946106dc 100644 --- a/macros/tests/compile-fail/ignored_params.rs +++ b/macros/tests/compile-fail/ignored_params.rs @@ -2,6 +2,6 @@ #![plugin(rocket_macros)] #[get("/")] //~ ERROR 'name' is declared -fn get(_: &str) -> &'static str { "hi" } //~ ERROR isn't in the function +fn get(other: &str) -> &'static str { "hi" } //~ ERROR isn't in the function fn main() { } diff --git a/macros/tests/compile-fail/malformed-param-list.rs b/macros/tests/compile-fail/malformed-param-list.rs new file mode 100644 index 00000000..735fed22 --- /dev/null +++ b/macros/tests/compile-fail/malformed-param-list.rs @@ -0,0 +1,25 @@ +#![feature(plugin)] +#![plugin(rocket_macros)] + +#[get("/><")] //~ ERROR malformed +fn get() -> &'static str { "hi" } + +#[get("/<")] //~ ERROR malformed +fn get(name: &str) -> &'static str { "hi" } + +#[get("/<<<<")] //~ ERROR identifiers +fn get(name: &str) -> &'static str { "hi" } + +#[get("/")] //~ ERROR identifiers +fn get() -> &'static str { "hi" } + +#[get("/<_>")] //~ ERROR ignored +fn get() -> &'static str { "hi" } + +#[get("/<1>")] //~ ERROR identifiers +fn get() -> &'static str { "hi" } + +#[get("/<>name><")] //~ ERROR cannot be empty +fn get() -> &'static str { "hi" } + +fn main() { }