From 912863bc1e085ddd9290165e5798164d465950a9 Mon Sep 17 00:00:00 2001 From: Greg Shuflin Date: Thu, 12 Jan 2023 19:25:28 -0800 Subject: [PATCH] Add `!include` directives (#1470) --- .github/workflows/ci.yaml | 3 - README.md | 41 ++++++++ src/error.rs | 32 ++++++ src/loader.rs | 205 +++++++++++++++++++++++++++++++++++++- src/run.rs | 10 +- tests/choose.rs | 2 +- tests/error_messages.rs | 6 +- tests/includes.rs | 91 +++++++++++++++++ tests/lib.rs | 9 ++ tests/test.rs | 13 ++- 10 files changed, 392 insertions(+), 20 deletions(-) create mode 100644 tests/includes.rs diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index e0d8adea..4a746192 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -30,9 +30,6 @@ jobs: - uses: Swatinem/rust-cache@v1 - - name: Check Lockfile - run: cargo update --locked --package just - - name: Clippy run: cargo clippy --all --all-targets diff --git a/README.md b/README.md index e99aff00..18bae44f 100644 --- a/README.md +++ b/README.md @@ -2175,6 +2175,47 @@ But they must match: $ just foo/a bar/b error: Conflicting path arguments: `foo/` and `bar/` ``` +### Include Directives + +The `!include` directive, currently unstable, can be used to include the +verbatim text of another file. + +If you have the following `justfile`: + +```mf +!include foo/bar.just + +a: b + @echo A + +``` + +And the following text in `foo/bar.just`: + +```mf +b: + @echo B +``` + +`foo/bar.just` will be included in `justfile` and recipe `b` will be defined: + +```sh +$ just --unstable b +B +$ just --unstable a +B +A +``` + +The `!include` directive path can be absolute or relative to the location of +the justfile containing it. `!include` directives must appear at the beginning +of a line. line. + +`!include` directives are only processed before the first non-blank, +non-comment line. + +Included files can themselves contain `!include` directives, which are +processed recursively. ### Hiding `justfile`s diff --git a/src/error.rs b/src/error.rs index 15e42b94..54bebc8c 100644 --- a/src/error.rs +++ b/src/error.rs @@ -31,6 +31,10 @@ pub(crate) enum Error<'src> { chooser: OsString, io_error: io::Error, }, + CircularInclude { + current: PathBuf, + include: PathBuf, + }, Code { recipe: &'src str, line_number: Option, @@ -84,12 +88,19 @@ pub(crate) enum Error<'src> { function: Name<'src>, message: String, }, + IncludeMissingPath { + file: PathBuf, + line: usize, + }, InitExists { justfile: PathBuf, }, Internal { message: String, }, + InvalidDirective { + line: String, + }, Io { recipe: &'src str, io_error: io::Error, @@ -330,6 +341,12 @@ impl<'src> ColorDisplay for Error<'src> { io_error )?; } + CircularInclude { current, include } => { + write!( + f, + "Include `{}` in `{}` is a circular include", include.display(), current.display() + )?; + }, Code { recipe, line_number, @@ -482,6 +499,18 @@ impl<'src> ColorDisplay for Error<'src> { message )?; } + IncludeMissingPath { + file: justfile, line + } => { + + write!( + f, + "!include directive on line {} of `{}` has no argument", + line.ordinal(), + justfile.display(), + )?; + + }, InitExists { justfile } => { write!(f, "Justfile `{}` already exists", justfile.display())?; } @@ -493,6 +522,9 @@ impl<'src> ColorDisplay for Error<'src> { message )?; } + InvalidDirective { line } => { + write!(f, "Invalid directive: {line}")?; + } Io { recipe, io_error } => { match io_error.kind() { io::ErrorKind::NotFound => write!( diff --git a/src/loader.rs b/src/loader.rs index 95fa7c34..7ea5612e 100644 --- a/src/loader.rs +++ b/src/loader.rs @@ -1,21 +1,216 @@ use super::*; +use std::collections::HashSet; + +struct LinesWithEndings<'a> { + input: &'a str, +} + +impl<'a> LinesWithEndings<'a> { + fn new(input: &'a str) -> Self { + Self { input } + } +} + +impl<'a> Iterator for LinesWithEndings<'a> { + type Item = &'a str; + + fn next(&mut self) -> Option<&'a str> { + if self.input.is_empty() { + return None; + } + let split = self.input.find('\n').map_or(self.input.len(), |i| i + 1); + let (line, rest) = self.input.split_at(split); + self.input = rest; + Some(line) + } +} pub(crate) struct Loader { arena: Arena, + unstable: bool, } impl Loader { - pub(crate) fn new() -> Self { + pub(crate) fn new(unstable: bool) -> Self { Loader { arena: Arena::new(), + unstable, } } pub(crate) fn load<'src>(&'src self, path: &Path) -> RunResult<&'src str> { - let src = fs::read_to_string(path).map_err(|io_error| Error::Load { - path: path.to_owned(), - io_error, - })?; + let src = self.load_recursive(path, HashSet::new())?; Ok(self.arena.alloc(src)) } + + fn load_file<'a>(path: &Path) -> RunResult<'a, String> { + fs::read_to_string(path).map_err(|io_error| Error::Load { + path: path.to_owned(), + io_error, + }) + } + + fn load_recursive(&self, file: &Path, seen: HashSet) -> RunResult { + let src = Self::load_file(file)?; + + let mut output = String::new(); + + let mut seen_content = false; + + for (i, line) in LinesWithEndings::new(&src).enumerate() { + if !seen_content && line.starts_with('!') { + let include = line + .strip_prefix("!include") + .ok_or_else(|| Error::InvalidDirective { line: line.into() })?; + + if !self.unstable { + return Err(Error::Unstable { + message: "The !include directive is currently unstable.".into(), + }); + } + + let argument = include.trim(); + + if argument.is_empty() { + return Err(Error::IncludeMissingPath { + file: file.to_owned(), + line: i, + }); + } + + let contents = self.process_include(file, Path::new(argument), &seen)?; + + output.push_str(&contents); + } else { + if !(line.trim().is_empty() || line.trim().starts_with('#')) { + seen_content = true; + } + output.push_str(line); + } + } + + Ok(output) + } + + fn process_include( + &self, + file: &Path, + include: &Path, + seen: &HashSet, + ) -> RunResult { + let canonical_path = if include.is_relative() { + let current_dir = file.parent().ok_or(Error::Internal { + message: format!( + "Justfile path `{}` has no parent directory", + include.display() + ), + })?; + current_dir.join(include) + } else { + include.to_owned() + }; + + let canonical_path = canonical_path.lexiclean(); + + if seen.contains(&canonical_path) { + return Err(Error::CircularInclude { + current: file.to_owned(), + include: canonical_path, + }); + } + + let mut seen_paths = seen.clone(); + seen_paths.insert(file.lexiclean()); + + self.load_recursive(&canonical_path, seen_paths) + } +} + +#[cfg(test)] +mod tests { + use super::{Error, Lexiclean, Loader}; + use temptree::temptree; + + #[test] + fn include_justfile() { + let justfile_a = r#" +# A comment at the top of the file +!include ./justfile_b + +some_recipe: recipe_b + echo "some recipe" +"#; + + let justfile_b = r#"!include ./subdir/justfile_c + +recipe_b: recipe_c + echo "recipe b" +"#; + + let justfile_c = r#"recipe_c: + echo "recipe c" +"#; + + let tmp = temptree! { + justfile: justfile_a, + justfile_b: justfile_b, + subdir: { + justfile_c: justfile_c + } + }; + + let full_concatenated_output = r#" +# A comment at the top of the file +recipe_c: + echo "recipe c" + +recipe_b: recipe_c + echo "recipe b" + +some_recipe: recipe_b + echo "some recipe" +"#; + + let loader = Loader::new(true); + + let justfile_a_path = tmp.path().join("justfile"); + let loader_output = loader.load(&justfile_a_path).unwrap(); + + assert_eq!(loader_output, full_concatenated_output); + } + + #[test] + fn recursive_includes_fail() { + let justfile_a = r#" +# A comment at the top of the file +!include ./subdir/justfile_b + +some_recipe: recipe_b + echo "some recipe" + +"#; + + let justfile_b = r#" +!include ../justfile + +recipe_b: + echo "recipe b" +"#; + let tmp = temptree! { + justfile: justfile_a, + subdir: { + justfile_b: justfile_b + } + }; + + let loader = Loader::new(true); + + let justfile_a_path = tmp.path().join("justfile"); + let loader_output = loader.load(&justfile_a_path).unwrap_err(); + + assert_matches!(loader_output, Error::CircularInclude { current, include } + if current == tmp.path().join("subdir").join("justfile_b").lexiclean() && + include == tmp.path().join("justfile").lexiclean() + ); + } } diff --git a/src/run.rs b/src/run.rs index 950fa0bd..a1a18c24 100644 --- a/src/run.rs +++ b/src/run.rs @@ -17,14 +17,14 @@ pub fn run() -> Result<(), i32> { info!("Parsing command line arguments…"); let matches = app.get_matches(); - let loader = Loader::new(); - let config = Config::from_matches(&matches).map_err(Error::from); - let (color, verbosity) = config + let (color, verbosity, unstable) = config .as_ref() - .map(|config| (config.color, config.verbosity)) - .unwrap_or((Color::auto(), Verbosity::default())); + .map(|config| (config.color, config.verbosity, config.unstable)) + .unwrap_or((Color::auto(), Verbosity::default(), false)); + + let loader = Loader::new(unstable); config .and_then(|config| config.run(&loader)) diff --git a/tests/choose.rs b/tests/choose.rs index d89c31bc..b7e9f442 100644 --- a/tests/choose.rs +++ b/tests/choose.rs @@ -125,7 +125,7 @@ fn invoke_error_function() { echo bar ", ) - .stderr_regex("error: Chooser `/ -cu fzf` invocation failed: .*") + .stderr_regex("error: Chooser `/ -cu fzf` invocation failed: .*\n") .status(EXIT_FAILURE) .shell(false) .args(["--shell", "/", "--choose"]) diff --git a/tests/error_messages.rs b/tests/error_messages.rs index 8e5fb36a..e17a4afd 100644 --- a/tests/error_messages.rs +++ b/tests/error_messages.rs @@ -26,11 +26,11 @@ test! { test! { name: unexpected_character, - justfile: "!~", + justfile: "&~", stderr: " - error: Expected character `=` + error: Expected character `&` | - 1 | !~ + 1 | &~ | ^ ", status: EXIT_FAILURE, diff --git a/tests/includes.rs b/tests/includes.rs new file mode 100644 index 00000000..8850b9b6 --- /dev/null +++ b/tests/includes.rs @@ -0,0 +1,91 @@ +use super::*; + +#[test] +fn include_fails_without_unstable() { + Test::new() + .justfile("!include ./include.justfile") + .status(EXIT_FAILURE) + .stderr("error: The !include directive is currently unstable. Invoke `just` with the `--unstable` flag to enable unstable features.\n") + .run(); +} + +#[test] +fn include_succeeds_with_unstable() { + Test::new() + .tree(tree! { + "include.justfile": " + b: + @echo B + ", + }) + .justfile( + " + !include ./include.justfile + + a: b + @echo A + ", + ) + .arg("--unstable") + .test_round_trip(false) + .arg("a") + .stdout("B\nA\n") + .run(); +} + +#[test] +fn trailing_spaces_after_include_are_ignored() { + Test::new() + .tree(tree! { + "include.justfile": " + a: + @echo A + ", + }) + .justfile("!include ./include.justfile\x20") + .arg("--unstable") + .test_round_trip(false) + .stdout("A\n") + .run(); +} + +#[test] +fn include_directive_with_no_path() { + Test::new() + .justfile("!include") + .arg("--unstable") + .status(EXIT_FAILURE) + .stderr_regex("error: !include directive on line 1 of `.*` has no argument\n") + .run(); +} + +#[test] +fn trailing_include() { + Test::new() + .justfile( + " + b: + !include ./include.justfile + ", + ) + .arg("--unstable") + .status(EXIT_FAILURE) + .stderr("error: Expected character `=`\n |\n2 | !include ./include.justfile\n | ^\n") + .run(); +} + +#[test] +fn circular_include() { + Test::new() + .justfile("!include a") + .tree(tree! { + a: "!include b", + b: "!include a", + }) + .arg("--unstable") + .status(EXIT_FAILURE) + .stderr_regex(path_for_regex( + "error: Include `.*/a` in `.*/b` is a circular include\n", + )) + .run(); +} diff --git a/tests/lib.rs b/tests/lib.rs index 29ddf2cc..6dbc733e 100644 --- a/tests/lib.rs +++ b/tests/lib.rs @@ -51,6 +51,7 @@ mod fallback; mod fmt; mod functions; mod ignore_comments; +mod includes; mod init; #[cfg(unix)] mod interrupts; @@ -93,3 +94,11 @@ fn path(s: &str) -> String { s.into() } } + +fn path_for_regex(s: &str) -> String { + if cfg!(windows) { + s.replace('/', "\\\\") + } else { + s.into() + } +} diff --git a/tests/test.rs b/tests/test.rs index 62bc0895..8271074f 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -47,6 +47,7 @@ pub(crate) struct Test { pub(crate) stdout: String, pub(crate) stdout_regex: Option, pub(crate) tempdir: TempDir, + pub(crate) test_round_trip: bool, pub(crate) unindent_stdout: bool, } @@ -69,6 +70,7 @@ impl Test { stdout: String::new(), stdout_regex: None, tempdir, + test_round_trip: true, unindent_stdout: true, } } @@ -125,7 +127,7 @@ impl Test { } pub(crate) fn stderr_regex(mut self, stderr_regex: impl AsRef) -> Self { - self.stderr_regex = Some(Regex::new(&format!("(?m)^{}$", stderr_regex.as_ref())).unwrap()); + self.stderr_regex = Some(Regex::new(&format!("^{}$", stderr_regex.as_ref())).unwrap()); self } @@ -140,7 +142,12 @@ impl Test { } pub(crate) fn stdout_regex(mut self, stdout_regex: impl AsRef) -> Self { - self.stdout_regex = Some(Regex::new(&format!("(?m)^{}$", stdout_regex.as_ref())).unwrap()); + self.stdout_regex = Some(Regex::new(&format!("^{}$", stdout_regex.as_ref())).unwrap()); + self + } + + pub(crate) fn test_round_trip(mut self, test_round_trip: bool) -> Self { + self.test_round_trip = test_round_trip; self } @@ -245,7 +252,7 @@ impl Test { panic!("Output mismatch."); } - if self.status == EXIT_SUCCESS { + if self.test_round_trip && self.status == EXIT_SUCCESS { test_round_trip(self.tempdir.path()); }