From 83a96136ff7646f7749ad3f1dfe988db5cf38faa Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Mon, 21 Oct 2024 11:07:17 -0700 Subject: [PATCH 1/6] Allow interchangable recipe redefinitions --- src/analyzer.rs | 9 +++++++++ src/compiler.rs | 10 +--------- src/parser.rs | 24 +++--------------------- src/recipe.rs | 2 -- src/testing.rs | 11 ++--------- src/unresolved_recipe.rs | 1 - tests/imports.rs | 31 +++++++++++++++++++++++++++++++ 7 files changed, 46 insertions(+), 42 deletions(-) diff --git a/src/analyzer.rs b/src/analyzer.rs index ef6bcb2293..519aedb4c7 100644 --- a/src/analyzer.rs +++ b/src/analyzer.rs @@ -141,6 +141,15 @@ impl<'run, 'src> Analyzer<'run, 'src> { let mut deduplicated_recipes = Table::<'src, UnresolvedRecipe<'src>>::default(); for recipe in self.recipes { + // compare name tokens, which include file path and source location, so + // will only return true for the same recipe imported from the same file + if deduplicated_recipes + .values() + .any(|previous| recipe.name == previous.name) + { + continue; + } + Self::define( &mut definitions, recipe.name, diff --git a/src/compiler.rs b/src/compiler.rs index 1a555ebabb..e8d96f2edb 100644 --- a/src/compiler.rs +++ b/src/compiler.rs @@ -21,7 +21,6 @@ impl Compiler { let tokens = Lexer::lex(relative, src)?; let mut ast = Parser::parse( current.file_depth, - ¤t.path, ¤t.import_offsets, ¤t.namepath, &tokens, @@ -214,14 +213,7 @@ impl Compiler { #[cfg(test)] pub(crate) fn test_compile(src: &str) -> CompileResult { let tokens = Lexer::test_lex(src)?; - let ast = Parser::parse( - 0, - &PathBuf::new(), - &[], - &Namepath::default(), - &tokens, - &PathBuf::new(), - )?; + let ast = Parser::parse(0, &[], &Namepath::default(), &tokens, &PathBuf::new())?; let root = PathBuf::from("justfile"); let mut asts: HashMap = HashMap::new(); asts.insert(root.clone(), ast); diff --git a/src/parser.rs b/src/parser.rs index ea727204e5..99544acd13 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -26,7 +26,6 @@ use {super::*, TokenKind::*}; pub(crate) struct Parser<'run, 'src> { expected_tokens: BTreeSet, file_depth: u32, - file_path: &'run Path, import_offsets: Vec, module_namepath: &'run Namepath<'src>, next_token: usize, @@ -39,7 +38,6 @@ impl<'run, 'src> Parser<'run, 'src> { /// Parse `tokens` into an `Ast` pub(crate) fn parse( file_depth: u32, - file_path: &'run Path, import_offsets: &[usize], module_namepath: &'run Namepath<'src>, tokens: &'run [Token<'src>], @@ -48,7 +46,6 @@ impl<'run, 'src> Parser<'run, 'src> { Self { expected_tokens: BTreeSet::new(), file_depth, - file_path, import_offsets: import_offsets.to_vec(), module_namepath, next_token: 0, @@ -910,7 +907,6 @@ impl<'run, 'src> Parser<'run, 'src> { dependencies, doc, file_depth: self.file_depth, - file_path: self.file_path.into(), import_offsets: self.import_offsets.clone(), name, namepath: self.module_namepath.join(name), @@ -1162,15 +1158,8 @@ mod tests { fn test(text: &str, want: Tree) { let unindented = unindent(text); let tokens = Lexer::test_lex(&unindented).expect("lexing failed"); - let justfile = Parser::parse( - 0, - &PathBuf::new(), - &[], - &Namepath::default(), - &tokens, - &PathBuf::new(), - ) - .expect("parsing failed"); + let justfile = Parser::parse(0, &[], &Namepath::default(), &tokens, &PathBuf::new()) + .expect("parsing failed"); let have = justfile.tree(); if have != want { println!("parsed text: {unindented}"); @@ -1208,14 +1197,7 @@ mod tests { ) { let tokens = Lexer::test_lex(src).expect("Lexing failed in parse test..."); - match Parser::parse( - 0, - &PathBuf::new(), - &[], - &Namepath::default(), - &tokens, - &PathBuf::new(), - ) { + match Parser::parse(0, &[], &Namepath::default(), &tokens, &PathBuf::new()) { Ok(_) => panic!("Parsing unexpectedly succeeded"), Err(have) => { let want = CompileError { diff --git a/src/recipe.rs b/src/recipe.rs index bb9fe14e33..05516225a6 100644 --- a/src/recipe.rs +++ b/src/recipe.rs @@ -26,8 +26,6 @@ pub(crate) struct Recipe<'src, D = Dependency<'src>> { #[serde(skip)] pub(crate) file_depth: u32, #[serde(skip)] - pub(crate) file_path: PathBuf, - #[serde(skip)] pub(crate) import_offsets: Vec, pub(crate) name: Name<'src>, pub(crate) namepath: Namepath<'src>, diff --git a/src/testing.rs b/src/testing.rs index a10c398834..12069feede 100644 --- a/src/testing.rs +++ b/src/testing.rs @@ -59,15 +59,8 @@ pub(crate) fn analysis_error( ) { let tokens = Lexer::test_lex(src).expect("Lexing failed in parse test..."); - let ast = Parser::parse( - 0, - &PathBuf::new(), - &[], - &Namepath::default(), - &tokens, - &PathBuf::new(), - ) - .expect("Parsing failed in analysis test..."); + let ast = Parser::parse(0, &[], &Namepath::default(), &tokens, &PathBuf::new()) + .expect("Parsing failed in analysis test..."); let root = PathBuf::from("justfile"); let mut asts: HashMap = HashMap::new(); diff --git a/src/unresolved_recipe.rs b/src/unresolved_recipe.rs index 6cbc95da91..661d75649b 100644 --- a/src/unresolved_recipe.rs +++ b/src/unresolved_recipe.rs @@ -50,7 +50,6 @@ impl<'src> UnresolvedRecipe<'src> { dependencies, doc: self.doc, file_depth: self.file_depth, - file_path: self.file_path, import_offsets: self.import_offsets, name: self.name, namepath: self.namepath, diff --git a/tests/imports.rs b/tests/imports.rs index 693a67cd3b..2022b20581 100644 --- a/tests/imports.rs +++ b/tests/imports.rs @@ -360,3 +360,34 @@ fn reused_import_are_allowed() { }) .run(); } + +#[test] +fn multiply_imported_recipes_do_not_conflict() { + Test::new() + .justfile( + " + import 'a.just' + import 'a.just' + foo: bar + ", + ) + .write("a.just", "bar:") + .run(); +} + +#[test] +fn multiply_imported_recipes_in_nested_imports_do_not_conflict() { + Test::new() + .justfile( + " + import 'a.just' + import 'b.just' + foo: bar + ", + ) + .write("a.just", "import 'c.just'") + .write("b.just", "import 'c.just'") + .write("c.just", "@bar:\n echo 'hello'") + .stdout("hello\n") + .run(); +} From 62162dda5ce2e055ccecad554e1393d58f60b3d5 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Mon, 21 Oct 2024 19:55:12 -0700 Subject: [PATCH 2/6] Ignore duplicate imports --- src/analyzer.rs | 15 +++++---------- tests/imports.rs | 25 +++++++++++++++++++++---- 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/src/analyzer.rs b/src/analyzer.rs index 519aedb4c7..06622205f4 100644 --- a/src/analyzer.rs +++ b/src/analyzer.rs @@ -40,6 +40,8 @@ impl<'run, 'src> Analyzer<'run, 'src> { let ast = asts.get(root).unwrap(); stack.push(ast); + let mut imported = HashSet::new(); + while let Some(ast) = stack.pop() { for item in &ast.items { match item { @@ -54,7 +56,9 @@ impl<'run, 'src> Analyzer<'run, 'src> { Item::Comment(_) => (), Item::Import { absolute, .. } => { if let Some(absolute) = absolute { - stack.push(asts.get(absolute).unwrap()); + if imported.insert(absolute) { + stack.push(asts.get(absolute).unwrap()); + } } } Item::Module { @@ -141,15 +145,6 @@ impl<'run, 'src> Analyzer<'run, 'src> { let mut deduplicated_recipes = Table::<'src, UnresolvedRecipe<'src>>::default(); for recipe in self.recipes { - // compare name tokens, which include file path and source location, so - // will only return true for the same recipe imported from the same file - if deduplicated_recipes - .values() - .any(|previous| recipe.name == previous.name) - { - continue; - } - Self::define( &mut definitions, recipe.name, diff --git a/tests/imports.rs b/tests/imports.rs index 2022b20581..93b3fc75c4 100644 --- a/tests/imports.rs +++ b/tests/imports.rs @@ -362,7 +362,7 @@ fn reused_import_are_allowed() { } #[test] -fn multiply_imported_recipes_do_not_conflict() { +fn multiply_imported_items_do_not_conflict() { Test::new() .justfile( " @@ -371,12 +371,21 @@ fn multiply_imported_recipes_do_not_conflict() { foo: bar ", ) - .write("a.just", "bar:") + .write( + "a.just", + " +x := 'y' + +@bar: + echo hello +", + ) + .stdout("hello\n") .run(); } #[test] -fn multiply_imported_recipes_in_nested_imports_do_not_conflict() { +fn nested_multiply_imported_items_do_not_conflict() { Test::new() .justfile( " @@ -387,7 +396,15 @@ fn multiply_imported_recipes_in_nested_imports_do_not_conflict() { ) .write("a.just", "import 'c.just'") .write("b.just", "import 'c.just'") - .write("c.just", "@bar:\n echo 'hello'") + .write( + "c.just", + " +x := 'y' + +@bar: + echo hello +", + ) .stdout("hello\n") .run(); } From 5a1fd8ab759dbde820523f2851f4eb5f46c4a183 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Mon, 21 Oct 2024 19:57:10 -0700 Subject: [PATCH 3/6] Modify --- src/analyzer.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/analyzer.rs b/src/analyzer.rs index 06622205f4..9a26ef0455 100644 --- a/src/analyzer.rs +++ b/src/analyzer.rs @@ -35,13 +35,12 @@ impl<'run, 'src> Analyzer<'run, 'src> { root: &Path, ) -> CompileResult<'src, Justfile<'src>> { let mut definitions = HashMap::new(); + let mut imports = HashSet::new(); let mut stack = Vec::new(); let ast = asts.get(root).unwrap(); stack.push(ast); - let mut imported = HashSet::new(); - while let Some(ast) = stack.pop() { for item in &ast.items { match item { @@ -56,7 +55,7 @@ impl<'run, 'src> Analyzer<'run, 'src> { Item::Comment(_) => (), Item::Import { absolute, .. } => { if let Some(absolute) = absolute { - if imported.insert(absolute) { + if imports.insert(absolute) { stack.push(asts.get(absolute).unwrap()); } } From bc009d75e27a58baa29f30aa79104b55315534c9 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Fri, 25 Oct 2024 13:58:07 -0700 Subject: [PATCH 4/6] Revise --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index 535b2bc402..60e9b509d9 100644 --- a/README.md +++ b/README.md @@ -3321,6 +3321,8 @@ import? 'foo/bar.just' Missing source files for optional imports do not produce an error. +Importing the same source file multiple times is not an error. + ### Modules1.19.0 A `justfile` can declare modules using `mod` statements. From e9280d24a6f8c404b350fac6b078d3b9a60fe71f Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Wed, 30 Oct 2024 15:19:00 -0700 Subject: [PATCH 5/6] Adjust --- README.md | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 0fe014bd49..fc18345223 100644 --- a/README.md +++ b/README.md @@ -3319,9 +3319,33 @@ Imports may be made optional by putting a `?` after the `import` keyword: import? 'foo/bar.just' ``` -Missing source files for optional imports do not produce an error. +Importing the same source file multiple times is not an errormaster. +This allows importing multiple justfiles, for example `foo.just` and +`bar.just`, which both import a third justfile containing shared recipes, for +example `baz.just`, without the duplicate import of `baz.just` being an error: -Importing the same source file multiple times is not an error. +```just +# justfile +import 'foo.just' +import 'bar.just' +``` + +```just +# foo.just +import 'baz.just' +foo: baz +``` + +```just +# bar.just +import 'baz.just' +bar: baz +``` + +```just +# baz +baz: +``` ### Modules1.19.0 From 92c8fb503d98df348d5b0142f8381efb7936705b Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Wed, 30 Oct 2024 15:20:39 -0700 Subject: [PATCH 6/6] Amend --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index fc18345223..1f2f5409eb 100644 --- a/README.md +++ b/README.md @@ -3324,19 +3324,19 @@ This allows importing multiple justfiles, for example `foo.just` and `bar.just`, which both import a third justfile containing shared recipes, for example `baz.just`, without the duplicate import of `baz.just` being an error: -```just +```mf # justfile import 'foo.just' import 'bar.just' ``` -```just +```mf # foo.just import 'baz.just' foo: baz ``` -```just +```mf # bar.just import 'baz.just' bar: baz