Repository: mitsuhiko/minijinja (language: rust)

Issue:
None value is converted to string when passed down to filter
## Description

I made a custom filter like:

```rust
fn my_filter(value: Option<String>) {}
```

and use it in a template, where the filter is applied for a field of a struct. That field has `None` value, but the filter got `Some("none")`.

To demonstrate the issue, I created a test case here:

https://github.com/hongquan/minijinja/blob/test/filter_expecting_optional_string/minijinja/tests/test_templates.rs#L534

## Reproduction steps

Please run the test case above.

Additional helpful information:

- Version of minijinja: v1.0.4
- Version of rustc: v1.70
- Operating system and version: Ubuntu 22.04

## What did you expect

The filter get `None` as its argument.

[What you think should have happened]





Discussion / hints:
The is intentional though confusing in this case. `None` on filter arguments means that the value was undefined (not passed). I’m not sure if `None` or `String` can be expressed today. You might need to work with `Value` here. 
Ok.

Fix patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 950156e7..88e6e8c5 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,6 +6,9 @@ All notable changes to MiniJinja are documented here.
 
 - Added the ability to merge multiple values with the `context!`
   macro.  (#317)
+- `Option<T>` now accepts `none` in filters.  Previously only
+  undefined values were accepted.  This bugfix might have a minor impact
+  on code that relied in this behavior.  (#320)
 
 ## 1.0.4
 
diff --git a/minijinja/src/filters.rs b/minijinja/src/filters.rs
index 655b6205..ddf48557 100644
--- a/minijinja/src/filters.rs
+++ b/minijinja/src/filters.rs
@@ -1144,7 +1144,11 @@ mod builtins {
             if !args.is_empty() {
                 return Err(Error::from(ErrorKind::TooManyArguments));
             }
-            let default = ok!(kwargs.get::<Option<Value>>("default"));
+            let default = if kwargs.has("default") {
+                ok!(kwargs.get::<Value>("default"))
+            } else {
+                Value::UNDEFINED
+            };
             for value in ok!(state.undefined_behavior().try_iter(value)) {
                 let sub_val = match attr.as_str() {
                     Some(path) => value.get_path(path),
@@ -1153,17 +1157,13 @@ mod builtins {
                 rv.push(match (sub_val, &default) {
                     (Ok(attr), _) => {
                         if attr.is_undefined() {
-                            if let Some(ref default) = default {
-                                default.clone()
-                            } else {
-                                Value::UNDEFINED
-                            }
+                            default.clone()
                         } else {
                             attr
                         }
                     }
-                    (Err(_), Some(default)) => default.clone(),
-                    (Err(err), None) => return Err(err),
+                    (Err(_), default) if !default.is_undefined() => default.clone(),
+                    (Err(err), _) => return Err(err),
                 });
             }
             ok!(kwargs.assert_all_used());
diff --git a/minijinja/src/value/argtypes.rs b/minijinja/src/value/argtypes.rs
index 586ca712..cfc814e0 100644
--- a/minijinja/src/value/argtypes.rs
+++ b/minijinja/src/value/argtypes.rs
@@ -548,7 +548,7 @@ impl<'a, T: ArgType<'a>> ArgType<'a> for Option<T> {
     fn from_value(value: Option<&'a Value>) -> Result<Self::Output, Error> {
         match value {
             Some(value) => {
-                if value.is_undefined() {
+                if value.is_undefined() || value.is_none() {
                     Ok(None)
                 } else {
                     T::from_value(Some(value)).map(Some)
@@ -1008,4 +1008,12 @@ mod tests {
         assert_eq!(kwargs.get::<Value>("foo").unwrap(), Value::from(1));
         assert_eq!(kwargs.get::<Value>("bar").unwrap(), Value::from(2));
     }
+
+    #[test]
+    fn test_optional_none() {
+        let (one,) = from_args::<(Option<i32>,)>(args!(None::<i32>)).unwrap();
+        assert!(one.is_none());
+        let (one,) = from_args::<(Option<i32>,)>(args!(Some(Value::UNDEFINED))).unwrap();
+        assert!(one.is_none());
+    }
 }


Test patch:
diff --git a/minijinja/tests/inputs/filters.txt b/minijinja/tests/inputs/filters.txt
index dd67b16e..cdf46af6 100644
--- a/minijinja/tests/inputs/filters.txt
+++ b/minijinja/tests/inputs/filters.txt
@@ -96,6 +96,7 @@ select-attr: {{ [dict(active=true, key=1), dict(active=false, key=2)]|selectattr
 reject-attr: {{ [dict(active=true, key=1), dict(active=false, key=2)]|rejectattr("key", "even") }}
 map-maps: {{ [-1, -2, 3, 4, -5]|map("abs") }}
 map-attr: {{ [dict(a=1), dict(a=2), {}]|map(attribute='a', default=None) }}
+map-attr-undefined: {{ [dict(a=1), dict(a=2), {}]|map(attribute='a', default=definitely_undefined) }}
 map-attr-deep: {{ [dict(a=[1]), dict(a=[2]), dict(a=[])]|map(attribute='a.0', default=None) }}
 map-attr-int: {{ [[1], [1, 2]]|map(attribute=1, default=999) }}
 attr-filter: {{ map|attr("a") }}
diff --git a/minijinja/tests/snapshots/test_templates__vm@filters.txt.snap b/minijinja/tests/snapshots/test_templates__vm@filters.txt.snap
index 246dcd52..24f4ac66 100644
--- a/minijinja/tests/snapshots/test_templates__vm@filters.txt.snap
+++ b/minijinja/tests/snapshots/test_templates__vm@filters.txt.snap
@@ -1,6 +1,6 @@
 ---
 source: minijinja/tests/test_templates.rs
-description: "lower: {{ word|lower }}\nupper: {{ word|upper }}\ntitle: {{ word|title }}\ntitle-sentence: {{ \"the bIrd, is The:word\"|title }}\ntitle-three-words: {{ three_words|title }}\ncapitalize: {{ word|capitalize }}\ncapitalize-three-words: {{ three_words|capitalize }}\nreplace: {{ word|replace(\"B\", \"th\") }}\nescape: {{ \"<\"|escape }}\ne: {{ \"<\"|e }}\ndouble-escape: {{ \"<\"|escape|escape }}\nsafe: {{ \"<\"|safe|escape }}\nlist-length: {{ list|length }}\nlist-from-list: {{ list|list }}\nlist-from-map: {{ map|list }}\nlist-from-word: {{ word|list }}\nlist-from-undefined: {{ undefined|list }}\nbool-empty-string: {{ \"\"|bool }}\nbool-non-empty-string: {{ \"hello\"|bool }}\nbool-empty-list: {{ []|bool }}\nbool-non-empty-list: {{ [42]|bool }}\nbool-undefined: {{ undefined|bool }}\nmap-length: {{ map|length }}\nstring-length: {{ word|length }}\nstring-count: {{ word|count }}\nreverse-list: {{ list|reverse }}\nreverse-string: {{ word|reverse }}\ntrim: |{{ word_with_spaces|trim }}|\ntrim-bird: {{ word|trim(\"Bd\") }}\njoin-default: {{ list|join }}\njoin-pipe: {{ list|join(\"|\") }}\njoin_string: {{ word|join('-') }}\ndefault: {{ undefined|default == \"\" }}\ndefault-value: {{ undefined|default(42) }}\nfirst-list: {{ list|first }}\nfirst-word: {{ word|first }}\nfirst-undefined: {{ []|first is undefined }}\nlast-list: {{ list|last }}\nlast-word: {{ word|last }}\nlast-undefined: {{ []|first is undefined }}\nmin: {{ other_list|min }}\nmax: {{ other_list|max }}\nsort: {{ other_list|sort }}\nsort-reverse: {{ other_list|sort(reverse=true) }}\nsort-case-insensitive: {{ [\"B\", \"a\", \"C\", \"z\"]|sort }}\nsort-case-sensitive: {{ [\"B\", \"a\", \"C\", \"z\"]|sort(case_sensitive=true) }}\nsort-case-insensitive-mixed: {{ [0, 1, \"true\", \"false\", \"True\", \"False\", true, false]|sort }}\nsort-case-sensitive-mixed: {{ [0, 1, \"true\", \"false\", \"True\", \"False\", true, false]|sort(case_sensitive=true) }}\nsort-attribute {{ objects|sort(attribute=\"name\") }}\nd: {{ undefined|d == \"\" }}\njson: {{ map|tojson }}\njson-pretty: {{ map|tojson(true) }}\njson-scary-html: {{ scary_html|tojson }}\nurlencode: {{ \"hello world/foo-bar_baz.txt\"|urlencode }}\nurlencode-kv: {{ dict(a=\"x y\", b=2, c=3)|urlencode }}\nbatch: {{ range(10)|batch(3) }}\nbatch-fill: {{ range(10)|batch(3, '-') }}\nslice: {{ range(10)|slice(3) }}\nslice-fill: {{ range(10)|slice(3, '-') }}\nitems: {{ dict(a=1)|items }}\nindent: {{ \"foo\\nbar\\nbaz\"|indent(2)|tojson }}\nindent-first-line: {{ \"foo\\nbar\\nbaz\"|indent(2, true)|tojson }}\nint-abs: {{ -42|abs }}\nfloat-abs: {{ -42.5|abs }}\nint-round: {{ 42|round }}\nfloat-round: {{ 42.5|round }}\nfloat-round-prec2: {{ 42.512345|round(2) }}\nselect-odd: {{ [1, 2, 3, 4, 5, 6]|select(\"odd\") }}\nselect-truthy: {{ [undefined, null, 0, 42, 23, \"\", \"aha\"]|select }}\nreject-truthy: {{ [undefined, null, 0, 42, 23, \"\", \"aha\"]|reject }}\nreject-odd: {{ [1, 2, 3, 4, 5, 6]|reject(\"odd\") }}\nselect-attr: {{ [dict(active=true, key=1), dict(active=false, key=2)]|selectattr(\"active\") }}\nreject-attr: {{ [dict(active=true, key=1), dict(active=false, key=2)]|rejectattr(\"active\") }}\nselect-attr: {{ [dict(active=true, key=1), dict(active=false, key=2)]|selectattr(\"key\", \"even\") }}\nreject-attr: {{ [dict(active=true, key=1), dict(active=false, key=2)]|rejectattr(\"key\", \"even\") }}\nmap-maps: {{ [-1, -2, 3, 4, -5]|map(\"abs\") }}\nmap-attr: {{ [dict(a=1), dict(a=2), {}]|map(attribute='a', default=None) }}\nmap-attr-deep: {{ [dict(a=[1]), dict(a=[2]), dict(a=[])]|map(attribute='a.0', default=None) }}\nmap-attr-int: {{ [[1], [1, 2]]|map(attribute=1, default=999) }}\nattr-filter: {{ map|attr(\"a\") }}\nunique-filter: {{ [1, 1, 1, 4, 3, 0, 0, 5]|unique }}\npprint-filter: {{ objects|pprint }}"
+description: "lower: {{ word|lower }}\nupper: {{ word|upper }}\ntitle: {{ word|title }}\ntitle-sentence: {{ \"the bIrd, is The:word\"|title }}\ntitle-three-words: {{ three_words|title }}\ncapitalize: {{ word|capitalize }}\ncapitalize-three-words: {{ three_words|capitalize }}\nreplace: {{ word|replace(\"B\", \"th\") }}\nescape: {{ \"<\"|escape }}\ne: {{ \"<\"|e }}\ndouble-escape: {{ \"<\"|escape|escape }}\nsafe: {{ \"<\"|safe|escape }}\nlist-length: {{ list|length }}\nlist-from-list: {{ list|list }}\nlist-from-map: {{ map|list }}\nlist-from-word: {{ word|list }}\nlist-from-undefined: {{ undefined|list }}\nbool-empty-string: {{ \"\"|bool }}\nbool-non-empty-string: {{ \"hello\"|bool }}\nbool-empty-list: {{ []|bool }}\nbool-non-empty-list: {{ [42]|bool }}\nbool-undefined: {{ undefined|bool }}\nmap-length: {{ map|length }}\nstring-length: {{ word|length }}\nstring-count: {{ word|count }}\nreverse-list: {{ list|reverse }}\nreverse-string: {{ word|reverse }}\ntrim: |{{ word_with_spaces|trim }}|\ntrim-bird: {{ word|trim(\"Bd\") }}\njoin-default: {{ list|join }}\njoin-pipe: {{ list|join(\"|\") }}\njoin_string: {{ word|join('-') }}\ndefault: {{ undefined|default == \"\" }}\ndefault-value: {{ undefined|default(42) }}\nfirst-list: {{ list|first }}\nfirst-word: {{ word|first }}\nfirst-undefined: {{ []|first is undefined }}\nlast-list: {{ list|last }}\nlast-word: {{ word|last }}\nlast-undefined: {{ []|first is undefined }}\nmin: {{ other_list|min }}\nmax: {{ other_list|max }}\nsort: {{ other_list|sort }}\nsort-reverse: {{ other_list|sort(reverse=true) }}\nsort-case-insensitive: {{ [\"B\", \"a\", \"C\", \"z\"]|sort }}\nsort-case-sensitive: {{ [\"B\", \"a\", \"C\", \"z\"]|sort(case_sensitive=true) }}\nsort-case-insensitive-mixed: {{ [0, 1, \"true\", \"false\", \"True\", \"False\", true, false]|sort }}\nsort-case-sensitive-mixed: {{ [0, 1, \"true\", \"false\", \"True\", \"False\", true, false]|sort(case_sensitive=true) }}\nsort-attribute {{ objects|sort(attribute=\"name\") }}\nd: {{ undefined|d == \"\" }}\njson: {{ map|tojson }}\njson-pretty: {{ map|tojson(true) }}\njson-scary-html: {{ scary_html|tojson }}\nurlencode: {{ \"hello world/foo-bar_baz.txt\"|urlencode }}\nurlencode-kv: {{ dict(a=\"x y\", b=2, c=3)|urlencode }}\nbatch: {{ range(10)|batch(3) }}\nbatch-fill: {{ range(10)|batch(3, '-') }}\nslice: {{ range(10)|slice(3) }}\nslice-fill: {{ range(10)|slice(3, '-') }}\nitems: {{ dict(a=1)|items }}\nindent: {{ \"foo\\nbar\\nbaz\"|indent(2)|tojson }}\nindent-first-line: {{ \"foo\\nbar\\nbaz\"|indent(2, true)|tojson }}\nint-abs: {{ -42|abs }}\nfloat-abs: {{ -42.5|abs }}\nint-round: {{ 42|round }}\nfloat-round: {{ 42.5|round }}\nfloat-round-prec2: {{ 42.512345|round(2) }}\nselect-odd: {{ [1, 2, 3, 4, 5, 6]|select(\"odd\") }}\nselect-truthy: {{ [undefined, null, 0, 42, 23, \"\", \"aha\"]|select }}\nreject-truthy: {{ [undefined, null, 0, 42, 23, \"\", \"aha\"]|reject }}\nreject-odd: {{ [1, 2, 3, 4, 5, 6]|reject(\"odd\") }}\nselect-attr: {{ [dict(active=true, key=1), dict(active=false, key=2)]|selectattr(\"active\") }}\nreject-attr: {{ [dict(active=true, key=1), dict(active=false, key=2)]|rejectattr(\"active\") }}\nselect-attr: {{ [dict(active=true, key=1), dict(active=false, key=2)]|selectattr(\"key\", \"even\") }}\nreject-attr: {{ [dict(active=true, key=1), dict(active=false, key=2)]|rejectattr(\"key\", \"even\") }}\nmap-maps: {{ [-1, -2, 3, 4, -5]|map(\"abs\") }}\nmap-attr: {{ [dict(a=1), dict(a=2), {}]|map(attribute='a', default=None) }}\nmap-attr-undefined: {{ [dict(a=1), dict(a=2), {}]|map(attribute='a', default=definitely_undefined) }}\nmap-attr-deep: {{ [dict(a=[1]), dict(a=[2]), dict(a=[])]|map(attribute='a.0', default=None) }}\nmap-attr-int: {{ [[1], [1, 2]]|map(attribute=1, default=999) }}\nattr-filter: {{ map|attr(\"a\") }}\nunique-filter: {{ [1, 1, 1, 4, 3, 0, 0, 5]|unique }}\npprint-filter: {{ objects|pprint }}"
 info:
   list:
     - 1
@@ -104,6 +104,7 @@ select-attr: [{"active": false, "key": 2}]
 reject-attr: [{"active": true, "key": 1}]
 map-maps: [1, 2, 3, 4, 5]
 map-attr: [1, 2, None]
+map-attr-undefined: [1, 2, Undefined]
 map-attr-deep: [1, 2, None]
 map-attr-int: [999, 2]
 attr-filter: b
diff --git a/minijinja/tests/test_filters.rs b/minijinja/tests/test_filters.rs
index 9ffcb6cb..ebdf5f6d 100644
--- a/minijinja/tests/test_filters.rs
+++ b/minijinja/tests/test_filters.rs
@@ -1,7 +1,35 @@
+use minijinja::value::Value;
+use minijinja::{args, Environment};
 use similar_asserts::assert_eq;
 
 use minijinja::filters::indent;
 
+#[test]
+fn test_filter_with_non() {
+    fn filter(value: Option<String>) -> String {
+        format!("[{}]", value.unwrap_or_default())
+    }
+
+    let mut env = Environment::new();
+    env.add_filter("filter", filter);
+    let state = env.empty_state();
+
+    let rv = state
+        .apply_filter("filter", args!(Value::UNDEFINED))
+        .unwrap();
+    assert_eq!(rv, Value::from("[]"));
+
+    let rv = state
+        .apply_filter("filter", args!(Value::from(())))
+        .unwrap();
+    assert_eq!(rv, Value::from("[]"));
+
+    let rv = state
+        .apply_filter("filter", args!(Value::from("wat")))
+        .unwrap();
+    assert_eq!(rv, Value::from("[wat]"));
+}
+
 #[test]
 fn test_indent_one_empty_line() {
     let teststring = String::from("\n");