From 477f3be8df341e970998a47fe93bdf4e517da200 Mon Sep 17 00:00:00 2001
From: JT <547158+jntrnr@users.noreply.github.com>
Date: Tue, 8 Mar 2022 07:45:47 -0500
Subject: [PATCH] Make `if` blocks work like a def-env (#4656)

* Make `if` work like a def-env

* Add some tests

* Add an example
---
 crates/nu-command/src/core_commands/if_.rs | 71 +++++++++++++++++-----
 src/tests/test_engine.rs                   | 13 ++++
 2 files changed, 70 insertions(+), 14 deletions(-)

diff --git a/crates/nu-command/src/core_commands/if_.rs b/crates/nu-command/src/core_commands/if_.rs
index 5b0fe0b298..9eda4ba6ae 100644
--- a/crates/nu-command/src/core_commands/if_.rs
+++ b/crates/nu-command/src/core_commands/if_.rs
@@ -37,50 +37,88 @@ impl Command for If {
     fn run(
         &self,
         engine_state: &EngineState,
-        stack: &mut Stack,
+        caller_stack: &mut Stack,
         call: &Call,
         input: PipelineData,
     ) -> Result<nu_protocol::PipelineData, nu_protocol::ShellError> {
         let cond = &call.positional[0];
-        let then_block: CaptureBlock = call.req(engine_state, stack, 1)?;
+        let then_block: CaptureBlock = call.req(engine_state, caller_stack, 1)?;
         let else_case = call.positional.get(2);
 
-        let result = eval_expression(engine_state, stack, cond)?;
+        let result = eval_expression(engine_state, caller_stack, cond)?;
         match &result {
             Value::Bool { val, .. } => {
                 if *val {
                     let block = engine_state.get_block(then_block.block_id);
-                    let mut stack = stack.captures_to_stack(&then_block.captures);
-                    eval_block(
+                    let mut callee_stack = caller_stack.captures_to_stack(&then_block.captures);
+                    let result = eval_block(
                         engine_state,
-                        &mut stack,
+                        &mut callee_stack,
                         block,
                         input,
                         call.redirect_stdout,
                         call.redirect_stderr,
-                    )
+                    );
+                    let caller_env_vars = caller_stack.get_env_var_names(engine_state);
+
+                    // remove env vars that are present in the caller but not in the callee
+                    // (the callee hid them)
+                    for var in caller_env_vars.iter() {
+                        if !callee_stack.has_env_var(engine_state, var) {
+                            caller_stack.remove_env_var(engine_state, var);
+                        }
+                    }
+
+                    // add new env vars from callee to caller
+                    for env_vars in callee_stack.env_vars {
+                        for (var, value) in env_vars {
+                            caller_stack.add_env_var(var, value);
+                        }
+                    }
+
+                    result
                 } else if let Some(else_case) = else_case {
                     if let Some(else_expr) = else_case.as_keyword() {
                         if let Some(block_id) = else_expr.as_block() {
-                            let result = eval_expression(engine_state, stack, else_expr)?;
+                            let result = eval_expression(engine_state, caller_stack, else_expr)?;
                             let else_block: CaptureBlock = FromValue::from_value(&result)?;
 
-                            let mut stack = stack.captures_to_stack(&else_block.captures);
+                            let mut callee_stack =
+                                caller_stack.captures_to_stack(&else_block.captures);
                             let block = engine_state.get_block(block_id);
-                            eval_block(
+                            let result = eval_block(
                                 engine_state,
-                                &mut stack,
+                                &mut callee_stack,
                                 block,
                                 input,
                                 call.redirect_stdout,
                                 call.redirect_stderr,
-                            )
+                            );
+
+                            let caller_env_vars = caller_stack.get_env_var_names(engine_state);
+
+                            // remove env vars that are present in the caller but not in the callee
+                            // (the callee hid them)
+                            for var in caller_env_vars.iter() {
+                                if !callee_stack.has_env_var(engine_state, var) {
+                                    caller_stack.remove_env_var(engine_state, var);
+                                }
+                            }
+
+                            // add new env vars from callee to caller
+                            for env_vars in callee_stack.env_vars {
+                                for (var, value) in env_vars {
+                                    caller_stack.add_env_var(var, value);
+                                }
+                            }
+
+                            result
                         } else {
-                            eval_expression(engine_state, stack, else_expr)
+                            eval_expression(engine_state, caller_stack, else_expr)
                                 .map(|x| x.into_pipeline_data())
                         }
                     } else {
-                        eval_expression(engine_state, stack, else_case)
+                        eval_expression(engine_state, caller_stack, else_case)
                             .map(|x| x.into_pipeline_data())
                     }
                 } else {
@@ -112,6 +150,11 @@ impl Command for If {
                 example: "if 5 < 3 { 'yes!' } else if 4 < 5 { 'no!' } else { 'okay!' }",
                 result: Some(Value::test_string("no!")),
             },
+            Example {
+                description: "Update the environment based on a condition",
+                example: r#"if true { let-env ENV_VAR = "new value" }"#,
+                result: None,
+            },
         ]
     }
 }
diff --git a/src/tests/test_engine.rs b/src/tests/test_engine.rs
index db9ca46bd8..5ff72ad43c 100644
--- a/src/tests/test_engine.rs
+++ b/src/tests/test_engine.rs
@@ -350,3 +350,16 @@ fn default_value11() -> TestResult {
 fn default_value12() -> TestResult {
     fail_test(r#"def foo [--x:int = "a"] { $x }"#, "default value not int")
 }
+
+#[test]
+fn def_env_if() -> TestResult {
+    run_test(r#"if true { let-env FOO = "QUX" }; $env.FOO"#, "QUX")
+}
+
+#[test]
+fn def_env_if2() -> TestResult {
+    run_test(
+        r#"if false { let-env FOO = "QUX" } else { let-env FOO = "QUUX" }; $env.FOO"#,
+        "QUUX",
+    )
+}