diff --git a/src/compiler/destructuring_planner/parameters.rs b/src/compiler/destructuring_planner/parameters.rs index fd5574a..71d1534 100644 --- a/src/compiler/destructuring_planner/parameters.rs +++ b/src/compiler/destructuring_planner/parameters.rs @@ -37,20 +37,19 @@ pub fn create_loop_index_binding_plan( pub fn create_parameter_binding_plan( param_expr: &ExprRef, context: &T, + scoping: ScopingMode, ) -> Result { let mut newly_bound = BTreeSet::new(); - let destructuring_plan = create_destructuring_plan_with_tracking( - param_expr, - context, - ScopingMode::AllowShadowing, - &mut newly_bound, - ) - .ok_or_else(|| BindingPlannerError::FailedToCreateDestructuringPlan { - plan_type: "parameter".to_string(), - span: param_expr.span().clone(), - })?; + let destructuring_plan = + create_destructuring_plan_with_tracking(param_expr, context, scoping, &mut newly_bound) + .ok_or_else(|| BindingPlannerError::FailedToCreateDestructuringPlan { + plan_type: "parameter".to_string(), + span: param_expr.span().clone(), + })?; - validate_pattern_bindings(param_expr, &newly_bound, context)?; + if scoping == ScopingMode::AllowShadowing { + validate_pattern_bindings(param_expr, &newly_bound, context)?; + } Ok(BindingPlan::Parameter { param_expr: param_expr.clone(), diff --git a/src/compiler/hoist.rs b/src/compiler/hoist.rs index cbae7a4..bba31e0 100644 --- a/src/compiler/hoist.rs +++ b/src/compiler/hoist.rs @@ -381,7 +381,9 @@ impl LoopHoister { for param in args { // Create binding plan for function parameter match super::destructuring_planner::create_parameter_binding_plan( - param, &context, + param, + &context, + ScopingMode::AllowShadowing, ) { Ok(binding_plan) => { let expr_idx = param.as_ref().eidx(); @@ -708,7 +710,9 @@ impl LoopHoister { // If the last parameter expression contains unbound vars, create a binding plan if let Some(last_param) = params.last() { match super::destructuring_planner::create_parameter_binding_plan( - last_param, context, + last_param, + context, + ScopingMode::RespectParent, ) { Ok(binding_plan) => { let expr_idx = last_param.as_ref().eidx(); diff --git a/src/languages/rego/compiler/core.rs b/src/languages/rego/compiler/core.rs index 11b1382..5d0d72a 100644 --- a/src/languages/rego/compiler/core.rs +++ b/src/languages/rego/compiler/core.rs @@ -28,7 +28,8 @@ impl<'a> Compiler<'a> { if !self.is_builtin(builtin_name) { return Err(CompilerError::NotBuiltinFunction { name: builtin_name.to_string(), - }); + } + .into()); } // Check if we already have an index for this builtin @@ -44,7 +45,8 @@ impl<'a> Compiler<'a> { } else { return Err(CompilerError::UnknownBuiltinFunction { name: builtin_name.to_string(), - }); + } + .into()); }; // Create builtin info and add it to the program @@ -199,10 +201,12 @@ impl<'a> Compiler<'a> { expr: &ExprRef, context: &str, ) -> Result { - self.get_binding_plan_for_expr(expr) - .ok_or_else(|| CompilerError::MissingBindingPlan { + self.get_binding_plan_for_expr(expr).ok_or_else(|| { + CompilerError::MissingBindingPlan { context: context.to_string(), - }) + } + .at(expr.span()) + }) } pub(super) fn resolve_variable(&mut self, var_name: &str, span: &Span) -> Result { diff --git a/src/languages/rego/compiler/destructuring.rs b/src/languages/rego/compiler/destructuring.rs index 6601f3f..3cb8b4e 100644 --- a/src/languages/rego/compiler/destructuring.rs +++ b/src/languages/rego/compiler/destructuring.rs @@ -42,7 +42,7 @@ impl<'a> Compiler<'a> { rhs_expr, lhs_plan, .. } => { let rhs_reg = self.compile_rego_expr_with_span(rhs_expr, rhs_expr.span(), false)?; - self.apply_destructuring_plan( + let _ = self.apply_destructuring_plan( lhs_plan, rhs_reg, span, @@ -54,21 +54,31 @@ impl<'a> Compiler<'a> { rhs_expr, lhs_plan, .. } => { let rhs_reg = self.compile_rego_expr_with_span(rhs_expr, rhs_expr.span(), false)?; - self.apply_destructuring_plan(lhs_plan, rhs_reg, span, PlanContext::Assignment)?; + let _ = self.apply_destructuring_plan( + lhs_plan, + rhs_reg, + span, + PlanContext::Assignment, + )?; Ok(self.load_bool_literal(true, span)) } AssignmentPlan::EqualsBindRight { lhs_expr, rhs_plan, .. } => { let lhs_reg = self.compile_rego_expr_with_span(lhs_expr, lhs_expr.span(), false)?; - self.apply_destructuring_plan(rhs_plan, lhs_reg, span, PlanContext::Assignment)?; + let _ = self.apply_destructuring_plan( + rhs_plan, + lhs_reg, + span, + PlanContext::Assignment, + )?; Ok(self.load_bool_literal(true, span)) } AssignmentPlan::EqualsBothSides { element_pairs, .. } => { for (value_expr, value_plan) in element_pairs { let value_reg = self.compile_rego_expr_with_span(value_expr, value_expr.span(), false)?; - self.apply_destructuring_plan( + let _ = self.apply_destructuring_plan( value_plan, value_reg, span, @@ -124,7 +134,7 @@ impl<'a> Compiler<'a> { plan: &BindingPlan, value_register: Register, span: &Span, - ) -> Result<()> { + ) -> Result> { match plan { BindingPlan::Assignment { .. } => { bail!("assignment binding plans should be handled via compile_assignment_plan") @@ -160,9 +170,11 @@ impl<'a> Compiler<'a> { span: &Span, ) -> Result<()> { if let (Some(plan), Some(register)) = (key_plan, key_register) { - self.apply_destructuring_plan(plan, register, span, PlanContext::SomeIn)?; + let _ = self.apply_destructuring_plan(plan, register, span, PlanContext::SomeIn)?; } - self.apply_destructuring_plan(value_plan, value_register, span, PlanContext::SomeIn) + let _ = + self.apply_destructuring_plan(value_plan, value_register, span, PlanContext::SomeIn)?; + Ok(()) } fn apply_destructuring_plan( @@ -171,7 +183,7 @@ impl<'a> Compiler<'a> { value_register: Register, span: &Span, context: PlanContext, - ) -> Result<()> { + ) -> Result> { match plan { DestructuringPlan::Var(name_span) => { self.bind_variable(name_span, value_register, span, context)?; @@ -189,6 +201,9 @@ impl<'a> Compiler<'a> { }, span, ); + if self.soft_assert_mode { + return Ok(Some(cmp_reg)); + } self.emit_instruction(Instruction::AssertCondition { condition: cmp_reg }, span); } DestructuringPlan::EqualityValue(expected_value) => { @@ -202,6 +217,9 @@ impl<'a> Compiler<'a> { }, span, ); + if self.soft_assert_mode { + return Ok(Some(cmp_reg)); + } self.emit_instruction(Instruction::AssertCondition { condition: cmp_reg }, span); } DestructuringPlan::Array { element_plans } => { @@ -225,7 +243,8 @@ impl<'a> Compiler<'a> { span, ); } - self.apply_destructuring_plan(element_plan, element_reg, span, context)?; + let _ = + self.apply_destructuring_plan(element_plan, element_reg, span, context)?; } } DestructuringPlan::Object { @@ -249,7 +268,7 @@ impl<'a> Compiler<'a> { }, span, ); - self.apply_destructuring_plan(field_plan, field_reg, span, context)?; + let _ = self.apply_destructuring_plan(field_plan, field_reg, span, context)?; } for (key_expr, field_plan) in dynamic_fields { @@ -270,11 +289,11 @@ impl<'a> Compiler<'a> { }, span, ); - self.apply_destructuring_plan(field_plan, field_reg, span, context)?; + let _ = self.apply_destructuring_plan(field_plan, field_reg, span, context)?; } } } - Ok(()) + Ok(None) } fn bind_variable( diff --git a/src/languages/rego/compiler/error.rs b/src/languages/rego/compiler/error.rs index 1a457f0..f2aabcd 100644 --- a/src/languages/rego/compiler/error.rs +++ b/src/languages/rego/compiler/error.rs @@ -4,6 +4,9 @@ use alloc::format; use alloc::string::String; +use crate::lexer::Span; +use core::fmt; + #[derive(thiserror::Error, Debug)] pub enum CompilerError { #[error("Not a builtin function: {name}")] @@ -68,4 +71,52 @@ impl From for CompilerError { } } -pub type Result = ::core::result::Result; +#[derive(Debug)] +pub struct SpannedCompilerError { + pub error: CompilerError, + pub span: Option, +} + +impl SpannedCompilerError { + pub fn new(error: CompilerError) -> Self { + Self { error, span: None } + } + + pub fn with_span(mut self, span: &Span) -> Self { + if self.span.is_none() { + self.span = Some(span.clone()); + } + self + } +} + +impl fmt::Display for SpannedCompilerError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + if let Some(span) = &self.span { + let msg = format!("{}", self.error); + write!(f, "{}", span.message("error", &msg)) + } else { + write!(f, "{}", self.error) + } + } +} + +impl From for SpannedCompilerError { + fn from(error: CompilerError) -> Self { + Self::new(error) + } +} + +impl core::error::Error for SpannedCompilerError { + fn source(&self) -> Option<&(dyn core::error::Error + 'static)> { + Some(&self.error) + } +} + +impl CompilerError { + pub fn at(self, span: &Span) -> SpannedCompilerError { + SpannedCompilerError::from(self).with_span(span) + } +} + +pub type Result = ::core::result::Result; diff --git a/src/languages/rego/compiler/expressions.rs b/src/languages/rego/compiler/expressions.rs index 5395968..574b73b 100644 --- a/src/languages/rego/compiler/expressions.rs +++ b/src/languages/rego/compiler/expressions.rs @@ -66,11 +66,12 @@ impl<'a> Compiler<'a> { let result: Result = match binding_plan { BindingPlan::Assignment { plan } => self .compile_assignment_plan_using_hoisted_destructuring(&plan, span) - .map_err(CompilerError::from), + .map_err(|e| CompilerError::from(e).at(span)), other => Err(CompilerError::UnexpectedBindingPlan { context: "assignment expression".to_string(), found: format!("{other:?}"), - }), + } + .at(span)), }; return result; diff --git a/src/languages/rego/compiler/expressions/operations.rs b/src/languages/rego/compiler/expressions/operations.rs index a24887b..9dfb884 100644 --- a/src/languages/rego/compiler/expressions/operations.rs +++ b/src/languages/rego/compiler/expressions/operations.rs @@ -220,7 +220,7 @@ impl<'a> Compiler<'a> { ); Ok(dest) } - _ => Err(CompilerError::InvalidUnaryMinus), + _ => Err(CompilerError::InvalidUnaryMinus.at(span)), } } diff --git a/src/languages/rego/compiler/function_calls.rs b/src/languages/rego/compiler/function_calls.rs index c98418a..e3f19df 100644 --- a/src/languages/rego/compiler/function_calls.rs +++ b/src/languages/rego/compiler/function_calls.rs @@ -3,11 +3,24 @@ use super::{Compiler, CompilerError, Register, Result}; use crate::ast::ExprRef; +use crate::builtins; +use crate::compiler::destructuring_planner::plans::BindingPlan; use crate::lexer::Span; use crate::rvm::instructions::{BuiltinCallParams, FunctionCallParams}; use crate::rvm::Instruction; use crate::utils::get_path_string; -use alloc::vec::Vec; +use alloc::{format, string::ToString, vec::Vec}; + +enum CallTarget { + User { + rule_index: u16, + expected_args: Option, + }, + Builtin { + builtin_index: u16, + expected_args: Option, + }, +} impl<'a> Compiler<'a> { pub(super) fn compile_function_call( @@ -16,61 +29,160 @@ impl<'a> Compiler<'a> { params: &[ExprRef], span: Span, ) -> Result { - let fcn_path = - get_path_string(fcn, None).map_err(|_| CompilerError::InvalidFunctionExpression)?; + let fcn_path = get_path_string(fcn, None) + .map_err(|_| CompilerError::InvalidFunctionExpression.at(&span))?; let original_fcn_path = fcn_path.clone(); let full_fcn_path = if self.policy.inner.rules.contains_key(&fcn_path) { fcn_path } else { get_path_string(fcn, Some(&self.current_package)) - .map_err(|_| CompilerError::InvalidFunctionExpressionWithPackage)? + .map_err(|_| CompilerError::InvalidFunctionExpressionWithPackage.at(&span))? }; + let mut out_param_plan: Option<(BindingPlan, Span)> = None; + let mut params_to_compile = params.len(); + + let call_target = self.determine_call_target(&original_fcn_path, &full_fcn_path, &span)?; + + let expected_args = match &call_target { + CallTarget::User { expected_args, .. } => *expected_args, + CallTarget::Builtin { expected_args, .. } => *expected_args, + }; + + if let Some(expected) = expected_args { + if params.len() == expected + 1 { + if let Some(last_param) = params.last() { + let plan = self.expect_binding_plan_for_expr( + last_param, + &format!("extra argument for function '{}'", original_fcn_path), + )?; + + match plan { + BindingPlan::Parameter { .. } => { + out_param_plan = Some((plan, last_param.span().clone())); + params_to_compile -= 1; + } + other => { + return Err(CompilerError::UnexpectedBindingPlan { + context: "function extra argument".to_string(), + found: format!("{other:?}"), + } + .at(last_param.span())); + } + } + } + } + } + let mut arg_regs = Vec::new(); - for param in params.iter() { + for param in params.iter().take(params_to_compile) { let param_reg = self.compile_rego_expr_with_span(param, param.span(), false)?; arg_regs.push(param_reg); } let dest = self.alloc_register(); - if self.is_user_defined_function(&full_fcn_path) { - let rule_index = self.get_or_assign_rule_index(&full_fcn_path)?; - let mut args_array = [0u8; 8]; - let num_args = arg_regs.len().min(8) as u8; - for (i, ®) in arg_regs.iter().take(8).enumerate() { - args_array[i] = reg; - } + match call_target { + CallTarget::User { rule_index, .. } => { + let mut args_array = [0u8; 8]; + let num_args = arg_regs.len().min(8) as u8; + for (i, ®) in arg_regs.iter().take(8).enumerate() { + args_array[i] = reg; + } - let params_index = self.program.add_function_call_params(FunctionCallParams { - func_rule_index: rule_index, - dest, - num_args, - args: args_array, - }); - self.emit_instruction(Instruction::FunctionCall { params_index }, &span); - } else if self.is_builtin(&original_fcn_path) { - let builtin_index = self.get_builtin_index(&original_fcn_path)?; - let mut args_array = [0u8; 8]; - let num_args = arg_regs.len().min(8) as u8; - for (i, ®) in arg_regs.iter().take(8).enumerate() { - args_array[i] = reg; + let params_index = self.program.add_function_call_params(FunctionCallParams { + func_rule_index: rule_index, + dest, + num_args, + args: args_array, + }); + self.emit_instruction(Instruction::FunctionCall { params_index }, &span); } + CallTarget::Builtin { builtin_index, .. } => { + let mut args_array = [0u8; 8]; + let num_args = arg_regs.len().min(8) as u8; + for (i, ®) in arg_regs.iter().take(8).enumerate() { + args_array[i] = reg; + } - let params_index = self.program.add_builtin_call_params(BuiltinCallParams { - dest, - builtin_index, - num_args, - args: args_array, - }); - self.emit_instruction(Instruction::BuiltinCall { params_index }, &span); - } else { - return Err(CompilerError::UnknownFunction { - name: original_fcn_path, - }); + let params_index = self.program.add_builtin_call_params(BuiltinCallParams { + dest, + builtin_index, + num_args, + args: args_array, + }); + self.emit_instruction(Instruction::BuiltinCall { params_index }, &span); + } + } + + if let Some((plan, plan_span)) = &out_param_plan { + let plan_result = self + .apply_binding_plan(plan, dest, plan_span) + .map_err(|err| CompilerError::from(err).at(plan_span))?; + if let Some(result_reg) = plan_result { + self.emit_instruction( + Instruction::Move { + dest, + src: result_reg, + }, + &span, + ); + } else { + self.emit_instruction(Instruction::LoadBool { dest, value: true }, &span); + } } Ok(dest) } + + fn lookup_builtin_arity(&self, name: &str) -> Option { + if name == "print" { + Some(2) + } else { + builtins::BUILTINS + .get(name) + .map(|(_, arity)| *arity as usize) + } + } +} + +impl<'a> Compiler<'a> { + fn determine_call_target( + &mut self, + original_fcn_path: &str, + full_fcn_path: &str, + span: &Span, + ) -> Result { + if self.is_user_defined_function(full_fcn_path) { + let rule_index = self.get_or_assign_rule_index(full_fcn_path)?; + let expected_args = self + .policy + .inner + .functions + .get(full_fcn_path) + .map(|(_, arity, _)| *arity as usize) + .or_else(|| { + self.rule_function_param_count + .get(rule_index as usize) + .and_then(|count| *count) + }); + Ok(CallTarget::User { + rule_index, + expected_args, + }) + } else if self.is_builtin(original_fcn_path) { + let builtin_index = self.get_builtin_index(original_fcn_path)?; + let expected_args = self.lookup_builtin_arity(original_fcn_path); + Ok(CallTarget::Builtin { + builtin_index, + expected_args, + }) + } else { + Err(CompilerError::UnknownFunction { + name: original_fcn_path.to_string(), + } + .at(span)) + } + } } diff --git a/src/languages/rego/compiler/loops.rs b/src/languages/rego/compiler/loops.rs index 1d4f5ac..f02300c 100644 --- a/src/languages/rego/compiler/loops.rs +++ b/src/languages/rego/compiler/loops.rs @@ -20,11 +20,14 @@ impl<'a> Compiler<'a> { .loop_hoisting_table .get_statement_loops(self.current_module_index, stmt.sidx) .cloned() - .ok_or_else(|| CompilerError::General { - message: format!( - "missing loop hoisting data for statement at {}:{}", - stmt.span.line, stmt.span.col - ), + .ok_or_else(|| { + CompilerError::General { + message: format!( + "missing loop hoisting data for statement at {}:{}", + stmt.span.line, stmt.span.col + ), + } + .at(&stmt.span) }) } @@ -70,7 +73,8 @@ impl<'a> Compiler<'a> { } LoopType::Walk => Err(CompilerError::General { message: "walk loops are not yet supported in the RVM compiler".to_string(), - }), + } + .into()), } } @@ -195,7 +199,8 @@ impl<'a> Compiler<'a> { return Err(CompilerError::UnexpectedBindingPlan { context: format!("loop index pattern {}", key_var.span().text()), found: format!("{binding_plan:?}"), - }); + } + .at(key_var.span())); } } else { match key_var.as_ref() { @@ -217,7 +222,8 @@ impl<'a> Compiler<'a> { _ => { return Err(CompilerError::MissingBindingPlan { context: format!("loop index pattern {}", key_var.span().text()), - }); + } + .at(key_var.span())); } } } @@ -243,8 +249,9 @@ impl<'a> Compiler<'a> { let body_start = self.program.instructions.len() as u16; if let Some((binding_plan, plan_span)) = key_binding_plan.as_ref() { - self.apply_binding_plan(binding_plan, key_reg, plan_span) - .map_err(CompilerError::from)?; + let _ = self + .apply_binding_plan(binding_plan, key_reg, plan_span) + .map_err(|e| CompilerError::from(e).at(plan_span))?; } let body_stmts = &remaining_stmts[0..]; @@ -335,12 +342,13 @@ impl<'a> Compiler<'a> { value_reg, collection.span(), ) - .map_err(CompilerError::from)?; + .map_err(|e| CompilerError::from(e).at(collection.span()))?; } else { return Err(CompilerError::UnexpectedBindingPlan { context: format!("some-in binding {}", collection.span().text()), found: format!("{binding_plan:?}"), - }); + } + .at(collection.span())); } } else { if let Some(key_expr) = key { @@ -348,13 +356,24 @@ impl<'a> Compiler<'a> { ast::Expr::Var { value: var_name, .. } => { - let var_name = var_name.as_string()?.to_string(); + let var_name = var_name + .as_string() + .map_err(|err| { + CompilerError::General { + message: format!( + "Failed to read some-in key variable name: {err}" + ), + } + .at(key_expr.span()) + })? + .to_string(); self.store_variable(var_name, key_reg); } _ => { return Err(CompilerError::MissingBindingPlan { context: format!("some-in key pattern {}", key_expr.span().text()), - }); + } + .at(key_expr.span())); } } } @@ -363,13 +382,24 @@ impl<'a> Compiler<'a> { ast::Expr::Var { value: var_name, .. } => { - let var_name = var_name.as_string()?.to_string(); + let var_name = var_name + .as_string() + .map_err(|err| { + CompilerError::General { + message: format!( + "Failed to read some-in value variable name: {err}" + ), + } + .at(value.span()) + })? + .to_string(); self.store_variable(var_name, value_reg); } _ => { return Err(CompilerError::MissingBindingPlan { context: format!("some-in value pattern {}", value.span().text()), - }); + } + .at(value.span())); } } } diff --git a/src/languages/rego/compiler/mod.rs b/src/languages/rego/compiler/mod.rs index ac131fc..8da0ecb 100644 --- a/src/languages/rego/compiler/mod.rs +++ b/src/languages/rego/compiler/mod.rs @@ -125,6 +125,7 @@ pub struct Compiler<'a> { current_rule_path: String, current_call_stack: Vec, entry_points: IndexMap, + soft_assert_mode: bool, } impl<'a> Compiler<'a> { @@ -157,6 +158,18 @@ impl<'a> Compiler<'a> { current_rule_path: String::new(), current_call_stack: Vec::new(), entry_points: IndexMap::new(), + soft_assert_mode: false, } } + + pub(super) fn with_soft_assert_mode(&mut self, enabled: bool, f: F) -> R + where + F: FnOnce(&mut Self) -> R, + { + let previous = self.soft_assert_mode; + self.soft_assert_mode = enabled; + let result = f(self); + self.soft_assert_mode = previous; + result + } } diff --git a/src/languages/rego/compiler/program.rs b/src/languages/rego/compiler/program.rs index 25a56ca..df7654e 100644 --- a/src/languages/rego/compiler/program.rs +++ b/src/languages/rego/compiler/program.rs @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -use super::{Compiler, Result}; +use super::{Compiler, CompilerError, Result}; use crate::interpreter::Interpreter; use crate::rvm::program::{Program, RuleType, SpanInfo}; use crate::rvm::Instruction; @@ -129,7 +129,9 @@ impl<'a> Compiler<'a> { self.program.entry_points = self.entry_points; if !self.program.builtin_info_table.is_empty() { - self.program.initialize_resolved_builtins()?; + self.program + .initialize_resolved_builtins() + .map_err(CompilerError::from)?; } Ok(self.program) diff --git a/src/languages/rego/compiler/queries.rs b/src/languages/rego/compiler/queries.rs index 87831c0..4987c2f 100644 --- a/src/languages/rego/compiler/queries.rs +++ b/src/languages/rego/compiler/queries.rs @@ -192,7 +192,7 @@ impl<'a> Compiler<'a> { } Ok(()) } else { - Err(CompilerError::MissingYieldContext) + Err(CompilerError::MissingYieldContext.into()) } } @@ -204,7 +204,7 @@ impl<'a> Compiler<'a> { self.compile_rego_expr_with_span(expr, &stmt.span, assert_condition)?; } ast::Literal::SomeIn { .. } => { - return Err(CompilerError::SomeInNotHoisted); + return Err(CompilerError::SomeInNotHoisted.at(&stmt.span)); } ast::Literal::Every { key, @@ -221,7 +221,9 @@ impl<'a> Compiler<'a> { } } ast::Literal::NotExpr { expr, .. } => { - let expr_reg = self.compile_rego_expr_with_span(expr, expr.span(), false)?; + let expr_reg = self.with_soft_assert_mode(true, |compiler| { + compiler.compile_rego_expr_with_span(expr, expr.span(), false) + })?; let negated_reg = self.alloc_register(); self.emit_instruction( diff --git a/src/languages/rego/compiler/references.rs b/src/languages/rego/compiler/references.rs index fbc6989..0d739b3 100644 --- a/src/languages/rego/compiler/references.rs +++ b/src/languages/rego/compiler/references.rs @@ -81,7 +81,7 @@ pub(super) fn parse_reference_chain(expr: &ExprRef) -> Result { current_expr = refr; } _ => { - return Err(CompilerError::NotSimpleReferenceChain); + return Err(CompilerError::NotSimpleReferenceChain.at(current_expr.span())); } } } @@ -117,7 +117,7 @@ impl<'a> Compiler<'a> { fn compile_data_chain(&mut self, chain: &ReferenceChain, span: &Span) -> Result { if chain.components.is_empty() { // Just "data" - direct access to data root is not allowed - return Err(CompilerError::DirectDataAccess); + return Err(CompilerError::DirectDataAccess.at(span)); } // Build the static prefix path components for rule matching @@ -301,7 +301,8 @@ impl<'a> Compiler<'a> { // No rule found - undefined variable Err(CompilerError::UndefinedVariable { name: chain.root.clone(), - }) + } + .at(span)) } /// Compile chain access using appropriate instructions based on chain length and complexity diff --git a/src/languages/rego/compiler/rules.rs b/src/languages/rego/compiler/rules.rs index 11340df..01979cb 100644 --- a/src/languages/rego/compiler/rules.rs +++ b/src/languages/rego/compiler/rules.rs @@ -21,7 +21,8 @@ impl<'a> Compiler<'a> { let Some(definitions) = self.policy.inner.rules.get(rule_path) else { return Err(CompilerError::General { message: format!("no definitions found for rule path '{}'", rule_path), - }); + } + .into()); }; let rule_types: BTreeSet = definitions @@ -51,15 +52,16 @@ impl<'a> Compiler<'a> { "internal: rule '{}' has multiple types: {:?}", rule_path, rule_types ), - }); + } + .into()); } - rule_types - .into_iter() - .next() - .ok_or_else(|| CompilerError::RuleTypeNotFound { + rule_types.into_iter().next().ok_or_else(|| { + CompilerError::RuleTypeNotFound { rule_path: rule_path.to_string(), - }) + } + .into() + }) } pub(super) fn get_or_assign_rule_index(&mut self, rule_path: &str) -> Result { @@ -130,13 +132,19 @@ impl<'a> Compiler<'a> { for policy_rule in &module.policy { let policy_rule_ptr = policy_rule.as_ref() as *const Rule; if policy_rule_ptr == rule_ptr { - let package_path = get_path_string(&module.package.refr, Some("data")) - .map_err(|e| CompilerError::General { - message: format!( - "Failed to get package path for module: {}", - e - ), - })?; + let package_path = + match get_path_string(&module.package.refr, Some("data")) { + Ok(path) => path, + Err(e) => { + return Err(CompilerError::General { + message: format!( + "Failed to get package path for module: {}", + e + ), + } + .into()); + } + }; return Ok((package_path, module_index as u32)); } } @@ -212,7 +220,8 @@ impl<'a> Compiler<'a> { "Compile-time recursion detected in rule call chain: {}", chain.join(" -> ") ), - }); + } + .into()); } } @@ -225,7 +234,8 @@ impl<'a> Compiler<'a> { } else { return Err(CompilerError::General { message: format!("Rule index not found for '{}'", entry.rule_path), - }); + } + .into()); }; call_stack.push(entry.rule_path.clone()); @@ -264,14 +274,15 @@ impl<'a> Compiler<'a> { let saved_register_counter = self.register_counter; if let Some(rule_definitions) = rules.get(rule_path) { - let rule_index = self.rule_index_map.get(rule_path).copied().ok_or_else(|| { - CompilerError::General { + let Some(rule_index) = self.rule_index_map.get(rule_path).copied() else { + return Err(CompilerError::General { message: format!( "Rule '{}' not found in rule index map during compilation", rule_path ), } - })?; + .into()); + }; let rule_type = self.rule_types[rule_index as usize].clone(); let result_register = 0; @@ -357,13 +368,15 @@ impl<'a> Compiler<'a> { self.expect_binding_plan_for_expr(arg, &context_desc)?; if let BindingPlan::Parameter { .. } = &binding_plan { - self.apply_binding_plan(&binding_plan, param_reg, arg.span()) - .map_err(CompilerError::from)?; + let _ = self + .apply_binding_plan(&binding_plan, param_reg, arg.span()) + .map_err(|e| CompilerError::from(e).at(arg.span()))?; } else { return Err(CompilerError::UnexpectedBindingPlan { context: context_desc, found: format!("{binding_plan:?}"), - }); + } + .at(arg.span())); } last_param_span = Some(arg.span().clone()); @@ -396,7 +409,8 @@ impl<'a> Compiler<'a> { "Function rule '{}' definition {} has {} parameters but expected {} parameters", rule_path, def_idx, param_names.len(), expected_count ), - }); + } + .at(span)); } } } diff --git a/tests/opa.rs b/tests/opa.rs index b353ceb..39b7a40 100644 --- a/tests/opa.rs +++ b/tests/opa.rs @@ -20,21 +20,9 @@ const OPA_REPO: &str = "https://github.com/open-policy-agent/opa"; const OPA_BRANCH: &str = "v1.2.0"; const OPA_TODO_FOLDERS: &[&str] = &[ - "arithmetic", "aggregates", - "array", - "base64builtins", - "base64urlbuiltins", "baseandvirtualdocs", - "bitsand", - "bitsnegate", - "bitsor", - "bitsshiftleft", - "bitsshiftright", - "bitsxor", - "casts", "comparisonexpr", - "comprehensions", "dataderef", "defaultkeyword", "disjunction", @@ -44,57 +32,17 @@ const OPA_TODO_FOLDERS: &[&str] = &[ "example", "fix1863", "functions", - "functionerrors", - "globmatch", - "globquotemeta", - "hexbuiltins", - "indirectreferences", - "intersection", - "jsonbuiltins", - "jsonfilter", - "jsonfilteridempotent", - "jsonremove", - "jsonremoveidempotent", "jsonschema", - "netcidrcontains", - "netcidrisvalid", - "numbersrange", - "objectfilter", - "objectfilteridempotent", - "objectfilternonstringkey", - "objectget", - "objectremove", - "objectremoveidempotent", - "objectremovenonstringkey", - "objectunion", "partialdocconstants", "partialobjectdoc", "planner-ir", "rand", - "reachable", "refheads", - "regexfind", - "regexfindallstringsubmatch", - "regexisvalid", - "regexmatchtemplate", - "regexsplit", "replacen", - "semvercompare", "semverisvalid", "sets", - "strings", "time", - "trim", - "trimleft", - "trimprefix", - "trimright", - "trimspace", - "trimsuffix", "type", - "typebuiltin", - "typenamebuiltin", - "union", - "urlbuiltins", "varreferences", "virtualdocs", "walkbuiltin", diff --git a/tests/rvm/rego/cases/builtins_out_params.yaml b/tests/rvm/rego/cases/builtins_out_params.yaml new file mode 100644 index 0000000..4a7b2a7 --- /dev/null +++ b/tests/rvm/rego/cases/builtins_out_params.yaml @@ -0,0 +1,95 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +# Builtin Out-Parameter Test Suite +# Verifies the compiler handles builtin return-argument syntax consistently +# across variable bindings, equality checks, scheduler ordering, and literals. + +cases: + - note: builtin_out_param_simple_binding + data: {} + modules: + - | + package test + + rule1 if { + floor(1.001, x) + x == 1 + } + query: data.test.rule1 + want_result: true + + - note: builtin_out_param_scheduler_reordering + data: {} + modules: + - | + package test + + rule2 if { + x == 1 + floor(1.001, x) + } + query: data.test.rule2 + want_result: true + + - note: builtin_out_param_existing_binding_equality + data: {} + modules: + - | + package test + + rule3 if { + x := 1 + floor(1.001, x) + } + query: data.test.rule3 + want_result: true + + - note: builtin_out_param_existing_binding_mismatch + data: {} + modules: + - | + package test + + rule31 if { + x := 2 + floor(1.001, x) + } + query: data.test.rule31 + want_result: "#undefined" + + - note: builtin_out_param_literal_success + data: {} + modules: + - | + package test + + rule4 if { + floor(1.001, 1) + } + query: data.test.rule4 + want_result: true + + - note: builtin_out_param_literal_failure + data: {} + modules: + - | + package test + + rule5 if { + floor(1.001, 2) + } + query: data.test.rule5 + want_result: "#undefined" + + - note: builtin_out_param_negated_equality + data: {} + modules: + - | + package test + + rule5 if { + not abs(-5, 3) + } + query: data.test.rule5 + want_result: true diff --git a/tests/rvm/rego/cases/user_fcn_out_params.yaml b/tests/rvm/rego/cases/user_fcn_out_params.yaml new file mode 100644 index 0000000..1feb9ac --- /dev/null +++ b/tests/rvm/rego/cases/user_fcn_out_params.yaml @@ -0,0 +1,107 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +# User-Defined Out-Parameter Test Suite +# Mirrors builtin coverage but routes through the my_floor helper rule +# to ensure planner handling stays correct for user-defined function rules. + +cases: + - note: user_fcn_out_param_simple_binding + data: {} + modules: + - | + package test + + my_floor(x) := y if { + floor(x, y) + } + + rule1 if { + my_floor(1.001, x) + x == 1 + } + query: data.test.rule1 + want_result: true + + - note: user_fcn_out_param_scheduler_reordering + data: {} + modules: + - | + package test + + my_floor(x) := y if { + floor(x, y) + } + + rule2 if { + x == 1 + my_floor(1.001, x) + } + query: data.test.rule2 + want_result: true + + - note: user_fcn_out_param_existing_binding_equality + data: {} + modules: + - | + package test + + my_floor(x) := y if { + floor(x, y) + } + + rule3 if { + x := 1 + my_floor(1.001, x) + } + query: data.test.rule3 + want_result: true + + - note: user_fcn_out_param_existing_binding_mismatch + data: {} + modules: + - | + package test + + my_floor(x) := y if { + floor(x, y) + } + + rule31 if { + x := 2 + my_floor(1.001, x) + } + query: data.test.rule31 + want_result: "#undefined" + + - note: user_fcn_out_param_literal_success + data: {} + modules: + - | + package test + + my_floor(x) := y if { + floor(x, y) + } + + rule4 if { + my_floor(1.001, 1) + } + query: data.test.rule4 + want_result: true + + - note: user_fcn_out_param_literal_failure + data: {} + modules: + - | + package test + + my_floor(x) := y if { + floor(x, y) + } + + rule5 if { + my_floor(1.001, 2) + } + query: data.test.rule5 + want_result: "#undefined"