From 17b8ab694c914fa4e7e85ccefbbee375bcdf2fc4 Mon Sep 17 00:00:00 2001 From: Sergio Benitez Date: Thu, 14 Sep 2017 03:02:28 -0700 Subject: [PATCH] Use 'FromUriParam' trait for better ergonomics in 'uri!'. --- codegen/src/lib.rs | 1 + codegen/src/macros/uri.rs | 51 +++++++-- codegen/src/utils/expr_ext.rs | 19 ++++ codegen/src/utils/mod.rs | 8 +- .../tests/compile-fail/typed-uri-bad-type.rs | 8 +- codegen/tests/typed-uris.rs | 103 +++++++++++++----- lib/src/http/uri/from_uri_param.rs | 2 +- 7 files changed, 152 insertions(+), 40 deletions(-) create mode 100644 codegen/src/utils/expr_ext.rs diff --git a/codegen/src/lib.rs b/codegen/src/lib.rs index aefcf134..ba61fead 100644 --- a/codegen/src/lib.rs +++ b/codegen/src/lib.rs @@ -138,6 +138,7 @@ #![crate_type = "dylib"] #![feature(quote, concat_idents, plugin_registrar, rustc_private)] +#![feature(iterator_for_each)] #![feature(custom_attribute)] #![feature(i128_type)] #![allow(unused_attributes)] diff --git a/codegen/src/macros/uri.rs b/codegen/src/macros/uri.rs index 0206d26e..ed9d2d16 100644 --- a/codegen/src/macros/uri.rs +++ b/codegen/src/macros/uri.rs @@ -4,6 +4,7 @@ use std::fmt::Display; use URI_INFO_MACRO_PREFIX; use super::prefix_path; +use utils::{SpanExt, IdentExt, split_idents, ExprExt}; use parser::{UriParams, InternalUriParams, Validation}; @@ -108,8 +109,7 @@ pub fn uri_internal( let mut format_assign_tokens = vec![]; let mut fmt_string = internal.uri_fmt_string(); if let Some(mount_point) = internal.uri_params.mount_point { - // TODO: Should all expressions, not just string literals, be allowed? - // let as_ref = ecx.expr_method_call(span, expr, Ident::from_str("as_ref"), v![]); + // generating: let mount: &str = $mount_string; let mount_string = mount_point.node; argument_stmts.push(ecx.stmt_let_typed( mount_point.span, @@ -119,20 +119,53 @@ pub fn uri_internal( quote_expr!(ecx, $mount_string), )); - format_assign_tokens.push(quote_tokens!(ecx, mount = mount,)); + // generating: format string arg for `mount` + let mut tokens = quote_tokens!(ecx, mount = mount,); + tokens.iter_mut().for_each(|tree| tree.set_span(mount_point.span)); + format_assign_tokens.push(tokens); + + // Ensure the `format!` string contains the `{mount}` parameter. fmt_string = "{mount}".to_string() + &fmt_string; } // Now the user's parameters. + // Building <$T as ::rocket::http::uri::FromUriParam<_>>::from_uri_param($e). for (i, &(ident, ref ty)) in internal.fn_args.iter().enumerate() { - let (span, expr) = (exprs[i].span, exprs[i].clone()); - let into = ecx.expr_method_call(span, expr, Ident::from_str("into"), vec![]); - let stmt = ecx.stmt_let_typed(span, false, ident.node, ty.clone(), into); + let (span, mut expr) = (exprs[i].span, exprs[i].clone()); + // path for call: >::from_uri_param + let idents = split_idents("rocket::http::uri::FromUriParam"); + let generics = vec![ecx.ty(span, ast::TyKind::Infer)]; + let trait_path = ecx.path_all(span, true, idents, vec![], generics, vec![]); + let method = span.wrap(Ident::from_str("from_uri_param")); + let (qself, path) = ecx.qpath(ty.clone(), trait_path, method); + + // replace &expr with [let tmp = expr; &tmp] so that borrows of + // temporary expressions live at least as long as the call to + // `from_uri_param`. Otherwise, exprs like &S { .. } won't compile. + let cloned_expr = expr.clone().unwrap(); + if let ast::ExprKind::AddrOf(_, inner) = cloned_expr.node { + // Only reassign temporary expressions, not locations. + if !inner.is_location() { + let tmp_ident = ident.node.append("_tmp"); + let tmp_stmt = ecx.stmt_let(span, false, tmp_ident, inner); + argument_stmts.push(tmp_stmt); + expr = ecx.expr_ident(span, tmp_ident); + } + } + + // generating: let $ident = path($expr); + let path_expr = ecx.expr_qpath(span, qself, path); + let call = ecx.expr_call(span, path_expr, vec![expr]); + let stmt = ecx.stmt_let(span, false, ident.node, call); + debug!("Emitting URI typecheck statement: {:?}", stmt); argument_stmts.push(stmt); - format_assign_tokens.push(quote_tokens!(ecx, - $ident = &$ident as &::rocket::http::uri::UriDisplay, - )); + + // generating: arg assignment tokens for format string + let uri_display = quote_path!(ecx, ::rocket::http::uri::UriDisplay); + let mut tokens = quote_tokens!(ecx, $ident = &$ident as &$uri_display,); + tokens.iter_mut().for_each(|tree| tree.set_span(span)); + format_assign_tokens.push(tokens); } MacEager::expr(quote_expr!(ecx, { diff --git a/codegen/src/utils/expr_ext.rs b/codegen/src/utils/expr_ext.rs new file mode 100644 index 00000000..0d592598 --- /dev/null +++ b/codegen/src/utils/expr_ext.rs @@ -0,0 +1,19 @@ +use syntax::ast::Expr; +use syntax::ast::ExprKind::*; + +pub trait ExprExt { + fn is_location(&self) -> bool; +} + +impl ExprExt for Expr { + fn is_location(&self) -> bool { + match self.node { + Path(..) => true, + Cast(ref expr, _) => expr.is_location(), + Field(ref expr, _) => expr.is_location(), + TupField(ref expr, _) => expr.is_location(), + Index(ref expr, _) => expr.is_location(), + _ => false + } + } +} diff --git a/codegen/src/utils/mod.rs b/codegen/src/utils/mod.rs index 29e67ae6..51c7001c 100644 --- a/codegen/src/utils/mod.rs +++ b/codegen/src/utils/mod.rs @@ -3,19 +3,21 @@ mod arg_ext; mod parser_ext; mod ident_ext; mod span_ext; +mod expr_ext; pub use self::arg_ext::ArgExt; pub use self::meta_item_ext::MetaItemExt; pub use self::parser_ext::ParserExt; pub use self::ident_ext::IdentExt; pub use self::span_ext::SpanExt; +pub use self::expr_ext::ExprExt; use std::convert::AsRef; use syntax; use syntax::parse::token::Token; use syntax::tokenstream::TokenTree; -use syntax::ast::{Item, Expr}; +use syntax::ast::{Item, Ident, Expr}; use syntax::ext::base::{Annotatable, ExtCtxt}; use syntax::codemap::{Span, Spanned, DUMMY_SP}; use syntax::ext::quote::rt::ToTokens; @@ -129,6 +131,10 @@ pub fn is_valid_ident>(s: S) -> bool { true } +pub fn split_idents(path: &str) -> Vec { + path.split("::").map(|segment| Ident::from_str(segment)).collect() +} + macro_rules! quote_enum { ($ecx:expr, $var:expr => $(::$root:ident)+ { $($variant:ident),+ ; $($extra:pat => $result:expr),* }) => ({ diff --git a/codegen/tests/compile-fail/typed-uri-bad-type.rs b/codegen/tests/compile-fail/typed-uri-bad-type.rs index 1761ea1f..54040b12 100644 --- a/codegen/tests/compile-fail/typed-uri-bad-type.rs +++ b/codegen/tests/compile-fail/typed-uri-bad-type.rs @@ -18,7 +18,6 @@ impl<'a> FromParam<'a> for S { fn simple(id: i32) -> &'static str { "" } #[post("//")] - //~^ ERROR the trait bound `S: rocket::http::uri::UriDisplay` is not satisfied fn not_uri_display(id: i32, name: S) -> &'static str { "" } #[post("//")] @@ -26,10 +25,11 @@ fn not_uri_display_but_unused(id: i32, name: S) -> &'static str { "" } fn main() { uri!(simple: id = "hi"); - //~^ ERROR the trait bound `i32: std::convert::From<&str>` is not satisfied + //~^ ERROR trait bound `i32: rocket::http::uri::FromUriParam<&str>` is not satisfied uri!(simple: "hello"); - //~^ ERROR the trait bound `i32: std::convert::From<&str>` is not satisfied + //~^ ERROR trait bound `i32: rocket::http::uri::FromUriParam<&str>` is not satisfied uri!(simple: id = 239239i64); - //~^ ERROR the trait bound `i32: std::convert::From` is not satisfied + //~^ ERROR trait bound `i32: rocket::http::uri::FromUriParam` is not satisfied uri!(not_uri_display: 10, S); + //~^ ERROR trait bound `S: rocket::http::uri::FromUriParam<_>` is not satisfied } diff --git a/codegen/tests/typed-uris.rs b/codegen/tests/typed-uris.rs index 8dc73a65..461a7afe 100644 --- a/codegen/tests/typed-uris.rs +++ b/codegen/tests/typed-uris.rs @@ -8,7 +8,7 @@ use std::fmt; use std::path::PathBuf; use rocket::http::{RawStr, Cookies}; -use rocket::http::uri::{Uri, UriDisplay}; +use rocket::http::uri::{Uri, UriDisplay, FromUriParam}; use rocket::request::Form; #[derive(FromForm)] @@ -26,8 +26,9 @@ impl<'a> UriDisplay for User<'a> { } } -impl<'a, 'b> From<(&'a str, &'b str)> for User<'a> { - fn from((name, nickname): (&'a str, &'b str)) -> User<'a> { +impl<'a, 'b> FromUriParam<(&'a str, &'b str)> for User<'a> { + type Target = User<'a>; + fn from_uri_param((name, nickname): (&'a str, &'b str)) -> User<'a> { User { name: name.into(), nickname: nickname.to_string() } } } @@ -99,18 +100,16 @@ fn check_simple_unnamed() { uri!(simple2_flipped: 100, "hello".to_string()) => "/100/hello", } - // Ensure that `.into()` is called. + // Ensure that `.from_uri_param()` is called. assert_uri_eq! { - uri!(simple2: 100i8, "hello") => "/100/hello", - uri!(simple2: 100i16, "hi") => "/100/hi", uri!(simple2: 100, "hello") => "/100/hello", uri!(simple2_flipped: 1349, "hey") => "/1349/hey", } // Ensure that the `UriDisplay` trait is being used. assert_uri_eq! { - uri!(simple2: 100i8, "hello there") => "/100/hello%20there", - uri!(simple2_flipped: 100i8, "hello there") => "/100/hello%20there", + uri!(simple2: 100, "hello there") => "/100/hello%20there", + uri!(simple2_flipped: 100, "hello there") => "/100/hello%20there", } } @@ -128,24 +127,22 @@ fn check_simple_named() { uri!(simple2_flipped: name = "hello".to_string(), id = 100) => "/100/hello", } - // Ensure that `.into()` is called. + // Ensure that `.from_uri_param()` is called. assert_uri_eq! { - uri!(simple2: id = 100i8, name = "hello") => "/100/hello", - uri!(simple2: id = 100i16, name = "hi") => "/100/hi", + uri!(simple2: id = 100, name = "hello") => "/100/hello", + uri!(simple2: id = 100, name = "hi") => "/100/hi", uri!(simple2: id = 1349, name = "hey") => "/1349/hey", - uri!(simple2: name = "hello", id = 100i8) => "/100/hello", - uri!(simple2: name = "hi", id = 100i16) => "/100/hi", - uri!(simple2: name = "hey", id = 1349) => "/1349/hey", + uri!(simple2: name = "hello", id = 100) => "/100/hello", + uri!(simple2: name = "hi", id = 100) => "/100/hi", uri!(simple2_flipped: id = 1349, name = "hey") => "/1349/hey", - uri!(simple2_flipped: name = "hello", id = 100i8) => "/100/hello", } // Ensure that the `UriDisplay` trait is being used. assert_uri_eq! { - uri!(simple2: id = 100i8, name = "hello there") => "/100/hello%20there", - uri!(simple2: name = "hello there", id = 100i8) => "/100/hello%20there", - uri!(simple2_flipped: id = 100i8, name = "hello there") => "/100/hello%20there", - uri!(simple2_flipped: name = "hello there", id = 100i8) => "/100/hello%20there", + uri!(simple2: id = 100, name = "hello there") => "/100/hello%20there", + uri!(simple2: name = "hello there", id = 100) => "/100/hello%20there", + uri!(simple2_flipped: id = 100, name = "hello there") => "/100/hello%20there", + uri!(simple2_flipped: name = "hello there", id = 100) => "/100/hello%20there", } } @@ -190,13 +187,23 @@ fn check_with_segments() { uri!("/c", segments: PathBuf::from("one/tw o/")) => "/c/a/one/tw%20o/", uri!("/c", segments: path = PathBuf::from("one/tw o/")) => "/c/a/one/tw%20o/", uri!(segments: PathBuf::from("one/ tw?o/")) => "/a/one/%20tw%3Fo/", - uri!(param_and_segments: 10usize, PathBuf::from("a/b")) => "/a/10/then/a/b", - uri!(param_and_segments: id = 10usize, path = PathBuf::from("a/b")) + uri!(param_and_segments: 10, PathBuf::from("a/b")) => "/a/10/then/a/b", + uri!(param_and_segments: id = 10, path = PathBuf::from("a/b")) => "/a/10/then/a/b", - uri!(guarded_segments: 10usize, PathBuf::from("a/b")) => "/a/10/then/a/b", - uri!(guarded_segments: id = 10usize, path = PathBuf::from("a/b")) + uri!(guarded_segments: 10, PathBuf::from("a/b")) => "/a/10/then/a/b", + uri!(guarded_segments: id = 10, path = PathBuf::from("a/b")) => "/a/10/then/a/b", } + + // Now check the `from_uri_param()` conversions for `PathBuf`. + assert_uri_eq! { + uri!(segments: "one/two/three") => "/a/one/two/three", + uri!("/oh", segments: path = "one/two/three") => "/oh/a/one/two/three", + uri!(segments: "one/ tw?o/") => "/a/one/%20tw%3Fo/", + uri!(param_and_segments: id = 10, path = "a/b") => "/a/10/then/a/b", + uri!(guarded_segments: 10, "a/b") => "/a/10/then/a/b", + uri!(guarded_segments: id = 10, path = "a/b") => "/a/10/then/a/b", + } } #[test] @@ -205,11 +212,13 @@ fn check_complex() { uri!(complex: "no idea", ("A B C", "a c")) => "/no%20idea?name=A+B+C&nickname=a+c", uri!(complex: "Bob", User { name: "Robert".into(), nickname: "Bob".into() }) => "/Bob?name=Robert&nickname=Bob", + uri!(complex: "Bob", &User { name: "Robert".into(), nickname: "Bob".into() }) + => "/Bob?name=Robert&nickname=Bob", uri!(complex: "no idea", User { name: "Robert Mike".into(), nickname: "Bob".into() }) => "/no%20idea?name=Robert+Mike&nickname=Bob", uri!("/some/path", complex: "no idea", ("A B C", "a c")) => "/some/path/no%20idea?name=A+B+C&nickname=a+c", - uri!(complex: name = "Bob", query = User { name: "Robert".into(), nickname: "Bob".into() }) + uri!(complex: name = "Bob", query = &User { name: "Robert".into(), nickname: "Bob".into() }) => "/Bob?name=Robert&nickname=Bob", uri!(complex: query = User { name: "Robert".into(), nickname: "Bob".into() }, name = "Bob") => "/Bob?name=Robert&nickname=Bob", @@ -220,6 +229,50 @@ fn check_complex() { uri!("/hey", complex: name = "no idea", query = ("A B C", "a c")) => "/hey/no%20idea?name=A+B+C&nickname=a+c", } + + // Ensure variables are correctly processed. + let user = User { name: "Robert".into(), nickname: "Bob".into() }; + assert_uri_eq! { + uri!(complex: "complex", &user) => "/complex?name=Robert&nickname=Bob", + uri!(complex: "complex", user) => "/complex?name=Robert&nickname=Bob", + } +} + +#[test] +fn check_location_promotion() { + struct S1(String); + struct S2 { name: String }; + + let s1 = S1("Bob".into()); + let s2 = S2 { name: "Bob".into() }; + + assert_uri_eq! { + uri!(simple2: 1, &S1("A".into()).0) => "/1/A", + uri!(simple2: 1, S1("A".into()).0) => "/1/A", + uri!(simple2: 1, &S2 { name: "A".into() }.name) => "/1/A", + uri!(simple2: 1, S2 { name: "A".into() }.name) => "/1/A", + uri!(simple2: 1, &s1.0) => "/1/Bob", + uri!(simple2: 1, &s2.name) => "/1/Bob", + uri!(simple2: 2, &s1.0) => "/2/Bob", + uri!(simple2: 2, &s2.name) => "/2/Bob", + uri!(simple2: 2, s1.0) => "/2/Bob", + uri!(simple2: 2, s2.name) => "/2/Bob", + } + + let mut s1 = S1("Bob".into()); + let mut s2 = S2 { name: "Bob".into() }; + assert_uri_eq! { + uri!(simple2: 1, &mut S1("A".into()).0) => "/1/A", + uri!(simple2: 1, S1("A".into()).0) => "/1/A", + uri!(simple2: 1, &mut S2 { name: "A".into() }.name) => "/1/A", + uri!(simple2: 1, S2 { name: "A".into() }.name) => "/1/A", + uri!(simple2: 1, &mut s1.0) => "/1/Bob", + uri!(simple2: 1, &mut s2.name) => "/1/Bob", + uri!(simple2: 2, &mut s1.0) => "/2/Bob", + uri!(simple2: 2, &mut s2.name) => "/2/Bob", + uri!(simple2: 2, s1.0) => "/2/Bob", + uri!(simple2: 2, s2.name) => "/2/Bob", + } } #[test] @@ -243,7 +296,7 @@ mod typed_uris { uri!(simple: id = 100) => "/typed_uris/100", uri!(::simple: id = 100) => "/100", uri!("/mount", ::simple: id = 100) => "/mount/100", - uri!(::simple2: id = 100i8, name = "hello") => "/100/hello", + uri!(::simple2: id = 100, name = "hello") => "/100/hello", } } diff --git a/lib/src/http/uri/from_uri_param.rs b/lib/src/http/uri/from_uri_param.rs index 4c0faa3b..da71245f 100644 --- a/lib/src/http/uri/from_uri_param.rs +++ b/lib/src/http/uri/from_uri_param.rs @@ -3,7 +3,7 @@ use std::path::{Path, PathBuf}; use http::RawStr; use http::uri::UriDisplay; -trait FromUriParam: UriDisplay { +pub trait FromUriParam: UriDisplay { type Target: UriDisplay; fn from_uri_param(param: T) -> Self::Target; }