Repository: mitsuhiko/minijinja
Pull request #39 (2022-01-02 08:08:31)

Added integer division support
The `FloorDiv` token was not implemented correctly and the regular division operator deviated from Jinja2 by returning integers for integer divisions. This now matches Jinja2 largely except for using euclidean division instead of flooring division.


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index b3aa0d0..c82a288 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -13,6 +13,10 @@ All notable changes to MiniJinja are documented here.
 - Added `batch` and `slice` filter.
 - Removed value internal distinction between maps and structs.
 - Added `list` filter.
+- Implemented integer division operator (``//``) and changed division to always
+  return floats like documented and to match the Jinja2 implementation.  To
+  make this more convenient whole integer floats are now handled like integers in
+  some situations.
 
 # 0.10.0
 
diff --git a/minijinja/src/compiler.rs b/minijinja/src/compiler.rs
index 70d8ee3..bfd5d0b 100644
--- a/minijinja/src/compiler.rs
+++ b/minijinja/src/compiler.rs
@@ -323,6 +323,7 @@ impl<'source> Compiler<'source> {
                 });
             }
             ast::Expr::BinOp(c) => {
+                self.set_location_from_span(c.span());
                 let instr = match c.op {
                     ast::BinOpKind::Eq => Instruction::Eq,
                     ast::BinOpKind::Ne => Instruction::Ne,
@@ -342,14 +343,12 @@ impl<'source> Compiler<'source> {
                     ast::BinOpKind::Sub => Instruction::Sub,
                     ast::BinOpKind::Mul => Instruction::Mul,
                     ast::BinOpKind::Div => Instruction::Div,
-                    // TODO: floor div maps to normal div for now
-                    ast::BinOpKind::FloorDiv => Instruction::Div,
+                    ast::BinOpKind::FloorDiv => Instruction::IntDiv,
                     ast::BinOpKind::Rem => Instruction::Rem,
                     ast::BinOpKind::Pow => Instruction::Pow,
                     ast::BinOpKind::Concat => Instruction::StringConcat,
                     ast::BinOpKind::In => Instruction::In,
                 };
-                self.set_location_from_span(c.span());
                 self.compile_expr(&c.left)?;
                 self.compile_expr(&c.right)?;
                 self.add(instr);
diff --git a/minijinja/src/instructions.rs b/minijinja/src/instructions.rs
index da1256d..fc5edb2 100644
--- a/minijinja/src/instructions.rs
+++ b/minijinja/src/instructions.rs
@@ -48,6 +48,13 @@ pub enum Instruction<'source> {
     /// Divide the top two values
     Div,
 
+    /// Integer divde the top two values as "integer".
+    ///
+    /// Note that in MiniJinja this currently uses an euclidean
+    /// division to match the rem implementation.  In Python this
+    /// instead uses a flooring division and a flooring remainder.
+    IntDiv,
+
     /// Calculate the remainder the top two values
     Rem,
 
@@ -182,6 +189,7 @@ impl<'source> fmt::Debug for Instruction<'source> {
             Instruction::Sub => write!(f, "SUB"),
             Instruction::Mul => write!(f, "MUL"),
             Instruction::Div => write!(f, "DIV"),
+            Instruction::IntDiv => write!(f, "INT_DIV"),
             Instruction::Rem => write!(f, "REM"),
             Instruction::Pow => write!(f, "Pow"),
             Instruction::Neg => write!(f, "NEG"),
diff --git a/minijinja/src/key.rs b/minijinja/src/key.rs
index 03f3ad6..16dea09 100644
--- a/minijinja/src/key.rs
+++ b/minijinja/src/key.rs
@@ -82,7 +82,18 @@ impl<'a> Key<'a> {
             ValueRepr::I128(ref v) => TryFrom::try_from(**v)
                 .map(Key::I64)
                 .map_err(|_| ErrorKind::NonKey.into()),
-            ValueRepr::F64(_) => Err(ErrorKind::NonKey.into()),
+            ValueRepr::F64(x) => {
+                // if a float is in fact looking like an integer we
+                // allow this to be used for indexing.  Why?  Because
+                // in Jinja division is always a division resuling
+                // in floating point values (4 / 2 == 2.0).
+                let intval = x as i64;
+                if intval as f64 == x {
+                    Ok(Key::I64(intval))
+                } else {
+                    Err(ErrorKind::NonKey.into())
+                }
+            }
             ValueRepr::Char(c) => Ok(Key::Char(c)),
             ValueRepr::String(ref s) => Ok(Key::Str(s)),
             _ => Err(ErrorKind::NonKey.into()),
diff --git a/minijinja/src/syntax.rs b/minijinja/src/syntax.rs
index a1d6ba3..bca0ecb 100644
--- a/minijinja/src/syntax.rs
+++ b/minijinja/src/syntax.rs
@@ -88,12 +88,16 @@
 //!
 //! - ``+``: Adds two numbers up. ``{{ 1 + 1 }}`` is ``2``.
 //! - ``-``: Subtract the second number from the first one.  ``{{ 3 - 2 }}`` is ``1``.
-//! - ``/``: Divide two numbers. ``{{ 1 / 2 }}`` is ``{{ 0.5 }}``.
+//! - ``/``: Divide two numbers. ``{{ 1 / 2 }}`` is ``0.5``.  See note on divisions below.
+//! - ``//``: Integer divide two numbers. ``{{ 5 // 3 }}`` is ``1``.  See note on divisons below.
 //! - ``%``: Calculate the remainder of an integer division.  ``{{ 11 % 7 }}`` is ``4``.
 //! - ``*``: Multiply the left operand with the right one.  ``{{ 2 * 2 }}`` would return ``4``.
 //! - ``**``: Raise the left operand to the power of the right operand.  ``{{ 2**3 }}``
 //!   would return ``8``.
 //!
+//! Note on divisions: divisions in Jinja2 are flooring, divisions in MiniJinja
+//! are at present using euclidean division.  They are almost the same but not quite.
+//!
 //! ## Comparisons
 //!  
 //! - ``==``: Compares two objects for equality.
diff --git a/minijinja/src/value.rs b/minijinja/src/value.rs
index 2564957..ab821f2 100644
--- a/minijinja/src/value.rs
+++ b/minijinja/src/value.rs
@@ -456,10 +456,6 @@ fn as_f64(value: &Value) -> Option<f64> {
     })
 }
 
-fn is_numeric(value: &Value) -> bool {
-    matches!(value.kind(), ValueKind::Bool | ValueKind::Number)
-}
-
 fn coerce(a: &Value, b: &Value) -> Option<CoerceResult> {
     match (&a.0, &b.0) {
         // equal mappings are trivial
@@ -476,11 +472,10 @@ fn coerce(a: &Value, b: &Value) -> Option<CoerceResult> {
         (_, ValueRepr::F64(b)) => Some(CoerceResult::F64(as_f64(a)?, *b)),
 
         // everything else goes up to i128
-        (_, _) if is_numeric(a) && is_numeric(b) => Some(CoerceResult::I128(
+        _ => Some(CoerceResult::I128(
             i128::try_from(a.clone()).ok()?,
             i128::try_from(b.clone()).ok()?,
         )),
-        _ => None,
     }
 }
 
@@ -491,7 +486,7 @@ impl fmt::Display for Value {
             ValueRepr::Bool(val) => write!(f, "{}", val),
             ValueRepr::U64(val) => write!(f, "{}", val),
             ValueRepr::I64(val) => write!(f, "{}", val),
-            ValueRepr::F64(val) => write!(f, "{}", val),
+            ValueRepr::F64(val) => write!(f, "{:.1}", val),
             ValueRepr::Char(val) => write!(f, "{}", val),
             ValueRepr::None => write!(f, "none"),
             ValueRepr::I128(val) => write!(f, "{}", val),
@@ -541,7 +536,7 @@ fn int_as_value(val: i128) -> Value {
 macro_rules! math_binop {
     ($name:ident, $int:ident, $float:tt) => {
         pub(crate) fn $name(lhs: &Value, rhs: &Value) -> Result<Value, Error> {
-            pub fn do_it(lhs: &Value, rhs: &Value) -> Option<Value> {
+            fn do_it(lhs: &Value, rhs: &Value) -> Option<Value> {
                 match coerce(lhs, rhs)? {
                     CoerceResult::I128(a, b) => Some(int_as_value(a.$int(b))),
                     CoerceResult::F64(a, b) => Some((a $float b).into()),
@@ -565,9 +560,45 @@ macro_rules! math_binop {
 math_binop!(add, wrapping_add, +);
 math_binop!(sub, wrapping_sub, -);
 math_binop!(mul, wrapping_mul, *);
-math_binop!(div, wrapping_div, /);
 math_binop!(rem, wrapping_rem_euclid, %);
 
+pub(crate) fn div(lhs: &Value, rhs: &Value) -> Result<Value, Error> {
+    fn do_it(lhs: &Value, rhs: &Value) -> Option<Value> {
+        let a = as_f64(lhs)?;
+        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()
+            ),
+        )
+    })
+}
+
+pub(crate) fn int_div(lhs: &Value, rhs: &Value) -> Result<Value, Error> {
+    fn do_it(lhs: &Value, rhs: &Value) -> Option<Value> {
+        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()),
+        }
+    }
+    do_it(lhs, rhs).ok_or_else(|| {
+        Error::new(
+            ErrorKind::ImpossibleOperation,
+            format!(
+                "tried to use // operator on unsupported types {} and {}",
+                lhs.kind(),
+                rhs.kind()
+            ),
+        )
+    })
+}
+
 /// Implements a binary `pow` operation on values.
 pub(crate) fn pow(lhs: &Value, rhs: &Value) -> Result<Value, Error> {
     pub fn do_it(lhs: &Value, rhs: &Value) -> Option<Value> {
@@ -646,7 +677,7 @@ pub(crate) fn contains(container: &Value, value: &Value) -> Result<Value, Error>
 
 macro_rules! primitive_try_from {
     ($ty:ident, {
-        $($pat:pat => $expr:expr,)*
+        $($pat:pat $(if $if_expr:expr)? => $expr:expr,)*
     }) => {
 
         impl TryFrom<Value> for $ty {
@@ -654,7 +685,7 @@ macro_rules! primitive_try_from {
 
             fn try_from(value: Value) -> Result<Self, Self::Error> {
                 let opt = match value.0 {
-                    $($pat => TryFrom::try_from($expr).ok(),)*
+                    $($pat $(if $if_expr)? => TryFrom::try_from($expr).ok(),)*
                     _ => None
                 };
                 opt.ok_or_else(|| {
@@ -698,6 +729,8 @@ macro_rules! primitive_int_try_from {
             ValueRepr::Bool(val) => val as usize,
             ValueRepr::I64(val) => val,
             ValueRepr::U64(val) => val,
+            // for the intention here see Key::from_borrowed_value
+            ValueRepr::F64(val) if (val as i64 as f64 == val) => val as i64,
             ValueRepr::I128(ref val) => **val,
             ValueRepr::U128(ref val) => **val,
         });
@@ -1813,6 +1846,20 @@ fn test_string_key_lookup() {
     assert_eq!(m.get_item(&Value::from("foo")).unwrap(), Value::from(42));
 }
 
+#[test]
+fn test_int_key_lookup() {
+    let mut m = BTreeMap::new();
+    m.insert(Key::I64(42), Value::from(42));
+    m.insert(Key::I64(23), Value::from(23));
+    let m = Value::from(m);
+    assert_eq!(m.get_item(&Value::from(42.0f32)).unwrap(), Value::from(42));
+    assert_eq!(m.get_item(&Value::from(42u32)).unwrap(), Value::from(42));
+
+    let s = Value::from(vec![42i32, 23]);
+    assert_eq!(s.get_item(&Value::from(0.0f32)).unwrap(), Value::from(42));
+    assert_eq!(s.get_item(&Value::from(0i32)).unwrap(), Value::from(42));
+}
+
 #[test]
 fn test_value_serialization() {
     // make sure if we serialize to json we get regular values
diff --git a/minijinja/src/vm.rs b/minijinja/src/vm.rs
index 960f837..44c91da 100644
--- a/minijinja/src/vm.rs
+++ b/minijinja/src/vm.rs
@@ -577,6 +577,7 @@ impl<'env> Vm<'env> {
                 Instruction::Sub => func_binop!(sub),
                 Instruction::Mul => func_binop!(mul),
                 Instruction::Div => func_binop!(div),
+                Instruction::IntDiv => func_binop!(int_div),
                 Instruction::Rem => func_binop!(rem),
                 Instruction::Pow => func_binop!(pow),
                 Instruction::Eq => op_binop!(==),


Test patch:
diff --git a/minijinja/tests/inputs/math.txt b/minijinja/tests/inputs/math.txt
new file mode 100644
index 0000000..a16acaf
--- /dev/null
+++ b/minijinja/tests/inputs/math.txt
@@ -0,0 +1,6 @@
+var: 5
+---
+should be -2.5: {{ 1.5 * 2.5 * 2 / 3 - var }}
+should be -3.0: {{ 1.5 * 2.5 * 2 // 3 - var }}
+should be 2.0: {{ 4 / 2 }}
+should be 2: {{ 4 // 2 }}
diff --git a/minijinja/tests/lexer-inputs/operators.txt b/minijinja/tests/lexer-inputs/operators.txt
index 548cec2..5422d5c 100644
--- a/minijinja/tests/lexer-inputs/operators.txt
+++ b/minijinja/tests/lexer-inputs/operators.txt
@@ -1,1 +1,1 @@
-{{ (!a + b) * (c - d) / e % f ~ g }}
\ No newline at end of file
+{{ (!a + b) * (c - d) / e % f // g ~ h }}
\ No newline at end of file
diff --git a/minijinja/tests/snapshots/test_lexer__lexer@operators.txt.snap b/minijinja/tests/snapshots/test_lexer__lexer@operators.txt.snap
index eb0da7b..a174e5f 100644
--- a/minijinja/tests/snapshots/test_lexer__lexer@operators.txt.snap
+++ b/minijinja/tests/snapshots/test_lexer__lexer@operators.txt.snap
@@ -1,7 +1,7 @@
 ---
-source: tests/test_lexer.rs
+source: minijinja/tests/test_lexer.rs
 expression: "&tokens"
-input_file: tests/lexer-inputs/operators.txt
+input_file: minijinja/tests/lexer-inputs/operators.txt
 
 ---
 [
@@ -22,7 +22,9 @@ input_file: tests/lexer-inputs/operators.txt
     IDENT(e),
     MOD,
     IDENT(f),
-    TILDE,
+    FLOORDIV,
     IDENT(g),
+    TILDE,
+    IDENT(h),
     VARIABLE_END(false),
 ]
diff --git a/minijinja/tests/snapshots/test_templates__vm@math.txt.snap b/minijinja/tests/snapshots/test_templates__vm@math.txt.snap
new file mode 100644
index 0000000..eaec508
--- /dev/null
+++ b/minijinja/tests/snapshots/test_templates__vm@math.txt.snap
@@ -0,0 +1,11 @@
+---
+source: minijinja/tests/test_templates.rs
+expression: "&rendered"
+input_file: minijinja/tests/inputs/math.txt
+
+---
+should be -2.5: -2.5
+should be -3.0: -3.0
+should be 2.0: 2.0
+should be 2: 2
+