From e00df9d5ac9caf90967bc7c53035457a11aea37d Mon Sep 17 00:00:00 2001
From: Alex Bethel <abethel@cs.nmt.edu>
Date: Tue, 13 Jul 2021 13:54:27 -0500
Subject: [PATCH] Prettier error handling

---
 src/brian.rs     | 33 +++++++++++++++++++++++++++++++++
 src/error.rs     | 33 ++++++++++++++++++++++++++++++++-
 src/repl.rs      | 19 +++++++++----------
 src/variables.rs |  2 +-
 4 files changed, 75 insertions(+), 12 deletions(-)

diff --git a/src/brian.rs b/src/brian.rs
index 8ee63c1f..1a4bf120 100644
--- a/src/brian.rs
+++ b/src/brian.rs
@@ -20,6 +20,8 @@
 
 use std::{
     collections::VecDeque,
+    error::Error,
+    fmt::Display,
     io::{Read, Write},
 };
 
@@ -288,6 +290,25 @@ pub enum ProgramError {
     TapeSizeExceededLimit,
 }
 
+impl Display for ProgramError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(
+            f,
+            "{}",
+            match self {
+                ProgramError::DataPointerUnderflow => "data pointer underflow",
+                ProgramError::IntegerOverflow => "integer overflow",
+                ProgramError::IntegerUnderflow => "integer underflow",
+                ProgramError::InputReadError => "input read error",
+                ProgramError::UnmatchedOpeningBracket => "unmatched `[`",
+                ProgramError::UnmatchedClosingBracket => "unmatched `]`",
+                ProgramError::TapeSizeExceededLimit => "tape size exceeded",
+            }
+        )
+    }
+}
+impl Error for ProgramError {}
+
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 /// An error that occurred while the interpreter was being run start-to-end all in one go
 pub enum InterpretError {
@@ -297,6 +318,18 @@ pub enum InterpretError {
     OutputWriteError,
 }
 
+impl Display for InterpretError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            InterpretError::ProgramError(e) => write!(f, "program error: {}", e),
+            InterpretError::EndOfInput => write!(f, "unexpected end of input"),
+            InterpretError::OutputBufferFull => write!(f, "output buffer full"),
+            InterpretError::OutputWriteError => write!(f, "output write error"),
+        }
+    }
+}
+impl Error for InterpretError {}
+
 impl From<ProgramError> for InterpretError {
     fn from(e: ProgramError) -> Self {
         InterpretError::ProgramError(e)
diff --git a/src/error.rs b/src/error.rs
index a17aa5bc..ea31434a 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -1,4 +1,4 @@
-use std::{io, ops::Range};
+use std::{fmt::Display, io, ops::Range};
 
 use crate::{brian::InterpretError, lexer::Token};
 
@@ -36,6 +36,37 @@ impl Error {
     }
 }
 
+impl Display for Error {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(
+            f,
+            "Error at range {}-{}: {}",
+            self.span.start, self.span.end, self.kind
+        )
+    }
+}
+impl std::error::Error for Error {}
+
+impl Display for ErrorKind {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            ErrorKind::SyntaxError(desc) => write!(f, "syntax error: {}", desc),
+            ErrorKind::UnexpectedEof => write!(f, "unexpected end of file"),
+            ErrorKind::UnexpectedToken(token) => write!(f, "unexpected token {:?}", token),
+            ErrorKind::InvalidIdentifier => write!(f, "invalid identifier"),
+            ErrorKind::UnknownVariable(name) => write!(f, "unknown identifier \"{}\"", name),
+            ErrorKind::MeloVariable(name) => write!(f, "banned variable \"{}\"", name),
+            ErrorKind::TypeError(desc) => write!(f, "type error: {}", desc),
+            ErrorKind::TopLevelBreak => write!(f, "can only `break` out of a loop"),
+            ErrorKind::BfInterpretError(err) => write!(f, "brainfuck error: {}", err),
+            // TODO: give concrete numbers here.
+            ErrorKind::MismatchedArgumentError => write!(f, "wrong number of function arguments"),
+            ErrorKind::MissingLhs => write!(f, "missing expression before binary operation"),
+            ErrorKind::IOError(err) => write!(f, "I/O error: {}", err),
+        }
+    }
+}
+
 impl From<io::Error> for Error {
     fn from(e: io::Error) -> Self {
         Self {
diff --git a/src/repl.rs b/src/repl.rs
index 21fd50c0..29486c38 100644
--- a/src/repl.rs
+++ b/src/repl.rs
@@ -1,4 +1,3 @@
-use logos::Source;
 use rustyline::Editor;
 
 use crate::{interpret::ExecEnv, parser::Parser};
@@ -10,7 +9,13 @@ pub fn repl(ast_print: bool) {
         let readline = rl.readline(":: ");
         match readline {
             Ok(line) => {
-                if &line == "exit" {
+                // NOTE(Alex): `readline()` leaves a newline at the
+                // end of the string if stdin is connected to a file
+                // or unsupported terminal; this can interfere with
+                // error printing.
+                let line = line.trim_end();
+
+                if line == "exit" {
                     println!("bye");
                     break;
                 }
@@ -23,16 +28,10 @@ pub fn repl(ast_print: bool) {
                 });
 
                 if let Err(e) = value {
-                    println!(
-                        "Error `{:?}` occurred at span: {:?} = `{:?}`",
-                        e.kind,
-                        e.span.clone(),
-                        line.slice(e.span.clone())
-                    );
-
+                    println!("{}", e);
                     println!(" | {}", line);
                     println!(
-                        "   {}{}-- Here",
+                        "   {}{}",
                         " ".repeat(e.span.start),
                         "^".repeat((e.span.end - e.span.start).max(1))
                     );
diff --git a/src/variables.rs b/src/variables.rs
index f90173e0..db8d53cc 100644
--- a/src/variables.rs
+++ b/src/variables.rs
@@ -126,7 +126,7 @@ impl Value {
         match self {
             Value::Int(i) => Ok(i),
             _ => Err(Error {
-                kind: ErrorKind::TypeError(format!("Expected int, got {}", self)),
+                kind: ErrorKind::TypeError(format!("expected int, got {}", self)),
                 span: span.clone(),
             }),
         }