From 586b9ab906db83b624963622bef81dd5449d51b6 Mon Sep 17 00:00:00 2001 From: Adam Snaider Date: Sat, 3 Aug 2024 21:32:05 -0400 Subject: [PATCH 1/8] Add a private field to assignments --- src/binding.rs | 8 +++++++ src/evaluator.rs | 16 ++++++++++--- src/parser.rs | 34 +++++++++++++++++++++++++--- src/scope.rs | 4 +++- tests/json.rs | 59 ++++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 114 insertions(+), 7 deletions(-) diff --git a/src/binding.rs b/src/binding.rs index 6a8c0c70cd..e28d14cee9 100644 --- a/src/binding.rs +++ b/src/binding.rs @@ -9,10 +9,18 @@ pub(crate) struct Binding<'src, V = String> { pub(crate) export: bool, /// Binding name pub(crate) name: Name<'src>, + /// Whether this binding is private to the script + pub(crate) private: bool, /// Binding value pub(crate) value: V, } +impl Binding<'_, V> { + pub fn is_public(&self) -> bool { + !self.private + } +} + impl<'src, V> Keyed<'src> for Binding<'src, V> { fn key(&self) -> &'src str { self.name.lexeme() diff --git a/src/evaluator.rs b/src/evaluator.rs index 9efe807dc6..3cb324a91d 100644 --- a/src/evaluator.rs +++ b/src/evaluator.rs @@ -32,7 +32,12 @@ impl<'src, 'run> Evaluator<'src, 'run> { for (name, value) in overrides { if let Some(assignment) = module.assignments.get(name) { - scope.bind(assignment.export, assignment.name, value.clone()); + scope.bind( + assignment.export, + assignment.is_private(), + assignment.name, + value.clone(), + ); } else { unknown_overrides.push(name.clone()); } @@ -63,7 +68,12 @@ impl<'src, 'run> Evaluator<'src, 'run> { if !self.scope.bound(name) { let value = self.evaluate_expression(&assignment.value)?; - self.scope.bind(assignment.export, assignment.name, value); + self.scope.bind( + assignment.export, + assignment.is_private(), + assignment.name, + value, + ); } Ok(self.scope.value(name).unwrap()) @@ -321,7 +331,7 @@ impl<'src, 'run> Evaluator<'src, 'run> { }; evaluator .scope - .bind(parameter.export, parameter.name, value); + .bind(parameter.export, false, parameter.name, value); } Ok((evaluator.scope, positional)) diff --git a/src/parser.rs b/src/parser.rs index adc95048e7..0e69e70b01 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -343,7 +343,9 @@ impl<'run, 'src> Parser<'run, 'src> { } Some(Keyword::Export) if self.next_are(&[Identifier, Identifier, ColonEquals]) => { self.presume_keyword(Keyword::Export)?; - items.push(Item::Assignment(self.parse_assignment(true)?)); + items.push(Item::Assignment( + self.parse_assignment(true, take_attributes())?, + )); } Some(Keyword::Unexport) if self.next_are(&[Identifier, Identifier, Eof]) @@ -412,7 +414,9 @@ impl<'run, 'src> Parser<'run, 'src> { } _ => { if self.next_are(&[Identifier, ColonEquals]) { - items.push(Item::Assignment(self.parse_assignment(false)?)); + items.push(Item::Assignment( + self.parse_assignment(false, take_attributes())?, + )); } else { let doc = pop_doc_comment(&mut items, eol_since_last_comment); items.push(Item::Recipe(self.parse_recipe( @@ -473,8 +477,13 @@ impl<'run, 'src> Parser<'run, 'src> { } /// Parse an assignment, e.g. `foo := bar` - fn parse_assignment(&mut self, export: bool) -> CompileResult<'src, Assignment<'src>> { + fn parse_assignment( + &mut self, + export: bool, + attributes: BTreeSet>, + ) -> CompileResult<'src, Assignment<'src>> { let name = self.parse_name()?; + let private = name.lexeme().starts_with('_') || attributes.contains(&Attribute::Private); self.presume_any(&[Equals, ColonEquals])?; let value = self.parse_expression()?; self.expect_eol()?; @@ -483,6 +492,7 @@ impl<'run, 'src> Parser<'run, 'src> { export, name, value, + private, }) } @@ -1237,6 +1247,15 @@ mod tests { tree: (justfile (assignment #export x "hello")), } + test! { + name: private_export, + text: r#" + [private] + export x := "hello" + "#, + tree: (justfile (assignment #export x "hello")), + } + test! { name: export_equals, text: r#"export x := "hello""#, @@ -1251,6 +1270,15 @@ mod tests { tree: (justfile (assignment x "hello")), } + test! { + name: private_assignment, + text: r#" + [private] + x := "hello" + "#, + tree: (justfile (assignment x "hello")), + } + test! { name: assignment_equals, text: r#"x := "hello""#, diff --git a/src/scope.rs b/src/scope.rs index dd7888c1ba..14bd4d2f32 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -22,6 +22,7 @@ impl<'src, 'run> Scope<'src, 'run> { for (key, value) in constants() { root.bind( + false, false, Name { token: Token { @@ -41,12 +42,13 @@ impl<'src, 'run> Scope<'src, 'run> { root } - pub(crate) fn bind(&mut self, export: bool, name: Name<'src>, value: String) { + pub(crate) fn bind(&mut self, export: bool, private: bool, name: Name<'src>, value: String) { self.bindings.insert(Binding { depth: 0, export, name, value, + private, }); } diff --git a/tests/json.rs b/tests/json.rs index 961bdfa4ff..aa09dd87ad 100644 --- a/tests/json.rs +++ b/tests/json.rs @@ -82,6 +82,7 @@ fn assignment() { "name": "foo", "value": "bar", "depth": 0, + "private": false, } }, "first": null, @@ -114,6 +115,62 @@ fn assignment() { ); } +#[test] +fn private_assignment() { + case( + " + _foo := 'foo' + [private] + bar := 'bar' + ", + json!({ + "aliases": {}, + "assignments": { + "_foo": { + "export": false, + "name": "_foo", + "value": "foo", + "depth": 0, + "private": true, + }, + "bar": { + "export": false, + "name": "bar", + "value": "bar", + "depth": 0, + "private": true, + }, + }, + "first": null, + "doc": null, + "groups": [], + "modules": {}, + "recipes": {}, + "settings": { + "allow_duplicate_recipes": false, + "allow_duplicate_variables": false, + "dotenv_filename": null, + "dotenv_load": false, + "dotenv_path": null, + "dotenv_required": false, + "export": false, + "fallback": false, + "ignore_comments": false, + "positional_arguments": false, + "quiet": false, + "shell": null, + "tempdir" : null, + "unstable": false, + "windows_powershell": false, + "windows_shell": null, + "working_directory" : null, + }, + "unexports": [], + "warnings": [], + }), + ); +} + #[test] fn body() { case( @@ -272,6 +329,7 @@ fn dependency_argument() { "name": "x", "value": "foo", "depth": 0, + "private": false, }, }, "groups": [], @@ -436,6 +494,7 @@ fn duplicate_variables() { "name": "x", "value": "bar", "depth": 0, + "private": false, } }, "first": null, From 8e09beb55c4d9ed171dc117d3f60261ff1ef0fbb Mon Sep 17 00:00:00 2001 From: Adam Snaider Date: Sat, 3 Aug 2024 22:19:40 -0400 Subject: [PATCH 2/8] Error when overriding private variables --- src/evaluator.rs | 15 +++++------ src/subcommand.rs | 11 +++++--- tests/evaluate.rs | 26 ++++++++++++++++++ tests/private.rs | 67 +++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 108 insertions(+), 11 deletions(-) diff --git a/src/evaluator.rs b/src/evaluator.rs index 3cb324a91d..09c9aaa2cb 100644 --- a/src/evaluator.rs +++ b/src/evaluator.rs @@ -32,12 +32,11 @@ impl<'src, 'run> Evaluator<'src, 'run> { for (name, value) in overrides { if let Some(assignment) = module.assignments.get(name) { - scope.bind( - assignment.export, - assignment.is_private(), - assignment.name, - value.clone(), - ); + if assignment.is_public() { + scope.bind(assignment.export, false, assignment.name, value.clone()); + } else { + unknown_overrides.push(name.clone()); + } } else { unknown_overrides.push(name.clone()); } @@ -70,7 +69,7 @@ impl<'src, 'run> Evaluator<'src, 'run> { let value = self.evaluate_expression(&assignment.value)?; self.scope.bind( assignment.export, - assignment.is_private(), + assignment.private, assignment.name, value, ); @@ -331,7 +330,7 @@ impl<'src, 'run> Evaluator<'src, 'run> { }; evaluator .scope - .bind(parameter.export, false, parameter.name, value); + .bind(parameter.export, true, parameter.name, value); } Ok((evaluator.scope, positional)) diff --git a/src/subcommand.rs b/src/subcommand.rs index 3f2c1ce90c..cf1eaf1dc2 100644 --- a/src/subcommand.rs +++ b/src/subcommand.rs @@ -84,7 +84,7 @@ impl Subcommand { List { path } => Self::list(config, justfile, path)?, Show { path } => Self::show(config, justfile, path)?, Summary => Self::summary(config, justfile), - Variables => Self::variables(justfile), + Variables => Self::public_variables(justfile), Changelog | Completions { .. } | Edit | Init | Man | Run { .. } => unreachable!(), } @@ -713,8 +713,13 @@ impl Subcommand { } } - fn variables(justfile: &Justfile) { - for (i, (_, assignment)) in justfile.assignments.iter().enumerate() { + fn public_variables(justfile: &Justfile) { + for (i, (_, assignment)) in justfile + .assignments + .iter() + .filter(|(_, binding)| binding.is_public()) + .enumerate() + { if i > 0 { print!(" "); } diff --git a/tests/evaluate.rs b/tests/evaluate.rs index 9c38b344a9..b74c6b356b 100644 --- a/tests/evaluate.rs +++ b/tests/evaluate.rs @@ -77,3 +77,29 @@ test! { ", status: EXIT_FAILURE, } + +test! { + name: evaluate_private, + justfile: " + [private] + foo := 'one' + bar := 'two' + _baz := 'three' + ", + args: ("--evaluate"), + stdout: "_baz := \"three\"\nbar := \"two\"\nfoo := \"one\"\n", + status: EXIT_SUCCESS, +} + +test! { + name: evaluate_single_private, + justfile: " + [private] + foo := 'one' + bar := 'two' + _baz := 'three' + ", + args: ("--evaluate", "foo"), + stdout: "one", + status: EXIT_SUCCESS, +} diff --git a/tests/private.rs b/tests/private.rs index 9ab0cd82ce..cdad65a8a6 100644 --- a/tests/private.rs +++ b/tests/private.rs @@ -38,3 +38,70 @@ fn private_attribute_for_alias() { ) .run(); } + +#[test] +fn private_attribute_for_assignment() { + Test::new() + .justfile( + " + [private] + foo := 'one' + bar := 'two' + _baz := 'three' + ", + ) + .args(["--variables"]) + .stdout("bar\n") + .run(); +} + +test! { + name: no_private_overrides, + justfile: " + [private] + foo := 'one' + bar := 'two' + _baz := 'three' + + default: + @echo hello + ", + args: ("foo=two"), + stdout: "", + stderr: "error: Variable `foo` overridden on the command line but not present in justfile\n", + status: EXIT_FAILURE, +} + +test! { + name: no_private_implicit_overrides, + justfile: " + [private] + foo := 'one' + bar := 'two' + _baz := 'three' + + default: + @echo hello + ", + args: ("_baz=two"), + stdout: "", + stderr: "error: Variable `_baz` overridden on the command line but not present in justfile\n", + status: EXIT_FAILURE, +} + +test! { + name: allowed_public_overrides, + justfile: " + [private] + foo := 'one' + bar := 'two' + _baz := 'three' + + default: + @echo hello + ", + args: ("bar=two"), + stdout: "hello\n", + stderr: "", + status: EXIT_SUCCESS, +} From 8795773dee592ab8de6b081745cb1d3ab4f216c1 Mon Sep 17 00:00:00 2001 From: Adam Snaider Date: Sat, 3 Aug 2024 23:26:40 -0400 Subject: [PATCH 3/8] Add setting to opt in to private variables --- src/evaluator.rs | 17 ++++++++++++++-- src/justfile.rs | 11 ++++++++++- src/keyword.rs | 1 + src/node.rs | 1 + src/parser.rs | 3 +++ src/setting.rs | 2 ++ src/settings.rs | 4 ++++ src/subcommand.rs | 3 ++- tests/json.rs | 22 +++++++++++++++++++++ tests/private.rs | 50 ++++++++++++++++++++++++++++++++++++++--------- 10 files changed, 101 insertions(+), 13 deletions(-) diff --git a/src/evaluator.rs b/src/evaluator.rs index 09c9aaa2cb..caf7e21110 100644 --- a/src/evaluator.rs +++ b/src/evaluator.rs @@ -7,6 +7,13 @@ pub(crate) struct Evaluator<'src: 'run, 'run> { pub(crate) scope: Scope<'src, 'run>, } +fn assignment_can_override( + assignment: &Binding<'_, Expression<'_>>, + settings: &Settings<'_>, +) -> bool { + !settings.allow_private_variables || assignment.is_public() +} + impl<'src, 'run> Evaluator<'src, 'run> { pub(crate) fn evaluate_assignments( config: &'run Config, @@ -15,6 +22,7 @@ impl<'src, 'run> Evaluator<'src, 'run> { overrides: &BTreeMap, parent: &'run Scope<'src, 'run>, search: &'run Search, + settings: &'run Settings<'src>, ) -> RunResult<'src, Scope<'src, 'run>> where 'src: 'run, @@ -32,8 +40,13 @@ impl<'src, 'run> Evaluator<'src, 'run> { for (name, value) in overrides { if let Some(assignment) = module.assignments.get(name) { - if assignment.is_public() { - scope.bind(assignment.export, false, assignment.name, value.clone()); + if assignment_can_override(assignment, settings) { + scope.bind( + assignment.export, + assignment.private, + assignment.name, + value.clone(), + ); } else { unknown_overrides.push(name.clone()); } diff --git a/src/justfile.rs b/src/justfile.rs index b31084ef18..ae770ccabf 100644 --- a/src/justfile.rs +++ b/src/justfile.rs @@ -110,7 +110,15 @@ impl<'src> Justfile<'src> { let root = Scope::root(); - let scope = Evaluator::evaluate_assignments(config, &dotenv, self, overrides, &root, search)?; + let scope = Evaluator::evaluate_assignments( + config, + &dotenv, + self, + overrides, + &root, + search, + &self.settings, + )?; match &config.subcommand { Subcommand::Command { @@ -284,6 +292,7 @@ impl<'src> Justfile<'src> { &BTreeMap::new(), parent, search, + &self.settings, )?; let scope = arena.alloc(scope); scopes.insert(path, scope); diff --git a/src/keyword.rs b/src/keyword.rs index ff06c39d27..8091ee192d 100644 --- a/src/keyword.rs +++ b/src/keyword.rs @@ -6,6 +6,7 @@ pub(crate) enum Keyword { Alias, AllowDuplicateRecipes, AllowDuplicateVariables, + AllowPrivateVariables, Assert, DotenvFilename, DotenvLoad, diff --git a/src/node.rs b/src/node.rs index 3ccf862d57..ca73e48e48 100644 --- a/src/node.rs +++ b/src/node.rs @@ -288,6 +288,7 @@ impl<'src> Node<'src> for Set<'src> { match &self.value { Setting::AllowDuplicateRecipes(value) | Setting::AllowDuplicateVariables(value) + | Setting::AllowPrivateVariables(value) | Setting::DotenvLoad(value) | Setting::DotenvRequired(value) | Setting::Export(value) diff --git a/src/parser.rs b/src/parser.rs index 0e69e70b01..e0e8d577ce 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -948,6 +948,9 @@ impl<'run, 'src> Parser<'run, 'src> { Keyword::AllowDuplicateVariables => { Some(Setting::AllowDuplicateVariables(self.parse_set_bool()?)) } + Keyword::AllowPrivateVariables => { + Some(Setting::AllowPrivateVariables(self.parse_set_bool()?)) + } Keyword::DotenvLoad => Some(Setting::DotenvLoad(self.parse_set_bool()?)), Keyword::DotenvRequired => Some(Setting::DotenvRequired(self.parse_set_bool()?)), Keyword::Export => Some(Setting::Export(self.parse_set_bool()?)), diff --git a/src/setting.rs b/src/setting.rs index 2b68713580..ecc9fceefe 100644 --- a/src/setting.rs +++ b/src/setting.rs @@ -4,6 +4,7 @@ use super::*; pub(crate) enum Setting<'src> { AllowDuplicateRecipes(bool), AllowDuplicateVariables(bool), + AllowPrivateVariables(bool), DotenvFilename(StringLiteral<'src>), DotenvLoad(bool), DotenvPath(StringLiteral<'src>), @@ -27,6 +28,7 @@ impl<'src> Display for Setting<'src> { match self { Self::AllowDuplicateRecipes(value) | Self::AllowDuplicateVariables(value) + | Self::AllowPrivateVariables(value) | Self::DotenvLoad(value) | Self::DotenvRequired(value) | Self::Export(value) diff --git a/src/settings.rs b/src/settings.rs index 795ddebd1f..443c6e7bc6 100644 --- a/src/settings.rs +++ b/src/settings.rs @@ -9,6 +9,7 @@ pub(crate) const WINDOWS_POWERSHELL_ARGS: &[&str] = &["-NoLogo", "-Command"]; pub(crate) struct Settings<'src> { pub(crate) allow_duplicate_recipes: bool, pub(crate) allow_duplicate_variables: bool, + pub(crate) allow_private_variables: bool, pub(crate) dotenv_filename: Option, pub(crate) dotenv_load: bool, pub(crate) dotenv_path: Option, @@ -40,6 +41,9 @@ impl<'src> Settings<'src> { Setting::AllowDuplicateVariables(allow_duplicate_variables) => { settings.allow_duplicate_variables = allow_duplicate_variables; } + Setting::AllowPrivateVariables(allow_private_variables) => { + settings.allow_private_variables = allow_private_variables; + } Setting::DotenvFilename(filename) => { settings.dotenv_filename = Some(filename.cooked); } diff --git a/src/subcommand.rs b/src/subcommand.rs index cf1eaf1dc2..5756c8faf6 100644 --- a/src/subcommand.rs +++ b/src/subcommand.rs @@ -714,10 +714,11 @@ impl Subcommand { } fn public_variables(justfile: &Justfile) { + let filter_private = justfile.settings.allow_private_variables; for (i, (_, assignment)) in justfile .assignments .iter() - .filter(|(_, binding)| binding.is_public()) + .filter(|(_, binding)| !filter_private || binding.is_public()) .enumerate() { if i > 0 { diff --git a/tests/json.rs b/tests/json.rs index aa09dd87ad..ab3f187537 100644 --- a/tests/json.rs +++ b/tests/json.rs @@ -47,6 +47,7 @@ fn alias() { "settings": { "allow_duplicate_recipes": false, "allow_duplicate_variables": false, + "allow_private_variables": false, "dotenv_filename": null, "dotenv_load": false, "dotenv_path": null, @@ -93,6 +94,7 @@ fn assignment() { "settings": { "allow_duplicate_recipes": false, "allow_duplicate_variables": false, + "allow_private_variables": false, "dotenv_filename": null, "dotenv_load": false, "dotenv_path": null, @@ -149,6 +151,7 @@ fn private_assignment() { "settings": { "allow_duplicate_recipes": false, "allow_duplicate_variables": false, + "allow_private_variables": false, "dotenv_filename": null, "dotenv_load": false, "dotenv_path": null, @@ -207,6 +210,7 @@ fn body() { "settings": { "allow_duplicate_recipes": false, "allow_duplicate_variables": false, + "allow_private_variables": false, "dotenv_filename": null, "dotenv_load": false, "dotenv_path": null, @@ -277,6 +281,7 @@ fn dependencies() { "settings": { "allow_duplicate_recipes": false, "allow_duplicate_variables": false, + "allow_private_variables": false, "dotenv_filename": null, "dotenv_load": false, "dotenv_path": null, @@ -386,6 +391,7 @@ fn dependency_argument() { "settings": { "allow_duplicate_recipes": false, "allow_duplicate_variables": false, + "allow_private_variables": false, "dotenv_filename": null, "dotenv_load": false, "dotenv_path": null, @@ -456,6 +462,7 @@ fn duplicate_recipes() { "settings": { "allow_duplicate_recipes": true, "allow_duplicate_variables": false, + "allow_private_variables": false, "dotenv_filename": null, "dotenv_load": false, "dotenv_path": null, @@ -505,6 +512,7 @@ fn duplicate_variables() { "settings": { "allow_duplicate_recipes": false, "allow_duplicate_variables": true, + "allow_private_variables": false, "dotenv_filename": null, "dotenv_load": false, "dotenv_path": null, @@ -556,6 +564,7 @@ fn doc_comment() { "settings": { "allow_duplicate_recipes": false, "allow_duplicate_variables": false, + "allow_private_variables": false, "dotenv_filename": null, "dotenv_load": false, "dotenv_path": null, @@ -593,6 +602,7 @@ fn empty_justfile() { "settings": { "allow_duplicate_recipes": false, "allow_duplicate_variables": false, + "allow_private_variables": false, "dotenv_filename": null, "dotenv_load": false, "dotenv_path": null, @@ -751,6 +761,7 @@ fn parameters() { "settings": { "allow_duplicate_recipes": false, "allow_duplicate_variables": false, + "allow_private_variables": false, "dotenv_filename": null, "dotenv_load": false, "dotenv_path": null, @@ -842,6 +853,7 @@ fn priors() { "settings": { "allow_duplicate_recipes": false, "allow_duplicate_variables": false, + "allow_private_variables": false, "dotenv_filename": null, "dotenv_load": false, "dotenv_path": null, @@ -893,6 +905,7 @@ fn private() { "settings": { "allow_duplicate_recipes": false, "allow_duplicate_variables": false, + "allow_private_variables": false, "dotenv_filename": null, "dotenv_load": false, "dotenv_path": null, @@ -944,6 +957,7 @@ fn quiet() { "settings": { "allow_duplicate_recipes": false, "allow_duplicate_variables": false, + "allow_private_variables": false, "dotenv_filename": null, "dotenv_load": false, "dotenv_path": null, @@ -1007,6 +1021,7 @@ fn settings() { "settings": { "allow_duplicate_recipes": false, "allow_duplicate_variables": false, + "allow_private_variables": false, "dotenv_filename": "filename", "dotenv_load": true, "dotenv_path": "path", @@ -1064,6 +1079,7 @@ fn shebang() { "settings": { "allow_duplicate_recipes": false, "allow_duplicate_variables": false, + "allow_private_variables": false, "dotenv_filename": null, "dotenv_load": false, "dotenv_path": null, @@ -1115,6 +1131,7 @@ fn simple() { "settings": { "allow_duplicate_recipes": false, "allow_duplicate_variables": false, + "allow_private_variables": false, "dotenv_filename": null, "dotenv_load": false, "dotenv_path": null, @@ -1169,6 +1186,7 @@ fn attribute() { "settings": { "allow_duplicate_recipes": false, "allow_duplicate_variables": false, + "allow_private_variables": false, "dotenv_filename": null, "dotenv_load": false, "dotenv_path": null, @@ -1238,6 +1256,7 @@ fn module() { "settings": { "allow_duplicate_recipes": false, "allow_duplicate_variables": false, + "allow_private_variables": false, "dotenv_filename": null, "dotenv_load": false, "dotenv_path": null, @@ -1262,6 +1281,7 @@ fn module() { "settings": { "allow_duplicate_recipes": false, "allow_duplicate_variables": false, + "allow_private_variables": false, "dotenv_filename": null, "dotenv_load": false, "dotenv_path": null, @@ -1333,6 +1353,7 @@ fn module_group() { "settings": { "allow_duplicate_recipes": false, "allow_duplicate_variables": false, + "allow_private_variables": false, "dotenv_filename": null, "dotenv_load": false, "dotenv_path": null, @@ -1357,6 +1378,7 @@ fn module_group() { "settings": { "allow_duplicate_recipes": false, "allow_duplicate_variables": false, + "allow_private_variables": false, "dotenv_filename": null, "dotenv_load": false, "dotenv_path": null, diff --git a/tests/private.rs b/tests/private.rs index cdad65a8a6..00fd6d8f7d 100644 --- a/tests/private.rs +++ b/tests/private.rs @@ -39,25 +39,36 @@ fn private_attribute_for_alias() { .run(); } -#[test] -fn private_attribute_for_assignment() { - Test::new() - .justfile( - " +test! { + name: dont_list_private_variables, + justfile: " + set allow-private-variables [private] foo := 'one' bar := 'two' _baz := 'three' ", - ) - .args(["--variables"]) - .stdout("bar\n") - .run(); + args: ("--variables"), + stdout: "bar\n", +} + +test! { + name: list_private_variables_if_not_opted_in, + justfile: " + [private] + foo := 'one' + bar := 'two' + _baz := 'three' + ", + args: ("--variables"), + stdout: "_baz bar foo\n", } test! { name: no_private_overrides, justfile: " + set allow-private-variables + [private] foo := 'one' bar := 'two' @@ -75,6 +86,8 @@ test! { test! { name: no_private_implicit_overrides, justfile: " + set allow-private-variables + [private] foo := 'one' bar := 'two' @@ -92,6 +105,8 @@ test! { test! { name: allowed_public_overrides, justfile: " + set allow-private-variables + [private] foo := 'one' bar := 'two' @@ -105,3 +120,20 @@ test! { stderr: "", status: EXIT_SUCCESS, } + +test! { + name: ignore_private_without_setting, + justfile: " + [private] + foo := 'one' + bar := 'two' + _baz := 'three' + + default: + @echo hello + ", + args: ("foo=two"), + stdout: "hello\n", + stderr: "", + status: EXIT_SUCCESS, +} From e591af7c3ed2401ca50f2f3b43c3e9cf7e7c7e7c Mon Sep 17 00:00:00 2001 From: Adam Snaider Date: Sun, 4 Aug 2024 21:40:25 -0400 Subject: [PATCH 4/8] Allow overrides on private assignments --- src/evaluator.rs | 24 ++++++------------------ src/justfile.rs | 11 +---------- tests/private.rs | 18 ++++++++---------- 3 files changed, 15 insertions(+), 38 deletions(-) diff --git a/src/evaluator.rs b/src/evaluator.rs index caf7e21110..db2f0a7063 100644 --- a/src/evaluator.rs +++ b/src/evaluator.rs @@ -7,13 +7,6 @@ pub(crate) struct Evaluator<'src: 'run, 'run> { pub(crate) scope: Scope<'src, 'run>, } -fn assignment_can_override( - assignment: &Binding<'_, Expression<'_>>, - settings: &Settings<'_>, -) -> bool { - !settings.allow_private_variables || assignment.is_public() -} - impl<'src, 'run> Evaluator<'src, 'run> { pub(crate) fn evaluate_assignments( config: &'run Config, @@ -22,7 +15,6 @@ impl<'src, 'run> Evaluator<'src, 'run> { overrides: &BTreeMap, parent: &'run Scope<'src, 'run>, search: &'run Search, - settings: &'run Settings<'src>, ) -> RunResult<'src, Scope<'src, 'run>> where 'src: 'run, @@ -40,16 +32,12 @@ impl<'src, 'run> Evaluator<'src, 'run> { for (name, value) in overrides { if let Some(assignment) = module.assignments.get(name) { - if assignment_can_override(assignment, settings) { - scope.bind( - assignment.export, - assignment.private, - assignment.name, - value.clone(), - ); - } else { - unknown_overrides.push(name.clone()); - } + scope.bind( + assignment.export, + assignment.private, + assignment.name, + value.clone(), + ); } else { unknown_overrides.push(name.clone()); } diff --git a/src/justfile.rs b/src/justfile.rs index ae770ccabf..b31084ef18 100644 --- a/src/justfile.rs +++ b/src/justfile.rs @@ -110,15 +110,7 @@ impl<'src> Justfile<'src> { let root = Scope::root(); - let scope = Evaluator::evaluate_assignments( - config, - &dotenv, - self, - overrides, - &root, - search, - &self.settings, - )?; + let scope = Evaluator::evaluate_assignments(config, &dotenv, self, overrides, &root, search)?; match &config.subcommand { Subcommand::Command { @@ -292,7 +284,6 @@ impl<'src> Justfile<'src> { &BTreeMap::new(), parent, search, - &self.settings, )?; let scope = arena.alloc(scope); scopes.insert(path, scope); diff --git a/tests/private.rs b/tests/private.rs index 00fd6d8f7d..2126868a83 100644 --- a/tests/private.rs +++ b/tests/private.rs @@ -65,7 +65,7 @@ test! { } test! { - name: no_private_overrides, + name: allows_private_overrides, justfile: " set allow-private-variables @@ -75,16 +75,15 @@ test! { _baz := 'three' default: - @echo hello + @echo {{foo}} ", args: ("foo=two"), - stdout: "", - stderr: "error: Variable `foo` overridden on the command line but not present in justfile\n", - status: EXIT_FAILURE, + stdout: "two\n", + status: EXIT_SUCCESS, } test! { - name: no_private_implicit_overrides, + name: allows_implicit_private_overrides, justfile: " set allow-private-variables @@ -94,12 +93,11 @@ test! { _baz := 'three' default: - @echo hello + @echo {{_baz}} ", args: ("_baz=two"), - stdout: "", - stderr: "error: Variable `_baz` overridden on the command line but not present in justfile\n", - status: EXIT_FAILURE, + stdout: "two\n", + status: EXIT_SUCCESS, } test! { From 710f95a9b15c311958ad160c0bfc1de5d4744892 Mon Sep 17 00:00:00 2001 From: Adam Snaider Date: Sun, 4 Aug 2024 21:47:34 -0400 Subject: [PATCH 5/8] Hide private variables from evaluate --- src/justfile.rs | 14 ++++++++------ tests/evaluate.rs | 17 +++++++++++++++++ 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/src/justfile.rs b/src/justfile.rs index b31084ef18..0f1b779836 100644 --- a/src/justfile.rs +++ b/src/justfile.rs @@ -164,12 +164,14 @@ impl<'src> Justfile<'src> { let width = scope.names().fold(0, |max, name| name.len().max(max)); for binding in scope.bindings() { - println!( - "{0:1$} := \"{2}\"", - binding.name.lexeme(), - width, - binding.value - ); + if !self.settings.allow_private_variables || binding.is_public() { + println!( + "{0:1$} := \"{2}\"", + binding.name.lexeme(), + width, + binding.value + ); + } } } diff --git a/tests/evaluate.rs b/tests/evaluate.rs index b74c6b356b..e7cf2eb822 100644 --- a/tests/evaluate.rs +++ b/tests/evaluate.rs @@ -80,6 +80,21 @@ test! { test! { name: evaluate_private, + justfile: " + set allow-private-variables + + [private] + foo := 'one' + bar := 'two' + _baz := 'three' + ", + args: ("--evaluate"), + stdout: "bar := \"two\"\n", + status: EXIT_SUCCESS, +} + +test! { + name: evaluate_private_not_enabled, justfile: " [private] foo := 'one' @@ -94,6 +109,8 @@ test! { test! { name: evaluate_single_private, justfile: " + set allow-private-variables + [private] foo := 'one' bar := 'two' From 7b2f886d04b3c08412069d2b90b07425896ed249 Mon Sep 17 00:00:00 2001 From: Adam Snaider Date: Sat, 10 Aug 2024 13:37:56 -0400 Subject: [PATCH 6/8] Revert "Add setting to opt in to private variables" This reverts commit 4f2128f282216f4eb6ee94c0c351b02471328642. --- src/justfile.rs | 2 +- src/keyword.rs | 1 - src/node.rs | 1 - src/parser.rs | 3 --- src/setting.rs | 2 -- src/settings.rs | 4 ---- src/subcommand.rs | 3 +-- tests/evaluate.rs | 17 ----------------- tests/json.rs | 22 ---------------------- tests/private.rs | 41 +++-------------------------------------- 10 files changed, 5 insertions(+), 91 deletions(-) diff --git a/src/justfile.rs b/src/justfile.rs index 0f1b779836..c3b97acb9c 100644 --- a/src/justfile.rs +++ b/src/justfile.rs @@ -164,7 +164,7 @@ impl<'src> Justfile<'src> { let width = scope.names().fold(0, |max, name| name.len().max(max)); for binding in scope.bindings() { - if !self.settings.allow_private_variables || binding.is_public() { + if binding.is_public() { println!( "{0:1$} := \"{2}\"", binding.name.lexeme(), diff --git a/src/keyword.rs b/src/keyword.rs index 8091ee192d..ff06c39d27 100644 --- a/src/keyword.rs +++ b/src/keyword.rs @@ -6,7 +6,6 @@ pub(crate) enum Keyword { Alias, AllowDuplicateRecipes, AllowDuplicateVariables, - AllowPrivateVariables, Assert, DotenvFilename, DotenvLoad, diff --git a/src/node.rs b/src/node.rs index ca73e48e48..3ccf862d57 100644 --- a/src/node.rs +++ b/src/node.rs @@ -288,7 +288,6 @@ impl<'src> Node<'src> for Set<'src> { match &self.value { Setting::AllowDuplicateRecipes(value) | Setting::AllowDuplicateVariables(value) - | Setting::AllowPrivateVariables(value) | Setting::DotenvLoad(value) | Setting::DotenvRequired(value) | Setting::Export(value) diff --git a/src/parser.rs b/src/parser.rs index e0e8d577ce..0e69e70b01 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -948,9 +948,6 @@ impl<'run, 'src> Parser<'run, 'src> { Keyword::AllowDuplicateVariables => { Some(Setting::AllowDuplicateVariables(self.parse_set_bool()?)) } - Keyword::AllowPrivateVariables => { - Some(Setting::AllowPrivateVariables(self.parse_set_bool()?)) - } Keyword::DotenvLoad => Some(Setting::DotenvLoad(self.parse_set_bool()?)), Keyword::DotenvRequired => Some(Setting::DotenvRequired(self.parse_set_bool()?)), Keyword::Export => Some(Setting::Export(self.parse_set_bool()?)), diff --git a/src/setting.rs b/src/setting.rs index ecc9fceefe..2b68713580 100644 --- a/src/setting.rs +++ b/src/setting.rs @@ -4,7 +4,6 @@ use super::*; pub(crate) enum Setting<'src> { AllowDuplicateRecipes(bool), AllowDuplicateVariables(bool), - AllowPrivateVariables(bool), DotenvFilename(StringLiteral<'src>), DotenvLoad(bool), DotenvPath(StringLiteral<'src>), @@ -28,7 +27,6 @@ impl<'src> Display for Setting<'src> { match self { Self::AllowDuplicateRecipes(value) | Self::AllowDuplicateVariables(value) - | Self::AllowPrivateVariables(value) | Self::DotenvLoad(value) | Self::DotenvRequired(value) | Self::Export(value) diff --git a/src/settings.rs b/src/settings.rs index 443c6e7bc6..795ddebd1f 100644 --- a/src/settings.rs +++ b/src/settings.rs @@ -9,7 +9,6 @@ pub(crate) const WINDOWS_POWERSHELL_ARGS: &[&str] = &["-NoLogo", "-Command"]; pub(crate) struct Settings<'src> { pub(crate) allow_duplicate_recipes: bool, pub(crate) allow_duplicate_variables: bool, - pub(crate) allow_private_variables: bool, pub(crate) dotenv_filename: Option, pub(crate) dotenv_load: bool, pub(crate) dotenv_path: Option, @@ -41,9 +40,6 @@ impl<'src> Settings<'src> { Setting::AllowDuplicateVariables(allow_duplicate_variables) => { settings.allow_duplicate_variables = allow_duplicate_variables; } - Setting::AllowPrivateVariables(allow_private_variables) => { - settings.allow_private_variables = allow_private_variables; - } Setting::DotenvFilename(filename) => { settings.dotenv_filename = Some(filename.cooked); } diff --git a/src/subcommand.rs b/src/subcommand.rs index 5756c8faf6..cf1eaf1dc2 100644 --- a/src/subcommand.rs +++ b/src/subcommand.rs @@ -714,11 +714,10 @@ impl Subcommand { } fn public_variables(justfile: &Justfile) { - let filter_private = justfile.settings.allow_private_variables; for (i, (_, assignment)) in justfile .assignments .iter() - .filter(|(_, binding)| !filter_private || binding.is_public()) + .filter(|(_, binding)| binding.is_public()) .enumerate() { if i > 0 { diff --git a/tests/evaluate.rs b/tests/evaluate.rs index e7cf2eb822..110184c837 100644 --- a/tests/evaluate.rs +++ b/tests/evaluate.rs @@ -81,8 +81,6 @@ test! { test! { name: evaluate_private, justfile: " - set allow-private-variables - [private] foo := 'one' bar := 'two' @@ -93,24 +91,9 @@ test! { status: EXIT_SUCCESS, } -test! { - name: evaluate_private_not_enabled, - justfile: " - [private] - foo := 'one' - bar := 'two' - _baz := 'three' - ", - args: ("--evaluate"), - stdout: "_baz := \"three\"\nbar := \"two\"\nfoo := \"one\"\n", - status: EXIT_SUCCESS, -} - test! { name: evaluate_single_private, justfile: " - set allow-private-variables - [private] foo := 'one' bar := 'two' diff --git a/tests/json.rs b/tests/json.rs index ab3f187537..aa09dd87ad 100644 --- a/tests/json.rs +++ b/tests/json.rs @@ -47,7 +47,6 @@ fn alias() { "settings": { "allow_duplicate_recipes": false, "allow_duplicate_variables": false, - "allow_private_variables": false, "dotenv_filename": null, "dotenv_load": false, "dotenv_path": null, @@ -94,7 +93,6 @@ fn assignment() { "settings": { "allow_duplicate_recipes": false, "allow_duplicate_variables": false, - "allow_private_variables": false, "dotenv_filename": null, "dotenv_load": false, "dotenv_path": null, @@ -151,7 +149,6 @@ fn private_assignment() { "settings": { "allow_duplicate_recipes": false, "allow_duplicate_variables": false, - "allow_private_variables": false, "dotenv_filename": null, "dotenv_load": false, "dotenv_path": null, @@ -210,7 +207,6 @@ fn body() { "settings": { "allow_duplicate_recipes": false, "allow_duplicate_variables": false, - "allow_private_variables": false, "dotenv_filename": null, "dotenv_load": false, "dotenv_path": null, @@ -281,7 +277,6 @@ fn dependencies() { "settings": { "allow_duplicate_recipes": false, "allow_duplicate_variables": false, - "allow_private_variables": false, "dotenv_filename": null, "dotenv_load": false, "dotenv_path": null, @@ -391,7 +386,6 @@ fn dependency_argument() { "settings": { "allow_duplicate_recipes": false, "allow_duplicate_variables": false, - "allow_private_variables": false, "dotenv_filename": null, "dotenv_load": false, "dotenv_path": null, @@ -462,7 +456,6 @@ fn duplicate_recipes() { "settings": { "allow_duplicate_recipes": true, "allow_duplicate_variables": false, - "allow_private_variables": false, "dotenv_filename": null, "dotenv_load": false, "dotenv_path": null, @@ -512,7 +505,6 @@ fn duplicate_variables() { "settings": { "allow_duplicate_recipes": false, "allow_duplicate_variables": true, - "allow_private_variables": false, "dotenv_filename": null, "dotenv_load": false, "dotenv_path": null, @@ -564,7 +556,6 @@ fn doc_comment() { "settings": { "allow_duplicate_recipes": false, "allow_duplicate_variables": false, - "allow_private_variables": false, "dotenv_filename": null, "dotenv_load": false, "dotenv_path": null, @@ -602,7 +593,6 @@ fn empty_justfile() { "settings": { "allow_duplicate_recipes": false, "allow_duplicate_variables": false, - "allow_private_variables": false, "dotenv_filename": null, "dotenv_load": false, "dotenv_path": null, @@ -761,7 +751,6 @@ fn parameters() { "settings": { "allow_duplicate_recipes": false, "allow_duplicate_variables": false, - "allow_private_variables": false, "dotenv_filename": null, "dotenv_load": false, "dotenv_path": null, @@ -853,7 +842,6 @@ fn priors() { "settings": { "allow_duplicate_recipes": false, "allow_duplicate_variables": false, - "allow_private_variables": false, "dotenv_filename": null, "dotenv_load": false, "dotenv_path": null, @@ -905,7 +893,6 @@ fn private() { "settings": { "allow_duplicate_recipes": false, "allow_duplicate_variables": false, - "allow_private_variables": false, "dotenv_filename": null, "dotenv_load": false, "dotenv_path": null, @@ -957,7 +944,6 @@ fn quiet() { "settings": { "allow_duplicate_recipes": false, "allow_duplicate_variables": false, - "allow_private_variables": false, "dotenv_filename": null, "dotenv_load": false, "dotenv_path": null, @@ -1021,7 +1007,6 @@ fn settings() { "settings": { "allow_duplicate_recipes": false, "allow_duplicate_variables": false, - "allow_private_variables": false, "dotenv_filename": "filename", "dotenv_load": true, "dotenv_path": "path", @@ -1079,7 +1064,6 @@ fn shebang() { "settings": { "allow_duplicate_recipes": false, "allow_duplicate_variables": false, - "allow_private_variables": false, "dotenv_filename": null, "dotenv_load": false, "dotenv_path": null, @@ -1131,7 +1115,6 @@ fn simple() { "settings": { "allow_duplicate_recipes": false, "allow_duplicate_variables": false, - "allow_private_variables": false, "dotenv_filename": null, "dotenv_load": false, "dotenv_path": null, @@ -1186,7 +1169,6 @@ fn attribute() { "settings": { "allow_duplicate_recipes": false, "allow_duplicate_variables": false, - "allow_private_variables": false, "dotenv_filename": null, "dotenv_load": false, "dotenv_path": null, @@ -1256,7 +1238,6 @@ fn module() { "settings": { "allow_duplicate_recipes": false, "allow_duplicate_variables": false, - "allow_private_variables": false, "dotenv_filename": null, "dotenv_load": false, "dotenv_path": null, @@ -1281,7 +1262,6 @@ fn module() { "settings": { "allow_duplicate_recipes": false, "allow_duplicate_variables": false, - "allow_private_variables": false, "dotenv_filename": null, "dotenv_load": false, "dotenv_path": null, @@ -1353,7 +1333,6 @@ fn module_group() { "settings": { "allow_duplicate_recipes": false, "allow_duplicate_variables": false, - "allow_private_variables": false, "dotenv_filename": null, "dotenv_load": false, "dotenv_path": null, @@ -1378,7 +1357,6 @@ fn module_group() { "settings": { "allow_duplicate_recipes": false, "allow_duplicate_variables": false, - "allow_private_variables": false, "dotenv_filename": null, "dotenv_load": false, "dotenv_path": null, diff --git a/tests/private.rs b/tests/private.rs index 2126868a83..96762b8050 100644 --- a/tests/private.rs +++ b/tests/private.rs @@ -42,33 +42,19 @@ fn private_attribute_for_alias() { test! { name: dont_list_private_variables, justfile: " - set allow-private-variables [private] foo := 'one' bar := 'two' _baz := 'three' ", - args: ("--variables"), - stdout: "bar\n", -} - -test! { - name: list_private_variables_if_not_opted_in, - justfile: " - [private] - foo := 'one' - bar := 'two' - _baz := 'three' - ", - args: ("--variables"), - stdout: "_baz bar foo\n", + args: ("--variables"), + stdout: "bar\n", + status: EXIT_SUCCESS, } test! { name: allows_private_overrides, justfile: " - set allow-private-variables - [private] foo := 'one' bar := 'two' @@ -85,8 +71,6 @@ test! { test! { name: allows_implicit_private_overrides, justfile: " - set allow-private-variables - [private] foo := 'one' bar := 'two' @@ -103,8 +87,6 @@ test! { test! { name: allowed_public_overrides, justfile: " - set allow-private-variables - [private] foo := 'one' bar := 'two' @@ -118,20 +100,3 @@ test! { stderr: "", status: EXIT_SUCCESS, } - -test! { - name: ignore_private_without_setting, - justfile: " - [private] - foo := 'one' - bar := 'two' - _baz := 'three' - - default: - @echo hello - ", - args: ("foo=two"), - stdout: "hello\n", - stderr: "", - status: EXIT_SUCCESS, -} From 9da3db251794324dbe7bb6a8350d6a6d62ae881f Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Sat, 10 Aug 2024 11:18:37 -0700 Subject: [PATCH 7/8] Reform --- src/analyzer.rs | 8 +++--- src/binding.rs | 14 ++--------- src/evaluator.rs | 6 ++--- src/justfile.rs | 2 +- src/parser.rs | 19 +++++++------- src/scope.rs | 8 +++--- src/subcommand.rs | 2 +- tests/json.rs | 5 ---- tests/private.rs | 64 +++++++---------------------------------------- 9 files changed, 32 insertions(+), 96 deletions(-) diff --git a/src/analyzer.rs b/src/analyzer.rs index 65bd382c31..de023c41ba 100644 --- a/src/analyzer.rs +++ b/src/analyzer.rs @@ -159,11 +159,9 @@ impl<'src> Analyzer<'src> { return Err(assignment.name.token.error(DuplicateVariable { variable })); } - if self - .assignments - .get(variable) - .map_or(true, |original| assignment.depth <= original.depth) - { + if self.assignments.get(variable).map_or(true, |original| { + assignment.file_depth <= original.file_depth + }) { self.assignments.insert(assignment.clone()); } diff --git a/src/binding.rs b/src/binding.rs index e28d14cee9..7e7890c7a1 100644 --- a/src/binding.rs +++ b/src/binding.rs @@ -3,24 +3,14 @@ use super::*; /// A binding of `name` to `value` #[derive(Debug, Clone, PartialEq, Serialize)] pub(crate) struct Binding<'src, V = String> { - /// Module depth where binding appears - pub(crate) depth: u32, - /// Export binding as an environment variable to child processes pub(crate) export: bool, - /// Binding name + #[serde(skip)] + pub(crate) file_depth: u32, pub(crate) name: Name<'src>, - /// Whether this binding is private to the script pub(crate) private: bool, - /// Binding value pub(crate) value: V, } -impl Binding<'_, V> { - pub fn is_public(&self) -> bool { - !self.private - } -} - impl<'src, V> Keyed<'src> for Binding<'src, V> { fn key(&self) -> &'src str { self.name.lexeme() diff --git a/src/evaluator.rs b/src/evaluator.rs index db2f0a7063..4ed00036d1 100644 --- a/src/evaluator.rs +++ b/src/evaluator.rs @@ -34,8 +34,8 @@ impl<'src, 'run> Evaluator<'src, 'run> { if let Some(assignment) = module.assignments.get(name) { scope.bind( assignment.export, - assignment.private, assignment.name, + assignment.private, value.clone(), ); } else { @@ -70,8 +70,8 @@ impl<'src, 'run> Evaluator<'src, 'run> { let value = self.evaluate_expression(&assignment.value)?; self.scope.bind( assignment.export, - assignment.private, assignment.name, + assignment.private, value, ); } @@ -331,7 +331,7 @@ impl<'src, 'run> Evaluator<'src, 'run> { }; evaluator .scope - .bind(parameter.export, true, parameter.name, value); + .bind(parameter.export, parameter.name, false, value); } Ok((evaluator.scope, positional)) diff --git a/src/justfile.rs b/src/justfile.rs index c3b97acb9c..ab5fc6dbb8 100644 --- a/src/justfile.rs +++ b/src/justfile.rs @@ -164,7 +164,7 @@ impl<'src> Justfile<'src> { let width = scope.names().fold(0, |max, name| name.len().max(max)); for binding in scope.bindings() { - if binding.is_public() { + if !binding.private { println!( "{0:1$} := \"{2}\"", binding.name.lexeme(), diff --git a/src/parser.rs b/src/parser.rs index 0e69e70b01..b896cd0b13 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -483,16 +483,15 @@ impl<'run, 'src> Parser<'run, 'src> { attributes: BTreeSet>, ) -> CompileResult<'src, Assignment<'src>> { let name = self.parse_name()?; - let private = name.lexeme().starts_with('_') || attributes.contains(&Attribute::Private); - self.presume_any(&[Equals, ColonEquals])?; + self.presume(ColonEquals)?; let value = self.parse_expression()?; self.expect_eol()?; Ok(Assignment { - depth: self.file_depth, + file_depth: self.file_depth, export, name, value, - private, + private: name.lexeme().starts_with('_') || attributes.contains(&Attribute::Private), }) } @@ -1249,10 +1248,10 @@ mod tests { test! { name: private_export, - text: r#" + text: " [private] - export x := "hello" - "#, + export x := 'hello' + ", tree: (justfile (assignment #export x "hello")), } @@ -1272,10 +1271,10 @@ mod tests { test! { name: private_assignment, - text: r#" + text: " [private] - x := "hello" - "#, + x := 'hello' + ", tree: (justfile (assignment x "hello")), } diff --git a/src/scope.rs b/src/scope.rs index 14bd4d2f32..36bbedc8aa 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -22,7 +22,6 @@ impl<'src, 'run> Scope<'src, 'run> { for (key, value) in constants() { root.bind( - false, false, Name { token: Token { @@ -35,6 +34,7 @@ impl<'src, 'run> Scope<'src, 'run> { src: key, }, }, + false, (*value).into(), ); } @@ -42,13 +42,13 @@ impl<'src, 'run> Scope<'src, 'run> { root } - pub(crate) fn bind(&mut self, export: bool, private: bool, name: Name<'src>, value: String) { + pub(crate) fn bind(&mut self, export: bool, name: Name<'src>, private: bool, value: String) { self.bindings.insert(Binding { - depth: 0, export, + file_depth: 0, name, - value, private, + value, }); } diff --git a/src/subcommand.rs b/src/subcommand.rs index cf1eaf1dc2..cdc32fd823 100644 --- a/src/subcommand.rs +++ b/src/subcommand.rs @@ -717,7 +717,7 @@ impl Subcommand { for (i, (_, assignment)) in justfile .assignments .iter() - .filter(|(_, binding)| binding.is_public()) + .filter(|(_, binding)| !binding.private) .enumerate() { if i > 0 { diff --git a/tests/json.rs b/tests/json.rs index aa09dd87ad..bf962a1e3f 100644 --- a/tests/json.rs +++ b/tests/json.rs @@ -81,7 +81,6 @@ fn assignment() { "export": false, "name": "foo", "value": "bar", - "depth": 0, "private": false, } }, @@ -130,14 +129,12 @@ fn private_assignment() { "export": false, "name": "_foo", "value": "foo", - "depth": 0, "private": true, }, "bar": { "export": false, "name": "bar", "value": "bar", - "depth": 0, "private": true, }, }, @@ -328,7 +325,6 @@ fn dependency_argument() { "export": false, "name": "x", "value": "foo", - "depth": 0, "private": false, }, }, @@ -493,7 +489,6 @@ fn duplicate_variables() { "export": false, "name": "x", "value": "bar", - "depth": 0, "private": false, } }, diff --git a/tests/private.rs b/tests/private.rs index 96762b8050..73d4636c59 100644 --- a/tests/private.rs +++ b/tests/private.rs @@ -39,64 +39,18 @@ fn private_attribute_for_alias() { .run(); } -test! { - name: dont_list_private_variables, - justfile: " +#[test] +fn private_variables_are_not_listed() { + Test::new() + .justfile( + " [private] foo := 'one' bar := 'two' _baz := 'three' ", - args: ("--variables"), - stdout: "bar\n", - status: EXIT_SUCCESS, -} - -test! { - name: allows_private_overrides, - justfile: " - [private] - foo := 'one' - bar := 'two' - _baz := 'three' - - default: - @echo {{foo}} - ", - args: ("foo=two"), - stdout: "two\n", - status: EXIT_SUCCESS, -} - -test! { - name: allows_implicit_private_overrides, - justfile: " - [private] - foo := 'one' - bar := 'two' - _baz := 'three' - - default: - @echo {{_baz}} - ", - args: ("_baz=two"), - stdout: "two\n", - status: EXIT_SUCCESS, -} - -test! { - name: allowed_public_overrides, - justfile: " - [private] - foo := 'one' - bar := 'two' - _baz := 'three' - - default: - @echo hello - ", - args: ("bar=two"), - stdout: "hello\n", - stderr: "", - status: EXIT_SUCCESS, + ) + .args(["--variables"]) + .stdout("bar\n") + .run(); } From b828f64d735bc235444e90600bfcab6b34c33236 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Sat, 10 Aug 2024 11:19:44 -0700 Subject: [PATCH 8/8] Reform --- src/subcommand.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/subcommand.rs b/src/subcommand.rs index cdc32fd823..fa6f1e0b7e 100644 --- a/src/subcommand.rs +++ b/src/subcommand.rs @@ -84,7 +84,7 @@ impl Subcommand { List { path } => Self::list(config, justfile, path)?, Show { path } => Self::show(config, justfile, path)?, Summary => Self::summary(config, justfile), - Variables => Self::public_variables(justfile), + Variables => Self::variables(justfile), Changelog | Completions { .. } | Edit | Init | Man | Run { .. } => unreachable!(), } @@ -713,7 +713,7 @@ impl Subcommand { } } - fn public_variables(justfile: &Justfile) { + fn variables(justfile: &Justfile) { for (i, (_, assignment)) in justfile .assignments .iter()