fix(interpreter,rvm): correct partial object rule iteration and classification (#718)

Partial object rules with dynamic keys (e.g. `violations[k] if { ... }`)
only produced a single entry instead of collecting all bindings. Two
independent bugs caused this:

1. Interpreter: the early-return optimization in eval_output_expr_in_loop
   checked whether the rule_ref was constant but never verified whether
   the key expression was also constant. A variable key like `k` was
   treated as constant output, causing the loop to exit after the first
   iteration. Fixed by gating early-return on key_expr constness.

2. RVM: compute_rule_type incorrectly classified `p[k] if { ... }` as
   PartialSet instead of PartialObject. OPA v1 semantics define this
   form as a partial object (key -> true). Fixed the classification and
   added compiler error guards for patterns the RVM codegen cannot yet
   handle (constant keys, nested bracket keys), ensuring graceful
   fallback to the interpreter.

The OPA test harness now skips RVM validation per-case when partial
object compiler errors are raised, rather than blanket-skipping entire
folders. This preserves RVM coverage for unrelated tests in the same
folders.

Closes #712

Co-authored-by: Anand Krishnamoorthi <anakrish@microsoft.com>
This commit is contained in:
Copilot
2026-05-18 15:14:08 -05:00
committed by GitHub
parent 3111bf58f2
commit dae3052781
6 changed files with 1167 additions and 1 deletions
+1
View File
@@ -1782,6 +1782,7 @@ impl Interpreter {
let mut comps = self.eval_rule_ref(&rule_ref)?;
if let Some(ke) = &key_expr {
is_const_rule = is_const_rule && Self::is_simple_literal(ke)?;
comps.push(self.eval_expr(ke)?);
}
let output = if let Some(oe) = &output_expr {
+8
View File
@@ -63,6 +63,14 @@ pub enum CompilerError {
#[error("Invalid function expression with package")]
InvalidFunctionExpressionWithPackage,
#[error("partial object rules with constant keys are not yet supported by the RVM compiler")]
PartialObjectConstantKeyUnsupported,
#[error(
"partial object rules with nested bracket keys are not yet supported by the RVM compiler"
)]
PartialObjectNestedKeyUnsupported,
#[error("Compilation error: {message}")]
General { message: String },
}
+53 -1
View File
@@ -59,7 +59,7 @@ impl<'a> Compiler<'a> {
crate::ast::Expr::RefBrack { .. } if assign.is_some() => {
RuleType::PartialObject
}
crate::ast::Expr::RefBrack { .. } => RuleType::PartialSet,
crate::ast::Expr::RefBrack { .. } => RuleType::PartialObject,
_ => RuleType::Complete,
},
_ => RuleType::Complete,
@@ -88,6 +88,54 @@ impl<'a> Compiler<'a> {
})
}
fn validate_partial_object_shape(&self, refr: &ExprRef) -> Result<()> {
let Expr::RefBrack {
refr: prefix,
index,
..
} = refr.as_ref()
else {
return Ok(());
};
if Self::has_unsupported_bracket_prefix(prefix) {
return Err(CompilerError::PartialObjectNestedKeyUnsupported.at(refr.span()));
}
if Self::is_simple_literal(index) {
return Err(CompilerError::PartialObjectConstantKeyUnsupported.at(index.span()));
}
Ok(())
}
fn has_unsupported_bracket_prefix(expr: &ExprRef) -> bool {
match expr.as_ref() {
Expr::RefBrack { refr, index, .. } => {
!Self::is_string_literal(index) || Self::has_unsupported_bracket_prefix(refr)
}
Expr::RefDot { refr, .. } => Self::has_unsupported_bracket_prefix(refr),
_ => false,
}
}
fn is_string_literal(expr: &ExprRef) -> bool {
matches!(expr.as_ref(), Expr::String { .. } | Expr::RawString { .. })
}
fn is_simple_literal(expr: &ExprRef) -> bool {
match expr.as_ref() {
Expr::String { .. }
| Expr::RawString { .. }
| Expr::Number { .. }
| Expr::Bool { .. }
| Expr::Null { .. } => true,
// Unary expressions like `-1` are constant literals too.
Expr::UnaryExpr { expr, .. } => Self::is_simple_literal(expr),
_ => false,
}
}
pub(super) fn get_or_assign_rule_index(&mut self, rule_path: &str) -> Result<u16> {
if let Some(&index) = self.rule_index_map.get(rule_path) {
return Ok(index);
@@ -345,6 +393,10 @@ impl<'a> Compiler<'a> {
let (key_expr, value_expr) = match head {
RuleHead::Compr { refr, assign, .. } => {
if rule_type == RuleType::PartialObject {
self.validate_partial_object_shape(refr)?;
}
self.rule_definition_function_params[rule_index as usize].push(None);
self.rule_definition_destructuring_patterns[rule_index as usize]
.push(None);