From f08c055198c7bee56d516cda456b64b94145b035 Mon Sep 17 00:00:00 2001
From: reuben olinsky <reubeno@users.noreply.github.com>
Date: Tue, 21 Jan 2025 10:19:38 -0800
Subject: [PATCH] fix(for): correct semantics for "for" without "in"

---
 brush-core/src/interp.rs                      | 68 +++++++++++--------
 .../tests/cases/compound_cmds/for.yaml        | 26 +++++++
 2 files changed, 64 insertions(+), 30 deletions(-)

diff --git a/brush-core/src/interp.rs b/brush-core/src/interp.rs
index 8e8df574..45e17c78 100644
--- a/brush-core/src/interp.rs
+++ b/brush-core/src/interp.rs
@@ -555,54 +555,62 @@ impl Execute for ast::ForClauseCommand {
     ) -> Result<ExecutionResult, error::Error> {
         let mut result = ExecutionResult::success();
 
+        // If we were given explicit words to iterate over, then expand them all, with splitting
+        // enabled.
+        let mut expanded_values = vec![];
         if let Some(unexpanded_values) = &self.values {
-            // Expand all values, with splitting enabled.
-            let mut expanded_values = vec![];
             for value in unexpanded_values {
                 let mut expanded = expansion::full_expand_and_split_word(shell, value).await?;
                 expanded_values.append(&mut expanded);
             }
+        } else {
+            // Otherwise, we use the current positional parameters.
+            expanded_values.extend_from_slice(&shell.positional_parameters);
+        }
 
-            for value in expanded_values {
-                if shell.options.print_commands_and_arguments {
+        for value in expanded_values {
+            if shell.options.print_commands_and_arguments {
+                if let Some(unexpanded_values) = &self.values {
                     shell.trace_command(std::format!(
                         "for {} in {}",
                         self.variable_name,
                         unexpanded_values.iter().join(" ")
                     ))?;
+                } else {
+                    shell.trace_command(std::format!("for {}", self.variable_name,))?;
                 }
+            }
 
-                // Update the variable.
-                shell.env.update_or_add(
-                    &self.variable_name,
-                    ShellValueLiteral::Scalar(value),
-                    |_| Ok(()),
-                    EnvironmentLookup::Anywhere,
-                    EnvironmentScope::Global,
-                )?;
+            // Update the variable.
+            shell.env.update_or_add(
+                &self.variable_name,
+                ShellValueLiteral::Scalar(value),
+                |_| Ok(()),
+                EnvironmentLookup::Anywhere,
+                EnvironmentScope::Global,
+            )?;
 
-                result = self.body.0.execute(shell, params).await?;
-                if result.exit_shell || result.return_from_function_or_script {
-                    break;
-                }
+            result = self.body.0.execute(shell, params).await?;
+            if result.exit_shell || result.return_from_function_or_script {
+                break;
+            }
 
-                if let Some(continue_count) = &result.continue_loop {
-                    if *continue_count == 0 {
-                        result.continue_loop = None;
-                    } else {
-                        result.continue_loop = Some(*continue_count - 1);
-                        break;
-                    }
-                }
-                if let Some(break_count) = &result.break_loop {
-                    if *break_count == 0 {
-                        result.break_loop = None;
-                    } else {
-                        result.break_loop = Some(*break_count - 1);
-                    }
+            if let Some(continue_count) = &result.continue_loop {
+                if *continue_count == 0 {
+                    result.continue_loop = None;
+                } else {
+                    result.continue_loop = Some(*continue_count - 1);
                     break;
                 }
             }
+            if let Some(break_count) = &result.break_loop {
+                if *break_count == 0 {
+                    result.break_loop = None;
+                } else {
+                    result.break_loop = Some(*break_count - 1);
+                }
+                break;
+            }
         }
 
         shell.last_exit_status = result.exit_code;
diff --git a/brush-shell/tests/cases/compound_cmds/for.yaml b/brush-shell/tests/cases/compound_cmds/for.yaml
index 427c1f69..85312d31 100644
--- a/brush-shell/tests/cases/compound_cmds/for.yaml
+++ b/brush-shell/tests/cases/compound_cmds/for.yaml
@@ -4,10 +4,36 @@ cases:
     stdin: |
       for f in 1 2 3; do echo $f; done
 
+  - name: "Multi-line for loop"
+    stdin: |
+      for f in 1 2 3; do
+        echo $f
+      done
+
   - name: "Empty for loop"
     stdin: |
       for f in; do echo $f; done
 
+  - name: "for loop without in"
+    stdin: |
+      set -- a b c
+
+      echo "Loop 1"
+      for f; do echo $f; done
+
+      echo "Loop 2: no semicolon"
+      for f do echo $f; done
+
+  - name: "for loop without in but spaces"
+    stdin: |
+      set -- a "b c" d
+
+      echo "Loop 1"
+      for f; do echo $f; done
+
+      echo "Loop 2: no semicolon"
+      for f do echo $f; done
+
   - name: "Break in for loop"
     stdin: |
       for f in 1 2 3; do