Use proper ident definition. Add param parse tests.

macros/src/decorators/route.rs

@@ -78,12 +78,7 @@ impl RouteGenerateExt for RouteParams {
         }
 
         let arg = arg.unwrap();
-        if arg.ident().is_none() {
+        let name = arg.ident().expect("form param identifier").prepend(PARAM_PREFIX);
-            ecx.span_err(arg.pat.span, "argument names must be identifiers");
-            return None;
-        };
-
-        let name = arg.ident().unwrap().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.

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)]

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<Param> {
+        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('>')) {
+        let (start, end) = match self.string.find('<') {
-            (Some(i), Some(j)) => (i, j),
+            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 err(self.ctxt, self.span, "malformed parameter list");
-            return None;
         }
 
         // 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");
+            err(self.ctxt, param_span, "parameter names cannot be empty")
-            None
+        } else if !is_valid_ident(param) {
-        } else if param.contains(|c: char| !c.is_alphanumeric()) {
+            err(self.ctxt, param_span, "parameter names must be valid identifiers")
-            self.ctxt.span_err(param_span, "parameter names must be alphanumeric");
+        } else if param.starts_with("_") {
-            None
+            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")

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| {
+        self.ident().map(|ident| &ident.name)
-            &ident.name
-        })
     }
 
     fn named(&self, name: &Name) -> bool {

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<Ty>) -> P<Ty> {
     TyLifetimeRemover.fold_ty(ty)
 }
+
+// Lifted from Rust's lexer, except this takes a `char`, not an `Option<char>`.
+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<char>`.
+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: AsRef<str>>(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
+}

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() {  }

macros/tests/compile-fail/ignored_params.rs

@@ -2,6 +2,6 @@
 #![plugin(rocket_macros)]
 
 #[get("/<name>")] //~ 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() {  }

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("/<name><")] //~ ERROR malformed
+fn get(name: &str) -> &'static str { "hi" }
+
+#[get("/<<<<name><")] //~ 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() {  }