Repository: casey/just
Pull request #1480 (2023-01-06 22:15:22)

Allow recipe parameters to shadow variables
Recipe parameters cannot have the same name as bindings in the environment.

This has been striking me as an unnecessarily conservative restriction, and counter to the way that most programming languages work, so this PR removes it.

I'm interested in feedback before merging.

@neunenak Thoughts?

neunenak: I don't see any issues. In fact, I think this might've made a justfile that I used at my previous job that was trying to provide a default env var value in the justfile but also allow it to be overridden on the command line to be slightly more concise.
casey: Thanks for the feedback! I'm going to let this marinate for a week and then probably merge it.

Patch:
diff --git a/src/analyzer.rs b/src/analyzer.rs
index 53247bb4..c8ee53ee 100644
--- a/src/analyzer.rs
+++ b/src/analyzer.rs
@@ -97,16 +97,6 @@ impl<'src> Analyzer<'src> {
 
     let recipes = RecipeResolver::resolve_recipes(self.recipes, &assignments)?;
 
-    for recipe in recipes.values() {
-      for parameter in &recipe.parameters {
-        if assignments.contains_key(parameter.name.lexeme()) {
-          return Err(parameter.name.token().error(ParameterShadowsVariable {
-            parameter: parameter.name.lexeme(),
-          }));
-        }
-      }
-    }
-
     let mut aliases = Table::new();
     while let Some(alias) = self.aliases.pop() {
       aliases.insert(Self::resolve_alias(&recipes, alias)?);
@@ -316,16 +306,6 @@ mod tests {
     kind:   DuplicateParameter{recipe: "a", parameter: "b"},
   }
 
-  analysis_error! {
-    name:   parameter_shadows_variable,
-    input:  "foo := \"h\"\na foo:",
-    offset:  13,
-    line:   1,
-    column: 2,
-    width:  3,
-    kind:   ParameterShadowsVariable{parameter: "foo"},
-  }
-
   analysis_error! {
     name:   duplicate_recipe,
     input:  "a:\nb:\na:",
diff --git a/src/compile_error.rs b/src/compile_error.rs
index c7dcbdf4..5a90b523 100644
--- a/src/compile_error.rs
+++ b/src/compile_error.rs
@@ -226,12 +226,6 @@ impl Display for CompileError<'_> {
       ParameterFollowsVariadicParameter { parameter } => {
         write!(f, "Parameter `{parameter}` follows variadic parameter")?;
       }
-      ParameterShadowsVariable { parameter } => {
-        write!(
-          f,
-          "Parameter `{parameter}` shadows variable of the same name",
-        )?;
-      }
       ParsingRecursionDepthExceeded => {
         write!(f, "Parsing recursion depth exceeded")?;
       }
diff --git a/src/compile_error_kind.rs b/src/compile_error_kind.rs
index 8c1fcdfc..acd8b2f7 100644
--- a/src/compile_error_kind.rs
+++ b/src/compile_error_kind.rs
@@ -79,9 +79,6 @@ pub(crate) enum CompileErrorKind<'src> {
   ParameterFollowsVariadicParameter {
     parameter: &'src str,
   },
-  ParameterShadowsVariable {
-    parameter: &'src str,
-  },
   ParsingRecursionDepthExceeded,
   RequiredParameterFollowsDefaultParameter {
     parameter: &'src str,


Test patch:
diff --git a/tests/lib.rs b/tests/lib.rs
index ce246688..cceacc86 100644
--- a/tests/lib.rs
+++ b/tests/lib.rs
@@ -72,6 +72,7 @@ mod recursion_limit;
 mod regexes;
 mod run;
 mod search;
+mod shadowing_parameters;
 mod shebang;
 mod shell;
 mod show;
diff --git a/tests/misc.rs b/tests/misc.rs
index 4a903bca..6c2fb935 100644
--- a/tests/misc.rs
+++ b/tests/misc.rs
@@ -1151,19 +1151,6 @@ c: b a
   stdout:   "",
 }
 
-test! {
-  name:     parameter_shadows_variable,
-  justfile: "FOO := 'hello'\na FOO:",
-  args:     ("a"),
-  stdout:   "",
-  stderr:   "error: Parameter `FOO` shadows variable of the same name
-  |
-2 | a FOO:
-  |   ^^^
-",
-  status:   EXIT_FAILURE,
-}
-
 test! {
   name:     unknown_function_in_assignment,
   justfile: r#"foo := foo() + "hello"
diff --git a/tests/shadowing_parameters.rs b/tests/shadowing_parameters.rs
new file mode 100644
index 00000000..37789967
--- /dev/null
+++ b/tests/shadowing_parameters.rs
@@ -0,0 +1,23 @@
+test! {
+  name:     parameter_may_shadow_variable,
+  justfile: "FOO := 'hello'\na FOO:\n echo {{FOO}}\n",
+  args:     ("a", "bar"),
+  stdout:   "bar\n",
+  stderr:   "echo bar\n",
+}
+
+test! {
+  name:     shadowing_parameters_do_not_change_environment,
+  justfile: "export FOO := 'hello'\na FOO:\n echo $FOO\n",
+  args:     ("a", "bar"),
+  stdout:   "hello\n",
+  stderr:   "echo $FOO\n",
+}
+
+test! {
+  name:     exporting_shadowing_parameters_does_change_environment,
+  justfile: "export FOO := 'hello'\na $FOO:\n echo $FOO\n",
+  args:     ("a", "bar"),
+  stdout:   "bar\n",
+  stderr:   "echo $FOO\n",
+}