Allow unexporting environment variables (#2098)

README.md

@@ -2053,6 +2053,23 @@ a $A $B=`echo $A`:
 When [export](#export) is set, all `just` variables are exported as environment
 variables.
 
+#### Unexporting Environment Variables<sup>master</sup>
+
+Environment variables can be unexported with the `unexport keyword`:
+
+```just
+unexport FOO
+
+@foo:
+  echo $FOO
+```
+
+```
+$ export FOO=bar
+$ just foo
+sh: FOO: unbound variable
+```
+
 #### Getting Environment Variables from the environment
 
 Environment variables from the environment are passed automatically to the

src/analyzer.rs

@@ -37,6 +37,8 @@ impl<'src> Analyzer<'src> {
 
     let mut modules: Table<Justfile> = Table::new();
 
+    let mut unexports: HashSet<String> = HashSet::new();
+
     let mut definitions: HashMap<&str, (&'static str, Name)> = HashMap::new();
 
     let mut define = |name: Name<'src>,
@@ -98,6 +100,13 @@ impl<'src> Analyzer<'src> {
             self.analyze_set(set)?;
             self.sets.insert(set.clone());
           }
+          Item::Unexport { name } => {
+            if !unexports.insert(name.lexeme().to_string()) {
+              return Err(name.token.error(DuplicateUnexport {
+                variable: name.lexeme(),
+              }));
+            }
+          }
         }
       }
 
@@ -109,21 +118,23 @@ impl<'src> Analyzer<'src> {
     let mut recipe_table: Table<'src, UnresolvedRecipe<'src>> = Table::default();
 
     for assignment in assignments {
-      if !settings.allow_duplicate_variables
+      let variable = assignment.name.lexeme();
-        && self.assignments.contains_key(assignment.name.lexeme())
+
-      {
+      if !settings.allow_duplicate_variables && self.assignments.contains_key(variable) {
-        return Err(assignment.name.token.error(DuplicateVariable {
+        return Err(assignment.name.token.error(DuplicateVariable { variable }));
-          variable: assignment.name.lexeme(),
-        }));
       }
 
       if self
         .assignments
-        .get(assignment.name.lexeme())
+        .get(variable)
         .map_or(true, |original| assignment.depth <= original.depth)
       {
         self.assignments.insert(assignment.clone());
       }
+
+      if unexports.contains(variable) {
+        return Err(assignment.name.token.error(ExportUnexported { variable }));
+      }
     }
 
     AssignmentResolver::resolve_assignments(&self.assignments)?;
@@ -167,6 +178,7 @@ impl<'src> Analyzer<'src> {
       recipes,
       settings,
       source: root.into(),
+      unexports,
       warnings,
     })
   }

src/command_ext.rs

@@ -1,25 +1,41 @@
 use super::*;
 
 pub(crate) trait CommandExt {
-  fn export(&mut self, settings: &Settings, dotenv: &BTreeMap<String, String>, scope: &Scope);
+  fn export(
+    &mut self,
+    settings: &Settings,
+    dotenv: &BTreeMap<String, String>,
+    scope: &Scope,
+    unexports: &HashSet<String>,
+  );
 
-  fn export_scope(&mut self, settings: &Settings, scope: &Scope);
+  fn export_scope(&mut self, settings: &Settings, scope: &Scope, unexports: &HashSet<String>);
 }
 
 impl CommandExt for Command {
-  fn export(&mut self, settings: &Settings, dotenv: &BTreeMap<String, String>, scope: &Scope) {
+  fn export(
+    &mut self,
+    settings: &Settings,
+    dotenv: &BTreeMap<String, String>,
+    scope: &Scope,
+    unexports: &HashSet<String>,
+  ) {
     for (name, value) in dotenv {
       self.env(name, value);
     }
 
     if let Some(parent) = scope.parent() {
-      self.export_scope(settings, parent);
+      self.export_scope(settings, parent, unexports);
     }
   }
 
-  fn export_scope(&mut self, settings: &Settings, scope: &Scope) {
+  fn export_scope(&mut self, settings: &Settings, scope: &Scope, unexports: &HashSet<String>) {
     if let Some(parent) = scope.parent() {
-      self.export_scope(settings, parent);
+      self.export_scope(settings, parent, unexports);
+    }
+
+    for unexport in unexports {
+      self.env_remove(unexport);
     }
 
     for binding in scope.bindings() {

src/compile_error.rs

@@ -131,6 +131,9 @@ impl Display for CompileError<'_> {
       DuplicateVariable { variable } => {
         write!(f, "Variable `{variable}` has multiple definitions")
       }
+      DuplicateUnexport { variable } => {
+        write!(f, "Variable `{variable}` is unexported multiple times")
+      }
       ExpectedKeyword { expected, found } => {
         let expected = List::or_ticked(expected);
         if found.kind == TokenKind::Identifier {
@@ -143,6 +146,9 @@ impl Display for CompileError<'_> {
           write!(f, "Expected keyword {expected} but found `{}`", found.kind)
         }
       }
+      ExportUnexported { variable } => {
+        write!(f, "Variable {variable} is both exported and unexported")
+      }
       ExtraLeadingWhitespace => write!(f, "Recipe line has extra leading whitespace"),
       FunctionArgumentCountMismatch {
         function,

src/compile_error_kind.rs

@@ -52,10 +52,16 @@ pub(crate) enum CompileErrorKind<'src> {
   DuplicateVariable {
     variable: &'src str,
   },
+  DuplicateUnexport {
+    variable: &'src str,
+  },
   ExpectedKeyword {
     expected: Vec<Keyword>,
     found: Token<'src>,
   },
+  ExportUnexported {
+    variable: &'src str,
+  },
   ExtraLeadingWhitespace,
   FunctionArgumentCountMismatch {
     function: &'src str,

src/evaluator.rs

@@ -8,6 +8,7 @@ pub(crate) struct Evaluator<'src: 'run, 'run> {
   pub(crate) scope: Scope<'src, 'run>,
   pub(crate) search: &'run Search,
   pub(crate) settings: &'run Settings<'run>,
+  unsets: &'run HashSet<String>,
 }
 
 impl<'src, 'run> Evaluator<'src, 'run> {
@@ -19,6 +20,7 @@ impl<'src, 'run> Evaluator<'src, 'run> {
     scope: Scope<'src, 'run>,
     search: &'run Search,
     settings: &'run Settings<'run>,
+    unsets: &'run HashSet<String>,
   ) -> RunResult<'src, Scope<'src, 'run>> {
     let mut evaluator = Self {
       assignments: Some(assignments),
@@ -28,6 +30,7 @@ impl<'src, 'run> Evaluator<'src, 'run> {
       scope,
       search,
       settings,
+      unsets,
     };
 
     for assignment in assignments.values() {
@@ -217,7 +220,7 @@ impl<'src, 'run> Evaluator<'src, 'run> {
     cmd.arg(command);
     cmd.args(args);
     cmd.current_dir(&self.search.working_directory);
-    cmd.export(self.settings, self.dotenv, &self.scope);
+    cmd.export(self.settings, self.dotenv, &self.scope, self.unsets);
     cmd.stdin(Stdio::inherit());
     cmd.stderr(if self.config.verbosity.quiet() {
       Stdio::null()
@@ -261,6 +264,7 @@ impl<'src, 'run> Evaluator<'src, 'run> {
     scope: &'run Scope<'src, 'run>,
     search: &'run Search,
     settings: &'run Settings,
+    unsets: &'run HashSet<String>,
   ) -> RunResult<'src, (Scope<'src, 'run>, Vec<String>)> {
     let mut evaluator = Self {
       assignments: None,
@@ -270,6 +274,7 @@ impl<'src, 'run> Evaluator<'src, 'run> {
       scope: scope.child(),
       search,
       settings,
+      unsets,
     };
 
     let mut scope = scope.child();
@@ -316,6 +321,7 @@ impl<'src, 'run> Evaluator<'src, 'run> {
     scope: &'run Scope<'src, 'run>,
     search: &'run Search,
     settings: &'run Settings,
+    unsets: &'run HashSet<String>,
   ) -> Self {
     Self {
       assignments: None,
@@ -325,6 +331,7 @@ impl<'src, 'run> Evaluator<'src, 'run> {
       scope: Scope::child(scope),
       search,
       settings,
+      unsets,
     }
   }
 }

src/item.rs

@@ -20,6 +20,9 @@ pub(crate) enum Item<'src> {
   },
   Recipe(UnresolvedRecipe<'src>),
   Set(Set<'src>),
+  Unexport {
+    name: Name<'src>,
+  },
 }
 
 impl<'src> Display for Item<'src> {
@@ -61,6 +64,7 @@ impl<'src> Display for Item<'src> {
       }
       Self::Recipe(recipe) => write!(f, "{}", recipe.color_display(Color::never())),
       Self::Set(set) => write!(f, "{set}"),
+      Self::Unexport { name } => write!(f, "unexport {name}"),
     }
   }
 }

src/justfile.rs

@@ -24,6 +24,7 @@ pub(crate) struct Justfile<'src> {
   pub(crate) settings: Settings<'src>,
   #[serde(skip)]
   pub(crate) source: PathBuf,
+  pub(crate) unexports: HashSet<String>,
   pub(crate) warnings: Vec<Warning>,
 }
 
@@ -113,6 +114,7 @@ impl<'src> Justfile<'src> {
       scope,
       search,
       &self.settings,
+      &self.unexports,
     )
   }
 
@@ -163,7 +165,7 @@ impl<'src> Justfile<'src> {
 
         let scope = scope.child();
 
-        command.export(&self.settings, &dotenv, &scope);
+        command.export(&self.settings, &dotenv, &scope, &self.unexports);
 
         let status = InterruptHandler::guard(|| command.status()).map_err(|io_error| {
           Error::CommandInvoke {
@@ -286,6 +288,7 @@ impl<'src> Justfile<'src> {
         scope: invocation.scope,
         search,
         settings: invocation.settings,
+        unexports: &self.unexports,
       };
 
       Self::run_recipe(
@@ -441,6 +444,7 @@ impl<'src> Justfile<'src> {
       context.scope,
       search,
       context.settings,
+      context.unexports,
     )?;
 
     let scope = outer.child();
@@ -452,6 +456,7 @@ impl<'src> Justfile<'src> {
       &scope,
       search,
       context.settings,
+      context.unexports,
     );
 
     if !context.config.no_dependencies {

src/keyword.rs

@@ -25,6 +25,7 @@ pub(crate) enum Keyword {
   Shell,
   Tempdir,
   True,
+  Unexport,
   WindowsPowershell,
   WindowsShell,
   X,

src/lib.rs

@@ -42,7 +42,7 @@ pub(crate) use {
   std::{
     borrow::Cow,
     cmp,
-    collections::{BTreeMap, BTreeSet, HashMap},
+    collections::{BTreeMap, BTreeSet, HashMap, HashSet},
     env,
     ffi::OsString,
     fmt::{self, Debug, Display, Formatter},

src/node.rs

@@ -54,6 +54,11 @@ impl<'src> Node<'src> for Item<'src> {
       }
       Self::Recipe(recipe) => recipe.tree(),
       Self::Set(set) => set.tree(),
+      Self::Unexport { name } => {
+        let mut unexport = Tree::atom(Keyword::Unexport.lexeme());
+        unexport.push_mut(name.lexeme().replace('-', "_"));
+        unexport
+      }
     }
   }
 }

src/parser.rs

@@ -340,6 +340,11 @@ impl<'run, 'src> Parser<'run, 'src> {
             self.presume_keyword(Keyword::Export)?;
             items.push(Item::Assignment(self.parse_assignment(true)?));
           }
+          Some(Keyword::Unexport) => {
+            self.presume_keyword(Keyword::Unexport)?;
+            let name = self.parse_name()?;
+            items.push(Item::Unexport { name });
+          }
           Some(Keyword::Import)
             if self.next_are(&[Identifier, StringToken])
               || self.next_are(&[Identifier, Identifier, StringToken])

src/recipe.rs

@@ -169,6 +169,7 @@ impl<'src, D> Recipe<'src, D> {
       scope,
       context.search,
       context.settings,
+      context.unexports,
     );
 
     if self.shebang {
@@ -279,7 +280,7 @@ impl<'src, D> Recipe<'src, D> {
         cmd.stdout(Stdio::null());
       }
 
-      cmd.export(context.settings, context.dotenv, scope);
+      cmd.export(context.settings, context.dotenv, scope, context.unexports);
 
       match InterruptHandler::guard(|| cmd.status()) {
         Ok(exit_status) => {
@@ -425,7 +426,7 @@ impl<'src, D> Recipe<'src, D> {
       command.args(positional);
     }
 
-    command.export(context.settings, context.dotenv, scope);
+    command.export(context.settings, context.dotenv, scope, context.unexports);
 
     // run it!
     match InterruptHandler::guard(|| command.status()) {

src/recipe_context.rs

@@ -7,4 +7,5 @@ pub(crate) struct RecipeContext<'src: 'run, 'run> {
   pub(crate) scope: &'run Scope<'src, 'run>,
   pub(crate) search: &'run Search,
   pub(crate) settings: &'run Settings<'src>,
+  pub(crate) unexports: &'run HashSet<String>,
 }

tests/completions.rs

@@ -29,10 +29,10 @@ fn bash() {
 #[test]
 fn replacements() {
   for shell in ["bash", "elvish", "fish", "powershell", "zsh"] {
-    let status = Command::new(executable_path("just"))
+    let output = Command::new(executable_path("just"))
       .args(["--completions", shell])
-      .status()
+      .output()
       .unwrap();
-    assert!(status.success());
+    assert!(output.status.success());
   }
 }

tests/json.rs

@@ -59,6 +59,7 @@ fn alias() {
         "windows_powershell": false,
         "windows_shell": null,
       },
+      "unexports": [],
       "warnings": [],
     }),
   );
@@ -98,6 +99,7 @@ fn assignment() {
         "windows_powershell": false,
         "windows_shell": null,
       },
+      "unexports": [],
       "warnings": [],
     }),
   );
@@ -151,6 +153,7 @@ fn body() {
         "windows_powershell": false,
         "windows_shell": null,
       },
+      "unexports": [],
       "warnings": [],
     }),
   );
@@ -216,6 +219,7 @@ fn dependencies() {
         "windows_powershell": false,
         "windows_shell": null,
       },
+      "unexports": [],
       "warnings": [],
     }),
   );
@@ -319,6 +323,7 @@ fn dependency_argument() {
         "windows_powershell": false,
         "windows_shell": null,
       },
+      "unexports": [],
       "warnings": [],
     }),
   );
@@ -384,6 +389,7 @@ fn duplicate_recipes() {
         "windows_powershell": false,
         "windows_shell": null,
       },
+      "unexports": [],
       "warnings": [],
     }),
   );
@@ -427,6 +433,7 @@ fn duplicate_variables() {
         "windows_powershell": false,
         "windows_shell": null,
       },
+      "unexports": [],
       "warnings": [],
     }),
   );
