From 1d71df30b68e6af158681a3fc5e94c3a00fcd8f6 Mon Sep 17 00:00:00 2001 From: Anand Krishnamoorthi <35780660+anakrish@users.noreply.github.com> Date: Mon, 29 Dec 2025 13:53:46 -0600 Subject: [PATCH] chore: Fix lint errors in lookup.rs (#530) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Made the lookup module crate-visible to address clippy’s redundant visibility lint. - Replaced unchecked as casts with a fallible usize_from_u32 helper and propagate conversion errors in lookup accessors. - Switched LookupIndexError to implement core::error::Error for no_std correctness. - Fixed the pattern type mismatch by matching on the value in the Display impl. - Promoted trivial helpers to const fn (new, module_len) per clippy suggestions. - Centralized bounds-checked slot access via slot_ref/slot_mut to keep getters/clearers lint-clean and avoid unchecked indexing. Signed-off-by: Anand Krishnamoorthi --- src/lib.rs | 2 +- src/lookup.rs | 148 +++++++++++++++++++++++++++++++------------------- 2 files changed, 93 insertions(+), 57 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 60e84a8..c02ea44 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -135,7 +135,7 @@ pub mod languages { } mod lexer; -mod lookup; +pub(crate) mod lookup; mod number; mod parser; mod policy_info; diff --git a/src/lookup.rs b/src/lookup.rs index aad6430..f2e4968 100644 --- a/src/lookup.rs +++ b/src/lookup.rs @@ -1,14 +1,5 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -#![allow( - clippy::std_instead_of_core, - clippy::arithmetic_side_effects, - clippy::indexing_slicing, - clippy::missing_const_for_fn, - clippy::if_then_some_else_none, - clippy::as_conversions, - clippy::pattern_type_mismatch -)] //! Lookup table for associating data structures with AST nodes. //! @@ -16,8 +7,14 @@ //! with expressions, queries, and statements using their respective indices. use crate::*; +use core::convert::TryFrom as _; use core::fmt; +#[inline] +fn usize_from_u32(value: u32) -> Option { + usize::try_from(value).ok() +} + /// Error indicating that lookup indices are out of bounds. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum LookupIndexError { @@ -33,7 +30,7 @@ pub enum LookupIndexError { impl fmt::Display for LookupIndexError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { + match *self { LookupIndexError::ModuleOutOfBounds { module_idx, modules, @@ -55,8 +52,7 @@ impl fmt::Display for LookupIndexError { } } -#[cfg(feature = "std")] -impl std::error::Error for LookupIndexError {} +impl core::error::Error for LookupIndexError {} pub type LookupResult = core::result::Result; @@ -75,99 +71,139 @@ impl Default for Lookup { impl Lookup { /// Create a new lookup table. - pub fn new() -> Self { + pub(crate) const fn new() -> Self { Self { slots: Vec::new() } } /// Ensure the lookup table has capacity for the given module and node index. - pub fn ensure_capacity(&mut self, module_idx: u32, node_idx: u32) { - let module_idx = module_idx as usize; - let node_idx = node_idx as usize; + pub(crate) fn ensure_capacity(&mut self, module_idx: u32, node_idx: u32) { + let Some(module_idx) = usize_from_u32(module_idx) else { + debug_assert!(false, "module_idx does not fit in usize"); + return; + }; + let Some(node_idx) = usize_from_u32(node_idx) else { + debug_assert!(false, "node_idx does not fit in usize"); + return; + }; // Ensure we have enough modules if self.slots.len() <= module_idx { - self.slots.resize(module_idx + 1, Vec::new()); + let needed = module_idx.saturating_add(1); + self.slots.resize(needed, Vec::new()); } // Ensure the module has enough capacity for this node index - if self.slots[module_idx].len() <= node_idx { - self.slots[module_idx].resize(node_idx + 1, None); + if let Some(module) = self.slots.get_mut(module_idx) { + if module.len() <= node_idx { + let needed = node_idx.saturating_add(1); + module.resize(needed, None); + } } } /// Set data using direct indices with bounds checking. /// Returns Ok(()) if written, Err if either index is out of bounds. - pub fn set_checked(&mut self, module_idx: u32, node_idx: u32, value: T) -> LookupResult<()> { - let (m, n) = self.validate_indices(module_idx, node_idx)?; - self.slots[m][n] = Some(value); + pub(crate) fn set_checked( + &mut self, + module_idx: u32, + node_idx: u32, + value: T, + ) -> LookupResult<()> { + let slot = self.slot_mut(module_idx, node_idx)?; + *slot = Some(value); Ok(()) } /// Get data using direct indices with bounds checking. /// Returns Ok(None) if the entry is unset, Err if indices are out of range. - pub fn get_checked(&self, module_idx: u32, node_idx: u32) -> LookupResult> { - let (m, n) = self.validate_indices(module_idx, node_idx)?; - Ok(self.slots[m][n].as_ref()) + pub(crate) fn get_checked(&self, module_idx: u32, node_idx: u32) -> LookupResult> { + self.slot_ref(module_idx, node_idx) } /// Clear data at the given indices by setting it to None. - pub fn clear(&mut self, module_idx: u32, node_idx: u32) { - if (module_idx as usize) < self.slots.len() - && (node_idx as usize) < self.slots[module_idx as usize].len() - { - self.slots[module_idx as usize][node_idx as usize] = None; + pub(crate) fn clear(&mut self, module_idx: u32, node_idx: u32) { + if let Ok(slot) = self.slot_mut(module_idx, node_idx) { + *slot = None; } } - pub fn truncate_modules(&mut self, module_count: usize) { + pub(crate) fn truncate_modules(&mut self, module_count: usize) { self.slots.truncate(module_count); } - pub fn module_len(&self) -> usize { + pub(crate) const fn module_len(&self) -> usize { self.slots.len() } - pub fn push_module(&mut self, module: Vec>) { + pub(crate) fn push_module(&mut self, module: Vec>) { self.slots.push(module); } - pub fn remove_module(&mut self, module_idx: usize) -> Option>> { - if module_idx < self.slots.len() { - Some(self.slots.remove(module_idx)) - } else { - None - } + pub(crate) fn remove_module(&mut self, module_idx: usize) -> Option>> { + (module_idx < self.slots.len()).then(|| self.slots.remove(module_idx)) } /// Validate indices and return them as usize on success. fn validate_indices(&self, module_idx: u32, node_idx: u32) -> LookupResult<(usize, usize)> { - let m = module_idx as usize; - if m >= self.slots.len() { - debug_assert!( - m < self.slots.len(), - "module_idx {m} out of bounds (modules={})", - self.slots.len() - ); - return Err(LookupIndexError::ModuleOutOfBounds { - module_idx, - modules: self.slots.len(), - }); - } + let m = usize_from_u32(module_idx).ok_or(LookupIndexError::ModuleOutOfBounds { + module_idx, + modules: self.slots.len(), + })?; + let module = match self.slots.get(m) { + Some(module) => module, + None => { + debug_assert!( + m < self.slots.len(), + "module_idx {m} out of bounds (modules={})", + self.slots.len() + ); + return Err(LookupIndexError::ModuleOutOfBounds { + module_idx, + modules: self.slots.len(), + }); + } + }; - let n = node_idx as usize; - if n >= self.slots[m].len() { + let n = usize_from_u32(node_idx).ok_or(LookupIndexError::NodeOutOfBounds { + module_idx, + node_idx, + nodes: module.len(), + })?; + if n >= module.len() { debug_assert!( - n < self.slots[m].len(), + n < module.len(), "node_idx {n} out of bounds for module {m} (nodes={})", - self.slots[m].len() + module.len() ); return Err(LookupIndexError::NodeOutOfBounds { module_idx, node_idx, - nodes: self.slots[m].len(), + nodes: module.len(), }); } Ok((m, n)) } + + fn slot_ref(&self, module_idx: u32, node_idx: u32) -> LookupResult> { + let (m, n) = self.validate_indices(module_idx, node_idx)?; + let slot = self.slots.get(m).and_then(|module| module.get(n)); + Ok(slot.and_then(|value| value.as_ref())) + } + + fn slot_mut(&mut self, module_idx: u32, node_idx: u32) -> LookupResult<&mut Option> { + let (m, n) = self.validate_indices(module_idx, node_idx)?; + let nodes = self.slots.get(m).map_or(0, |module| module.len()); + if let Some(slot) = self.slots.get_mut(m).and_then(|module| module.get_mut(n)) { + return Ok(slot); + } + + // Should be unreachable because validate_indices guarantees bounds. + debug_assert!(false, "validated indices should be in-bounds"); + Err(LookupIndexError::NodeOutOfBounds { + module_idx, + node_idx, + nodes, + }) + } }