clippy moment

src/bin/main.rs

@@ -3,9 +3,9 @@ use std::{rc::Rc, cell::RefCell, fs};
 use complexpr::{eval::Environment, interpreter::interpret, value::Value, stdlib};
 use rustyline::{self, error::ReadlineError};
 
-const C_RESET: &'static str = "\x1b[0m";
-const C_BLUE: &'static str = "\x1b[94m";
-const PROMPT: &'static str = "\x1b[94m>> \x1b[0m";
+const C_RESET: &str = "\x1b[0m";
+const C_BLUE: &str = "\x1b[94m";
+const PROMPT: &str = "\x1b[94m>> \x1b[0m";
 
 fn main() -> Result<(), Box<dyn std::error::Error>> {
     let args: Vec<String> = std::env::args().collect();

src/eval.rs

@@ -23,12 +23,12 @@ impl Environment {
         Self { parent: Some(parent), map: HashMap::new() }
     }
 
-    pub fn get(&self, name: &str) -> Result<Value,()> {
+    pub fn get(&self, name: &str) -> Option<Value> {
         match self.map.get(name) {
-            Some(v) => Ok(v.clone()),
+            Some(v) => Some(v.clone()),
             None => match self.parent {
                 Some(ref p) => p.borrow().get(name),
-                None => Err(())
+                None => None
             }
         }
     }
@@ -48,6 +48,12 @@ impl Environment {
     }
 }
 
+impl Default for Environment {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
 pub enum Unwind {
     Continue{pos: Position}, Break{pos: Position}, Return{pos: Position, value: Value}, Error(RuntimeError)
 }
@@ -69,7 +75,7 @@ impl From<RuntimeError> for Unwind {
     }
 }
 
-fn unwrap_ident_token<'a>(tok: &'a Token) -> &'a Rc<str> {
+fn unwrap_ident_token(tok: &Token) -> &Rc<str> {
     if let Token { ty: TokenType::Ident(s),.. } = tok {
         s
     } else {
@@ -97,31 +103,33 @@ pub fn eval_stmt(stmt: &Stmt, env: EnvRef) -> Result<(), Unwind> {
             for ic in if_clauses {
                 let cond = eval_expr(&ic.0, env.clone())?;
                 if cond.truthy() {
-                    return eval_stmt(&ic.1, env.clone())
+                    return eval_stmt(&ic.1, env)
                 }
             }
             if let Some(ec) = else_clause {
-                return eval_stmt(&ec, env)
+                return eval_stmt(ec, env)
             }
         },
         Stmt::For { var, expr, stmt } => {
             let name = unwrap_ident_token(var);
             let iter = eval_expr(expr, env.clone())?;
             env.borrow_mut().declare(name.clone(), Value::Nil);
-            if let Ok(i) = iter.iter() {
+            let iterator = iter.iter();
+            if let Err(e) = iterator {
+                return Err(RuntimeError { message: e, pos: var.pos.clone() }.into())
+            }
+            if let Ok(i) = iterator {
                 for v in i {
                     let env = env.clone();
                     env.borrow_mut().set(name.clone(), v).expect("unreachable");
-                    match eval_stmt(&stmt, env) {
+                    match eval_stmt(stmt, env) {
                         Ok(_) => (),
                         Err(Unwind::Break{..}) => break,
                         Err(Unwind::Continue{..}) => continue,
                         Err(e) => return Err(e)
                     }
                 }
-            } else {
-                return Err(RuntimeError { message: "Cannot iterate this type".into(), pos: var.pos.clone() }.into())
-            };
+            } 
         },
         Stmt::While { expr, stmt } => {
             loop {
@@ -129,7 +137,7 @@ pub fn eval_stmt(stmt: &Stmt, env: EnvRef) -> Result<(), Unwind> {
                 if !cond.truthy() {
                     break
                 }
-                match eval_stmt(&stmt, env.clone()) {
+                match eval_stmt(stmt, env.clone()) {
                     Ok(_) => (),
                     Err(Unwind::Break{..}) => break,
                     Err(Unwind::Continue{..}) => continue,
@@ -201,15 +209,15 @@ pub fn eval_ident(token: &Token, env: EnvRef) -> Result<Value, RuntimeError> {
     if let Token { ty: TokenType::Ident(name), ..} = token {
         env.borrow_mut()
             .get(name)
-            .map_err(|_| RuntimeError { message: "Variable not defined in scope".into(), pos: token.pos.clone() })
+            .ok_or_else(|| RuntimeError { message: "Variable not defined in scope".into(), pos: token.pos.clone() })
     } else { 
         unreachable!() 
     }
 }
 
-pub fn eval_assignment(lhs: &Box<Expr>, rhs: &Box<Expr>, op: &Token, env: EnvRef) -> Result<Value, RuntimeError> {
+pub fn eval_assignment(lhs: &Expr, rhs: &Expr, op: &Token, env: EnvRef) -> Result<Value, RuntimeError> {
     // lhs must be an identifier (checked in parser)
-    if let Expr::Ident{value: Token{ty: TokenType::Ident(name),..}} = &**lhs {
+    if let Expr::Ident{value: Token{ty: TokenType::Ident(name),..}} = lhs {
         if op.ty == TokenType::Equal {
             // plain assignment
             let r = eval_expr(rhs, env.clone())?;
@@ -221,7 +229,7 @@ pub fn eval_assignment(lhs: &Box<Expr>, rhs: &Box<Expr>, op: &Token, env: EnvRef
             // compound assignment
             let prev_value = env.borrow_mut()
                 .get(name)
-                .map_err(|_| RuntimeError { message: "Variable not defined in scope".into(), pos: op.pos.clone()})?;
+                .ok_or_else(|| RuntimeError { message: "Variable not defined in scope".into(), pos: op.pos.clone()})?;
             let r = eval_expr(rhs, env.clone())?;
 
             let result = match op.ty {
@@ -237,7 +245,7 @@ pub fn eval_assignment(lhs: &Box<Expr>, rhs: &Box<Expr>, op: &Token, env: EnvRef
                 .set(name.clone(), result.clone()).expect("unreachable");
             Ok(result)
         }
-    } else if let Expr::Index { lhs, index, pos } = &**lhs {
+    } else if let Expr::Index { lhs, index, pos } = lhs {
         let l = eval_expr(lhs, env.clone())?;
         let idx = eval_expr(index, env.clone())?;
         if op.ty == TokenType::Equal {
@@ -263,7 +271,7 @@ pub fn eval_assignment(lhs: &Box<Expr>, rhs: &Box<Expr>, op: &Token, env: EnvRef
     }
 }
 
-pub fn eval_binary(lhs: &Box<Expr>, rhs: &Box<Expr>, op: &Token, env: EnvRef) -> Result<Value, RuntimeError> {
+pub fn eval_binary(lhs: &Expr, rhs: &Expr, op: &Token, env: EnvRef) -> Result<Value, RuntimeError> {
     let l = eval_expr(lhs, env.clone())?;
     let r = eval_expr(rhs, env)?;
     match op.ty {
@@ -276,7 +284,7 @@ pub fn eval_binary(lhs: &Box<Expr>, rhs: &Box<Expr>, op: &Token, env: EnvRef) ->
     }.map_err(|e| RuntimeError { message: e, pos: op.pos.clone() })
 }
 
-pub fn eval_boolean(lhs: &Box<Expr>, rhs: &Box<Expr>, op: &Token, env: EnvRef) -> Result<Value, RuntimeError> {
+pub fn eval_boolean(lhs: &Expr, rhs: &Expr, op: &Token, env: EnvRef) -> Result<Value, RuntimeError> {
     let l = eval_expr(lhs, env.clone())?;
     match op.ty {
         TokenType::DoubleAmper => if l.truthy() {
@@ -293,7 +301,7 @@ pub fn eval_boolean(lhs: &Box<Expr>, rhs: &Box<Expr>, op: &Token, env: EnvRef) -
     }
 }
 
-pub fn eval_comp(lhs: &Box<Expr>, rhs: &Box<Expr>, op: &Token, env: EnvRef) -> Result<Value, RuntimeError> {
+pub fn eval_comp(lhs: &Expr, rhs: &Expr, op: &Token, env: EnvRef) -> Result<Value, RuntimeError> {
     let l = eval_expr(lhs, env.clone())?;
     let r = eval_expr(rhs, env)?;
     let mk_err = || RuntimeError {
@@ -322,7 +330,7 @@ pub fn eval_comp(lhs: &Box<Expr>, rhs: &Box<Expr>, op: &Token, env: EnvRef) -> R
     }
 }
 
-pub fn eval_unary(arg: &Box<Expr>, op: &Token, env: EnvRef) -> Result<Value, RuntimeError> {
+pub fn eval_unary(arg: &Expr, op: &Token, env: EnvRef) -> Result<Value, RuntimeError> {
     let a = eval_expr(arg, env)?;
     match op.ty {
         TokenType::Minus => -a,

src/expr.rs

@@ -49,7 +49,7 @@ impl fmt::Debug for Expr {
 
 impl Expr {
     pub fn is_lvalue(&self) -> bool {
-        return matches!(self, Expr::Ident{..} | Expr::Index{..})
+        matches!(self, Expr::Ident{..} | Expr::Index{..})
     }
 
     pub fn is_assignment(&self) -> bool {

src/lexer.rs

@@ -15,7 +15,7 @@ pub struct Lexer {
 
 impl Lexer {
     pub fn new(code: &str, filename: Option<String>) -> Self {
-        Self { filename: filename.map(|s| Rc::from(s)), line: 1, col: 1, tokens: vec![], start: 0, current: 0, code: code.chars().collect() }
+        Self { filename: filename.map(Rc::from), line: 1, col: 1, tokens: vec![], start: 0, current: 0, code: code.chars().collect() }
     }
 
     pub fn into_tokens(self) -> Vec<Token> {
@@ -181,7 +181,7 @@ impl Lexer {
             // TODO Escapes
         }
         if self.at_end() { return Err(self.mk_error("Unexpected EOF in character literal")) }
-        self.expect(&['\'']).ok_or(self.mk_error("Expected ' to terminate character literal"))?;
+        self.expect(&['\'']).ok_or_else(|| self.mk_error("Expected ' to terminate character literal"))?;
         self.add_token(TokenType::Char(c), self.collect_literal());
         Ok(())
     }

src/parser.rs

@@ -126,10 +126,10 @@ impl Parser {
                     }
                 }
                 let next = self.next();
-                return match next.ty {
+                match next.ty {
                     TokenType::Semicolon => Ok(Stmt::Let{lhs: tok, rhs: Some(*rhs)}),
                     _ => Err(self.mk_error("Missing semicolon after 'let' statement".to_owned()))
-                };
+                }
             } else {
                 Err(self.mk_error("Invalid expression after 'let'".to_owned()))
             }
@@ -142,10 +142,10 @@ impl Parser {
                 }
             }
             let next = self.next();
-            return match next.ty {
+            match next.ty {
                 TokenType::Semicolon => Ok(Stmt::Let{lhs: tok, rhs: None}),
                 _ => Err(self.mk_error("Missing semicolon after 'let' statement".to_owned()))
-            };
+            }
         } else {
             Err(self.mk_error("Invalid expression after 'let'".to_owned()))
         }
@@ -164,15 +164,13 @@ impl Parser {
                 _ => break
             }
         }
-        let else_clause;
-        if ec {
-            else_clause = Some(Box::new(self.statement()?));
+        let else_clause = if ec {
+            Some(Box::new(self.statement()?))
         } else {
-            else_clause = None;
-        }
-        return Ok(Stmt::If{ 
-            if_clauses: if_clauses,
-            else_clause: else_clause
+            None
+        };
+        Ok(Stmt::If{ 
+            if_clauses, else_clause
         })
     }
 
@@ -210,7 +208,7 @@ impl Parser {
         }
         self.err_on_eof()?;
         self.next();
-        return Ok(Stmt::Block{ stmts })
+        Ok(Stmt::Block{ stmts })
     }
 
     // Generic method for left-associative operators
@@ -320,7 +318,7 @@ impl Parser {
     fn fncall_inner(&mut self, expr: Expr) -> Result<Expr, ParserError> {
         let lparen = self.next();
         let args = self.commalist(TokenType::RParen, Self::assignment)?;
-        Ok(Expr::FuncCall { func: Box::new(expr), args, pos: lparen.pos.clone() })
+        Ok(Expr::FuncCall { func: Box::new(expr), args, pos: lparen.pos })
     }
 
     fn arrindex_inner(&mut self, expr: Expr) -> Result<Expr, ParserError> {
@@ -328,9 +326,9 @@ impl Parser {
         let index = self.assignment()?;
         self.err_on_eof()?;
         if self.next().ty != TokenType::RBrack {
-            return Err(ParserError { message: "Expected RBrack after collection index".into(), pos: lbrack.pos.clone() });
+            return Err(ParserError { message: "Expected RBrack after collection index".into(), pos: lbrack.pos });
         }
-        Ok(Expr::Index { lhs: Box::new(expr), index: Box::new(index), pos: lbrack.pos.clone() })
+        Ok(Expr::Index { lhs: Box::new(expr), index: Box::new(index), pos: lbrack.pos })
     }
 
     fn expr_base(&mut self) -> Result<Expr, ParserError> {

src/stdlib/mod.rs

@@ -1,4 +1,4 @@
-use std::{rc::Rc, io::Write};
+use std::{rc::Rc, io::Write, cmp::Ordering};
 
 use num_traits::ToPrimitive;
 
@@ -53,7 +53,7 @@ fn fn_input(_: Vec<Value>) -> Result<Value, String> {
     let mut buffer = String::new();
     let stdin = std::io::stdin();
     stdin.read_line(&mut buffer).map_err(|e| e.to_string())?;
-    if buffer.ends_with("\n") {
+    if buffer.ends_with('\n') {
         buffer.pop();
     }
     Ok(Value::from(buffer))
@@ -84,14 +84,12 @@ fn fn_range(args: Vec<Value>) -> Result<Value, String> {
     let min = &args[0];
     let max = &args[1];
     match (min, max) {
-        (Value::Int(a), Value::Int(b)) => {
-            if a == b {
-                Ok(Value::from(vec![]))
-            } else if a < b {
-                Ok(Value::from((*a..*b).map(|x| Value::Int(x)).collect::<Vec<Value>>()))
-            } else {
-                Ok(Value::from(((*b+1)..(*a+1)).rev().map(|x| Value::Int(x)).collect::<Vec<Value>>()))
-            }
+        (Value::Int(a), Value::Int(b)) => match a.cmp(b) {
+            Ordering::Equal => Ok(Value::from(vec![])),
+            Ordering::Less =>
+                Ok(Value::from((*a..*b).map(Value::Int).collect::<Vec<Value>>())),
+            Ordering::Greater =>
+                Ok(Value::from(((*b+1)..(*a+1)).rev().map(Value::Int).collect::<Vec<Value>>()))
         },
         _ => Err("Both arguments to range must be integers".into())
     }

src/token.rs

@@ -65,7 +65,7 @@ impl TokenType {
     }
 }
 
-#[derive(Clone,Copy,Debug,PartialEq)]
+#[derive(Clone,Copy,Debug,PartialEq,Eq)]
 pub enum OpType {
     Assignment, Comparison, Pipeline, Additive, Multiplicative, Exponential, Boolean
 }

src/value.rs

@@ -61,28 +61,27 @@ impl Value {
         }
     }
 
-    pub fn iter(&self) -> Result<Box<dyn Iterator<Item=Value> + '_>, ()> {
+    pub fn iter(&self) -> Result<Box<dyn Iterator<Item=Value> + '_>, String> {
         match self {
             Value::String(s) 
                 => Ok(Box::new(s.chars()
-                    .map(|c| Value::Char(c)))),
+                    .map(Value::Char))),
             Value::List(l) => Ok(Box::new(l.borrow().clone().into_iter())),
-            _ => Err(())
+            v => Err(format!("{:?} is not iterable", v))
         }
     }
 
     pub fn call(&self, args: Vec<Value>, pos: &Position) -> Result<Value, RuntimeError> {
         match self {
             Value::BuiltinFn(f) => {
-                if args.len() == f.arg_count {
-                    (f.func)(args).map_err(|e| RuntimeError { message: e, pos: pos.clone() })
-                } else if args.len() < f.arg_count {
-                    Err(RuntimeError { 
+                match args.len().cmp(&f.arg_count) {
+                    Ordering::Equal => 
+                        (f.func)(args).map_err(|e| RuntimeError { message: e, pos: pos.clone() }),
+                    Ordering::Less => Err(RuntimeError { 
                         message: format!("Not enough arguments for function: expected {}, got {}", f.arg_count, args.len()), 
                         pos: pos.clone() 
-                    })
-                } else {
-                    Err(RuntimeError { 
+                    }),
+                    Ordering::Greater => Err(RuntimeError { 
                         message: format!("Too many arguments for function: expected {}, got {}", f.arg_count, args.len()), 
                         pos: pos.clone() 
                     })
@@ -133,7 +132,7 @@ impl Value {
             Self::String(s) => match idx {
                 Value::Int(i) if *i >= 0 => s.chars().nth(*i as usize)
                     .ok_or_else(|| format!("String index {} out of bounds for length {}", i, s.chars().count()))
-                    .map(|c| Value::Char(c)),
+                    .map(Value::Char),
                 Value::Int(i) => Err(format!("String index {} cannot be negative", i)),
                 _ => Err(format!("Cannot index {:?} with {:?}", self, idx))
             },