Repository: microsoft/regorus (language: python)

Issue:
`or` keyword
# Problem
Currently, there is no straight forward way to describe logical or in Rego. 
To express the condition A or B, helper rules or else clauses must be used.
E.g:

```rego
 effect := parameters.effect if {
    resource.type == "Microsoft.Storage/storageaccounts"
    resource.properties.networkAcls.defaultAction == "Deny"
}
 
effect := parameters.effect if {
    resource.type == "Microsoft.Storage/storageaccounts"
    count(resource.properties.networkAcls.ipRules) >= 1
}
```
or 
```rego
 resource_has_violation
   if resource.properties.networkAcls.defaultAction == "Deny"
   else if count(resource.properties.networkAcls.ipRules) >= 1
 
effect := parameters.effect if {
   resource.type == "Microsoft.Storage/storageaccounts"
   resource_has_violation
}
```

An `or` operator makes it easier to express the above policy:
```rego
effect := parameters.effect if { 
    resource.type == "Microsoft.Storage/storageaccounts"
    resource.properties.networkAcls.defaultAction == "Deny"  
         or count(resource.properties.networkAcls.ipRules) >= 1
}
```

The blog post https://www.styra.com/blog/how-to-express-or-in-rego/ describes various ways to express logical-or in Rego.
However, having first-class support makes expressing policies easier as seen above.
 
