From b76f7ef778dd37db08cb4dba2dc9329b15bbe984 Mon Sep 17 00:00:00 2001 From: vallentin Date: Sat, 2 Jan 2021 23:02:59 +0100 Subject: Removed implicit borrowing of literals, calls, and more (fixes #404) --- askama_shared/src/filters/json.rs | 6 ++- askama_shared/src/filters/mod.rs | 96 +++++++++++++++++++-------------------- askama_shared/src/filters/yaml.rs | 6 ++- askama_shared/src/generator.rs | 13 ++++-- askama_shared/src/parser.rs | 39 ++++++++++++++++ testing/tests/simple.rs | 8 ++-- 6 files changed, 109 insertions(+), 59 deletions(-) diff --git a/askama_shared/src/filters/json.rs b/askama_shared/src/filters/json.rs index ba7e61b..c0df707 100644 --- a/askama_shared/src/filters/json.rs +++ b/askama_shared/src/filters/json.rs @@ -8,8 +8,8 @@ use serde::Serialize; /// /// This will panic if `S`'s implementation of `Serialize` decides to fail, /// or if `T` contains a map with non-string keys. -pub fn json(e: E, s: &S) -> Result> { - match serde_json::to_string_pretty(s) { +pub fn json(e: E, s: S) -> Result> { + match serde_json::to_string_pretty(&s) { Ok(s) => Ok(MarkupDisplay::new_safe(s, e)), Err(e) => Err(Error::from(e)), } @@ -22,6 +22,8 @@ mod tests { #[test] fn test_json() { + assert_eq!(json(Html, true).unwrap().to_string(), "true"); + assert_eq!(json(Html, "foo").unwrap().to_string(), r#""foo""#); assert_eq!(json(Html, &true).unwrap().to_string(), "true"); assert_eq!(json(Html, &"foo").unwrap().to_string(), r#""foo""#); assert_eq!( diff --git a/askama_shared/src/filters/mod.rs b/askama_shared/src/filters/mod.rs index 35989ca..6838d57 100644 --- a/askama_shared/src/filters/mod.rs +++ b/askama_shared/src/filters/mod.rs @@ -141,7 +141,7 @@ pub fn filesizeformat(b: &B) -> Result { /// To encode `/` as well, see [`urlencode_strict`](./fn.urlencode_strict.html). /// /// [`urlencode_strict`]: ./fn.urlencode_strict.html -pub fn urlencode(s: &dyn fmt::Display) -> Result { +pub fn urlencode(s: T) -> Result { let s = s.to_string(); Ok(utf8_percent_encode(&s, URLENCODE_SET).to_string()) } @@ -161,7 +161,7 @@ pub fn urlencode(s: &dyn fmt::Display) -> Result { /// ``` /// /// If you want to preserve `/`, see [`urlencode`](./fn.urlencode.html). -pub fn urlencode_strict(s: &dyn fmt::Display) -> Result { +pub fn urlencode_strict(s: T) -> Result { let s = s.to_string(); Ok(utf8_percent_encode(&s, URLENCODE_STRICT_SET).to_string()) } @@ -199,7 +199,7 @@ pub fn format() {} /// /// A single newline becomes an HTML line break `
` and a new line /// followed by a blank line becomes a paragraph break `

`. -pub fn linebreaks(s: &dyn fmt::Display) -> Result { +pub fn linebreaks(s: T) -> Result { let s = s.to_string(); let linebroken = s.replace("\n\n", "

").replace("\n", "
"); @@ -207,46 +207,46 @@ pub fn linebreaks(s: &dyn fmt::Display) -> Result { } /// Converts all newlines in a piece of plain text to HTML line breaks -pub fn linebreaksbr(s: &dyn fmt::Display) -> Result { +pub fn linebreaksbr(s: T) -> Result { let s = s.to_string(); Ok(s.replace("\n", "
")) } /// Converts to lowercase -pub fn lower(s: &dyn fmt::Display) -> Result { +pub fn lower(s: T) -> Result { let s = s.to_string(); Ok(s.to_lowercase()) } /// Alias for the `lower()` filter -pub fn lowercase(s: &dyn fmt::Display) -> Result { +pub fn lowercase(s: T) -> Result { lower(s) } /// Converts to uppercase -pub fn upper(s: &dyn fmt::Display) -> Result { +pub fn upper(s: T) -> Result { let s = s.to_string(); Ok(s.to_uppercase()) } /// Alias for the `upper()` filter -pub fn uppercase(s: &dyn fmt::Display) -> Result { +pub fn uppercase(s: T) -> Result { upper(s) } /// Strip leading and trailing whitespace -pub fn trim(s: &dyn fmt::Display) -> Result { +pub fn trim(s: T) -> Result { let s = s.to_string(); Ok(s.trim().to_owned()) } /// Limit string length, appends '...' if truncated -pub fn truncate(s: &dyn fmt::Display, len: &usize) -> Result { +pub fn truncate(s: T, len: usize) -> Result { let mut s = s.to_string(); - if s.len() <= *len { + if s.len() <= len { Ok(s) } else { - let mut real_len = *len; + let mut real_len = len; while !s.is_char_boundary(real_len) { real_len += 1; } @@ -257,7 +257,7 @@ pub fn truncate(s: &dyn fmt::Display, len: &usize) -> Result { } /// Indent lines with `width` spaces -pub fn indent(s: &dyn fmt::Display, width: &usize) -> Result { +pub fn indent(s: T, width: usize) -> Result { let s = s.to_string(); let mut indented = String::new(); @@ -266,7 +266,7 @@ pub fn indent(s: &dyn fmt::Display, width: &usize) -> Result { indented.push(c); if c == '\n' && i < s.len() - 1 { - for _ in 0..*width { + for _ in 0..width { indented.push(' '); } } @@ -277,7 +277,7 @@ pub fn indent(s: &dyn fmt::Display, width: &usize) -> Result { #[cfg(feature = "num-traits")] /// Casts number to f64 -pub fn into_f64(number: &T) -> Result +pub fn into_f64(number: T) -> Result where T: NumCast, { @@ -286,7 +286,7 @@ where #[cfg(feature = "num-traits")] /// Casts number to isize -pub fn into_isize(number: &T) -> Result +pub fn into_isize(number: T) -> Result where T: NumCast, { @@ -325,7 +325,7 @@ where } /// Capitalize a value. The first character will be uppercase, all others lowercase. -pub fn capitalize(s: &dyn fmt::Display) -> Result { +pub fn capitalize(s: T) -> Result { let mut s = s.to_string(); match s.get_mut(0..1).map(|s| { @@ -371,7 +371,7 @@ pub fn center(src: &dyn fmt::Display, dst_len: usize) -> Result { } /// Count the words in that string -pub fn wordcount(s: &dyn fmt::Display) -> Result { +pub fn wordcount(s: T) -> Result { let s = s.to_string(); Ok(s.split_whitespace().count()) @@ -480,36 +480,36 @@ mod tests { #[test] fn test_truncate() { - assert_eq!(truncate(&"hello", &2).unwrap(), "he..."); + assert_eq!(truncate(&"hello", 2).unwrap(), "he..."); let a = String::from("您好"); assert_eq!(a.len(), 6); assert_eq!(String::from("您").len(), 3); - assert_eq!(truncate(&"您好", &1).unwrap(), "您..."); - assert_eq!(truncate(&"您好", &2).unwrap(), "您..."); - assert_eq!(truncate(&"您好", &3).unwrap(), "您..."); - assert_eq!(truncate(&"您好", &4).unwrap(), "您好..."); - assert_eq!(truncate(&"您好", &6).unwrap(), "您好"); - assert_eq!(truncate(&"您好", &7).unwrap(), "您好"); + assert_eq!(truncate(&"您好", 1).unwrap(), "您..."); + assert_eq!(truncate(&"您好", 2).unwrap(), "您..."); + assert_eq!(truncate(&"您好", 3).unwrap(), "您..."); + assert_eq!(truncate(&"您好", 4).unwrap(), "您好..."); + assert_eq!(truncate(&"您好", 6).unwrap(), "您好"); + assert_eq!(truncate(&"您好", 7).unwrap(), "您好"); let s = String::from("🤚a🤚"); assert_eq!(s.len(), 9); assert_eq!(String::from("🤚").len(), 4); - assert_eq!(truncate(&"🤚a🤚", &1).unwrap(), "🤚..."); - assert_eq!(truncate(&"🤚a🤚", &2).unwrap(), "🤚..."); - assert_eq!(truncate(&"🤚a🤚", &3).unwrap(), "🤚..."); - assert_eq!(truncate(&"🤚a🤚", &4).unwrap(), "🤚..."); - assert_eq!(truncate(&"🤚a🤚", &5).unwrap(), "🤚a..."); - assert_eq!(truncate(&"🤚a🤚", &6).unwrap(), "🤚a🤚..."); - assert_eq!(truncate(&"🤚a🤚", &9).unwrap(), "🤚a🤚"); - assert_eq!(truncate(&"🤚a🤚", &10).unwrap(), "🤚a🤚"); + assert_eq!(truncate(&"🤚a🤚", 1).unwrap(), "🤚..."); + assert_eq!(truncate(&"🤚a🤚", 2).unwrap(), "🤚..."); + assert_eq!(truncate(&"🤚a🤚", 3).unwrap(), "🤚..."); + assert_eq!(truncate(&"🤚a🤚", 4).unwrap(), "🤚..."); + assert_eq!(truncate(&"🤚a🤚", 5).unwrap(), "🤚a..."); + assert_eq!(truncate(&"🤚a🤚", 6).unwrap(), "🤚a🤚..."); + assert_eq!(truncate(&"🤚a🤚", 9).unwrap(), "🤚a🤚"); + assert_eq!(truncate(&"🤚a🤚", 10).unwrap(), "🤚a🤚"); } #[test] fn test_indent() { - assert_eq!(indent(&"hello", &2).unwrap(), "hello"); - assert_eq!(indent(&"hello\n", &2).unwrap(), "hello\n"); - assert_eq!(indent(&"hello\nfoo", &2).unwrap(), "hello\n foo"); + assert_eq!(indent(&"hello", 2).unwrap(), "hello"); + assert_eq!(indent(&"hello\n", 2).unwrap(), "hello\n"); + assert_eq!(indent(&"hello\nfoo", 2).unwrap(), "hello\n foo"); assert_eq!( - indent(&"hello\nfoo\n bar", &4).unwrap(), + indent(&"hello\nfoo\n bar", 4).unwrap(), "hello\n foo\n bar" ); } @@ -518,22 +518,22 @@ mod tests { #[test] #[allow(clippy::float_cmp)] fn test_into_f64() { - assert_eq!(into_f64(&1).unwrap(), 1.0_f64); - assert_eq!(into_f64(&1.9).unwrap(), 1.9_f64); - assert_eq!(into_f64(&-1.9).unwrap(), -1.9_f64); - assert_eq!(into_f64(&(INFINITY as f32)).unwrap(), INFINITY); - assert_eq!(into_f64(&(-INFINITY as f32)).unwrap(), -INFINITY); + assert_eq!(into_f64(1).unwrap(), 1.0_f64); + assert_eq!(into_f64(1.9).unwrap(), 1.9_f64); + assert_eq!(into_f64(-1.9).unwrap(), -1.9_f64); + assert_eq!(into_f64(INFINITY as f32).unwrap(), INFINITY); + assert_eq!(into_f64(-INFINITY as f32).unwrap(), -INFINITY); } #[cfg(feature = "num-traits")] #[test] fn test_into_isize() { - assert_eq!(into_isize(&1).unwrap(), 1_isize); - assert_eq!(into_isize(&1.9).unwrap(), 1_isize); - assert_eq!(into_isize(&-1.9).unwrap(), -1_isize); - assert_eq!(into_isize(&(1.5_f64)).unwrap(), 1_isize); - assert_eq!(into_isize(&(-1.5_f64)).unwrap(), -1_isize); - match into_isize(&INFINITY) { + assert_eq!(into_isize(1).unwrap(), 1_isize); + assert_eq!(into_isize(1.9).unwrap(), 1_isize); + assert_eq!(into_isize(-1.9).unwrap(), -1_isize); + assert_eq!(into_isize(1.5_f64).unwrap(), 1_isize); + assert_eq!(into_isize(-1.5_f64).unwrap(), -1_isize); + match into_isize(INFINITY) { Err(Fmt(fmt::Error)) => {} _ => panic!("Should return error of type Err(Fmt(fmt::Error))"), }; diff --git a/askama_shared/src/filters/yaml.rs b/askama_shared/src/filters/yaml.rs index fdd03f6..a6059b7 100644 --- a/askama_shared/src/filters/yaml.rs +++ b/askama_shared/src/filters/yaml.rs @@ -8,8 +8,8 @@ use serde::Serialize; /// /// This will panic if `S`'s implementation of `Serialize` decides to fail, /// or if `T` contains a map with non-string keys. -pub fn yaml(e: E, s: &S) -> Result> { - match serde_yaml::to_string(s) { +pub fn yaml(e: E, s: S) -> Result> { + match serde_yaml::to_string(&s) { Ok(s) => Ok(MarkupDisplay::new_safe(s, e)), Err(e) => Err(Error::from(e)), } @@ -22,6 +22,8 @@ mod tests { #[test] fn test_yaml() { + assert_eq!(yaml(Html, true).unwrap().to_string(), "---\ntrue"); + assert_eq!(yaml(Html, "foo").unwrap().to_string(), "---\nfoo"); assert_eq!(yaml(Html, &true).unwrap().to_string(), "---\ntrue"); assert_eq!(yaml(Html, &"foo").unwrap().to_string(), "---\nfoo"); assert_eq!( diff --git a/askama_shared/src/generator.rs b/askama_shared/src/generator.rs index cde0ead..801db37 100644 --- a/askama_shared/src/generator.rs +++ b/askama_shared/src/generator.rs @@ -1275,9 +1275,12 @@ impl<'a, S: std::hash::BuildHasher> Generator<'a, S> { for (i, arg) in args.iter().enumerate() { if i > 0 { - buf.write(", &"); - } else { - buf.write("&"); + buf.write(", "); + } + + let borrow = !arg.is_copyable(); + if borrow { + buf.write("&("); } let scoped = matches!(arg, @@ -1293,6 +1296,10 @@ impl<'a, S: std::hash::BuildHasher> Generator<'a, S> { } else { self.visit_expr(buf, arg)?; } + + if borrow { + buf.write(")"); + } } Ok(()) } diff --git a/askama_shared/src/parser.rs b/askama_shared/src/parser.rs index 43377a6..3b3d4f4 100644 --- a/askama_shared/src/parser.rs +++ b/askama_shared/src/parser.rs @@ -51,6 +51,45 @@ pub enum Expr<'a> { RustMacro(&'a str, &'a str), } +impl Expr<'_> { + /// Returns `true` if enough assumptions can be made, + /// to determine that `self` is copyable. + pub fn is_copyable(&self) -> bool { + self.is_copyable_within_op(false) + } + + fn is_copyable_within_op(&self, within_op: bool) -> bool { + use Expr::*; + match self { + BoolLit(_) | NumLit(_) | StrLit(_) | CharLit(_) => true, + Unary(.., expr) => expr.is_copyable_within_op(true), + BinOp(_, lhs, rhs) => { + lhs.is_copyable_within_op(true) && rhs.is_copyable_within_op(true) + } + // The result of a call likely doesn't need to be borrowed, + // as in that case the call is more likely to return a + // reference in the first place then. + VarCall(..) | PathCall(..) | MethodCall(..) => true, + // If the `expr` is within a `Unary` or `BinOp` then + // an assumption can be made that the operand is copy. + // If not, then the value is moved and adding `.clone()` + // will solve that issue. However, if the operand is + // implicitly borrowed, then it's likely not even possible + // to get the template to compile. + _ => within_op && self.is_attr_self(), + } + } + + /// Returns `true` if this is an `Attr` where the `obj` is `"self"`. + pub fn is_attr_self(&self) -> bool { + match self { + Expr::Attr(obj, _) if matches!(obj.as_ref(), Expr::Var("self")) => true, + Expr::Attr(obj, _) if matches!(obj.as_ref(), Expr::Attr(..)) => obj.is_attr_self(), + _ => false, + } + } +} + pub type When<'a> = ( WS, Option>, diff --git a/testing/tests/simple.rs b/testing/tests/simple.rs index 5ef03b1..0c6b17e 100644 --- a/testing/tests/simple.rs +++ b/testing/tests/simple.rs @@ -265,7 +265,7 @@ fn test_slice_literal() { #[derive(Template)] #[template(source = "Hello, {{ world(\"123\", 4) }}!", ext = "txt")] struct FunctionRefTemplate { - world: fn(s: &str, v: &u8) -> String, + world: fn(s: &str, v: u8) -> String, } #[test] @@ -277,7 +277,7 @@ fn test_func_ref_call() { } #[allow(clippy::trivially_copy_pass_by_ref)] -fn world2(s: &str, v: &u8) -> String { +fn world2(s: &str, v: u8) -> String { format!("world{}{}", v, s) } @@ -291,7 +291,7 @@ fn test_path_func_call() { } #[derive(Template)] -#[template(source = "{{ ::std::string::ToString::to_string(123) }}", ext = "txt")] +#[template(source = "{{ ::std::string::String::from(\"123\") }}", ext = "txt")] struct RootPathFunctionTemplate; #[test] @@ -305,7 +305,7 @@ struct FunctionTemplate; impl FunctionTemplate { #[allow(clippy::trivially_copy_pass_by_ref)] - fn world3(&self, s: &str, v: &u8) -> String { + fn world3(&self, s: &str, v: u8) -> String { format!("world{}{}", s, v) } } -- cgit