overhaul lookup_internal

4 files changed, 184 insertions, 114 deletions

diff --git a/src/makefile/input.rs b/src/makefile/input.rs
index 25d33e4..bbfdaee 100644
--- a/src/makefile/input.rs
+++ b/src/makefile/input.rs
@@ -24,6 +24,7 @@ use super::r#macro::ExportConfig;
 use super::r#macro::{Macro, Set as MacroSet};
 use super::target::{StaticTargetSet, Target};
 use super::token::{Token, TokenString};
+use super::LookupInternal;
 use super::{builtin_targets, ItemSource};
 
 enum LineType {
@@ -591,39 +592,7 @@ impl<'a, 'parent, R: BufRead> MakefileReader<'a, 'parent, R> {
         let mut deferred_eval_context = DeferredEvalContext::new(self);
         let prerequisites = self
             .macros
-            .with_lookup(&|macro_name: &str| {
-                let macro_pieces = if macro_name.starts_with('@') {
-                    // The $@ shall evaluate to the full target name of the
-                    // current target.
-                    targets.iter()
-                } else {
-                    bail!("unknown internal macro")
-                };
-
-                let macro_pieces = if macro_name.ends_with('D') {
-                    macro_pieces
-                        .map(|x| {
-                            Path::new(x)
-                                .parent()
-                                .ok_or_else(|| eyre!("no parent"))
-                                .map(|x| x.to_string_lossy().into())
-                        })
-                        .collect::<Result<Vec<String>, _>>()?
-                } else if macro_name.ends_with('F') {
-                    macro_pieces
-                        .map(|x| {
-                            Path::new(x)
-                                .file_name()
-                                .ok_or_else(|| eyre!("no filename"))
-                                .map(|x| x.to_string_lossy().into())
-                        })
-                        .collect::<Result<Vec<String>, _>>()?
-                } else {
-                    macro_pieces.map(|&x| x.to_owned()).collect::<Vec<String>>()
-                };
-
-                Ok(macro_pieces.join(" "))
-            })
+            .with_lookup(LookupInternal::new_partial(&targets))
             .expand(
                 &prerequisites,
                 #[cfg(feature = "full")]
diff --git a/src/makefile/lookup_internal.rs b/src/makefile/lookup_internal.rs
new file mode 100644
index 0000000..c0fc08b
--- /dev/null
+++ b/src/makefile/lookup_internal.rs
@@ -0,0 +1,164 @@
+use eyre::{bail, eyre, Result};
+use std::cell::RefCell;
+use std::path::Path;
+use std::rc::Rc;
+
+use super::target::Target;
+
+#[derive(Clone)]
+pub enum LookupInternal<'a> {
+    Partial {
+        targets: &'a Vec<&'a str>,
+    },
+    Complete {
+        target: Option<&'a Target>,
+        get_target: &'a dyn Fn(&str) -> Result<Rc<RefCell<Target>>>,
+    },
+}
+
+impl<'a> LookupInternal<'a> {
+    pub fn new_partial(targets: &'a Vec<&str>) -> Self {
+        Self::Partial { targets }
+    }
+
+    pub fn new(
+        target: Option<&'a Target>,
+        get_target: &'a dyn Fn(&str) -> Result<Rc<RefCell<Target>>>,
+    ) -> Self {
+        Self::Complete { target, get_target }
+    }
+
+    pub fn lookup(&self, macro_name: &str) -> Result<String> {
+        let macro_pieces = match macro_name.chars().next() {
+            Some('@') => self.target_name()?,
+            Some('?') => self.newer_prerequisites()?,
+            Some('<') => self.inference_prerequisite()?,
+            Some('*') => self.target_stem()?,
+            #[cfg(feature = "full")]
+            Some('^') => self.all_prerequisites()?,
+            _ => bail!("unknown internal macro {}", macro_name),
+        };
+
+        let macro_pieces = if macro_name.ends_with('D') {
+            macro_pieces
+                .into_iter()
+                .map(|x| {
+                    Path::new(&x)
+                        .parent()
+                        .ok_or_else(|| eyre!("no parent"))
+                        .map(|x| x.to_string_lossy().into())
+                })
+                .collect::<Result<_, _>>()?
+        } else if macro_name.ends_with('F') {
+            macro_pieces
+                .into_iter()
+                .map(|x| {
+                    Path::new(&x)
+                        .file_name()
+                        .ok_or_else(|| eyre!("no filename"))
+                        .map(|x| x.to_string_lossy().into())
+                })
+                .collect::<Result<_, _>>()?
+        } else {
+            macro_pieces
+        };
+
+        Ok(macro_pieces.join(" "))
+    }
+
+    /// POSIX: The $@ shall evaluate to the full target name of the current target.
+    fn target_name(&self) -> Result<Vec<String>> {
+        match self {
+            Self::Partial { targets } => {
+                Ok(targets.iter().map(|target| target.to_string()).collect())
+            }
+            Self::Complete {
+                target: Some(target),
+                ..
+            } => Ok(vec![target.name.clone()]),
+            Self::Complete { target: None, .. } => {
+                bail!("tried to expand internal macro with no target")
+            }
+        }
+    }
+
+    /// POSIX: The $? macro shall evaluate to the list of prerequisites that are newer than the current target.
+    fn newer_prerequisites(&self) -> Result<Vec<String>> {
+        match self {
+            Self::Partial { .. } => bail!("can’t expand $? when target not defined"),
+            Self::Complete {
+                target: Some(target),
+                get_target,
+            } => Ok(target
+                .prerequisites
+                .iter()
+                .filter(|prereq| {
+                    get_target(prereq)
+                        .ok()
+                        .and_then(|prereq| prereq.borrow().newer_than(target))
+                        .unwrap_or(false)
+                })
+                .cloned()
+                .collect()),
+            Self::Complete { target: None, .. } => {
+                bail!("tried to expand internal macro with no target")
+            }
+        }
+    }
+
+    /// POSIX: In an inference rule, the $< macro shall evaluate to the filename whose existence allowed the inference rule to be chosen for the target. In the .DEFAULT rule, the $< macro shall evaluate to the current target name.
+    ///
+    /// GNU: The name of the first prerequisite.
+    fn inference_prerequisite(&self) -> Result<Vec<String>> {
+        match self {
+            Self::Partial { .. } => bail!("can’t expand $< when target not defined"),
+            Self::Complete {
+                target: Some(target),
+                ..
+            } => {
+                // TODO check that exists_but_inferring_anyway won’t break this
+                Ok(vec![target
+                    .prerequisites
+                    .first()
+                    .cloned()
+                    .unwrap_or_default()])
+            }
+            Self::Complete { target: None, .. } => {
+                bail!("tried to expand internal macro with no target")
+            }
+        }
+    }
+
+    /// POSIX: The $* macro shall evaluate to the current target name with its suffix deleted.
+    fn target_stem(&self) -> Result<Vec<String>> {
+        match self {
+            Self::Partial { .. } => bail!("can’t expand $* when target not defined"),
+            Self::Complete {
+                target: Some(target),
+                ..
+            } => Ok(vec![target
+                .stem
+                .as_ref()
+                .unwrap_or(&target.name)
+                .to_owned()]),
+            Self::Complete { target: None, .. } => {
+                bail!("tried to expand internal macro with no target")
+            }
+        }
+    }
+
+    /// GNU: The names of all the prerequisites.
+    #[cfg(feature = "full")]
+    fn all_prerequisites(&self) -> Result<Vec<String>> {
+        match self {
+            Self::Partial { .. } => bail!("can’t expand $^ when target not defined"),
+            Self::Complete {
+                target: Some(target),
+                ..
+            } => Ok(target.prerequisites.clone()),
+            Self::Complete { target: None, .. } => {
+                bail!("tried to expand internal macro with no target")
+            }
+        }
+    }
+}
diff --git a/src/makefile/macro.rs b/src/makefile/macro.rs
index b135f42..4a82fe9 100644
--- a/src/makefile/macro.rs
+++ b/src/makefile/macro.rs
@@ -11,6 +11,7 @@ use std::rc::Rc;
 use super::functions;
 use super::token::{Token, TokenString};
 use super::ItemSource;
+use super::LookupInternal;
 #[cfg(feature = "full")]
 use crate::makefile::eval_context::DeferredEvalContext;
 use eyre::{bail, Result, WrapErr};
@@ -25,10 +26,6 @@ pub struct Macro {
     pub eagerly_expanded: bool,
 }
 
-pub trait LookupInternal: for<'a> Fn(&'a str) -> Result<String> {}
-
-impl<F: for<'a> Fn(&'a str) -> Result<String>> LookupInternal for F {}
-
 #[cfg(feature = "full")]
 #[derive(Clone, Debug)]
 pub enum ExportConfig {
@@ -91,7 +88,7 @@ impl ExportConfig {
 pub struct Set<'parent, 'lookup> {
     parent: Option<&'parent Set<'parent, 'lookup>>,
     pub data: HashMap<String, Macro>,
-    lookup_internal: Option<&'lookup dyn LookupInternal>,
+    lookup_internal: Option<LookupInternal<'lookup>>,
     #[cfg(feature = "full")]
     pub exported: ExportConfig,
     warnings: Rc<RefCell<HashSet<String>>>,
@@ -140,8 +137,8 @@ impl<'parent, 'lookup> Set<'parent, 'lookup> {
     }
 
     fn lookup_internal(&self, name: &str) -> Result<String> {
-        if let Some(lookup) = self.lookup_internal {
-            lookup(name)
+        if let Some(lookup) = self.lookup_internal.as_ref() {
+            lookup.lookup(name)
         } else if let Some(parent) = self.parent {
             parent.lookup_internal(name)
         } else {
@@ -341,7 +338,7 @@ impl<'parent, 'lookup> Set<'parent, 'lookup> {
         }
     }
 
-    pub fn with_lookup<'l, 's: 'l>(&'s self, lookup: &'l dyn LookupInternal) -> Set<'s, 'l> {
+    pub fn with_lookup<'l, 's: 'l>(&'s self, lookup: LookupInternal<'l>) -> Set<'s, 'l> {
         Set {
             parent: Some(self),
             data: HashMap::new(),
@@ -392,7 +389,10 @@ impl fmt::Debug for Set<'_, '_> {
         let mut r#struct = f.debug_struct("Set");
         r#struct.field("parent", &self.parent);
         r#struct.field("data", &self.data);
-        r#struct.field("lookup_internal", &self.lookup_internal.map(|_| ()));
+        r#struct.field(
+            "lookup_internal",
+            &self.lookup_internal.as_ref().map(|_| ()),
+        );
         #[cfg(feature = "full")]
         r#struct.field("exported", &self.exported);
         r#struct.field("warnings", &self.warnings);
diff --git a/src/makefile/mod.rs b/src/makefile/mod.rs
index 8fd7113..9255ebf 100644
--- a/src/makefile/mod.rs
+++ b/src/makefile/mod.rs
@@ -11,6 +11,7 @@ use command_line::CommandLine;
 use inference_rules::InferenceRule;
 use input::FinishedMakefileReader;
 pub use input::MakefileReader;
+use lookup_internal::LookupInternal;
 use r#macro::{Macro, Set as MacroSet};
 use target::{DynamicTargetSet, Target};
 use token::TokenString;
@@ -28,6 +29,7 @@ mod eval_context;
 mod functions;
 mod inference_rules;
 mod input;
+mod lookup_internal;
 mod r#macro;
 mod pattern;
 mod target;
@@ -347,78 +349,13 @@ impl<'a> Makefile<'a> {
     }
 
     fn expand_macros(&self, text: &TokenString, target: Option<&Target>) -> Result<String> {
-        let target = target.cloned();
-        let lookup_internal = move |macro_name: &str| {
-            let target = target
-                .as_ref()
-                .ok_or_else(|| eyre!("internal macro but no current target!"))?;
-            let macro_pieces = if macro_name.starts_with('@') {
-                // The $@ shall evaluate to the full target name of the
-                // current target.
-                vec![target.name.clone()]
-            } else if macro_name.starts_with('?') {
-                // The $? macro shall evaluate to the list of prerequisites
-                // that are newer than the current target.
-                target
-                    .prerequisites
-                    .iter()
-                    .filter(|prereq| {
-                        self.get_target(prereq)
-                            .ok()
-                            .and_then(|prereq| prereq.borrow().newer_than(target))
-                            .unwrap_or(false)
-                    })
-                    .cloned()
-                    .collect()
-            } else if macro_name.starts_with('<') {
-                // In an inference rule, the $< macro shall evaluate to the
-                // filename whose existence allowed the inference rule to be
-                // chosen for the target. In the .DEFAULT rule, the $< macro
-                // shall evaluate to the current target name.
-                // TODO make that actually be the case (rn exists_but_inferring_anyway might fuck that up)
-                vec![target.prerequisites.first().cloned().unwrap_or_default()]
-            } else if macro_name.starts_with('*') {
-                // The $* macro shall evaluate to the current target name with
-                // its suffix deleted. (GNUism: the match stem)
-                vec![target.stem.as_ref().unwrap_or(&target.name).to_owned()]
-            } else if macro_name.starts_with('^') {
-                target.prerequisites.clone()
-            } else {
-                unreachable!()
-            };
-
-            let macro_pieces = if macro_name.ends_with('D') {
-                macro_pieces
-                    .into_iter()
-                    .map(|x| {
-                        Path::new(&x)
-                            .parent()
-                            .ok_or_else(|| eyre!("no parent"))
-                            .map(|x| x.to_string_lossy().into())
-                    })
-                    .collect::<Result<_, _>>()?
-            } else if macro_name.ends_with('F') {
-                macro_pieces
-                    .into_iter()
-                    .map(|x| {
-                        Path::new(&x)
-                            .file_name()
-                            .ok_or_else(|| eyre!("no filename"))
-                            .map(|x| x.to_string_lossy().into())
-                    })
-                    .collect::<Result<_, _>>()?
-            } else {
-                macro_pieces
-            };
-
-            Ok(macro_pieces.join(" "))
-        };
-
-        self.macros.with_lookup(&lookup_internal).expand(
-            text,
-            #[cfg(feature = "full")]
-            NO_EVAL,
-        )
+        self.macros
+            .with_lookup(LookupInternal::new(target, &|name| self.get_target(name)))
+            .expand(
+                text,
+                #[cfg(feature = "full")]
+                NO_EVAL,
+            )
     }
 }