Allow environment variables to be hidden (#3950)

* Allow environment variables to be hidden

This change allows environment variables in Nushell to have a value of
`Nothing`, which can be set by the user by passing `$nothing` to
`let-env` and friends.

Environment variables with a value of Nothing behave as if they are not
set at all. This allows a user to shadow the value of an environment
variable in a parent scope, effectively removing it from their current
scope. This was not possible before, because a scope can not affect its
parent scopes.

This is a workaround for issues like #3920.

Additionally, this allows a user to simultaneously set, change and
remove multiple environment variables via `load-env`. Any environment
variables set to $nothing will be hidden and thus act as if they are
removed. This simplifies working with virtual environments, which rely
on setting multiple environment variables, including PATH, to specific
values, and remove/change them on deactivation.

One surprising behavior is that an environment variable set to $nothing
will act as if it is not set when querying it (via $nu.env.X), but it is
still possible to remove it entirely via `unlet-env`. If the same
environment variable is present in the parent scope, the value in the
parent scope will be visible to the user. This might be surprising
behavior to users who are not familiar with the implementation details.

An additional corner case is the the shorthand form of `with-env` does
not work with this feature. Using `X=$nothing` will set $nu.env.X to the
string "$nothing". The long-form works as expected: `with-env [X
$nothing] {...}`.

* Remove unused import

* Allow all primitives to be convert to strings

crates/nu-engine/src/evaluate/envvar.rs

@@ -0,0 +1,50 @@
+use std::convert::TryFrom;
+
+use nu_errors::ShellError;
+use nu_protocol::{SpannedTypeName, Value};
+
+#[derive(Debug, Clone)]
+pub enum EnvVar {
+    Proper(String),
+    Nothing,
+}
+
+impl TryFrom<Value> for EnvVar {
+    type Error = ShellError;
+
+    fn try_from(value: Value) -> Result<Self, Self::Error> {
+        if value.value.is_none() {
+            Ok(EnvVar::Nothing)
+        } else if value.is_primitive() {
+            Ok(EnvVar::Proper(value.convert_to_string()))
+        } else {
+            Err(ShellError::type_error(
+                "primitive",
+                value.spanned_type_name(),
+            ))
+        }
+    }
+}
+
+impl TryFrom<&Value> for EnvVar {
+    type Error = ShellError;
+
+    fn try_from(value: &Value) -> Result<Self, Self::Error> {
+        if value.value.is_none() {
+            Ok(EnvVar::Nothing)
+        } else if value.is_primitive() {
+            Ok(EnvVar::Proper(value.convert_to_string()))
+        } else {
+            Err(ShellError::type_error(
+                "primitive",
+                value.spanned_type_name(),
+            ))
+        }
+    }
+}
+
+impl From<String> for EnvVar {
+    fn from(string: String) -> Self {
+        EnvVar::Proper(string)
+    }
+}

crates/nu-engine/src/evaluate/mod.rs

@@ -1,4 +1,5 @@
 pub(crate) mod block;
+pub(crate) mod envvar;
 pub(crate) mod evaluate_args;
 pub mod evaluator;
 pub(crate) mod expr;

crates/nu-engine/src/evaluate/scope.rs

@@ -1,4 +1,7 @@
-use crate::whole_stream_command::{whole_stream_command, Command};
+use crate::{
+    evaluate::envvar::EnvVar,
+    whole_stream_command::{whole_stream_command, Command},
+};
 use indexmap::IndexMap;
 use nu_errors::ShellError;
 use nu_parser::ParserScope;
@@ -203,6 +206,7 @@ impl Scope {
         }
     }
 
+    // This is used for starting processes, keep it string -> string
     pub fn get_env_vars(&self) -> IndexMap<String, String> {
         //FIXME: should this be an iterator?
         let mut output = IndexMap::new();
@@ -216,12 +220,21 @@ impl Scope {
         }
 
         output
+            .into_iter()
+            .filter_map(|(k, v)| match v {
+                EnvVar::Proper(s) => Some((k, s)),
+                EnvVar::Nothing => None,
+            })
+            .collect()
     }
 
     pub fn get_env(&self, name: &str) -> Option<String> {
         for frame in self.frames.lock().iter().rev() {
             if let Some(v) = frame.env.get(name) {
-                return Some(v.clone());
+                return match v {
+                    EnvVar::Proper(string) => Some(string.clone()),
+                    EnvVar::Nothing => None,
+                };
             }
         }
 
@@ -252,36 +265,36 @@ impl Scope {
         }
     }
 
