From f745316e881b96803373f9060d48ddccc226de44 Mon Sep 17 00:00:00 2001 From: Greg Shuflin Date: Tue, 21 Nov 2023 11:28:59 -0800 Subject: [PATCH] Move !include processing into compiler (#1618) --- src/analyzer.rs | 65 +++++++----- src/compilation.rs | 19 ++++ src/compile_error.rs | 2 + src/compile_error_kind.rs | 4 + src/compiler.rs | 138 +++++++++++++++++++++++++- src/error.rs | 15 --- src/fuzzing.rs | 2 +- src/item.rs | 5 + src/lexer.rs | 82 +++++++++++----- src/lib.rs | 5 +- src/loader.rs | 202 +------------------------------------- src/node.rs | 1 + src/parser.rs | 41 +++++++- src/run.rs | 8 +- src/subcommand.rs | 37 +++---- src/summary.rs | 24 +++-- src/testing.rs | 19 ++-- src/token_kind.rs | 2 + src/tree.rs | 36 ++----- tests/byte_order_mark.rs | 4 +- tests/includes.rs | 23 ++++- 21 files changed, 387 insertions(+), 347 deletions(-) create mode 100644 src/compilation.rs diff --git a/src/analyzer.rs b/src/analyzer.rs index 4516621..ff4051c 100644 --- a/src/analyzer.rs +++ b/src/analyzer.rs @@ -8,35 +8,54 @@ pub(crate) struct Analyzer<'src> { } impl<'src> Analyzer<'src> { - pub(crate) fn analyze(ast: &Ast<'src>) -> CompileResult<'src, Justfile<'src>> { - Analyzer::default().justfile(ast) + pub(crate) fn analyze( + asts: &HashMap>, + root: &Path, + ) -> CompileResult<'src, Justfile<'src>> { + Analyzer::default().justfile(asts, root) } - fn justfile(mut self, ast: &Ast<'src>) -> CompileResult<'src, Justfile<'src>> { + fn justfile( + mut self, + asts: &HashMap>, + root: &Path, + ) -> CompileResult<'src, Justfile<'src>> { let mut recipes = Vec::new(); - for item in &ast.items { - match item { - Item::Alias(alias) => { - self.analyze_alias(alias)?; - self.aliases.insert(alias.clone()); - } - Item::Assignment(assignment) => { - self.analyze_assignment(assignment)?; - self.assignments.insert(assignment.clone()); - } - Item::Comment(_) => (), - Item::Recipe(recipe) => { - if recipe.enabled() { - Self::analyze_recipe(recipe)?; - recipes.push(recipe); + let mut stack = Vec::new(); + stack.push(asts.get(root).unwrap()); + + let mut warnings = Vec::new(); + + while let Some(ast) = stack.pop() { + for item in &ast.items { + match item { + Item::Alias(alias) => { + self.analyze_alias(alias)?; + self.aliases.insert(alias.clone()); + } + Item::Assignment(assignment) => { + self.analyze_assignment(assignment)?; + self.assignments.insert(assignment.clone()); + } + Item::Comment(_) => (), + Item::Recipe(recipe) => { + if recipe.enabled() { + Self::analyze_recipe(recipe)?; + recipes.push(recipe); + } + } + Item::Set(set) => { + self.analyze_set(set)?; + self.sets.insert(set.clone()); + } + Item::Include { absolute, .. } => { + stack.push(asts.get(absolute.as_ref().unwrap()).unwrap()); } } - Item::Set(set) => { - self.analyze_set(set)?; - self.sets.insert(set.clone()); - } } + + warnings.extend(ast.warnings.iter().cloned()); } let settings = Settings::from_setting_iter(self.sets.into_iter().map(|(_, set)| set.value)); @@ -65,7 +84,6 @@ impl<'src> Analyzer<'src> { } Ok(Justfile { - warnings: ast.warnings.clone(), first: recipes .values() .fold(None, |accumulator, next| match accumulator { @@ -80,6 +98,7 @@ impl<'src> Analyzer<'src> { assignments: self.assignments, recipes, settings, + warnings, }) } diff --git a/src/compilation.rs b/src/compilation.rs new file mode 100644 index 0000000..c3bd9d5 --- /dev/null +++ b/src/compilation.rs @@ -0,0 +1,19 @@ +use super::*; + +#[derive(Debug)] +pub(crate) struct Compilation<'src> { + pub(crate) asts: HashMap>, + pub(crate) justfile: Justfile<'src>, + pub(crate) root: PathBuf, + pub(crate) srcs: HashMap, +} + +impl<'src> Compilation<'src> { + pub(crate) fn root_ast(&self) -> &Ast<'src> { + self.asts.get(&self.root).unwrap() + } + + pub(crate) fn root_src(&self) -> &'src str { + self.srcs.get(&self.root).unwrap() + } +} diff --git a/src/compile_error.rs b/src/compile_error.rs index 7a0d6f9..97406d4 100644 --- a/src/compile_error.rs +++ b/src/compile_error.rs @@ -135,6 +135,7 @@ impl Display for CompileError<'_> { Count("argument", *found), expected.display(), ), + IncludeMissingPath => write!(f, "!include directive has no argument",), InconsistentLeadingWhitespace { expected, found } => write!( f, "Recipe line has inconsistent leading whitespace. Recipe started with `{}` but found \ @@ -202,6 +203,7 @@ impl Display for CompileError<'_> { UnknownDependency { recipe, unknown } => { write!(f, "Recipe `{recipe}` has unknown dependency `{unknown}`") } + UnknownDirective { directive } => write!(f, "Unknown directive `!{directive}`"), UnknownFunction { function } => write!(f, "Call to unknown function `{function}`"), UnknownSetting { setting } => write!(f, "Unknown setting `{setting}`"), UnknownStartOfToken => write!(f, "Unknown start of token:"), diff --git a/src/compile_error_kind.rs b/src/compile_error_kind.rs index acd8b2f..9cbd68b 100644 --- a/src/compile_error_kind.rs +++ b/src/compile_error_kind.rs @@ -58,6 +58,7 @@ pub(crate) enum CompileErrorKind<'src> { found: usize, expected: Range, }, + IncludeMissingPath, InconsistentLeadingWhitespace { expected: &'src str, found: &'src str, @@ -110,6 +111,9 @@ pub(crate) enum CompileErrorKind<'src> { recipe: &'src str, unknown: &'src str, }, + UnknownDirective { + directive: &'src str, + }, UnknownFunction { function: &'src str, }, diff --git a/src/compiler.rs b/src/compiler.rs index 999feea..c035921 100644 --- a/src/compiler.rs +++ b/src/compiler.rs @@ -3,11 +3,141 @@ use super::*; pub(crate) struct Compiler; impl Compiler { - pub(crate) fn compile(src: &str) -> CompileResult<(Ast, Justfile)> { + pub(crate) fn compile<'src>( + unstable: bool, + loader: &'src Loader, + root: &Path, + ) -> RunResult<'src, Compilation<'src>> { + let mut srcs: HashMap = HashMap::new(); + let mut asts: HashMap = HashMap::new(); + + let mut paths: Vec = Vec::new(); + paths.push(root.into()); + + while let Some(current) = paths.pop() { + let src = loader.load(¤t)?; + let tokens = Lexer::lex(src)?; + let mut ast = Parser::parse(&tokens)?; + + srcs.insert(current.clone(), src); + + for item in &mut ast.items { + if let Item::Include { relative, absolute } = item { + if !unstable { + return Err(Error::Unstable { + message: "The !include directive is currently unstable.".into(), + }); + } + + let include = current.parent().unwrap().join(relative).lexiclean(); + + if srcs.contains_key(&include) { + return Err(Error::CircularInclude { current, include }); + } + + *absolute = Some(include.clone()); + + paths.push(include); + } + } + + asts.insert(current.clone(), ast.clone()); + } + + let justfile = Analyzer::analyze(&asts, root)?; + + Ok(Compilation { + asts, + srcs, + justfile, + root: root.into(), + }) + } + + #[cfg(test)] + pub(crate) fn test_compile(src: &str) -> CompileResult { let tokens = Lexer::lex(src)?; let ast = Parser::parse(&tokens)?; - let justfile = Analyzer::analyze(&ast)?; - - Ok((ast, justfile)) + let root = PathBuf::from(""); + let mut asts: HashMap = HashMap::new(); + asts.insert(root.clone(), ast); + Analyzer::analyze(&asts, &root) + } +} + +#[cfg(test)] +mod tests { + use {super::*, 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 +some_recipe: + 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 loader = Loader::new(); + + let justfile_a_path = tmp.path().join("justfile"); + let compilation = Compiler::compile(true, &loader, &justfile_a_path).unwrap(); + + assert_eq!(compilation.root_src(), justfile_a); + } + + #[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(); + + let justfile_a_path = tmp.path().join("justfile"); + let loader_output = Compiler::compile(true, &loader, &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/error.rs b/src/error.rs index 6054869..d7573cf 100644 --- a/src/error.rs +++ b/src/error.rs @@ -91,19 +91,12 @@ pub(crate) enum Error<'src> { GetConfirmation { io_error: io::Error, }, - IncludeMissingPath { - file: PathBuf, - line: usize, - }, InitExists { justfile: PathBuf, }, Internal { message: String, }, - InvalidDirective { - line: String, - }, Io { recipe: &'src str, io_error: io::Error, @@ -338,11 +331,6 @@ impl<'src> ColorDisplay for Error<'src> { GetConfirmation { io_error } => { write!(f, "Failed to read confirmation from stdin: {io_error}")?; } - IncludeMissingPath { file: justfile, line } => { - let line = line.ordinal(); - let justfile = justfile.display(); - write!(f, "!include directive on line {line} of `{justfile}` has no argument")?; - } InitExists { justfile } => { write!(f, "Justfile `{}` already exists", justfile.display())?; } @@ -350,9 +338,6 @@ impl<'src> ColorDisplay for Error<'src> { write!(f, "Internal runtime error, this may indicate a bug in just: {message} \ consider filing an issue: https://github.com/casey/just/issues/new")?; } - InvalidDirective { line } => { - write!(f, "Invalid directive: {line}")?; - } Io { recipe, io_error } => { match io_error.kind() { io::ErrorKind::NotFound => write!(f, "Recipe `{recipe}` could not be run because just could not find the shell: {io_error}"), diff --git a/src/fuzzing.rs b/src/fuzzing.rs index 4b8884d..c4438eb 100644 --- a/src/fuzzing.rs +++ b/src/fuzzing.rs @@ -1,5 +1,5 @@ use super::*; pub fn compile(text: &str) { - let _ = compiler::Compiler::compile(text); + let _ = testing::compile(text); } diff --git a/src/item.rs b/src/item.rs index 4fdbddf..894bae1 100644 --- a/src/item.rs +++ b/src/item.rs @@ -8,6 +8,10 @@ pub(crate) enum Item<'src> { Comment(&'src str), Recipe(UnresolvedRecipe<'src>), Set(Set<'src>), + Include { + relative: &'src str, + absolute: Option, + }, } impl<'src> Display for Item<'src> { @@ -18,6 +22,7 @@ impl<'src> Display for Item<'src> { Item::Comment(comment) => write!(f, "{comment}"), Item::Recipe(recipe) => write!(f, "{}", recipe.color_display(Color::never())), Item::Set(set) => write!(f, "{set}"), + Item::Include { relative, .. } => write!(f, "!include {relative}"), } } } diff --git a/src/lexer.rs b/src/lexer.rs index b175a97..3dca62b 100644 --- a/src/lexer.rs +++ b/src/lexer.rs @@ -470,7 +470,7 @@ impl<'src> Lexer<'src> { fn lex_normal(&mut self, start: char) -> CompileResult<'src, ()> { match start { ' ' | '\t' => self.lex_whitespace(), - '!' => self.lex_digraph('!', '=', BangEquals), + '!' => self.lex_bang(), '#' => self.lex_comment(), '$' => self.lex_single(Dollar), '&' => self.lex_digraph('&', '&', AmpersandAmpersand), @@ -674,6 +674,33 @@ impl<'src> Lexer<'src> { !self.open_delimiters.is_empty() } + fn lex_bang(&mut self) -> CompileResult<'src, ()> { + self.presume('!')?; + + // Try to lex a `!=` + if self.accepted('=')? { + self.token(BangEquals); + return Ok(()); + } + + // Otherwise, lex a `!` + self.token(Bang); + + if self.next.map(Self::is_identifier_start).unwrap_or_default() { + self.lex_identifier()?; + + while !self.at_eol_or_eof() { + self.advance()?; + } + + if self.current_token_length() > 0 { + self.token(Text); + } + } + + Ok(()) + } + /// Lex a two-character digraph fn lex_digraph(&mut self, left: char, right: char, token: TokenKind) -> CompileResult<'src, ()> { self.presume(left)?; @@ -942,6 +969,7 @@ mod tests { AmpersandAmpersand => "&&", Asterisk => "*", At => "@", + Bang => "!", BangEquals => "!=", BraceL => "{", BraceR => "}", @@ -2051,6 +2079,30 @@ mod tests { ), } + test! { + name: bang_eof, + text: "!", + tokens: (Bang), + } + + test! { + name: character_after_bang, + text: "!{", + tokens: (Bang, BraceL) + } + + test! { + name: identifier_after_bang, + text: "!include", + tokens: (Bang, Identifier:"include") + } + + test! { + name: identifier_after_bang_with_more_stuff, + text: "!include some/stuff", + tokens: (Bang, Identifier:"include", Text:" some/stuff") + } + error! { name: tokenize_space_then_tab, input: "a: @@ -2222,12 +2274,12 @@ mod tests { error! { name: unexpected_character_after_bang, - input: "!{", + input: "!%", offset: 1, line: 0, column: 1, width: 1, - kind: UnexpectedCharacter { expected: '=' }, + kind: UnknownStartOfToken, } error! { @@ -2244,30 +2296,6 @@ mod tests { }, } - error! { - name: bang_eof, - input: "!", - offset: 1, - line: 0, - column: 1, - width: 0, - kind: UnexpectedEndOfToken { - expected: '=', - }, - } - - error! { - name: bang_unexpected, - input: "!%", - offset: 1, - line: 0, - column: 1, - width: 1, - kind: UnexpectedCharacter { - expected: '=', - }, - } - error! { name: ampersand_eof, input: "&", diff --git a/src/lib.rs b/src/lib.rs index fa4b4e6..bd4b833 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -12,7 +12,7 @@ pub(crate) use { crate::{ alias::Alias, analyzer::Analyzer, assignment::Assignment, assignment_resolver::AssignmentResolver, ast::Ast, attribute::Attribute, binding::Binding, - color::Color, color_display::ColorDisplay, command_ext::CommandExt, + color::Color, color_display::ColorDisplay, command_ext::CommandExt, compilation::Compilation, compile_error::CompileError, compile_error_kind::CompileErrorKind, compiler::Compiler, conditional_operator::ConditionalOperator, config::Config, config_error::ConfigError, count::Count, delimiter::Delimiter, dependency::Dependency, dump_format::DumpFormat, @@ -34,7 +34,7 @@ pub(crate) use { }, std::{ cmp, - collections::{BTreeMap, BTreeSet}, + collections::{BTreeMap, BTreeSet, HashMap}, env, ffi::{OsStr, OsString}, fmt::{self, Debug, Display, Formatter}, @@ -113,6 +113,7 @@ mod binding; mod color; mod color_display; mod command_ext; +mod compilation; mod compile_error; mod compile_error_kind; mod compiler; diff --git a/src/loader.rs b/src/loader.rs index 7ea5612..b1cc094 100644 --- a/src/loader.rs +++ b/src/loader.rs @@ -1,216 +1,22 @@ 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(unstable: bool) -> Self { + pub(crate) fn new() -> Self { Loader { arena: Arena::new(), - unstable, } } pub(crate) fn load<'src>(&'src self, path: &Path) -> RunResult<&'src str> { - 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 { + let src = 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() - ); + Ok(self.arena.alloc(src)) } } diff --git a/src/node.rs b/src/node.rs index 924bcf3..a95933b 100644 --- a/src/node.rs +++ b/src/node.rs @@ -23,6 +23,7 @@ impl<'src> Node<'src> for Item<'src> { Item::Comment(comment) => comment.tree(), Item::Recipe(recipe) => recipe.tree(), Item::Set(set) => set.tree(), + Item::Include { relative, .. } => Tree::atom("include").push(format!("\"{relative}\"")), } } } diff --git a/src/parser.rs b/src/parser.rs index 15590c0..613cb11 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -350,6 +350,9 @@ impl<'tokens, 'src> Parser<'tokens, 'src> { } } } + } else if self.next_is(Bang) { + let directive = self.parse_directive()?; + items.push(directive); } else if self.accepted(At)? { let doc = pop_doc_comment(&mut items, eol_since_last_comment); items.push(Item::Recipe(self.parse_recipe( @@ -775,6 +778,24 @@ impl<'tokens, 'src> Parser<'tokens, 'src> { Ok(value) } + fn parse_directive(&mut self) -> CompileResult<'src, Item<'src>> { + self.presume(Bang)?; + let name = self.expect(Identifier)?; + match name.lexeme() { + "include" => { + if let Some(include_line) = self.accept(Text)? { + Ok(Item::Include { + relative: include_line.lexeme().trim(), + absolute: None, + }) + } else { + Err(self.error(CompileErrorKind::IncludeMissingPath)?) + } + } + directive => Err(name.error(CompileErrorKind::UnknownDirective { directive })), + } + } + /// Parse a setting fn parse_set(&mut self) -> CompileResult<'src, Set<'src>> { self.presume_keyword(Keyword::Set)?; @@ -1958,6 +1979,12 @@ mod tests { tree: (justfile (assignment a (if b == c d (if b == c d e)))), } + test! { + name: include_directive, + text: "!include some/file/path.txt \n", + tree: (justfile (include "some/file/path.txt")), + } + error! { name: alias_syntax_multiple_rhs, input: "alias foo := bar baz", @@ -2048,7 +2075,7 @@ mod tests { column: 0, width: 1, kind: UnexpectedToken { - expected: vec![At, BracketL, Comment, Eof, Eol, Identifier], + expected: vec![At, Bang, BracketL, Comment, Eof, Eol, Identifier], found: BraceL, }, } @@ -2400,4 +2427,16 @@ mod tests { expected: 3..3, }, } + + error! { + name: unknown_directive, + input: "!inclood", + offset: 1, + line: 0, + column: 1, + width: 7, + kind: UnknownDirective { + directive: "inclood" + }, + } } diff --git a/src/run.rs b/src/run.rs index fd8176c..764440e 100644 --- a/src/run.rs +++ b/src/run.rs @@ -20,12 +20,12 @@ pub fn run() -> Result<(), i32> { let config = Config::from_matches(&matches).map_err(Error::from); - let (color, verbosity, unstable) = config + let (color, verbosity) = config .as_ref() - .map(|config| (config.color, config.verbosity, config.unstable)) - .unwrap_or((Color::auto(), Verbosity::default(), false)); + .map(|config| (config.color, config.verbosity)) + .unwrap_or((Color::auto(), Verbosity::default())); - let loader = Loader::new(unstable); + let loader = Loader::new(); config .and_then(|config| config.run(&loader)) diff --git a/src/subcommand.rs b/src/subcommand.rs index b58cc9a..99dacec 100644 --- a/src/subcommand.rs +++ b/src/subcommand.rs @@ -65,7 +65,10 @@ impl Subcommand { return Self::edit(&search); } - let (src, ast, justfile) = Self::compile(config, loader, &search)?; + let compilation = Self::compile(config, loader, &search)?; + let justfile = &compilation.justfile; + let ast = compilation.root_ast(); + let src = compilation.root_src(); match self { Choose { overrides, chooser } => { @@ -86,7 +89,7 @@ impl Subcommand { Ok(()) } - pub(crate) fn run<'src>( + fn run<'src>( config: &Config, loader: &'src Loader, arguments: &[String], @@ -165,8 +168,8 @@ impl Subcommand { overrides: &BTreeMap, search: &Search, ) -> Result<(), (Error<'src>, bool)> { - let (_src, _ast, justfile) = - Self::compile(config, loader, search).map_err(|err| (err, false))?; + let compilation = Self::compile(config, loader, search).map_err(|err| (err, false))?; + let justfile = &compilation.justfile; justfile .run(config, search, overrides, arguments) .map_err(|err| (err, justfile.settings.fallback)) @@ -176,18 +179,16 @@ impl Subcommand { config: &Config, loader: &'src Loader, search: &Search, - ) -> Result<(&'src str, Ast<'src>, Justfile<'src>), Error<'src>> { - let src = loader.load(&search.justfile)?; - - let (ast, justfile) = Compiler::compile(src)?; + ) -> Result, Error<'src>> { + let compilation = Compiler::compile(config.unstable, loader, &search.justfile)?; if config.verbosity.loud() { - for warning in &justfile.warnings { + for warning in &compilation.justfile.warnings { eprintln!("{}", warning.color_display(config.color.stderr())); } } - Ok((src, ast, justfile)) + Ok(compilation) } fn changelog() { @@ -196,7 +197,7 @@ impl Subcommand { fn choose<'src>( config: &Config, - justfile: Justfile<'src>, + justfile: &Justfile<'src>, search: &Search, overrides: &BTreeMap, chooser: Option<&str>, @@ -326,10 +327,10 @@ impl Subcommand { Ok(()) } - fn dump(config: &Config, ast: Ast, justfile: Justfile) -> Result<(), Error<'static>> { + fn dump(config: &Config, ast: &Ast, justfile: &Justfile) -> Result<(), Error<'static>> { match config.dump_format { DumpFormat::Json => { - serde_json::to_writer(io::stdout(), &justfile) + serde_json::to_writer(io::stdout(), justfile) .map_err(|serde_json_error| Error::DumpJson { serde_json_error })?; println!(); } @@ -360,7 +361,7 @@ impl Subcommand { Ok(()) } - fn format(config: &Config, search: &Search, src: &str, ast: Ast) -> Result<(), Error<'static>> { + fn format(config: &Config, search: &Search, src: &str, ast: &Ast) -> Result<(), Error<'static>> { config.require_unstable("The `--fmt` command is currently unstable.")?; let formatted = ast.to_string(); @@ -425,7 +426,7 @@ impl Subcommand { } } - fn list(config: &Config, justfile: Justfile) { + fn list(config: &Config, justfile: &Justfile) { // Construct a target to alias map. let mut recipe_aliases: BTreeMap<&str, Vec<&str>> = BTreeMap::new(); for alias in justfile.aliases.values() { @@ -507,7 +508,7 @@ impl Subcommand { } } - fn show<'src>(config: &Config, name: &str, justfile: Justfile<'src>) -> Result<(), Error<'src>> { + fn show<'src>(config: &Config, name: &str, justfile: &Justfile<'src>) -> Result<(), Error<'src>> { if let Some(alias) = justfile.get_alias(name) { let recipe = justfile.get_recipe(alias.target.name.lexeme()).unwrap(); println!("{alias}"); @@ -524,7 +525,7 @@ impl Subcommand { } } - fn summary(config: &Config, justfile: Justfile) { + fn summary(config: &Config, justfile: &Justfile) { if justfile.count() == 0 { if config.verbosity.loud() { eprintln!("Justfile contains no recipes."); @@ -540,7 +541,7 @@ impl Subcommand { } } - fn variables(justfile: Justfile) { + fn variables(justfile: &Justfile) { for (i, (_, assignment)) in justfile.assignments.iter().enumerate() { if i > 0 { print!(" "); diff --git a/src/summary.rs b/src/summary.rs index becfccb..1677f69 100644 --- a/src/summary.rs +++ b/src/summary.rs @@ -13,8 +13,8 @@ //! of existing justfiles. use { - crate::compiler::Compiler, - std::{collections::BTreeMap, fs, io, path::Path}, + crate::{compiler::Compiler, error::Error, loader::Loader}, + std::{collections::BTreeMap, io, path::Path}, }; mod full { @@ -26,11 +26,15 @@ mod full { } pub fn summary(path: &Path) -> Result, io::Error> { - let text = fs::read_to_string(path)?; + let loader = Loader::new(); - match Compiler::compile(&text) { - Ok((_, justfile)) => Ok(Ok(Summary::new(justfile))), - Err(compilation_error) => Ok(Err(compilation_error.to_string())), + match Compiler::compile(false, &loader, path) { + Ok(compilation) => Ok(Ok(Summary::new(&compilation.justfile))), + Err(error) => Ok(Err(if let Error::Compile { compile_error } = error { + compile_error.to_string() + } else { + format!("{error:?}") + })), } } @@ -41,7 +45,7 @@ pub struct Summary { } impl Summary { - fn new(justfile: full::Justfile) -> Summary { + fn new(justfile: &full::Justfile) -> Summary { let mut aliases = BTreeMap::new(); for alias in justfile.aliases.values() { @@ -54,11 +58,11 @@ impl Summary { Summary { recipes: justfile .recipes - .into_iter() + .iter() .map(|(name, recipe)| { ( - name.to_owned(), - Recipe::new(&recipe, aliases.remove(name).unwrap_or_default()), + (*name).to_string(), + Recipe::new(recipe, aliases.remove(name).unwrap_or_default()), ) }) .collect(), diff --git a/src/testing.rs b/src/testing.rs index 4085721..b526e83 100644 --- a/src/testing.rs +++ b/src/testing.rs @@ -1,10 +1,7 @@ -use {super::*, crate::compiler::Compiler, pretty_assertions::assert_eq}; +use {super::*, pretty_assertions::assert_eq}; -pub(crate) fn compile(text: &str) -> Justfile { - match Compiler::compile(text) { - Ok((_, justfile)) => justfile, - Err(error) => panic!("Expected successful compilation but got error:\n {error}"), - } +pub(crate) fn compile(src: &str) -> Justfile { + Compiler::test_compile(src).expect("expected successful compilation") } pub(crate) fn config(args: &[&str]) -> Config { @@ -64,7 +61,11 @@ pub(crate) fn analysis_error( let ast = Parser::parse(&tokens).expect("Parsing failed in analysis test..."); - match Analyzer::analyze(&ast) { + let root = PathBuf::from(""); + let mut asts: HashMap = HashMap::new(); + asts.insert(root.clone(), ast); + + match Analyzer::analyze(&asts, &root) { Ok(_) => panic!("Analysis unexpectedly succeeded"), Err(have) => { let want = CompileError { @@ -97,9 +98,7 @@ macro_rules! run_error { let search = $crate::testing::search(&config); if let Subcommand::Run{ overrides, arguments } = &config.subcommand { - match $crate::compiler::Compiler::compile(&$crate::unindent::unindent($src)) - .expect("Expected successful compilation") - .1 + match $crate::testing::compile(&$crate::unindent::unindent($src)) .run( &config, &search, diff --git a/src/token_kind.rs b/src/token_kind.rs index b00517f..87c70d5 100644 --- a/src/token_kind.rs +++ b/src/token_kind.rs @@ -6,6 +6,7 @@ pub(crate) enum TokenKind { Asterisk, At, Backtick, + Bang, BangEquals, BraceL, BraceR, @@ -48,6 +49,7 @@ impl Display for TokenKind { Asterisk => "'*'", At => "'@'", Backtick => "backtick", + Bang => "'!'", BangEquals => "'!='", BraceL => "'{'", BraceR => "'}'", diff --git a/src/tree.rs b/src/tree.rs index c005762..d1b1c2a 100644 --- a/src/tree.rs +++ b/src/tree.rs @@ -7,57 +7,39 @@ use { /// Tree type, are only used in the Parser unit tests, providing a concise /// notation for representing the expected results of parsing a given string. macro_rules! tree { - { - ($($child:tt)*) - } => { + { ($($child:tt)*) } => { $crate::tree::Tree::List(vec![$(tree!($child),)*]) }; - { - $atom:ident - } => { + { $atom:ident } => { $crate::tree::Tree::atom(stringify!($atom)) }; - { - $atom:literal - } => { + { $atom:literal } => { $crate::tree::Tree::atom(format!("\"{}\"", $atom)) }; - { - # - } => { + { # } => { $crate::tree::Tree::atom("#") }; - { - + - } => { + { + } => { $crate::tree::Tree::atom("+") }; - { - * - } => { + { * } => { $crate::tree::Tree::atom("*") }; - { - && - } => { + { && } => { $crate::tree::Tree::atom("&&") }; - { - == - } => { + { == } => { $crate::tree::Tree::atom("==") }; - { - != - } => { + { != } => { $crate::tree::Tree::atom("!=") }; } diff --git a/tests/byte_order_mark.rs b/tests/byte_order_mark.rs index 58184a7..13283d8 100644 --- a/tests/byte_order_mark.rs +++ b/tests/byte_order_mark.rs @@ -26,7 +26,7 @@ fn non_leading_byte_order_mark_produces_error() { ) .stderr( " - error: Expected \'@\', \'[\', comment, end of file, end of line, or identifier, but found byte order mark + error: Expected \'@\', '!', \'[\', comment, end of file, end of line, or identifier, but found byte order mark | 3 | \u{feff} | ^ @@ -41,7 +41,7 @@ fn dont_mention_byte_order_mark_in_errors() { .justfile("{") .stderr( " - error: Expected '@', '[', comment, end of file, end of line, or identifier, but found '{' + error: Expected '@', '!', '[', comment, end of file, end of line, or identifier, but found '{' | 1 | { | ^ diff --git a/tests/includes.rs b/tests/includes.rs index 8850b9b..9c22832 100644 --- a/tests/includes.rs +++ b/tests/includes.rs @@ -55,22 +55,35 @@ fn include_directive_with_no_path() { .justfile("!include") .arg("--unstable") .status(EXIT_FAILURE) - .stderr_regex("error: !include directive on line 1 of `.*` has no argument\n") + .stderr( + " +error: !include directive has no argument + | +1 | !include + | ^ + ", + ) .run(); } #[test] -fn trailing_include() { +fn include_after_recipe() { Test::new() + .tree(tree! { + "include.justfile": " + a: + @echo A + ", + }) .justfile( " - b: + b: a !include ./include.justfile ", ) .arg("--unstable") - .status(EXIT_FAILURE) - .stderr("error: Expected character `=`\n |\n2 | !include ./include.justfile\n | ^\n") + .test_round_trip(false) + .stdout("A\n") .run(); }