Add hide-env to hide environment variables (#6313)

* Add hide-env to hide env vars; Cleanup tests

Also, there were some old unalias tests that I converted to hide.

* Add missing file

* Re-enable hide for env vars

* Fix test

* Rename did you mean error back

It was causing random tests to break

crates/nu-command/src/core_commands/hide.rs

@@ -19,11 +19,11 @@ impl Command for Hide {
     }
 
     fn usage(&self) -> &str {
-        "Hide symbols in the current scope"
+        "Hide definitions in the current scope"
     }
 
     fn extra_usage(&self) -> &str {
-        r#"Symbols are hidden by priority: First aliases, then custom commands, then environment variables.
+        r#"Definitions are hidden by priority: First aliases, then custom commands.
 
 This command is a parser keyword. For details, check:
   https://www.nushell.sh/book/thinking_in_nushell.html"#

crates/nu-command/src/core_commands/hide_env.rs

@@ -0,0 +1,75 @@
+use nu_engine::CallExt;
+use nu_protocol::ast::Call;
+use nu_protocol::engine::{Command, EngineState, Stack};
+use nu_protocol::{
+    did_you_mean, Category, Example, PipelineData, ShellError, Signature, Span, Spanned,
+    SyntaxShape, Value,
+};
+
+#[derive(Clone)]
+pub struct HideEnv;
+
+impl Command for HideEnv {
+    fn name(&self) -> &str {
+        "hide-env"
+    }
+
+    fn signature(&self) -> nu_protocol::Signature {
+        Signature::build("hide-env")
+            .rest(
+                "name",
+                SyntaxShape::String,
+                "environment variable names to hide",
+            )
+            .switch(
+                "ignore-errors",
+                "do not throw an error if an environment variable was not found",
+                Some('i'),
+            )
+            .category(Category::Core)
+    }
+
+    fn usage(&self) -> &str {
+        "Hide environment variables in the current scope"
+    }
+
+    fn run(
+        &self,
+        engine_state: &EngineState,
+        stack: &mut Stack,
+        call: &Call,
+        _input: PipelineData,
+    ) -> Result<nu_protocol::PipelineData, nu_protocol::ShellError> {
+        let env_var_names: Vec<Spanned<String>> = call.rest(engine_state, stack, 0)?;
+        let ignore_errors = call.has_flag("ignore-errors");
+
+        for name in env_var_names {
+            if stack.remove_env_var(engine_state, &name.item).is_none() && !ignore_errors {
+                let all_names: Vec<String> = stack
+                    .get_env_var_names(engine_state)
+                    .iter()
+                    .cloned()
+                    .collect();
+                if let Some(closest_match) = did_you_mean(&all_names, &name.item) {
+                    return Err(ShellError::DidYouMeanCustom(
+                        format!("Environment variable '{}' not found", name.item),
+                        closest_match,
+                        name.span,
+                    ));
+                } else {
+                    return Err(ShellError::EnvVarNotFoundAtRuntime(name.item, name.span));
+                }
+            }
+        }
+
+        Ok(PipelineData::new(call.head))
+    }
+
+    fn examples(&self) -> Vec<Example> {
+        vec![Example {
+            description: "Hide an environment variable",
+            example: r#"let-env HZ_ENV_ABC = 1; hide-env HZ_ENV_ABC; 'HZ_ENV_ABC' in (env).name"#,
+            result: Some(Value::boolean(false, Span::test_data())),
+        }]
+    }
+}

crates/nu-command/src/core_commands/mod.rs

@@ -17,6 +17,7 @@ mod extern_;
 mod for_;
 pub mod help;
 mod hide;
+mod hide_env;
 mod if_;
 mod ignore;
 mod let_;
@@ -46,6 +47,7 @@ pub use extern_::Extern;
 pub use for_::For;
 pub use help::Help;
 pub use hide::Hide;
+pub use hide_env::HideEnv;
 pub use if_::If;
 pub use ignore::Ignore;
 pub use let_::Let;

crates/nu-command/src/default_context.rs

@@ -47,6 +47,7 @@ pub fn create_default_context() -> EngineState {
             For,
             Help,
             Hide,
+            HideEnv,
             If,
             Ignore,
             Overlay,

crates/nu-protocol/src/shell_error.rs

@@ -626,6 +626,15 @@ Either make sure {0} is a string, or add a 'to_string' entry for it in ENV_CONVE
     #[diagnostic(code(nu::shell::name_not_found), url(docsrs))]
     DidYouMean(String, #[label("did you mean '{0}'?")] Span),
 
+    /// A name was not found. Did you mean a different name?
+    ///
+    /// ## Resolution
+    ///
+    /// The error message will suggest a possible match for what you meant.
+    #[error("{0}")]
+    #[diagnostic(code(nu::shell::did_you_mean_custom), url(docsrs))]
+    DidYouMeanCustom(String, String, #[label("did you mean '{1}'?")] Span),
+
     /// The given input must be valid UTF-8 for further processing.
     ///
     /// ## Resolution

src/tests/test_engine.rs

@@ -191,7 +191,7 @@ fn not_def_env() -> TestResult {
 #[test]
 fn def_env_hiding_something() -> TestResult {
     fail_test(
-        r#"let-env FOO = "foo"; def-env bob [] { hide FOO }; bob; $env.FOO"#,
+        r#"let-env FOO = "foo"; def-env bob [] { hide-env FOO }; bob; $env.FOO"#,
         "did you mean",
     )
 }
@@ -199,7 +199,7 @@ fn def_env_hiding_something() -> TestResult {
 #[test]
 fn def_env_then_hide() -> TestResult {
     fail_test(
-        r#"def-env bob [] { let-env BOB = "bob" }; def-env un-bob [] { hide BOB }; bob; un-bob; $env.BOB"#,
+        r#"def-env bob [] { let-env BOB = "bob" }; def-env un-bob [] { hide-env BOB }; bob; un-bob; $env.BOB"#,
         "did you mean",
     )
 }

src/tests/test_hiding.rs

@@ -19,7 +19,10 @@ fn hides_alias() -> TestResult {
 
 #[test]
 fn hides_env() -> TestResult {
-    fail_test(r#"let-env foo = "foo"; hide foo; $env.foo"#, "did you mean")
+    fail_test(
+        r#"let-env foo = "foo"; hide-env foo; $env.foo"#,
+        "did you mean",
+    )
 }
 
 #[test]
@@ -43,7 +46,7 @@ fn hides_alias_then_redefines() -> TestResult {
 #[test]
 fn hides_env_then_redefines() -> TestResult {
     run_test(
-        r#"let-env foo = "foo"; hide foo; let-env foo = "bar"; $env.foo"#,
+        r#"let-env foo = "foo"; hide-env foo; let-env foo = "bar"; $env.foo"#,
         "bar",
     )
 }
@@ -115,7 +118,7 @@ fn hides_alias_in_scope_4() -> TestResult {
 #[test]
 fn hides_env_in_scope_1() -> TestResult {
     fail_test(
-        r#"let-env foo = "foo"; do { hide foo; $env.foo }"#,
+        r#"let-env foo = "foo"; do { hide-env foo; $env.foo }"#,
         "did you mean",
     )
 }
@@ -123,7 +126,7 @@ fn hides_env_in_scope_1() -> TestResult {
 #[test]
 fn hides_env_in_scope_2() -> TestResult {
     run_test(
-        r#"let-env foo = "foo"; do { let-env foo = "bar"; hide foo; $env.foo }"#,
+        r#"let-env foo = "foo"; do { let-env foo = "bar"; hide-env foo; $env.foo }"#,
         "foo",
     )
 }
@@ -131,7 +134,7 @@ fn hides_env_in_scope_2() -> TestResult {
 #[test]
 fn hides_env_in_scope_3() -> TestResult {
     fail_test(
-        r#"let-env foo = "foo"; do { hide foo; let-env foo = "bar"; hide foo; $env.foo }"#,
+        r#"let-env foo = "foo"; do { hide-env foo; let-env foo = "bar"; hide-env foo; $env.foo }"#,
         "did you mean",
     )
 }
@@ -139,7 +142,7 @@ fn hides_env_in_scope_3() -> TestResult {
 #[test]
 fn hides_env_in_scope_4() -> TestResult {
     fail_test(
-        r#"let-env foo = "foo"; do { let-env foo = "bar"; hide foo; hide foo; $env.foo }"#,
+        r#"let-env foo = "foo"; do { let-env foo = "bar"; hide-env foo; hide-env foo; $env.foo }"#,
         "did you mean",
     )
 }
@@ -163,9 +166,19 @@ fn hide_alias_twice_not_allowed() -> TestResult {
 }
 
 #[test]
-#[ignore]
 fn hide_env_twice_not_allowed() -> TestResult {
-    fail_test(r#"let-env foo = "foo"; hide foo; hide foo"#, "did not find")
+    fail_test(
+        r#"let-env foo = "foo"; hide-env foo; hide-env foo"#,
+        "did you mean",
+    )
+}
+
+#[test]
+fn hide_env_twice_allowed() -> TestResult {
+    fail_test(
+        r#"let-env foo = "foo"; hide-env foo; hide-env -i foo; $env.foo"#,
+        "did you mean",
+    )
 }
 
 #[test]
@@ -200,30 +213,6 @@ fn hides_alias_runs_def_2() -> TestResult {
     )
 }
 
-#[test]
-fn hides_alias_runs_env_1() -> TestResult {
-    run_test(
-        r#"let-env foo = "bar"; alias foo = echo "foo"; hide foo; $env.foo"#,
-        "bar",
-    )
-}
-
-#[test]
-fn hides_alias_runs_env_2() -> TestResult {
-    run_test(
-        r#"alias foo = echo "foo"; let-env foo = "bar"; hide foo; $env.foo"#,
-        "bar",
-    )
-}
-
-#[test]
-fn hides_def_and_env() -> TestResult {
-    fail_test(
-        r#"let-env foo = "bar"; def foo [] { "foo" }; hide foo; hide foo; $env.foo"#,
-        "did you mean",
-    )
-}
-
 #[test]
 fn hides_alias_and_def() -> TestResult {
     fail_test(
@@ -232,22 +221,6 @@ fn hides_alias_and_def() -> TestResult {
     )
 }
 
-#[test]
-fn hides_alias_def_and_env_1() -> TestResult {
-    fail_test(
-        r#"alias foo = echo "foo"; def foo [] { "foo" }; let-env foo = "bar"; hide foo; hide foo; hide foo; $env.foo"#,
-        "", // we just care if it errors
-    )
-}
-
-#[test]
-fn hides_alias_def_and_env_2() -> TestResult {
-    fail_test(
-        r#"alias foo = echo "foo"; def foo [] { "foo" }; let-env foo = "bar"; hide foo; hide foo; hide foo; foo"#,
-        "", // we just care if it errors
-    )
-}
-
 #[test]
 fn hides_def_import_1() -> TestResult {
     fail_test(
@@ -363,23 +336,25 @@ fn hides_alias_import_then_reimports() -> TestResult {
 #[test]
 fn hides_env_import_1() -> TestResult {
     fail_test(
-        r#"module spam { export env foo { "foo" } }; use spam; hide spam foo; $env.'spam foo'"#,
+        r#"module spam { export env foo { "foo" } }; use spam; hide-env 'spam foo'; $env.'spam foo'"#,
         "did you mean",
     )
 }
 
 #[test]
+#[ignore]
 fn hides_env_import_2() -> TestResult {
     fail_test(
-        r#"module spam { export env foo { "foo" } }; use spam; hide spam; $env.'spam foo'"#,
+        r#"module spam { export env foo { "foo" } }; use spam; hide-env spam; $env.'spam foo'"#,
         "did you mean",
     )
 }
 
 #[test]
+#[ignore]
 fn hides_env_import_3() -> TestResult {
     fail_test(
-        r#"module spam { export env foo { "foo" } }; use spam; hide spam [foo]; $env.'spam foo'"#,
+        r#"module spam { export env foo { "foo" } }; use spam; hide-env spam [foo]; $env.'spam foo'"#,
         "did you mean",
     )
 }
@@ -387,7 +362,7 @@ fn hides_env_import_3() -> TestResult {
 #[test]
 fn hides_env_import_4() -> TestResult {
     fail_test(
-        r#"module spam { export env foo { "foo" } }; use spam foo; hide foo; $env.foo"#,
+        r#"module spam { export env foo { "foo" } }; use spam foo; hide-env foo; $env.foo"#,
         "did you mean",
     )
 }
@@ -395,15 +370,16 @@ fn hides_env_import_4() -> TestResult {
 #[test]
 fn hides_env_import_5() -> TestResult {
     fail_test(
-        r#"module spam { export env foo { "foo" } }; use spam *; hide foo; $env.foo"#,
+        r#"module spam { export env foo { "foo" } }; use spam *; hide-env foo; $env.foo"#,
         "did you mean",
     )
 }
 
 #[test]
+#[ignore]
 fn hides_env_import_6() -> TestResult {
     fail_test(
-        r#"module spam { export env foo { "foo" } }; use spam *; hide spam *; $env.foo"#,
+        r#"module spam { export env foo { "foo" } }; use spam *; hide-env spam *; $env.foo"#,
         "did you mean",
     )
 }
@@ -419,7 +395,7 @@ fn hides_def_runs_env_import() -> TestResult {
 #[test]
 fn hides_def_and_env_import_1() -> TestResult {
     fail_test(
-        r#"module spam { export env foo { "foo" }; export def foo [] { "bar" } }; use spam foo; hide foo; hide foo; $env.foo"#,
+        r#"module spam { export env foo { "foo" }; export def foo [] { "bar" } }; use spam foo; hide foo; hide-env foo; $env.foo"#,
         "did you mean",
     )
 }
@@ -427,7 +403,7 @@ fn hides_def_and_env_import_1() -> TestResult {
 #[test]
 fn hides_def_and_env_import_2() -> TestResult {
     fail_test(
-        r#"module spam { export env foo { "foo" }; export def foo [] { "bar" } }; use spam foo; hide foo; hide foo; foo"#,
+        r#"module spam { export env foo { "foo" }; export def foo [] { "bar" } }; use spam foo; hide foo; hide-env foo; foo"#,
         "", // we just care if it errors
     )
 }
@@ -443,7 +419,7 @@ fn use_def_import_after_hide() -> TestResult {
 #[test]
 fn use_env_import_after_hide() -> TestResult {
     run_test(
-        r#"module spam { export env foo { "foo" } }; use spam foo; hide foo; use spam foo; $env.foo"#,
+        r#"module spam { export env foo { "foo" } }; use spam foo; hide-env foo; use spam foo; $env.foo"#,
         "foo",
     )
 }
@@ -459,7 +435,7 @@ fn hide_shadowed_decl() -> TestResult {
 #[test]
 fn hide_shadowed_env() -> TestResult {
     run_test(
-        r#"module spam { export env foo { "bar" } }; let-env foo = "foo"; do { use spam foo; hide foo; $env.foo }"#,
+        r#"module spam { export env foo { "bar" } }; let-env foo = "foo"; do { use spam foo; hide-env foo; $env.foo }"#,
         "foo",
     )
 }
@@ -475,7 +451,7 @@ fn hides_all_decls_within_scope() -> TestResult {
 #[test]
 fn hides_all_envs_within_scope() -> TestResult {
     fail_test(
-        r#"module spam { export env foo { "bar" } }; let-env foo = "foo"; use spam foo; hide foo; $env.foo"#,
+        r#"module spam { export env foo { "bar" } }; let-env foo = "foo"; use spam foo; hide-env foo; $env.foo"#,
         "did you mean",
     )
 }

tests/shell/pipeline/commands/internal.rs

@@ -374,7 +374,7 @@ fn let_env_hides_variable() {
         r#"
             let-env TESTENVVAR = "hello world"
             echo $env.TESTENVVAR
-            hide TESTENVVAR
+            hide-env TESTENVVAR
             echo $env.TESTENVVAR
         "#
     );
@@ -391,7 +391,7 @@ fn let_env_hides_variable_in_parent_scope() {
             let-env TESTENVVAR = "hello world"
             echo $env.TESTENVVAR
             do {
-                hide TESTENVVAR
+                hide-env TESTENVVAR
                 echo $env.TESTENVVAR
             }
             echo $env.TESTENVVAR
@@ -408,7 +408,7 @@ fn unlet_env_variable() {
         cwd: ".",
         r#"
             let-env TEST_VAR = "hello world"
-            hide TEST_VAR
+            hide-env TEST_VAR
             echo $env.TEST_VAR
         "#
     );
@@ -421,7 +421,7 @@ fn unlet_nonexistent_variable() {
     let actual = nu!(
         cwd: ".",
         r#"
-            hide NONEXISTENT_VARIABLE
+            hide-env NONEXISTENT_VARIABLE
         "#
     );
 
@@ -438,7 +438,7 @@ fn unlet_variable_in_parent_scope() {
             do {
                 let-env DEBUG = "2"
                 echo $env.DEBUG
-                hide DEBUG
+                hide-env DEBUG
                 echo $env.DEBUG
             }
             echo $env.DEBUG
@@ -1171,17 +1171,15 @@ fn nothing_string_1() {
     assert_eq!(actual.out, "false");
 }
 
-// FIXME: no current way to hide aliases
-#[ignore]
 #[test]
-fn unalias_shadowing() {
+fn hide_alias_shadowing() {
     let actual = nu!(
         cwd: ".", pipeline(
         r#"
         def test-shadowing [] {
             alias greet = echo hello;
             let xyz = { greet };
-            unalias greet;
+            hide greet;
             do $xyz
         };
         test-shadowing
@@ -1190,16 +1188,16 @@ fn unalias_shadowing() {
     assert_eq!(actual.out, "hello");
 }
 
-// FIXME: no current way to hide aliases
+// FIXME: Seems like subexpression are no longer scoped. Should we remove this test?
 #[ignore]
 #[test]
-fn unalias_does_not_escape_scope() {
+fn hide_alias_does_not_escape_scope() {
     let actual = nu!(
         cwd: ".", pipeline(
         r#"
         def test-alias [] {
             alias greet = echo hello;
-            (unalias greet);
+            (hide greet);
             greet
         };
         test-alias
@@ -1208,22 +1206,20 @@ fn unalias_does_not_escape_scope() {
     assert_eq!(actual.out, "hello");
 }
 
-// FIXME: no current way to hide aliases
-#[ignore]
 #[test]
-fn unalias_hides_alias() {
+fn hide_alias_hides_alias() {
     let actual = nu!(cwd: ".", pipeline(
         r#"
         def test-alias [] {
             alias ll = ls -l;
-            unalias ll;
+            hide ll;
             ll
         };
         test-alias
         "#)
     );
 
-    assert!(actual.err.contains("not found"));
+    assert!(actual.err.contains("did you mean"));
 }
 
 mod parse {