Skip to content

Commit

Permalink
Removed implicit borrowing of literals, calls, and more (fixes #404)
Browse files Browse the repository at this point in the history
  • Loading branch information
vallentin authored and djc committed Jan 5, 2021
1 parent c2102d4 commit b76f7ef
Show file tree
Hide file tree
Showing 6 changed files with 109 additions and 59 deletions.
6 changes: 4 additions & 2 deletions askama_shared/src/filters/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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: Escaper, S: Serialize>(e: E, s: &S) -> Result<MarkupDisplay<E, String>> {
match serde_json::to_string_pretty(s) {
pub fn json<E: Escaper, S: Serialize>(e: E, s: S) -> Result<MarkupDisplay<E, String>> {
match serde_json::to_string_pretty(&s) {
Ok(s) => Ok(MarkupDisplay::new_safe(s, e)),
Err(e) => Err(Error::from(e)),
}
Expand All @@ -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!(
Expand Down
96 changes: 48 additions & 48 deletions askama_shared/src/filters/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ pub fn filesizeformat<B: FileSize>(b: &B) -> Result<String> {
/// 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<String> {
pub fn urlencode<T: fmt::Display>(s: T) -> Result<String> {
let s = s.to_string();
Ok(utf8_percent_encode(&s, URLENCODE_SET).to_string())
}
Expand All @@ -161,7 +161,7 @@ pub fn urlencode(s: &dyn fmt::Display) -> Result<String> {
/// ```
///
/// If you want to preserve `/`, see [`urlencode`](./fn.urlencode.html).
pub fn urlencode_strict(s: &dyn fmt::Display) -> Result<String> {
pub fn urlencode_strict<T: fmt::Display>(s: T) -> Result<String> {
let s = s.to_string();
Ok(utf8_percent_encode(&s, URLENCODE_STRICT_SET).to_string())
}
Expand Down Expand Up @@ -199,54 +199,54 @@ pub fn format() {}
///
/// A single newline becomes an HTML line break `<br>` and a new line
/// followed by a blank line becomes a paragraph break `<p>`.
pub fn linebreaks(s: &dyn fmt::Display) -> Result<String> {
pub fn linebreaks<T: fmt::Display>(s: T) -> Result<String> {
let s = s.to_string();
let linebroken = s.replace("\n\n", "</p><p>").replace("\n", "<br/>");

Ok(format!("<p>{}</p>", linebroken))
}

/// Converts all newlines in a piece of plain text to HTML line breaks
pub fn linebreaksbr(s: &dyn fmt::Display) -> Result<String> {
pub fn linebreaksbr<T: fmt::Display>(s: T) -> Result<String> {
let s = s.to_string();
Ok(s.replace("\n", "<br/>"))
}

/// Converts to lowercase
pub fn lower(s: &dyn fmt::Display) -> Result<String> {
pub fn lower<T: fmt::Display>(s: T) -> Result<String> {
let s = s.to_string();
Ok(s.to_lowercase())
}

/// Alias for the `lower()` filter
pub fn lowercase(s: &dyn fmt::Display) -> Result<String> {
pub fn lowercase<T: fmt::Display>(s: T) -> Result<String> {
lower(s)
}

/// Converts to uppercase
pub fn upper(s: &dyn fmt::Display) -> Result<String> {
pub fn upper<T: fmt::Display>(s: T) -> Result<String> {
let s = s.to_string();
Ok(s.to_uppercase())
}

/// Alias for the `upper()` filter
pub fn uppercase(s: &dyn fmt::Display) -> Result<String> {
pub fn uppercase<T: fmt::Display>(s: T) -> Result<String> {
upper(s)
}

/// Strip leading and trailing whitespace
pub fn trim(s: &dyn fmt::Display) -> Result<String> {
pub fn trim<T: fmt::Display>(s: T) -> Result<String> {
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<String> {
pub fn truncate<T: fmt::Display>(s: T, len: usize) -> Result<String> {
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;
}
Expand All @@ -257,7 +257,7 @@ pub fn truncate(s: &dyn fmt::Display, len: &usize) -> Result<String> {
}

/// Indent lines with `width` spaces
pub fn indent(s: &dyn fmt::Display, width: &usize) -> Result<String> {
pub fn indent<T: fmt::Display>(s: T, width: usize) -> Result<String> {
let s = s.to_string();

let mut indented = String::new();
Expand All @@ -266,7 +266,7 @@ pub fn indent(s: &dyn fmt::Display, width: &usize) -> Result<String> {
indented.push(c);

if c == '\n' && i < s.len() - 1 {
for _ in 0..*width {
for _ in 0..width {
indented.push(' ');
}
}
Expand All @@ -277,7 +277,7 @@ pub fn indent(s: &dyn fmt::Display, width: &usize) -> Result<String> {

#[cfg(feature = "num-traits")]
/// Casts number to f64
pub fn into_f64<T>(number: &T) -> Result<f64>
pub fn into_f64<T>(number: T) -> Result<f64>
where
T: NumCast,
{
Expand All @@ -286,7 +286,7 @@ where

#[cfg(feature = "num-traits")]
/// Casts number to isize
pub fn into_isize<T>(number: &T) -> Result<isize>
pub fn into_isize<T>(number: T) -> Result<isize>
where
T: NumCast,
{
Expand Down Expand Up @@ -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<String> {
pub fn capitalize<T: fmt::Display>(s: T) -> Result<String> {
let mut s = s.to_string();

match s.get_mut(0..1).map(|s| {
Expand Down Expand Up @@ -371,7 +371,7 @@ pub fn center(src: &dyn fmt::Display, dst_len: usize) -> Result<String> {
}

/// Count the words in that string
pub fn wordcount(s: &dyn fmt::Display) -> Result<usize> {
pub fn wordcount<T: fmt::Display>(s: T) -> Result<usize> {
let s = s.to_string();

Ok(s.split_whitespace().count())
Expand Down Expand Up @@ -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"
);
}
Expand All @@ -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))"),
};
Expand Down
6 changes: 4 additions & 2 deletions askama_shared/src/filters/yaml.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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: Escaper, S: Serialize>(e: E, s: &S) -> Result<MarkupDisplay<E, String>> {
match serde_yaml::to_string(s) {
pub fn yaml<E: Escaper, S: Serialize>(e: E, s: S) -> Result<MarkupDisplay<E, String>> {
match serde_yaml::to_string(&s) {
Ok(s) => Ok(MarkupDisplay::new_safe(s, e)),
Err(e) => Err(Error::from(e)),
}
Expand All @@ -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!(
Expand Down
13 changes: 10 additions & 3 deletions askama_shared/src/generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -1293,6 +1296,10 @@ impl<'a, S: std::hash::BuildHasher> Generator<'a, S> {
} else {
self.visit_expr(buf, arg)?;
}

if borrow {
buf.write(")");
}
}
Ok(())
}
Expand Down
39 changes: 39 additions & 0 deletions askama_shared/src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<MatchVariant<'a>>,
Expand Down
8 changes: 4 additions & 4 deletions testing/tests/simple.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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)
}

Expand All @@ -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]
Expand All @@ -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)
}
}
Expand Down

0 comments on commit b76f7ef

Please sign in to comment.