From f3560b8e626a528f2b5cc167c00d610ab9546b82 Mon Sep 17 00:00:00 2001
From: ridiculousfish <corydoras@ridiculousfish.com>
Date: Fri, 12 Jun 2015 16:05:59 -0700
Subject: [PATCH] Correctly un-export an env var when it is shadowed

Prior to this fix, if you exported a variable in one scope
and then unexported it in the next, it would remain exported.
Example:

    set -gx VAR 1
    function foo; set -l VAR; env; end
    foo

Here 'VAR' would be exported to 'env' because we failed to
notice that the env var is shadowed by an unexported variable.
This occurred at env var computation time, not in env_set!

Fixes #2132
---
 env.cpp         | 19 ++++++++++++++-----
 tests/test3.in  |  8 ++++++++
 tests/test3.out |  2 ++
 3 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/env.cpp b/env.cpp
index 1a04665eb..6b09c07f8 100644
--- a/env.cpp
+++ b/env.cpp
@@ -1264,7 +1264,7 @@ wcstring_list_t env_get_names(int flags)
   Get list of all exported variables
 */
 
-static void get_exported(const env_node_t *n, std::map<wcstring, wcstring> &h)
+static void get_exported(const env_node_t *n, std::map<wcstring, wcstring> *h)
 {
     if (!n)
         return;
@@ -1279,10 +1279,19 @@ static void get_exported(const env_node_t *n, std::map<wcstring, wcstring> &h)
     {
         const wcstring &key = iter->first;
         const var_entry_t &val_entry = iter->second;
-        if (val_entry.exportv && (val_entry.val != ENV_NULL))
+
+        if (val_entry.exportv && val_entry.val != ENV_NULL)
         {
-            // Don't use std::map::insert here, since we need to overwrite existing values from previous scopes
-            h[key] = val_entry.val;
+            // Export the variable
+            // Don't use std::map::insert here, since we need to overwrite existing
+            // values from previous scopes
+            (*h)[key] = val_entry.val;
+        }
+        else
+        {
+            // We need to erase from the map if we are not exporting,
+            // since a lower scope may have exported. See #2132
+            h->erase(key);
         }
     }
 }
@@ -1333,7 +1342,7 @@ static void update_export_array_if_necessary(bool recalc)
 
         debug(4, L"env_export_arr() recalc");
 
-        get_exported(top, vals);
+        get_exported(top, &vals);
 
         if (uvars())
         {
diff --git a/tests/test3.in b/tests/test3.in
index 970741d16..fe20928cb 100644
--- a/tests/test3.in
+++ b/tests/test3.in
@@ -227,6 +227,14 @@ else
   echo Test 16 fail
 end
 
+# Test that shadowing with a non-exported variable works
+set -gx __fish_test_env17 UNSHADOWED
+env | sgrep __fish_test_env17
+function __fish_test_shadow
+  set -l __fish_test_env17
+  env | sgrep __fish_test_env17 ; or echo SHADOWED
+end
+__fish_test_shadow
 
 # clear for other shells
 set -eU __fish_test_universal_variables_variable_foo
diff --git a/tests/test3.out b/tests/test3.out
index a6d5d3ac5..806ea7081 100644
--- a/tests/test3.out
+++ b/tests/test3.out
@@ -16,6 +16,8 @@ Test 15 pass
 Foo change detected
 Foo change detected
 Test 16 pass
+__fish_test_env17=UNSHADOWED
+SHADOWED
 Testing Universal Startup
 1
 1