Core: support unsetting variables, fix is_null

Empty variable values are all treated as null value. This simplifies
the previous code and fixes some logic errors.

7 files changed, 105 insertions, 25 deletions

diff --git a/bashast/libbashWalker.g b/bashast/libbashWalker.g
index e278d1b..203cd28 100644
--- a/bashast/libbashWalker.g
+++ b/bashast/libbashWalker.g
@@ -119,10 +119,9 @@ var_def
 @declarations {
 	std::map<int, std::string> values;
 	unsigned index = 0;
-	bool is_null = true;
 }
-	:^(EQUALS name (string_expr { is_null = false; })?) {
-		walker->set_value($name.libbash_value, $string_expr.libbash_value, $name.index, is_null);
+	:^(EQUALS name string_expr?) {
+		walker->set_value($name.libbash_value, $string_expr.libbash_value, $name.index);
 	}
 	|^(EQUALS libbash_name=name_base ^(ARRAY (
 										(expr=string_expr
diff --git a/scripts/var_def.bash.result b/scripts/var_def.bash.result
index e840ebe..1b9e287 100644
--- a/scripts/var_def.bash.result
+++ b/scripts/var_def.bash.result
@@ -5,8 +5,8 @@ $- has not been implemented yet
 $! has not been implemented yet
 ARRAY01=1 2 3 4 5
 ARRAY02=1 2 4 5
-ARRAY03=2 3
-ARRAY04=2 3
+ARRAY03= 2 3
+ARRAY04= 2 3
 ARRAY05=1 2 3 4 5
 ARRAY06=1 2 3 4 5
 ARRAY07=1 2 3 4 5
diff --git a/src/core/interpreter.cpp b/src/core/interpreter.cpp
index 9548e8a..f0bf61e 100644
--- a/src/core/interpreter.cpp
+++ b/src/core/interpreter.cpp
@@ -178,3 +178,24 @@ void interpreter::get_all_function_names(std::vector<std::string>& function_name
 {
   boost::copy(functions | boost::adaptors::map_keys, back_inserter(function_names));
 }
+
+void interpreter::unset(const std::string& name)
+{
+  auto iter = members.find(name);
+  if(iter == members.end())
+    return;
+  else if(iter->second->is_readonly())
+    throw interpreter_exception("Can't unset readonly variable " + name);
+  else
+    members.erase(name);
+}
+
+void interpreter::unset(const std::string& name,
+                        const unsigned index)
+{
+  auto iter = members.find(name);
+  if(iter == members.end())
+    return;
+  else
+    iter->second->unset_value(index);
+}
diff --git a/src/core/interpreter.h b/src/core/interpreter.h
index e471b94..dc446fc 100644
--- a/src/core/interpreter.h
+++ b/src/core/interpreter.h
@@ -468,14 +468,13 @@ public:
   template <typename T>
   const T& set_value(const std::string& name,
                      const T& new_value,
-                     const unsigned index=0,
-                     bool is_null=false)
+                     const unsigned index=0)
   {
     auto i = members.find(name);
     if(i == members.end())
-      define(name, new_value, false, is_null, index);
+      define(name, new_value, false, index);
     else
-      i->second->set_value(new_value, index, is_null);
+      i->second->set_value(new_value, index);
     return new_value;
   }
 
@@ -494,6 +493,15 @@ public:
     return resolve<T>("?");
   }
 
+  /// \brief unset a variable
+  /// \param the name of the variable
+  void unset(const std::string& name);
+
+  /// \brief unset a array member
+  /// \param the name of the array
+  /// \param the index of the member
+  void unset(const std::string& name, const unsigned index);
+
   /// \brief define a new global variable
   /// \param the name of the variable
   /// \param the value of the variable
@@ -503,11 +511,10 @@ public:
   void define(const std::string& name,
               const T& value,
               bool readonly=false,
-              bool is_null=false,
               const unsigned index=0)
   {
     std::shared_ptr<variable> target(
-        new variable(name, value, readonly, is_null, index));
+        new variable(name, value, readonly, index));
     members[name] = target;
   }
 
diff --git a/src/core/symbols.hpp b/src/core/symbols.hpp
index b11d338..89fd657 100644
--- a/src/core/symbols.hpp
+++ b/src/core/symbols.hpp
@@ -137,12 +137,10 @@ public:
   variable(const std::string& name,
            const T& v,
            bool ro=false,
-           bool is_null=false,
            const unsigned index=0)
     : name(name), readonly(ro)
   {
-    if(!is_null)
-        value[index] = v;
+    value[index] = v;
   }
 
   /// \brief retrieve actual value of the variable, if index is out of bound,
@@ -182,16 +180,22 @@ public:
   /// \param whether to set the variable to null value, default is false
   template <typename T>
   void set_value(const T& new_value,
-                 const unsigned index=0,
-                 bool is_null=false)
+                 const unsigned index=0)
   {
     if(readonly)
       throw interpreter_exception(get_name() + " is readonly variable");
 
-    if(is_null)
-        value.erase(index);
-    else
-        value[index] = new_value;
+    value[index] = new_value;
+  }
+
+  /// \brief unset the variable, only used for array variable
+  /// \param the index to be unset
+  void unset_value(const unsigned index)
+  {
+    if(readonly)
+      throw interpreter_exception(get_name() + " is readonly variable");
+
+    value.erase(index);
   }
 
   /// \brief get the length of a variable
@@ -211,10 +215,24 @@ public:
 
   /// \brief check whether the value of the variable is null
   /// \return whether the value of the variable is null
-  bool is_null(const unsigned index=0) const
+  bool is_unset(const unsigned index=0) const
   {
     return value.find(index) == value.end();
   }
+
+  /// \brief check whether the value of the variable is unset
+  /// \return whether the value of the variable is unset
+  bool is_null(const unsigned index=0) const
+  {
+    return get_value<std::string>(index) == "";
+  }
+
+  /// \brief check whether the value of the variable is readonly
+  /// \return whether the value of the variable is readonly
+  bool is_readonly() const
+  {
+    return readonly;
+  }
 };
 
 // specialization for arrays
@@ -222,7 +240,7 @@ template <>
 inline variable::variable<>(const std::string& name,
                             const std::map<int, std::string>& v,
                             bool ro,
-                            bool, unsigned)
+                            unsigned)
     : name(name), value(v.begin(), v.end()), readonly(ro)
 {
 }
