Apply feedkback from porting cargo (#240)

* fix!: Make `Document::root` private

This gives us more implementation flexibility

BREAKING CHANGE: Instead of dealing with `Document::root`, use
`Document::as_table()` or `Document::as_table_mut()`.

* refactor: Use types to enforce root item's type

* feat: Make it easier to work with root table

In porting cargo to toml_edit, I found it would be nice if we allowed
the document to be used as the table it wraps.

* fix!: Unify the normal/easy `Index` traits

This trait isn't normally interacted with by users

* feat: Non-panicking indexing on Items

This is being pulled over from the `easy` API.  Found this would be
useful inside of cargo.  Some direct document interation is needed
before using serde to convert it to native types.

* style: Make clippy happy

src/document.rs

@@ -2,13 +2,12 @@ use std::str::FromStr;
 
 use crate::parser;
 use crate::table::Iter;
-use crate::{InternalString, Item, Table};
+use crate::{InternalString, Table};
 
 /// Type representing a TOML document
 #[derive(Debug, Clone)]
 pub struct Document {
-    /// Root should always be `Item::Table`.
-    pub root: Item,
+    pub(crate) root: Table,
     // Trailing comments and whitespaces
     pub(crate) trailing: InternalString,
 }
@@ -21,22 +20,17 @@ impl Document {
 
     /// Returns a reference to the root table.
     pub fn as_table(&self) -> &Table {
-        self.root.as_table().expect("root should always be a table")
+        &self.root
     }
 
     /// Returns a mutable reference to the root table.
     pub fn as_table_mut(&mut self) -> &mut Table {
-        self.root
-            .as_table_mut()
-            .expect("root should always be a table")
+        &mut self.root
     }
 
     /// Returns an iterator over the root table.
     pub fn iter(&self) -> Iter<'_> {
-        self.root
-            .as_table()
-            .expect("root should always be a table")
-            .iter()
+        self.root.iter()
     }
 
     /// Set whitespace after last element
@@ -53,7 +47,7 @@ impl Document {
 impl Default for Document {
     fn default() -> Self {
         Self {
-            root: Item::Table(Table::with_pos(Some(0))),
+            root: Table::with_pos(Some(0)),
             trailing: Default::default(),
         }
     }
@@ -67,3 +61,17 @@ impl FromStr for Document {
         parser::TomlParser::parse(s.as_bytes())
     }
 }
+
+impl std::ops::Deref for Document {
+    type Target = Table;
+
+    fn deref(&self) -> &Self::Target {
+        &self.root
+    }
+}
+
+impl std::ops::DerefMut for Document {
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        &mut self.root
+    }
+}

src/easy/de/mod.rs

@@ -113,7 +113,7 @@ impl<'de, 'a> serde::Deserializer<'de> for Deserializer {
     where
         V: serde::de::Visitor<'de>,
     {
-        ItemDeserializer::new(self.input.root).deserialize_any(visitor)
+        TableDeserializer::new(self.input.root).deserialize_any(visitor)
     }
 
     // `None` is interpreted as a missing field so be sure to implement `Some`
@@ -135,21 +135,16 @@ impl<'de, 'a> serde::Deserializer<'de> for Deserializer {
     where
         V: serde::de::Visitor<'de>,
     {
-        match self.input.root {
-            crate::Item::Table(v) => {
-                if v.is_empty() {
-                    Err(crate::easy::de::Error::custom(
-                        "wanted exactly 1 element, found 0 elements",
-                    ))
-                } else if v.len() != 1 {
-                    Err(crate::easy::de::Error::custom(
-                        "wanted exactly 1 element, more than 1 element",
-                    ))
-                } else {
-                    visitor.visit_enum(crate::easy::de::TableMapAccess::new(v))
-                }
-            }
-            _ => Err(crate::easy::de::Error::custom("wanted table")),
+        if self.input.root.is_empty() {
+            Err(crate::easy::de::Error::custom(
+                "wanted exactly 1 element, found 0 elements",
+            ))
+        } else if self.input.root.len() != 1 {
+            Err(crate::easy::de::Error::custom(
+                "wanted exactly 1 element, more than 1 element",
+            ))
+        } else {
+            visitor.visit_enum(crate::easy::de::TableMapAccess::new(self.input.root))
         }
     }
 

src/easy/de/table.rs

@@ -2,6 +2,77 @@ use serde::de::IntoDeserializer;
 
 use crate::easy::de::Error;
 
+pub(crate) struct TableDeserializer {
+    input: crate::Table,
+}
+
+impl TableDeserializer {
+    pub(crate) fn new(input: crate::Table) -> Self {
+        Self { input }
+    }
+}
+
+impl<'de, 'a> serde::Deserializer<'de> for TableDeserializer {
+    type Error = Error;
+
+    fn deserialize_any<V>(self, visitor: V) -> Result<V::Value, Self::Error>
+    where
+        V: serde::de::Visitor<'de>,
+    {
+        visitor.visit_map(crate::easy::de::TableMapAccess::new(self.input))
+    }
+
+    // `None` is interpreted as a missing field so be sure to implement `Some`
+    // as a present field.
+    fn deserialize_option<V>(self, visitor: V) -> Result<V::Value, Error>
+    where
+        V: serde::de::Visitor<'de>,
+    {
+        visitor.visit_some(self)
+    }
+
+    fn deserialize_struct<V>(
+        self,
+        _name: &'static str,
+        _fields: &'static [&'static str],
+        visitor: V,
+    ) -> Result<V::Value, Error>
+    where
+        V: serde::de::Visitor<'de>,
+    {
+        self.deserialize_any(visitor)
+    }
+
+    // Called when the type to deserialize is an enum, as opposed to a field in the type.
+    fn deserialize_enum<V>(
+        self,
+        _name: &'static str,
+        _variants: &'static [&'static str],
+        visitor: V,
+    ) -> Result<V::Value, Error>
+    where
+        V: serde::de::Visitor<'de>,
+    {
+        if self.input.is_empty() {
+            Err(crate::easy::de::Error::custom(
+                "wanted exactly 1 element, found 0 elements",
+            ))
+        } else if self.input.len() != 1 {
+            Err(crate::easy::de::Error::custom(
+                "wanted exactly 1 element, more than 1 element",
+            ))
+        } else {
+            visitor.visit_enum(crate::easy::de::TableMapAccess::new(self.input))
+        }
+    }
+
+    serde::forward_to_deserialize_any! {
+        bool u8 u16 u32 u64 i8 i16 i32 i64 f32 f64 char str string seq
+        bytes byte_buf map unit newtype_struct
+        ignored_any unit_struct tuple_struct tuple identifier
+    }
+}
+
 pub(crate) struct TableMapAccess {
     iter: indexmap::map::IntoIter<crate::InternalString, crate::table::TableKeyValue>,
     value: Option<crate::Item>,

src/easy/macros.rs

@@ -7,26 +7,24 @@ use crate::easy::value::{Array, Table, Value};
 /// [`toml_edit::easy::Value`]: value/enum.Value.html
 ///
 /// ```rust
-/// fn main() {
-///     let cargo_toml = toml_edit::easy::toml! {
-///         [package]
-///         name = "toml"
-///         version = "0.4.5"
-///         authors = ["Alex Crichton <alex@alexcrichton.com>"]
+/// let cargo_toml = toml_edit::easy::toml! {
+///     [package]
+///     name = "toml"
+///     version = "0.4.5"
+///     authors = ["Alex Crichton <alex@alexcrichton.com>"]
 ///
-///         [badges]
-///         travis-ci = { repository = "alexcrichton/toml-rs" }
+///     [badges]
+///     travis-ci = { repository = "alexcrichton/toml-rs" }
 ///
-///         [dependencies]
-///         serde = "1.0"
+///     [dependencies]
+///     serde = "1.0"
 ///
-///         [dev-dependencies]
-///         serde_derive = "1.0"
-///         serde_json = "1.0"
-///     };
+///     [dev-dependencies]
+///     serde_derive = "1.0"
+///     serde_json = "1.0"
+/// };
 ///
-///     println!("{:#?}", cargo_toml);
-/// }
+/// println!("{:#?}", cargo_toml);
 /// ```
 #[macro_export]
 macro_rules! toml {

src/easy/ser/table.rs

@@ -39,7 +39,7 @@ impl serde::ser::SerializeMap for SerializeDocument {
 
     fn end(self) -> Result<Self::Ok, Self::Error> {
         self.inner.end().map(|root| crate::Document {
-            root: crate::Item::Table(root),
+            root,
             ..Default::default()
         })
     }
@@ -62,7 +62,7 @@ impl serde::ser::SerializeStruct for SerializeDocument {
 
     fn end(self) -> Result<Self::Ok, Self::Error> {
         self.inner.end().map(|root| crate::Document {
-            root: crate::Item::Table(root),
+            root,
             ..Default::default()
         })
     }

src/index.rs

@@ -9,38 +9,27 @@ use crate::{value, InlineTable, InternalString, Item, Table, Value};
 // https://github.com/serde-rs/json/blob/master/src/value/index.rs
 
 pub trait Index: crate::private::Sealed {
-    /// Return `Option::None` if the key is not already in the array or table.
     #[doc(hidden)]
-    fn index<'v>(&self, v: &'v Item) -> Option<&'v Item>;
-
-    /// Panic if array index out of bounds. If key is not already in the table,
-    /// insert it with a value of `Item::None`. Panic if `v` has a type that cannot be
-    /// indexed into, except if `v` is `Item::None` then it can be treated as an empty
-    /// inline table.
+    fn index<'v>(&self, val: &'v Item) -> Option<&'v Item>;
     #[doc(hidden)]
-    fn index_or_insert<'v>(&self, v: &'v mut Item) -> &'v mut Item;
+    fn index_mut<'v>(&self, val: &'v mut Item) -> Option<&'v mut Item>;
 }
 
 impl Index for usize {
     fn index<'v>(&self, v: &'v Item) -> Option<&'v Item> {
         match *v {
             Item::ArrayOfTables(ref aot) => aot.values.get(*self),
-            Item::Value(ref a) if a.is_array() => a.as_array().unwrap().values.get(*self),
+            Item::Value(ref a) if a.is_array() => a.as_array().and_then(|a| a.values.get(*self)),
             _ => None,
         }
     }
