diff --git a/src/ast.rs b/src/ast.rs index 74cbf6b..a86584f 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -1,4 +1,3 @@ -#![allow(clippy::missing_const_for_fn, clippy::pattern_type_mismatch)] // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. @@ -265,61 +264,59 @@ pub enum Expr { } impl Expr { - pub fn span(&self) -> &Span { - use Expr::*; - match self { - String { span, .. } - | RawString { span, .. } - | Number { span, .. } - | Bool { span, .. } - | Null { span, .. } - | Var { span, .. } - | Array { span, .. } - | Set { span, .. } - | Object { span, .. } - | ArrayCompr { span, .. } - | SetCompr { span, .. } - | ObjectCompr { span, .. } - | Call { span, .. } - | UnaryExpr { span, .. } - | RefDot { span, .. } - | RefBrack { span, .. } - | BinExpr { span, .. } - | BoolExpr { span, .. } - | ArithExpr { span, .. } - | AssignExpr { span, .. } - | Membership { span, .. } => span, + pub const fn span(&self) -> &Span { + match *self { + Self::String { ref span, .. } + | Self::RawString { ref span, .. } + | Self::Number { ref span, .. } + | Self::Bool { ref span, .. } + | Self::Null { ref span, .. } + | Self::Var { ref span, .. } + | Self::Array { ref span, .. } + | Self::Set { ref span, .. } + | Self::Object { ref span, .. } + | Self::ArrayCompr { ref span, .. } + | Self::SetCompr { ref span, .. } + | Self::ObjectCompr { ref span, .. } + | Self::Call { ref span, .. } + | Self::UnaryExpr { ref span, .. } + | Self::RefDot { ref span, .. } + | Self::RefBrack { ref span, .. } + | Self::BinExpr { ref span, .. } + | Self::BoolExpr { ref span, .. } + | Self::ArithExpr { ref span, .. } + | Self::AssignExpr { ref span, .. } + | Self::Membership { ref span, .. } => span, #[cfg(feature = "rego-extensions")] - OrExpr { span, .. } => span, + Self::OrExpr { ref span, .. } => span, } } - pub fn eidx(&self) -> u32 { - use Expr::*; - match self { - String { eidx, .. } - | RawString { eidx, .. } - | Number { eidx, .. } - | Bool { eidx, .. } - | Null { eidx, .. } - | Var { eidx, .. } - | Array { eidx, .. } - | Set { eidx, .. } - | Object { eidx, .. } - | ArrayCompr { eidx, .. } - | SetCompr { eidx, .. } - | ObjectCompr { eidx, .. } - | Call { eidx, .. } - | UnaryExpr { eidx, .. } - | RefDot { eidx, .. } - | RefBrack { eidx, .. } - | BinExpr { eidx, .. } - | BoolExpr { eidx, .. } - | ArithExpr { eidx, .. } - | AssignExpr { eidx, .. } - | Membership { eidx, .. } => *eidx, + pub const fn eidx(&self) -> u32 { + match *self { + Self::String { eidx, .. } + | Self::RawString { eidx, .. } + | Self::Number { eidx, .. } + | Self::Bool { eidx, .. } + | Self::Null { eidx, .. } + | Self::Var { eidx, .. } + | Self::Array { eidx, .. } + | Self::Set { eidx, .. } + | Self::Object { eidx, .. } + | Self::ArrayCompr { eidx, .. } + | Self::SetCompr { eidx, .. } + | Self::ObjectCompr { eidx, .. } + | Self::Call { eidx, .. } + | Self::UnaryExpr { eidx, .. } + | Self::RefDot { eidx, .. } + | Self::RefBrack { eidx, .. } + | Self::BinExpr { eidx, .. } + | Self::BoolExpr { eidx, .. } + | Self::ArithExpr { eidx, .. } + | Self::AssignExpr { eidx, .. } + | Self::Membership { eidx, .. } => eidx, #[cfg(feature = "rego-extensions")] - OrExpr { eidx, .. } => *eidx, + Self::OrExpr { eidx, .. } => eidx, } } } @@ -435,9 +432,9 @@ pub enum Rule { } impl Rule { - pub fn span(&self) -> &Span { - match self { - Self::Spec { span, .. } | Self::Default { span, .. } => span, + pub const fn span(&self) -> &Span { + match *self { + Self::Spec { ref span, .. } | Self::Default { ref span, .. } => span, } } } diff --git a/src/compiler.rs b/src/compiler.rs index ffeb93d..61bb013 100644 --- a/src/compiler.rs +++ b/src/compiler.rs @@ -1,14 +1,6 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -#![allow( - clippy::missing_const_for_fn, - clippy::option_if_let_else, - clippy::if_then_some_else_none, - clippy::unused_self, - clippy::semicolon_if_nothing_returned, - clippy::useless_let_if_seq -)] //! Compiler-related functionality for Regorus. //! //! This module contains utilities and data structures used during diff --git a/src/compiler/context.rs b/src/compiler/context.rs index 36d07e0..636719e 100644 --- a/src/compiler/context.rs +++ b/src/compiler/context.rs @@ -65,7 +65,7 @@ pub struct ScopeContext { impl ScopeContext { /// Create a new context with Query type (default, no output expressions) - pub fn new() -> Self { + pub const fn new() -> Self { Self { context_type: ContextType::Query, bound_vars: BTreeSet::new(), @@ -81,7 +81,7 @@ impl ScopeContext { /// Create a new context with a specific context type #[allow(dead_code)] - pub fn with_context_type(context_type: ContextType) -> Self { + pub const fn with_context_type(context_type: ContextType) -> Self { Self { context_type, bound_vars: BTreeSet::new(), @@ -97,7 +97,7 @@ impl ScopeContext { /// Create a new context with output expressions (for rules and comprehensions) #[allow(dead_code)] - pub fn with_output_exprs( + pub const fn with_output_exprs( context_type: ContextType, key_expr: Option, value_expr: Option, diff --git a/src/compiler/destructuring_planner/utils.rs b/src/compiler/destructuring_planner/utils.rs index fcde1d6..114e585 100644 --- a/src/compiler/destructuring_planner/utils.rs +++ b/src/compiler/destructuring_planner/utils.rs @@ -149,10 +149,9 @@ pub(crate) fn check_literal_structure( } pub(crate) fn ensure_literal_match(plan: &DestructuringPlan, expr: &ExprRef) -> Result<()> { - match check_literal_structure(plan, expr).into_error() { - Some(err) => Err(err), - None => Ok(()), - } + check_literal_structure(plan, expr) + .into_error() + .map_or(Ok(()), Err) } pub(crate) fn collect_pattern_var_spans(expr: &ExprRef, spans: &mut Vec) { @@ -248,13 +247,7 @@ pub(crate) fn ensure_structural_compatibility( /// Helper that discards destructuring plans which do not bind any variables. pub(crate) fn plan_only_if_binds(plan: Option) -> Option { - plan.and_then(|plan| { - if plan.introduces_binding() || plan.contains_wildcards() { - Some(plan) - } else { - None - } - }) + plan.and_then(|plan| (plan.introduces_binding() || plan.contains_wildcards()).then_some(plan)) } pub(crate) fn extract_literal_key(expr: &ExprRef) -> Option { diff --git a/src/compiler/hoist.rs b/src/compiler/hoist.rs index 233fde9..67eeafd 100644 --- a/src/compiler/hoist.rs +++ b/src/compiler/hoist.rs @@ -576,20 +576,20 @@ impl LoopHoister { // Get the scheduled order if available let stmt_order: Vec = if let Some(ref schedule) = self.schedule { - if let Some(query_schedule) = schedule + schedule .queries .get_checked(module_idx, query.qidx) .map_err(|err| anyhow!("schedule out of bounds: {err}"))? - { - query_schedule - .order - .iter() - .map(|&idx| idx as usize) - .collect() - } else { - // No schedule for this query, use source order - (0..query.stmts.len()).collect() - } + .map_or_else( + || (0..query.stmts.len()).collect(), + |query_schedule| { + query_schedule + .order + .iter() + .map(|&idx| idx as usize) + .collect() + }, + ) } else { // No schedule available, use source order (0..query.stmts.len()).collect() diff --git a/src/engine.rs b/src/engine.rs index 24bef16..7bc0bb9 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -1,13 +1,6 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -#![allow( - clippy::indexing_slicing, - clippy::missing_const_for_fn, - clippy::semicolon_if_nothing_returned, - clippy::print_stderr, - clippy::as_conversions, - clippy::pattern_type_mismatch -)] +#![allow(clippy::print_stderr)] use crate::ast::*; use crate::compiled_policy::CompiledPolicy; @@ -20,7 +13,7 @@ use crate::value::*; use crate::*; use crate::{Extension, QueryResults}; -use anyhow::{bail, Result}; +use anyhow::{anyhow, bail, Result}; /// The Rego evaluation engine. /// @@ -104,7 +97,7 @@ impl Engine { /// # } /// ``` /// - pub fn set_rego_v0(&mut self, rego_v0: bool) { + pub const fn set_rego_v0(&mut self, rego_v0: bool) { self.rego_v1 = !rego_v0; } @@ -398,7 +391,7 @@ impl Engine { /// **_NOTE:_** Currently not all builtins honor this flag and will always strictly raise errors. /// ---- pub fn set_strict_builtin_errors(&mut self, b: bool) { - self.interpreter.set_strict_builtin_errors(b) + self.interpreter.set_strict_builtin_errors(b); } #[doc(hidden)] @@ -782,13 +775,23 @@ impl Engine { /// ``` pub fn eval_bool_query(&mut self, query: String, enable_tracing: bool) -> Result { let results = self.eval_query(query, enable_tracing)?; - match results.result.len() { - 0 => bail!("query did not produce any values"), - 1 if results.result[0].expressions.len() == 1 => { - results.result[0].expressions[0].value.as_bool().copied() - } - _ => bail!("query produced more than one value"), + let entries = results.result.as_slice(); + let entry = entries + .first() + .ok_or_else(|| anyhow!("query did not produce any values"))?; + if entries.len() > 1 { + bail!("query produced more than one value"); } + + let expressions = entry.expressions.as_slice(); + let expr = expressions + .first() + .ok_or_else(|| anyhow!("query result missing expression"))?; + if expressions.len() > 1 { + bail!("query produced more than one value"); + } + + expr.value.as_bool().copied() } /// Evaluate an `allow` query. @@ -804,12 +807,12 @@ impl Engine { /// let enable_tracing = false; /// assert_eq!(engine.eval_allow_query("1 > 2".to_string(), enable_tracing), false); /// assert_eq!(engine.eval_allow_query("1 < 2".to_string(), enable_tracing), true); - /// /// assert_eq!(engine.eval_allow_query("1+1".to_string(), enable_tracing), false); /// assert_eq!(engine.eval_allow_query("true; true".to_string(), enable_tracing), false); /// assert_eq!(engine.eval_allow_query("true; false; true".to_string(), enable_tracing), false); /// # Ok(()) /// # } + /// ``` pub fn eval_allow_query(&mut self, query: String, enable_tracing: bool) -> bool { matches!(self.eval_bool_query(query, enable_tracing), Ok(true)) } @@ -832,6 +835,7 @@ impl Engine { /// assert_eq!(engine.eval_deny_query("true; false; true".to_string(), enable_tracing), true); /// # Ok(()) /// # } + /// ``` pub fn eval_deny_query(&mut self, query: String, enable_tracing: bool) -> bool { !matches!(self.eval_bool_query(query, enable_tracing), Ok(false)) } @@ -857,7 +861,8 @@ impl Engine { // Populate loop hoisting for the query snippet // Query snippets are treated as if they're in a module appended at the end (same as analyzer) // The loop hoisting table already has capacity for this (ensured in prepare_for_eval) - let module_idx = self.modules.len() as u32; + let module_idx = u32::try_from(self.modules.len()) + .map_err(|_| anyhow!("module count exceeds u32::MAX"))?; use crate::compiler::hoist::LoopHoister; @@ -1192,14 +1197,14 @@ impl Engine { /// If `enable` is different from the current value, then any existing coverage /// information will be cleared. pub fn set_enable_coverage(&mut self, enable: bool) { - self.interpreter.set_enable_coverage(enable) + self.interpreter.set_enable_coverage(enable); } #[cfg(feature = "coverage")] #[cfg_attr(docsrs, doc(cfg(feature = "coverage")))] /// Clear the gathered policy coverage data. pub fn clear_coverage_data(&mut self) { - self.interpreter.clear_coverage_data() + self.interpreter.clear_coverage_data(); } /// Gather output from print statements instead of emiting to stderr. @@ -1339,39 +1344,46 @@ impl Engine { let mut modifiers = vec![]; for rule in &m.policy { - // Extract parameter definitions from the policy rule - // e.g. default parameters.a = 5 - if let Rule::Default { refr, .. } = rule.as_ref() { - let path = Parser::get_path_ref_components(refr)?; - let paths: Vec<&str> = path.iter().map(|s| s.text()).collect(); + match *rule.as_ref() { + // Extract parameter definitions from the policy rule + // e.g. default parameters.a = 5 + Rule::Default { ref refr, .. } => { + let path = Parser::get_path_ref_components(refr)?; + let paths: Vec<&str> = path.iter().map(|s| s.text()).collect(); - if paths.len() == 2 && paths[0] == "parameters" { - // Todo: Fetch fields other than name from rego metadoc for the parameter - parameters.push(PolicyParameter { - name: paths[1].to_string(), - modifiable: false, - required: false, - }) - } - } - - // Extract modifiers to the parameters from the policy rule - // e.g. parameters.a = 5 - if let Rule::Spec { head, .. } = rule.as_ref() { - match head { - RuleHead::Compr { refr, .. } => { - let path = Parser::get_path_ref_components(refr)?; - let paths: Vec<&str> = path.iter().map(|s| s.text()).collect(); - - if paths.len() == 2 && paths[0] == "parameters" { + if paths.len() == 2 && paths.first().is_some_and(|p| *p == "parameters") { + if let Some(name) = paths.get(1) { // Todo: Fetch fields other than name from rego metadoc for the parameter - modifiers.push(PolicyModifier { - name: paths[1].to_string(), - }) + parameters.push(PolicyParameter { + name: (*name).to_string(), + modifiable: false, + required: false, + }); } } - RuleHead::Func { .. } => {} - RuleHead::Set { .. } => {} + } + // Extract modifiers to the parameters from the policy rule + // e.g. parameters.a = 5 + Rule::Spec { ref head, .. } => { + match *head { + RuleHead::Compr { ref refr, .. } => { + let path = Parser::get_path_ref_components(refr)?; + let paths: Vec<&str> = path.iter().map(|s| s.text()).collect(); + + if paths.len() == 2 + && paths.first().is_some_and(|p| *p == "parameters") + { + if let Some(name) = paths.get(1) { + // Todo: Fetch fields other than name from rego metadoc for the parameter + modifiers.push(PolicyModifier { + name: (*name).to_string(), + }); + } + } + } + RuleHead::Func { .. } => {} + RuleHead::Set { .. } => {} + } } } }