diff --git a/src/core/tests/interpreter_test.cpp b/src/core/tests/interpreter_test.cpp
index 2e92a22..54f25b0 100644
--- a/src/core/tests/interpreter_test.cpp
+++ b/src/core/tests/interpreter_test.cpp
@@ -54,7 +54,7 @@ TEST(interpreter, define_resolve_array)
   EXPECT_STREQ("3", walker.resolve<string>("array", 2).c_str());
   EXPECT_STREQ("", walker.resolve<string>("undefined",100).c_str());
 
-  walker.define("partial", 10, false, false, 8);
+  walker.define("partial", 10, false, 8);
   EXPECT_EQ(1, walker.get_array_length("partial"));
   EXPECT_EQ(10, walker.resolve<int>("partial", 8));
 }
@@ -142,6 +142,33 @@ TEST(interpreter, get_array_values)
   EXPECT_EQ(3, array_values[2]);
 }
 
+TEST(interpreter, unset_values)
+{
+  interpreter walker;
+  std::map<int, std::string> values = {{0, "1"}, {1, "2"}, {2, "3"}};
+  walker.define("array", values);
+  walker.define("ro_array", values, true);
+  walker.define("var", "123");
+  walker.define("ro_var", "123", true);
+
+  EXPECT_STREQ("2", walker.resolve<string>("array", 1).c_str());
+  walker.unset("array", 1);
+  EXPECT_STREQ("", walker.resolve<string>("array", 1).c_str());
+  walker.unset("array");
+  EXPECT_STREQ("", walker.resolve<string>("array", 0).c_str());
+  EXPECT_STREQ("", walker.resolve<string>("array", 1).c_str());
+  EXPECT_STREQ("", walker.resolve<string>("array", 2).c_str());
+
+  EXPECT_THROW(walker.unset("ro_array", 1), interpreter_exception);
+  EXPECT_THROW(walker.unset("ro_array"), interpreter_exception);
+
+  EXPECT_STREQ("123", walker.resolve<string>("var").c_str());
+  walker.unset("var");
+  EXPECT_STREQ("", walker.resolve<string>("var").c_str());
+
+  EXPECT_THROW(walker.unset("ro_var"), interpreter_exception);
+}
+
 TEST(interperter, substring_expansion_exception)
 {
   interpreter walker;
diff --git a/src/core/tests/symbols_test.cpp b/src/core/tests/symbols_test.cpp
index d614947..f151318 100644
--- a/src/core/tests/symbols_test.cpp
+++ b/src/core/tests/symbols_test.cpp
@@ -122,9 +122,17 @@ TEST(symbol_test, is_null)
 {
   variable var("foo", 10);
   EXPECT_FALSE(var.is_null());
-  var.set_value("bar", 0, true);
+  var.set_value("");
   EXPECT_TRUE(var.is_null());
-  EXPECT_TRUE(variable("foo", "", false, true).is_null());
+  EXPECT_TRUE(variable("foo", "").is_null());
+}
+
+TEST(symbol_test, is_unset)
+{
+  map<int, string> values = {{0, "1"}, {1, "2"}, {2, "3"}};
+  variable array("foo", values);
+  array.unset_value(1);
+  EXPECT_TRUE(array.is_unset(1));
 }
 
 TEST(symbol_test, get_length)