Cleaned up some todos and clippy warnings

numbat/src/column_formatter.rs

@@ -45,9 +45,7 @@ impl ColumnFormatter {
             }
 
             for num_columns in min_num_columns..=self.terminal_width {
-                // TODO: once we have Rust 1.73, use the div_ceil implementation:
-                // let num_rows = entries.len().div_ceil(num_columns);
-                let num_rows = (entries.len() + num_columns - 1) / num_columns;
+                let num_rows = entries.len().div_ceil(num_columns);
 
                 let mut table: Vec<Vec<Option<&str>>> = vec![vec![None; num_columns]; num_rows];
                 for (idx, entry) in entries.iter().enumerate() {

numbat/src/datetime.rs

@@ -37,7 +37,7 @@ pub fn parse_datetime(input: &str) -> Result<Zoned, jiff::Error> {
 
     for format in FORMATS {
         // Try to match the given format plus an additional UTC offset (%z)
-        if let Ok(dt) = Zoned::strptime(&format!("{format} %z"), input) {
+        if let Ok(dt) = Zoned::strptime(format!("{format} %z"), input) {
             return Ok(dt);
         }
 

numbat/src/prefix_parser.rs

@@ -1,5 +1,5 @@
+use indexmap::IndexMap;
 use std::collections::HashMap;
-
 use std::sync::OnceLock;
 
 use crate::span::Span;
@@ -82,10 +82,7 @@ struct UnitInfo {
 
 #[derive(Debug, Clone)]
 pub struct PrefixParser {
-    units: HashMap<String, UnitInfo>,
-    // This is the exact same information as in the "units" hashmap, only faster to iterate over.
-    // TODO: maybe use an external crate for this (e.g. indexmap?)
-    units_vec: Vec<(String, UnitInfo)>,
+    units: IndexMap<String, UnitInfo>,
 
     other_identifiers: HashMap<String, Span>,
 
@@ -95,8 +92,7 @@ pub struct PrefixParser {
 impl PrefixParser {
     pub fn new() -> Self {
         Self {
-            units: HashMap::new(),
-            units_vec: Vec::new(),
+            units: IndexMap::new(),
             other_identifiers: HashMap::new(),
             reserved_identifiers: &["_", "ans"],
         }
@@ -237,7 +233,6 @@ impl PrefixParser {
             full_name: full_name.into(),
         };
         self.units.insert(unit_name.into(), unit_info.clone());
-        self.units_vec.push((unit_name.into(), unit_info));
 
         Ok(())
     }
@@ -260,7 +255,7 @@ impl PrefixParser {
             );
         }
 
-        for (unit_name, info) in &self.units_vec {
+        for (unit_name, info) in &self.units {
             if !input.ends_with(unit_name.as_str()) {
                 continue;
             }

numbat/src/quantity.rs

@@ -192,7 +192,8 @@ impl Quantity {
             let group_representative = group_as_unit
                 .iter()
                 .max_by(|&f1, &f2| {
-                    // TODO: describe this heuristic
+                    // prefer base units over non-base. if multiple base units, prefer
+                    // those with a larger exponent
                     (f1.unit_id.is_base().cmp(&f2.unit_id.is_base()))
                         .then(f1.exponent.cmp(&f2.exponent))
                 })

numbat/src/typechecker/constraints.rs

@@ -160,6 +160,7 @@ pub enum TrivialResolution {
 }
 
 impl TrivialResolution {
+    #[allow(clippy::wrong_self_convention)]
     pub fn is_trivially_violated(self) -> bool {
         matches!(self, TrivialResolution::Violated)
     }

numbat/src/typechecker/mod.rs

@@ -125,7 +125,7 @@ impl TypeChecker {
                     .map_err(TypeCheckError::RegistryError)?;
 
                 // Replace BaseDimension("D") with TVar("D") for all type parameters
-                for (factor, _) in dtype.factors.iter_mut() {
+                for (factor, _) in dtype.factors_mut() {
                     *factor = match factor {
                         DTypeFactor::BaseDimension(ref n)
                             if self

numbat/src/typechecker/substitutions.rs

@@ -92,7 +92,7 @@ impl ApplySubstitution for Type {
 impl ApplySubstitution for DType {
     fn apply(&mut self, substitution: &Substitution) -> Result<(), SubstitutionError> {
         let mut new_dtype = self.clone();
-        for (f, power) in &self.factors {
+        for (f, power) in self.factors() {
             match f {
                 DTypeFactor::TVar(tv) => {
                     if let Some(type_) = substitution.lookup(tv) {

numbat/src/typed_ast.rs

@@ -41,10 +41,22 @@ type DtypeFactorPower = (DTypeFactor, Exponent);
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub struct DType {
     // Always in canonical form
-    pub factors: Vec<DtypeFactorPower>, // TODO make this private
+    factors: Vec<DtypeFactorPower>,
 }
 
 impl DType {
+    pub fn new(factors: Vec<DtypeFactorPower>) -> Self {
+        Self { factors }
+    }
+
+    pub fn factors(&self) -> &[DtypeFactorPower] {
+        &self.factors
+    }
+
+    pub fn factors_mut(&mut self) -> &mut [DtypeFactorPower] {
+        &mut self.factors
+    }
+
     pub fn from_factors(factors: &[DtypeFactorPower]) -> DType {
         let mut dtype = DType {
             factors: factors.into(),