Add 'allow-duplicate-variables' setting (#1922)

GRAMMAR.md

@@ -62,6 +62,7 @@ assignment    : NAME ':=' expression eol
 export        : 'export' assignment
 
 setting       : 'set' 'allow-duplicate-recipes' boolean?
+              | 'set' 'allow-duplicate-variables' boolean?
               | 'set' 'dotenv-filename' ':=' string
               | 'set' 'dotenv-load' boolean?
               | 'set' 'dotenv-path' ':=' string

README.md

@@ -792,6 +792,7 @@ foo:
 | Name | Value | Default | Description |
 |------|-------|---------|-------------|
 | `allow-duplicate-recipes` | boolean | `false` | Allow recipes appearing later in a `justfile` to override earlier recipes with the same name. |
+| `allow-duplicate-variables` | boolean | `false` | Allow variables appearing later in a `justfile` to override earlier variables with the same name. |
 | `dotenv-filename` | string | - | Load a `.env` file with a custom name, if present. |
 | `dotenv-load` | boolean | `false` | Load a `.env` file, if present. |
 | `dotenv-path` | string | - | Load a `.env` file from a custom path, if present. Overrides `dotenv-filename`. |
@@ -837,6 +838,27 @@ $ just foo
 bar
 ```
 
+#### Allow Duplicate Variables
+
+If `allow-duplicate-variables` is set to `true`, defining multiple variables
+with the same name is not an error and the last definition is used. Defaults to
+`false`.
+
+```just
+set allow-duplicate-variables
+
+a := "foo"
+a := "bar"
+
+@foo:
+  echo $a
+```
+
+```sh
+$ just foo
+bar
+```
+
 #### Dotenv Settings
 
 If `dotenv-load`, `dotenv-filename` or `dotenv-path` is set, `just` will load
@@ -2738,7 +2760,8 @@ Imported files can themselves contain `import`s, which are processed
 recursively.
 
 When `allow-duplicate-recipes` is set, recipes in parent modules override
-recipes in imports.
+recipes in imports. In a similar manner, when `allow-duplicate-variables` is
+set, variables in parent modules override variables in imports.
 
 Imports may be made optional by putting a `?` after the `import` keyword:
 

src/analyzer.rs

@@ -26,6 +26,8 @@ impl<'src> Analyzer<'src> {
   ) -> CompileResult<'src, Justfile<'src>> {
     let mut recipes = Vec::new();
 
+    let mut assignments = Vec::new();
+
     let mut stack = Vec::new();
     stack.push(asts.get(root).unwrap());
 
@@ -70,8 +72,7 @@ impl<'src> Analyzer<'src> {
             self.aliases.insert(alias.clone());
           }
           Item::Assignment(assignment) => {
-            self.analyze_assignment(assignment)?;
-            self.assignments.insert(assignment.clone());
+            assignments.push(assignment);
           }
           Item::Comment(_) => (),
           Item::Import { absolute, .. } => {
@@ -108,6 +109,24 @@ impl<'src> Analyzer<'src> {
 
     let mut recipe_table: Table<'src, UnresolvedRecipe<'src>> = Table::default();
 
+    for assignment in assignments {
+      if !settings.allow_duplicate_variables
+        && self.assignments.contains_key(assignment.name.lexeme())
+      {
+        return Err(assignment.name.token.error(DuplicateVariable {
+          variable: assignment.name.lexeme(),
+        }));
+      }
+
+      if self
+        .assignments
+        .get(assignment.name.lexeme())
+        .map_or(true, |original| assignment.depth <= original.depth)
+      {
+        self.assignments.insert(assignment.clone());
+      }
+    }
+
     AssignmentResolver::resolve_assignments(&self.assignments)?;
 
     for recipe in recipes {
@@ -199,15 +218,6 @@ impl<'src> Analyzer<'src> {
     Ok(())
   }
 
-  fn analyze_assignment(&self, assignment: &Assignment<'src>) -> CompileResult<'src> {
-    if self.assignments.contains_key(assignment.name.lexeme()) {
-      return Err(assignment.name.token.error(DuplicateVariable {
-        variable: assignment.name.lexeme(),
-      }));
-    }
-    Ok(())
-  }
-
   fn analyze_alias(alias: &Alias<'src, Name<'src>>) -> CompileResult<'src> {
     let name = alias.name.lexeme();
 

src/binding.rs

@@ -3,6 +3,8 @@ 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

src/keyword.rs

@@ -5,6 +5,7 @@ use super::*;
 pub(crate) enum Keyword {
   Alias,
   AllowDuplicateRecipes,
+  AllowDuplicateVariables,
   DotenvFilename,
   DotenvLoad,
   DotenvPath,

src/node.rs

@@ -265,6 +265,7 @@ impl<'src> Node<'src> for Set<'src> {
 
     match &self.value {
       Setting::AllowDuplicateRecipes(value)
+      | Setting::AllowDuplicateVariables(value)
       | Setting::DotenvLoad(value)
       | Setting::Export(value)
       | Setting::Fallback(value)

src/parser.rs

@@ -456,6 +456,7 @@ impl<'run, 'src> Parser<'run, 'src> {
     let value = self.parse_expression()?;
     self.expect_eol()?;
     Ok(Assignment {
+      depth: self.submodule_depth,
       export,
       name,
       value,
@@ -847,6 +848,9 @@ impl<'run, 'src> Parser<'run, 'src> {
       Keyword::AllowDuplicateRecipes => {
         Some(Setting::AllowDuplicateRecipes(self.parse_set_bool()?))
       }
+      Keyword::AllowDuplicateVariables => {
+        Some(Setting::AllowDuplicateVariables(self.parse_set_bool()?))
+      }
       Keyword::DotenvLoad => Some(Setting::DotenvLoad(self.parse_set_bool()?)),
       Keyword::Export => Some(Setting::Export(self.parse_set_bool()?)),
       Keyword::Fallback => Some(Setting::Fallback(self.parse_set_bool()?)),
@@ -1913,6 +1917,12 @@ mod tests {
     tree: (justfile (set allow_duplicate_recipes true)),
   }
 
+  test! {
+    name: set_allow_duplicate_variables_implicit,
+    text: "set allow-duplicate-variables",
+    tree: (justfile (set allow_duplicate_variables true)),
+  }
+
   test! {
     name: set_dotenv_load_true,
     text: "set dotenv-load := true",

src/scope.rs

@@ -23,6 +23,7 @@ impl<'src, 'run> Scope<'src, 'run> {
 
   pub(crate) fn bind(&mut self, export: bool, name: Name<'src>, value: String) {
     self.bindings.insert(Binding {
+      depth: 0,
       export,
       name,
       value,

src/setting.rs

@@ -3,6 +3,7 @@ use super::*;
 #[derive(Debug, Clone)]
 pub(crate) enum Setting<'src> {
   AllowDuplicateRecipes(bool),
+  AllowDuplicateVariables(bool),
   DotenvFilename(String),
   DotenvLoad(bool),
   DotenvPath(String),
@@ -21,6 +22,7 @@ impl<'src> Display for Setting<'src> {
   fn fmt(&self, f: &mut Formatter) -> Result<(), fmt::Error> {
     match self {
       Setting::AllowDuplicateRecipes(value)
+      | Setting::AllowDuplicateVariables(value)
       | Setting::DotenvLoad(value)
       | Setting::Export(value)
       | Setting::Fallback(value)

src/settings.rs

@@ -8,6 +8,7 @@ pub(crate) const WINDOWS_POWERSHELL_ARGS: &[&str] = &["-NoLogo", "-Command"];
 #[derive(Debug, PartialEq, Serialize, Default)]
 pub(crate) struct Settings<'src> {
   pub(crate) allow_duplicate_recipes: bool,
+  pub(crate) allow_duplicate_variables: bool,
   pub(crate) dotenv_filename: Option<String>,
   pub(crate) dotenv_load: Option<bool>,
   pub(crate) dotenv_path: Option<PathBuf>,
@@ -31,6 +32,9 @@ impl<'src> Settings<'src> {
         Setting::AllowDuplicateRecipes(allow_duplicate_recipes) => {
           settings.allow_duplicate_recipes = allow_duplicate_recipes;
         }
+        Setting::AllowDuplicateVariables(allow_duplicate_variables) => {
+          settings.allow_duplicate_variables = allow_duplicate_variables;
+        }
         Setting::DotenvFilename(filename) => {
           settings.dotenv_filename = Some(filename);
         }

tests/allow_duplicate_variables.rs

@@ -0,0 +1,21 @@
+use super::*;
+
+#[test]
+fn allow_duplicate_variables() {
+  Test::new()
+    .justfile(
+      "
+      a := 'foo'
+      a := 'bar'
+
+      set allow-duplicate-variables
+
+      b:
+        echo {{a}}
+      ",
+    )
+    .arg("b")
+    .stdout("bar\n")
+    .stderr("echo bar\n")
+    .run();
+}

tests/imports.rs

@@ -182,12 +182,12 @@ fn recipes_in_import_are_overridden_by_recipes_in_parent() {
     })
     .justfile(
       "
+        a:
+          @echo ROOT
+
         import './import.justfile'
 
         set allow-duplicate-recipes
-
-        a:
-          @echo ROOT
       ",
     )
     .test_round_trip(false)
@@ -196,6 +196,32 @@ fn recipes_in_import_are_overridden_by_recipes_in_parent() {
     .run();
 }
 
+#[test]
+fn variables_in_import_are_overridden_by_variables_in_parent() {
+  Test::new()
+    .tree(tree! {
+      "import.justfile": "
+    f := 'foo'
+    ",
+    })
+    .justfile(
+      "
+      f := 'bar'
+
+      import './import.justfile'
+
+      set allow-duplicate-variables
+
+      a:
+        @echo {{f}}
+    ",
+    )
+    .test_round_trip(false)
+    .arg("a")
+    .stdout("bar\n")
+    .run();
+}
+
 #[cfg(not(windows))]
 #[test]
 fn import_paths_beginning_with_tilde_are_expanded_to_homdir() {

tests/json.rs

@@ -44,6 +44,7 @@ fn alias() {
       },
       "settings": {
         "allow_duplicate_recipes": false,
+        "allow_duplicate_variables": false,
         "dotenv_filename": null,
         "dotenv_load": null,
         "dotenv_path": null,
@@ -73,6 +74,7 @@ fn assignment() {
           "export": false,
           "name": "foo",
           "value": "bar",
+          "depth": 0,
         }
       },
       "first": null,
@@ -80,6 +82,7 @@ fn assignment() {
       "recipes": {},
       "settings": {
         "allow_duplicate_recipes": false,
+        "allow_duplicate_variables": false,
         "dotenv_filename": null,
         "dotenv_load": null,
         "dotenv_path": null,
@@ -131,6 +134,7 @@ fn body() {
       },
       "settings": {
         "allow_duplicate_recipes": false,
+        "allow_duplicate_variables": false,
         "dotenv_filename": null,
         "dotenv_load": null,
         "dotenv_path": null,
@@ -194,6 +198,7 @@ fn dependencies() {
       },
       "settings": {
         "allow_duplicate_recipes": false,
+        "allow_duplicate_variables": false,
         "dotenv_filename": null,
         "dotenv_load": null,
         "dotenv_path": null,
@@ -240,6 +245,7 @@ fn dependency_argument() {
           "export": false,
           "name": "x",
           "value": "foo",
+          "depth": 0,
         },
       },
       "modules": {},
@@ -294,6 +300,7 @@ fn dependency_argument() {
       },
       "settings": {
         "allow_duplicate_recipes": false,
+        "allow_duplicate_variables": false,
         "dotenv_filename": null,
         "dotenv_load": null,
         "dotenv_path": null,
@@ -357,6 +364,49 @@ fn duplicate_recipes() {
       },
       "settings": {
         "allow_duplicate_recipes": true,
+        "allow_duplicate_variables": false,
+        "dotenv_filename": null,
+        "dotenv_load": null,
+        "dotenv_path": null,
+        "export": false,
+        "fallback": false,
+        "ignore_comments": false,
+        "positional_arguments": false,
+        "quiet": false,
+        "shell": null,
+        "tempdir" : null,
+        "windows_powershell": false,
+        "windows_shell": null,
+      },
+      "warnings": [],
+    }),
+  );
+}
+
+#[test]
+fn duplicate_variables() {
+  case(
+    "
+      set allow-duplicate-variables
+      x := 'foo'
+      x := 'bar'
+    ",
+    json!({
+      "aliases": {},
+      "assignments": {
+        "x": {
+          "export": false,
+          "name": "x",
+          "value": "bar",
+          "depth": 0,
+        }
+      },
+      "first": null,
+      "modules": {},
+      "recipes": {},
+      "settings": {
+        "allow_duplicate_recipes": false,
+        "allow_duplicate_variables": true,
         "dotenv_filename": null,
         "dotenv_load": null,
         "dotenv_path": null,
@@ -401,6 +451,7 @@ fn doc_comment() {
       },
       "settings": {
         "allow_duplicate_recipes": false,
+        "allow_duplicate_variables": false,
         "dotenv_filename": null,
         "dotenv_load": null,
         "dotenv_path": null,
@@ -431,6 +482,7 @@ fn empty_justfile() {
       "recipes": {},
       "settings": {
         "allow_duplicate_recipes": false,
+        "allow_duplicate_variables": false,
         "dotenv_filename": null,
         "dotenv_load": null,
         "dotenv_path": null,
@@ -582,6 +634,7 @@ fn parameters() {
       },
       "settings": {
         "allow_duplicate_recipes": false,
+        "allow_duplicate_variables": false,
         "dotenv_filename": null,
         "dotenv_load": null,
         "dotenv_path": null,
@@ -666,6 +719,7 @@ fn priors() {
       },
       "settings": {
         "allow_duplicate_recipes": false,
+        "allow_duplicate_variables": false,
         "dotenv_filename": null,
         "dotenv_load": null,
         "dotenv_path": null,
@@ -710,6 +764,7 @@ fn private() {
       },
       "settings": {
         "allow_duplicate_recipes": false,
+        "allow_duplicate_variables": false,
         "dotenv_filename": null,
         "dotenv_load": null,
         "dotenv_path": null,
@@ -754,6 +809,7 @@ fn quiet() {
       },
       "settings": {
         "allow_duplicate_recipes": false,
+        "allow_duplicate_variables": false,
         "dotenv_filename": null,
         "dotenv_load": null,
         "dotenv_path": null,
@@ -810,6 +866,7 @@ fn settings() {
       },
       "settings": {
         "allow_duplicate_recipes": false,
+        "allow_duplicate_variables": false,
         "dotenv_filename": "filename",
         "dotenv_load": true,
         "dotenv_path": "path",
@@ -860,6 +917,7 @@ fn shebang() {
       },
       "settings": {
         "allow_duplicate_recipes": false,
+        "allow_duplicate_variables": false,
         "dotenv_filename": null,
         "dotenv_load": null,
         "dotenv_path": null,
@@ -904,6 +962,7 @@ fn simple() {
       },
       "settings": {
         "allow_duplicate_recipes": false,
+        "allow_duplicate_variables": false,
         "dotenv_filename": null,
         "dotenv_load": null,
         "dotenv_path": null,
@@ -951,6 +1010,7 @@ fn attribute() {
       },
       "settings": {
         "allow_duplicate_recipes": false,
+        "allow_duplicate_variables": false,
         "dotenv_filename": null,
         "dotenv_load": null,
         "dotenv_path": null,
@@ -1011,6 +1071,7 @@ fn module() {
             },
             "settings": {
               "allow_duplicate_recipes": false,
+              "allow_duplicate_variables": false,
               "dotenv_filename": null,
               "dotenv_load": null,
               "dotenv_path": null,
@@ -1030,6 +1091,7 @@ fn module() {
         "recipes": {},
         "settings": {
           "allow_duplicate_recipes": false,
+          "allow_duplicate_variables": false,
           "dotenv_filename": null,
           "dotenv_load": null,
           "dotenv_path": null,

tests/lib.rs

@@ -33,6 +33,7 @@ pub(crate) use {
 mod test;
 
 mod allow_duplicate_recipes;
+mod allow_duplicate_variables;
 mod assert_stdout;
 mod assert_success;
 mod attributes;

tests/modules.rs

@@ -200,7 +200,11 @@ fn modules_use_module_settings() {
   Test::new()
     .write(
       "foo.just",
-      "set allow-duplicate-recipes\nfoo:\nfoo:\n @echo FOO\n",
+      "set allow-duplicate-recipes
+foo:
+foo:
+  @echo FOO
+",
     )
     .justfile(
       "
@@ -215,7 +219,13 @@ fn modules_use_module_settings() {
     .run();
 
   Test::new()
-    .write("foo.just", "\nfoo:\nfoo:\n @echo FOO\n")
+    .write(
+      "foo.just",
+      "foo:
+foo:
+  @echo FOO
+",
+    )
     .justfile(
       "
         mod foo
@@ -230,10 +240,10 @@ fn modules_use_module_settings() {
     .arg("foo")
     .stderr(
       "
-      error: Recipe `foo` first defined on line 2 is redefined on line 3
-       ——▶ foo.just:3:1
+      error: Recipe `foo` first defined on line 1 is redefined on line 2
+       ——▶ foo.just:2:1
         │
-      3 │ foo:
+      2 │ foo:
         │ ^^^
     ",
     )