Fix overlays not preserving hidden env vars (#6475)

* Fix overlays not preserving hidden env vars

* Add a few more test

* Add one more test of resetting hidden env vars

* Move removed source-env tests

crates/nu-command/src/core_commands/overlay/use_.rs

@@ -118,6 +118,7 @@ impl Command for OverlayUse {
             // Add environment variables only if:
             // a) adding a new overlay
             // b) refreshing an active overlay (the origin module changed)
+
             let module = engine_state.get_module(module_id);
 
             for (name, block_id) in module.env_vars() {

crates/nu-command/tests/commands/source_env.rs

@@ -1,4 +1,5 @@
-use nu_test_support::fs::{AbsolutePath, Stub::FileWithContent};
+use nu_test_support::fs::AbsolutePath;
+use nu_test_support::fs::Stub::{FileWithContent, FileWithContentToBeTrimmed};
 use nu_test_support::nu;
 use nu_test_support::pipeline;
 use nu_test_support::playground::Playground;
@@ -155,3 +156,152 @@ fn can_source_dynamic_path() {
         assert_eq!(actual.out, "foo");
     });
 }
+
+#[test]
+fn source_env_eval_export_env() {
+    Playground::setup("source_env_eval_export_env", |dirs, sandbox| {
+        sandbox.with_files(vec![FileWithContentToBeTrimmed(
+            "spam.nu",
+            r#"
+                export-env { let-env FOO = 'foo' }
+            "#,
+        )]);
+
+        let inp = &[r#"source-env spam.nu"#, r#"$env.FOO"#];
+
+        let actual = nu!(cwd: dirs.test(), pipeline(&inp.join("; ")));
+
+        assert_eq!(actual.out, "foo");
+    })
+}
+
+#[test]
+fn source_env_eval_export_env_hide() {
+    Playground::setup("source_env_eval_export_env", |dirs, sandbox| {
+        sandbox.with_files(vec![FileWithContentToBeTrimmed(
+            "spam.nu",
+            r#"
+                export-env { hide-env FOO }
+            "#,
+        )]);
+
+        let inp = &[
+            r#"let-env FOO = 'foo'"#,
+            r#"source-env spam.nu"#,
+            r#"$env.FOO"#,
+        ];
+
+        let actual = nu!(cwd: dirs.test(), pipeline(&inp.join("; ")));
+
+        assert!(actual.err.contains("did you mean"));
+    })
+}
+
+#[test]
+fn source_env_do_cd() {
+    Playground::setup("source_env_do_cd", |dirs, sandbox| {
+        sandbox
+            .mkdir("test1/test2")
+            .with_files(vec![FileWithContentToBeTrimmed(
+                "test1/test2/spam.nu",
+                r#"
+                    cd test1/test2
+                "#,
+            )]);
+
+        let inp = &[
+            r#"source-env test1/test2/spam.nu"#,
+            r#"$env.PWD | path basename"#,
+        ];
+
+        let actual = nu!(cwd: dirs.test(), pipeline(&inp.join("; ")));
+
+        assert_eq!(actual.out, "test2");
+    })
+}
+
+#[test]
+fn source_env_do_cd_file_relative() {
+    Playground::setup("source_env_do_cd_file_relative", |dirs, sandbox| {
+        sandbox
+            .mkdir("test1/test2")
+            .with_files(vec![FileWithContentToBeTrimmed(
+                "test1/test2/spam.nu",
+                r#"
+                    cd ($env.FILE_PWD | path join '..')
+                "#,
+            )]);
+
+        let inp = &[
+            r#"source-env test1/test2/spam.nu"#,
+            r#"$env.PWD | path basename"#,
+        ];
+
+        let actual = nu!(cwd: dirs.test(), pipeline(&inp.join("; ")));
+
+        assert_eq!(actual.out, "test1");
+    })
+}
+
+#[test]
+fn source_env_dont_cd_overlay() {
+    Playground::setup("source_env_dont_cd_overlay", |dirs, sandbox| {
+        sandbox
+            .mkdir("test1/test2")
+            .with_files(vec![FileWithContentToBeTrimmed(
+                "test1/test2/spam.nu",
+                r#"
+                    overlay new spam
+                    cd test1/test2
+                    overlay hide spam
+                "#,
+            )]);
+
+        let inp = &[
+            r#"source-env test1/test2/spam.nu"#,
+            r#"$env.PWD | path basename"#,
+        ];
+
+        let actual = nu!(cwd: dirs.test(), pipeline(&inp.join("; ")));
+
+        assert_eq!(actual.out, "source_env_dont_cd_overlay");
+    })
+}
+
+#[test]
+fn source_env_nice_parse_error() {
+    Playground::setup("source_env_nice_parse_error", |dirs, sandbox| {
+        sandbox.with_files(vec![FileWithContentToBeTrimmed(
+            "spam.nu",
+            r#"
+                let x
+            "#,
+        )]);
+
+        let inp = &[r#"source-env spam.nu"#];
+
+        let actual = nu!(cwd: dirs.test(), pipeline(&inp.join("; ")));
+
+        assert!(actual.err.contains("cannot parse this file"));
+        assert!(actual.err.contains("───"));
+    })
+}
+
+#[test]
+fn source_env_nice_shell_error() {
+    Playground::setup("source_env_nice_shell_error", |dirs, sandbox| {
+        sandbox.with_files(vec![FileWithContentToBeTrimmed(
+            "spam.nu",
+            r#"
+                let-env FILE_PWD = 'foo'
+            "#,
+        )]);
+
+        let inp = &[r#"source-env spam.nu"#];
+
+        let actual = nu!(cwd: dirs.test(), pipeline(&inp.join("; ")));
+
+        assert!(actual.err.contains("cannot evaluate this file"));
+        assert!(actual.err.contains("───"));
+    })
+}

crates/nu-protocol/src/engine/stack.rs

@@ -379,8 +379,6 @@ impl Stack {
     }
 
     pub fn add_overlay(&mut self, name: String) {
-        self.env_hidden.remove(&name);
-
         self.active_overlays.retain(|o| o != &name);
         self.active_overlays.push(name);
     }

tests/overlays/mod.rs

@@ -878,3 +878,103 @@ fn overlay_use_find_scoped_module() {
         assert_eq!(actual.out, "spam");
     })
 }
+
+#[test]
+fn overlay_preserve_hidden_env_1() {
+    let inp = &[
+        r#"overlay new spam"#,
+        r#"let-env FOO = 'foo'"#,
+        r#"overlay new eggs"#,
+        r#"let-env FOO = 'bar'"#,
+        r#"hide-env FOO"#,
+        r#"overlay use eggs"#,
+        r#"$env.FOO"#,
+    ];
+
+    let actual = nu!(cwd: "tests/overlays", pipeline(&inp.join("; ")));
+    let actual_repl = nu!(cwd: "tests/overlays", nu_repl_code(inp));
+
+    assert_eq!(actual.out, "foo");
+    assert_eq!(actual_repl.out, "foo");
+}
+
+#[test]
+fn overlay_preserve_hidden_env_2() {
+    let inp = &[
+        r#"overlay new spam"#,
+        r#"let-env FOO = 'foo'"#,
+        r#"overlay hide spam"#,
+        r#"overlay new eggs"#,
+        r#"let-env FOO = 'bar'"#,
+        r#"hide-env FOO"#,
+        r#"overlay hide eggs"#,
+        r#"overlay use spam"#,
+        r#"overlay use eggs"#,
+        r#"$env.FOO"#,
+    ];
+
+    let actual = nu!(cwd: "tests/overlays", pipeline(&inp.join("; ")));
+    let actual_repl = nu!(cwd: "tests/overlays", nu_repl_code(inp));
+
+    assert_eq!(actual.out, "foo");
+    assert_eq!(actual_repl.out, "foo");
+}
+
+#[test]
+fn overlay_reset_hidden_env() {
+    let inp = &[
+        r#"overlay new spam"#,
+        r#"let-env FOO = 'foo'"#,
+        r#"overlay new eggs"#,
+        r#"let-env FOO = 'bar'"#,
+        r#"hide-env FOO"#,
+        r#"module eggs { export-env { let-env FOO = 'bar' } }"#,
+        r#"overlay use eggs"#,
+        r#"$env.FOO"#,
+    ];
+
+    let actual = nu!(cwd: "tests/overlays", pipeline(&inp.join("; ")));
+    let actual_repl = nu!(cwd: "tests/overlays", nu_repl_code(inp));
+
+    assert_eq!(actual.out, "bar");
+    assert_eq!(actual_repl.out, "bar");
+}
+
+#[ignore = "TODO: For this to work, we'd need to make predecls respect overlays"]
+#[test]
+fn overlay_preserve_hidden_decl() {
+    let inp = &[
+        r#"overlay new spam"#,
+        r#"def foo [] { 'foo' }"#,
+        r#"overlay new eggs"#,
+        r#"def foo [] { 'bar' }"#,
+        r#"hide foo"#,
+        r#"overlay use eggs"#,
+        r#"foo"#,
+    ];
+
+    let actual = nu!(cwd: "tests/overlays", pipeline(&inp.join("; ")));
+    let actual_repl = nu!(cwd: "tests/overlays", nu_repl_code(inp));
+
+    assert_eq!(actual.out, "foo");
+    assert_eq!(actual_repl.out, "foo");
+}
+
+#[test]
+fn overlay_preserve_hidden_alias() {
+    let inp = &[
+        r#"overlay new spam"#,
+        r#"alias foo = 'foo'"#,
+        r#"overlay new eggs"#,
+        r#"alias foo = 'bar'"#,
+        r#"hide foo"#,
+        r#"overlay use eggs"#,
+        r#"foo"#,
+    ];
+
+    let actual = nu!(cwd: "tests/overlays", pipeline(&inp.join("; ")));
+    let actual_repl = nu!(cwd: "tests/overlays", nu_repl_code(inp));
+
+    assert_eq!(actual.out, "foo");
+    assert_eq!(actual_repl.out, "foo");
+}

tests/shell/environment/mod.rs

@@ -1,5 +1,4 @@
 mod env;
-mod source_env;
 
 // FIXME: nu_env tests depend on autoenv which hasn't been ported yet
 // mod nu_env;

tests/shell/environment/source_env.rs

@@ -1,152 +0,0 @@
-use nu_test_support::fs::Stub::FileWithContentToBeTrimmed;
-use nu_test_support::playground::Playground;
-use nu_test_support::{nu, pipeline};
-
-#[test]
-fn source_env_eval_export_env() {
-    Playground::setup("source_env_eval_export_env", |dirs, sandbox| {
-        sandbox.with_files(vec![FileWithContentToBeTrimmed(
-            "spam.nu",
-            r#"
-                export-env { let-env FOO = 'foo' }
-            "#,
-        )]);
-
-        let inp = &[r#"source-env spam.nu"#, r#"$env.FOO"#];
-
-        let actual = nu!(cwd: dirs.test(), pipeline(&inp.join("; ")));
-
-        assert_eq!(actual.out, "foo");
-    })
-}
-
-#[test]
-fn source_env_eval_export_env_hide() {
-    Playground::setup("source_env_eval_export_env", |dirs, sandbox| {
-        sandbox.with_files(vec![FileWithContentToBeTrimmed(
-            "spam.nu",
-            r#"
-                export-env { hide-env FOO }
-            "#,
-        )]);
-
-        let inp = &[
-            r#"let-env FOO = 'foo'"#,
-            r#"source-env spam.nu"#,
-            r#"$env.FOO"#,
-        ];
-
-        let actual = nu!(cwd: dirs.test(), pipeline(&inp.join("; ")));
-
-        assert!(actual.err.contains("did you mean"));
-    })
-}
-
-#[test]
-fn source_env_do_cd() {
-    Playground::setup("source_env_do_cd", |dirs, sandbox| {
-        sandbox
-            .mkdir("test1/test2")
-            .with_files(vec![FileWithContentToBeTrimmed(
-                "test1/test2/spam.nu",
-                r#"
-                    cd test1/test2
-                "#,
-            )]);
-
-        let inp = &[
-            r#"source-env test1/test2/spam.nu"#,
-            r#"$env.PWD | path basename"#,
-        ];
-
-        let actual = nu!(cwd: dirs.test(), pipeline(&inp.join("; ")));
-
-        assert_eq!(actual.out, "test2");
-    })
-}
-
-#[test]
-fn source_env_do_cd_file_relative() {
-    Playground::setup("source_env_do_cd_file_relative", |dirs, sandbox| {
-        sandbox
-            .mkdir("test1/test2")
-            .with_files(vec![FileWithContentToBeTrimmed(
-                "test1/test2/spam.nu",
-                r#"
-                    cd ($env.FILE_PWD | path join '..')
-                "#,
-            )]);
-
-        let inp = &[
-            r#"source-env test1/test2/spam.nu"#,
-            r#"$env.PWD | path basename"#,
-        ];
-
-        let actual = nu!(cwd: dirs.test(), pipeline(&inp.join("; ")));
-
-        assert_eq!(actual.out, "test1");
-    })
-}
-
-#[test]
-fn source_env_dont_cd_overlay() {
-    Playground::setup("source_env_dont_cd_overlay", |dirs, sandbox| {
-        sandbox
-            .mkdir("test1/test2")
-            .with_files(vec![FileWithContentToBeTrimmed(
-                "test1/test2/spam.nu",
-                r#"
-                    overlay new spam
-                    cd test1/test2
-                    overlay hide spam
-                "#,
-            )]);
-
-        let inp = &[
-            r#"source-env test1/test2/spam.nu"#,
-            r#"$env.PWD | path basename"#,
-        ];
-
-        let actual = nu!(cwd: dirs.test(), pipeline(&inp.join("; ")));
-
-        assert_eq!(actual.out, "source_env_dont_cd_overlay");
-    })
-}
-
-#[test]
-fn source_env_nice_parse_error() {
-    Playground::setup("source_env_nice_parse_error", |dirs, sandbox| {
-        sandbox.with_files(vec![FileWithContentToBeTrimmed(
-            "spam.nu",
-            r#"
-                let x
-            "#,
-        )]);
-
-        let inp = &[r#"source-env spam.nu"#];
-
-        let actual = nu!(cwd: dirs.test(), pipeline(&inp.join("; ")));
-
-        assert!(actual.err.contains("cannot parse this file"));
-        assert!(actual.err.contains("───"));
-    })
-}
-
-#[test]
-fn source_env_nice_shell_error() {
-    Playground::setup("source_env_nice_shell_error", |dirs, sandbox| {
-        sandbox.with_files(vec![FileWithContentToBeTrimmed(
-            "spam.nu",
-            r#"
-                let-env FILE_PWD = 'foo'
-            "#,
-        )]);
-
-        let inp = &[r#"source-env spam.nu"#];
-
-        let actual = nu!(cwd: dirs.test(), pipeline(&inp.join("; ")));
-
-        assert!(actual.err.contains("cannot evaluate this file"));
-        assert!(actual.err.contains("───"));
-    })
-}