Clarify deferral comments and dynamic path docs

src/interpreter.rs

@@ -586,7 +586,7 @@ impl Interpreter {
                                 } else {
                                     format!("{ref_path}.{index}.{}", path.join("."))
                                 };
-                                self.ensure_rule_evaluated(ref_path)?;
+                                self.ensure_matching_rules_for_dynamic_data_index(&ref_path)?;
                             }
                         }
 
@@ -2960,6 +2960,136 @@ impl Interpreter {
         Ok(())
     }
 
+    /// Ensures all rule/default-rule paths matching a dynamic data lookup are evaluated.
+    /// Matches both exact path (`data.a.b`) and descendants with the `data.a.b.` prefix.
+    fn ensure_matching_rules_for_dynamic_data_index(&mut self, path: &str) -> Result<()> {
+        self.check_execution_time()?;
+        let path_prefix = format!("{path}.");
+        let mut matching_paths: Vec<String> = self
+            .compiled_policy
+            .default_rules
+            .keys()
+            .chain(self.compiled_policy.rules.keys())
+            .filter(|rule_path| *rule_path == path || rule_path.starts_with(&path_prefix))
+            .cloned()
+            .collect();
+        matching_paths.sort();
+        matching_paths.dedup();
+
+        for rule_path in matching_paths {
+            self.ensure_rule_evaluated(rule_path)?;
+        }
+
+        Ok(())
+    }
+
+    /// Builds a canonical `data` path from field components.
+    /// For an empty field list, returns `"data"`.
+    fn build_data_path(fields: &[&str]) -> String {
+        if fields.is_empty() {
+            "data".to_string()
+        } else {
+            format!("data.{}", fields.join("."))
+        }
+    }
+
+    /// Returns `true` when `prefix` matches `path` on segment boundaries.
+    ///
+    /// Examples:
+    /// - `path_is_prefix("data.auth", "data.auth") == true`
+    /// - `path_is_prefix("data.auth", "data.auth.allow") == true`
+    /// - `path_is_prefix("data.auth", "data.authorization") == false`
+    fn path_is_prefix(prefix: &str, path: &str) -> bool {
+        if path == prefix {
+            return true;
+        }
+        path.get(prefix.len()..)
+            .is_some_and(|suffix| suffix.starts_with('.'))
+    }
+
+    /// Checks whether a `requested_path` can contain values produced by an active rule.
+    ///
+    /// The active rule path is logically `module_path.rule_path`, but this check avoids
+    /// allocating that joined string in tight evaluation loops.
+    ///
+    /// Examples:
+    /// - request `data` matches module `data.authz` (module expansion needed)
+    /// - request `data.authz` matches rule `allow`
+    /// - request `data.authz.allow` matches rule `allow`
+    /// - request `data.auth` does not match module `data.authz`
+    fn request_matches_active_rule_path(
+        requested_path: &str,
+        module_path: &str,
+        rule_path: &str,
+    ) -> bool {
+        if Self::path_is_prefix(requested_path, module_path) {
+            return true;
+        }
+
+        requested_path
+            .strip_prefix(module_path)
+            .and_then(|suffix| suffix.strip_prefix('.'))
+            .is_some_and(|requested_rule_prefix| {
+                Self::path_is_prefix(requested_rule_prefix, rule_path)
+            })
+    }
+
+    fn should_defer_module_eval_for_path(&self, requested_path: &str) -> Result<bool> {
+        for active_rule in &self.active_rules {
+            let module = self.get_rule_module(active_rule)?;
+            let module_path = get_path_string(&module.package.refr, Some("data"))?;
+            let rule_path = get_path_string(Self::get_rule_refr(active_rule), None)?;
+            if Self::request_matches_active_rule_path(requested_path, &module_path, &rule_path) {
+                return Ok(true);
+            }
+        }
+        Ok(false)
+    }
+
+    /// Resolves `data.<fields...>` while preserving correct rule semantics.
+    /// When a rule is already active, this avoids eager module-wide evaluation so legitimate
+    /// cross-package references are not misidentified as cyclic recursion.
+    fn lookup_data_path(&mut self, fields: &[&str]) -> Result<Value> {
+        if self.is_processed(fields)? {
+            return Ok(Self::get_value_chained(self.data.clone(), fields));
+        }
+
+        // If "data" is used in a query without any fields, then evaluate all modules.
+        if fields.is_empty() && self.active_rules.is_empty() {
+            for module in self.compiled_policy.modules.clone().iter() {
+                for rule in &module.policy {
+                    self.eval_rule(module, rule)?;
+                }
+            }
+        }
+
+        // While a rule is active, avoid eagerly evaluating all matching modules.
+        // This prevents re-entry through sibling rules or other modules from being
+        // misclassified as cyclic recursion.
+        let requested_path = Self::build_data_path(fields);
+        if self.active_rules.is_empty()
+            || !self.should_defer_module_eval_for_path(&requested_path)?
+        {
+            self.ensure_module_evaluated(requested_path.clone())?;
+        }
+
+        for i in (1..=fields.len()).rev() {
+            let prefix = fields.iter().take(i).copied().collect::<Vec<_>>();
+            let prefix_path = Self::build_data_path(&prefix);
+            if self.compiled_policy.rules.contains_key(&prefix_path)
+                || self
+                    .compiled_policy
+                    .default_rules
+                    .contains_key(&prefix_path)
+            {
+                self.ensure_rule_evaluated(prefix_path)?;
+                break;
+            }
+        }
+
+        Ok(Self::get_value_chained(self.data.clone(), fields))
+    }
+
     fn is_processed(&self, path: &[&str]) -> Result<bool> {
         let mut obj = &self.processed_paths;
         for p in path {
@@ -3010,39 +3140,7 @@ impl Interpreter {
 
         // Ensure that rules are evaluated
         if name.text() == "data" {
-            if self.is_processed(fields)? {
-                return Ok(Self::get_value_chained(self.data.clone(), fields));
-            }
-
-            // If "data" is used in a query, without any fields, then evaluate all the modules.
-            if fields.is_empty() && self.active_rules.is_empty() {
-                for module in self.compiled_policy.modules.clone().iter() {
-                    for rule in &module.policy {
-                        self.eval_rule(module, rule)?;
-                    }
-                }
-            }
-
-            // With modifiers may be used to specify part of a module that that not yet been
-            // evaluated. Therefore ensure that module is evaluated first.
-            let requested_path = format!("data.{}", fields.join("."));
-            self.ensure_module_evaluated(requested_path.clone())?;
-
-            for i in (1..=fields.len()).rev() {
-                let prefix = fields.iter().take(i).copied().collect::<Vec<_>>();
-                let prefix_path = format!("data.{}", prefix.join("."));
-                if self.compiled_policy.rules.contains_key(&prefix_path)
-                    || self
-                        .compiled_policy
-                        .default_rules
-                        .contains_key(&prefix_path)
-                {
-                    self.ensure_rule_evaluated(prefix_path)?;
-                    break;
-                }
-            }
-
-            Ok(Self::get_value_chained(self.data.clone(), fields))
+            self.lookup_data_path(fields)
         } else if !self.compiled_policy.modules.is_empty() {
             let module = self.current_module()?;
             let parsed_path = Parser::get_path_ref_components(&module.package.refr)?;
@@ -3092,6 +3190,17 @@ impl Interpreter {
 
             if !found {
                 if let Some(imported_var) = self.compiled_policy.imports.get(&rule_path).cloned() {
+                    if let Ok(import_path) = get_path_string(&imported_var, None) {
+                        if import_path == "data" || import_path.starts_with("data.") {
+                            let combined_path = if fields.is_empty() {
+                                import_path
+                            } else {
+                                format!("{}.{}", import_path, fields.join("."))
+                            };
+                            let data_fields: Vec<&str> = combined_path.split('.').skip(1).collect();
+                            return self.lookup_data_path(&data_fields);
+                        }
+                    }
                     return Ok(Self::get_value_chained(
                         self.eval_expr(&imported_var)?,
                         fields,

src/languages/rego/compiler/references.rs

@@ -338,6 +338,15 @@ impl<'a> Compiler<'a> {
         // No rule found; fall back to module-level imports.
         let import_key = format!("{}.{}", &self.current_package, root);
         if let Some(import_expr) = self.policy.inner.imports.get(&import_key) {
+            if let Ok(mut import_chain) = parse_reference_chain(import_expr) {
+                if let ReferenceRoot::Variable(import_root) = &import_chain.root {
+                    if import_root == "data" {
+                        import_chain.components.extend(chain.components.clone());
+                        return self.compile_data_chain(&import_chain, span);
+                    }
+                }
+            }
+
             let import_reg =
                 self.compile_rego_expr_with_span(import_expr, import_expr.span(), false)?;
             if chain.components.is_empty() {

src/parser.rs

@@ -354,33 +354,6 @@ impl<'source> Parser<'source> {
         }
     }
 
-    /// Parse a field name after `.` in a ref expression.
-    ///
-    /// Unlike [`Self::parse_var`] and [`Self::parse_ident`], this method accepts **any**
-    /// `TokenKind::Ident` token, including reserved keywords (e.g. `as`, `default`, `else`,
-    /// `false`, `if`, `import`, `in`, `not`, `null`, `package`, `some`, `true`, `with`).
-    ///
-    /// The position immediately after `.` is unambiguously a field name, so there is no
-    /// syntactic ambiguity with statement-level keywords. This matches OPA's
-    /// `keywords_in_refs` capability, which is enabled by default in standard OPA builds.
-    ///
-    /// # Example
-    /// ```rego
-    /// allow if { input.v0.package.format == "npm" }  # `package` is a keyword but valid here
-    /// ```
-    fn parse_ref_field(&mut self) -> Result<Span> {
-        let span = self.tok.1.clone();
-        match self.tok.0 {
-            TokenKind::Ident => {
-                self.next_token()?;
-                Ok(span)
-            }
-            _ => Err(self
-                .source
-                .error(self.tok.1.line, self.tok.1.col, "expecting identifier")),
-        }
-    }
-
     fn read_number(&mut self, span: Span) -> Result<Expr> {
         match Number::from_str(span.text()) {
             Ok(v) => Ok(Expr::Number {
@@ -770,10 +743,9 @@ impl<'source> Parser<'source> {
                     );
                 }
                 "." => {
-                    // Read identifier. Keywords are allowed as field names in
-                    // dot-notation refs (e.g. `input.package.name`).
+                    // Read identifier.
                     self.next_token()?;
-                    let field = self.parse_ref_field()?;
+                    let field = self.parse_var()?;
                     span.end = self.end;
 
                     // Disallow any whitespace between . and identifier.
@@ -1446,10 +1418,9 @@ impl<'source> Parser<'source> {
                     );
                 }
                 "." => {
-                    // Read identifier. Keywords are allowed as field names in
-                    // dot-notation refs (e.g. `import data.my.package`).
+                    // Read identifier.
                     self.next_token()?;
-                    let field = self.parse_ref_field()?;
+                    let field = self.parse_ident()?;
                     span.end = self.end;
 
                     // Disallow any whitespace between . and identifier.
@@ -1552,8 +1523,7 @@ impl<'source> Parser<'source> {
                 "." => {
                     let sep_pos = self.tok.1.start;
                     self.next_token()?;
-                    // Keywords are allowed as field names in dot-notation refs.
-                    let field = self.parse_ref_field()?;
+                    let field = self.parse_var()?;
                     span.end = self.end;
 
                     // Disallow any whitespace between . and identifier.

tests/engine/mod.rs

@@ -197,3 +197,34 @@ fn get_policy_parameters() -> Result<()> {
 
     Ok(())
 }
+
+#[test]
+fn cross_package_import_lookup_does_not_false_cycle() -> Result<()> {
+    let mut engine = Engine::new();
+    engine.add_policy(
+        "registry.rego".to_string(),
+        r#"package registry
+           import data.registry.packages.package_a
+           import rego.v1
+
+           allow_stage1 if package_a.allow_stage1
+         "#
+        .to_string(),
+    )?;
+    engine.add_policy(
+        "package_a.rego".to_string(),
+        r#"package registry.packages.package_a
+           import data.registry
+           import rego.v1
+
+           allow_stage2 if registry.allow_stage1
+           allow_stage1 := true
+         "#
+        .to_string(),
+    )?;
+
+    let result = engine.eval_rule("data.registry.packages.package_a.allow_stage2".to_string())?;
+    assert_eq!(result, Value::Bool(true));
+
+    Ok(())
+}

tests/interpreter/cases/refr/keywords_in_refs.yaml

@@ -1,268 +0,0 @@
-# Copyright (c) Microsoft Corporation.
-# Licensed under the MIT License.
-#
-# Tests for keywords-as-field-names in dot-notation refs.
-# Matches OPA's `keywords_in_refs` behavior, enabled by default.
-cases:
-  - note: keywords_in_refs/package field
-    modules:
-      - |
-        package test
-        allow if {
-          input.v0.package.format == "npm"
-        }
-    input:
-      v0:
-        package:
-          format: npm
-    query: data.test.allow
-    want_result: true
-
-  - note: keywords_in_refs/as field
-    modules:
-      - |
-        package test
-        x = input.as.type
-    input:
-      as:
-        type: string
-    query: data.test.x
-    want_result: string
-
-  - note: keywords_in_refs/default field
-    modules:
-      - |
-        package test
-        x = input.default.value
-    input:
-      default:
-        value: 42
-    query: data.test.x
-    want_result: 42
-
-  - note: keywords_in_refs/else field
-    modules:
-      - |
-        package test
-        x = input.else.value
-    input:
-      else:
-        value: hello
-    query: data.test.x
-    want_result: hello
-
-  - note: keywords_in_refs/import field
-    modules:
-      - |
-        package test
-        x = input.import.name
-    input:
-      import:
-        name: foo
-    query: data.test.x
-    want_result: foo
-
-  - note: keywords_in_refs/not field
-    modules:
-      - |
-        package test
-        x = input.not.allowed
-    input:
-      not:
-        allowed: false
-    query: data.test.x
-    want_result: false
-
-  - note: keywords_in_refs/null field
-    modules:
-      - |
-        package test
-        x = input.null.value
-    input:
-      "null":
-        value: 1
-    query: data.test.x
-    want_result: 1
-
-  - note: keywords_in_refs/some field
-    modules:
-      - |
-        package test
-        x = input.some.field
-    input:
-      some:
-        field: bar
-    query: data.test.x
-    want_result: bar
-
-  - note: keywords_in_refs/true field
-    modules:
-      - |
-        package test
-        x = input.true.x
-    input:
-      "true":
-        x: 2
-    query: data.test.x
-    want_result: 2
-
-  - note: keywords_in_refs/false field
-    modules:
-      - |
-        package test
-        x = input.false.x
-    input:
-      "false":
-        x: 3
-    query: data.test.x
-    want_result: 3
-
-  - note: keywords_in_refs/with field
-    modules:
-      - |
-        package test
-        x = input.with.config
-    input:
-      with:
-        config: test
-    query: data.test.x
-    want_result: test
-
-  - note: keywords_in_refs/future keywords (if, in, every, contains)
-    modules:
-      - |
-        package test
-        import future.keywords
-        x if {
-          input.if.condition == true
-          input.in.set == "member"
-          input.every.item == "x"
-          input.contains.key == "val"
-        }
-    input:
-      if:
-        condition: true
-      in:
-        set: member
-      every:
-        item: x
-      contains:
-        key: val
-    query: data.test.x
-    want_result: true
-
-  - note: keywords_in_refs/chained keywords
-    modules:
-      - |
-        package test
-        x = input.package.import.default
-    input:
-      package:
-        import:
-          default: chained
-    query: data.test.x
-    want_result: chained
-
-  - note: keywords_in_refs/data path with keyword
-    modules:
-      - |
-        package test
-        x = data.mydata.package.name
-    data:
-      mydata:
-        package:
-          name: mypackage
-    query: data.test.x
-    want_result: mypackage
-
-  - note: keywords_in_refs/rego v1 all keywords
-    modules:
-      - |
-        package test
-        import rego.v1
-        allow if {
-          input.package.format == "npm"
-          input.default.value == 1
-          input.if.enabled == true
-          input.in.set == "member"
-          input.not.flag == false
-          input.with.config == "ok"
-        }
-    input:
-      package:
-        format: npm
-      default:
-        value: 1
-      if:
-        enabled: true
-      in:
-        set: member
-      not:
-        flag: false
-      with:
-        config: ok
-    query: data.test.allow
-    want_result: true
-
-  - note: keywords_in_refs/future keywords without import
-    modules:
-      - |
-        package test
-        x = [input.if.flag, input.in.value, input.every.item, input.contains.key]
-    input:
-      if:
-        flag: true
-      in:
-        value: member
-      every:
-        item: each
-      contains:
-        key: present
-    query: data.test.x
-    want_result: [true, "member", "each", "present"]
-
-  - note: keywords_in_refs/package path keywords
-    modules:
-      - |
-        package words.if.default
-        value = 7
-      - |
-        package test
-        x = data.words.if.default.value
-    query: data.test.x
-    want_result: 7
-
-  - note: keywords_in_refs/import path keywords
-    modules:
-      - |
-        package test
-        import data.catalog.if.default as kw
-        x = kw.value
-    data:
-      catalog:
-        if:
-          default:
-            value: 99
-    query: data.test.x
-    want_result: 99
-
-  - note: keywords_in_refs/rule head keyword path
-    modules:
-      - |
-        package test
-        policy.default.level := 3
-    query: data.test.policy.default.level
-    want_result: 3
-
-  - note: keywords_in_refs/mixed dot keyword and dynamic bracket
-    modules:
-      - |
-        package test
-        x = input.package[segment].value
-        segment = "import"
-    input:
-      package:
-        import:
-          value: from_dynamic
-    query: data.test.x
-    want_result: from_dynamic

tests/rvm/rego/cases/chained_access.yaml

@@ -315,109 +315,3 @@ cases:
         }
     query: data.test.main
     want_result: "/api/v1/users"
-
-  - note: keywords_in_refs/package_field
-    data: {}
-    input:
-      v0:
-        package:
-          format: npm
-    modules:
-      - |
-        package test
-        allow := true if {
-          input.v0.package.format == "npm"
-        }
-    query: data.test.allow
-    want_result: true
-
-  - note: keywords_in_refs/multiple_keywords
-    data: {}
-    input:
-      default:
-        value: 42
-      import:
-        name: foo
-      not:
-        allowed: false
-      with:
-        config: ok
-    modules:
-      - |
-        package test
-        import rego.v1
-        result if {
-          input.default.value == 42
-          input.import.name == "foo"
-          input.not.allowed == false
-          input.with.config == "ok"
-        }
-    query: data.test.result
-    want_result: true
-
-  - note: keywords_in_refs/future_keywords_without_import
-    data: {}
-    input:
-      if:
-        flag: true
-      in:
-        value: member
-      every:
-        item: each
-      contains:
-        key: present
-    modules:
-      - |
-        package test
-        result := [input.if.flag, input.in.value, input.every.item, input.contains.key]
-    query: data.test.result
-    want_result: [true, "member", "each", "present"]
-
-  - note: keywords_in_refs/package_path_keywords
-    data: {}
-    modules:
-      - |
-        package words.if.default
-        value := 7
-      - |
-        package test
-        result := data.words.if.default.value
-    query: data.test.result
-    want_result: 7
-
-  - note: keywords_in_refs/import_path_keywords
-    data:
-      catalog:
-        if:
-          default:
-            value: 99
-    modules:
-      - |
-        package test
-        import data.catalog.if.default as kw
-        result := kw.value
-    query: data.test.result
-    want_result: 99
-
-  - note: keywords_in_refs/rule_head_keyword_path
-    data: {}
-    modules:
-      - |
-        package test
-        policy.default.level := 3
-    query: data.test.policy.default.level
-    want_result: 3
-
-  - note: keywords_in_refs/mixed_dot_keyword_and_dynamic_bracket
-    data: {}
-    input:
-      package:
-        import:
-          value: from_dynamic
-    modules:
-      - |
-        package test
-        segment := "import"
-        result := input.package[segment].value
-    query: data.test.result
-    want_result: "from_dynamic"

tests/rvm/rego/cases/imports.yaml

@@ -95,3 +95,37 @@ cases:
         }
     query: data.rules.present
     want_result: true
+  - note: import_cross_package_no_false_cycle
+    modules:
+      - |
+        package registry
+        import data.registry.packages.package_a
+        import rego.v1
+
+        allow_stage1 if package_a.allow_stage1
+      - |
+        package registry.packages.package_a
+        import data.registry
+        import rego.v1
+
+        allow_stage2 if registry.allow_stage1
+        allow_stage1 := true
+    query: data.registry.packages.package_a.allow_stage2
+    want_result: true
+  - note: dynamic_data_index_evaluates_matching_rules
+    modules:
+      - |
+        package registry
+        import rego.v1
+
+        allow_stage1 if {
+          name := "package_a"
+          data.registry.packages[name].allow_stage2
+        }
+      - |
+        package registry.packages.package_a
+        import rego.v1
+
+        allow_stage2 := true
+    query: data.registry.allow_stage1
+    want_result: true