There is also a [backlog issue](https://github.com/open-policy-agent/opa/issues/2345) in OPA repository to add support for an operator to express logical or.


# Semantics

The `or` keyword will
- evaluate lhs first
- if lhs is not `false`, `null` or `undefined` then lhs value is returned. Rhs is not evaluated.
- Otherwise the value of rhs is returned.

Grammar changes
- `or` expression will have least precedence.
- `or` expression is left associative

Feature changes
- Available with `rego-extensions` cargo feature.


Discussion / hints:

Fix patch:
diff --git a/.github/workflows/pr-extensions.yml b/.github/workflows/pr-extensions.yml
new file mode 100644
index 00000000..c13b568a
--- /dev/null
+++ b/.github/workflows/pr-extensions.yml
@@ -0,0 +1,33 @@
+name: tests/release
+
+on:
+  push:
+    branches: [ "main" ]
+  pull_request:
+    branches: [ "main" ]
+
+env:
+  CARGO_TERM_COLOR: always
+
+jobs:
+  test:
+
+    runs-on: ubuntu-latest
+
+    steps:
+    - uses: actions/checkout@v4
+    - name: Build only std
+      run: cargo build -r --example regorus --no-default-features --features "std,rego-extensions"
+    - name: Doc Tests
+      run: cargo test -r --doc --features rego-extensions
+    - name: Run tests
+      run: cargo test -r --features rego-extensions
+    - name: Run example
+      run: cargo run --example regorus --features rego-extensions -- eval -d examples/server/allowed_server.rego -i examples/server/input.json data.example
+    - name: Run tests (ACI)
+      run: cargo test -r --test aci --features rego-extensions
+    - name: Run tests (KATA)
+      run: cargo test -r --test kata --features rego-extensions
+    - name: Run tests (OPA Conformance)
+      run: >-
+        cargo test -r --test opa --features opa-testutil,serde_json/arbitrary_precision,rego-extensions  -- $(tr '\n' ' ' < tests/opa.passing)
diff --git a/Cargo.toml b/Cargo.toml
index 8743e0de..d5db9008 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -67,7 +67,9 @@ full-opa = [
     "time",
     "uuid",
     "urlquery",
-    "yaml"
+    "yaml",
+
+    #"rego-extensions"
 ]
 
 # Features that can be used in no_std environments.
@@ -89,6 +91,9 @@ opa-no-std = [
   "lazy_static/spin_no_std"
 ]
 
+# Rego language extensions
+rego-extensions = []
+
 # This feature enables some testing utils for OPA tests.
 opa-testutil = []
 rand = ["dep:rand"]
diff --git a/docs/grammar.md b/docs/grammar.md
index c6f49de2..9056dde1 100644
--- a/docs/grammar.md
+++ b/docs/grammar.md
@@ -61,9 +61,9 @@ in-expr          ::= in-expr 'in' bool-expr
 bool-expr        ::= bool-expr bool-op or-expr
                  | or-expr
 bool-op          ::= '<' | '<=' | '==' | '>=' | '>' | '!='
-or-expr          ::= or-expr '|' and-expr
-                 | and-expr
-and-expr         ::= and-expr '&' arith-expr
+set-union-expr   ::= set-union-expr '|' set-intersection-expr
+                 | set-intersection-expr
+set-intersection-expr ::= set-intersection-expr '&' arith-expr
                  | arith-expr
 arith-expr       ::= arith-expr ('+' | '-') mul-div-expr
                  | mul-div-expr
diff --git a/scripts/pre-push b/scripts/pre-push
index d57f1309..3488b66d 100755
--- a/scripts/pre-push
+++ b/scripts/pre-push
@@ -30,6 +30,11 @@ if [ -f Cargo.toml ]; then
    cargo test -r --test aci
    cargo test -r --test kata
 
+   # Ensure that all tests pass with extensions
+   cargo test -r --features rego-extensions
+   cargo test -r --test aci rego-extensions
+   cargo test -r --test kata rego-extensions
+
    # Ensure that OPA conformance tests don't regress.
-   cargo test -r --features opa-testutil,serde_json/arbitrary_precision --test opa -- $(tr '\n' ' ' < tests/opa.passing)
+   cargo test -r --features opa-testutil,serde_json/arbitrary_precision,rego-extensions --test opa -- $(tr '\n' ' ' < tests/opa.passing)
 fi
diff --git a/src/ast.rs b/src/ast.rs
index 139bc0a4..394b0d75 100644
--- a/src/ast.rs
+++ b/src/ast.rs
@@ -10,8 +10,8 @@ use core::{cmp, fmt, ops::Deref};
 #[derive(Debug, PartialEq, Eq, Clone)]
 #[cfg_attr(feature = "ast", derive(serde::Serialize))]
 pub enum BinOp {
-    And,
-    Or,
+    Intersection,
+    Union,
 }
 
 #[derive(Debug, PartialEq, Eq, Clone)]
@@ -209,6 +209,13 @@ pub enum Expr {
         value: Ref<Expr>,
         collection: Ref<Expr>,
     },
+
+    #[cfg(feature = "rego-extensions")]
+    OrExpr {
+        span: Span,
+        lhs: Ref<Expr>,
+        rhs: Ref<Expr>,
+    },
 }
 
 impl Expr {
@@ -232,6 +239,8 @@ impl Expr {
             | ArithExpr { span, .. }
             | AssignExpr { span, .. }
             | Membership { span, .. } => span,
+            #[cfg(feature = "rego-extensions")]
+            OrExpr { span, .. } => span,
         }
     }
 }
diff --git a/src/interpreter.rs b/src/interpreter.rs
index 6031757c..580d07b1 100644
--- a/src/interpreter.rs
+++ b/src/interpreter.rs
@@ -484,6 +484,12 @@ impl Interpreter {
                 self.hoist_loops_impl(rhs, loops);
             }
 
+            #[cfg(feature = "rego-extensions")]
+            OrExpr { lhs, rhs, .. } => {
+                self.hoist_loops_impl(lhs, loops);
+                self.hoist_loops_impl(rhs, loops);
+            }
+
             Membership {
                 key,
                 value,
@@ -554,8 +560,8 @@ impl Interpreter {
         }
 
         match op {
-            BinOp::Or => builtins::sets::union(lhs, rhs, lhs_value, rhs_value),
-            BinOp::And => builtins::sets::intersection(lhs, rhs, lhs_value, rhs_value),
+            BinOp::Union => builtins::sets::union(lhs, rhs, lhs_value, rhs_value),
+            BinOp::Intersection => builtins::sets::intersection(lhs, rhs, lhs_value, rhs_value),
         }
     }
 
