From b1e16ea7e5de8c65a00a383b6305b0eea704895d Mon Sep 17 00:00:00 2001 From: Josh Taylor Date: Fri, 2 Sep 2022 10:31:05 +0800 Subject: [PATCH 1/3] Add string adding `('foo' + 'bar')` --- minijinja/src/value.rs | 45 +++++++++++++++++++ minijinja/tests/inputs/adding.txt | 16 +++++++ minijinja/tests/inputs/set.txt | 2 + .../test_templates__vm@adding.txt.snap | 18 ++++++++ .../snapshots/test_templates__vm@set.txt.snap | 4 +- 5 files changed, 84 insertions(+), 1 deletion(-) create mode 100644 minijinja/tests/inputs/adding.txt create mode 100644 minijinja/tests/snapshots/test_templates__vm@adding.txt.snap diff --git a/minijinja/src/value.rs b/minijinja/src/value.rs index 70f27cd4..d8f20307 100644 --- a/minijinja/src/value.rs +++ b/minijinja/src/value.rs @@ -301,6 +301,7 @@ impl PartialEq for Value { _ => match coerce(self, other) { Some(CoerceResult::F64(a, b)) => a == b, Some(CoerceResult::I128(a, b)) => a == b, + Some(CoerceResult::String(a, b)) => a == b, None => false, }, } @@ -319,6 +320,7 @@ impl PartialOrd for Value { _ => match coerce(self, other) { Some(CoerceResult::F64(a, b)) => a.partial_cmp(&b), Some(CoerceResult::I128(a, b)) => a.partial_cmp(&b), + Some(CoerceResult::String(a, b)) => a.partial_cmp(&b), None => None, }, } @@ -444,6 +446,7 @@ value_from!(char, Char); enum CoerceResult { I128(i128, i128), F64(f64, f64), + String(String, String), } fn as_f64(value: &Value) -> Option { @@ -465,6 +468,9 @@ fn coerce(a: &Value, b: &Value) -> Option { (ValueRepr::U128(a), ValueRepr::U128(b)) => { Some(CoerceResult::I128(**a as i128, **b as i128)) } + (ValueRepr::String(a), ValueRepr::String(b)) => { + Some(CoerceResult::String(a.to_string(), b.to_string())) + } (ValueRepr::I64(a), ValueRepr::I64(b)) => Some(CoerceResult::I128(*a as i128, *b as i128)), (ValueRepr::I128(ref a), ValueRepr::I128(ref b)) => Some(CoerceResult::I128(**a, **b)), (ValueRepr::F64(a), ValueRepr::F64(b)) => Some(CoerceResult::F64(*a, *b)), @@ -548,6 +554,8 @@ macro_rules! math_binop { match coerce(lhs, rhs)? { CoerceResult::I128(a, b) => Some(int_as_value(a.$int(b))), CoerceResult::F64(a, b) => Some((a $float b).into()), + CoerceResult::String(a, b) if "+" == stringify!($float) => Some(Value::from([a, b].concat())), + _ => None } } do_it(lhs, rhs).ok_or_else(|| { @@ -593,6 +601,7 @@ pub(crate) fn int_div(lhs: &Value, rhs: &Value) -> Result { match coerce(lhs, rhs)? { CoerceResult::I128(a, b) => Some(int_as_value(a.div_euclid(b))), CoerceResult::F64(a, b) => Some(a.div_euclid(b).into()), + CoerceResult::String(_, _) => None, } } do_it(lhs, rhs).ok_or_else(|| { @@ -613,6 +622,7 @@ pub(crate) fn pow(lhs: &Value, rhs: &Value) -> Result { match coerce(lhs, rhs)? { CoerceResult::I128(a, b) => Some(int_as_value(a.pow(TryFrom::try_from(b).ok()?))), CoerceResult::F64(a, b) => Some((a.powf(b)).into()), + CoerceResult::String(_, _) => None, } } do_it(lhs, rhs).ok_or_else(|| { @@ -1732,6 +1742,41 @@ fn test_adding() { ); assert_eq!(add(&value!(1), &value!(2)), Ok(value!(3))); + assert_eq!(add(&value!("foo"), &value!("bar")), Ok(value!("foobar"))); +} + +#[test] +fn test_subtracting() { + let err = sub(&value!("a"), &value!(42)).unwrap_err(); + assert_eq!( + err.to_string(), + "impossible operation: tried to use - operator on unsupported types string and number" + ); + + let err = sub(&value!("foo"), &value!("bar")).unwrap_err(); + assert_eq!( + err.to_string(), + "impossible operation: tried to use - operator on unsupported types string and string" + ); + + assert_eq!(sub(&value!(2), &value!(1)), Ok(value!(1))); +} + +#[test] +fn test_dividing() { + let err = div(&value!("a"), &value!(42)).unwrap_err(); + assert_eq!( + err.to_string(), + "impossible operation: tried to use / operator on unsupported types string and number" + ); + + let err = div(&value!("foo"), &value!("bar")).unwrap_err(); + assert_eq!( + err.to_string(), + "impossible operation: tried to use / operator on unsupported types string and string" + ); + + assert_eq!(div(&value!(100), &value!(2)), Ok(value!(50.0))); } #[test] diff --git a/minijinja/tests/inputs/adding.txt b/minijinja/tests/inputs/adding.txt new file mode 100644 index 00000000..7d3dc7be --- /dev/null +++ b/minijinja/tests/inputs/adding.txt @@ -0,0 +1,16 @@ +{ + "name": "World" +} +--- +Strings: +{{ 'to' + name + 'you' }}! +{{ 'to' + 'you' }}! + +Adding: +{{ 1 + 2 }} + +Minus: +{{ 2 + 1 }} + +Divide: +{{ 100 / 2 }} \ No newline at end of file diff --git a/minijinja/tests/inputs/set.txt b/minijinja/tests/inputs/set.txt index de9f91bb..c16f4e59 100644 --- a/minijinja/tests/inputs/set.txt +++ b/minijinja/tests/inputs/set.txt @@ -21,4 +21,6 @@ Into Loop: Conditional: {% if true %}{% set foo = "was true" %}{% endif %} +String set: +{% set foo = "foo" + "bar" %} {{ foo }} \ No newline at end of file diff --git a/minijinja/tests/snapshots/test_templates__vm@adding.txt.snap b/minijinja/tests/snapshots/test_templates__vm@adding.txt.snap new file mode 100644 index 00000000..b1470578 --- /dev/null +++ b/minijinja/tests/snapshots/test_templates__vm@adding.txt.snap @@ -0,0 +1,18 @@ +--- +source: minijinja/tests/test_templates.rs +expression: "&rendered" +input_file: minijinja/tests/inputs/adding.txt +--- +Strings: +toWorldyou! +toyou! + +Adding: +3 + +Minus: +3 + +Divide: +50.0 + diff --git a/minijinja/tests/snapshots/test_templates__vm@set.txt.snap b/minijinja/tests/snapshots/test_templates__vm@set.txt.snap index 89a4cb46..e23f0848 100644 --- a/minijinja/tests/snapshots/test_templates__vm@set.txt.snap +++ b/minijinja/tests/snapshots/test_templates__vm@set.txt.snap @@ -30,5 +30,7 @@ Into Loop: Conditional: -was true +String set: + +foobar From 2ae7f2efea4c250ff5ab9bf822cc413322b22c09 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Fri, 2 Sep 2022 12:56:54 +0200 Subject: [PATCH 2/3] Avoid depending on stringify for logic --- minijinja/src/value.rs | 64 ++++++++++++++++++------------------------ 1 file changed, 27 insertions(+), 37 deletions(-) diff --git a/minijinja/src/value.rs b/minijinja/src/value.rs index d8f20307..0782f45f 100644 --- a/minijinja/src/value.rs +++ b/minijinja/src/value.rs @@ -547,6 +547,18 @@ fn int_as_value(val: i128) -> Value { } } +fn impossible_op(op: &str, lhs: &Value, rhs: &Value) -> Error { + Error::new( + ErrorKind::ImpossibleOperation, + format!( + "tried to use {} operator on unsupported types {} and {}", + op, + lhs.kind(), + rhs.kind() + ), + ) +} + macro_rules! math_binop { ($name:ident, $int:ident, $float:tt) => { pub(crate) fn $name(lhs: &Value, rhs: &Value) -> Result { @@ -554,26 +566,27 @@ macro_rules! math_binop { match coerce(lhs, rhs)? { CoerceResult::I128(a, b) => Some(int_as_value(a.$int(b))), CoerceResult::F64(a, b) => Some((a $float b).into()), - CoerceResult::String(a, b) if "+" == stringify!($float) => Some(Value::from([a, b].concat())), _ => None } } do_it(lhs, rhs).ok_or_else(|| { - Error::new( - ErrorKind::ImpossibleOperation, - format!( - "tried to use {} operator on unsupported types {} and {}", - stringify!($float), - lhs.kind(), - rhs.kind() - ) - ) + impossible_op(stringify!($float), lhs, rhs) }) } } } -math_binop!(add, wrapping_add, +); +pub(crate) fn add(lhs: &Value, rhs: &Value) -> Result { + fn do_it(lhs: &Value, rhs: &Value) -> Option { + match coerce(lhs, rhs)? { + CoerceResult::I128(a, b) => Some(int_as_value(a.wrapping_add(b))), + CoerceResult::F64(a, b) => Some((a + b).into()), + CoerceResult::String(a, b) => Some(Value::from([a, b].concat())), + } + } + do_it(lhs, rhs).ok_or_else(|| impossible_op("+", lhs, rhs)) +} + math_binop!(sub, wrapping_sub, -); math_binop!(mul, wrapping_mul, *); math_binop!(rem, wrapping_rem_euclid, %); @@ -584,16 +597,7 @@ pub(crate) fn div(lhs: &Value, rhs: &Value) -> Result { let b = as_f64(rhs)?; Some((a / b).into()) } - do_it(lhs, rhs).ok_or_else(|| { - Error::new( - ErrorKind::ImpossibleOperation, - format!( - "tried to use / operator on unsupported types {} and {}", - lhs.kind(), - rhs.kind() - ), - ) - }) + do_it(lhs, rhs).ok_or_else(|| impossible_op("/", lhs, rhs)) } pub(crate) fn int_div(lhs: &Value, rhs: &Value) -> Result { @@ -604,16 +608,7 @@ pub(crate) fn int_div(lhs: &Value, rhs: &Value) -> Result { CoerceResult::String(_, _) => None, } } - do_it(lhs, rhs).ok_or_else(|| { - Error::new( - ErrorKind::ImpossibleOperation, - format!( - "tried to use // operator on unsupported types {} and {}", - lhs.kind(), - rhs.kind() - ), - ) - }) + do_it(lhs, rhs).ok_or_else(|| impossible_op("//", lhs, rhs)) } /// Implements a binary `pow` operation on values. @@ -625,12 +620,7 @@ pub(crate) fn pow(lhs: &Value, rhs: &Value) -> Result { CoerceResult::String(_, _) => None, } } - do_it(lhs, rhs).ok_or_else(|| { - Error::new( - ErrorKind::ImpossibleOperation, - concat!("could not calculate the power"), - ) - }) + do_it(lhs, rhs).ok_or_else(|| impossible_op("**", lhs, rhs)) } /// Implements an unary `neg` operation on value. From bfbd7366bcda406554c55377c9a70478ef2d0585 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Fri, 2 Sep 2022 13:05:21 +0200 Subject: [PATCH 3/3] Run CI on pull_request --- .github/workflows/clippy.yml | 2 +- .github/workflows/rustfmt.yml | 2 +- .github/workflows/tests.yml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/clippy.yml b/.github/workflows/clippy.yml index e812a0e5..aa834207 100644 --- a/.github/workflows/clippy.yml +++ b/.github/workflows/clippy.yml @@ -1,6 +1,6 @@ name: Clippy -on: [push] +on: [push, pull_request] jobs: build: diff --git a/.github/workflows/rustfmt.yml b/.github/workflows/rustfmt.yml index 90029629..a7a0ce60 100644 --- a/.github/workflows/rustfmt.yml +++ b/.github/workflows/rustfmt.yml @@ -1,6 +1,6 @@ name: Rustfmt -on: [push] +on: [push, pull_request] jobs: build: diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index af14db36..52282688 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -1,6 +1,6 @@ name: Tests -on: [push] +on: [push, pull_request] jobs: test-latest: