Convert 'for' to a statement (#7086)

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

@@ -1,10 +1,7 @@
 use nu_engine::{eval_block, eval_expression, CallExt};
 use nu_protocol::ast::Call;
-use nu_protocol::engine::{Closure, Command, EngineState, Stack};
+use nu_protocol::engine::{Block, Command, EngineState, Stack};
-use nu_protocol::{
+use nu_protocol::{Category, Example, ListStream, PipelineData, Signature, SyntaxShape, Value};
-    Category, Example, IntoInterruptiblePipelineData, ListStream, PipelineData, Signature, Span,
-    SyntaxShape, Type, Value,
-};
 
 #[derive(Clone)]
 pub struct For;
@@ -20,7 +17,6 @@ impl Command for For {
 
     fn signature(&self) -> nu_protocol::Signature {
         Signature::build("for")
-            .input_output_types(vec![(Type::Nothing, Type::List(Box::new(Type::Any)))])
             .required(
                 "var_name",
                 SyntaxShape::VarWithOptType,
@@ -71,72 +67,22 @@ impl Command for For {
             .expect("internal error: missing keyword");
         let values = eval_expression(engine_state, stack, keyword_expr)?;
 
-        let capture_block: Closure = call.req(engine_state, stack, 2)?;
+        let block: Block = call.req(engine_state, stack, 2)?;
 
         let numbered = call.has_flag("numbered");
 
         let ctrlc = engine_state.ctrlc.clone();
         let engine_state = engine_state.clone();
-        let block = engine_state.get_block(capture_block.block_id).clone();
+        let block = engine_state.get_block(block.block_id).clone();
-        let mut stack = stack.captures_to_stack(&capture_block.captures);
-        let orig_env_vars = stack.env_vars.clone();
-        let orig_env_hidden = stack.env_hidden.clone();
         let redirect_stdout = call.redirect_stdout;
         let redirect_stderr = call.redirect_stderr;
 
         match values {
             Value::List { vals, .. } => {
-                Ok(ListStream::from_stream(vals.into_iter(), ctrlc.clone())
+                for (idx, x) in ListStream::from_stream(vals.into_iter(), ctrlc).enumerate() {
-                    .enumerate()
-                    .map(move |(idx, x)| {
-                        // with_env() is used here to ensure that each iteration uses
-                        // a different set of environment variables.
-                        // Hence, a 'cd' in the first loop won't affect the next loop.
-                        stack.with_env(&orig_env_vars, &orig_env_hidden);
-
-                        stack.add_var(
-                            var_id,
-                            if numbered {
-                                Value::Record {
-                                    cols: vec!["index".into(), "item".into()],
-                                    vals: vec![
-                                        Value::Int {
-                                            val: idx as i64,
-                                            span: head,
-                                        },
-                                        x,
-                                    ],
-                                    span: head,
-                                }
-                            } else {
-                                x
-                            },
-                        );
-
-                        //let block = engine_state.get_block(block_id);
-                        match eval_block(
-                            &engine_state,
-                            &mut stack,
-                            &block,
-                            PipelineData::new(head),
-                            redirect_stdout,
-                            redirect_stderr,
-                        ) {
-                            Ok(pipeline_data) => pipeline_data.into_value(head),
-                            Err(error) => Value::Error { error },
-                        }
-                    })
-                    .filter(|x| !x.is_nothing())
-                    .into_pipeline_data(ctrlc))
-            }
-            Value::Range { val, .. } => Ok(val
-                .into_range_iter(ctrlc.clone())?
-                .enumerate()
-                .map(move |(idx, x)| {
                     // with_env() is used here to ensure that each iteration uses
                     // a different set of environment variables.
                     // Hence, a 'cd' in the first loop won't affect the next loop.
-                    stack.with_env(&orig_env_vars, &orig_env_hidden);
 
                     stack.add_var(
                         var_id,
@@ -158,78 +104,84 @@ impl Command for For {
                     );
 
                     //let block = engine_state.get_block(block_id);
-                    match eval_block(
+                    eval_block(
                         &engine_state,
-                        &mut stack,
+                        stack,
                         &block,
                         PipelineData::new(head),
                         redirect_stdout,
                         redirect_stderr,
-                    ) {
+                    )?
-                        Ok(pipeline_data) => pipeline_data.into_value(head),
+                    .into_value(head);
-                        Err(error) => Value::Error { error },
+                }
-                    }
+            }
-                })
+            Value::Range { val, .. } => {
-                .filter(|x| !x.is_nothing())
+                for (idx, x) in val.into_range_iter(ctrlc)?.enumerate() {
-                .into_pipeline_data(ctrlc)),
+                    stack.add_var(
+                        var_id,
+                        if numbered {
+                            Value::Record {
+                                cols: vec!["index".into(), "item".into()],
+                                vals: vec![
+                                    Value::Int {
+                                        val: idx as i64,
+                                        span: head,
+                                    },
+                                    x,
+                                ],
+                                span: head,
+                            }
+                        } else {
+                            x
+                        },
+                    );
+
+                    //let block = engine_state.get_block(block_id);
+                    eval_block(
+                        &engine_state,
+                        stack,
+                        &block,
+                        PipelineData::new(head),
+                        redirect_stdout,
+                        redirect_stderr,
+                    )?
+                    .into_value(head);
+                }
+            }
             x => {
                 stack.add_var(var_id, x);
 
                 eval_block(
                     &engine_state,
-                    &mut stack,
+                    stack,
                     &block,
                     PipelineData::new(head),
                     redirect_stdout,
                     redirect_stderr,
-                )
+                )?
+                .into_value(head);
             }
         }
+        Ok(PipelineData::new(head))
     }
 
     fn examples(&self) -> Vec<Example> {
-        let span = Span::test_data();
         vec![
             Example {
                 description: "Echo the square of each integer",
-                example: "for x in [1 2 3] { $x * $x }",
+                example: "for x in [1 2 3] { print ($x * $x) }",
-                result: Some(Value::List {
+                result: None,
-                    vals: vec![
-                        Value::Int { val: 1, span },
-                        Value::Int { val: 4, span },
-                        Value::Int { val: 9, span },
-                    ],
-                    span,
-                }),
             },
             Example {
                 description: "Work with elements of a range",
-                example: "for $x in 1..3 { $x }",
+                example: "for $x in 1..3 { print $x }",
-                result: Some(Value::List {
+                result: None,
-                    vals: vec![
-                        Value::Int { val: 1, span },
-                        Value::Int { val: 2, span },
-                        Value::Int { val: 3, span },
-                    ],
-                    span,
-                }),
             },
             Example {
                 description: "Number each item and echo a message",
-                example: "for $it in ['bob' 'fred'] --numbered { $\"($it.index) is ($it.item)\" }",
+                example:
-                result: Some(Value::List {
+                    "for $it in ['bob' 'fred'] --numbered { print $\"($it.index) is ($it.item)\" }",
-                    vals: vec![
+                result: None,
-                        Value::String {
-                            val: "0 is bob".into(),
-                            span,
-                        },
-                        Value::String {
-                            val: "1 is fred".into(),
-                            span,
-                        },
-                    ],
-                    span,
-                }),
             },
         ]
     }

crates/nu-parser/src/parser.rs

@@ -4732,6 +4732,16 @@ pub fn parse_expression(
                     spans[0],
                 )),
             ),
+            b"for" => (
+                parse_call(
+                    working_set,
+                    &spans[pos..],
+                    spans[0],
+                    expand_aliases_denylist,
+                )
+                .0,
+                Some(ParseError::BuiltinCommandInPipeline("for".into(), spans[0])),
+            ),
             b"let" => (
                 parse_call(
                     working_set,
@@ -4866,7 +4876,6 @@ pub fn parse_expression(
                 )),
             ),
 
-            b"for" => parse_for(working_set, spans, expand_aliases_denylist),
             _ => parse_call(
                 working_set,
                 &spans[pos..],

src/tests/test_engine.rs

@@ -128,11 +128,6 @@ fn proper_variable_captures_with_nesting() -> TestResult {
     )
 }
 
-#[test]
-fn proper_variable_for() -> TestResult {
-    run_test(r#"for x in 1..3 { if $x == 2 { "bob" } } | get 0"#, "bob")
-}
-
 #[test]
 fn divide_duration() -> TestResult {
     run_test(r#"4ms / 4ms"#, "1")

src/tests/test_iteration.rs

@@ -32,11 +32,6 @@ fn row_condition2() -> TestResult {
     )
 }
 
-#[test]
-fn for_loops() -> TestResult {
-    run_test(r#"(for x in [1, 2, 3] { $x + 10 }).1"#, "12")
-}
-
 #[test]
 fn par_each() -> TestResult {
     run_test(

tests/shell/pipeline/commands/internal.rs

@@ -83,6 +83,18 @@ fn argument_subexpression() {
     assert_eq!(actual.out, "foo");
 }
 
+#[test]
+fn for_loop() {
+    let actual = nu!(
+        cwd: ".",
+        r#"
+            for i in 1..3 { print $i }
+        "#
+    );
+
+    assert_eq!(actual.out, "123");
+}
+
 #[test]
 fn subexpression_handles_dot() {
     Playground::setup("subexpression_handles_dot", |dirs, sandbox| {