Repository: mitsuhiko/minijinja (language: rust)

Issue:
TooManyArguments, detail: "unknown keyword argument 'tz'", name: "<string>", line: 1 }
## Description
Trying to use the dateformat filter with timezone.
 
[What happened]

Error with the example provided in the documentation (https://docs.rs/minijinja-contrib/latest/minijinja_contrib/filters/fn.dateformat.html)

## Reproduction steps

1. Add the dependencies (using latest, 1.0.11):

```toml
minijinja = { workspace = true, default-features = false, features = [

  "builtins",
  "macros",
  "speedups",
  "urlencode",
] }
minijinja-contrib = { workspace = true, features = ["datetime", "timezone", "time-tz", "time"] }
chrono = { workspace = true, features = ["serde"] }

```

2. Make a function to create the jinja engine:

```rust

fn get_jinja_engine<'a>() -> Result<&'a Environment<'static>, Box<dyn Error>> {
    let engine = {
        if JINJA_ENGINE.get().is_none() {
            let mut env = Environment::new();
            minijinja_contrib::add_to_environment(&mut env);
            JINJA_ENGINE
                .set(env)
                .map_err(|_env| "could not setup jinja".to_string())?;
        }
        JINJA_ENGINE
            .get()
            .ok_or("could not extract engine from cell")
    }?;
    Ok(engine)
}
```

3. Make a simple unit test:

```rust
    #[tokio::test]
    async fn test_date_and_time() {
        #[derive(Serialize, Deserialize)]
        struct Whatever {
            dt: NaiveDateTime,
            d: NaiveDate,
        }

        let ctx = Whatever {
            dt: Local::now().naive_local(),
            d: NaiveDate::from_ymd_opt(2024, 1, 1).unwrap(),
        };

        let tpl = r#"{{ d|dateformat(format="short", tz='Europe/Brussels') }}"#;
        let engine = get_jinja_engine().unwrap();
        let res = engine.render_str(tpl, ctx).unwrap();
        println!("{}", res);
    }
}
```

Additional helpful information:

- Version of minijinja: 1.0.11
- Version of rustc: 1.75
- Operating system and version: PopOS 22.0.4

## What did you expect

I should get the date format like e.g `01/01/2024`





Discussion / hints:
This is likely happening because you are using datetime objects without timezone. The system has not been built to support this. Potentially support for this can be added but it's something that is not supported today.

Fix patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 49a023ce..9870e3de 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,6 +6,7 @@ All notable changes to MiniJinja are documented here.
 
 - The `urlencode` filter now correctly skips over none and undefined.  #394
 - The `dict` function now supports merging in of extra arguments.  #395
+- Added support for primitive datetimes in the contrib module.  #398
 
 ## 1.0.11
 
diff --git a/Makefile b/Makefile
index 95ab4c88..0bf611a2 100644
--- a/Makefile
+++ b/Makefile
@@ -12,13 +12,18 @@ build:
 doc:
 	@cd minijinja; RUSTC_BOOTSTRAP=1 RUSTDOCFLAGS="--cfg=docsrs --html-in-header doc-header.html" cargo doc -p minijinja -p minijinja-autoreload -p minijinja-stack-ref -p minijinja-contrib --no-deps --features=$(DOC_FEATURES)
 
-.PHONY: test
-test:
+.PHONY: test-msrv
+test-msrv:
 	@$(MAKE) run-tests FEATURES=$(TEST_FEATURES)
 	@$(MAKE) run-tests FEATURES=$(TEST_FEATURES),preserve_order,key_interning,unicode
 	@echo "CARGO TEST ALL FEATURES"
 	@cd minijinja; cargo test --all-features
 
+.PHONY: test
+test: test-msrv
+	@echo "CARGO TEST MINIJINJA-CONTRIB ALL FEATURES"
+	@cd minijinja-contrib; cargo test --all-features
+
 .PHONY: wasi-test
 wasi-test:
 	@cd minijinja; cargo test --all-features --target=wasm32-wasi -- --nocapture
@@ -63,6 +68,12 @@ check:
 	@echo "check minijinja-autoreload:"
 	@cd minijinja-autoreload; cargo check
 	@cd minijinja-autoreload; cargo check --no-default-features
+	@echo "check minijinja-contrib:"
+	@cd minijinja-contrib; cargo check
+	@cd minijinja-contrib; cargo check --all-features
+	@cd minijinja-contrib; cargo check --no-default-features
+	@echo "check minijinja-stack-ref:"
+	@cd minijinja-stack-ref; cargo check
 
 .PHONY: format
 format:
diff --git a/minijinja-contrib/src/filters/datetime.rs b/minijinja-contrib/src/filters/datetime.rs
index d0cd833a..7634aabf 100644
--- a/minijinja-contrib/src/filters/datetime.rs
+++ b/minijinja-contrib/src/filters/datetime.rs
@@ -5,7 +5,7 @@ use minijinja::{Error, ErrorKind, State};
 use serde::de::value::SeqDeserializer;
 use serde::Deserialize;
 use time::format_description::well_known::iso8601::Iso8601;
-use time::{format_description, Date, OffsetDateTime};
+use time::{format_description, Date, OffsetDateTime, PrimitiveDateTime};
 
 fn handle_serde_error(err: serde::de::value::Error) -> Error {
     Error::new(ErrorKind::InvalidOperation, "not a valid date or timestamp").with_source(err)
@@ -18,19 +18,30 @@ fn value_to_datetime(
     kwargs: &Kwargs,
     allow_date: bool,
 ) -> Result<OffsetDateTime, Error> {
+    #[allow(unused)]
+    let mut timezone_already_handled = false;
+
     #[allow(unused_mut)]
     let (mut datetime, had_time) = if let Some(s) = value.as_str() {
         match OffsetDateTime::parse(s, &Iso8601::PARSING) {
             Ok(dt) => (dt, true),
-            Err(original_err) => match Date::parse(s, &Iso8601::PARSING) {
-                Ok(date) => (date.with_hms(0, 0, 0).unwrap().assume_utc(), false),
-                Err(_) => {
-                    return Err(Error::new(
-                        ErrorKind::InvalidOperation,
-                        "not a valid date or timestamp",
-                    )
-                    .with_source(original_err))
-                }
+            Err(original_err) => match PrimitiveDateTime::parse(s, &Iso8601::PARSING) {
+                Ok(dt) => attach_timezone_to_primitive_datetime(
+                    state,
+                    kwargs,
+                    &mut timezone_already_handled,
+                    dt,
+                )?,
+                Err(_) => match Date::parse(s, &Iso8601::PARSING) {
+                    Ok(date) => (date.with_hms(0, 0, 0).unwrap().assume_utc(), false),
+                    Err(_) => {
+                        return Err(Error::new(
+                            ErrorKind::InvalidOperation,
+                            "not a valid date or timestamp",
+                        )
+                        .with_source(original_err))
+                    }
+                },
             },
         }
     } else if let Ok(v) = f64::try_from(value.clone()) {
@@ -53,6 +64,10 @@ fn value_to_datetime(
                     .assume_utc(),
                 false,
             )
+        } else if items.len() == 6 {
+            let dt = PrimitiveDateTime::deserialize(SeqDeserializer::new(items.into_iter()))
+                .map_err(handle_serde_error)?;
+            attach_timezone_to_primitive_datetime(state, kwargs, &mut timezone_already_handled, dt)?
         } else {
             (
                 OffsetDateTime::deserialize(SeqDeserializer::new(items.into_iter()))
@@ -70,23 +85,12 @@ fn value_to_datetime(
     if had_time {
         #[cfg(feature = "timezone")]
         {
-            use time_tz::OffsetDateTimeExt;
-            let configured_tz = state.lookup("TIMEZONE");
-            let tzname = kwargs.get::<Option<&str>>("tz")?.unwrap_or_else(|| {
-                configured_tz
-                    .as_ref()
-                    .and_then(|x| x.as_str())
-                    .unwrap_or("original")
-            });
-            if tzname != "original" {
-                let tz = time_tz::timezones::get_by_name(tzname).ok_or_else(|| {
-                    Error::new(
-                        ErrorKind::InvalidOperation,
-                        format!("unknown timezone '{}'", tzname),
-                    )
-                })?;
-                datetime = datetime.to_timezone(tz)
-            };
+            if !timezone_already_handled {
+                if let Some(tz) = get_timezone(state, kwargs)? {
+                    use time_tz::OffsetDateTimeExt;
+                    datetime = datetime.to_timezone(tz);
+                }
+            }
         }
     } else if !allow_date {
         return Err(Error::new(
@@ -98,6 +102,57 @@ fn value_to_datetime(
     Ok(datetime)
 }
 
+#[allow(unused)]
+fn attach_timezone_to_primitive_datetime(
+    state: &State<'_, '_>,
+    kwargs: &Kwargs,
+    timezone_already_handled: &mut bool,
+    dt: PrimitiveDateTime,
+) -> Result<(OffsetDateTime, bool), Error> {
+    #[cfg(feature = "timezone")]
+    {
+        if let Some(tz) = get_timezone(state, kwargs)? {
+            *timezone_already_handled = true;
+            Ok((
+                time_tz::PrimitiveDateTimeExt::assume_timezone(&dt, tz).unwrap_first(),
+                true,
+            ))
+        } else {
+            Ok((dt.assume_utc(), true))
+        }
+    }
+    #[cfg(not(feature = "timezone"))]
+    {
+        Ok((dt.assume_utc(), true))
+    }
+}
+
+#[cfg(feature = "timezone")]
+fn get_timezone(
+    state: &State<'_, '_>,
+    kwargs: &Kwargs,
+) -> Result<Option<&'static time_tz::Tz>, Error> {
+    let configured_tz = state.lookup("TIMEZONE");
+    let tzname = kwargs.get::<Option<&str>>("tz")?.unwrap_or_else(|| {
+        configured_tz
+            .as_ref()
+            .and_then(|x| x.as_str())
+            .unwrap_or("original")
+    });
+    if tzname != "original" {
+        Ok(Some(time_tz::timezones::get_by_name(tzname).ok_or_else(
+            || {
+                Error::new(
+                    ErrorKind::InvalidOperation,
+                    format!("unknown timezone '{}'", tzname),
+                )
+            },
+        )?))
+    } else {
+        Ok(None)
+    }
+}
+
 /// Formats a timestamp as date and time.
 ///
 /// The value needs to be a unix timestamp, or a parsable string (ISO 8601) or a


Test patch:
diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml
index 1c0e94f4..720224de 100644
--- a/.github/workflows/tests.yml
+++ b/.github/workflows/tests.yml
@@ -96,7 +96,7 @@ jobs:
       - name: Restore Cargo.lock
         run: cp Cargo.lock.msrv Cargo.lock
       - name: Test
-        run: make test
+        run: make test-msrv
 
   test-no-lock:
     name: Test on Latest (No Lock)
diff --git a/minijinja-contrib/tests/datetime.rs b/minijinja-contrib/tests/datetime.rs
index 8a12b34c..983185e2 100644
--- a/minijinja-contrib/tests/datetime.rs
+++ b/minijinja-contrib/tests/datetime.rs
@@ -1,6 +1,7 @@
 #![cfg(all(feature = "datetime", feature = "timezone"))]
 use minijinja::context;
 use similar_asserts::assert_eq;
+use time::format_description::well_known::Iso8601;
 
 #[test]
 fn test_datetimeformat() {
@@ -180,6 +181,33 @@ fn test_dateformat_chrono_rs() {
     );
 }
 
+#[test]
+fn test_datetime_format_naive() {
+    let mut env = minijinja::Environment::new();
+    minijinja_contrib::add_to_environment(&mut env);
+
+    let d = time::Date::from_calendar_date(2024, time::Month::January, 18)
+        .unwrap()
+        .with_hms(0, 1, 2)
+        .unwrap();
+
+    let expr = env
+        .compile_expression("d|datetimeformat(format=format, tz='Europe/Brussels')")
+        .unwrap();
+    assert_eq!(
+        expr.eval(
+            context!(d => d.format(&Iso8601::DATE_TIME).unwrap().to_string(), format => "iso")
+        )
+        .unwrap()
+        .to_string(),
+        "2024-01-18T00:01:02+01:00"
+    );
+    assert_eq!(
+        expr.eval(context!(d, format => "iso")).unwrap().to_string(),
+        "2024-01-18T00:01:02+01:00"
+    );
+}
+
 #[test]
 fn test_timeformat() {
     let mut env = minijinja::Environment::new();