From 07ac3c3aabe9d9ad5479235ddc0642d5af9c6027 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= <kubouch@gmail.com>
Date: Sun, 8 May 2022 16:09:39 +0300
Subject: [PATCH] Add Nushell REPL simulator; Fix bug in overlay add (#5478)

* Add Nushell REPL simulator; Fix bug in overlay add

The `nu_repl` function takes an array of strings and processes them as
if they were REPL lines entered one by one. This helps to discover bugs
due to the state changes between the parse and eval stages.

* Fix REPL tests on Windows
---
 crates/nu-cli/src/util.rs                     |   4 +-
 .../src/core_commands/overlay/add.rs          |  40 +-
 tests/main.rs                                 |   1 +
 tests/nu_repl/mod.rs                          |  96 +++++
 tests/overlays/mod.rs                         | 400 ++++++++++--------
 5 files changed, 354 insertions(+), 187 deletions(-)
 create mode 100644 tests/nu_repl/mod.rs

diff --git a/crates/nu-cli/src/util.rs b/crates/nu-cli/src/util.rs
index 7a478c67cf..031413a0b6 100644
--- a/crates/nu-cli/src/util.rs
+++ b/crates/nu-cli/src/util.rs
@@ -211,8 +211,8 @@ pub fn eval_source(
         (output, working_set.render())
     };
 
-    let cwd = match nu_engine::env::current_dir_str(engine_state, stack) {
-        Ok(p) => PathBuf::from(p),
+    let cwd = match nu_engine::env::current_dir(engine_state, stack) {
+        Ok(p) => p,
         Err(e) => {
             let working_set = StateWorkingSet::new(engine_state);
             report_error(&working_set, &e);
diff --git a/crates/nu-command/src/core_commands/overlay/add.rs b/crates/nu-command/src/core_commands/overlay/add.rs
index d63d9d794c..d17f39c985 100644
--- a/crates/nu-command/src/core_commands/overlay/add.rs
+++ b/crates/nu-command/src/core_commands/overlay/add.rs
@@ -53,8 +53,46 @@ https://www.nushell.sh/book/thinking_in_nushell.html#parsing-and-evaluation-are-
 
         // TODO: This logic is duplicated in the parser.
         if stack.has_env_overlay(&name_arg.item, engine_state) {
-            stack.add_overlay(name_arg.item);
+            // Activate existing overlay
+            stack.add_overlay(name_arg.item.clone());
+
+            if let Some(module_id) = engine_state
+                .find_overlay(name_arg.item.as_bytes())
+                .map(|id| engine_state.get_overlay(id).origin)
+            {
+                if let Some(new_module_id) = engine_state.find_module(name_arg.item.as_bytes(), &[])
+                {
+                    if module_id != new_module_id {
+                        // The origin module of an overlay changed => update it
+                        let module = engine_state.get_module(new_module_id);
+
+                        for (name, block_id) in module.env_vars() {
+                            let name = if let Ok(s) = String::from_utf8(name.clone()) {
+                                s
+                            } else {
+                                return Err(ShellError::NonUtf8(name_arg.span));
+                            };
+
+                            let block = engine_state.get_block(block_id);
+
+                            let val = eval_block(
+                                engine_state,
+                                stack,
+                                block,
+                                PipelineData::new(call.head),
+                                false,
+                                true,
+                            )?
+                            .into_value(call.head);
+
+                            stack.add_env_var(name, val);
+                        }
+                    }
+                }
+            } else {
+            }
         } else {
+            // Create a new overlay from a module
             let (overlay_name, module) =
                 if let Some(module_id) = engine_state.find_module(name_arg.item.as_bytes(), &[]) {
                     (name_arg.item, engine_state.get_module(module_id))
diff --git a/tests/main.rs b/tests/main.rs
index 5931ef6478..ef20f920df 100644
--- a/tests/main.rs
+++ b/tests/main.rs
@@ -1,5 +1,6 @@
 extern crate nu_test_support;
 
+mod nu_repl;
 mod overlays;
 mod parsing;
 mod path;
diff --git a/tests/nu_repl/mod.rs b/tests/nu_repl/mod.rs
new file mode 100644
index 0000000000..7ab30b4ade
--- /dev/null
+++ b/tests/nu_repl/mod.rs
@@ -0,0 +1,96 @@
+use nu_command::create_default_context;
+use nu_engine::eval_block;
+use nu_parser::parse;
+use nu_protocol::engine::{Stack, StateDelta, StateWorkingSet};
+use nu_protocol::{PipelineData, Span, Value};
+use nu_test_support::fs::in_directory;
+use nu_test_support::Outcome;
+
+fn outcome_err(msg: String) -> Outcome {
+    Outcome {
+        out: String::new(),
+        err: msg,
+    }
+}
+
+fn outcome_ok(msg: String) -> Outcome {
+    Outcome {
+        out: msg,
+        err: String::new(),
+    }
+}
+
+pub fn nu_repl(cwd: &str, source_lines: &[&str]) -> Outcome {
+    let cwd = in_directory(cwd);
+
+    let mut engine_state = create_default_context(&cwd);
+    let mut stack = Stack::new();
+
+    stack.add_env_var(
+        "PWD".to_string(),
+        Value::String {
+            val: cwd.to_string(),
+            span: Span::test_data(),
+        },
+    );
+
+    let delta = StateDelta::new(&engine_state);
+    if let Err(err) = engine_state.merge_delta(delta, Some(&mut stack), cwd) {
+        return outcome_err(format!("{:?}", &err));
+    }
+
+    let mut last_output = String::new();
+
+    for (i, line) in source_lines.iter().enumerate() {
+        let (block, delta) = {
+            let mut working_set = StateWorkingSet::new(&engine_state);
+            let (block, err) = parse(
+                &mut working_set,
+                Some(&format!("line{}", i)),
+                line.as_bytes(),
+                false,
+                &[],
+            );
+
+            if let Some(err) = err {
+                return outcome_err(format!("{:?}", err));
+            }
+            (block, working_set.render())
+        };
+
+        let cwd = match nu_engine::env::current_dir(&engine_state, &stack) {
+            Ok(p) => p,
+            Err(e) => {
+                return outcome_err(format!("{:?}", &e));
+            }
+        };
+
+        if let Err(err) = engine_state.merge_delta(delta, Some(&mut stack), &cwd) {
+            return outcome_err(format!("{:?}", err));
+        }
+
+        let input = PipelineData::new(Span::test_data());
+        let config = engine_state.get_config();
+
+        match eval_block(&engine_state, &mut stack, &block, input, false, false) {
+            Ok(pipeline_data) => match pipeline_data.collect_string("", config) {
+                Ok(s) => last_output = s,
+                Err(err) => return outcome_err(format!("{:?}", err)),
+            },
+            Err(err) => return outcome_err(format!("{:?}", err)),
+        }
+
+        // FIXME: permanent state changes like this hopefully in time can be removed
+        // and be replaced by just passing the cwd in where needed
+        if let Some(cwd) = stack.get_env_var(&engine_state, "PWD") {
+            let path = match cwd.as_string() {
+                Ok(p) => p,
+                Err(err) => return outcome_err(format!("{:?}", err)),
+            };
+            let _ = std::env::set_current_dir(path);
+            engine_state.add_env_var("PWD".into(), cwd);
+        }
+    }
+
+    outcome_ok(last_output)
+}
diff --git a/tests/overlays/mod.rs b/tests/overlays/mod.rs
index 4e0ce6257c..fdc5966f4b 100644
--- a/tests/overlays/mod.rs
+++ b/tests/overlays/mod.rs
@@ -1,310 +1,342 @@
+use super::nu_repl::nu_repl;
 use nu_test_support::{nu, pipeline};
 
 #[test]
 fn add_overlay() {
-    let actual = nu!(
-        cwd: "tests/overlays", pipeline(
-        r#"
-            module spam { export def foo [] { "foo" } };
-            overlay add spam;
-            foo
-        "#
-    ));
+    let inp = &[
+        r#"module spam { export def foo [] { "foo" } }"#,
+        r#"overlay add spam"#,
+        r#"foo"#,
+    ];
+
+    let actual = nu!(cwd: "tests/overlays", pipeline(&inp.join("; ")));
+    let actual_repl = nu_repl("tests/overlays", inp);
 
     assert_eq!(actual.out, "foo");
+    assert_eq!(actual_repl.out, "foo");
 }
 
 #[test]
 fn add_overlay_env() {
-    let actual = nu!(
-        cwd: "tests/overlays", pipeline(
-        r#"
-            module spam { export env FOO { "foo" } };
-            overlay add spam;
-            $env.FOO
-        "#
-    ));
+    let inp = &[
+        r#"module spam { export env FOO { "foo" } }"#,
+        r#"overlay add spam"#,
+        r#"$env.FOO"#,
+    ];
+
+    let actual = nu!(cwd: "tests/overlays", pipeline(&inp.join("; ")));
+    let actual_repl = nu_repl("tests/overlays", inp);
 
     assert_eq!(actual.out, "foo");
+    assert_eq!(actual_repl.out, "foo");
 }
 
 #[test]
 fn add_overlay_from_file_decl() {
-    let actual = nu!(
-        cwd: "tests/overlays", pipeline(
-        r#"
-            overlay add samples/spam.nu;
-            foo
-        "#
-    ));
+    let inp = &[r#"overlay add samples/spam.nu"#, r#"foo"#];
+
+    let actual = nu!(cwd: "tests/overlays", pipeline(&inp.join("; ")));
+    let actual_repl = nu_repl("tests/overlays", inp);
 
     assert_eq!(actual.out, "foo");
+    assert_eq!(actual_repl.out, "foo");
+}
+
+// This one tests that the `nu_repl()` loop works correctly
+#[test]
+fn add_overlay_from_file_decl_cd() {
+    let inp = &[r#"cd samples"#, r#"overlay add spam.nu"#, r#"foo"#];
+
+    let actual_repl = nu_repl("tests/overlays", inp);
+
+    assert_eq!(actual_repl.out, "foo");
 }
 
 #[test]
 fn add_overlay_from_file_alias() {
-    let actual = nu!(
-        cwd: "tests/overlays", pipeline(
-        r#"
-            overlay add samples/spam.nu;
-            bar
-        "#
-    ));
+    let inp = &[r#"overlay add samples/spam.nu"#, r#"bar"#];
+
+    let actual = nu!(cwd: "tests/overlays", pipeline(&inp.join("; ")));
+    let actual_repl = nu_repl("tests/overlays", inp);
 
     assert_eq!(actual.out, "bar");
+    assert_eq!(actual_repl.out, "bar");
 }
 
 #[test]
 fn add_overlay_from_file_env() {
-    let actual = nu!(
-        cwd: "tests/overlays", pipeline(
-        r#"
-            overlay add samples/spam.nu;
-            $env.BAZ
-        "#
-    ));
+    let inp = &[r#"overlay add samples/spam.nu"#, r#"$env.BAZ"#];
+
+    let actual = nu!(cwd: "tests/overlays", pipeline(&inp.join("; ")));
+    let actual_repl = nu_repl("tests/overlays", inp);
 
     assert_eq!(actual.out, "baz");
+    assert_eq!(actual_repl.out, "baz");
 }
 
 #[test]
 fn add_overlay_scoped() {
-    let actual = nu!(
-        cwd: "tests/overlays", pipeline(
-        r#"
-            module spam { export def foo [] { "foo" } };
-            do { overlay add spam };
-            foo
-        "#
-    ));
+    let inp = &[
+        r#"module spam { export def foo [] { "foo" } }"#,
+        r#"do { overlay add spam }"#,
+        r#"foo"#,
+    ];
 
-    assert!(!actual.err.is_empty())
+    let actual = nu!(cwd: "tests/overlays", pipeline(&inp.join("; ")));
+    let actual_repl = nu_repl("tests/overlays", inp);
+
+    assert!(!actual.err.is_empty());
+    #[cfg(windows)]
+    assert!(actual_repl.out != "foo");
+    #[cfg(not(windows))]
+    assert!(!actual_repl.err.is_empty());
 }
 
 #[test]
 fn update_overlay_from_module() {
-    let actual = nu!(
-        cwd: "tests/overlays", pipeline(
-        r#"
-            module spam { export def foo [] { "foo" } };
-            overlay add spam;
-            module spam { export def foo [] { "bar" } };
-            overlay add spam;
-            foo
-        "#
-    ));
+    let inp = &[
+        r#"module spam { export def foo [] { "foo" } }"#,
+        r#"overlay add spam"#,
+        r#"module spam { export def foo [] { "bar" } }"#,
+        r#"overlay add spam"#,
+        r#"foo"#,
+    ];
+
+    let actual = nu!(cwd: "tests/overlays", pipeline(&inp.join("; ")));
+    let actual_repl = nu_repl("tests/overlays", inp);
 
     assert_eq!(actual.out, "bar");
+    assert_eq!(actual_repl.out, "bar");
 }
 
 #[test]
 fn update_overlay_from_module_env() {
-    let actual = nu!(
-        cwd: "tests/overlays", pipeline(
-        r#"
-            module spam { export env FOO { "foo" } };
-            overlay add spam;
-            module spam { export env FOO { "bar" } };
-            overlay add spam;
-            $env.FOO
-        "#
-    ));
+    let inp = &[
+        r#"module spam { export env FOO { "foo" } }"#,
+        r#"overlay add spam"#,
+        r#"module spam { export env FOO { "bar" } }"#,
+        r#"overlay add spam"#,
+        r#"$env.FOO"#,
+    ];
+
+    let actual = nu!(cwd: "tests/overlays", pipeline(&inp.join("; ")));
+    let actual_repl = nu_repl("tests/overlays", inp);
 
     assert_eq!(actual.out, "bar");
+    assert_eq!(actual_repl.out, "bar");
 }
 
 #[test]
 fn remove_overlay() {
-    let actual = nu!(
-        cwd: "tests/overlays", pipeline(
-        r#"
-            module spam { export def foo [] { "foo" } };
-            overlay add spam;
-            overlay remove spam;
-            foo
-        "#
-    ));
+    let inp = &[
+        r#"module spam { export def foo [] { "foo" } }"#,
+        r#"overlay add spam"#,
+        r#"overlay remove spam"#,
+        r#"foo"#,
+    ];
+
+    let actual = nu!(cwd: "tests/overlays", pipeline(&inp.join("; ")));
+    let actual_repl = nu_repl("tests/overlays", inp);
 
     assert!(!actual.err.is_empty());
+    #[cfg(windows)]
+    assert!(actual_repl.out != "foo");
+    #[cfg(not(windows))]
+    assert!(!actual_repl.err.is_empty());
 }
 
 #[test]
 fn remove_last_overlay() {
-    let actual = nu!(
-        cwd: "tests/overlays", pipeline(
-        r#"
-            module spam { export def foo [] { "foo" } };
-            overlay add spam;
-            overlay remove;
-            foo
-        "#
-    ));
+    let inp = &[
+        r#"module spam { export def foo [] { "foo" } }"#,
+        r#"overlay add spam"#,
+        r#"overlay remove"#,
+        r#"foo"#,
+    ];
+
+    let actual = nu!(cwd: "tests/overlays", pipeline(&inp.join("; ")));
+    let actual_repl = nu_repl("tests/overlays", inp);
 
     assert!(!actual.err.is_empty());
+    #[cfg(windows)]
+    assert!(actual_repl.out != "foo");
+    #[cfg(not(windows))]
+    assert!(!actual_repl.err.is_empty());
 }
 
 #[test]
 fn remove_overlay_scoped() {
-    let actual = nu!(
-        cwd: "tests/overlays", pipeline(
-        r#"
-            module spam { export def foo [] { "foo" } };
-            overlay add spam;
-            do {
-                overlay remove spam
-            };
-            foo
-        "#
-    ));
+    let inp = &[
+        r#"module spam { export def foo [] { "foo" } }"#,
+        r#"overlay add spam"#,
+        r#"do { overlay remove spam }"#,
+        r#"foo"#,
+    ];
+
+    let actual = nu!(cwd: "tests/overlays", pipeline(&inp.join("; ")));
+    let actual_repl = nu_repl("tests/overlays", inp);
 
     assert_eq!(actual.out, "foo");
+    assert_eq!(actual_repl.out, "foo");
 }
 
 #[test]
 fn remove_overlay_env() {
-    let actual = nu!(
-        cwd: "tests/overlays", pipeline(
-        r#"
-            module spam { export env FOO { "foo" } };
-            overlay add spam;
-            overlay remove spam;
-            $env.FOO
-        "#
-    ));
+    let inp = &[
+        r#"module spam { export env FOO { "foo" } }"#,
+        r#"overlay add spam"#,
+        r#"overlay remove spam"#,
+        r#"$env.FOO"#,
+    ];
+
+    let actual = nu!(cwd: "tests/overlays", pipeline(&inp.join("; ")));
+    let actual_repl = nu_repl("tests/overlays", inp);
 
     assert!(actual.err.contains("did you mean"));
+    assert!(actual_repl.err.contains("DidYouMean"));
 }
 
 #[test]
 fn remove_overlay_scoped_env() {
-    let actual = nu!(
-        cwd: "tests/overlays", pipeline(
-        r#"
-            module spam { export env FOO { "foo" } };
-            overlay add spam;
-            do {
-                overlay remove spam
-            };
-            $env.FOO
-        "#
-    ));
+    let inp = &[
+        r#"module spam { export env FOO { "foo" } }"#,
+        r#"overlay add spam"#,
+        r#"do { overlay remove spam }"#,
+        r#"$env.FOO"#,
+    ];
+
+    let actual = nu!(cwd: "tests/overlays", pipeline(&inp.join("; ")));
+    let actual_repl = nu_repl("tests/overlays", inp);
 
     assert_eq!(actual.out, "foo");
+    assert_eq!(actual_repl.out, "foo");
 }
 
 #[test]
 fn list_default_overlay() {
-    let actual = nu!(
-        cwd: "tests/overlays", pipeline(
-        r#"
-            overlay list | last
-        "#,
-    ));
+    let inp = &[r#"overlay list | last"#];
+
+    let actual = nu!(cwd: "tests/overlays", pipeline(&inp.join("; ")));
+    let actual_repl = nu_repl("tests/overlays", inp);
 
     assert_eq!(actual.out, "zero");
+    assert_eq!(actual_repl.out, "zero");
 }
 
 #[test]
 fn list_last_overlay() {
-    let actual = nu!(
-        cwd: "tests/overlays", pipeline(
-        r#"
-            module spam { export def foo [] { "foo" } };
-            overlay add spam;
-            overlay list | last
-        "#,
-    ));
+    let inp = &[
+        r#"module spam { export def foo [] { "foo" } }"#,
+        r#"overlay add spam"#,
+        r#"overlay list | last"#,
+    ];
+
+    let actual = nu!(cwd: "tests/overlays", pipeline(&inp.join("; ")));
+    let actual_repl = nu_repl("tests/overlays", inp);
 
     assert_eq!(actual.out, "spam");
+    assert_eq!(actual_repl.out, "spam");
 }
 
 #[test]
 fn list_overlay_scoped() {
-    let actual = nu!(
-        cwd: "tests/overlays", pipeline(
-        r#"
-            module spam { export def foo [] { "foo" } };
-            overlay add spam;
-            do { overlay list | last }
-        "#
-    ));
+    let inp = &[
+        r#"module spam { export def foo [] { "foo" } }"#,
+        r#"overlay add spam"#,
+        r#"do { overlay list | last }"#,
+    ];
+
+    let actual = nu!(cwd: "tests/overlays", pipeline(&inp.join("; ")));
+    let actual_repl = nu_repl("tests/overlays", inp);
 
     assert_eq!(actual.out, "spam");
+    assert_eq!(actual_repl.out, "spam");
 }
 
 #[test]
 fn remove_overlay_discard_decl() {
-    let actual = nu!(
-        cwd: "tests/overlays", pipeline(
-        r#"
-            overlay add samples/spam.nu;
-            def bagr [] { "bagr" };
-            overlay remove spam;
-            bagr
-        "#
-    ));
+    let inp = &[
+        r#"overlay add samples/spam.nu"#,
+        r#"def bagr [] { "bagr" }"#,
+        r#"overlay remove spam"#,
+        r#"bagr"#,
+    ];
+
+    let actual = nu!(cwd: "tests/overlays", pipeline(&inp.join("; ")));
+    let actual_repl = nu_repl("tests/overlays", inp);
 
     assert!(!actual.err.is_empty());
+    #[cfg(windows)]
+    assert!(actual_repl.out != "bagr");
+    #[cfg(not(windows))]
+    assert!(!actual_repl.err.is_empty());
 }
 
 #[test]
 fn remove_overlay_discard_alias() {
-    let actual = nu!(
-        cwd: "tests/overlays", pipeline(
-        r#"
-            overlay add samples/spam.nu;
-            alias bagr = "bagr";
-            overlay remove spam;
-            bagr
-        "#
-    ));
+    let inp = &[
+        r#"overlay add samples/spam.nu"#,
+        r#"alias bagr = "bagr""#,
+        r#"overlay remove spam"#,
+        r#"bagr"#,
+    ];
+
+    let actual = nu!(cwd: "tests/overlays", pipeline(&inp.join("; ")));
+    let actual_repl = nu_repl("tests/overlays", inp);
 
     assert!(!actual.err.is_empty());
+    #[cfg(windows)]
+    assert!(actual_repl.out != "bagr");
+    #[cfg(not(windows))]
+    assert!(!actual_repl.err.is_empty());
 }
 
 #[test]
 fn remove_overlay_discard_env() {
-    let actual = nu!(
-        cwd: "tests/overlays", pipeline(
-        r#"
-            overlay add samples/spam.nu;
-            let-env BAGR = "bagr";
-            overlay remove spam;
-            $env.bagr
-        "#
-    ));
+    let inp = &[
+        r#"overlay add samples/spam.nu"#,
+        r#"let-env BAGR = "bagr""#,
+        r#"overlay remove spam"#,
+        r#"$env.BAGR"#,
+    ];
+
+    let actual = nu!(cwd: "tests/overlays", pipeline(&inp.join("; ")));
+    let actual_repl = nu_repl("tests/overlays", inp);
 
     assert!(actual.err.contains("did you mean"));
+    assert!(actual_repl.err.contains("DidYouMean"));
 }
 
 #[test]
 fn preserve_overrides() {
-    let actual = nu!(
-        cwd: "tests/overlays", pipeline(
-        r#"
-            overlay add samples/spam.nu;
-            def foo [] { "new-foo" };
-            overlay remove spam;
-            overlay add spam;
-            foo
-        "#
-    ));
+    let inp = &[
+        r#"overlay add samples/spam.nu"#,
+        r#"def foo [] { "new-foo" }"#,
+        r#"overlay remove spam"#,
+        r#"overlay add spam"#,
+        r#"foo"#,
+    ];
+
+    let actual = nu!(cwd: "tests/overlays", pipeline(&inp.join("; ")));
+    let actual_repl = nu_repl("tests/overlays", inp);
 
     assert_eq!(actual.out, "new-foo");
+    assert_eq!(actual_repl.out, "new-foo");
 }
 
 #[test]
 fn reset_overrides() {
-    let actual = nu!(
-        cwd: "tests/overlays", pipeline(
-        r#"
-            overlay add samples/spam.nu;
-            def foo [] { "new-foo" };
-            overlay remove spam;
-            overlay add samples/spam.nu;
-            foo
-        "#
-    ));
+    let inp = &[
+        r#"overlay add samples/spam.nu"#,
+        r#"def foo [] { "new-foo" }"#,
+        r#"overlay remove spam"#,
+        r#"overlay add samples/spam.nu"#,
+        r#"foo"#,
+    ];
+
+    let actual = nu!(cwd: "tests/overlays", pipeline(&inp.join("; ")));
+    let actual_repl = nu_repl("tests/overlays", inp);
 
     assert_eq!(actual.out, "foo");
+    assert_eq!(actual_repl.out, "foo");
 }