Nooooooo

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

core/core.mal

@@ -51,6 +51,15 @@
 (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
+        (+ (floor n) 1))))
+
+; Other functions
 (def! assert-msg (fn* [e m]
     (if (not e)
         (raise m))))

libs/string.mal

@@ -23,16 +23,18 @@
 (def! _split-ch (fn* [s c]
     "Split the string at every occurrence of character sc"
     (def! s (boom s))
-    (def! split-r (fn* [l t r]
+    (def! split-r (fn* [l tt r]
         (if (empty? l)
-            (cons t r)
+            (cons tt r)
             (do (def! cc (car l))
                 (if (= cc c)
-                    (split-r (cdr l) "" (cons t r))
+                    (split-r (cdr l) "" (cons tt r))
-                    (split-r (cdr l) (str t cc) r))))))
+                    (split-r (cdr l) (str tt cc) r))))))
     (reverse (split-r s "" '()))))
 
-(def! split-h (fn* [string delimiter]
+(def! split (fn* [string delimiter]
+    "Split the string at every occurrence of substring delimiter"
+    "An empty delimiter is splitting every character"
     (if (= (strlen delimiter) 1)
         (_split-ch string (char delimiter))
         (do (def! delimiter (boom delimiter))
@@ -48,20 +50,13 @@
                             (split-r string delimiter "" (str chunk matches curr) chunks)))))))
             (reverse (split-r (boom string) delimiter "" "" '()))))))
 
-(def! split (fn* [string delimiter]
-    "Split the string at every occurrence of substring delimiter"
-    "An empty delimiter is splitting every character"
-    (if (= (strlen delimiter) 0)
-        (cdr (split-h string delimiter))
-        (split-h string delimiter))))
-
 (def! join (fn* [l s]
     "Join element of list l to a stiring, using s as separator"
     (def! s (or s ""))
-    (def! join-r (fn* [l t]
+    (def! join-r (fn* [l tt]
         (if (empty? l)
-            t
+            tt
-            (join-r (cdr l) (str t s (car l))))))
+            (join-r (cdr l) (str tt s (car l))))))
     (join-r (cdr l) (car l))))
 
 (def! chsub (fn* [s c1 c2]

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()
+            .keys()
-            .map(|(k, _)| k.to_string())
+            .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
@@ -57,12 +56,12 @@ pub fn env_get(env: &Env, sym: &str) -> MalRet {
 
 pub fn env_binds(outer: Env, binds: &MalType, exprs: &[MalType]) -> Result<Env, MalErr> {
     let env = env_new(Some(outer));
-    let binds = binds.if_vec()?;
+    let binds = binds.if_list()?;
     let binl = binds.len();
     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()) {
@@ -128,7 +127,7 @@ pub fn call_func(func: &MalType, args: &[MalType]) -> CallRet {
                 _ => Err(MalErr::unrecoverable("Map argument must be string or key")),
             }
         }
-        M::Vector(v) => {
+        M::Vector(v) | M::List(v) => {
             if args.is_empty() {
                 return Err(MalErr::unrecoverable("No key provided to Vector construct"));
             }
@@ -150,13 +149,25 @@ pub fn call_func(func: &MalType, args: &[MalType]) -> CallRet {
 }
 
 pub fn any_zero(list: &[MalType]) -> Result<&[MalType], MalErr> {
-    if list
+    match list.len() {
-        .iter()
+        1 => {
-        .any(|x| matches!(x, M::Num(v) if v.exact_zero()))
+            if list[0].if_number()?.get_num() == 0 {
-    {
+                Err(MalErr::unrecoverable("Attempting division by 0"))
-        return Err(MalErr::unrecoverable("Attempting division by 0"));
+            } else {
+                Ok(list)
+            }
+        }
+        _ => {
+            if list[1..list.len()]
+                .iter()
+                .any(|x| matches!(x, M::Num(v) if v.exact_zero()))
+            {
+                Err(MalErr::unrecoverable("Attempting division by 0"))
+            } else {
+                Ok(list)
+            }
+        }
     }
-    Ok(list)
 }
 
 pub fn arithmetic_op(set: isize, f: fn(Frac, Frac) -> Frac, args: &[MalType]) -> MalRet {
@@ -169,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

@@ -75,7 +75,7 @@ fn let_star_form(list: &[MalType], env: Env) -> Result<(MalType, Env), MalErr> {
     let inner_env = env_new(Some(env.clone()));
     // change the inner environment
     let (car, cdr) = car_cdr(list)?;
-    let list = car.if_vec()?;
+    let list = car.if_list()?;
     if list.len() % 2 != 0 {
         return Err(MalErr::unrecoverable(
             "let* form, number of arguments must be even",
@@ -114,7 +114,7 @@ fn if_form(list: &[MalType], env: Env) -> MalRet {
 
 fn fn_star_form(list: &[MalType], env: Env) -> MalRet {
     let (binds, exprs) = car_cdr(list)?;
-    binds.if_vec()?;
+    binds.if_list()?;
     Ok(M::MalFun {
         // eval: eval_ast,
         params: Rc::new(binds.clone()),
@@ -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(|_| {
+    let mut file = File::open(filename)
-        MalErr::unrecoverable(format!("Failed to open file '{}'", filename).as_str())
+        .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!(
+        format!("(eval (read-string (str \"(do \" (slurp \"{filename}\") \"\nnil)\")))").as_str(),
-            "(eval (read-string (str \"(do \" (slurp \"{}\") \"\nnil)\")))",
-            filename
-        )
-        .as_str(),
         env,
     )
 } // WTF this is becoming ever less like rust and more like lisp, did I really completely skip the file reading?
@@ -70,15 +65,26 @@ 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* '({}))", argv[1..].iter().map(|x| x.to_string() + " ").collect::<String>()).as_str(), &env).unwrap();
+    eval_str(
+        format!(
+            "(def! *ARGV* '({}))",
+            argv[1..]
+                .iter()
+                .map(|x| "\"".to_string() + x + "\" ")
+                .collect::<String>()
+        )
+        .as_str(),
+        env,
+    )
+    .unwrap();
 }
 
 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
@@ -107,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) => {
@@ -134,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 {
+    //pub fn frac(num: isize, den: usize) -> Self {
-        Self { num, den }
+    //    Self { num, den }
-    }
+    //}
 
     pub fn num(num: isize) -> Self {
         Self { num, den: 1 }
@@ -95,48 +107,64 @@ 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
+    // form without problems, occasional simplification is done for numeric
+    // stability, ensure to simplify after insert and before using any funcition
+    // (floor, ceil, etc.)
     pub fn simplify(&self) -> Frac {
         // Euclid's algorithm to reduce fraction
         // 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> {
-        match tk.find("/") {
+        let frac = match tk.find("/") {
-            Some(v) => Self {
+            Some(v) => {
-                num: tk[0..v].parse::<isize>().unwrap(),
+                let num = match tk[0..v].parse::<isize>() {
-                den: tk[v + 1..tk.len()].parse::<usize>().unwrap(),
+                    Ok(v) => v,
-            },
+                    Err(_) => return None,
-            None => Frac::num(tk.parse::<isize>().unwrap()),
+                };
-        }
+                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
+        Some(frac.simplify())
     }
 }
 
@@ -173,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(),
             )),
@@ -182,9 +210,9 @@ impl MalType {
 
     pub fn if_list(&self) -> Result<&[MalType], MalErr> {
         match self {
-            Self::List(list) => Ok(list),
+            Self::List(list) | Self::Vector(list) => Ok(list),
             _ => Err(MalErr::unrecoverable(
-                format!("{:?} is not a list", prt(self)).as_str(),
+                format!("{:?} is not an iterable", prt(self)).as_str(),
             )),
         }
     }