-    fn index_or_insert<'v>(&self, v: &'v mut Item) -> &'v mut Item {
+    fn index_mut<'v>(&self, v: &'v mut Item) -> Option<&'v mut Item> {
         match *v {
-            Item::ArrayOfTables(ref mut vec) => {
-                vec.values.get_mut(*self).expect("index out of bounds")
+            Item::ArrayOfTables(ref mut vec) => vec.values.get_mut(*self),
+            Item::Value(ref mut a) if a.is_array() => {
+                a.as_array_mut().and_then(|a| a.values.get_mut(*self))
             }
-            Item::Value(ref mut a) if a.is_array() => a
-                .as_array_mut()
-                .unwrap()
-                .values
-                .get_mut(*self)
-                .expect("index out of bounds"),
-            _ => panic!("cannot access index {}", self),
+            _ => None,
         }
     }
 }
@@ -55,7 +44,7 @@ impl Index for str {
             _ => None,
         }
     }
-    fn index_or_insert<'v>(&self, v: &'v mut Item) -> &'v mut Item {
+    fn index_mut<'v>(&self, v: &'v mut Item) -> Option<&'v mut Item> {
         if let Item::None = *v {
             let mut t = InlineTable::default();
             t.items.insert(
@@ -65,17 +54,15 @@ impl Index for str {
             *v = value(Value::InlineTable(t));
         }
         match *v {
-            Item::Table(ref mut t) => t.entry(self).or_insert(Item::None),
-            Item::Value(ref mut v) if v.is_inline_table() => {
-                &mut v
-                    .as_inline_table_mut()
-                    .unwrap()
+            Item::Table(ref mut t) => Some(t.entry(self).or_insert(Item::None)),
+            Item::Value(ref mut v) if v.is_inline_table() => v.as_inline_table_mut().map(|t| {
+                &mut t
                     .items
                     .entry(InternalString::from(self))
                     .or_insert_with(|| TableKeyValue::new(Key::new(self), Item::None))
                     .value
-            }
-            _ => panic!("cannot access key {}", self),
+            }),
+            _ => None,
         }
     }
 }
