From 45aa827da739b1528ce62175c54bf683821d199b Mon Sep 17 00:00:00 2001 From: Greg Lutostanski Date: Wed, 9 Feb 2022 19:42:01 -0600 Subject: [PATCH 01/13] WIP on allow-duplicates --- src/analyzer.rs | 74 ++++++++++++++++++++++++++++++++----------------- src/keyword.rs | 1 + src/node.rs | 2 +- src/parser.rs | 14 +++++++++- src/setting.rs | 4 ++- src/settings.rs | 2 ++ 6 files changed, 69 insertions(+), 28 deletions(-) diff --git a/src/analyzer.rs b/src/analyzer.rs index c8bfe3df47..d389f81fbd 100644 --- a/src/analyzer.rs +++ b/src/analyzer.rs @@ -1,4 +1,5 @@ use crate::common::*; +use crate::compile_error::*; use CompileErrorKind::*; @@ -16,6 +17,9 @@ impl<'src> Analyzer<'src> { } pub(crate) fn justfile(mut self, ast: Ast<'src>) -> CompileResult<'src, Justfile<'src>> { + + let mut settings = Settings::new(); + for item in ast.items { match item { Item::Alias(alias) => { @@ -28,12 +32,20 @@ impl<'src> Analyzer<'src> { } Item::Comment(_) => (), Item::Recipe(recipe) => { - self.analyze_recipe(&recipe)?; + let r = self.analyze_recipe(&recipe); + match r { + Err(CompileError{token: _, kind: DuplicateRecipe{..}}) if settings.allow_duplicates => { + self.recipes.remove(recipe.name.lexeme()); + }, + Err(e) => return Err(e), + Ok(_) => (), + }; self.recipes.insert(recipe); } Item::Set(set) => { self.analyze_set(&set)?; - self.sets.insert(set); + // self.sets.insert(set); + self.resolve_set(&mut settings, set)?; } } } @@ -59,29 +71,6 @@ impl<'src> Analyzer<'src> { aliases.insert(Self::resolve_alias(&recipes, alias)?); } - let mut settings = Settings::new(); - - for (_, set) in self.sets { - match set.value { - Setting::DotenvLoad(dotenv_load) => { - settings.dotenv_load = Some(dotenv_load); - } - Setting::Export(export) => { - settings.export = export; - } - Setting::PositionalArguments(positional_arguments) => { - settings.positional_arguments = positional_arguments; - } - Setting::Shell(shell) => { - assert!(settings.shell.is_none()); - settings.shell = Some(shell); - } - Setting::WindowsPowerShell(windows_powershell) => { - settings.windows_powershell = windows_powershell; - } - } - } - Ok(Justfile { warnings: ast.warnings, first: recipes @@ -186,6 +175,31 @@ impl<'src> Analyzer<'src> { Ok(()) } + fn resolve_set(&self, settings: &mut Settings<'src>, set: Set<'src>) -> CompileResult<'src, ()> { + match set.value { + Setting::AllowDuplicates(allow_duplicates) => { + settings.allow_duplicates = allow_duplicates; + } + Setting::DotenvLoad(dotenv_load) => { + settings.dotenv_load = Some(dotenv_load); + } + Setting::Export(export) => { + settings.export = export; + } + Setting::PositionalArguments(positional_arguments) => { + settings.positional_arguments = positional_arguments; + } + Setting::Shell(shell) => { + assert!(settings.shell.is_none()); + settings.shell = Some(shell); + } + Setting::WindowsPowerShell(windows_powershell) => { + settings.windows_powershell = windows_powershell; + } + }; + Ok(()) + } + fn resolve_alias( recipes: &Table<'src, Rc>>, alias: Alias<'src, Name<'src>>, @@ -303,6 +317,16 @@ mod tests { width: 1, kind: DuplicateRecipe{recipe: "a", first: 0}, } + + analysis_error! { + name: duplicate_recipes_with_allowed_duplicates, + input: "set allow-duplicates := true\na:\nb:\na:", + offset: 6, + line: 2, + column: 0, + width: 1, + kind: DuplicateRecipe{recipe: "a", first: 0}, + } analysis_error! { name: duplicate_variable, diff --git a/src/keyword.rs b/src/keyword.rs index 2ed06d8bbb..9c1e6f3b05 100644 --- a/src/keyword.rs +++ b/src/keyword.rs @@ -4,6 +4,7 @@ use crate::common::*; #[strum(serialize_all = "kebab_case")] pub(crate) enum Keyword { Alias, + AllowDuplicates, DotenvLoad, Else, Export, diff --git a/src/node.rs b/src/node.rs index 270d6bb2b8..eab2cb8986 100644 --- a/src/node.rs +++ b/src/node.rs @@ -221,7 +221,7 @@ impl<'src> Node<'src> for Set<'src> { set.push_mut(self.name.lexeme().replace('-', "_")); match &self.value { - DotenvLoad(value) | Export(value) | PositionalArguments(value) | WindowsPowerShell(value) => { + AllowDuplicates(value) | DotenvLoad(value) | Export(value) | PositionalArguments(value) | WindowsPowerShell(value) => { set.push_mut(value.to_string()); } Shell(setting::Shell { command, arguments }) => { diff --git a/src/parser.rs b/src/parser.rs index 996c07a4ca..653cd1b6c4 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -729,7 +729,13 @@ impl<'tokens, 'src> Parser<'tokens, 'src> { let name = Name::from_identifier(self.presume(Identifier)?); let lexeme = name.lexeme(); - if Keyword::DotenvLoad == lexeme { + if Keyword::AllowDuplicates == lexeme { + let value = self.parse_set_bool()?; + return Ok(Set { + value: Setting::AllowDuplicates(value), + name, + }); + } else if Keyword::DotenvLoad == lexeme { let value = self.parse_set_bool()?; return Ok(Set { value: Setting::DotenvLoad(value), @@ -1713,6 +1719,12 @@ mod tests { text: "set dotenv-load", tree: (justfile (set dotenv_load true)), } + + test! { + name: set_allow_duplicates_implicit, + text: "set allow-duplicates", + tree: (justfile (set allow_duplicates true)), + } test! { name: set_dotenv_load_true, diff --git a/src/setting.rs b/src/setting.rs index 39c9ecadc6..7850edbf0a 100644 --- a/src/setting.rs +++ b/src/setting.rs @@ -2,6 +2,7 @@ use crate::common::*; #[derive(Debug, Clone)] pub(crate) enum Setting<'src> { + AllowDuplicates(bool), DotenvLoad(bool), Export(bool), PositionalArguments(bool), @@ -18,7 +19,8 @@ pub(crate) struct Shell<'src> { impl<'src> Display for Setting<'src> { fn fmt(&self, f: &mut Formatter) -> Result<(), fmt::Error> { match self { - Setting::DotenvLoad(value) + Setting::AllowDuplicates(value) + | Setting::DotenvLoad(value) | Setting::Export(value) | Setting::PositionalArguments(value) | Setting::WindowsPowerShell(value) => write!(f, "{}", value), diff --git a/src/settings.rs b/src/settings.rs index 8078ebc505..7ff9ff68f4 100644 --- a/src/settings.rs +++ b/src/settings.rs @@ -7,6 +7,7 @@ pub(crate) const WINDOWS_POWERSHELL_ARGS: &[&str] = &["-NoLogo", "-Command"]; #[derive(Debug, PartialEq, Serialize)] pub(crate) struct Settings<'src> { + pub(crate) allow_duplicates: bool, pub(crate) dotenv_load: Option, pub(crate) export: bool, pub(crate) positional_arguments: bool, @@ -17,6 +18,7 @@ pub(crate) struct Settings<'src> { impl<'src> Settings<'src> { pub(crate) fn new() -> Settings<'src> { Settings { + allow_duplicates: false, dotenv_load: None, export: false, positional_arguments: false, From 57f3d6085b945b9563564a2e8457927c70fac961 Mon Sep 17 00:00:00 2001 From: Greg Lutostanski Date: Wed, 9 Feb 2022 19:48:48 -0600 Subject: [PATCH 02/13] fixup tests --- src/analyzer.rs | 10 --------- tests/json.rs | 59 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 10 deletions(-) diff --git a/src/analyzer.rs b/src/analyzer.rs index d389f81fbd..ebac25f217 100644 --- a/src/analyzer.rs +++ b/src/analyzer.rs @@ -318,16 +318,6 @@ mod tests { kind: DuplicateRecipe{recipe: "a", first: 0}, } - analysis_error! { - name: duplicate_recipes_with_allowed_duplicates, - input: "set allow-duplicates := true\na:\nb:\na:", - offset: 6, - line: 2, - column: 0, - width: 1, - kind: DuplicateRecipe{recipe: "a", first: 0}, - } - analysis_error! { name: duplicate_variable, input: "a := \"0\"\na := \"0\"", diff --git a/tests/json.rs b/tests/json.rs index 2604f6f3d1..8d8ab5b5d9 100644 --- a/tests/json.rs +++ b/tests/json.rs @@ -39,6 +39,7 @@ fn alias() { } }, "settings": { + "allow_duplicates": false, "dotenv_load": null, "export": false, "positional_arguments": false, @@ -66,6 +67,7 @@ fn assignment() { "first": null, "recipes": {}, "settings": { + "allow_duplicates": false, "dotenv_load": null, "export": false, "positional_arguments": false, @@ -106,6 +108,7 @@ fn body() { } }, "settings": { + "allow_duplicates": false, "dotenv_load": null, "export": false, "positional_arguments": false, @@ -156,6 +159,7 @@ fn dependencies() { } }, "settings": { + "allow_duplicates": false, "dotenv_load": null, "export": false, "positional_arguments": false, @@ -243,6 +247,52 @@ fn dependency_argument() { } }, "settings": { + "allow_duplicates": false, + "dotenv_load": null, + "export": false, + "positional_arguments": false, + "shell": null, + "windows_powershell": false, + }, + "warnings": [], + }), + ); +} + +#[test] +fn duplicates() { + test( + " + set allow-duplicates + alias f := foo + + foo: + foo: + ", + json!({ + "first": "foo", + "aliases": { + "f": { + "name": "f", + "target": "foo", + } + }, + "assignments": {}, + "recipes": { + "foo": { + "body": [], + "dependencies": [], + "doc": null, + "name": "foo", + "parameters": [], + "priors": 0, + "private": false, + "quiet": false, + "shebang": false, + } + }, + "settings": { + "allow_duplicates": true, "dotenv_load": null, "export": false, "positional_arguments": false, @@ -276,6 +326,7 @@ fn doc_comment() { } }, "settings": { + "allow_duplicates": false, "dotenv_load": null, "export": false, "positional_arguments": false, @@ -297,6 +348,7 @@ fn empty_justfile() { "first": null, "recipes": {}, "settings": { + "allow_duplicates": false, "dotenv_load": null, "export": false, "positional_arguments": false, @@ -427,6 +479,7 @@ fn parameters() { }, }, "settings": { + "allow_duplicates": false, "dotenv_load": null, "export": false, "positional_arguments": false, @@ -496,6 +549,7 @@ fn priors() { }, }, "settings": { + "allow_duplicates": false, "dotenv_load": null, "export": false, "positional_arguments": false, @@ -529,6 +583,7 @@ fn private() { } }, "settings": { + "allow_duplicates": false, "dotenv_load": null, "export": false, "positional_arguments": false, @@ -562,6 +617,7 @@ fn quiet() { } }, "settings": { + "allow_duplicates": false, "dotenv_load": null, "export": false, "positional_arguments": false, @@ -613,6 +669,7 @@ fn settings() { } }, "settings": { + "allow_duplicates": false, "dotenv_load": true, "export": true, "positional_arguments": true, @@ -652,6 +709,7 @@ fn shebang() { } }, "settings": { + "allow_duplicates": false, "dotenv_load": null, "export": false, "positional_arguments": false, @@ -685,6 +743,7 @@ fn simple() { } }, "settings": { + "allow_duplicates": false, "dotenv_load": null, "export": false, "positional_arguments": false, From a24a064c9a67be4b9805d525367d66155500aa00 Mon Sep 17 00:00:00 2001 From: Greg Lutostanski Date: Thu, 10 Feb 2022 04:28:32 -0600 Subject: [PATCH 03/13] rename allow-duplicates to allow-duplicate-recipes --- src/analyzer.rs | 6 +++--- src/keyword.rs | 2 +- src/node.rs | 2 +- src/parser.rs | 12 ++++++------ src/setting.rs | 4 ++-- src/settings.rs | 4 ++-- tests/json.rs | 34 +++++++++++++++++----------------- 7 files changed, 32 insertions(+), 32 deletions(-) diff --git a/src/analyzer.rs b/src/analyzer.rs index ebac25f217..7581ce3bc1 100644 --- a/src/analyzer.rs +++ b/src/analyzer.rs @@ -34,7 +34,7 @@ impl<'src> Analyzer<'src> { Item::Recipe(recipe) => { let r = self.analyze_recipe(&recipe); match r { - Err(CompileError{token: _, kind: DuplicateRecipe{..}}) if settings.allow_duplicates => { + Err(CompileError{token: _, kind: DuplicateRecipe{..}}) if settings.allow_duplicate_recipes => { self.recipes.remove(recipe.name.lexeme()); }, Err(e) => return Err(e), @@ -177,8 +177,8 @@ impl<'src> Analyzer<'src> { fn resolve_set(&self, settings: &mut Settings<'src>, set: Set<'src>) -> CompileResult<'src, ()> { match set.value { - Setting::AllowDuplicates(allow_duplicates) => { - settings.allow_duplicates = allow_duplicates; + Setting::AllowDuplicateRecipes(allow_duplicate_recipes) => { + settings.allow_duplicate_recipes = allow_duplicate_recipes; } Setting::DotenvLoad(dotenv_load) => { settings.dotenv_load = Some(dotenv_load); diff --git a/src/keyword.rs b/src/keyword.rs index 9c1e6f3b05..0f3f6fc717 100644 --- a/src/keyword.rs +++ b/src/keyword.rs @@ -4,7 +4,7 @@ use crate::common::*; #[strum(serialize_all = "kebab_case")] pub(crate) enum Keyword { Alias, - AllowDuplicates, + AllowDuplicateRecipes, DotenvLoad, Else, Export, diff --git a/src/node.rs b/src/node.rs index eab2cb8986..255876f3a9 100644 --- a/src/node.rs +++ b/src/node.rs @@ -221,7 +221,7 @@ impl<'src> Node<'src> for Set<'src> { set.push_mut(self.name.lexeme().replace('-', "_")); match &self.value { - AllowDuplicates(value) | DotenvLoad(value) | Export(value) | PositionalArguments(value) | WindowsPowerShell(value) => { + AllowDuplicateRecipes(value) | DotenvLoad(value) | Export(value) | PositionalArguments(value) | WindowsPowerShell(value) => { set.push_mut(value.to_string()); } Shell(setting::Shell { command, arguments }) => { diff --git a/src/parser.rs b/src/parser.rs index 653cd1b6c4..cbefc1ad13 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -729,10 +729,10 @@ impl<'tokens, 'src> Parser<'tokens, 'src> { let name = Name::from_identifier(self.presume(Identifier)?); let lexeme = name.lexeme(); - if Keyword::AllowDuplicates == lexeme { + if Keyword::AllowDuplicateRecipes == lexeme { let value = self.parse_set_bool()?; return Ok(Set { - value: Setting::AllowDuplicates(value), + value: Setting::AllowDuplicateRecipes(value), name, }); } else if Keyword::DotenvLoad == lexeme { @@ -1719,11 +1719,11 @@ mod tests { text: "set dotenv-load", tree: (justfile (set dotenv_load true)), } - + test! { - name: set_allow_duplicates_implicit, - text: "set allow-duplicates", - tree: (justfile (set allow_duplicates true)), + name: set_allow_duplicate_recipes_implicit, + text: "set allow-duplicate-recipes", + tree: (justfile (set allow_duplicate_recipes true)), } test! { diff --git a/src/setting.rs b/src/setting.rs index 7850edbf0a..220ce9dcf5 100644 --- a/src/setting.rs +++ b/src/setting.rs @@ -2,7 +2,7 @@ use crate::common::*; #[derive(Debug, Clone)] pub(crate) enum Setting<'src> { - AllowDuplicates(bool), + AllowDuplicateRecipes(bool), DotenvLoad(bool), Export(bool), PositionalArguments(bool), @@ -19,7 +19,7 @@ pub(crate) struct Shell<'src> { impl<'src> Display for Setting<'src> { fn fmt(&self, f: &mut Formatter) -> Result<(), fmt::Error> { match self { - Setting::AllowDuplicates(value) + Setting::AllowDuplicateRecipes(value) | Setting::DotenvLoad(value) | Setting::Export(value) | Setting::PositionalArguments(value) diff --git a/src/settings.rs b/src/settings.rs index 7ff9ff68f4..9ba196d780 100644 --- a/src/settings.rs +++ b/src/settings.rs @@ -7,7 +7,7 @@ pub(crate) const WINDOWS_POWERSHELL_ARGS: &[&str] = &["-NoLogo", "-Command"]; #[derive(Debug, PartialEq, Serialize)] pub(crate) struct Settings<'src> { - pub(crate) allow_duplicates: bool, + pub(crate) allow_duplicate_recipes: bool, pub(crate) dotenv_load: Option, pub(crate) export: bool, pub(crate) positional_arguments: bool, @@ -18,7 +18,7 @@ pub(crate) struct Settings<'src> { impl<'src> Settings<'src> { pub(crate) fn new() -> Settings<'src> { Settings { - allow_duplicates: false, + allow_duplicate_recipes: false, dotenv_load: None, export: false, positional_arguments: false, diff --git a/tests/json.rs b/tests/json.rs index 8d8ab5b5d9..30f3254ec0 100644 --- a/tests/json.rs +++ b/tests/json.rs @@ -39,7 +39,7 @@ fn alias() { } }, "settings": { - "allow_duplicates": false, + "allow_duplicate_recipes": false, "dotenv_load": null, "export": false, "positional_arguments": false, @@ -67,7 +67,7 @@ fn assignment() { "first": null, "recipes": {}, "settings": { - "allow_duplicates": false, + "allow_duplicate_recipes": false, "dotenv_load": null, "export": false, "positional_arguments": false, @@ -108,7 +108,7 @@ fn body() { } }, "settings": { - "allow_duplicates": false, + "allow_duplicate_recipes": false, "dotenv_load": null, "export": false, "positional_arguments": false, @@ -159,7 +159,7 @@ fn dependencies() { } }, "settings": { - "allow_duplicates": false, + "allow_duplicate_recipes": false, "dotenv_load": null, "export": false, "positional_arguments": false, @@ -247,7 +247,7 @@ fn dependency_argument() { } }, "settings": { - "allow_duplicates": false, + "allow_duplicate_recipes": false, "dotenv_load": null, "export": false, "positional_arguments": false, @@ -260,10 +260,10 @@ fn dependency_argument() { } #[test] -fn duplicates() { +fn duplicate_recipes() { test( " - set allow-duplicates + set allow-duplicate-recipes alias f := foo foo: @@ -292,7 +292,7 @@ fn duplicates() { } }, "settings": { - "allow_duplicates": true, + "allow_duplicate_recipes": true, "dotenv_load": null, "export": false, "positional_arguments": false, @@ -326,7 +326,7 @@ fn doc_comment() { } }, "settings": { - "allow_duplicates": false, + "allow_duplicate_recipes": false, "dotenv_load": null, "export": false, "positional_arguments": false, @@ -348,7 +348,7 @@ fn empty_justfile() { "first": null, "recipes": {}, "settings": { - "allow_duplicates": false, + "allow_duplicate_recipes": false, "dotenv_load": null, "export": false, "positional_arguments": false, @@ -479,7 +479,7 @@ fn parameters() { }, }, "settings": { - "allow_duplicates": false, + "allow_duplicate_recipes": false, "dotenv_load": null, "export": false, "positional_arguments": false, @@ -549,7 +549,7 @@ fn priors() { }, }, "settings": { - "allow_duplicates": false, + "allow_duplicate_recipes": false, "dotenv_load": null, "export": false, "positional_arguments": false, @@ -583,7 +583,7 @@ fn private() { } }, "settings": { - "allow_duplicates": false, + "allow_duplicate_recipes": false, "dotenv_load": null, "export": false, "positional_arguments": false, @@ -617,7 +617,7 @@ fn quiet() { } }, "settings": { - "allow_duplicates": false, + "allow_duplicate_recipes": false, "dotenv_load": null, "export": false, "positional_arguments": false, @@ -669,7 +669,7 @@ fn settings() { } }, "settings": { - "allow_duplicates": false, + "allow_duplicate_recipes": false, "dotenv_load": true, "export": true, "positional_arguments": true, @@ -709,7 +709,7 @@ fn shebang() { } }, "settings": { - "allow_duplicates": false, + "allow_duplicate_recipes": false, "dotenv_load": null, "export": false, "positional_arguments": false, @@ -743,7 +743,7 @@ fn simple() { } }, "settings": { - "allow_duplicates": false, + "allow_duplicate_recipes": false, "dotenv_load": null, "export": false, "positional_arguments": false, From 0d8befe83d7d2fd5eec82f8278be6a28e8896a7e Mon Sep 17 00:00:00 2001 From: Greg Lutostanski Date: Thu, 10 Feb 2022 05:03:36 -0600 Subject: [PATCH 04/13] ensure allow-duplicate-recipes works anywhere in the justfile --- src/analyzer.rs | 89 ++++++++++++++++++++++++------------------------- tests/misc.rs | 8 +++++ 2 files changed, 52 insertions(+), 45 deletions(-) diff --git a/src/analyzer.rs b/src/analyzer.rs index 7581ce3bc1..0da6a9b24a 100644 --- a/src/analyzer.rs +++ b/src/analyzer.rs @@ -1,5 +1,4 @@ use crate::common::*; -use crate::compile_error::*; use CompileErrorKind::*; @@ -17,8 +16,7 @@ impl<'src> Analyzer<'src> { } pub(crate) fn justfile(mut self, ast: Ast<'src>) -> CompileResult<'src, Justfile<'src>> { - - let mut settings = Settings::new(); + let mut recipes: Vec> = Vec::new(); for item in ast.items { match item { @@ -32,20 +30,38 @@ impl<'src> Analyzer<'src> { } Item::Comment(_) => (), Item::Recipe(recipe) => { - let r = self.analyze_recipe(&recipe); - match r { - Err(CompileError{token: _, kind: DuplicateRecipe{..}}) if settings.allow_duplicate_recipes => { - self.recipes.remove(recipe.name.lexeme()); - }, - Err(e) => return Err(e), - Ok(_) => (), - }; - self.recipes.insert(recipe); + self.analyze_recipe(&recipe)?; + recipes.push(recipe); } Item::Set(set) => { self.analyze_set(&set)?; - // self.sets.insert(set); - self.resolve_set(&mut settings, set)?; + self.sets.insert(set); + } + } + } + + let mut settings = Settings::new(); + + for (_, set) in self.sets { + match set.value { + Setting::AllowDuplicateRecipes(allow_duplicate_recipes) => { + settings.allow_duplicate_recipes = allow_duplicate_recipes; + } + Setting::DotenvLoad(dotenv_load) => { + settings.dotenv_load = Some(dotenv_load); + } + Setting::Export(export) => { + settings.export = export; + } + Setting::PositionalArguments(positional_arguments) => { + settings.positional_arguments = positional_arguments; + } + Setting::Shell(shell) => { + assert!(settings.shell.is_none()); + settings.shell = Some(shell); + } + Setting::WindowsPowerShell(windows_powershell) => { + settings.windows_powershell = windows_powershell; } } } @@ -54,6 +70,20 @@ impl<'src> Analyzer<'src> { AssignmentResolver::resolve_assignments(&assignments)?; + for recipe in recipes.into_iter() { + if let Some(original) = self.recipes.get(recipe.name.lexeme()) { + if settings.allow_duplicate_recipes { + self.recipes.remove(recipe.name.lexeme()); + } else { + return Err(recipe.name.token().error(DuplicateRecipe { + recipe: original.name(), + first: original.line_number(), + })); + } + } + self.recipes.insert(recipe); + } + let recipes = RecipeResolver::resolve_recipes(self.recipes, &assignments)?; for recipe in recipes.values() { @@ -91,12 +121,6 @@ impl<'src> Analyzer<'src> { } fn analyze_recipe(&self, recipe: &UnresolvedRecipe<'src>) -> CompileResult<'src, ()> { - if let Some(original) = self.recipes.get(recipe.name.lexeme()) { - return Err(recipe.name.token().error(DuplicateRecipe { - recipe: original.name(), - first: original.line_number(), - })); - } let mut parameters = BTreeSet::new(); let mut passed_default = false; @@ -175,31 +199,6 @@ impl<'src> Analyzer<'src> { Ok(()) } - fn resolve_set(&self, settings: &mut Settings<'src>, set: Set<'src>) -> CompileResult<'src, ()> { - match set.value { - Setting::AllowDuplicateRecipes(allow_duplicate_recipes) => { - settings.allow_duplicate_recipes = allow_duplicate_recipes; - } - Setting::DotenvLoad(dotenv_load) => { - settings.dotenv_load = Some(dotenv_load); - } - Setting::Export(export) => { - settings.export = export; - } - Setting::PositionalArguments(positional_arguments) => { - settings.positional_arguments = positional_arguments; - } - Setting::Shell(shell) => { - assert!(settings.shell.is_none()); - settings.shell = Some(shell); - } - Setting::WindowsPowerShell(windows_powershell) => { - settings.windows_powershell = windows_powershell; - } - }; - Ok(()) - } - fn resolve_alias( recipes: &Table<'src, Rc>>, alias: Alias<'src, Name<'src>>, diff --git a/tests/misc.rs b/tests/misc.rs index c707454d8c..1e4099ebb9 100644 --- a/tests/misc.rs +++ b/tests/misc.rs @@ -1221,6 +1221,14 @@ test! { status: EXIT_FAILURE, } +test! { + name: allow_duplicate_recipes, + justfile: "b:\n echo foo\nb:\n echo bar\nset allow-duplicate-recipes", + args: ("b"), + stdout: "bar\n", + stderr: "echo bar\n", +} + test! { name: duplicate_variable, justfile: "a := 'hello'\na := 'hello'\nfoo:", From 52f621ab7fcb1dbb935bf35a567950808d8406f9 Mon Sep 17 00:00:00 2001 From: Greg Lutostanski Date: Thu, 10 Feb 2022 05:04:04 -0600 Subject: [PATCH 05/13] Add Allow Duplicate Recipes section to the README --- README.md | 34 +++++++++++++++++++++++++++------- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 04e208d805..7653c1ad81 100644 --- a/README.md +++ b/README.md @@ -519,13 +519,14 @@ foo: #### Table of Settings -| Name | Value | Description | -| ---------------------- | ------------------ | -------------------------------------------------------------- | -| `dotenv-load` | boolean | Load a `.env` file, if present. | -| `export` | boolean | Export all variables as environment variables. | -| `positional-arguments` | boolean | Pass positional arguments. | -| `shell` | `[COMMAND, ARGS…]` | Set the command used to invoke recipes and evaluate backticks. | -| `windows-powershell` | boolean | Use PowerShell on Windows as default shell. | +| Name | Value | Description | +| ------------------------- | ------------------ | -------------------------------------------------------------- | +| `allow-duplicate-recipes` | boolean | Allow duplicate recipes (last definition is used). | +| `dotenv-load` | boolean | Load a `.env` file, if present. | +| `export` | boolean | Export all variables as environment variables. | +| `positional-arguments` | boolean | Pass positional arguments. | +| `shell` | `[COMMAND, ARGS…]` | Set the command used to invoke recipes and evaluate backticks. | +| `windows-powershell` | boolean | Use PowerShell on Windows as default shell. | Boolean settings can be written as: @@ -539,6 +540,25 @@ Which is equivalent to: set NAME := true ``` +#### Allow Duplicate Recipes + +If `allow-duplicate-recipes` is `true`, recipes can be redefined, (the last definition will be used). Defaults to `false`. + +```make +set allow-duplicate-recipes + +@foo: + echo foo + +@foo: + echo bar +``` + +```sh +$ just foo +bar +``` + #### Dotenv Load If `dotenv-load` is `true`, a `.env` file will be loaded if present. Defaults to `true`. From 073f009509a34a7203a8766726ca454df4490a69 Mon Sep 17 00:00:00 2001 From: Greg Lutostanski Date: Thu, 10 Feb 2022 05:09:15 -0600 Subject: [PATCH 06/13] small code cleanup --- src/analyzer.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/analyzer.rs b/src/analyzer.rs index 0da6a9b24a..cb66524707 100644 --- a/src/analyzer.rs +++ b/src/analyzer.rs @@ -70,7 +70,7 @@ impl<'src> Analyzer<'src> { AssignmentResolver::resolve_assignments(&assignments)?; - for recipe in recipes.into_iter() { + for recipe in recipes { if let Some(original) = self.recipes.get(recipe.name.lexeme()) { if settings.allow_duplicate_recipes { self.recipes.remove(recipe.name.lexeme()); @@ -316,7 +316,7 @@ mod tests { width: 1, kind: DuplicateRecipe{recipe: "a", first: 0}, } - + analysis_error! { name: duplicate_variable, input: "a := \"0\"\na := \"0\"", From 3e543425626bca75824e8806604f3ebfbdf72385 Mon Sep 17 00:00:00 2001 From: Greg Lutostanski Date: Thu, 10 Feb 2022 05:11:57 -0600 Subject: [PATCH 07/13] cleanup README wording --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 7653c1ad81..da52251878 100644 --- a/README.md +++ b/README.md @@ -542,7 +542,7 @@ set NAME := true #### Allow Duplicate Recipes -If `allow-duplicate-recipes` is `true`, recipes can be redefined, (the last definition will be used). Defaults to `false`. +If `allow-duplicate-recipes` is `true`, recipes can be redefined and the last definition of duplicate recipes will be used. Defaults to `false`. ```make set allow-duplicate-recipes From d5b24dd45a20770bf2d2914a81935aaf18b99ef2 Mon Sep 17 00:00:00 2001 From: Greg Lutostanski Date: Thu, 10 Feb 2022 07:30:49 -0600 Subject: [PATCH 08/13] remove unneeded type annotation --- src/analyzer.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/analyzer.rs b/src/analyzer.rs index cb66524707..0f7bb70be7 100644 --- a/src/analyzer.rs +++ b/src/analyzer.rs @@ -16,7 +16,7 @@ impl<'src> Analyzer<'src> { } pub(crate) fn justfile(mut self, ast: Ast<'src>) -> CompileResult<'src, Justfile<'src>> { - let mut recipes: Vec> = Vec::new(); + let mut recipes = Vec::new(); for item in ast.items { match item { From b6b0a811735c2ed53c329fafff2089c2dbf1218d Mon Sep 17 00:00:00 2001 From: Greg Lutostanski Date: Thu, 10 Feb 2022 20:47:53 -0600 Subject: [PATCH 09/13] Apply suggestions from code review Co-authored-by: Casey Rodarmor --- README.md | 4 ++-- src/analyzer.rs | 5 +---- tests/json.rs | 2 +- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index da52251878..1e14c68457 100644 --- a/README.md +++ b/README.md @@ -521,7 +521,7 @@ foo: | Name | Value | Description | | ------------------------- | ------------------ | -------------------------------------------------------------- | -| `allow-duplicate-recipes` | boolean | Allow duplicate recipes (last definition is used). | +| `allow-duplicate-recipes` | boolean | Allow recipes appearing later in a `justfile` to override earlier recipes with the same name. | | `dotenv-load` | boolean | Load a `.env` file, if present. | | `export` | boolean | Export all variables as environment variables. | | `positional-arguments` | boolean | Pass positional arguments. | @@ -542,7 +542,7 @@ set NAME := true #### Allow Duplicate Recipes -If `allow-duplicate-recipes` is `true`, recipes can be redefined and the last definition of duplicate recipes will be used. Defaults to `false`. +If `allow-duplicate-recipes` is `true`, defining multiple recipes with the same name is not an error, and the last definition is used. Defaults to `false`. ```make set allow-duplicate-recipes diff --git a/src/analyzer.rs b/src/analyzer.rs index 0f7bb70be7..415bb5b1be 100644 --- a/src/analyzer.rs +++ b/src/analyzer.rs @@ -72,9 +72,7 @@ impl<'src> Analyzer<'src> { for recipe in recipes { if let Some(original) = self.recipes.get(recipe.name.lexeme()) { - if settings.allow_duplicate_recipes { - self.recipes.remove(recipe.name.lexeme()); - } else { + if !settings.allow_duplicate_recipes { return Err(recipe.name.token().error(DuplicateRecipe { recipe: original.name(), first: original.line_number(), @@ -121,7 +119,6 @@ impl<'src> Analyzer<'src> { } fn analyze_recipe(&self, recipe: &UnresolvedRecipe<'src>) -> CompileResult<'src, ()> { - let mut parameters = BTreeSet::new(); let mut passed_default = false; diff --git a/tests/json.rs b/tests/json.rs index 30f3254ec0..6cad5bf1ec 100644 --- a/tests/json.rs +++ b/tests/json.rs @@ -267,7 +267,7 @@ fn duplicate_recipes() { alias f := foo foo: - foo: + foo bar: ", json!({ "first": "foo", From f56ed93fab618390005fdc8be2be0c24b6f50d93 Mon Sep 17 00:00:00 2001 From: Greg Lutostanski Date: Thu, 10 Feb 2022 21:07:48 -0600 Subject: [PATCH 10/13] cleanup and address last comments --- README.md | 14 ++++++------ tests/allow_duplicate_recipes.rs | 38 ++++++++++++++++++++++++++++++++ tests/json.rs | 9 +++++++- tests/lib.rs | 1 + tests/misc.rs | 8 ------- 5 files changed, 54 insertions(+), 16 deletions(-) create mode 100644 tests/allow_duplicate_recipes.rs diff --git a/README.md b/README.md index 1e14c68457..9b3e32eee7 100644 --- a/README.md +++ b/README.md @@ -519,14 +519,14 @@ foo: #### Table of Settings -| Name | Value | Description | -| ------------------------- | ------------------ | -------------------------------------------------------------- | +| Name | Value | Description | +| ------------------------- | ------------------ | --------------------------------------------------------------------------------------------- | | `allow-duplicate-recipes` | boolean | Allow recipes appearing later in a `justfile` to override earlier recipes with the same name. | -| `dotenv-load` | boolean | Load a `.env` file, if present. | -| `export` | boolean | Export all variables as environment variables. | -| `positional-arguments` | boolean | Pass positional arguments. | -| `shell` | `[COMMAND, ARGS…]` | Set the command used to invoke recipes and evaluate backticks. | -| `windows-powershell` | boolean | Use PowerShell on Windows as default shell. | +| `dotenv-load` | boolean | Load a `.env` file, if present. | +| `export` | boolean | Export all variables as environment variables. | +| `positional-arguments` | boolean | Pass positional arguments. | +| `shell` | `[COMMAND, ARGS…]` | Set the command used to invoke recipes and evaluate backticks. | +| `windows-powershell` | boolean | Use PowerShell on Windows as default shell. | Boolean settings can be written as: diff --git a/tests/allow_duplicate_recipes.rs b/tests/allow_duplicate_recipes.rs new file mode 100644 index 0000000000..d597ce0c61 --- /dev/null +++ b/tests/allow_duplicate_recipes.rs @@ -0,0 +1,38 @@ +use crate::common::*; + +#[test] +fn allow_duplicate_recipes() { + Test::new() + .justfile( + " + b: + echo foo + b: + echo bar + + set allow-duplicate-recipes + ", + ) + .stdout("bar\n") + .stderr("echo bar\n") + .run(); +} + +#[test] +fn allow_duplicate_recipes_with_args() { + Test::new() + .justfile( + " + b a: + echo foo + b c d: + echo bar {{c}} {{d}} + + set allow-duplicate-recipes + ", + ) + .args(&["b", "one", "two"]) + .stdout("bar one two\n") + .stderr("echo bar one two\n") + .run(); +} diff --git a/tests/json.rs b/tests/json.rs index 6cad5bf1ec..345ca2ae96 100644 --- a/tests/json.rs +++ b/tests/json.rs @@ -284,7 +284,14 @@ fn duplicate_recipes() { "dependencies": [], "doc": null, "name": "foo", - "parameters": [], + "parameters": [ + { + "name": "bar", + "export": false, + "default": null, + "kind": "singular", + }, + ], "priors": 0, "private": false, "quiet": false, diff --git a/tests/lib.rs b/tests/lib.rs index 7492e41877..873941576a 100644 --- a/tests/lib.rs +++ b/tests/lib.rs @@ -1,6 +1,7 @@ #[macro_use] mod test; +mod allow_duplicate_recipes; mod assert_stdout; mod assert_success; mod byte_order_mark; diff --git a/tests/misc.rs b/tests/misc.rs index 1e4099ebb9..c707454d8c 100644 --- a/tests/misc.rs +++ b/tests/misc.rs @@ -1221,14 +1221,6 @@ test! { status: EXIT_FAILURE, } -test! { - name: allow_duplicate_recipes, - justfile: "b:\n echo foo\nb:\n echo bar\nset allow-duplicate-recipes", - args: ("b"), - stdout: "bar\n", - stderr: "echo bar\n", -} - test! { name: duplicate_variable, justfile: "a := 'hello'\na := 'hello'\nfoo:", From 6ceaf7966723fbedfd36ef0cc2bc8234e9787845 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Mon, 14 Feb 2022 18:25:12 -0800 Subject: [PATCH 11/13] Update README.md --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 9de95bec80..e9c7ecd96f 100644 --- a/README.md +++ b/README.md @@ -544,7 +544,7 @@ set NAME := true #### Allow Duplicate Recipes -If `allow-duplicate-recipes` is `true`, defining multiple recipes with the same name is not an error, and the last definition is used. Defaults to `false`. +If `allow-duplicate-recipes` is set to `true`, defining multiple recipes with the same name is not an error and the last definition is used. Defaults to `false`. ```make set allow-duplicate-recipes From b491ac8d3197c7c75ebbf7c52b7d225a0447b144 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Mon, 14 Feb 2022 18:31:06 -0800 Subject: [PATCH 12/13] placate clippy --- src/analyzer.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/analyzer.rs b/src/analyzer.rs index 415bb5b1be..8ded49b7f9 100644 --- a/src/analyzer.rs +++ b/src/analyzer.rs @@ -30,7 +30,7 @@ impl<'src> Analyzer<'src> { } Item::Comment(_) => (), Item::Recipe(recipe) => { - self.analyze_recipe(&recipe)?; + Self::analyze_recipe(&recipe)?; recipes.push(recipe); } Item::Set(set) => { @@ -118,7 +118,7 @@ impl<'src> Analyzer<'src> { }) } - fn analyze_recipe(&self, recipe: &UnresolvedRecipe<'src>) -> CompileResult<'src, ()> { + fn analyze_recipe(recipe: &UnresolvedRecipe<'src>) -> CompileResult<'src, ()> { let mut parameters = BTreeSet::new(); let mut passed_default = false; From d5fb67cbd04a4874163ce04022c6e620d7273675 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Mon, 14 Feb 2022 18:34:17 -0800 Subject: [PATCH 13/13] Format --- src/node.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/node.rs b/src/node.rs index 255876f3a9..adba40203d 100644 --- a/src/node.rs +++ b/src/node.rs @@ -221,7 +221,11 @@ impl<'src> Node<'src> for Set<'src> { set.push_mut(self.name.lexeme().replace('-', "_")); match &self.value { - AllowDuplicateRecipes(value) | DotenvLoad(value) | Export(value) | PositionalArguments(value) | WindowsPowerShell(value) => { + AllowDuplicateRecipes(value) + | DotenvLoad(value) + | Export(value) + | PositionalArguments(value) + | WindowsPowerShell(value) => { set.push_mut(value.to_string()); } Shell(setting::Shell { command, arguments }) => {