From e61b4065476173046d64e1b2c4d6ccf8b0afe9eb Mon Sep 17 00:00:00 2001
From: Anand Krishnamoorthi <35780660+anakrish@users.noreply.github.com>
Date: Sat, 23 Dec 2023 11:55:25 -0800
Subject: [PATCH] OPA conformance (#71)

- Remove unnecessary memory allocations
- Add --non-strict flag
- Ensure that only empty modules (ones without rules) are initialzed prior to evaluating rules.
- Record rule as entry for each of its prefixes.
  For example, for a rule a.b.c =... in package test, record it in
  rules["data.test.a"], rules["data.test.a.b"] and rules["data.test.a.b.c"]

  This allows evaluating the correct list of rules based on expessions
  a.b.c, a.b, a, data.test.a.b.c, data.test.a.b, data.test.a

Closes #69
Closes #70

Signed-off-by: Anand Krishnamoorthi <anakrish@microsoft.com>
---
 examples/regorus.rs                     |  10 +-
 src/engine.rs                           |  16 ++-
 src/interpreter.rs                      | 127 ++++++++++++-------
 src/lexer.rs                            |   8 +-
 src/parser.rs                           | 154 ++++++++++++------------
 src/scheduler.rs                        |   4 +-
 src/utils.rs                            |   6 +-
 tests/interpreter/cases/data/tests.yaml |   6 +-
 tests/opa.passing                       |   4 +
 tests/opa.rs                            |   3 +-
 tests/parser/mod.rs                     |   2 +-
 11 files changed, 201 insertions(+), 139 deletions(-)

diff --git a/examples/regorus.rs b/examples/regorus.rs
index 23dd3e9..89f6ee4 100644
--- a/examples/regorus.rs
+++ b/examples/regorus.rs
@@ -10,10 +10,13 @@ fn rego_eval(
     input: Option<String>,
     query: String,
     enable_tracing: bool,
+    non_strict: bool,
 ) -> Result<()> {
     // Create engine.
     let mut engine = regorus::Engine::new();
 
+    engine.set_strict_builtin_errors(!non_strict);
+
     // Load files from given bundles.
     for dir in bundles.iter() {
         let entries =
@@ -130,6 +133,10 @@ enum RegorusCommand {
         /// Enable tracing.
         #[arg(long, short)]
         trace: bool,
+
+        // Non strict execution
+        #[arg(long, short)]
+        non_strict: bool,
     },
 
     /// Tokenize a Rego policy.
@@ -171,7 +178,8 @@ fn main() -> Result<()> {
             input,
             query,
             trace,
-        } => rego_eval(&bundles, &data, input, query, trace),
+            non_strict,
+        } => rego_eval(&bundles, &data, input, query, trace, non_strict),
         RegorusCommand::Lex { file, verbose } => rego_lex(file, verbose),
         RegorusCommand::Parse { file } => rego_parse(file),
     }
diff --git a/src/engine.rs b/src/engine.rs
index e94aaf0..7183373 100644
--- a/src/engine.rs
+++ b/src/engine.rs
@@ -121,10 +121,10 @@ impl Engine {
         self.prepare_for_eval(enable_tracing)?;
         self.interpreter.clean_internal_evaluation_state();
 
-        // Ensure that each module has an empty object
-        for m in &self.modules {
+        // Ensure that empty modules are created.
+        for m in self.modules.iter().filter(|m| m.policy.is_empty()) {
             let path = Parser::get_path_ref_components(&m.package.refr)?;
-            let path: Vec<&str> = path.iter().map(|s| *s.text()).collect();
+            let path: Vec<&str> = path.iter().map(|s| s.text()).collect();
             let vref =
                 Interpreter::make_or_get_value_mut(self.interpreter.get_data_mut(), &path[..])?;
             if *vref == Value::Undefined {
@@ -147,6 +147,16 @@ impl Engine {
             self.interpreter.set_current_module(prev_module)?;
         }
 
+        // Ensure that all modules are created.
+        for m in &self.modules {
+            let path = Parser::get_path_ref_components(&m.package.refr)?;
+            let path: Vec<&str> = path.iter().map(|s| s.text()).collect();
+            let vref =
+                Interpreter::make_or_get_value_mut(self.interpreter.get_data_mut(), &path[..])?;
+            if *vref == Value::Undefined {
+                *vref = Value::new_object();
+            }
+        }
         self.interpreter.create_rule_prefixes()?;
         Ok(self.interpreter.get_data_mut().clone())
     }
diff --git a/src/interpreter.rs b/src/interpreter.rs
index 2e68e40..bcd6761 100644
--- a/src/interpreter.rs
+++ b/src/interpreter.rs
@@ -268,13 +268,13 @@ impl Interpreter {
                 // Accumulate chained . field accesses.
                 Expr::RefDot { refr, field, .. } => {
                     expr = refr;
-                    path.push(*field.text());
+                    path.push(field.text());
                 }
                 Expr::RefBrack { refr, index, .. } => match index.as_ref() {
                     // refr["field"] is the same as refr.field
                     Expr::String(s) => {
                         expr = refr;
-                        path.push(*s.text());
+                        path.push(s.text());
                     }
                     // Handle other forms of refr.
                     // Note, we have the choice to evaluate a non-string index
@@ -758,7 +758,7 @@ impl Interpreter {
         let raise_error = is_last && type_match.get(expr).is_none();
 
         match (expr.as_ref(), value) {
-            (Expr::Var(ident), _) if ident.text().as_ref() == &"_" => Ok(true),
+            (Expr::Var(ident), _) if ident.text() == "_" => Ok(true),
             (Expr::Var(ident), _)
                 if check_existing_value
                     && self.lookup_local_var(&ident.source_str()) == Some(value.clone()) =>
@@ -1130,7 +1130,7 @@ impl Interpreter {
             // Apply with modifiers.
             for wm in &stmt.with_mods {
                 let path = Parser::get_path_ref_components(&wm.refr)?;
-                let path: Vec<&str> = path.iter().map(|s| *s.text()).collect();
+                let path: Vec<&str> = path.iter().map(|s| s.text()).collect();
                 let target = path.join(".");
 
                 let value = match self.eval_expr(&wm.r#as) {
@@ -2003,7 +2003,7 @@ impl Interpreter {
                     if allow_return_arg && self.lookup_local_var(&var.source_str()).is_none() =>
                 {
                     let value = self.eval_call_impl(span, fcn, &params[..params.len() - 1])?;
-                    if *var.text() != "_" {
+                    if var.text() != "_" {
                         self.add_variable(&var.source_str(), value)?;
                     }
                     Ok(Value::Bool(true))
@@ -2036,6 +2036,31 @@ impl Interpreter {
         None
     }
 
+    fn ensure_module_evaluated(&mut self, path: String) -> Result<()> {
+        for module in self.modules.clone() {
+            let module_path = get_path_string(&module.package.refr, Some("data"))?;
+            if module_path.starts_with(&path)
+                && (module_path.len() == path.len()
+                    || &module_path[path.len()..path.len() + 1] == ".")
+            {
+                // Ensure that the module is created.
+                {
+                    let path = Parser::get_path_ref_components(&module.package.refr)?;
+                    let path: Vec<&str> = path.iter().map(|s| s.text()).collect();
+                    let vref = Self::make_or_get_value_mut(&mut self.data, &path[..])?;
+                    if *vref == Value::Undefined {
+                        *vref = Value::new_object();
+                    }
+                }
+
+                for rule in &module.policy {
+                    self.eval_rule(&module, rule)?;
+                }
+            }
+        }
+        Ok(())
+    }
+
     fn ensure_rule_evaluated(&mut self, path: String) -> Result<()> {
         if let Some(rules) = self.rules.get(&path) {
             for r in rules.clone() {
@@ -2100,7 +2125,7 @@ impl Interpreter {
             // Find the rule to which the var being looked up corresponds to. This is the prefix for
             // which rules exist.
             let mut found = false;
-            for i in 1..fields.len() + 1 {
+            for i in (1..fields.len() + 1).rev() {
                 let path = "data.".to_owned() + &fields[0..i].join(".");
                 if self.rules.get(&path).is_some() || self.default_rules.get(&path).is_some() {
                     self.ensure_rule_evaluated(path)?;
@@ -2109,16 +2134,16 @@ impl Interpreter {
                 }
             }
 
-            // TODO: emit this error only if the var can belong to a rego module; not data specified via json/yaml.
-            //if !no_error && !found {
-            //    bail!(span.error("var is unsafe"));
-            //}
-            let _ = found;
+            if !found {
+                // This could be path to a module.
+                let path = "data.".to_owned() + &fields.join(".");
+                self.ensure_module_evaluated(path)?;
+            }
 
             Ok(Self::get_value_chained(self.data.clone(), fields))
         } else if !self.modules.is_empty() {
             let path = Parser::get_path_ref_components(&self.module.clone().unwrap().package.refr)?;
-            let mut path: Vec<&str> = path.iter().map(|s| *s.text()).collect();
+            let mut path: Vec<&str> = path.iter().map(|s| s.text()).collect();
             path.push(name.text());
 
             let v = Self::get_value_chained(self.data.clone(), &path);
@@ -2126,7 +2151,7 @@ impl Interpreter {
             // If the rule has already been evaluated or specified via a with modifier,
             // use that value.
             if v != Value::Undefined {
-                return Ok(v);
+                return Ok(Self::get_value_chained(v, fields));
             }
 
             // Ensure that all the rules having common prefix (name) are evaluated.
@@ -2139,7 +2164,26 @@ impl Interpreter {
                 bail!(span.error("var is unsafe"));
             }
 
-            self.ensure_rule_evaluated(rule_path)?;
+            // Find the rule to which the var being looked up corresponds to. This is the prefix for
+            // which rules exist.
+            let mut found = false;
+            for i in (0..fields.len() + 1).rev() {
+                let comps = &fields[0..i];
+                let path = if comps.is_empty() {
+                    rule_path.clone()
+                } else {
+                    rule_path.clone() + "." + &fields[0..i].join(".")
+                };
+
+                if self.rules.get(&path).is_some() || self.default_rules.get(&path).is_some() {
+                    self.ensure_rule_evaluated(path)?;
+                    found = true;
+                    break;
+                }
+            }
+
+            // TODO: Is found needed?
+            let _ = found;
 
             let value = Self::get_value_chained(self.data.clone(), &path[..]);
             Ok(Self::get_value_chained(value, fields))
@@ -2161,7 +2205,7 @@ impl Interpreter {
             Expr::True(_) => Ok(Value::Bool(true)),
             Expr::False(_) => Ok(Value::Bool(false)),
             Expr::Number(span) => {
-                let v = match Number::from_str(*span.text()) {
+                let v = match Number::from_str(span.text()) {
                     Ok(v) => Ok(Value::Number(v)),
                     Err(_) => Err(span
                         .source
@@ -2383,19 +2427,19 @@ impl Interpreter {
         while expr.is_some() {
             match expr {
                 Some(Expr::RefDot { refr, field, .. }) => {
-                    comps.push(*field.text());
+                    comps.push(field.text());
                     expr = Some(refr);
                 }
                 Some(Expr::RefBrack { refr, index, .. })
                     if matches!(index.as_ref(), Expr::String(_)) =>
                 {
                     if let Expr::String(s) = index.as_ref() {
-                        comps.push(*s.text());
+                        comps.push(s.text());
                         expr = Some(refr);
                     }
                 }
                 Some(Expr::Var(v)) => {
-                    comps.push(*v.text());
+                    comps.push(v.text());
                     expr = None;
                 }
                 _ => bail!(format!("internal error: not a simplee ref {expr:?}")),
@@ -2521,7 +2565,7 @@ impl Interpreter {
             };
 
             Parser::get_path_ref_components_into(refr, &mut path)?;
-            let paths: Vec<&str> = path.iter().map(|s| *s.text()).collect();
+            let paths: Vec<&str> = path.iter().map(|s| s.text()).collect();
 
             Self::check_default_value(value)?;
             let value = self.eval_expr(value)?;
@@ -2573,7 +2617,8 @@ impl Interpreter {
         if Self::get_value_chained(self.init_data.clone(), path) == Value::Undefined {
             Self::merge_rule_value(span, vref, value)
         } else {
-            Err(span.error("value for rule has already been specified in data document"))
+            // Retain specified value.
+            Ok(())
         }
     }
 
@@ -2636,7 +2681,7 @@ impl Interpreter {
                             v => v,
                         };
 
-                        let paths: Vec<&str> = path.iter().map(|s| *s.text()).collect();
+                        let paths: Vec<&str> = path.iter().map(|s| s.text()).collect();
 
                         if let RuleHead::Set { .. } = &rule_head {
                             // Ensure that sets are created as empty.
@@ -2655,7 +2700,7 @@ impl Interpreter {
                             Parser::get_path_ref_components(&self.current_module()?.package.refr)?;
 
                         Parser::get_path_ref_components_into(refr, &mut path)?;
-                        let path: Vec<&str> = path.iter().map(|s| *s.text()).collect();
+                        let path: Vec<&str> = path.iter().map(|s| s.text()).collect();
 
                         // Ensure that for functions with a nesting level (e.g: a.foo),
                         // `a` is created as an empty object.
@@ -2761,17 +2806,17 @@ impl Interpreter {
         loop {
             refr = match refr.as_ref() {
                 Expr::Var(v) => {
-                    components.push((*v.text().as_ref()).into());
+                    components.push(v.text().into());
                     break;
                 }
                 Expr::RefBrack { refr, index, .. } => {
                     if let Expr::String(s) = index.as_ref() {
-                        components.push((*s.text().as_ref()).into());
+                        components.push(s.text().into());
                     }
                     refr
                 }
                 Expr::RefDot { refr, field, .. } => {
-                    components.push((*field.text().as_ref()).into());
+                    components.push(field.text().into());
                     refr
                 }
                 _ => break,
@@ -2830,27 +2875,21 @@ impl Interpreter {
     }
 
     fn record_rule(&mut self, refr: &Ref<Expr>, rule: Ref<Rule>) -> Result<()> {
-        let path = get_root_var(refr)?;
-        let path = path.text();
-        let path = self.current_module_path.clone() + "." + path;
-        match self.rules.entry(path) {
-            Entry::Occupied(o) => {
-                o.into_mut().push(rule.clone());
-            }
-            Entry::Vacant(v) => {
-                v.insert(vec![rule.clone()]);
-            }
-        }
-        let path = Self::get_path_string(refr, None)?;
-        let path = self.current_module_path.clone() + "." + &path;
-        match self.rules.entry(path) {
-            Entry::Occupied(o) => {
-                o.into_mut().push(rule.clone());
-            }
-            Entry::Vacant(v) => {
-                v.insert(vec![rule.clone()]);
+        let comps = Parser::get_path_ref_components(refr)?;
+        let comps: Vec<&str> = comps.iter().map(|s| s.text()).collect();
+        for c in 0..comps.len() {
+            let path = self.current_module_path.clone() + "." + &comps[0..c + 1].join(".");
+
+            match self.rules.entry(path) {
+                Entry::Occupied(o) => {
+                    o.into_mut().push(rule.clone());
+                }
+                Entry::Vacant(v) => {
+                    v.insert(vec![rule.clone()]);
+                }
             }
         }
+
         Ok(())
     }
 
diff --git a/src/lexer.rs b/src/lexer.rs
index a95f271..8d8dc6f 100644
--- a/src/lexer.rs
+++ b/src/lexer.rs
@@ -186,8 +186,8 @@ pub struct Span {
 }
 
 impl Span {
-    pub fn text(&self) -> std::rc::Rc<&str> {
-        std::rc::Rc::new(&self.source.contents()[self.start as usize..self.end as usize])
+    pub fn text(&self) -> &str {
+        &self.source.contents()[self.start as usize..self.end as usize]
     }
 
     pub fn source_str(&self) -> SourceStr {
@@ -619,7 +619,7 @@ impl<'source> Lexer<'source> {
 	    _ if chr.is_ascii_digit() => self.read_number(),
 	    _ if chr.is_ascii_alphabetic() || chr == '_' => {
 		let mut ident = self.read_ident()?;
-		if *ident.1.text() == "set" && self.peek().1 == '(' {
+		if ident.1.text() == "set" && self.peek().1 == '(' {
 		    // set immediately followed by ( is treated as set( if
 		    // the next token is ).
 		    let state = (self.iter.clone(), self.line, self.col);
@@ -627,7 +627,7 @@ impl<'source> Lexer<'source> {
 
 		    // Check it next token is ).
 		    let next_tok = self.next_token()?;
-		    let is_setp = *next_tok.1.text() == ")";
+		    let is_setp = next_tok.1.text() == ")";
 
 		    // Restore state
 		    (self.iter, self.line, self.col) = state;
diff --git a/src/parser.rs b/src/parser.rs
index 2b82c0c..20a3caf 100644
--- a/src/parser.rs
+++ b/src/parser.rs
@@ -33,12 +33,12 @@ impl<'source> Parser<'source> {
         })
     }
 
-    pub fn token_text(&self) -> std::rc::Rc<&str> {
+    pub fn token_text(&self) -> &str {
         match self.tok.0 {
             TokenKind::Symbol | TokenKind::Number | TokenKind::Ident | TokenKind::Eof => {
                 self.tok.1.text()
             }
-            TokenKind::String | TokenKind::RawString => "".into(),
+            TokenKind::String | TokenKind::RawString => "",
         }
     }
 
@@ -50,7 +50,7 @@ impl<'source> Parser<'source> {
     }
 
     fn expect(&mut self, text: &str, context: &str) -> Result<()> {
-        if *self.token_text() == text {
+        if self.token_text() == text {
             self.next_token()
         } else {
             let msg = format!("expecting `{text}` {context}");
@@ -120,9 +120,9 @@ impl<'source> Parser<'source> {
     }
 
     fn handle_import_future_keywords(&mut self, comps: &[Span]) -> Result<bool> {
-        if comps.len() >= 2 && *comps[0].text() == "future" && *comps[1].text() == "keywords" {
+        if comps.len() >= 2 && comps[0].text() == "future" && comps[1].text() == "keywords" {
             match comps.len() - 2 {
-                1 => self.set_future_keyword(&comps[2].text(), &comps[2])?,
+                1 => self.set_future_keyword(comps[2].text(), &comps[2])?,
                 0 => {
                     let span = &comps[1];
                     for kw in FUTURE_KEYWORDS.iter() {
@@ -137,7 +137,7 @@ impl<'source> Parser<'source> {
                 }
             }
             Ok(true)
-        } else if !comps.is_empty() && *comps[0].text() == "future" {
+        } else if !comps.is_empty() && comps[0].text() == "future" {
             let s = &comps[0];
             Err(self
                 .source
@@ -153,7 +153,7 @@ impl<'source> Parser<'source> {
         is_optional: bool,
         context: &str,
     ) -> Result<()> {
-        if *self.token_text() == kw {
+        if self.token_text() == kw {
             match &self.future_keywords.get(kw) {
                 Some(_) => self.next_token(),
                 None => {
@@ -189,7 +189,7 @@ impl<'source> Parser<'source> {
     fn parse_ident(&mut self) -> Result<Span> {
         let span = self.tok.1.clone();
         match self.tok.0 {
-            TokenKind::Ident if self.is_keyword(*span.text()) => Err(self.source.error(
+            TokenKind::Ident if self.is_keyword(span.text()) => Err(self.source.error(
                 self.tok.1.line,
                 self.tok.1.col,
                 &format!("unexpected keyword `{}`", span.text()),
@@ -208,10 +208,10 @@ impl<'source> Parser<'source> {
         let span = self.tok.1.clone();
         match self.tok.0 {
             TokenKind::Ident
-                if self.is_keyword(*span.text())
-                    || (self.is_imported_future_keyword(*span.text())
+                if self.is_keyword(span.text())
+                    || (self.is_imported_future_keyword(span.text())
 		    // contains can be the name of a builtin even when a keyword
-		    && *span.text() != "contains") =>
+		    && span.text() != "contains") =>
             {
                 Err(self.source.error(
                     self.tok.1.line,
@@ -235,7 +235,7 @@ impl<'source> Parser<'source> {
             TokenKind::Number => Expr::Number(span),
             TokenKind::String => Expr::String(span),
             TokenKind::RawString => Expr::RawString(span),
-            TokenKind::Ident => match *self.token_text() {
+            TokenKind::Ident => match self.token_text() {
                 "null" => Expr::Null(span),
                 "true" => Expr::True(span),
                 "false" => Expr::False(span),
@@ -260,7 +260,7 @@ impl<'source> Parser<'source> {
 
         // Parse the first expression as a ref.
         let term = match self.parse_ref() {
-            Ok(e) if *self.token_text() == "|" => e,
+            Ok(e) if self.token_text() == "|" => e,
             _ => {
                 // Not a comprehension. Restore state.
                 *self = state;
@@ -305,11 +305,11 @@ impl<'source> Parser<'source> {
                 // No progress was made in parsing comprehension.
                 // Parse as array.
                 let mut items = vec![];
-                if *self.token_text() != "]" {
+                if self.token_text() != "]" {
                     items.push(Ref::new(self.parse_in_expr()?));
-                    while *self.token_text() == "," {
+                    while self.token_text() == "," {
                         self.next_token()?;
-                        match *self.token_text() {
+                        match self.token_text() {
                             "]" => break,
                             "" if self.tok.0 == TokenKind::Eof => break,
                             _ => items.push(Ref::new(self.parse_in_expr()?)),
@@ -348,7 +348,7 @@ impl<'source> Parser<'source> {
 
         // It could be a set, object or object comprehension.
         // In all the cases, the first expression must parse successfully.
-        if *self.token_text() == "}" {
+        if self.token_text() == "}" {
             self.next_token()?;
             span.end = self.end;
             return Ok(Expr::Object {
@@ -360,12 +360,12 @@ impl<'source> Parser<'source> {
         let mut item_span = self.tok.1.clone();
         let first = self.parse_in_expr()?;
 
-        if *self.token_text() != ":" {
+        if self.token_text() != ":" {
             // Parse as set.
             let mut items = vec![Ref::new(first)];
-            while *self.token_text() == "," {
+            while self.token_text() == "," {
                 self.next_token()?;
-                match *self.token_text() {
+                match self.token_text() {
                     "}" => break,
                     "" if self.tok.0 == TokenKind::Eof => break,
                     _ => items.push(Ref::new(self.parse_in_expr()?)),
@@ -405,10 +405,10 @@ impl<'source> Parser<'source> {
         item_span.end = self.end;
         items.push((item_span, Ref::new(first), Ref::new(value)));
 
-        while *self.token_text() == "," {
+        while self.token_text() == "," {
             self.next_token()?;
             let item_start = self.tok.1.start;
-            let key = match *self.token_text() {
+            let key = match self.token_text() {
                 "}" => break,
                 "" if self.tok.0 == TokenKind::Eof => break,
                 _ => self.parse_in_expr()?,
@@ -464,7 +464,7 @@ impl<'source> Parser<'source> {
 
     fn parse_ref(&mut self) -> Result<Expr> {
         let start = self.tok.1.start;
-        let mut term = match *self.token_text() {
+        let mut term = match self.token_text() {
             "[" if self.tok.0 == TokenKind::Symbol => self.parse_compr_or_array()?,
             "{" => self.parse_compr_set_or_object()?,
             "set(" => self.parse_empty_set()?,
@@ -494,7 +494,7 @@ impl<'source> Parser<'source> {
             let mut span = self.tok.1.clone();
             let sep_pos = span.start;
             span.start = start;
-            match *self.token_text() {
+            match self.token_text() {
                 "." | "[" if self.tok.1.start != self.end => {
                     if self.line != self.tok.1.line {
                         // Newline encountered. This could be a separate
@@ -552,11 +552,11 @@ impl<'source> Parser<'source> {
                 "(" if possible_fcn => {
                     self.next_token()?;
                     let mut args = vec![];
-                    if *self.token_text() != ")" {
+                    if self.token_text() != ")" {
                         args.push(Ref::new(self.parse_in_expr()?));
-                        while *self.token_text() == "," {
+                        while self.token_text() == "," {
                             self.next_token()?;
-                            match *self.token_text() {
+                            match self.token_text() {
                                 ")" => break,
                                 "" if self.tok.0 == TokenKind::Eof => break,
                                 _ => args.push(Ref::new(self.parse_in_expr()?)),
@@ -592,7 +592,7 @@ impl<'source> Parser<'source> {
         loop {
             let mut span = self.tok.1.clone();
             span.start = start;
-            let op = match *self.token_text() {
+            let op = match self.token_text() {
                 "*" => ArithOp::Mul,
                 "/" => ArithOp::Div,
                 "%" => ArithOp::Mod,
@@ -617,7 +617,7 @@ impl<'source> Parser<'source> {
         loop {
             let mut span = self.tok.1.clone();
             span.start = start;
-            let op = match *self.token_text() {
+            let op = match self.token_text() {
                 "+" => ArithOp::Add,
                 "-" => ArithOp::Sub,
                 _ => return Ok(expr),
@@ -638,7 +638,7 @@ impl<'source> Parser<'source> {
         let start = self.tok.1.start;
         let mut expr = self.parse_arith_expr()?;
 
-        while *self.token_text() == "&" {
+        while self.token_text() == "&" {
             let mut span = self.tok.1.clone();
             span.start = start;
             self.next_token()?;
@@ -658,7 +658,7 @@ impl<'source> Parser<'source> {
         let start = self.tok.1.start;
         let mut expr = self.parse_and_expr()?;
 
-        while *self.token_text() == "|" {
+        while self.token_text() == "|" {
             let mut span = self.tok.1.clone();
             span.start = start;
             self.next_token()?;
@@ -680,7 +680,7 @@ impl<'source> Parser<'source> {
         loop {
             let mut span = self.tok.1.clone();
             span.start = start;
-            let op = match *self.token_text() {
+            let op = match self.token_text() {
                 "<" => BoolOp::Lt,
                 "<=" => BoolOp::Le,
                 "==" => BoolOp::Eq,
@@ -726,7 +726,7 @@ impl<'source> Parser<'source> {
             };
             expr2 = None;
 
-            if *self.token_text() != "in" {
+            if self.token_text() != "in" {
                 break;
             }
         }
@@ -738,7 +738,7 @@ impl<'source> Parser<'source> {
         let start = self.tok.1.start;
         let mut expr = self.parse_bool_expr()?;
 
-        while *self.token_text() == "in" && self.future_keywords.get("in").is_some() {
+        while self.token_text() == "in" && self.future_keywords.get("in").is_some() {
             expr = self.parse_membership_tail(start, expr, None)?;
         }
 
@@ -749,13 +749,13 @@ impl<'source> Parser<'source> {
         let start = self.tok.1.start;
         let mut expr = self.parse_bool_expr()?;
 
-        if *self.token_text() == "," {
+        if self.token_text() == "," {
             self.next_token()?;
             let value = self.parse_bool_expr()?;
             expr = self.parse_membership_tail(start, expr, Some(value))?;
         }
 
-        while *self.token_text() == "in" && self.is_imported_future_keyword("in") {
+        while self.token_text() == "in" && self.is_imported_future_keyword("in") {
             expr = self.parse_membership_tail(start, expr, None)?;
         }
 
@@ -769,7 +769,7 @@ impl<'source> Parser<'source> {
 
         let mut span = self.tok.1.clone();
         span.start = start;
-        let op = match *self.token_text() {
+        let op = match self.token_text() {
             "=" => AssignOp::Eq,
             ":=" => AssignOp::ColEq,
             _ => {
@@ -791,7 +791,7 @@ impl<'source> Parser<'source> {
 
     fn parse_with_modifiers(&mut self) -> Result<Vec<WithModifier>> {
         let mut modifiers = vec![];
-        while *self.token_text() == "with" {
+        while self.token_text() == "with" {
             let mut span = self.tok.1.clone();
             self.next_token()?;
             let refr = self.parse_path_ref()?;
@@ -813,7 +813,7 @@ impl<'source> Parser<'source> {
         self.parse_future_keyword("every", false, context)?;
 
         let ident = self.parse_var()?;
-        let (key, value) = match *self.token_text() {
+        let (key, value) = match self.token_text() {
             "," => {
                 self.next_token()?;
                 match self.parse_var() {
@@ -854,7 +854,7 @@ impl<'source> Parser<'source> {
         let mut vars = vec![self.tok.1.clone()];
         let mut refs = vec![Ref::new(self.parse_ref()?)];
 
-        while *self.token_text() == "," {
+        while self.token_text() == "," {
             self.next_token()?;
             let mut span = self.tok.1.clone();
             refs.push(Ref::new(self.parse_ref()?));
@@ -862,8 +862,8 @@ impl<'source> Parser<'source> {
             vars.push(span);
         }
 
-        if *self.token_text() != "in" || !self.is_imported_future_keyword("in") {
-            if *self.token_text() == "in" {
+        if self.token_text() != "in" || !self.is_imported_future_keyword("in") {
+            if self.token_text() == "in" {
                 self.warn_future_keyword();
             }
             // All the refs must be identifiers
@@ -913,7 +913,7 @@ impl<'source> Parser<'source> {
     }
 
     fn parse_literal(&mut self) -> Result<Literal> {
-        match *self.token_text() {
+        match self.token_text() {
             "some" => return self.parse_some_stmt(),
             "every" => {
                 if self.future_keywords.get("every").is_some() {
@@ -924,7 +924,7 @@ impl<'source> Parser<'source> {
             _ => (),
         }
         let mut span = self.tok.1.clone();
-        let not_expr = if *self.token_text() == "not" {
+        let not_expr = if self.token_text() == "not" {
             self.next_token()?;
             true
         } else {
@@ -955,7 +955,7 @@ impl<'source> Parser<'source> {
 
     pub fn parse_query(&mut self, mut span: Span, end_delim: &str) -> Result<Query> {
         let state = self.clone();
-        let is_definite_query = matches!(*self.token_text(), "some" | "every");
+        let is_definite_query = matches!(self.token_text(), "some" | "every");
 
         // TODO: empty query?
         let mut literals = vec![];
@@ -971,7 +971,7 @@ impl<'source> Parser<'source> {
             }
         };
 
-        if *self.token_text() == "," {
+        if self.token_text() == "," {
             // This is likely an array or set.
             // Restore the state.
             *self = state;
@@ -981,7 +981,7 @@ impl<'source> Parser<'source> {
         literals.push(stmt);
 
         loop {
-            match *self.token_text() {
+            match self.token_text() {
                 t if t == end_delim => break,
                 "" if self.tok.0 == TokenKind::Eof => break,
                 ";" => self.next_token()?,
@@ -1009,7 +1009,7 @@ impl<'source> Parser<'source> {
     pub fn parse_rule_assign(&mut self) -> Result<Option<RuleAssign>> {
         let mut span = self.tok.1.clone();
 
-        let op = match *self.token_text() {
+        let op = match self.token_text() {
             "=" => {
                 self.next_token()?;
                 AssignOp::Eq
@@ -1039,14 +1039,14 @@ impl<'source> Parser<'source> {
             let mut span = self.tok.1.clone();
             let sep_pos = span.start;
             span.start = start;
-            match *self.token_text() {
+            match self.token_text() {
                 "." | "[" if self.tok.1.start != self.end => {
                     bail!(
                         "{}",
                         self.source.error(
                             self.tok.1.line,
                             self.tok.1.col - 1,
-                            format!("invalid whitespace before {}", *self.token_text()).as_str()
+                            format!("invalid whitespace before {}", self.token_text()).as_str()
                         )
                     );
                 }
@@ -1139,7 +1139,7 @@ impl<'source> Parser<'source> {
         loop {
             let mut span = self.tok.1.clone();
             span.start = start;
-            match *self.token_text() {
+            match self.token_text() {
                 // . and [ must not have any space between the previous token.
                 "." | "[" if self.tok.1.start != self.end => {
                     bail!(
@@ -1147,7 +1147,7 @@ impl<'source> Parser<'source> {
                         self.source.error(
                             self.tok.1.line,
                             self.tok.1.col - 1,
-                            format!("invalid whitespace before {}", *self.token_text()).as_str()
+                            format!("invalid whitespace before {}", self.token_text()).as_str()
                         )
                     );
                 }
@@ -1196,16 +1196,16 @@ impl<'source> Parser<'source> {
         let mut span = self.tok.1.clone();
 
         let rule_ref = Ref::new(self.parse_rule_ref()?);
-        match *self.token_text() {
+        match self.token_text() {
             "(" => {
                 self.check_rule_ref(&rule_ref)?;
                 self.next_token()?;
                 let mut args = vec![];
-                if *self.token_text() != ")" {
+                if self.token_text() != ")" {
                     args.push(Ref::new(self.parse_term()?));
-                    while *self.token_text() == "," {
+                    while self.token_text() == "," {
                         self.next_token()?;
-                        match *self.token_text() {
+                        match self.token_text() {
                             ")" => break,
                             "" if self.tok.0 == TokenKind::Eof => break,
                             _ => args.push(Ref::new(self.parse_term()?)),
@@ -1246,8 +1246,8 @@ impl<'source> Parser<'source> {
                 }
 
                 // Determine whether to create a set or a compr
-                let is_set_follower = !self.is_keyword(*self.token_text())
-                    && !self.is_imported_future_keyword(*self.token_text());
+                let is_set_follower = !self.is_keyword(self.token_text())
+                    && !self.is_imported_future_keyword(self.token_text());
                 if assign.is_none() && is_set_follower {
                     match rule_ref.as_ref() {
                         Expr::RefBrack { refr, index, .. }
@@ -1288,7 +1288,7 @@ impl<'source> Parser<'source> {
         let state = self.clone();
         let mut span = self.tok.1.clone();
 
-        if *self.token_text() == "{" {
+        if self.token_text() == "{" {
             self.next_token()?;
             let pos = self.end;
             match self.parse_query(span.clone(), "}") {
@@ -1313,7 +1313,7 @@ impl<'source> Parser<'source> {
         let mut bodies = vec![];
 
         let assign = None;
-        let has_query = match *self.token_text() {
+        let has_query = match self.token_text() {
             "if" if self.if_is_keyword() => {
                 self.next_token()?;
                 let query = Ref::new(self.parse_query_or_literal_stmt()?);
@@ -1343,7 +1343,7 @@ impl<'source> Parser<'source> {
             _ => false,
         };
 
-        match *self.token_text() {
+        match self.token_text() {
             "{" if has_query => self.parse_query_blocks(&mut bodies)?,
             "else" if has_query => self.parse_else_blocks(&mut bodies)?,
             _ => (),
@@ -1353,7 +1353,7 @@ impl<'source> Parser<'source> {
     }
 
     pub fn parse_query_blocks(&mut self, bodies: &mut Vec<RuleBody>) -> Result<()> {
-        while *self.token_text() == "{" {
+        while self.token_text() == "{" {
             let mut span = self.tok.1.clone();
             self.next_token()?;
             let query = Ref::new(self.parse_query(span.clone(), "}")?);
@@ -1371,7 +1371,7 @@ impl<'source> Parser<'source> {
         loop {
             let mut span = self.tok.1.clone();
 
-            match *self.token_text() {
+            match self.token_text() {
                 "{" => {
                     return Err(self.source.error(
                         self.tok.1.line,
@@ -1385,7 +1385,7 @@ impl<'source> Parser<'source> {
 
             let assign = self.parse_rule_assign()?;
 
-            match *self.token_text() {
+            match self.token_text() {
                 "if" if self.if_is_keyword() => {
                     self.next_token()?;
                     let query = Ref::new(self.parse_query_or_literal_stmt()?);
@@ -1407,7 +1407,7 @@ impl<'source> Parser<'source> {
                     });
                 }
                 _ if assign.is_none() => {
-                    if *self.token_text() == "if" {
+                    if self.token_text() == "if" {
                         self.warn_future_keyword();
                     }
                     return Err(self.source.error(
@@ -1428,16 +1428,16 @@ impl<'source> Parser<'source> {
         let rule_ref = Ref::new(self.parse_rule_ref()?);
 
         let mut args = vec![];
-        if *self.token_text() == "(" {
+        if self.token_text() == "(" {
             self.next_token()?;
-            if *self.token_text() != ")" {
+            if self.token_text() != ")" {
                 loop {
                     let arg = self.parse_ident()?;
-                    if *arg.text() != "_" && args.iter().any(|a: &Span| *a.text() == *arg.text()) {
+                    if arg.text() != "_" && args.iter().any(|a: &Span| *a.text() == *arg.text()) {
                         bail!(arg.error("repeating parameter name"));
                     }
                     args.push(arg);
-                    if *self.token_text() == ")" || self.tok.0 == TokenKind::Eof {
+                    if self.token_text() == ")" || self.tok.0 == TokenKind::Eof {
                         break;
                     }
                     self.expect(",", "while parsing default rule parameters")?;
@@ -1446,7 +1446,7 @@ impl<'source> Parser<'source> {
             self.expect(")", "while parsing default rule parameters")?;
         }
 
-        let op = match *self.token_text() {
+        let op = match self.token_text() {
             "=" => AssignOp::Eq,
             ":=" => AssignOp::ColEq,
             _ => {
@@ -1498,16 +1498,16 @@ impl<'source> Parser<'source> {
 
     fn check_and_add_import(&self, import: Import, imports: &mut Vec<Import>) -> Result<()> {
         let ref_comps = Self::get_path_ref_components(&import.refr)?;
-        let comps: Vec<std::rc::Rc<&str>> = ref_comps.iter().map(|s| s.text()).collect();
+        let comps: Vec<&str> = ref_comps.iter().map(|s| s.text()).collect();
 
-        if comps.len() >= 2 && comps[0].as_ref() == &"future" && comps[1].as_ref() == &"keywords" {
+        if comps.len() >= 2 && comps[0] == "future" && comps[1] == "keywords" {
             imports.push(import);
             return Ok(());
         }
 
         for imp in imports.iter() {
             let imp_comps = Self::get_path_ref_components(&imp.refr)?;
-            let imp_comps: Vec<std::rc::Rc<&str>> = imp_comps.iter().map(|s| s.text()).collect();
+            let imp_comps: Vec<&str> = imp_comps.iter().map(|s| s.text()).collect();
 
             let shadow = match (&imp.r#as, &import.r#as) {
                 (Some(i1), Some(i2)) if i1.text() == i2.text() => true,
@@ -1539,13 +1539,13 @@ impl<'source> Parser<'source> {
 
     fn parse_imports(&mut self) -> Result<Vec<Import>> {
         let mut imports = vec![];
-        while *self.token_text() == "import" {
+        while self.token_text() == "import" {
             let mut span = self.tok.1.clone();
             self.next_token()?;
             let refr = Ref::new(self.parse_path_ref()?);
 
             let comps = Self::get_path_ref_components(&refr)?;
-            if !matches!(*comps[0].text(), "data" | "future" | "input") {
+            if !matches!(comps[0].text(), "data" | "future" | "input") {
                 return Err(self.source.error(
                     comps[0].line,
                     comps[0].col,
@@ -1555,7 +1555,7 @@ impl<'source> Parser<'source> {
 
             let is_future_kw = self.handle_import_future_keywords(&comps)?;
 
-            let var = if *self.token_text() == "as" {
+            let var = if self.token_text() == "as" {
                 if is_future_kw {
                     return Err(self.source.error(
                         self.tok.1.line,
@@ -1566,7 +1566,7 @@ impl<'source> Parser<'source> {
 
                 self.next_token()?;
                 let var = self.parse_var()?;
-                if *var.text() == "_" {
+                if var.text() == "_" {
                     return Err(self.source.error(
                         var.line,
                         var.col,
diff --git a/src/scheduler.rs b/src/scheduler.rs
index 6579a69..762d46c 100644
--- a/src/scheduler.rs
+++ b/src/scheduler.rs
@@ -298,7 +298,7 @@ fn gather_assigned_vars(
 ) -> Result<()> {
     traverse(expr, &mut |e| match e.as_ref() {
         // Ignore _, input, data.
-        Var(v) if matches!(*v.text(), "_" | "input" | "data") => Ok(false),
+        Var(v) if matches!(v.text(), "_" | "input" | "data") => Ok(false),
 
         // Record local var that can shadow input var.
         Var(v) if can_shadow => {
@@ -623,7 +623,7 @@ impl Analyzer {
         let mut used_vars = vec![];
         let mut comprs = vec![];
         traverse(expr, &mut |e| match e.as_ref() {
-            Var(v) if !matches!(*v.text(), "_" | "input" | "data") => {
+            Var(v) if !matches!(v.text(), "_" | "input" | "data") => {
                 let name = v.source_str();
                 let is_extra_arg = match assigned_vars {
                     Some(vars) => vars.contains(&v.source_str()),
diff --git a/src/utils.rs b/src/utils.rs
index b2fa956..4670fde 100644
--- a/src/utils.rs
+++ b/src/utils.rs
@@ -86,17 +86,17 @@ pub fn get_path_string(refr: &Expr, document: Option<&str>) -> Result<String> {
     while expr.is_some() {
         match expr {
             Some(Expr::RefDot { refr, field, .. }) => {
-                comps.push(&field.text());
+                comps.push(field.text());
                 expr = Some(refr);
             }
             Some(Expr::RefBrack { refr, index, .. }) => {
                 if let Expr::String(s) = index.as_ref() {
-                    comps.push(&s.text());
+                    comps.push(s.text());
                 }
                 expr = Some(refr);
             }
             Some(Expr::Var(v)) => {
-                comps.push(&v.text());
+                comps.push(v.text());
                 expr = None;
             }
             _ => bail!("internal error: not a simple ref {expr:?}"),
diff --git a/tests/interpreter/cases/data/tests.yaml b/tests/interpreter/cases/data/tests.yaml
index e8e5614..5beb9d5 100644
--- a/tests/interpreter/cases/data/tests.yaml
+++ b/tests/interpreter/cases/data/tests.yaml
@@ -37,7 +37,7 @@ cases:
         - 100
         - 200
         
-  - note: overriding refs in data produces error
+  - note: overriding refs in data produces no error
     data:
       test:
         rule1: 0
@@ -46,8 +46,8 @@ cases:
         package test
 
         rule1 = 6
-    query: data.test
-    error: value for rule has already been specified
+    query: data.test.rule1
+    want_result: 0
       
   - note: rule named data
     data:
diff --git a/tests/opa.passing b/tests/opa.passing
index f4662dd..56116b9 100644
--- a/tests/opa.passing
+++ b/tests/opa.passing
@@ -50,6 +50,9 @@ intersection
 invalidkeyerror
 jsonfilter
 jsonfilteridempotent
+jsonremove
+jsonremoveidempotent
+jsonschema
 jwtencodesignheadererrors
 jwtencodesignpayloaderrors
 negation
@@ -96,5 +99,6 @@ typenamebuiltin
 undos
 union
 units
+uuid
 varreferences
 virtualdocs
\ No newline at end of file
diff --git a/tests/opa.rs b/tests/opa.rs
index 97e0f9c..491c03f 100644
--- a/tests/opa.rs
+++ b/tests/opa.rs
@@ -169,7 +169,8 @@ fn run_opa_tests(opa_tests_dir: String, folders: &[String]) -> Result<()> {
                 {
                     entry.0 += 1;
                 }
-                (Err(_), None) if case.want_error.is_some() => {
+                // TODO: Handle tests that specify both want_result and strict_error
+                (Err(_), _) if case.want_error.is_some() => {
                     // Expected failure.
                     entry.0 += 1;
                 }
diff --git a/tests/parser/mod.rs b/tests/parser/mod.rs
index 9344af8..7ffea6f 100644
--- a/tests/parser/mod.rs
+++ b/tests/parser/mod.rs
@@ -27,7 +27,7 @@ fn match_span(s: &Span, v: &Value) -> Result<()> {
     match &v {
         Value::String(vs) => {
             my_assert_eq!(
-                *s.text(),
+                s.text(),
                 vs.as_ref(),
                 "{}",
                 s.source