@@ -473,6 +480,7 @@ fn doc_comment() {
         "windows_powershell": false,
         "windows_shell": null,
       },
+      "unexports": [],
       "warnings": [],
     }),
   );
@@ -505,6 +513,7 @@ fn empty_justfile() {
         "windows_powershell": false,
         "windows_shell": null,
       },
+      "unexports": [],
       "warnings": [],
     }),
   );
@@ -658,6 +667,7 @@ fn parameters() {
         "windows_powershell": false,
         "windows_shell": null,
       },
+      "unexports": [],
       "warnings": [],
     }),
   );
@@ -744,6 +754,7 @@ fn priors() {
         "windows_powershell": false,
         "windows_shell": null,
       },
+      "unexports": [],
       "warnings": [],
     }),
   );
@@ -790,6 +801,7 @@ fn private() {
         "windows_powershell": false,
         "windows_shell": null,
       },
+      "unexports": [],
       "warnings": [],
     }),
   );
@@ -836,6 +848,7 @@ fn quiet() {
         "windows_powershell": false,
         "windows_shell": null,
       },
+      "unexports": [],
       "warnings": [],
     }),
   );
@@ -897,6 +910,7 @@ fn settings() {
         "windows_powershell": false,
         "windows_shell": null,
       },
+      "unexports": [],
       "warnings": [],
     }),
   );