@@ -84,8 +71,8 @@ impl Index for String {
     fn index<'v>(&self, v: &'v Item) -> Option<&'v Item> {
         self[..].index(v)
     }
-    fn index_or_insert<'v>(&self, v: &'v mut Item) -> &'v mut Item {
-        self[..].index_or_insert(v)
+    fn index_mut<'v>(&self, v: &'v mut Item) -> Option<&'v mut Item> {
+        self[..].index_mut(v)
     }
 }
 
@@ -96,8 +83,8 @@ where
     fn index<'v>(&self, v: &'v Item) -> Option<&'v Item> {
         (**self).index(v)
     }
-    fn index_or_insert<'v>(&self, v: &'v mut Item) -> &'v mut Item {
-        (**self).index_or_insert(v)
+    fn index_mut<'v>(&self, v: &'v mut Item) -> Option<&'v mut Item> {
+        (**self).index_mut(v)
     }
 }
 
@@ -108,8 +95,7 @@ where
     type Output = Item;
 
     fn index(&self, index: I) -> &Item {
-        static NONE: Item = Item::None;
-        index.index(self).unwrap_or(&NONE)
+        index.index(self).expect("index not found")
     }
 }
 
@@ -118,16 +104,15 @@ where
     I: Index,
 {
     fn index_mut(&mut self, index: I) -> &mut Item {
-        index.index_or_insert(self)
+        index.index_mut(self).expect("index not found")
     }
 }
 
 impl<'s> ops::Index<&'s str> for Table {
     type Output = Item;
 
     fn index(&self, key: &'s str) -> &Item {
-        static NONE: Item = Item::None;
-        self.get(key).unwrap_or(&NONE)
+        self.get(key).expect("index not found")
     }
 }