feat: support empty string for safe access operator (#1176)

* chore: apply pr #629 Co-authored-by: Roland Fredenhagen <[email protected]> * style: early return * feat: err on empty json string * docs: update docs * test: update test in accordance with decision * chore: attribution * docs: improve wording * docs: add breaking change notice to changelog * fix(changelog): fix pr link --------- Co-authored-by: Roland Fredenhagen <[email protected]>
elkowar · Oct 12, 2024 · 50ec181 · 50ec181
1 parent 8801fa3
commit 50ec181
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 6 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,10 @@ All notable changes to eww will be listed here, starting at changes since versio
 
 ## Unreleased
 
+### BREAKING CHANGES
+- [#1176](https://github.com/elkowar/eww/pull/1176) changed safe access (`?.`) behavior:
+  Attempting to index in an empty JSON string (`'""'`) is now an error.
+
 ### Fixes
 - Re-enable some scss features (By: w-lfchen)
 - Fix and refactor nix flake (By: w-lfchen)
@@ -23,6 +27,7 @@ All notable changes to eww will be listed here, starting at changes since versio
 - Add `flip-x`, `flip-y`, `vertical` options to the graph widget to determine its direction
 - Add `transform-origin-x`/`transform-origin-y` properties to transform widget (By: mario-kr)
 - Add keyboard support for button presses (By: julianschuler)
+- Support empty string for safe access operator (By: ModProg)
 
 ## [0.6.0] (21.04.2024)
 

diff --git a/crates/simplexpr/src/eval.rs b/crates/simplexpr/src/eval.rs
@@ -268,6 +268,10 @@ impl SimplExpr {
 
                 let is_safe = *safe == AccessType::Safe;
 
+                // Needs to be done first as `as_json_value` fails on empty string
+                if is_safe && val.as_string()?.is_empty() {
+                    return Ok(DynVal::from(&serde_json::Value::Null).at(*span));
+                }
                 match val.as_json_value()? {
                     serde_json::Value::Array(val) => {
                         let index = index.as_i32()?;
@@ -281,9 +285,6 @@ impl SimplExpr {
                             .unwrap_or(&serde_json::Value::Null);
                         Ok(DynVal::from(indexed_value).at(*span))
                     }
-                    serde_json::Value::String(val) if val.is_empty() && is_safe => {
-                        Ok(DynVal::from(&serde_json::Value::Null).at(*span))
-                    }
                     serde_json::Value::Null if is_safe => Ok(DynVal::from(&serde_json::Value::Null).at(*span)),
                     _ => Err(EvalError::CannotIndex(format!("{}", val)).at(*span)),
                 }
@@ -564,6 +565,8 @@ mod tests {
         string_to_string(r#""Hello""#) => Ok(DynVal::from("Hello".to_string())),
         safe_access_to_existing(r#"{ "a": { "b": 2 } }.a?.b"#) => Ok(DynVal::from(2)),
         safe_access_to_missing(r#"{ "a": { "b": 2 } }.b?.b"#) => Ok(DynVal::from(&serde_json::Value::Null)),
+        safe_access_to_empty(r#"""?.test"#) => Ok(DynVal::from(&serde_json::Value::Null)),
+        safe_access_to_empty_json_string(r#"'""'?.test"#) => Err(super::EvalError::CannotIndex("\"\"".to_string())),
         safe_access_index_to_existing(r#"[1, 2]?.[1]"#) => Ok(DynVal::from(2)),
         safe_access_index_to_missing(r#""null"?.[1]"#) => Ok(DynVal::from(&serde_json::Value::Null)),
         safe_access_index_to_non_indexable(r#"32?.[1]"#) => Err(super::EvalError::CannotIndex("32".to_string())),

diff --git a/docs/src/expression_language.md b/docs/src/expression_language.md
@@ -28,10 +28,10 @@ Supported currently are the following features:
     - if the left side is `""` or a JSON `null`, then returns the right side,
       otherwise evaluates to the left side.
 - Safe Access operator (`?.`) or (`?.[index]`)
-    - if the left side is `""` or a JSON `null`, then return `null`. Otherwise,
-      attempt to index.
+    - if the left side is an empty string or a JSON `null`, then return `null`. Otherwise,
+      attempt to index. Note that indexing an empty JSON string (`'""'`) is an error.
     - This can still cause an error to occur if the left hand side exists but is
-      not an object.
+      not an object or an array.
       (`Number` or `String`).
 - conditionals (`condition ? 'value' : 'other value'`)
 - numbers, strings, booleans and variable references (`12`, `'hi'`, `true`, `some_variable`)