From 7add38fe323996540ccd339ad2e4eb9dd33f8ec8 Mon Sep 17 00:00:00 2001
From: Wind <WindSoilder@outlook.com>
Date: Thu, 24 Apr 2025 19:47:04 +0800
Subject: [PATCH] IR: allow subexpression with redirection. (#15617)

# Description
Try to fixes https://github.com/nushell/nushell/issues/15326 in another
way.

The main point of this change is to avoid duplicate `write` and `close`
a redirected file. So during compile, if compiler know current element
is a sub-expression(defined by private `is_subexpression` function), it
will no longer invoke `finish_redirection`.

In this way, we can avoid duplicate `finish_redirection`.

# User-Facing Changes
`(^echo aa) o> /tmp/aaa` will no longer raise an error.

Here is the IR after the pr:
```
# 3 registers, 12 instructions, 11 bytes of data
# 1 file used for redirection
   0: load-literal           %1, string("aaa")
   1: open-file              file(0), %1, append = false
   2: load-literal           %1, glob-pattern("echo", no_expand = false)
   3: load-literal           %2, glob-pattern("true", no_expand = false)
   4: push-positional        %1
   5: push-positional        %2
   6: redirect-out           file(0)
   7: redirect-err           caller
   8: call                   decl 135 "run-external", %0
   9: write-file             file(0), %0
  10: close-file             file(0)
  11: return                 %0
```

# Tests + Formatting
Added 3 tests.

# After Submitting
Maybe need to update doc
https://github.com/nushell/nushell.github.io/pull/1876

---------

Co-authored-by: Stefan Holderbach <sholderbach@users.noreply.github.com>
---
 .../nu-command/tests/commands/redirection.rs  | 15 ++++++++++++++
 crates/nu-engine/src/compile/mod.rs           | 20 ++++++++++++++++---
 2 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/crates/nu-command/tests/commands/redirection.rs b/crates/nu-command/tests/commands/redirection.rs
index 02f85a90f8..c263f27d83 100644
--- a/crates/nu-command/tests/commands/redirection.rs
+++ b/crates/nu-command/tests/commands/redirection.rs
@@ -473,3 +473,18 @@ fn pipe_redirection_in_let_and_mut(
     );
     assert_eq!(actual.out, output);
 }
+
+#[rstest::rstest]
+#[case("o>", "bar")]
+#[case("e>", "")]
+#[case("o+e>", "bar\nbaz")] // in subexpression, the stderr is go to the terminal
+fn subexpression_redirection(#[case] redir: &str, #[case] stdout_file_body: &str) {
+    Playground::setup("file redirection with subexpression", |dirs, _| {
+        let actual = nu!(
+            cwd: dirs.test(),
+            format!("$env.BAR = 'bar'; $env.BAZ = 'baz'; (nu --testbin echo_env_mixed out-err BAR BAZ) {redir} result.txt")
+        );
+        assert!(actual.status.success());
+        assert!(file_contents(dirs.test().join("result.txt")).contains(stdout_file_body));
+    })
+}
diff --git a/crates/nu-engine/src/compile/mod.rs b/crates/nu-engine/src/compile/mod.rs
index 1bdc2c6b41..f68232f9cb 100644
--- a/crates/nu-engine/src/compile/mod.rs
+++ b/crates/nu-engine/src/compile/mod.rs
@@ -1,5 +1,5 @@
 use nu_protocol::{
-    ast::{Block, Pipeline, PipelineRedirection, RedirectionSource, RedirectionTarget},
+    ast::{Block, Expr, Pipeline, PipelineRedirection, RedirectionSource, RedirectionTarget},
     engine::StateWorkingSet,
     ir::{Instruction, IrBlock, RedirectMode},
     CompileError, IntoSpanned, RegId, Span,
@@ -194,11 +194,25 @@ fn compile_pipeline(
             out_reg,
         )?;
 
-        // Clean up the redirection
-        finish_redirection(builder, redirect_modes, out_reg)?;
+        // only clean up the redirection if current element is not
+        // a subexpression.  The subexpression itself already clean it.
+        if !is_subexpression(&element.expr.expr) {
+            // Clean up the redirection
+            finish_redirection(builder, redirect_modes, out_reg)?;
+        }
 
         // The next pipeline element takes input from this output
         in_reg = Some(out_reg);
     }
     Ok(())
 }
+
+fn is_subexpression(expr: &Expr) -> bool {
+    match expr {
+        Expr::FullCellPath(inner) => {
+            matches!(&inner.head.expr, &Expr::Subexpression(..))
+        }
+        Expr::Subexpression(..) => true,
+        _ => false,
+    }
+}