correctly handle conditional lines inside rule body

1 files changed, 174 insertions, 234 deletions

diff --git a/src/makefile/input.rs b/src/makefile/input.rs
index e8a4a42..918ce0a 100644
--- a/src/makefile/input.rs
+++ b/src/makefile/input.rs
@@ -1,10 +1,10 @@
-use std::cell::{Cell, RefCell};
+use std::cell::Cell;
 use std::collections::HashMap;
+use std::error::Error as StdError;
 use std::fs::File;
-use std::io::{BufRead, BufReader};
+use std::io::{BufRead, BufReader, Error as IoError, Lines};
 use std::iter::Peekable;
 use std::path::Path;
-use std::rc::Rc;
 
 use eyre::{bail, eyre, Context, Result};
 use lazy_static::lazy_static;
@@ -87,108 +87,90 @@ fn inference_match<'a>(
     }
 }
 
-pub struct MakefileReader<'a> {
-    inference_rules: Vec<InferenceRule>,
-    macros: MacroSet<'static, 'static>,
-    targets: HashMap<String, Target>,
-    pub first_non_special_target: Option<String>,
-    args: &'a Args,
-    // TODO borrow warnings from Python version
-}
+struct LineNumbers<T, E: StdError + Send + Sync + 'static, Inner: Iterator<Item = Result<T, E>>>(
+    Inner,
+    usize,
+);
 
-impl<'a> MakefileReader<'a> {
-    pub fn new(args: &'a Args) -> Self {
-        let mut inference_rules = vec![];
-        let mut macros = MacroSet::new();
-        let mut targets = HashMap::new();
-        let first_non_special_target = None;
-
-        if !args.no_builtin_rules {
-            inference_rules.extend(builtin_inference_rules());
-            macros.add_builtins();
-            targets.extend(builtin_targets().into_iter().map(|t| (t.name.clone(), t)));
-        }
-
-        macros.add_env();
+impl<T, E: StdError + Send + Sync + 'static, Inner: Iterator<Item = Result<T, E>>>
+    LineNumbers<T, E, Inner>
+{
+    fn new(inner: Inner) -> Self {
+        Self(inner, 0)
+    }
+}
 
-        for r#macro in args.macros() {
-            if let [name, value] = *r#macro.splitn(2, '=').collect::<Vec<_>>() {
-                macros.set(
-                    name.into(),
-                    MacroSource::CommandLineOrMakeflags,
-                    TokenString::text(value),
-                );
-            }
-        }
+impl<T, E: StdError + Send + Sync + 'static, Inner: Iterator<Item = Result<T, E>>> Iterator
+    for LineNumbers<T, E, Inner>
+{
+    type Item = (usize, Result<T>);
+
+    fn next(&mut self) -> Option<Self::Item> {
+        self.0.next().map(|x| {
+            self.1 = self.1.saturating_add(1);
+            (
+                self.1,
+                x.with_context(|| format!("failed to read line {} of makefile", self.1)),
+            )
+        })
+    }
+}
 
-        MakefileReader {
-            inference_rules,
-            macros,
-            targets,
-            first_non_special_target,
-            args,
-        }
+trait IteratorExt<T, E: StdError + Send + Sync + 'static>: Iterator<Item = Result<T, E>> {
+    fn line_numbered(self) -> LineNumbers<T, E, Self>
+    where
+        Self: Sized,
+    {
+        LineNumbers::new(self)
     }
+}
+impl<T, E: StdError + Send + Sync + 'static, I: Iterator<Item = Result<T, E>>> IteratorExt<T, E>
+    for I
+{
+}
 
-    pub fn and_read_file(&mut self, path: impl AsRef<Path>) -> Result<()> {
+pub struct MakefileReader<'a, R: BufRead> {
+    pub inference_rules: Vec<InferenceRule>,
+    pub macros: MacroSet<'static, 'static>,
+    pub targets: HashMap<String, Target>,
+    pub first_non_special_target: Option<String>,
+    args: &'a Args,
+    lines_iter: Peekable<LineNumbers<String, IoError, Lines<R>>>,
+    pending_line: Option<(usize, Result<String>)>,
+    #[cfg(feature = "full")]
+    conditional_stack: Vec<ConditionalState>,
+}
+
+impl<'a> MakefileReader<'a, BufReader<File>> {
+    pub fn read_file(args: &'a Args, path: impl AsRef<Path>) -> Result<Self> {
         let file = File::open(path);
         // TODO handle errors
         let file = file.context("couldn't open makefile!")?;
         let file_reader = BufReader::new(file);
-        self.and_read(file_reader)
+        Self::read(args, file_reader)
     }
+}
 
