From 5915c69f392cd41fa045581cfa9bffec73f094db Mon Sep 17 00:00:00 2001 From: Sergio Benitez Date: Thu, 8 Sep 2016 17:09:35 -0700 Subject: [PATCH] Fixed unchecked unwrap. Added codegen tests. --- codegen_tests/Cargo.toml | 9 +++++++ codegen_tests/src/main.rs | 3 +++ macros/Cargo.toml | 3 +++ macros/src/decorators/route.rs | 26 ++++++++++++------- macros/tests/compile-fail/ignored_params.rs | 7 +++++ .../compile-fail/phantom-declared-param.rs | 17 ++++++++++++ macros/tests/run-pass/empty-fn.rs | 11 ++++++++ .../tests/run-pass/issue-1-colliding-names.rs | 14 ++++++++++ macros/tests/tests.rs | 20 ++++++++++++++ 9 files changed, 101 insertions(+), 9 deletions(-) create mode 100644 codegen_tests/Cargo.toml create mode 100644 codegen_tests/src/main.rs create mode 100644 macros/tests/compile-fail/ignored_params.rs create mode 100644 macros/tests/compile-fail/phantom-declared-param.rs create mode 100644 macros/tests/run-pass/empty-fn.rs create mode 100644 macros/tests/run-pass/issue-1-colliding-names.rs create mode 100644 macros/tests/tests.rs diff --git a/codegen_tests/Cargo.toml b/codegen_tests/Cargo.toml new file mode 100644 index 00000000..e1800b9f --- /dev/null +++ b/codegen_tests/Cargo.toml @@ -0,0 +1,9 @@ +[package] +name = "codegen_tests" +version = "0.1.0" +authors = ["Sergio Benitez "] + +[dev-dependencies] +compiletest-rs = "*" +rocket = "../lib" +rocket_macros = "../macros" diff --git a/codegen_tests/src/main.rs b/codegen_tests/src/main.rs new file mode 100644 index 00000000..e7a11a96 --- /dev/null +++ b/codegen_tests/src/main.rs @@ -0,0 +1,3 @@ +fn main() { + println!("Hello, world!"); +} diff --git a/macros/Cargo.toml b/macros/Cargo.toml index 0e2ca964..6322a2bb 100644 --- a/macros/Cargo.toml +++ b/macros/Cargo.toml @@ -10,3 +10,6 @@ plugin = true rocket = { path = "../lib/" } log = "*" env_logger = "*" + +[dev-dependencies] +compiletest_rs = "*" diff --git a/macros/src/decorators/route.rs b/macros/src/decorators/route.rs index b7328da8..1adbf1a8 100644 --- a/macros/src/decorators/route.rs +++ b/macros/src/decorators/route.rs @@ -78,6 +78,11 @@ 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 ty = strip_ty_lifetimes(arg.ty.clone()); Some(quote_stmt!(ecx, @@ -147,12 +152,14 @@ impl RouteGenerateExt for RouteParams { // A from_request parameter is one that isn't declared, form, or query. let from_request = |a: &&Arg| { - !declared_set.contains(&*a.name().unwrap()) - && 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) - }) + a.name().map_or(false, |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) + }) + }) }; // Generate the code for `form_request` parameters. @@ -173,9 +180,10 @@ impl RouteGenerateExt for RouteParams { } fn generate_fn_arguments(&self, ecx: &ExtCtxt) -> Vec { - let args = self.annotated_fn.decl().inputs.iter().map(|a| { - a.ident().expect("function decl pat -> ident").prepend(PARAM_PREFIX) - }).collect::>(); + let args = self.annotated_fn.decl().inputs.iter() + .filter_map(|a| a.ident()) + .map(|ident| ident.prepend(PARAM_PREFIX)) + .collect::>(); sep_by_tok(ecx, &args, token::Comma) } diff --git a/macros/tests/compile-fail/ignored_params.rs b/macros/tests/compile-fail/ignored_params.rs new file mode 100644 index 00000000..59b487c4 --- /dev/null +++ b/macros/tests/compile-fail/ignored_params.rs @@ -0,0 +1,7 @@ +#![feature(plugin)] +#![plugin(rocket_macros)] + +#[get("/")] //~ ERROR 'name' is declared +fn get(_: &str) -> &'static str { "hi" } //~ ERROR isn't in the function + +fn main() { } diff --git a/macros/tests/compile-fail/phantom-declared-param.rs b/macros/tests/compile-fail/phantom-declared-param.rs new file mode 100644 index 00000000..8fc9e86d --- /dev/null +++ b/macros/tests/compile-fail/phantom-declared-param.rs @@ -0,0 +1,17 @@ +#![feature(plugin)] +#![plugin(rocket_macros)] + +#[get("/")] //~ ERROR declared +fn get() { } //~ ERROR isn't in the function signature + +#[get("/")] //~ ERROR declared +fn get2() { } //~ ERROR isn't in the function signature + +#[get("/a/b/c//")] + //~^ ERROR 'a' is declared + //~^^ ERROR 'b' is declared +fn get32() { } + //~^ ERROR isn't in the function signature + //~^^ ERROR isn't in the function signature + +fn main() { } diff --git a/macros/tests/run-pass/empty-fn.rs b/macros/tests/run-pass/empty-fn.rs new file mode 100644 index 00000000..ebc399af --- /dev/null +++ b/macros/tests/run-pass/empty-fn.rs @@ -0,0 +1,11 @@ +#![feature(plugin)] +#![plugin(rocket_macros)] + +extern crate rocket; + +#[get("")] +fn get() -> &'static str { "hi" } + +fn main() { + let _ = routes![get]; +} diff --git a/macros/tests/run-pass/issue-1-colliding-names.rs b/macros/tests/run-pass/issue-1-colliding-names.rs new file mode 100644 index 00000000..3dd8db35 --- /dev/null +++ b/macros/tests/run-pass/issue-1-colliding-names.rs @@ -0,0 +1,14 @@ +#![feature(plugin)] +#![plugin(rocket_macros)] + +extern crate rocket; + +#[get("/")] +fn todo(todo: &str) -> &str { + todo +} + +fn main() { + let _ = routes![todo]; +} + diff --git a/macros/tests/tests.rs b/macros/tests/tests.rs new file mode 100644 index 00000000..1a38b218 --- /dev/null +++ b/macros/tests/tests.rs @@ -0,0 +1,20 @@ +extern crate compiletest_rs as compiletest; + +use std::path::PathBuf; + +fn run_mode(mode: &'static str) { + let mut config = compiletest::default_config(); + let cfg_mode = mode.parse().ok().expect("Invalid mode"); + + config.mode = cfg_mode; + config.src_base = PathBuf::from(format!("tests/{}", mode)); + config.target_rustcflags = Some("-L ../target/debug/ -L ../target/debug/deps/".to_owned()); + + compiletest::run_tests(&config); +} + +#[test] +fn compile_test() { + run_mode("compile-fail"); + run_mode("run-pass"); +}