@@ -2831,6 +2837,15 @@ impl Interpreter {
                 ..
             } => self.eval_membership(key, value, collection),
 
+            #[cfg(feature = "rego-extensions")]
+            Expr::OrExpr { lhs, rhs, .. } => {
+                let lhs = self.eval_expr(lhs)?;
+                match lhs {
+                    Value::Bool(false) | Value::Null | Value::Undefined => self.eval_expr(rhs),
+                    _ => Ok(lhs),
+                }
+            }
+
             // Creation expression
             Expr::Array { items, .. } => self.eval_array(items),
             Expr::Object { fields, .. } => self.eval_object(fields),
@@ -3126,6 +3141,8 @@ impl Interpreter {
             ArithExpr { span, .. } => ("arithexpr", span),
             AssignExpr { span, .. } => ("assignexpr", span),
             Membership { span, .. } => ("membership", span),
+            #[cfg(feature = "rego-extensions")]
+            OrExpr { span, .. } => ("orexpr", span),
         };
 
         Err(span.error(format!("invalid `{kind}` in default value").as_str()))
diff --git a/src/parser.rs b/src/parser.rs
index 0c782602..5d73df0c 100644
--- a/src/parser.rs
+++ b/src/parser.rs
@@ -499,7 +499,7 @@ impl<'source> Parser<'source> {
 
     fn parse_parens_expr(&mut self) -> Result<Expr> {
         self.next_token()?;
-        let expr = self.parse_membership_expr()?;
+        let expr = self.parse_expr()?;
         self.expect(")", "while parsing parenthesized expression")?;
         //TODO: if needed introduce a parens-expr node or adjust expr's span.
         Ok(expr)
@@ -700,7 +700,7 @@ impl<'source> Parser<'source> {
         }
     }
 
-    fn parse_and_expr(&mut self) -> Result<Expr> {
+    fn parse_set_intersection_expr(&mut self) -> Result<Expr> {
         let start = self.tok.1.start;
         let mut expr = self.parse_arith_expr()?;
 
@@ -712,7 +712,7 @@ impl<'source> Parser<'source> {
             span.end = self.end;
             expr = Expr::BinExpr {
                 span,
-                op: BinOp::And,
+                op: BinOp::Intersection,
                 lhs: Ref::new(expr),
                 rhs: Ref::new(right),
             };
@@ -720,19 +720,19 @@ impl<'source> Parser<'source> {
         Ok(expr)
     }
 
-    fn parse_or_expr(&mut self) -> Result<Expr> {
+    fn parse_set_union_expr(&mut self) -> Result<Expr> {
         let start = self.tok.1.start;
-        let mut expr = self.parse_and_expr()?;
+        let mut expr = self.parse_set_intersection_expr()?;
 
         while self.token_text() == "|" {
             let mut span = self.tok.1.clone();
             span.start = start;
             self.next_token()?;
-            let right = self.parse_and_expr()?;
+            let right = self.parse_set_intersection_expr()?;
             span.end = self.end;
             expr = Expr::BinExpr {
                 span,
-                op: BinOp::Or,
+                op: BinOp::Union,
                 lhs: Ref::new(expr),
                 rhs: Ref::new(right),
             };
@@ -742,7 +742,7 @@ impl<'source> Parser<'source> {
 
     fn parse_bool_expr(&mut self) -> Result<Expr> {
         let start = self.tok.1.start;
-        let mut expr = self.parse_or_expr()?;
+        let mut expr = self.parse_set_union_expr()?;
         loop {
             let mut span = self.tok.1.clone();
             span.start = start;
@@ -756,7 +756,7 @@ impl<'source> Parser<'source> {
                 _ => break,
             };
             self.next_token()?;
-            let right = self.parse_or_expr()?;
+            let right = self.parse_set_union_expr()?;
             span.end = self.end;
             expr = Expr::BoolExpr {
                 span,
@@ -811,6 +811,32 @@ impl<'source> Parser<'source> {
         Ok(expr)
     }
 
+    pub fn parse_expr(&mut self) -> Result<Expr> {
+        #[cfg(feature = "rego-extensions")]
+        return self.parse_or_expr();
+
+        #[cfg(not(feature = "rego-extensions"))]
+        return self.parse_membership_expr();
+    }
+
+    #[cfg(feature = "rego-extensions")]
+    pub fn parse_or_expr(&mut self) -> Result<Expr> {
+        let start = self.tok.1.start;
+        let mut expr = self.parse_membership_expr()?;
+        while self.token_text() == "or" {
+            let mut span = self.tok.1.clone();
+            span.start = start;
+            self.next_token()?;
+            let rhs = self.parse_membership_expr()?;
+            expr = Expr::OrExpr {
+                span,
+                lhs: Ref::new(expr),
+                rhs: Ref::new(rhs),
+            };
+        }
+        Ok(expr)
+    }
+
     pub fn parse_membership_expr(&mut self) -> Result<Expr> {
         let start = self.tok.1.start;
         let mut expr = self.parse_bool_expr()?;
@@ -851,12 +877,12 @@ impl<'source> Parser<'source> {
             ":=" => AssignOp::ColEq,
             _ => {
                 *self = state;
-                return self.parse_membership_expr();
+                return self.parse_expr();
             }
         };
 
         self.next_token()?;
-        let right = self.parse_membership_expr()?;
+        let right = self.parse_expr()?;
         span.end = self.end;
         Ok(Expr::AssignExpr {
             span,
@@ -1048,7 +1074,13 @@ impl<'source> Parser<'source> {
                 // Treat { 1 | 1 } as a comprehension instead of a
                 // set of 1 element.
                 if let Literal::Expr { expr: e, .. } = &stmt.literal {
-                    if matches!(e.as_ref(), Expr::BinExpr { op: BinOp::Or, .. }) {
+                    if matches!(
+                        e.as_ref(),
+                        Expr::BinExpr {
+                            op: BinOp::Union,
+                            ..
+                        }
+                    ) {
                         *self = state;
                         bail!("try parse as comprehension");
                     }
@@ -1116,7 +1148,7 @@ impl<'source> Parser<'source> {
             _ => return Ok(None),
         };
 
-        let expr = Ref::new(self.parse_membership_expr()?);
+        let expr = Ref::new(self.parse_expr()?);
         span.end = self.end;
         Ok(Some(RuleAssign {
             span,
@@ -1264,7 +1296,7 @@ impl<'source> Parser<'source> {
                 }
                 "[" => {
                     self.next_token()?;
-                    let index = self.parse_membership_expr()?;
+                    let index = self.parse_expr()?;
                     span.end = self.end;
                     self.expect("]", "while parsing bracketed reference")?;
                     term = Expr::RefBrack {
@@ -1312,7 +1344,7 @@ impl<'source> Parser<'source> {
             }
             "contains" => {
                 self.next_token()?;
-                let key = Ref::new(self.parse_membership_expr()?);
+                let key = Ref::new(self.parse_expr()?);
                 span.end = self.end;
                 Ok(RuleHead::Set {
                     span,
diff --git a/src/scheduler.rs b/src/scheduler.rs
index 17815424..2893afe9 100644
--- a/src/scheduler.rs
+++ b/src/scheduler.rs
@@ -258,6 +258,12 @@ pub fn traverse(expr: &Ref<Expr>, f: &mut dyn FnMut(&Ref<Expr>) -> Result<bool>)
             traverse(rhs, f)?;
         }
 
+        #[cfg(feature = "rego-extensions")]
+        OrExpr { lhs, rhs, .. } => {
+            traverse(lhs, f)?;
+            traverse(rhs, f)?;
+        }
+
         Membership {
             key,
             value,


Test patch:
diff --git a/src/tests/interpreter/mod.rs b/src/tests/interpreter/mod.rs
index 4d71fa27..2abe00e4 100644
--- a/src/tests/interpreter/mod.rs
+++ b/src/tests/interpreter/mod.rs
@@ -385,6 +385,11 @@ fn yaml_test_impl(file: &str) -> Result<()> {
 }
 
 fn yaml_test(file: &str) -> Result<()> {
+    #[cfg(not(feature = "rego-extensions"))]
+    if file.contains("rego-extensions") {
+        return Ok(());
+    }
+
     match yaml_test_impl(file) {
         Ok(_) => Ok(()),
         Err(e) => {
diff --git a/tests/interpreter/cases/rego-extensions/or/tests.yaml b/tests/interpreter/cases/rego-extensions/or/tests.yaml
new file mode 100644
index 00000000..b0ca93e0
--- /dev/null
+++ b/tests/interpreter/cases/rego-extensions/or/tests.yaml
@@ -0,0 +1,52 @@
+# Copyright (c) Microsoft Corporation.
+# Licensed under the MIT License.
+cases:
+  - note: basic
+    modules:
+      - |
+        package test
+        import rego.v1
+
+        x := data.foo or 2 # undefined lhs
+        y := false or 3 # false rhs
+        z := null or 4
+        a := data.foo or false or null or 5
+        b := startswith("a", "b") or startswith("a", "a")
+        c := 5 in [1,2] or 6 in [6]
+        d := x if {
+           x := false or [1, 2][_]
+           x > 1
+        }
+        e if 1 > 2 or false
+    query: data.test
+    want_result:
+      x: 2
+      y: 3
+      z: 4
+      a: 5
+      b: true
+      c: true
+      d: 2
+  - note: Azure Policy
+    modules:
+      - |
+        package policy
+
+        effect := parameters.effect if {
+          resource.type == "Microsoft.Storage/storageaccounts"
+          resource.properties.networkAcls.defaultAction == "Deny" 
+            or count(resource.properties.networkAcls.ipRules) >= 1
+        }
+
+        resource := input.resource
+        parameters := input.parameters
+    input:
+      resource:
+        type: "Microsoft.Storage/storageaccounts"
+        properties:
+        networksAcls:
+          ipRules: ["rule1", "rule2"]
+      parameters:
+        effect: "Deny"
+    query: data.policy.effect
+    want_result: "Deny"
diff --git a/tests/parser/mod.rs b/tests/parser/mod.rs
index 301f411a..fadf2d64 100644
--- a/tests/parser/mod.rs
+++ b/tests/parser/mod.rs
@@ -198,6 +198,13 @@ fn match_expr_impl(e: &Expr, v: &Value) -> Result<()> {
             match_expr(value, &v["inexpr"]["value"])?;
             match_expr(collection, &v["inexpr"]["collection"])
         }
+
+        #[cfg(feature = "rego-extensions")]
+        Expr::OrExpr { span, lhs, rhs } => {
+            match_span_opt(span, &v["orexpr"]["span"])?;
+            match_expr(lhs, &v["orexpr"]["lhs"])?;
+            match_expr(rhs, &v["orexpr"]["rhs"])
+        }
     }
 }
 
@@ -324,8 +331,8 @@ fn match_expr_opt(s: &Span, e: &Option<Ref<Expr>>, v: &Value) -> Result<()> {
 
 fn match_bin_op(s: &Span, op: &BinOp, v: &Value) -> Result<()> {
     match (op, v) {
-        (BinOp::And, Value::String(s)) if s.as_ref() == "&" => Ok(()),
-        (BinOp::Or, Value::String(s)) if s.as_ref() == "|" => Ok(()),
+        (BinOp::Intersection, Value::String(s)) if s.as_ref() == "&" => Ok(()),
+        (BinOp::Union, Value::String(s)) if s.as_ref() == "|" => Ok(()),
         _ => bail!(
             "{}",
             s.source.message(