massively upgrade error handling

1 files changed, 81 insertions, 52 deletions

diff --git a/src/makefile/mod.rs b/src/makefile/mod.rs
index 7c2663b..7077161 100644
--- a/src/makefile/mod.rs
+++ b/src/makefile/mod.rs
@@ -3,11 +3,12 @@ use std::collections::HashMap;
 use std::env;
 use std::fmt;
 use std::fs::File;
-use std::io::{BufRead, BufReader, Error as IoError};
+use std::io::{BufRead, BufReader};
 use std::path::Path;
 use std::rc::Rc;
 
 use crate::args::Args;
+use anyhow::Context;
 use lazy_static::lazy_static;
 use regex::Regex;
 
@@ -131,27 +132,36 @@ impl<'a> Makefile<'a> {
         }
     }
 
-    pub(crate) fn and_read_file(&mut self, path: impl AsRef<Path>) {
+    pub(crate) fn and_read_file(&mut self, path: impl AsRef<Path>) -> anyhow::Result<()> {
         let file = File::open(path);
         // TODO handle errors
-        let file = file.expect("couldn't open makefile!");
+        let file = file.context("couldn't open makefile!")?;
         let file_reader = BufReader::new(file);
-        self.and_read(file_reader);
+        self.and_read(file_reader)
     }
 
-    pub(crate) fn and_read(&mut self, source: impl BufRead) {
-        let mut lines_iter = source.lines().enumerate().peekable();
+    pub(crate) fn and_read(&mut self, source: impl BufRead) -> anyhow::Result<()> {
+        let mut lines_iter = source
+            .lines()
+            .enumerate()
+            .map(|(number, line)| (number + 1, line))
+            .map(|(line, x)| {
+                (
+                    line,
+                    x.with_context(|| format!("failed to read line {} of makefile", line)),
+                )
+            })
+            .peekable();
         let mut conditional_stack: Vec<ConditionalState> = vec![];
         while let Some((line_number, line)) = lines_iter.next() {
-            // TODO handle I/O errors at all
-            let mut line = line.expect("failed to read line of makefile!");
+            let mut line = line?;
 
             // handle escaped newlines
             while line.ends_with('\\') {
                 line.pop();
                 line.push(' ');
                 if let Some((_, x)) = lines_iter.next() {
-                    line.push_str(x.expect("failed to read line of makefile!").trim_start())
+                    line.push_str(x?.trim_start())
                 }
             }
 
@@ -173,20 +183,21 @@ impl<'a> Makefile<'a> {
             if let Some(line) = line.strip_prefix("include ") {
                 // remove extra leading space
                 let line = line.trim_start();
-                let line = self.expand_macros(&tokenize(line), None);
+                let line = self.expand_macros(&tokenize(line)?, None)?;
                 let fields = line.split_whitespace();
                 // POSIX says we only have to handle a single filename, but GNU make
                 // handles arbitrarily many filenames, and it's not like that's more work
                 for field in fields {
-                    self.and_read_file(field);
+                    self.and_read_file(field)?;
                 }
-            } else if let Some(line) = ConditionalLine::from(&line, |t| self.expand_macros(t, None))
+            } else if let Some(line) =
+                ConditionalLine::from(&line, |t| self.expand_macros(t, None))?
             {
                 line.action(
                     conditional_stack.last(),
                     |name| self.macros.is_defined(name),
                     |t| self.expand_macros(t, None),
-                )
+                )?
                 .apply_to(&mut conditional_stack);
             } else if line.trim().is_empty() {
                 // handle blank lines
@@ -196,13 +207,15 @@ impl<'a> Makefile<'a> {
                 // macro tokenizing. so that's suboptimal.
 
                 // TODO errors
-                let line_tokens: TokenString = line.parse().unwrap();
+                let line_tokens: TokenString = line
+                    .parse()
+                    .with_context(|| format!("failed to parse line {}", line_number))?;
 
                 let line_type = LineType::of(&line_tokens);
 
                 match line_type {
-                    LineType::Rule => self.read_rule(&line_tokens, &mut lines_iter),
-                    LineType::Macro => self.read_macro(&line_tokens),
+                    LineType::Rule => self.read_rule(&line_tokens, line_number, &mut lines_iter)?,
+                    LineType::Macro => self.read_macro(&line_tokens, line_number)?,
                     LineType::Unknown => {
                         panic!(
                             "error: line {}: unknown line {:?}",
@@ -212,28 +225,37 @@ impl<'a> Makefile<'a> {
                 }
             }
         }
+
+        Ok(())
     }
 
     fn read_rule(
         &mut self,
         line_tokens: &TokenString,
-        lines_iter: &mut impl Iterator<Item = (usize, Result<String, IoError>)>,
-    ) {
+        line_number: usize,
+        lines_iter: &mut impl Iterator<Item = (usize, anyhow::Result<String>)>,
+    ) -> anyhow::Result<()> {
         let mut lines_iter = lines_iter.peekable();
-        let (targets, not_targets) = line_tokens.split_once(':').unwrap();
-        let targets = self.expand_macros(&targets, None);
+        let (targets, not_targets) = line_tokens
+            .split_once(':')
+            .with_context(|| format!("read_rule couldn't find a ':' on line {}", line_number))?;
+        let targets = self.expand_macros(&targets, None)?;
         let targets = targets.split_whitespace().collect::<Vec<_>>();
         let (prerequisites, mut commands) = match not_targets.split_once(';') {
             Some((prerequisites, mut command)) => {
-                while command.ends_with("\\") && lines_iter.peek().is_some() {
-                    command.strip_suffix("\\");
-                    command.extend(tokenize(&lines_iter.next().unwrap().1.unwrap()));
+                while command.ends_with("\\") {
+                    if let Some((_, next_line)) = lines_iter.next() {
+                        command.strip_suffix("\\");
+                        command.extend(tokenize(&next_line?)?);
+                    } else {
+                        break;
+                    }
                 }
                 (prerequisites, vec![command])
             }
             None => (not_targets, vec![]),
         };
-        let prerequisites = self.expand_macros(&prerequisites, None);
+        let prerequisites = self.expand_macros(&prerequisites, None)?;
         let prerequisites = prerequisites
             .split_whitespace()
             .map(|x| x.into())
@@ -244,7 +266,7 @@ impl<'a> Makefile<'a> {
                 .ok()
                 .map_or(false, |line| line.starts_with('\t') || line.is_empty())
         }) {
-            let mut line = x.unwrap();
+            let mut line = x?;
             if !line.is_empty() {
                 line.remove(0);
             }
@@ -261,7 +283,10 @@ impl<'a> Makefile<'a> {
                     _ => break,
                 }
             }
-            commands.push(line.parse().unwrap());
+            commands.push(
+                line.parse()
+                    .with_context(|| format!("failed to parse line {}", line_number))?,
+            );
         }
 
         let commands = commands
@@ -270,7 +295,7 @@ impl<'a> Makefile<'a> {
             .collect::<Vec<_>>();
 
         if targets.is_empty() {
-            return;
+            return Ok(());
         }
 
         // we don't know yet if it's a target rule or an inference rule
@@ -319,11 +344,15 @@ impl<'a> Makefile<'a> {
                 }
             }
         }
+
+        Ok(())
     }
 
-    fn read_macro(&mut self, line_tokens: &TokenString) {
-        let (name, mut value) = line_tokens.split_once('=').unwrap();
-        let name = self.expand_macros(&name, None);
+    fn read_macro(&mut self, line_tokens: &TokenString, line_number: usize) -> anyhow::Result<()> {
+        let (name, mut value) = line_tokens
+            .split_once('=')
+            .with_context(|| format!("read_rule couldn't find a ':' on line {}", line_number))?;
+        let name = self.expand_macros(&name, None)?;
         // GNUisms are annoying, but popular
         let mut expand_value = false;
         let mut skip_if_defined = false;
@@ -348,17 +377,17 @@ impl<'a> Makefile<'a> {
         value.trim_start();
 
         let value = if expand_value {
-            TokenString::text(self.expand_macros(&value, None))
+            TokenString::text(self.expand_macros(&value, None)?)
         } else {
             value
         };
 
         match self.macros.get(name) {
             // We always let command line or MAKEFLAGS macros override macros from the file.
-            Some((MacroSource::CommandLineOrMakeflags, _)) => return,
+            Some((MacroSource::CommandLineOrMakeflags, _)) => return Ok(()),
             // We let environment variables override macros from the file only if the command-line argument to do that was given
-            Some((MacroSource::Environment, _)) if self.args.environment_overrides => return,
-            _ if skip_if_defined => return,
+            Some((MacroSource::Environment, _)) if self.args.environment_overrides => return Ok(()),
+            _ if skip_if_defined => return Ok(()),
             _ => {}
         }
 
@@ -372,6 +401,8 @@ impl<'a> Makefile<'a> {
             _ => value,
         };
         self.macros.set(name.into(), MacroSource::File, value);
+
+        Ok(())
     }
 
     fn special_target_has_prereq(&self, target: &str, name: &str) -> bool {
@@ -385,13 +416,13 @@ impl<'a> Makefile<'a> {
         }
     }
 
-    pub(crate) fn get_target(&self, name: &str) -> Rc<RefCell<Target>> {
+    pub(crate) fn get_target(&self, name: &str) -> anyhow::Result<Rc<RefCell<Target>>> {
         // TODO implement .POSIX
         let follow_gnu = true;
 
         let vpath_options = match self.macros.get("VPATH") {
             Some((_, vpath)) if follow_gnu => {
-                let vpath = self.expand_macros(vpath, None);
+                let vpath = self.expand_macros(vpath, None)?;
                 env::split_paths(&vpath).collect()
             }
             _ => vec![],
@@ -480,19 +511,19 @@ impl<'a> Makefile<'a> {
         }
 
         let targets = self.targets.borrow();
-        targets.get(name).expect("Target not found!").clone()
+        Ok(targets.get(name).context("Target not found!")?.clone())
     }
 
-    pub(crate) fn update_target(&self, name: &str) {
-        self.get_target(name).borrow().update(self);
+    pub(crate) fn update_target(&self, name: &str) -> anyhow::Result<()> {
+        self.get_target(name)?.borrow().update(self)
     }
 
-    fn expand_macros(&self, text: &TokenString, target: Option<&Target>) -> String {
+    fn expand_macros(&self, text: &TokenString, target: Option<&Target>) -> anyhow::Result<String> {
         let target = target.cloned();
         let lookup_internal = move |name: &str| {
             let target = target
                 .as_ref()
-                .expect("internal macro but no current target!");
+                .context("internal macro but no current target!")?;
             let macro_pieces = if name.starts_with('@') {
                 // The $@ shall evaluate to the full target name of the
                 // current target.
@@ -505,8 +536,8 @@ impl<'a> Makefile<'a> {
                     .iter()
                     .filter(|prereq| {
                         self.get_target(prereq)
-                            .borrow()
-                            .newer_than(target)
+                            .ok()
+                            .and_then(|prereq| prereq.borrow().newer_than(target))
                             .unwrap_or(false)
                     })
                     .cloned()
@@ -531,27 +562,25 @@ impl<'a> Makefile<'a> {
                     .map(|x| {
                         Path::new(&x)
                             .parent()
-                            .expect("no parent")
-                            .to_string_lossy()
-                            .into()
+                            .context("no parent")
+                            .map(|x| x.to_string_lossy().into())
                     })
-                    .collect()
+                    .collect::<Result<_, _>>()?
             } else if name.ends_with('F') {
                 macro_pieces
                     .into_iter()
                     .map(|x| {
                         Path::new(&x)
                             .file_name()
-                            .expect("no filename")
-                            .to_string_lossy()
-                            .into()
+                            .context("no filename")
+                            .map(|x| x.to_string_lossy().into())
                     })
-                    .collect()
+                    .collect::<Result<_, _>>()?
             } else {
                 macro_pieces
             };
 
-            macro_pieces.join(" ")
+            Ok(macro_pieces.join(" "))
         };
 
         self.macros.with_lookup(&lookup_internal).expand(text)