Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove Item::None #301

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ impl IntoIterator for Array {
self.values
.into_iter()
.filter(|v| v.is_value())
.map(|v| v.into_value().unwrap()),
.map(|v| v.into_value()),
)
}
}
Expand Down
1 change: 0 additions & 1 deletion src/de/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@ impl<'de, 'a> serde::Deserializer<'de> for crate::Item {
V: serde::de::Visitor<'de>,
{
match self {
crate::Item::None => visitor.visit_none(),
crate::Item::Value(v) => v.deserialize_any(visitor),
crate::Item::Table(v) => visitor.visit_map(crate::de::TableMapAccess::new(v)),
crate::Item::ArrayOfTables(v) => {
Expand Down
34 changes: 7 additions & 27 deletions src/index.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
use std::ops;

use crate::document::Document;
use crate::key::Key;
use crate::table::TableKeyValue;
use crate::{value, InlineTable, InternalString, Item, Table, Value};
use crate::{InlineTable, Item, Table, Value};

// copied from
// https://github.com/serde-rs/json/blob/master/src/value/index.rs
Expand Down Expand Up @@ -39,34 +37,16 @@ impl Index for str {
Item::Value(ref v) => v
.as_inline_table()
.and_then(|t| t.items.get(self))
.and_then(|kv| {
if !kv.value.is_none() {
Some(&kv.value)
} else {
None
}
}),
.map(|kv| &kv.value),
_ => None,
}
}
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(
InternalString::from(self),
TableKeyValue::new(Key::new(self), Item::None),
);
*v = value(Value::InlineTable(t));
}
match *v {
Item::Table(ref mut t) => Some(t.entry(self).or_insert(Item::None)),
Item::Value(ref mut v) => 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
}),
Item::Table(ref mut t) => t.get_mut(self),
Item::Value(ref mut v) => v
.as_inline_table_mut()
.and_then(|t| t.items.get_mut(self).map(|kv| &mut kv.value)),
_ => None,
}
}
Expand Down Expand Up @@ -123,7 +103,7 @@ impl<'s> ops::Index<&'s str> for Table {

impl<'s> ops::IndexMut<&'s str> for Table {
fn index_mut(&mut self, key: &'s str) -> &mut Item {
self.entry(key).or_insert(Item::None)
self.get_mut(key).expect("index not found")
}
}

Expand Down
48 changes: 20 additions & 28 deletions src/inline_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,14 +205,8 @@ impl InlineTable {
match self.items.entry(key.into()) {
indexmap::map::Entry::Occupied(mut entry) => {
// Ensure it is a `Value` to simplify `InlineOccupiedEntry`'s code.
let scratch = std::mem::take(&mut entry.get_mut().value);
let scratch = Item::Value(
scratch
.into_value()
// HACK: `Item::None` is a corner case of a corner case, let's just pick a
// "safe" value
.unwrap_or_else(|_| Value::InlineTable(Default::default())),
);
let scratch = std::mem::replace(&mut entry.get_mut().value, crate::value(0));
let scratch = Item::Value(scratch.into_value());
entry.get_mut().value = scratch;

InlineEntry::Occupied(InlineOccupiedEntry { entry })
Expand All @@ -229,14 +223,8 @@ impl InlineTable {
match self.items.entry(key.get().into()) {
indexmap::map::Entry::Occupied(mut entry) => {
// Ensure it is a `Value` to simplify `InlineOccupiedEntry`'s code.
let scratch = std::mem::take(&mut entry.get_mut().value);
let scratch = Item::Value(
scratch
.into_value()
// HACK: `Item::None` is a corner case of a corner case, let's just pick a
// "safe" value
.unwrap_or_else(|_| Value::InlineTable(Default::default())),
);
let scratch = std::mem::replace(&mut entry.get_mut().value, crate::value(0));
let scratch = Item::Value(scratch.into_value());
entry.get_mut().value = scratch;

InlineEntry::Occupied(InlineOccupiedEntry { entry })
Expand Down Expand Up @@ -285,7 +273,7 @@ impl InlineTable {
let kv = TableKeyValue::new(Key::new(key), Item::Value(value));
self.items
.insert(InternalString::from(key), kv)
.and_then(|kv| kv.value.into_value().ok())
.map(|kv| kv.value.into_value())
}

/// Inserts a key-value pair into the map.
Expand All @@ -294,21 +282,20 @@ impl InlineTable {
self.items
.insert(InternalString::from(key.get()), kv)
.filter(|kv| kv.value.is_value())
.map(|kv| kv.value.into_value().unwrap())
.map(|kv| kv.value.into_value())
}

/// Removes an item given the key.
pub fn remove(&mut self, key: &str) -> Option<Value> {
self.items
.shift_remove(key)
.and_then(|kv| kv.value.into_value().ok())
self.items.shift_remove(key).map(|kv| kv.value.into_value())
}

/// Removes a key from the map, returning the stored key and value if the key was previously in the map.
pub fn remove_entry(&mut self, key: &str) -> Option<(Key, Value)> {
self.items.shift_remove(key).and_then(|kv| {
self.items.shift_remove(key).map(|kv| {
let key = kv.key;
kv.value.into_value().ok().map(|value| (key, value))
let value = kv.value.into_value();
(key, value)
})
}
}
Expand Down Expand Up @@ -351,7 +338,7 @@ impl IntoIterator for InlineTable {
self.items
.into_iter()
.filter(|(_, kv)| kv.value.is_value())
.map(|(k, kv)| (k, kv.value.into_value().unwrap())),
.map(|(k, kv)| (k, kv.value.into_value())),
)
}
}
Expand Down Expand Up @@ -395,6 +382,12 @@ impl TableLike for InlineTable {
.map(|(_, kv)| (kv.key.as_mut(), &mut kv.value)),
)
}
fn len(&self) -> usize {
self.len()
}
fn is_empty(&self) -> bool {
self.is_empty()
}
fn get<'s>(&'s self, key: &str) -> Option<&'s Item> {
self.items.get(key).map(|kv| &kv.value)
}
Expand All @@ -405,8 +398,7 @@ impl TableLike for InlineTable {
self.contains_key(key)
}
fn insert(&mut self, key: &str, value: Item) -> Option<Item> {
self.insert(key, value.into_value().unwrap())
.map(Item::Value)
self.insert(key, value.into_value()).map(Item::Value)
}
fn remove(&mut self, key: &str) -> Option<Item> {
self.remove(key).map(Item::Value)
Expand Down Expand Up @@ -526,12 +518,12 @@ impl<'a> InlineOccupiedEntry<'a> {
pub fn insert(&mut self, value: Value) -> Value {
let mut value = Item::Value(value);
std::mem::swap(&mut value, &mut self.entry.get_mut().value);
value.into_value().unwrap()
value.into_value()
}

/// Takes the value out of the entry, and returns it
pub fn remove(self) -> Value {
self.entry.shift_remove().value.into_value().unwrap()
self.entry.shift_remove().value.into_value()
}
}

Expand Down
40 changes: 7 additions & 33 deletions src/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ use crate::{Array, InlineTable, Table, Value};
/// Type representing either a value, a table, an array of tables, or none.
#[derive(Debug, Clone)]
pub enum Item {
/// Type representing none.
None,
/// Type representing value.
Value(Value),
/// Type representing table.
Expand All @@ -18,24 +16,12 @@ pub enum Item {
ArrayOfTables(ArrayOfTables),
}

impl Item {
/// Sets `self` to the given item iff `self` is none and
/// returns a mutable reference to `self`.
pub fn or_insert(&mut self, item: Item) -> &mut Item {
if self.is_none() {
*self = item
}
self
}
}

// TODO: This should be generated by macro or derive
/// Downcasting
impl Item {
/// Text description of value type
pub fn type_name(&self) -> &'static str {
match self {
Item::None => "none",
Item::Value(v) => v.type_name(),
Item::Table(..) => "table",
Item::ArrayOfTables(..) => "array of tables",
Expand Down Expand Up @@ -113,24 +99,23 @@ impl Item {
}
}
/// Casts `self` to value.
pub fn into_value(self) -> Result<Value, Self> {
pub fn into_value(self) -> Value {
match self {
Item::None => Err(self),
Item::Value(v) => Ok(v),
Item::Value(v) => v,
Item::Table(v) => {
let v = v.into_inline_table();
Ok(Value::InlineTable(v))
Value::InlineTable(v)
}
Item::ArrayOfTables(v) => {
let v = v.into_array();
Ok(Value::Array(v))
Value::Array(v)
}
}
}
/// In-place convert to a value
pub fn make_value(&mut self) {
let other = std::mem::take(self);
let other = other.into_value().map(Item::Value).unwrap_or(Item::None);
let other = std::mem::replace(self, value(0));
let other = Item::Value(other.into_value());
*self = other;
}
/// Casts `self` to table.
Expand Down Expand Up @@ -164,7 +149,7 @@ impl Item {
}
// Starting private because the name is unclear
pub(crate) fn make_item(&mut self) {
let other = std::mem::take(self);
let other = std::mem::replace(self, value(0));
let other = match other.into_table().map(crate::Item::Table) {
Ok(i) => i,
Err(i) => i,
Expand All @@ -187,10 +172,6 @@ impl Item {
pub fn is_array_of_tables(&self) -> bool {
self.as_array_of_tables().is_some()
}
/// Returns true iff `self` is `None`.
pub fn is_none(&self) -> bool {
matches!(*self, Item::None)
}

// Duplicate Value downcasting API

Expand Down Expand Up @@ -296,12 +277,6 @@ impl Item {
}
}

impl Default for Item {
fn default() -> Self {
Item::None
}
}

impl FromStr for Item {
type Err = crate::TomlError;

Expand All @@ -315,7 +290,6 @@ impl FromStr for Item {
impl std::fmt::Display for Item {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match &self {
Item::None => Ok(()),
Item::Value(v) => v.fmt(f),
Item::Table(v) => v.fmt(f),
Item::ArrayOfTables(v) => v.fmt(f),
Expand Down
1 change: 0 additions & 1 deletion src/parser/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,6 @@ impl TomlParser {
Item::Table(ref mut sweet_child_of_mine) => {
table = sweet_child_of_mine;
}
_ => unreachable!(),
}
}
Ok(table)
Expand Down
1 change: 0 additions & 1 deletion src/ser/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ impl crate::visit_mut::VisitMut for Pretty {

fn remove_table_prefix(node: &mut crate::Item) {
match node {
crate::Item::None => {}
crate::Item::Value(_) => {}
crate::Item::Table(t) => t.decor_mut().set_prefix(""),
crate::Item::ArrayOfTables(a) => {
Expand Down
26 changes: 8 additions & 18 deletions src/ser/table.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use super::{Error, ErrorKind, KeySerializer};
use super::{Error, KeySerializer};

#[doc(hidden)]
pub struct SerializeItemTable {
Expand Down Expand Up @@ -111,17 +111,12 @@ impl serde::ser::SerializeMap for SerializeKeyValuePairs {
let item = match res {
Ok(item) => item,
Err(e) => {
if e.kind != ErrorKind::UnsupportedNone {
return Err(e);
}
crate::Item::None
return Err(e);
}
};
if !item.is_none() {
let key = self.key.take().unwrap();
let kv = crate::table::TableKeyValue::new(crate::Key::new(&key), item);
self.items.insert(key, kv);
}
let key = self.key.take().unwrap();
let kv = crate::table::TableKeyValue::new(crate::Key::new(&key), item);
self.items.insert(key, kv);
Ok(())
}

Expand All @@ -146,16 +141,11 @@ impl serde::ser::SerializeStruct for SerializeKeyValuePairs {
let item = match res {
Ok(item) => item,
Err(e) => {
if e.kind != ErrorKind::UnsupportedNone {
return Err(e);
}
crate::Item::None
return Err(e);
}
};
if !item.is_none() {
let kv = crate::table::TableKeyValue::new(crate::Key::new(key), item);
self.items.insert(crate::InternalString::from(key), kv);
}
let kv = crate::table::TableKeyValue::new(crate::Key::new(key), item);
self.items.insert(crate::InternalString::from(key), kv);
Ok(())
}

Expand Down