Improve errors in rmc_assemble (#12)

* Include line numbers on assembler errors

* Use .ok_or() instead of match statements

* Return AssemblerErrors from main() instead of Strings

Still not a massive fan of how we're handling errors but it's okay

* Handle read errors instead of panicking

* Improve WriteError error message

Author: RandomSearch18

demos/tests/invalid.lmc

@@ -0,0 +1 @@
+ABC 1

src/rmc_assemble.rs

@@ -1,7 +1,5 @@
 use clap::Parser;
-use std::{
-    collections::HashMap, fs, path::PathBuf
-};
+use std::{collections::HashMap, fmt, fs, io, path::PathBuf};
 
 use rusty_man_computer::value::Value;
 
@@ -38,11 +36,36 @@ enum Line {
 }
 
 #[derive(Debug)]
-enum ParseError {
+enum ParseErrorType {
     InvalidOpcode(String),
     OperandOutOfRange(i16),
 }
 
+impl fmt::Display for ParseErrorType {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match self {
+            ParseErrorType::InvalidOpcode(opcode) => {
+                write!(f, "Invalid opcode: {}", opcode)
+            }
+            ParseErrorType::OperandOutOfRange(value) => {
+                write!(f, "Operand out of range: {}", value)
+            }
+        }
+    }
+}
+
+#[derive(Debug)]
+struct ParseError {
+    error: ParseErrorType,
+    line: usize,
+}
+
+impl fmt::Display for ParseError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "Parse error on line {}: {}", self.line, self.error)
+    }
+}
+
 fn parse_opcode(string: &str) -> Option<Opcode> {
     match string {
         "HLT" => Some(Opcode::HLT),
@@ -64,8 +87,10 @@ fn parse_opcode(string: &str) -> Option<Opcode> {
 fn parse_assembly(program: &str) -> Vec<Result<Line, ParseError>> {
     program
         .lines()
-        .map(|line| {
+        .enumerate()
+        .map(|(line_index, line)| {
             let line = line.trim();
+            let line_number = line_index + 1;
             if line.is_empty() || line.starts_with("//") {
                 return Ok(Line::Empty);
             }
@@ -84,15 +109,14 @@ fn parse_assembly(program: &str) -> Vec<Result<Line, ParseError>> {
             let opcode = match first_part_as_opcode {
                 Some(opcode) => opcode,
                 None => {
-                    let string = match parts.get(1) {
-                        Some(string) => string,
-                        // This means there's only one part: there's nothing to label, so it's just an invalid opcode
-                        None => return Err(ParseError::InvalidOpcode(parts[0].to_string())),
-                    };
-                    match parse_opcode(string) {
-                        Some(opcode) => opcode,
-                        None => return Err(ParseError::InvalidOpcode(string.to_string())),
-                    }
+                    let string = parts.get(1).ok_or(ParseError {
+                        error: ParseErrorType::InvalidOpcode(parts[0].to_string()),
+                        line: line_number,
+                    })?;
+                    parse_opcode(string).ok_or(ParseError {
+                        error: ParseErrorType::InvalidOpcode(string.to_string()),
+                        line: line_number,
+                    })?
                 }
             };
             let operand_part = if label.is_some() {
@@ -106,7 +130,10 @@ fn parse_assembly(program: &str) -> Vec<Result<Line, ParseError>> {
                 Some(string) => match string.parse::<i16>() {
                     Ok(value) => Some(Operand::Value(
                         // If the number doesn't fit within a Value, return an OperandOutOfRange error
-                        Value::new(value).map_err(|_| ParseError::OperandOutOfRange(value))?,
+                        Value::new(value).map_err(|_| ParseError {
+                            error: ParseErrorType::OperandOutOfRange(value),
+                            line: line_number,
+                        })?,
                     )),
                     Err(_) => Some(Operand::Label(string.to_string())),
                 },
@@ -179,10 +206,22 @@ fn generate_machine_code(lines: Vec<Line>) -> Result<Vec<Value>, &'static str> {
     Ok(output)
 }
 
-#[derive(Debug)]
 enum AssemblerError {
     ParseError(ParseError),
     MachineCodeError(&'static str),
+    ReadError(io::Error),
+    WriteError(io::Error),
+}
+
+impl fmt::Debug for AssemblerError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            AssemblerError::ParseError(e) => write!(f, "{}", e),
+            AssemblerError::MachineCodeError(e) => write!(f, "Machine code error: {}", e),
+            AssemblerError::WriteError(e) => write!(f, "Failed to write to output file: {}", e),
+            AssemblerError::ReadError(e) => write!(f, "Failed to read input file: {}", e),
+        }
+    }
 }
 
 fn assemble(program: &str) -> Result<Vec<Value>, AssemblerError> {
@@ -215,26 +254,17 @@ pub struct Args {
     output: PathBuf,
 }
 
-fn main() -> Result<(), String> {
+fn main() -> Result<(), AssemblerError> {
     let args = Args::parse();
-    let program = std::fs::read_to_string(args.program).expect("Failed to read program");
+    let program =
+        std::fs::read_to_string(args.program).map_err(|e| AssemblerError::ReadError(e))?;
     let assembler_result = assemble(&program);
     match assembler_result {
-        Err(error) => match error {
-            AssemblerError::ParseError(parse_error) => {
-                return Err(format!("Parse error: {:?}", parse_error));
-            }
-            AssemblerError::MachineCodeError(message) => {
-                return Err(message.to_string());
-            }
-        },
+        Err(error) => Err(error),
         Ok(machine_code) => {
-            let machine_code_bytes: Vec<u8> = machine_code
-                .iter()
-                .flat_map(|&i| i.to_be_bytes())
-                .collect();
-            fs::write(args.output, machine_code_bytes)
-                .map_err(|e| format!("Failed to write output file: {}", e))
+            let machine_code_bytes: Vec<u8> =
+                machine_code.iter().flat_map(|&i| i.to_be_bytes()).collect();
+            fs::write(args.output, machine_code_bytes).map_err(|e| AssemblerError::WriteError(e))
         }
     }
 }