Rework fix for table redefinition to avoid breaking AST-compatiblity

6 files changed, 123 insertions, 64 deletions

diff --git a/src/decoder/rustc_serialize.rs b/src/decoder/rustc_serialize.rs
index af38f9b..6e8fe59 100644
--- a/src/decoder/rustc_serialize.rs
+++ b/src/decoder/rustc_serialize.rs
@@ -3,7 +3,7 @@ use std::mem;
 
 use super::{Decoder, DecodeError};
 use super::DecodeErrorKind::*;
-use {Value, Table};
+use Value;
 
 impl rustc_serialize::Decoder for Decoder {
     type Error = DecodeError;
@@ -141,7 +141,7 @@ impl rustc_serialize::Decoder for Decoder {
             Some(Value::Table(..)) => {
                 let ret = try!(f(self));
                 match self.toml {
-                    Some(Value::Table(Table(ref t, _,))) if t.len() == 0 => {}
+                    Some(Value::Table(ref t)) if t.len() == 0 => {}
                     _ => return Ok(ret)
                 }
                 self.toml.take();
@@ -156,7 +156,7 @@ impl rustc_serialize::Decoder for Decoder {
     {
         let field = format!("{}", f_name);
         let toml = match self.toml {
-            Some(Value::Table(Table(ref mut table, _))) => {
+            Some(Value::Table(ref mut table)) => {
                 table.remove(&field)
                     .or_else(|| table.remove(&f_name.replace("_", "-")))
             },
@@ -165,7 +165,7 @@ impl rustc_serialize::Decoder for Decoder {
         let mut d = self.sub_decoder(toml, f_name);
         let ret = try!(f(&mut d));
         if let Some(value) = d.toml {
-            if let Some(Value::Table(Table(ref mut table, _))) = self.toml {
+            if let Some(Value::Table(ref mut table)) = self.toml {
                 table.insert(field, value);
             }
         }
@@ -260,7 +260,7 @@ impl rustc_serialize::Decoder for Decoder {
         where F: FnOnce(&mut Decoder, usize) -> Result<T, DecodeError>
     {
         let len = match self.toml {
-            Some(Value::Table(Table(ref table, _))) => table.len(),
+            Some(Value::Table(ref table)) => table.len(),
             ref found => return Err(self.mismatch("table", found)),
         };
         let ret = try!(f(self, len));
@@ -273,7 +273,7 @@ impl rustc_serialize::Decoder for Decoder {
     {
         match self.toml {
             Some(Value::Table(ref table)) => {
-                match table.0.iter().skip(idx).next() {
+                match table.iter().skip(idx).next() {
                     Some((key, _)) => {
                         let val = Value::String(format!("{}", key));
                         f(&mut self.sub_decoder(Some(val), &**key))
@@ -290,7 +290,7 @@ impl rustc_serialize::Decoder for Decoder {
     {
         match self.toml {
             Some(Value::Table(ref table)) => {
-                match table.0.iter().skip(idx).next() {
+                match table.iter().skip(idx).next() {
                     Some((_, value)) => {
                         // XXX: this shouldn't clone
                         f(&mut self.sub_decoder(Some(value.clone()), ""))
diff --git a/src/display.rs b/src/display.rs
index 74ec424..0c561e8 100644
--- a/src/display.rs
+++ b/src/display.rs
@@ -57,7 +57,7 @@ fn write_str(f: &mut fmt::Formatter, s: &str) -> fmt::Result {
 
 impl<'a, 'b> Printer<'a, 'b> {
     fn print(&mut self, table: &'a TomlTable) -> fmt::Result {
-        for (k, v) in table.0.iter() {
+        for (k, v) in table.iter() {
             match *v {
                 Table(..) => continue,
                 Array(ref a) => {
@@ -70,7 +70,7 @@ impl<'a, 'b> Printer<'a, 'b> {
             }
             try!(writeln!(self.output, "{} = {}", Key(&[k]), v));
         }
-        for (k, v) in table.0.iter() {
+        for (k, v) in table.iter() {
             match *v {
                 Table(ref inner) => {
                     self.stack.push(k);
@@ -127,14 +127,13 @@ impl<'a> fmt::Display for Key<'a> {
 #[allow(warnings)]
 mod tests {
     use Value;
-    use Table as TomlTable;
     use Value::{String, Integer, Float, Boolean, Datetime, Array, Table};
     use std::collections::BTreeMap;
 
     macro_rules! map( ($($k:expr => $v:expr),*) => ({
         let mut _m = BTreeMap::new();
         $(_m.insert($k.to_string(), $v);)*
-        TomlTable::new(_m)
+        _m
     }) );
 
     #[test]
diff --git a/src/encoder/mod.rs b/src/encoder/mod.rs
index ea8ef6a..21185f4 100644
--- a/src/encoder/mod.rs
+++ b/src/encoder/mod.rs
@@ -31,7 +31,7 @@ use {Value, Table};
 /// let mut e = Encoder::new();
 /// my_struct.encode(&mut e).unwrap();
 ///
-/// assert_eq!(e.toml.0.get(&"foo".to_string()), Some(&Value::Integer(4)))
+/// assert_eq!(e.toml.get(&"foo".to_string()), Some(&Value::Integer(4)))
 /// # }
 /// ```
 pub struct Encoder {
@@ -73,12 +73,12 @@ enum State {
 impl Encoder {
     /// Constructs a new encoder which will emit to the given output stream.
     pub fn new() -> Encoder {
-        Encoder { state: State::Start, toml: Table(BTreeMap::new(), false) }
+        Encoder { state: State::Start, toml: BTreeMap::new() }
     }
 
     fn emit_value(&mut self, v: Value) -> Result<(), Error> {
         match mem::replace(&mut self.state, State::Start) {
-            State::NextKey(key) => { self.toml.0.insert(key, v); Ok(()) }
+            State::NextKey(key) => { self.toml.insert(key, v); Ok(()) }
             State::NextArray(mut vec) => {
                 // TODO: validate types
                 vec.push(v);
@@ -122,7 +122,7 @@ impl Encoder {
             State::NextKey(key) => {
                 let mut nested = Encoder::new();
                 try!(f(&mut nested));
-                self.toml.0.insert(key, Value::Table(nested.toml));
+                self.toml.insert(key, Value::Table(nested.toml));
                 Ok(())
             }
             State::NextArray(mut arr) => {
diff --git a/src/encoder/rustc_serialize.rs b/src/encoder/rustc_serialize.rs
index 830eb5e..ab5e90f 100644
--- a/src/encoder/rustc_serialize.rs
+++ b/src/encoder/rustc_serialize.rs
@@ -193,8 +193,8 @@ impl rustc_serialize::Encodable for Value {
                 })
             }
             Value::Table(ref t) => {
-                e.emit_map(t.0.len(), |e| {
-                    for (i, (key, value)) in t.0.iter().enumerate() {
+                e.emit_map(t.len(), |e| {
+                    for (i, (key, value)) in t.iter().enumerate() {
                         try!(e.emit_map_elt_key(i, |e| e.emit_str(key)));
                         try!(e.emit_map_elt_val(i, |e| value.encode(e)));
                     }
@@ -212,7 +212,6 @@ mod tests {
 
     use {Encoder, Decoder, DecodeError};
     use Value;
-    use Table as TomlTable;
     use Value::{Table, Integer, Array, Float};
 
     macro_rules! encode( ($t:expr) => ({
@@ -229,7 +228,7 @@ mod tests {
     macro_rules! map( ($($k:ident, $v:expr),*) => ({
         let mut _m = BTreeMap::new();
         $(_m.insert(stringify!($k).to_string(), $v);)*
-        TomlTable::new(_m)
+        _m
     }) );
 
     #[test]
@@ -578,9 +577,7 @@ mod tests {
         #[derive(RustcEncodable, RustcDecodable, PartialEq, Debug)]
         struct Foo { a: BTreeMap<String, String> }
 
-        let mut v = Foo { a: BTreeMap::new() };
-        v.a.insert("a".to_string(), "foo".to_string());
-
+        let v = Foo { a: map! { a, "foo".to_string() } };
         let mut d = Decoder::new(Table(map! {
             a, Table(map! {
                 a, Value::String("foo".to_string())
diff --git a/src/lib.rs b/src/lib.rs
index 547c407..0196fbc 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -75,24 +75,8 @@ pub enum Value {
 /// Type representing a TOML array, payload of the Value::Array variant
 pub type Array = Vec<Value>;
 
-// The bool field flag is used during parsing and construction.
-// Is true if the given table was explicitly defined, false otherwise
-// e.g. in a toml document: `[a.b] foo = "bar"`, Table `a` would be false,
-// where table `b` (contained inside `a`) would be true.
 /// Type representing a TOML table, payload of the Value::Table variant
-#[derive(Debug, Clone)]
-pub struct Table (pub BTreeMap<string::String, Value>, bool);
-impl Table {
-    /// Creates new TOML table
-    pub fn new(map: BTreeMap<string::String, Value>) -> Table {
-        Table(map, false)
-    }
-}
-impl PartialEq for Table {
-    fn eq(&self, other: &Table) -> bool {
-        self.0.eq(&other.0)
-    }
-}
+pub type Table = BTreeMap<string::String, Value>;
 
 impl Value {
     /// Tests whether this and another value have the same type.
@@ -198,7 +182,7 @@ impl Value {
         let mut cur_value = self;
         for key in path.split('.') {
             match cur_value {
-                &Value::Table(Table(ref hm, _)) => {
+                &Value::Table(ref hm) => {
                     match hm.get(key) {
                         Some(v) => cur_value = v,
                         None => return None
diff --git a/src/parser.rs b/src/parser.rs
index ccf0d3a..068bf1d 100644
--- a/src/parser.rs
+++ b/src/parser.rs
@@ -5,13 +5,92 @@ use std::error::Error;
 use std::fmt;
 use std::str;
 
-use Table as TomlTable;
-use Value::{self, Array, Table, Float, Integer, Boolean, Datetime};
-
 macro_rules! try {
     ($e:expr) => (match $e { Some(s) => s, None => return None })
 }
 
+/* 
+ * We redefine Array, Table and Value, because we need to keep track of
+ * encountered table definitions, eg when parsing:
+ * [a]
+ * [a.b]
+ * [a]
+ * we have to error out on redefinition of [a].
+ * This bit of data is impossible to represent in the user-consumed table
+ * without breaking compatibility, so we use one AST structure during parsing
+ * and expose another (after running convert(...) on it) to the user.
+ */
+type Array = Vec<Value>;
+
+#[derive(PartialEq, Clone, Debug)]
+// If the bool flag is true, the table was explicitly defined
+// e.g. in a toml document: `[a.b] foo = "bar"`, Table `a` would be false,
+// where table `b` (contained inside `a`) would be true.
+struct TomlTable(BTreeMap<String, Value>, bool);
+impl TomlTable {
+    fn convert(self) -> super::Table {
+        self.0.into_iter().map(|(k,v)| (k,v.convert())).collect()
+    }
+}
+
+#[derive(PartialEq, Clone, Debug)]
+enum Value {
+    String(String),
+    Integer(i64),
+    Float(f64),
+    Boolean(bool),
+    Datetime(String),
+    Array(Array),
+    Table(TomlTable),
+}
+
+impl Value {
+    fn type_str(&self) -> &'static str {
+        match *self {
+            Value::String(..) => "string",
+            Value::Integer(..) => "integer",
+            Value::Float(..) => "float",
+            Value::Boolean(..) => "boolean",
+            Value::Datetime(..) => "datetime",
+            Value::Array(..) => "array",
+            Value::Table(..) => "table",
+        }
+    }
+    
+    fn as_table<'a>(&'a self) -> Option<&'a TomlTable> {
+        match *self { Value::Table(ref s) => Some(s), _ => None }
+    }
+
+    fn same_type(&self, other: &Value) -> bool {
+        match (self, other) {
+            (&Value::String(..), &Value::String(..)) |
+            (&Value::Integer(..), &Value::Integer(..)) |
+            (&Value::Float(..), &Value::Float(..)) |
+            (&Value::Boolean(..), &Value::Boolean(..)) |
+            (&Value::Datetime(..), &Value::Datetime(..)) |
+            (&Value::Array(..), &Value::Array(..)) |
+            (&Value::Table(..), &Value::Table(..)) => true,
+
+            _ => false,
+        }
+    }
+
+    fn convert(self) -> super::Value {
+        match self {
+            Value::String(x) => super::Value::String(x),
+            Value::Integer(x) => super::Value::Integer(x),
+            Value::Float(x) => super::Value::Float(x),
+            Value::Boolean(x) => super::Value::Boolean(x),
+            Value::Datetime(x) => super::Value::Datetime(x),
+            Value::Array(v) =>
+                super::Value::Array(
+                    v.into_iter().map(|x| x.convert()).collect()
+                ),
+            Value::Table(t) => super::Value::Table(t.convert())
+        }
+    }
+}
+
 /// Parser for converting a string to a TOML `Value` instance.
 ///
 /// This parser contains the string slice that is being parsed, and exports the
@@ -161,7 +240,7 @@ impl<'a> Parser<'a> {
     ///
     /// If an error occurs, the `errors` field of this parser can be consulted
     /// to determine the cause of the parse failure.
-    pub fn parse(&mut self) -> Option<TomlTable> {
+    pub fn parse(&mut self) -> Option<super::Table> {
         let mut ret = TomlTable(BTreeMap::new(), false);
         while self.peek(0).is_some() {
             self.ws();
@@ -192,7 +271,7 @@ impl<'a> Parser<'a> {
                 let mut table = TomlTable(BTreeMap::new(), false);
                 if !self.values(&mut table) { return None }
                 if array {
-                    self.insert_array(&mut ret, &*keys, Table(table), start)
+                    self.insert_array(&mut ret, &*keys, Value::Table(table), start)
                 } else {
                     self.insert_table(&mut ret, &*keys, table, start)
                 }
@@ -203,7 +282,7 @@ impl<'a> Parser<'a> {
         if self.errors.len() > 0 {
             None
         } else {
-            Some(ret)
+            Some(ret.convert())
         }
     }
 
@@ -519,9 +598,9 @@ impl<'a> Parser<'a> {
             };
             let input = input.trim_left_matches('+');
             if is_float {
-                input.parse().ok().map(Float)
+                input.parse().ok().map(Value::Float)
             } else {
-                input.parse().ok().map(Integer)
+                input.parse().ok().map(Value::Integer)
             }
         };
         if ret.is_none() {
@@ -603,12 +682,12 @@ impl<'a> Parser<'a> {
             for _ in 0..4 {
                 self.cur.next();
             }
-            Some(Boolean(true))
+            Some(Value::Boolean(true))
         } else if rest.starts_with("false") {
             for _ in 0..5 {
                 self.cur.next();
             }
-            Some(Boolean(false))
+            Some(Value::Boolean(false))
         } else {
             let next = self.next_pos();
             self.errors.push(ParserError {
@@ -659,7 +738,7 @@ impl<'a> Parser<'a> {
         valid = valid && it.next().map(is_digit).unwrap_or(false);
         valid = valid && it.next().map(|c| c == 'Z').unwrap_or(false);
         if valid {
-            Some(Datetime(date.clone()))
+            Some(Value::Datetime(date.clone()))
         } else {
             self.errors.push(ParserError {
                 lo: start,
@@ -683,7 +762,7 @@ impl<'a> Parser<'a> {
         loop {
             // Break out early if we see the closing bracket
             consume(self);
-            if self.eat(']') { return Some(Array(ret)) }
+            if self.eat(']') { return Some(Value::Array(ret)) }
 
             // Attempt to parse a value, triggering an error if it's the wrong
             // type.
@@ -709,14 +788,14 @@ impl<'a> Parser<'a> {
         }
         consume(self);
         if !self.expect(']') { return None }
-        return Some(Array(ret))
+        return Some(Value::Array(ret))
     }
 
     fn inline_table(&mut self, _start: usize) -> Option<Value> {
         if !self.expect('{') { return None }
         self.ws();
         let mut ret = TomlTable(BTreeMap::new(), true);
-        if self.eat('}') { return Some(Table(ret)) }
+        if self.eat('}') { return Some(Value::Table(ret)) }
         loop {
             let lo = self.next_pos();
             let key = try!(self.key_name());
@@ -729,7 +808,7 @@ impl<'a> Parser<'a> {
             if !self.expect(',') { return None }
             self.ws();
         }
-        return Some(Table(ret))
+        return Some(Value::Table(ret))
     }
 
     fn insert(&mut self, into: &mut TomlTable, key: String, value: Value,
@@ -753,13 +832,13 @@ impl<'a> Parser<'a> {
 
             if tmp.0.contains_key(part) {
                 match *tmp.0.get_mut(part).unwrap() {
-                    Table(ref mut table) => {
+                    Value::Table(ref mut table) => {
                         cur = table;
                         continue
                     }
-                    Array(ref mut array) => {
+                    Value::Array(ref mut array) => {
                         match array.last_mut() {
-                            Some(&mut Table(ref mut table)) => cur = table,
+                            Some(&mut Value::Table(ref mut table)) => cur = table,
                             _ => {
                                 self.errors.push(ParserError {
                                     lo: key_lo,
@@ -785,9 +864,9 @@ impl<'a> Parser<'a> {
             }
 
             // Initialize an empty table as part of this sub-key
-            tmp.0.insert(part.clone(), Table(TomlTable(BTreeMap::new(), false)));
+            tmp.0.insert(part.clone(), Value::Table(TomlTable(BTreeMap::new(), false)));
             match *tmp.0.get_mut(part).unwrap() {
-                Table(ref mut inner) => cur = inner,
+                Value::Table(ref mut inner) => cur = inner,
                 _ => unreachable!(),
             }
         }
@@ -803,11 +882,11 @@ impl<'a> Parser<'a> {
         let key = format!("{}", key);
         let mut added = false;
         if !into.0.contains_key(&key) {
-            into.0.insert(key.clone(), Table(TomlTable(BTreeMap::new(), true)));
+            into.0.insert(key.clone(), Value::Table(TomlTable(BTreeMap::new(), true)));
             added = true;
         }
         match into.0.get_mut(&key) {
-            Some(&mut Table(ref mut table)) => {
+            Some(&mut Value::Table(ref mut table)) => {
                 let any_tables = table.0.values().any(|v| v.as_table().is_some());
                 if !added && (!any_tables || table.1) {
                     self.errors.push(ParserError {
@@ -845,10 +924,10 @@ impl<'a> Parser<'a> {
         };
         let key = format!("{}", key);
         if !into.0.contains_key(&key) {
-            into.0.insert(key.clone(), Array(Vec::new()));
+            into.0.insert(key.clone(), Value::Array(Vec::new()));
         }
         match *into.0.get_mut(&key).unwrap() {
-            Array(ref mut vec) => {
+            Value::Array(ref mut vec) => {
                 match vec.first() {
                     Some(ref v) if !v.same_type(&value) => {
                         self.errors.push(ParserError {