Some fix

Improved number parsing, now does not panic on fail
Clippy-cloppy some stuff

core/core.mal

@@ -51,6 +51,9 @@
 (def! empty? (fn* [l]
     (= (count l) 0)))
 
+(def! mod (fn* [a b]
+    (- a (* (floor (/ a b)) b))))
+
 (def! ceil (fn* [n]
     (if (= (num n) 1)
         n

src/env.rs

@@ -3,7 +3,6 @@ use crate::types::MalErr;
 use crate::types::{Frac, MalMap, MalRet, MalType};
 use std::cell::RefCell;
 use std::rc::Rc;
-use std::usize;
 
 #[derive(Clone)]
 pub struct EnvType {
@@ -16,8 +15,8 @@ impl EnvType {
         let mut keys = self
             .data
             .borrow()
-            .iter()
-            .map(|(k, _)| k.to_string())
+            .keys()
+            .map(|k| k.to_string())
             .collect::<Vec<String>>();
         keys.sort_unstable();
         keys
@@ -45,11 +44,11 @@ pub fn env_get(env: &Env, sym: &str) -> MalRet {
             return Ok(val.clone());
         }
         if let Some(outer) = &iter_env.outer {
-            iter_env = &outer;
+            iter_env = outer;
             continue;
         }
         return Err(MalErr::unrecoverable(
-            format!("symbol {:?} not defined", sym).as_str(),
+            format!("symbol {sym:?} not defined").as_str(),
         ));
     }
     // Recursive was prettier, but we hate recursion
@@ -62,7 +61,7 @@ pub fn env_binds(outer: Env, binds: &MalType, exprs: &[MalType]) -> Result<Env,
     let expl = exprs.len();
     if binl < expl {
         return Err(MalErr::unrecoverable(
-            format!("Expected {} args, got {}", binl, expl).as_str(),
+            format!("Expected {binl} args, got {expl}").as_str(),
         ));
     }
     for (bind, expr) in binds.iter().zip(exprs.iter()) {
@@ -181,11 +180,11 @@ pub fn arithmetic_op(set: isize, f: fn(Frac, Frac) -> Frac, args: &[MalType]) ->
             for el in &args[1..] {
                 left = f(left, el.if_number()?);
 
-                const SIM_TRIG: usize = usize::isqrt(std::isize::MAX as usize);
+                const SIM_TRIG: usize = usize::isqrt(isize::MAX as usize);
 
                 // TODO: consider if simplifying at every operation or every N
                 //          or just at the end, no idea on how it scale
-                if std::cmp::max(left.get_num().abs() as usize, left.get_den()) > SIM_TRIG {
+                if std::cmp::max(left.get_num().unsigned_abs(), left.get_den()) > SIM_TRIG {
                     left = left.simplify();
                 }
             }

src/eval.rs

@@ -129,7 +129,7 @@ pub fn help_form(list: &[MalType], env: Env) -> MalRet {
     let (sym, _) = car_cdr(list)?;
     let sym_str = sym.if_symbol()?;
     match eval(sym, env.clone())? {
-        M::Fun(_, desc) => eprintln!("{}\t[builtin]: {}\n", sym_str, desc),
+        M::Fun(_, desc) => eprintln!("{sym_str}\t[builtin]: {desc}\n"),
         M::MalFun { params, ast, .. } => print_malfun(sym_str, params, ast),
         _ => eprintln!("{}\t[symbol]: {}\n", sym_str, prt(&env_get(&env, sym_str)?)),
     }

src/parse_tools.rs

@@ -35,22 +35,21 @@ pub fn load_home_file(filename: &str, env: &Env, warn: bool) {
 
     if Path::new(&full_filename).exists() {
         if let Err(e) = load_file(&full_filename, env) {
-            eprintln!("; reading \"{}\":", full_filename);
+            eprintln!("; reading \"{full_filename}\":");
             eprintln!("{}", e.message());
         }
     } else if warn {
-        eprintln!("; WARNING: file \"{}\" does not exist", full_filename);
+        eprintln!("; WARNING: file \"{full_filename}\" does not exist");
     }
 }
 
 pub fn read_file(filename: &str) -> Result<MalStr, MalErr> {
-    let mut file = File::open(filename).map_err(|_| {
-        MalErr::unrecoverable(format!("Failed to open file '{}'", filename).as_str())
-    })?;
+    let mut file = File::open(filename)
+        .map_err(|_| MalErr::unrecoverable(format!("Failed to open file '{filename}'").as_str()))?;
     let mut content = String::new();
 
     file.read_to_string(&mut content).map_err(|_| {
-        MalErr::unrecoverable(format!("Failed to read content of '{}'", filename).as_str())
+        MalErr::unrecoverable(format!("Failed to read content of '{filename}'").as_str())
     })?;
 
     Ok(content.into())
@@ -58,11 +57,7 @@ pub fn read_file(filename: &str) -> Result<MalStr, MalErr> {
 
 pub fn load_file(filename: &str, env: &Env) -> MalRet {
     eval_str(
-        format!(
-            "(eval (read-string (str \"(do \" (slurp \"{}\") \"\nnil)\")))",
-            filename
-        )
-        .as_str(),
+        format!("(eval (read-string (str \"(do \" (slurp \"{filename}\") \"\nnil)\")))").as_str(),
         env,
     )
 } // WTF this is becoming ever less like rust and more like lisp, did I really completely skip the file reading?
@@ -70,7 +65,7 @@ pub fn load_file(filename: &str, env: &Env) -> MalRet {
 use rustyline::error::ReadlineError;
 use rustyline::DefaultEditor;
 
-pub fn pre_load(argv: &Vec<String>, env: &Env) {
+pub fn pre_load(argv: &[String], env: &Env) {
     eval_str(
         format!(
             "(def! *ARGV* '({}))",
@@ -80,7 +75,7 @@ pub fn pre_load(argv: &Vec<String>, env: &Env) {
                 .collect::<String>()
         )
         .as_str(),
-        &env,
+        env,
     )
     .unwrap();
 }
@@ -89,7 +84,7 @@ pub fn interactive(env: Env) {
     const HISTORY: &str = ".mal-history";
     let home = get_home_path(&env).unwrap();
     let history = home + "/" + HISTORY;
-    eval_str(format!("(def! MAL_HISTORY \"{}\")", history).as_str(), &env).unwrap();
+    eval_str(format!("(def! MAL_HISTORY \"{history}\")").as_str(), &env).unwrap();
 
     // Using "Editor" instead of the standard I/O because I hate myself but not this much
     // TODO: remove unwrap and switch to a better error handling
@@ -118,14 +113,14 @@ pub fn interactive(env: Env) {
                     // TODO: should handle this in a different way
                     rl.add_history_entry(&line).unwrap_or_default();
                     rl.save_history(&history)
-                        .unwrap_or_else(|e| eprintln!("; WARNING: saving history: {}", e));
+                        .unwrap_or_else(|e| eprintln!("; WARNING: saving history: {e}"));
 
                     parser.push(&line);
 
                     // Perform rep on whole available input
                     match rep(&parser, &env) {
                         Ok(output) => output.iter().for_each(|el| {
-                            eprintln!("; [{}]> {}", num, el);
+                            eprintln!("; [{num}]> {el}");
                             num += 1;
                         }),
                         Err(error) => {
@@ -145,7 +140,7 @@ pub fn interactive(env: Env) {
                 }
                 Err(ReadlineError::Eof) => exit(0),
                 Err(err) => {
-                    eprint!("; Error reading lnie: {:?}", err);
+                    eprint!("; Error reading lnie: {err:?}");
                     break;
                 }
             }

src/printer.rs

@@ -56,7 +56,7 @@ pub fn pr_str(ast: &MalType, print_readably: bool) -> String {
         M::Atom(sub) => format!("Atom({})", pr_str(&sub.borrow(), print_readably)),
         M::Ch(c) => {
             if print_readably {
-                format!("#\\{}", c)
+                format!("#\\{c}")
             } else {
                 c.to_string()
             }

src/reader.rs

@@ -83,7 +83,7 @@ impl Reader {
             "]" => Ok(Vector(vector.into())),
             "}" => make_map(vector.into()),
             t => Err(MalErr::unrecoverable(
-                format!("Unknown collection terminator: {}", t).as_str(),
+                format!("Unknown collection terminator: {t}").as_str(),
             )),
         }
     }
@@ -99,7 +99,12 @@ impl Reader {
                     .unwrap()
                     .is_match(tk)
                 {
-                    return Ok(Num(Frac::from_str(&tk)));
+                    return match Frac::from_str(tk) {
+                        Some(v) => Ok(Num(v)),
+                        None => Err(MalErr::unrecoverable(
+                            format!("Cannot parse {tk} as a number").as_str(),
+                        )),
+                    };
                 }
                 if tk.starts_with('\"') {
                     if tk.len() > 1 && tk.ends_with('\"') {
@@ -110,7 +115,7 @@ impl Reader {
                     ));
                 }
                 if tk.starts_with(':') {
-                    return Ok(Key(format!("ʞ{}", tk).into()));
+                    return Ok(Key(format!("ʞ{tk}").into()));
                 }
                 Ok(Sym(tk.into()))
             }
@@ -191,7 +196,7 @@ mod tests {
     fn reader_setup1() -> Reader {
         let r = Reader::new();
         r.push("()[]{} \"str\" :key sym 1 ; comment");
-        return r;
+        r
     }
 
     #[test]
@@ -255,7 +260,7 @@ mod tests {
         let r = reader_setup1();
         assert!(!r.ended());
         for _ in r.tokens.borrow().iter() {
-            assert!(matches!(r.next(), Ok(_)))
+            assert!(r.next().is_ok())
         }
         assert!(r.ended());
     }

src/types.rs

@@ -54,7 +54,7 @@ impl Div for Frac {
         let other_sign = other.num.signum();
         Self {
             num: self.num * other.den as isize * other_sign,
-            den: self.den * other.num.abs() as usize,
+            den: self.den * other.num.unsigned_abs(),
         }
     }
 }
@@ -68,14 +68,26 @@ impl PartialOrd for Frac {
 impl PartialEq for Frac {
     fn eq(&self, other: &Self) -> bool {
         (!((self.num < 0) ^ (other.num < 0)))
-            && self.num.abs() as usize * other.den == other.num.abs() as usize * self.den
+            && self.num.unsigned_abs() * other.den == other.num.unsigned_abs() * self.den
+    }
+}
+
+use std::fmt;
+
+impl fmt::Display for Frac {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        if self.den == 1 {
+            write!(f, "{}", self.num)
+        } else {
+            write!(f, "{}/{}", self.num, self.den)
+        }
     }
 }
 
 impl Frac {
-    pub fn frac(num: isize, den: usize) -> Self {
-        Self { num, den }
-    }
+    //pub fn frac(num: isize, den: usize) -> Self {
+    //    Self { num, den }
+    //}
 
     pub fn num(num: isize) -> Self {
         Self { num, den: 1 }
@@ -95,14 +107,14 @@ impl Frac {
 
     fn _gcd(&self) -> usize {
         let mut t: usize;
-        let mut a = self.num.abs() as usize;
+        let mut a = self.num.unsigned_abs();
         let mut b = self.den;
         while b > 0 {
             t = b;
             b = a % b;
             a = t;
         }
-        return a;
+        a
     }
 
     // Ideally builtin functions ( + - * / ) can operate with the values in any
@@ -114,39 +126,45 @@ impl Frac {
         // TODO: (decide if implementing this automathically once fraction
         //          numbers become bigger than specified)
         let gcd = self._gcd();
-        return Frac {
+        Frac {
             num: self.num / gcd as isize,
             den: self.den / gcd,
-        };
+        }
     }
 
     pub fn int(&self) -> isize {
         self.num / self.den as isize
     }
 
-    pub fn to_string(&self) -> String {
-        let mut tmp = self.num.to_string();
-        if self.den != 1 {
-            tmp = tmp + "/" + &self.den.to_string()
-        }
-        return tmp;
-    }
     // return Ok(Num(Frac::num(tk.parse::<isize>().unwrap())));
 
-    pub fn from_str(tk: &str) -> Self {
+    pub fn from_str(tk: &str) -> Option<Self> {
         let frac = match tk.find("/") {
-            Some(v) => Self {
-                num: tk[0..v].parse::<isize>().unwrap(),
-                den: tk[v + 1..tk.len()].parse::<usize>().unwrap(),
-            },
-            None => Frac::num(tk.parse::<isize>().unwrap()),
+            Some(v) => {
+                let num = match tk[0..v].parse::<isize>() {
+                    Ok(v) => v,
+                    Err(_) => return None,
+                };
+                let den = match tk[v + 1..tk.len()].parse::<usize>() {
+                    Ok(v) => v,
+                    Err(_) => return None,
+                };
+                Self { num, den }
+            }
+            None => {
+                let num = match tk.parse::<isize>() {
+                    Ok(v) => v,
+                    Err(_) => return None,
+                };
+                Self::num(num)
+            }
         };
         // Ensure that value is simplified before being inserted
         // otherwise
         // (/ 4 4)  results in 1/1
         // 4/4      results in 4/4
         // this breaks some functions (like ceil) and doesn't make much sense
-        frac.simplify()
+        Some(frac.simplify())
     }
 }
 
@@ -183,7 +201,7 @@ impl Default for &MalType {
 impl MalType {
     pub fn if_number(&self) -> Result<Frac, MalErr> {
         match self {
-            Self::Num(val) => Ok(val.clone()),
+            Self::Num(val) => Ok(*val),
             _ => Err(MalErr::unrecoverable(
                 format!("{:?} is not a number", prt(self)).as_str(),
             )),