-    pub fn add_env_var(&self, name: impl Into<String>, value: String) {
+    pub fn add_env_var(&self, name: impl Into<String>, value: impl Into<EnvVar>) {
         if let Some(frame) = self.frames.lock().last_mut() {
-            frame.env.insert(name.into(), value);
+            frame.env.insert(name.into(), value.into());
         }
     }
 
     pub fn remove_env_var(&self, name: impl Into<String>) -> Option<String> {
         if let Some(frame) = self.frames.lock().last_mut() {
             if let Some(val) = frame.env.remove_entry(&name.into()) {
-                return Some(val.1);
+                return Some(val.0);
             }
         }
         None
     }
 
-    pub fn add_env(&self, env_vars: IndexMap<String, String>) {
+    pub fn add_env(&self, env_vars: IndexMap<String, EnvVar>) {
         if let Some(frame) = self.frames.lock().last_mut() {
             frame.env.extend(env_vars)
         }
     }
 
-    pub fn add_env_to_base(&self, env_vars: IndexMap<String, String>) {
+    pub fn add_env_to_base(&self, env_vars: IndexMap<String, EnvVar>) {
         if let Some(frame) = self.frames.lock().first_mut() {
             frame.env.extend(env_vars)
         }
     }
 
-    pub fn add_env_var_to_base(&self, name: impl Into<String>, value: String) {
+    pub fn add_env_var_to_base(&self, name: impl Into<String>, value: impl Into<EnvVar>) {
         if let Some(frame) = self.frames.lock().first_mut() {
-            frame.env.insert(name.into(), value);
+            frame.env.insert(name.into(), value.into());
         }
     }
 
@@ -426,7 +439,7 @@ impl ParserScope for Scope {
 #[derive(Debug, Clone)]
 pub struct ScopeFrame {
     pub vars: IndexMap<String, Value>,
-    pub env: IndexMap<String, String>,
+    pub env: IndexMap<String, EnvVar>,
     pub commands: IndexMap<String, Command>,
     pub custom_commands: IndexMap<String, Arc<Block>>,
     pub aliases: IndexMap<String, Vec<Spanned<String>>>,

crates/nu-engine/src/evaluation_context.rs

@@ -1,3 +1,4 @@
+use crate::evaluate::envvar::EnvVar;
 use crate::evaluate::evaluator::Variable;
 use crate::evaluate::scope::{Scope, ScopeFrame};
 use crate::shell::palette::ThemedPalette;
@@ -66,7 +67,12 @@ impl EvaluationContext {
     pub fn basic() -> EvaluationContext {
         let scope = Scope::new();
         let host = BasicHost {};
-        let env_vars = host.vars().iter().cloned().collect::<IndexMap<_, _>>();
+        let env_vars: IndexMap<String, EnvVar> = host
+            .vars()
+            .iter()
+            .cloned()
+            .map(|(k, v)| (k, v.into()))
+            .collect();
         scope.add_env(env_vars);
 
         EvaluationContext {
@@ -239,7 +245,12 @@ impl EvaluationContext {
         let tag = config::cfg_path_to_scope_tag(cfg_path.get_path());
 
         self.scope.enter_scope_with_tag(tag);
-        self.scope.add_env(cfg.env_map());
+        let config_env = cfg.env_map();
+        let env_vars = config_env
+            .into_iter()
+            .map(|(k, v)| (k, EnvVar::from(v)))
+            .collect();
+        self.scope.add_env(env_vars);
         if let Some(path) = joined_paths {
             self.scope.add_env_var(NATIVE_PATH_ENV_VAR, path);
         }
@@ -348,10 +359,16 @@ impl EvaluationContext {
 
         let tag = config::cfg_path_to_scope_tag(&cfg.file_path);
         let mut frame = ScopeFrame::with_tag(tag.clone());
-
-        frame.env = cfg.env_map();
+        let config_env = cfg.env_map();
+        let env_vars = config_env
+            .into_iter()
+            .map(|(k, v)| (k, EnvVar::from(v)))
+            .collect();
+        frame.env = env_vars;
         if let Some(path) = joined_paths {
-            frame.env.insert(NATIVE_PATH_ENV_VAR.to_string(), path);
+            frame
+                .env
+                .insert(NATIVE_PATH_ENV_VAR.to_string(), path.into());
         }
         frame.exitscripts = exit_scripts;
 

crates/nu-engine/src/lib.rs

@@ -23,6 +23,7 @@ pub use crate::documentation::{generate_docs, get_brief_help, get_documentation,
 pub use crate::env::host::FakeHost;
 pub use crate::env::host::Host;
 pub use crate::evaluate::block::run_block;
+pub use crate::evaluate::envvar::EnvVar;
 pub use crate::evaluate::scope::Scope;
 pub use crate::evaluate::{evaluator, evaluator::evaluate_baseline_expr};
 pub use crate::evaluation_context::EvaluationContext;