From 9bd81f34a522b1d5e48e1f1e8c308a2ac38f415d Mon Sep 17 00:00:00 2001
From: AnotherTest <ali.mpfard@gmail.com>
Date: Wed, 9 Dec 2020 15:34:55 +0330
Subject: [PATCH] Shell: Make for/match/functions put their variables only in
 the new scope

Otherwise, a function would, for example, overwrite its parent scope:
```sh
foo(x) { }
x=1
foo 2 # would make x=2 otherwise
```
---
 Shell/AST.cpp           | 6 +++---
 Shell/Shell.cpp         | 2 +-
 Shell/Tests/function.sh | 5 +++++
 3 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/Shell/AST.cpp b/Shell/AST.cpp
index 2de6c75cbd..a0350fa755 100644
--- a/Shell/AST.cpp
+++ b/Shell/AST.cpp
@@ -1034,7 +1034,7 @@ RefPtr<Value> ForLoop::run(RefPtr<Shell> shell)
 
         {
             auto frame = shell->push_frame(String::formatted("for ({})", this));
-            shell->set_local_variable(m_variable_name, value);
+            shell->set_local_variable(m_variable_name, value, true);
 
             block_value = m_block->run(shell);
         }
@@ -1587,7 +1587,7 @@ RefPtr<Value> MatchExpr::run(RefPtr<Shell> shell)
 
     auto frame = shell->push_frame(String::formatted("match ({})", this));
     if (!m_expr_name.is_empty())
-        shell->set_local_variable(m_expr_name, value);
+        shell->set_local_variable(m_expr_name, value, true);
 
     for (auto& entry : m_entries) {
         for (auto& option : entry.options) {
@@ -1598,7 +1598,7 @@ RefPtr<Value> MatchExpr::run(RefPtr<Shell> shell)
                         size_t i = 0;
                         for (auto& name : entry.match_names.value()) {
                             if (spans.size() > i)
-                                shell->set_local_variable(name, create<AST::StringValue>(spans[i]));
+                                shell->set_local_variable(name, create<AST::StringValue>(spans[i]), true);
                             ++i;
                         }
                     }
diff --git a/Shell/Shell.cpp b/Shell/Shell.cpp
index 605b27bcf3..9f079a8004 100644
--- a/Shell/Shell.cpp
+++ b/Shell/Shell.cpp
@@ -479,7 +479,7 @@ bool Shell::invoke_function(const AST::Command& command, int& retval)
     size_t index = 0;
     for (auto& arg : function.arguments) {
         ++index;
-        set_local_variable(arg, adopt(*new AST::StringValue(command.argv[index])));
+        set_local_variable(arg, adopt(*new AST::StringValue(command.argv[index])), true);
     }
 
     auto argv = command.argv;
diff --git a/Shell/Tests/function.sh b/Shell/Tests/function.sh
index f3ef5aaeed..b9c23308ea 100644
--- a/Shell/Tests/function.sh
+++ b/Shell/Tests/function.sh
@@ -34,3 +34,8 @@ fn() {
 fn2() { }
 
 test "$(fn foobar)" = "foobar" || echo 'Frames are somehow messed up in nested functions' && exit 1
+
+fn(xfoo) { }
+xfoo=1
+fn 2
+test $xfoo -eq 1 || echo 'Functions overwrite parent scopes' && exit 1