-    pub fn and_read(&mut self, source: impl BufRead) -> Result<()> {
-        let mut lines_iter = source
-            .lines()
-            .enumerate()
-            .map(|(number, line)| (number.saturating_add(1), line))
-            .map(|(line, x)| {
-                (
-                    line,
-                    x.with_context(|| format!("failed to read line {} of makefile", line)),
-                )
-            })
-            .peekable();
-        #[cfg(feature = "full")]
-        let mut conditional_stack: Vec<ConditionalState> = vec![];
-        while let Some((line_number, line)) = lines_iter.next() {
-            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?.trim_start())
-                }
-            }
-
-            // handle comments
-            lazy_static! {
-                static ref COMMENT: Regex = Regex::new("#.*$").unwrap();
-            }
-            let line = COMMENT.replace(&line, "").into_owned();
-
+impl<'a, R: BufRead> MakefileReader<'a, R> {
+    pub fn read(args: &'a Args, source: R) -> Result<Self> {
+        let mut reader = Self {
+            inference_rules: Vec::new(),
+            macros: MacroSet::new(),
+            targets: HashMap::new(),
+            first_non_special_target: None,
+            args,
+            lines_iter: source.lines().line_numbered().peekable(),
             #[cfg(feature = "full")]
-            if let Some(line) = ConditionalLine::from(&line, |t| self.expand_macros(t))? {
-                line.action(
-                    conditional_stack.last(),
-                    |name| self.macros.is_defined(name),
-                    |t| self.expand_macros(t),
-                )?
-                .apply_to(&mut conditional_stack);
-                continue;
-            }
+            conditional_stack: Vec::new(),
+            pending_line: None,
+        };
+        reader.read_all()?;
+        Ok(reader)
+    }
 
-            // skip lines if we need to
-            #[cfg(feature = "full")]
-            if conditional_stack
-                .last()
-                .map_or(false, ConditionalState::skipping)
-            {
-                continue;
-            }
+    fn read_all(&mut self) -> Result<()> {
+        while let Some((line_number, line)) = self.next_line(" ") {
+            let line = line?;
 
             // handle include lines
             if let Some(line) = line.strip_prefix("include ") {
@@ -199,7 +181,7 @@ impl<'a> MakefileReader<'a> {
                 // 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.extend(MakefileReader::read_file(self.args, field)?);
                 }
                 continue;
             }
@@ -228,8 +210,8 @@ impl<'a> MakefileReader<'a> {
             };
 
             match line_type {
-                LineType::Rule => self.read_rule(&line_tokens, line_number, &mut lines_iter)?,
-                LineType::Macro => self.read_macro(line_tokens, line_number, &mut lines_iter)?,
+                LineType::Rule => self.read_rule(&line_tokens, line_number)?,
+                LineType::Macro => self.read_macro(line_tokens, line_number)?,
                 LineType::Unknown => {
                     if !line_tokens.is_empty() {
                         bail!(
@@ -245,27 +227,97 @@ impl<'a> MakefileReader<'a> {
         Ok(())
     }
 
-    fn read_rule(
+    fn next_line(
+        &mut self,
+        escaped_newline_replacement: &'static str,
+    ) -> Option<(usize, Result<String>)> {
+        if let Some(x) = self.pending_line.take() {
+            return Some(x);
+        }
+        while let Some((line_number, line)) = self.lines_iter.next() {
+            let mut line = match line {
+                Ok(x) => x,
+                Err(err) => return Some((line_number, Err(err))),
+            };
+
+            // handle escaped newlines
+            while line.ends_with('\\') {
+                line.pop();
+                line.push_str(escaped_newline_replacement);
+                if let Some((n, x)) = self.lines_iter.next() {
+                    let x = match x {
+                        Ok(x) => x,
+                        Err(err) => return Some((n, Err(err))),
+                    };
+                    line.push_str(x.trim_start())
+                }
+            }
+
+            // handle comments
+            lazy_static! {
+                static ref COMMENT: Regex = Regex::new("#.*$").unwrap();
+            }
+            let line = COMMENT.replace(&line, "").into_owned();
+
+            #[cfg(feature = "full")]
+            {
+                let cond_line = ConditionalLine::from(&line, |t| self.expand_macros(t));
+                let cond_line = match cond_line {
+                    Ok(x) => x,
+                    Err(err) => return Some((line_number, Err(err))),
+                };
+                if let Some(line) = cond_line {
+                    let action = line.action(
+                        self.conditional_stack.last(),
+                        |name| self.macros.is_defined(name),
+                        |t| self.expand_macros(t),
+                    );
+                    let action = match action {
+                        Ok(x) => x,
+                        Err(err) => return Some((line_number, Err(err))),
+                    };
+                    action.apply_to(&mut self.conditional_stack);
+                    continue;
+                }
+
+                // skip lines if we need to
+                if self
+                    .conditional_stack
+                    .last()
+                    .map_or(false, ConditionalState::skipping)
+                {
+                    continue;
+                }
+            }
+
+            return Some((line_number, Ok(line)));
+        }
+        None
+    }
+
+    fn next_line_if(
         &mut self,
-        line_tokens: &TokenString,
-        line_number: usize,
-        lines_iter: &mut Peekable<impl Iterator<Item = (usize, Result<String>)>>,
-    ) -> Result<()> {
+        escaped_newline_replacement: &'static str,
+        predicate: impl FnOnce(&(usize, Result<String>)) -> bool,
+    ) -> Option<(usize, Result<String>)> {
+        let pending_line = self.next_line(escaped_newline_replacement)?;
+        if (predicate)(&pending_line) {
+            Some(pending_line)
+        } else {
+            self.pending_line = Some(pending_line);
+            None
+        }
+    }
+
+    fn read_rule(&mut self, line_tokens: &TokenString, line_number: usize) -> Result<()> {
         let (targets, not_targets) = line_tokens
             .split_once(':')
             .ok_or_else(|| eyre!("read_rule couldn't find a ':' on line {}", line_number))?;
         let targets = self.expand_macros(&targets)?;
         let targets = targets.split_whitespace().collect::<Vec<_>>();
         let (prerequisites, mut commands) = match not_targets.split_once(';') {
-            Some((prerequisites, mut command)) => {
-                while command.ends_with("\\") {
-                    if let Some((_, next_line)) = lines_iter.next() {
-                        command.strip_suffix("\\");
-                        command.extend(tokenize(&next_line?)?);
-                    } else {
-                        break;
-                    }
-                }
+            Some((prerequisites, command)) => {
+                // TODO make sure escaped newlines get retroactively treated correctly here
                 (prerequisites, vec![command])
             }
             None => (not_targets, vec![]),
@@ -276,7 +328,7 @@ impl<'a> MakefileReader<'a> {
             .map(|x| x.into())
             .collect::<Vec<String>>();
 
-        while let Some((_, x)) = lines_iter.next_if(|(_, x)| {
+        while let Some((_, x)) = self.next_line_if("\\\n", |(_, x)| {
             x.as_ref()
                 .ok()
                 .map_or(false, |line| line.starts_with('\t') || line.is_empty())
@@ -288,16 +340,6 @@ impl<'a> MakefileReader<'a> {
             if line.is_empty() {
                 continue;
             }
-            while line.ends_with('\\') {
-                match lines_iter.next() {
-                    Some((_, Ok(next_line))) => {
-                        let next_line = next_line.strip_prefix("\t").unwrap_or(&next_line);
-                        line.push('\n');
-                        line.push_str(next_line);
-                    }
-                    _ => break,
-                }
-            }
             commands.push(
                 line.parse()
                     .with_context(|| format!("failed to parse line {}", line_number))?,
@@ -361,12 +403,7 @@ impl<'a> MakefileReader<'a> {
         Ok(())
     }
 
-    fn read_macro(
-        &mut self,
-        mut line_tokens: TokenString,
-        line_number: usize,
-        lines_iter: &mut Peekable<impl Iterator<Item = (usize, Result<String>)>>,
-    ) -> Result<()> {
+    fn read_macro(&mut self, mut line_tokens: TokenString, line_number: usize) -> Result<()> {
         let (name, mut value) = if cfg!(feature = "full") && line_tokens.starts_with("define ") {
             line_tokens.strip_prefix("define ");
             if line_tokens.ends_with("=") {
@@ -374,7 +411,8 @@ impl<'a> MakefileReader<'a> {
                 line_tokens.trim_end();
             }
             let mut value = TokenString::empty();
-            for (_, line) in lines_iter {
+            // TODO what should be done with escaped newlines
+            while let Some((_, line)) = self.next_line(" ") {
                 let line = line?;
                 if line == "endef" {
                     break;
@@ -448,102 +486,17 @@ impl<'a> MakefileReader<'a> {
     fn expand_macros(&self, text: &TokenString) -> Result<String> {
         self.macros.expand(text)
     }
-}
 
-impl<'a> From<MakefileReader<'a>> for super::Makefile<'a> {
-    fn from(reader: MakefileReader<'a>) -> Self {
-        Self {
-            inference_rules: reader.inference_rules,
-            macros: reader.macros,
-            targets: RefCell::new(
-                reader
-                    .targets
-                    .into_iter()
-                    .map(|(k, v)| (k, Rc::new(RefCell::new(v))))
-                    .collect(),
-            ),
-            first_non_special_target: reader.first_non_special_target,
-            args: reader.args,
+    fn extend<R2: BufRead>(&mut self, new: MakefileReader<R2>) {
+        self.inference_rules.extend(new.inference_rules);
+        self.macros.extend(new.macros);
+        self.targets.extend(new.targets);
+        if self.first_non_special_target.is_none() {
+            self.first_non_special_target = new.first_non_special_target;
         }
     }
 }
 
-fn builtin_inference_rules() -> Vec<InferenceRule> {
-    // This is a terrible idea.
-    macro_rules! prepend_dot {
-        ($x:tt) => {
-            concat!(".", stringify!($x))
-        };
-        () => {
-            ""
-        };
-    }
-
-    macro_rules! make {
-        {$(.$first:tt$(.$second:tt)?:
-            $($cmd:literal)+)+} => {
-            vec![$(
-                InferenceRule {
-                    product: prepend_dot!($($second)?).into(),
-                    prereq: concat!(".", stringify!($first)).into(),
-                    commands: vec![$(CommandLine::from($cmd.parse().unwrap())),+],
-                }
-            ),+]
-        };
-    }
-
-    make! {
-        .c:
-            "$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $<"
-        .f:
-            "$(FC) $(FFLAGS) $(LDFLAGS) -o $@ $<"
-        .sh:
-            "cp $< $@"
-            "chmod a+x $@"
-
-        .c.o:
-            "$(CC) $(CFLAGS) -c $<"
-        .f.o:
-            "$(FC) $(FFLAGS) -c $<"
-        .y.o:
-            "$(YACC) $(YFLAGS) $<"
-            "$(CC) $(CFLAGS) -c y.tab.c"
-            "rm -f y.tab.c"
-            "mv y.tab.o $@"
-        .l.o:
-            "$(LEX) $(LFLAGS) $<"
-            "$(CC) $(CFLAGS) -c lex.yy.c"
-            "rm -f lex.yy.c"
-            "mv lex.yy.o $@"
-        .y.c:
-            "$(YACC) $(YFLAGS) $<"
-            "mv y.tab.c $@"
-        .l.c:
-            "$(LEX) $(LFLAGS) $<"
-            "mv lex.yy.c $@"
-        .c.a:
-            "$(CC) -c $(CFLAGS) $<"
-            "$(AR) $(ARFLAGS) $@ $*.o"
-            "rm -f $*.o"
-        .f.a:
-            "$(FC) -c $(FFLAGS) $<"
-            "$(AR) $(ARFLAGS) $@ $*.o"
-            "rm -f $*.o"
-    }
-}
-fn builtin_targets() -> Vec<Target> {
-    // even i'm not going to do that just for this
-    vec![Target {
-        name: ".SUFFIXES".into(),
-        prerequisites: vec![".o", ".c", ".y", ".l", ".a", ".sh", ".f"]
-            .into_iter()
-            .map(String::from)
-            .collect(),
-        commands: vec![],
-        already_updated: Cell::new(false),
-    }]
-}
-
 #[cfg(test)]
 mod test {
     use std::io::Cursor;
@@ -552,16 +505,6 @@ mod test {
 
     type R = Result<()>;
 
-    fn empty_makefile(args: &Args) -> MakefileReader {
-        MakefileReader {
-            inference_rules: vec![],
-            macros: MacroSet::new(),
-            targets: HashMap::new(),
-            first_non_special_target: None,
-            args,
-        }
-    }
-
     #[cfg(feature = "full")]
     #[test]
     fn basic_conditionals() -> R {
@@ -575,8 +518,7 @@ worked = perhaps
 endif
         ";
         let args = Args::empty();
-        let mut makefile = empty_makefile(&args);
-        makefile.and_read(Cursor::new(file))?;
+        let makefile = MakefileReader::read(&args, Cursor::new(file))?;
         assert_eq!(
             makefile.expand_macros(&TokenString::r#macro("worked"))?,
             "yes"
@@ -594,8 +536,7 @@ baz
 endef
         ";
         let args = Args::empty();
-        let mut makefile = empty_makefile(&args);
-        makefile.and_read(Cursor::new(file))?;
+        let makefile = MakefileReader::read(&args, Cursor::new(file))?;
         assert_eq!(
             makefile.expand_macros(&TokenString::r#macro("foo"))?,
             "bar\nbaz"
@@ -640,8 +581,7 @@ clean:
         ";
 
         let args = Args::empty();
-        let mut makefile = empty_makefile(&args);
-        makefile.and_read(Cursor::new(file))?;
+        let makefile = MakefileReader::read(&args, Cursor::new(file))?;
         assert!(makefile.targets.contains_key("server"));
         Ok(())
     }