From e9bec8d398cd8a561185cf6336fcffd0d15d31e9 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Wed, 20 Dec 2023 12:31:51 +0800 Subject: [PATCH] Stabilize `!include path` as `import 'path'` (#1771) --- README.md | 28 +++----- src/analyzer.rs | 2 +- src/compile_error.rs | 2 - src/compile_error_kind.rs | 4 -- src/compiler.rs | 34 ++++----- src/error.rs | 10 +-- src/item.rs | 6 +- src/keyword.rs | 3 +- src/lexer.rs | 59 ++-------------- src/node.rs | 2 +- src/parser.rs | 48 +++---------- src/subcommand.rs | 2 +- src/summary.rs | 2 +- src/token_kind.rs | 2 - tests/byte_order_mark.rs | 4 +- tests/error_messages.rs | 7 +- tests/imports.rs | 112 +++++++++++++++++++++++++++++ tests/includes.rs | 144 -------------------------------------- tests/lib.rs | 2 +- 19 files changed, 172 insertions(+), 301 deletions(-) create mode 100644 tests/imports.rs delete mode 100644 tests/includes.rs diff --git a/README.md b/README.md index c8f8771..d7865b1 100644 --- a/README.md +++ b/README.md @@ -2327,24 +2327,22 @@ $ (cd foo && just a b) And will both invoke recipes `a` and `b` in `foo/justfile`. -### Include Directives +### Imports -The `!include` directive, currently unstable, can be used to include the -verbatim text of another file. +One `justfile` can include the contents of another using an `import` statement. If you have the following `justfile`: ```mf -!include foo/bar.just +import 'foo/bar.just' a: b @echo A - ``` And the following text in `foo/bar.just`: -```mf +```just b: @echo B ``` @@ -2352,25 +2350,21 @@ b: `foo/bar.just` will be included in `justfile` and recipe `b` will be defined: ```sh -$ just --unstable b +$ just b B -$ just --unstable a +$ just 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. +The `import` path can be absolute or relative to the location of the justfile +containing it. Justfiles are insensitive to order, so included files can reference variables -and recipes defined after the `!include` directive. +and recipes defined after the `import` statement. -`!include` directives are only processed before the first non-blank, -non-comment line. - -Included files can themselves contain `!include` directives, which are -processed recursively. +Imported files can themselves contain `import`s, which are processed +recursively. ### Hiding `justfile`s diff --git a/src/analyzer.rs b/src/analyzer.rs index b5ae872..e8745e9 100644 --- a/src/analyzer.rs +++ b/src/analyzer.rs @@ -53,7 +53,7 @@ impl<'src> Analyzer<'src> { self.analyze_set(set)?; self.sets.insert(set.clone()); } - Item::Include { absolute, .. } => { + Item::Import { absolute, .. } => { stack.push(asts.get(absolute.as_ref().unwrap()).unwrap()); } } diff --git a/src/compile_error.rs b/src/compile_error.rs index 97406d4..7a0d6f9 100644 --- a/src/compile_error.rs +++ b/src/compile_error.rs @@ -135,7 +135,6 @@ 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 \ @@ -203,7 +202,6 @@ 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 9cbd68b..acd8b2f 100644 --- a/src/compile_error_kind.rs +++ b/src/compile_error_kind.rs @@ -58,7 +58,6 @@ pub(crate) enum CompileErrorKind<'src> { found: usize, expected: Range, }, - IncludeMissingPath, InconsistentLeadingWhitespace { expected: &'src str, found: &'src str, @@ -111,9 +110,6 @@ 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 59d981e..5545f6d 100644 --- a/src/compiler.rs +++ b/src/compiler.rs @@ -4,7 +4,6 @@ pub(crate) struct Compiler; impl Compiler { pub(crate) fn compile<'src>( - unstable: bool, loader: &'src Loader, root: &Path, ) -> RunResult<'src, Compilation<'src>> { @@ -26,18 +25,13 @@ impl Compiler { 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(), - }); + if let Item::Import { relative, absolute } = item { + let import = current.parent().unwrap().join(&relative.cooked).lexiclean(); + if srcs.contains_key(&import) { + return Err(Error::CircularImport { current, import }); } - let include = current.parent().unwrap().join(relative).lexiclean(); - if srcs.contains_key(&include) { - return Err(Error::CircularInclude { current, include }); - } - *absolute = Some(include.clone()); - stack.push(include); + *absolute = Some(import.clone()); + stack.push(import); } } @@ -75,14 +69,14 @@ mod tests { fn include_justfile() { let justfile_a = r#" # A comment at the top of the file -!include ./justfile_b +import "./justfile_b" #some_recipe: recipe_b some_recipe: echo "some recipe" "#; - let justfile_b = r#"!include ./subdir/justfile_c + let justfile_b = r#"import "./subdir/justfile_c" recipe_b: recipe_c echo "recipe b" @@ -103,7 +97,7 @@ recipe_b: recipe_c let loader = Loader::new(); let justfile_a_path = tmp.path().join("justfile"); - let compilation = Compiler::compile(true, &loader, &justfile_a_path).unwrap(); + let compilation = Compiler::compile(&loader, &justfile_a_path).unwrap(); assert_eq!(compilation.root_src(), justfile_a); } @@ -112,7 +106,7 @@ recipe_b: recipe_c fn recursive_includes_fail() { let justfile_a = r#" # A comment at the top of the file -!include ./subdir/justfile_b +import "./subdir/justfile_b" some_recipe: recipe_b echo "some recipe" @@ -120,7 +114,7 @@ some_recipe: recipe_b "#; let justfile_b = r#" -!include ../justfile +import "../justfile" recipe_b: echo "recipe b" @@ -135,11 +129,11 @@ recipe_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(); + let loader_output = Compiler::compile(&loader, &justfile_a_path).unwrap_err(); - assert_matches!(loader_output, Error::CircularInclude { current, include } + assert_matches!(loader_output, Error::CircularImport { current, import } if current == tmp.path().join("subdir").join("justfile_b").lexiclean() && - include == tmp.path().join("justfile").lexiclean() + import == tmp.path().join("justfile").lexiclean() ); } } diff --git a/src/error.rs b/src/error.rs index 2259b05..f6c1f68 100644 --- a/src/error.rs +++ b/src/error.rs @@ -31,9 +31,9 @@ pub(crate) enum Error<'src> { chooser: OsString, io_error: io::Error, }, - CircularInclude { + CircularImport { current: PathBuf, - include: PathBuf, + import: PathBuf, }, Code { recipe: &'src str, @@ -263,10 +263,10 @@ impl<'src> ColorDisplay for Error<'src> { let chooser = chooser.to_string_lossy(); write!(f, "Failed to write to chooser `{chooser}`: {io_error}")?; } - CircularInclude { current, include } => { - let include = include.display(); + CircularImport { current, import } => { + let import = import.display(); let current = current.display(); - write!(f, "Include `{include}` in `{current}` is a circular include")?; + write!(f, "Import `{import}` in `{current}` is circular")?; } Code { recipe, line_number, code, .. } => { if let Some(n) = line_number { diff --git a/src/item.rs b/src/item.rs index 894bae1..a12160c 100644 --- a/src/item.rs +++ b/src/item.rs @@ -8,8 +8,8 @@ pub(crate) enum Item<'src> { Comment(&'src str), Recipe(UnresolvedRecipe<'src>), Set(Set<'src>), - Include { - relative: &'src str, + Import { + relative: StringLiteral<'src>, absolute: Option, }, } @@ -22,7 +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}"), + Item::Import { relative, .. } => write!(f, "import {relative}"), } } } diff --git a/src/keyword.rs b/src/keyword.rs index db2ba95..212ce2a 100644 --- a/src/keyword.rs +++ b/src/keyword.rs @@ -14,13 +14,14 @@ pub(crate) enum Keyword { False, If, IgnoreComments, + Import, PositionalArguments, Set, Shell, + Tempdir, True, WindowsPowershell, WindowsShell, - Tempdir, } impl Keyword { diff --git a/src/lexer.rs b/src/lexer.rs index 3a9a429..37c8f7d 100644 --- a/src/lexer.rs +++ b/src/lexer.rs @@ -481,7 +481,7 @@ impl<'src> Lexer<'src> { fn lex_normal(&mut self, start: char) -> CompileResult<'src, ()> { match start { ' ' | '\t' => self.lex_whitespace(), - '!' => self.lex_bang(), + '!' => self.lex_digraph('!', '=', BangEquals), '#' => self.lex_comment(), '$' => self.lex_single(Dollar), '&' => self.lex_digraph('&', '&', AmpersandAmpersand), @@ -685,33 +685,6 @@ 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)?; @@ -729,6 +702,7 @@ impl<'src> Lexer<'src> { // …and advance past another character, self.advance()?; + // …so that the error we produce highlights the unexpected character. Err(self.error(UnexpectedCharacter { expected: right })) } @@ -980,7 +954,6 @@ mod tests { AmpersandAmpersand => "&&", Asterisk => "*", At => "@", - Bang => "!", BangEquals => "!=", BraceL => "{", BraceR => "}", @@ -2091,30 +2064,6 @@ 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: @@ -2285,8 +2234,8 @@ mod tests { } error! { - name: unexpected_character_after_bang, - input: "!%", + name: unexpected_character_after_at, + input: "@%", offset: 1, line: 0, column: 1, diff --git a/src/node.rs b/src/node.rs index a95933b..9c27ca9 100644 --- a/src/node.rs +++ b/src/node.rs @@ -23,7 +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}\"")), + Item::Import { relative, .. } => Tree::atom("import").push(format!("{relative}")), } } } diff --git a/src/parser.rs b/src/parser.rs index a8a04d8..73f25ee 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -328,6 +328,13 @@ impl<'tokens, 'src> Parser<'tokens, 'src> { self.presume_keyword(Keyword::Export)?; items.push(Item::Assignment(self.parse_assignment(true)?)); } + Some(Keyword::Import) if self.next_are(&[Identifier, StringToken]) => { + self.presume_keyword(Keyword::Import)?; + items.push(Item::Import { + relative: self.parse_string_literal()?, + absolute: None, + }); + } Some(Keyword::Set) if self.next_are(&[Identifier, Identifier, ColonEquals]) || self.next_are(&[Identifier, Identifier, Comment, Eof]) @@ -350,9 +357,6 @@ 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( @@ -778,24 +782,6 @@ 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)?; @@ -1981,9 +1967,9 @@ mod tests { } test! { - name: include_directive, - text: "!include some/file/path.txt \n", - tree: (justfile (include "some/file/path.txt")), + name: import, + text: "import \"some/file/path.txt\" \n", + tree: (justfile (import "some/file/path.txt")), } error! { @@ -2076,7 +2062,7 @@ mod tests { column: 0, width: 1, kind: UnexpectedToken { - expected: vec![At, Bang, BracketL, Comment, Eof, Eol, Identifier], + expected: vec![At, BracketL, Comment, Eof, Eol, Identifier], found: BraceL, }, } @@ -2428,16 +2414,4 @@ 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/subcommand.rs b/src/subcommand.rs index 294b0c1..b587657 100644 --- a/src/subcommand.rs +++ b/src/subcommand.rs @@ -180,7 +180,7 @@ impl Subcommand { loader: &'src Loader, search: &Search, ) -> Result, Error<'src>> { - let compilation = Compiler::compile(config.unstable, loader, &search.justfile)?; + let compilation = Compiler::compile(loader, &search.justfile)?; if config.verbosity.loud() { for warning in &compilation.justfile.warnings { diff --git a/src/summary.rs b/src/summary.rs index 1677f69..b8d2ea7 100644 --- a/src/summary.rs +++ b/src/summary.rs @@ -28,7 +28,7 @@ mod full { pub fn summary(path: &Path) -> Result, io::Error> { let loader = Loader::new(); - match Compiler::compile(false, &loader, path) { + match Compiler::compile(&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() diff --git a/src/token_kind.rs b/src/token_kind.rs index 87c70d5..b00517f 100644 --- a/src/token_kind.rs +++ b/src/token_kind.rs @@ -6,7 +6,6 @@ pub(crate) enum TokenKind { Asterisk, At, Backtick, - Bang, BangEquals, BraceL, BraceR, @@ -49,7 +48,6 @@ impl Display for TokenKind { Asterisk => "'*'", At => "'@'", Backtick => "backtick", - Bang => "'!'", BangEquals => "'!='", BraceL => "'{'", BraceR => "'}'", diff --git a/tests/byte_order_mark.rs b/tests/byte_order_mark.rs index cd72eec..2a45c06 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 --> justfile:3:1 | 3 | \u{feff} @@ -42,7 +42,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 '{' --> justfile:1:1 | 1 | { diff --git a/tests/error_messages.rs b/tests/error_messages.rs index 6e86027..9d376d5 100644 --- a/tests/error_messages.rs +++ b/tests/error_messages.rs @@ -75,9 +75,8 @@ error: Expected '*', ':', '$', identifier, or '+', but found end of file #[test] fn file_paths_are_relative() { Test::new() - .justfile("!include foo/bar.just") + .justfile("import 'foo/bar.just'") .write("foo/bar.just", "baz") - .args(["--unstable"]) .status(EXIT_FAILURE) .stderr(format!( " @@ -95,10 +94,10 @@ error: Expected '*', ':', '$', identifier, or '+', but found end of file #[test] fn file_paths_not_in_subdir_are_absolute() { Test::new() - .write("foo/justfile", "!include ../bar.just") + .write("foo/justfile", "import '../bar.just'") .write("bar.just", "baz") .no_justfile() - .args(["--unstable", "--justfile", "foo/justfile"]) + .args(["--justfile", "foo/justfile"]) .status(EXIT_FAILURE) .stderr_regex(format!( " diff --git a/tests/imports.rs b/tests/imports.rs new file mode 100644 index 0000000..38dfe98 --- /dev/null +++ b/tests/imports.rs @@ -0,0 +1,112 @@ +use super::*; + +#[test] +fn import_succeeds() { + Test::new() + .tree(tree! { + "import.justfile": " + b: + @echo B + ", + }) + .justfile( + " + import './import.justfile' + + a: b + @echo A + ", + ) + .test_round_trip(false) + .arg("a") + .stdout("B\nA\n") + .run(); +} + +#[test] +fn trailing_spaces_after_import_are_ignored() { + Test::new() + .tree(tree! { + "import.justfile": "", + }) + .justfile( + " + import './import.justfile'\x20 + a: + @echo A + ", + ) + .test_round_trip(false) + .stdout("A\n") + .run(); +} + +#[test] +fn import_after_recipe() { + Test::new() + .tree(tree! { + "import.justfile": " + a: + @echo A + ", + }) + .justfile( + " + b: a + import './import.justfile' + ", + ) + .test_round_trip(false) + .stdout("A\n") + .run(); +} + +#[test] +fn circular_import() { + Test::new() + .justfile("import 'a'") + .tree(tree! { + a: "import 'b'", + b: "import 'a'", + }) + .status(EXIT_FAILURE) + .stderr_regex(path_for_regex( + "error: Import `.*/a` in `.*/b` is circular\n", + )) + .run(); +} + +#[test] +fn import_recipes_are_not_default() { + Test::new() + .tree(tree! { + "import.justfile": "bar:", + }) + .justfile("import './import.justfile'") + .test_round_trip(false) + .status(EXIT_FAILURE) + .stderr("error: Justfile contains no default recipe.\n") + .run(); +} + +#[test] +fn listed_recipes_in_imports_are_in_load_order() { + Test::new() + .justfile( + " + import './import.justfile' + foo: + ", + ) + .write("import.justfile", "bar:") + .args(["--list", "--unsorted"]) + .test_round_trip(false) + .stdout( + " + Available recipes: + foo + bar + ", + ) + .run(); +} diff --git a/tests/includes.rs b/tests/includes.rs deleted file mode 100644 index 0bc615c..0000000 --- a/tests/includes.rs +++ /dev/null @@ -1,144 +0,0 @@ -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": "", - }) - .justfile( - " - !include ./include.justfile\x20 - a: - @echo A - ", - ) - .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( - " -error: !include directive has no argument - --> justfile:1:9 - | -1 | !include - | ^ - ", - ) - .run(); -} - -#[test] -fn include_after_recipe() { - Test::new() - .tree(tree! { - "include.justfile": " - a: - @echo A - ", - }) - .justfile( - " - b: a - !include ./include.justfile - ", - ) - .arg("--unstable") - .test_round_trip(false) - .stdout("A\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(); -} - -#[test] -fn include_recipes_are_not_default() { - Test::new() - .tree(tree! { - "include.justfile": "bar:", - }) - .justfile("!include ./include.justfile") - .arg("--unstable") - .test_round_trip(false) - .status(EXIT_FAILURE) - .stderr("error: Justfile contains no default recipe.\n") - .run(); -} - -#[test] -fn listed_recipes_in_includes_are_in_load_order() { - Test::new() - .justfile( - " - !include ./include.justfile - foo: - ", - ) - .write("include.justfile", "bar:") - .args(["--list", "--unstable", "--unsorted"]) - .test_round_trip(false) - .stdout( - " - Available recipes: - foo - bar - ", - ) - .run(); -} diff --git a/tests/lib.rs b/tests/lib.rs index 42153f5..04cbf88 100644 --- a/tests/lib.rs +++ b/tests/lib.rs @@ -55,7 +55,7 @@ mod fallback; mod fmt; mod functions; mod ignore_comments; -mod includes; +mod imports; mod init; #[cfg(unix)] mod interrupts;