@@ -946,6 +960,7 @@ fn shebang() {
         "windows_powershell": false,
         "windows_shell": null,
       },
+      "unexports": [],
       "warnings": [],
     }),
   );
@@ -992,6 +1007,7 @@ fn simple() {
         "windows_powershell": false,
         "windows_shell": null,
       },
+      "unexports": [],
       "warnings": [],
     }),
   );
@@ -1041,6 +1057,7 @@ fn attribute() {
         "windows_powershell": false,
         "windows_shell": null,
       },
+      "unexports": [],
       "warnings": [],
     }),
   );
@@ -1103,6 +1120,7 @@ fn module() {
               "windows_powershell": false,
               "windows_shell": null,
             },
+            "unexports": [],
             "warnings": [],
           },
         },
@@ -1124,6 +1142,7 @@ fn module() {
           "windows_powershell": false,
           "windows_shell": null,
         },
+        "unexports": [],
         "warnings": [],
       }))
       .unwrap()

tests/lib.rs

@@ -104,6 +104,7 @@ mod summary;
 mod tempdir;
 mod timestamps;
 mod undefined_variables;
+mod unexport;
 mod unstable;
 #[cfg(target_family = "windows")]
 mod windows_shell;

tests/unexport.rs

@@ -0,0 +1,104 @@
+use super::*;
+
+#[test]
+fn unexport_environment_variable_linewise() {
+  Test::new()
+    .justfile(
+      "
+     unexport JUST_TEST_VARIABLE
+
+     @recipe:
+         echo ${JUST_TEST_VARIABLE:-unset}
+      ",
+    )
+    .env("JUST_TEST_VARIABLE", "foo")
+    .stdout("unset\n")
+    .run();
+}
+
+#[test]
+fn unexport_environment_variable_shebang() {
+  Test::new()
+    .justfile(
+      "
+     unexport JUST_TEST_VARIABLE
+
+     recipe:
+         #!/usr/bin/env bash
+         echo ${JUST_TEST_VARIABLE:-unset}
+      ",
+    )
+    .env("JUST_TEST_VARIABLE", "foo")
+    .stdout("unset\n")
+    .run();
+}
+
+#[test]
+fn duplicate_unexport_fails() {
+  Test::new()
+    .justfile(
+      "
+     unexport JUST_TEST_VARIABLE
+
+     recipe:
+         echo \"variable: $JUST_TEST_VARIABLE\"
+
+     unexport JUST_TEST_VARIABLE
+      ",
+    )
+    .env("JUST_TEST_VARIABLE", "foo")
+    .stderr(
+      "
+        error: Variable `JUST_TEST_VARIABLE` is unexported multiple times
+         ——▶ justfile:6:10
+          │
+        6 │ unexport JUST_TEST_VARIABLE
+          │          ^^^^^^^^^^^^^^^^^^
+      ",
+    )
+    .status(1)
+    .run();
+}
+
+#[test]
+fn export_unexport_conflict() {
+  Test::new()
+    .justfile(
+      "
+     unexport JUST_TEST_VARIABLE
+
+     recipe:
+         echo variable: $JUST_TEST_VARIABLE
+
+     export JUST_TEST_VARIABLE := 'foo'
+      ",
+    )
+    .stderr(
+      "
+        error: Variable JUST_TEST_VARIABLE is both exported and unexported
+         ——▶ justfile:6:8
+          │
+        6 │ export JUST_TEST_VARIABLE := 'foo'
+          │        ^^^^^^^^^^^^^^^^^^
+      ",
+    )
+    .status(1)
+    .run();
+}
+
+#[test]
+fn unexport_doesnt_override_local_recipe_export() {
+  Test::new()
+    .justfile(
+      "
+     unexport JUST_TEST_VARIABLE
+
+     recipe $JUST_TEST_VARIABLE:
+         @echo \"variable: $JUST_TEST_VARIABLE\"
+      ",
+    )
+    .args(["recipe", "value"])
+    .stdout("variable: value\n")
+    .status(0)
+    .run();
+}