Repository: microsoft/regorus (language: python)

Issue:
Preserve `false` values in single-expression queries
If any expression evaluates to false in a query, then regorus currently returns no results for the query.
E.g.: `1; true; false` does not produce any result. Upstream OPA also behaves the same.

However, if a query has only one expression, then upstream OPA returns the value as a result, even if the value is false.
Currently Regorus treats it the same as multi-valued query and returns no result. The behavior must be changed to match upstream OPA.


Also there is a distinction between `=` and `==`. The former is a binding operator and would evaluate to Undefined in case of mismatch between lhs and rhs. The latter evaluates to either true or false.
Thus `1 = 2` must not produce results whereas `1 == 2` must produce results.

Note that `1 == 2; true` must not produce results since it is a multi-expression query.




Discussion / hints:

Fix patch:
diff --git a/src/engine.rs b/src/engine.rs
index 2de5af94..702a5195 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 8bf7cb2d..8d33b890 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 3d91ff89..493e21bc 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::*;


Test patch:
diff --git a/src/tests/interpreter/mod.rs b/src/tests/interpreter/mod.rs
index dcecd9b3..bd2d2d97 100644
--- a/src/tests/interpreter/mod.rs
+++ b/src/tests/interpreter/mod.rs
@@ -234,6 +234,7 @@ struct TestCase {
     query: String,
     sort_bindings: Option<bool>,
     want_result: Option<ValueOrVec>,
+    no_result: Option<bool>,
     skip: Option<bool>,
     error: Option<String>,
     traces: Option<bool>,
@@ -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 00000000..4bc42156
--- /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