More changes

src/error.rs

@@ -31,6 +31,10 @@ pub(crate) enum Error<'src> {
     chooser: OsString,
     io_error: io::Error,
   },
+  CircularInclude {
+    cur_path: PathBuf,
+    recursively_included_path: PathBuf,
+  },
   Code {
     recipe: &'src str,
     line_number: Option<usize>,
@@ -87,16 +91,9 @@ pub(crate) enum Error<'src> {
   InitExists {
     justfile: PathBuf,
   },
-  IncludeRecursive {
-    cur_path: PathBuf,
-    recursively_included_path: PathBuf,
-  },
   Internal {
     message: String,
   },
-  InvalidInclude {
-    line_number: usize,
-  },
   Io {
     recipe: &'src str,
     io_error: io::Error,
@@ -128,6 +125,9 @@ pub(crate) enum Error<'src> {
     recipe: &'src str,
     io_error: io::Error,
   },
+  TrailingInclude {
+    line_number: usize,
+  },
   Unknown {
     recipe: &'src str,
     line_number: Option<usize>,
@@ -337,6 +337,12 @@ impl<'src> ColorDisplay for Error<'src> {
           io_error
         )?;
       }
+      CircularInclude { cur_path, recursively_included_path } => {
+        write!(
+          f,
+          "Justfile at {} tries to recursively include {}", cur_path.display(), recursively_included_path.display()
+        )?;
+      },
       Code {
         recipe,
         line_number,
@@ -492,12 +498,6 @@ impl<'src> ColorDisplay for Error<'src> {
       InitExists { justfile } => {
         write!(f, "Justfile `{}` already exists", justfile.display())?;
       }
-      IncludeRecursive { cur_path, recursively_included_path } => {
-        write!(
-          f,
-          "Justfile at {} tries to recursively include {}", cur_path.display(), recursively_included_path.display()
-        )?;
-      },
       Internal { message } => {
         write!(
           f,
@@ -506,13 +506,6 @@ impl<'src> ColorDisplay for Error<'src> {
           message
         )?;
       }
-      InvalidInclude { line_number } => {
-        write!(
-          f,
-          "!include statement at line {} occurs after the first non-blank, non-comment line",
-          line_number
-        )?;
-      }
       Io { recipe, io_error } => {
         match io_error.kind() {
           io::ErrorKind::NotFound => write!(
@@ -587,6 +580,13 @@ impl<'src> ColorDisplay for Error<'src> {
         "Recipe `{recipe}` could not be run because of an IO error while trying to create a temporary \
          directory or write a file to that directory`:{io_error}",
       )?,
+      TrailingInclude { line_number } => {
+        write!(
+          f,
+          "!include statement at line {} occurs after the first non-blank, non-comment line",
+          line_number
+        )?;
+      }
       Unknown {
         recipe,
         line_number,

src/loader.rs

@@ -35,34 +35,24 @@ impl Loader {
 
   fn load_with_includes_recursive(
     &self,
-    path: &Path,
-    seen_paths: HashSet<PathBuf>,
-  ) -> RunResult<String> {
-    let src = Self::load_file(path)?;
-    self.process_includes(src, path, seen_paths)
-  }
-
-  /// Given the original contents of a Justfile (with include directives), load all the included
-  /// paths to produce one String with the contents of all the files concatenated.
-  fn process_includes(
-    &self,
-    original: String,
     current_justfile_path: &Path,
     seen_paths: HashSet<PathBuf>,
   ) -> RunResult<String> {
-    let has_final_newline = original.ends_with('\n');
+    let original_src = Self::load_file(current_justfile_path)?;
+
+    let has_final_newline = original_src.ends_with('\n');
 
     let include_regexp = Regex::new(INCLUDE_REGEX).unwrap();
 
     let mut buf = String::new();
-    let mut lines = original.lines().enumerate().peekable();
+    let mut lines = original_src.lines().enumerate().peekable();
 
     let mut seen_first_contentful_line = false;
 
     while let Some((line_num, line)) = lines.next() {
       match include_regexp.captures(line) {
         Some(_) if seen_first_contentful_line => {
-          return Err(Error::InvalidInclude {
+          return Err(Error::TrailingInclude {
             line_number: line_num + 1,
           });
         }
@@ -112,7 +102,7 @@ impl Loader {
     let canonical_path = canonical_path.lexiclean();
 
     if seen_paths.contains(&canonical_path) {
-      return Err(Error::IncludeRecursive {
+      return Err(Error::CircularInclude {
         cur_path: cur_path.to_owned(),
         recursively_included_path: canonical_path,
       });
@@ -208,14 +198,14 @@ recipe_b:
     let justfile_a_path = tmp.path().join("justfile");
     let loader_output = loader.load_with_includes(&justfile_a_path).unwrap_err();
 
-    assert_matches!(loader_output, Error::IncludeRecursive { cur_path, recursively_included_path }
+    assert_matches!(loader_output, Error::CircularInclude { cur_path, recursively_included_path }
         if cur_path == tmp.path().join("subdir").join("justfile_b").lexiclean() &&
         recursively_included_path == tmp.path().join("justfile").lexiclean()
     );
   }
 
   #[test]
-  fn invalid_includes() {
+  fn trailing_includes() {
     let justfile_a = r#"
 # A comment at the top of the file
 !include ./subdir/justfile_b
@@ -244,6 +234,6 @@ recipe_b:
     let justfile_a_path = tmp.path().join("justfile");
     let loader_output = loader.load_with_includes(&justfile_a_path).unwrap_err();
 
-    assert_matches!(loader_output, Error::InvalidInclude { line_number } if line_number == 4);
+    assert_matches!(loader_output, Error::TrailingInclude { line_number } if line_number == 4);
   }
 }