From 8d282f1ffd631ee0cab406e1857ce672e07a084b Mon Sep 17 00:00:00 2001 From: Anand Krishnamoorthi <35780660+anakrish@users.noreply.github.com> Date: Fri, 16 Feb 2024 06:11:07 -0800 Subject: [PATCH] Preserve false in single-expression queries (#145) Note: 1 = 2 is different from 1 == 2 See issue for details fixes #144 Signed-off-by: Anand Krishnamoorthi --- src/engine.rs | 2 +- src/interpreter.rs | 26 ++++++++++----- src/lib.rs | 42 ++++++++++++++++++++++-- src/tests/interpreter/mod.rs | 7 +++- tests/interpreter/cases/query/tests.yaml | 26 +++++++++++++++ 5 files changed, 90 insertions(+), 13 deletions(-) create mode 100644 tests/interpreter/cases/query/tests.yaml diff --git a/src/engine.rs b/src/engine.rs index 2de5af9..702a519 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -233,7 +233,7 @@ impl Engine { /// // Load input and make query. /// engine.set_input(Value::new_object()); /// let results = engine.eval_query("data.framework.mount_overlay.allowed".to_string(), false)?; - /// assert!(results.result.is_empty()); + /// assert_eq!(results.result[0].expressions[0].value, Value::from(false)); /// /// // Evaluate query with different inputs. /// engine.set_input(Value::from_json_file("tests/aci/input.json")?); diff --git a/src/interpreter.rs b/src/interpreter.rs index 8bf7cb2..8d33b89 100644 --- a/src/interpreter.rs +++ b/src/interpreter.rs @@ -661,7 +661,13 @@ impl Interpreter { .map(Value::Bool); } // Treat the assignment as comparison if neither lhs nor rhs is a variable - _ => return self.eval_bool_expr(&BoolOp::Eq, lhs, rhs), + _ => { + let r = self.eval_bool_expr(&BoolOp::Eq, lhs, rhs)?; + if r == Value::Bool(false) { + return Ok(Value::Undefined); + } + return Ok(r); + } } } AssignOp::ColEq => { @@ -1702,11 +1708,12 @@ impl Interpreter { .insert(Value::String(name.to_string().into()), value.clone()); } } - if result - .expressions - .iter() - .all(|v| v.value != Value::Undefined && v.value != Value::Bool(false)) - && !result.expressions.is_empty() + if result.expressions.len() == 1 // Single expression query + || result // Multi expression query where no value is false + .expressions + .iter() + .all(|v| v.value != Value::Undefined && v.value != Value::Bool(false)) + && !result.expressions.is_empty() { ctx.results.result.push(result); } @@ -1821,7 +1828,9 @@ impl Interpreter { .insert(Value::String(name.to_string().into()), value.clone()); } } - if result + + if result.expressions.len() == 1 // Single expression query + || result // Multi expression query where no value is false .expressions .iter() .all(|v| v.value != Value::Undefined && v.value != Value::Bool(false)) @@ -3242,7 +3251,8 @@ impl Interpreter { if let Some(r) = results.result.last() { if matches!(&r.bindings, Value::Object(obj) if obj.is_empty()) - && r.expressions.iter().any(|e| e.value == Value::Bool(false)) + && (r.expressions.len() > 1 + && r.expressions.iter().any(|e| e.value == Value::Bool(false))) { results = QueryResults::default(); } diff --git a/src/lib.rs b/src/lib.rs index 3d91ff8..493e21b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -186,17 +186,33 @@ impl Default for QueryResult { /// ``` /// # use regorus::*; /// # fn main() -> anyhow::Result<()> { -/// // Create engine and evaluate "true; true; false". +/// // Create engine and evaluate "1 + 1". /// let results = Engine::new().eval_query("1 + 1".to_string(), false)?; /// -/// assert!(results.result.len() == 1); +/// assert_eq!(results.result.len(), 1); /// assert_eq!(results.result[0].expressions[0].value, Value::from(2u64)); /// assert_eq!(results.result[0].expressions[0].text.as_ref(), "1 + 1"); /// # Ok(()) /// # } /// ``` /// -/// If any expression evaluates to false, then no results are produced. +/// If a query contains only one expression, and even if the expression evaluates +/// to false, the value will be returned. +/// ``` +/// # use regorus::*; +/// # fn main() -> anyhow::Result<()> { +/// // Create engine and evaluate "1 > 2" which is false. +/// let results = Engine::new().eval_query("1 > 2".to_string(), false)?; +/// +/// assert_eq!(results.result.len(), 1); +/// assert_eq!(results.result[0].expressions[0].value, Value::from(false)); +/// assert_eq!(results.result[0].expressions[0].text.as_ref(), "1 > 2"); +/// # Ok(()) +/// # } +/// ``` +/// +/// In a query containing multiple expressions, if any expression evaluates to false, +/// then no results are produced. /// ``` /// # use regorus::*; /// # fn main() -> anyhow::Result<()> { @@ -208,6 +224,26 @@ impl Default for QueryResult { /// # } /// ``` /// +/// Note that `=` is different from `==`. The former evaluates to undefined if the LHS and RHS +/// are not equal. The latter evaluates to either true or false. +/// ``` +/// # use regorus::*; +/// # fn main() -> anyhow::Result<()> { +/// // Create engine and evaluate "1 = 2" which is undefined and produces no resutl. +/// let results = Engine::new().eval_query("1 = 2".to_string(), false)?; +/// +/// assert_eq!(results.result.len(), 0); +/// +/// // Create engine and evaluate "1 == 2" which evaluates to false. +/// let results = Engine::new().eval_query("1 == 2".to_string(), false)?; +/// +/// assert_eq!(results.result.len(), 1); +/// assert_eq!(results.result[0].expressions[0].value, Value::from(false)); +/// assert_eq!(results.result[0].expressions[0].text.as_ref(), "1 == 2"); +/// # Ok(()) +/// # } +/// ``` +/// /// Queries containing loops produce multiple results. /// ``` /// # use regorus::*; diff --git a/src/tests/interpreter/mod.rs b/src/tests/interpreter/mod.rs index dcecd9b..bd2d2d9 100644 --- a/src/tests/interpreter/mod.rs +++ b/src/tests/interpreter/mod.rs @@ -234,6 +234,7 @@ struct TestCase { query: String, sort_bindings: Option, want_result: Option, + no_result: Option, skip: Option, error: Option, traces: Option, @@ -267,7 +268,10 @@ fn yaml_test_impl(file: &str) -> Result<()> { match (&case.want_result, &case.error) { (Some(_), None) | (None, Some(_)) => (), - _ => panic!("either want_result or error must be specified in test case."), + _ if case.no_result != Some(true) => { + panic!("either want_result, error or no_result must be specified in test case.") + } + _ => (), } let enable_tracing = case.traces.is_some() && case.traces.unwrap(); @@ -292,6 +296,7 @@ fn yaml_test_impl(file: &str) -> Result<()> { check_output(&results, &expected_results)?; } + _ if case.no_result == Some(true) => (), _ => bail!("eval succeeded and did not produce any errors"), }, Err(actual) => match &case.error { diff --git a/tests/interpreter/cases/query/tests.yaml b/tests/interpreter/cases/query/tests.yaml new file mode 100644 index 0000000..4bc4215 --- /dev/null +++ b/tests/interpreter/cases/query/tests.yaml @@ -0,0 +1,26 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. +cases: + - note: single-expression query producing false + data: {} + modules: [] + query: 1 == 2 + want_result: false + + - note: single-expression query producing no results (due to undefined) + data: {} + modules: [] + query: 1 = 2 + no_result: true + + - note: multi-expression query in which one expression is false (1) + data: {} + modules: [] + query: "1 == 1; 1 == 2" + no_result: true + + - note: multi-expression query in which one expression is false (2) + data: {} + modules: [] + query: "1 == 2; 1 == 1" + no_result: true