Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implemented 'set allow-duplicate-variables' #1922

Merged
merged 12 commits into from
May 15, 2024
1 change: 1 addition & 0 deletions GRAMMAR.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
25 changes: 24 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`. |
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:

Expand Down
32 changes: 21 additions & 11 deletions src/analyzer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());

Expand Down Expand Up @@ -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, .. } => {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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();

Expand Down
2 changes: 2 additions & 0 deletions src/binding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions src/keyword.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use super::*;
pub(crate) enum Keyword {
Alias,
AllowDuplicateRecipes,
AllowDuplicateVariables,
DotenvFilename,
DotenvLoad,
DotenvPath,
Expand Down
1 change: 1 addition & 0 deletions src/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
10 changes: 10 additions & 0 deletions src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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()?)),
Expand Down Expand Up @@ -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",
Expand Down
1 change: 1 addition & 0 deletions src/scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions src/setting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use super::*;
#[derive(Debug, Clone)]
pub(crate) enum Setting<'src> {
AllowDuplicateRecipes(bool),
AllowDuplicateVariables(bool),
DotenvFilename(String),
DotenvLoad(bool),
DotenvPath(String),
Expand All @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions src/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand All @@ -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);
}
Expand Down
21 changes: 21 additions & 0 deletions tests/allow_duplicate_variables.rs
Original file line number Diff line number Diff line change
@@ -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();
}
32 changes: 29 additions & 3 deletions tests/imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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() {
Expand Down
Loading