Skip to content
This repository has been archived by the owner on Mar 25, 2024. It is now read-only.

Hide numeric types #64

Merged
merged 14 commits into from
Apr 25, 2017
Merged

Hide numeric types #64

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
817eb67
First pass at hiding the numberic types
drusellers Apr 22, 2017
960096e
Fixing tests
drusellers Apr 23, 2017
0b81758
Switch to unreachable calls
drusellers Apr 23, 2017
382d90f
s/JSON/YAML/g
drusellers Apr 23, 2017
1e319d1
Adding u64 to the macro rules
drusellers Apr 23, 2017
0454a70
Macro correction
drusellers Apr 23, 2017
ef72b31
Delegate Value.is_f64 to Number.is_f64
drusellers Apr 23, 2017
d1bf380
Correcting clippy lints
drusellers Apr 23, 2017
995dac9
fixing clippy lints
drusellers Apr 23, 2017
466a35e
turn clippy off
drusellers Apr 23, 2017
bc121fc
Adjust lint allowance using clippy feature
killercup Apr 24, 2017
f9526af
Adding NaN and Infinite support
drusellers Apr 25, 2017
c8952e5
Add integer tests
drusellers Apr 25, 2017
a2f17a9
Tidying up the mess
drusellers Apr 25, 2017
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/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ extern crate yaml_rust;

pub use self::de::{from_reader, from_slice, from_str};
pub use self::ser::{to_string, to_vec, to_writer};
pub use self::value::{Sequence, Value, from_value, to_value};
pub use self::value::{Sequence, Value, from_value, to_value, Number};
pub use self::error::{Error, Result};
pub use self::mapping::Mapping;

Expand Down
37 changes: 26 additions & 11 deletions src/value/de.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ use serde::de::{
VariantAccess,
Visitor,
};
use num_traits::NumCast;

use super::Value;
use mapping::Mapping;
use error::Error;
use value::number::Number;

impl<'de> Deserialize<'de> for Value {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
Expand All @@ -41,22 +41,19 @@ impl<'de> Deserialize<'de> for Value {
fn visit_i64<E>(self, i: i64) -> Result<Value, E>
where E: SError,
{
Ok(Value::I64(i))
Ok(Value::Number(Number::from(i)))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, reading this again I can't help but think I'd write Value::Number(i.into())

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

easy change!

}

fn visit_u64<E>(self, u: u64) -> Result<Value, E>
where E: SError,
{
match NumCast::from(u) {
Some(i) => Ok(Value::I64(i)),
None => Ok(Value::String(u.to_string())),
}
Ok(Value::Number(Number::from(u)))
}

fn visit_f64<E>(self, f: f64) -> Result<Value, E>
where E: SError,
{
Ok(Value::F64(f))
Ok(Value::Number(Number::from_f64(f).unwrap()))
}

fn visit_str<E>(self, s: &str) -> Result<Value, E>
Expand Down Expand Up @@ -128,8 +125,17 @@ impl<'de> Deserializer<'de> for Value {
match self {
Value::Null => visitor.visit_unit(),
Value::Bool(v) => visitor.visit_bool(v),
Value::I64(i) => visitor.visit_i64(i),
Value::F64(f) => visitor.visit_f64(f),
Value::Number(ref n) => {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not need to use ref. We own the number.

if let Some(u) = n.as_u64() {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should use Number as Deserializer<'de> like serde_json does. That way you avoid the if / else if / else if / else unreachable mess.

Value::Number(n) => n.deserialize_any(visitor)

visitor.visit_u64(u)
} else if let Some(i) = n.as_i64() {
visitor.visit_i64(i)
} else if let Some(f) = n.as_f64() {
visitor.visit_f64(f)
} else {
unreachable!("unexpected number")
}
}
Value::String(v) => visitor.visit_string(v),
Value::Sequence(v) => {
let len = v.len();
Expand Down Expand Up @@ -420,8 +426,17 @@ impl Value {
match *self {
Value::Null => Unexpected::Unit,
Value::Bool(b) => Unexpected::Bool(b),
Value::I64(i) => Unexpected::Signed(i),
Value::F64(f) => Unexpected::Float(f),
Value::Number(ref n) => {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to implement this as n.unexpected() to remove the unreachable branch.

if let Some(u) = n.as_u64() {
Unexpected::Unsigned(u)
} else if let Some(i) = n.as_i64() {
Unexpected::Signed(i)
} else if let Some(f) = n.as_f64() {
Unexpected::Float(f)
} else {
unreachable!("unexpected number")
}
}
Value::String(ref s) => Unexpected::Str(s),
Value::Sequence(_) => Unexpected::Seq,
Value::Mapping(_) => Unexpected::Map,
Expand Down
7 changes: 4 additions & 3 deletions src/value/from.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use super::Value;
use mapping::Mapping;
use value::number::Number;

// Implement a bunch of conversion to make it easier to create YAML values
// on the fly.
Expand All @@ -9,7 +10,7 @@ macro_rules! from_integer {
$(
impl From<$ty> for Value {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This implementation now works for f32 and f64 too, no need for a separate From<f32> and From<f64> below.

fn from(n: $ty) -> Self {
Value::I64(n as i64)
Value::Number(Number::from(n))
}
}
)*
Expand All @@ -18,7 +19,7 @@ macro_rules! from_integer {

from_integer! {
i8 i16 i32 i64 isize
u8 u16 u32 usize
u8 u16 u32 u64 usize
}

impl From<f32> for Value {
Expand Down Expand Up @@ -57,7 +58,7 @@ impl From<f64> for Value {
/// # }
/// ```
fn from(f: f64) -> Self {
Value::F64(f)
Value::Number(Number::from_f64(f).unwrap())
}
}

Expand Down
14 changes: 7 additions & 7 deletions src/value/index.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use std::ops;

use super::Value;
use mapping::Mapping;
use value::number::Number;

// The original implementation of the indexing tricks below is from `serde_json`.

Expand Down Expand Up @@ -32,14 +33,14 @@ impl Index for usize {
fn index_into<'v>(&self, v: &'v Value) -> Option<&'v Value> {
match *v {
Value::Sequence(ref vec) => vec.get(*self),
Value::Mapping(ref vec) => vec.get(&Value::I64(*self as i64)),
Value::Mapping(ref vec) => vec.get(&Value::Number(Number::from(*self as i64))),
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This casting is lossy and no longer necessary. Since Number is opaque, and if the implementation of Eq and Hash are correct, you should be able to insert a number into a map by i64 key and look it up by u64 key. Same thing below.

_ => None,
}
}
fn index_into_mut<'v>(&self, v: &'v mut Value) -> Option<&'v mut Value> {
match *v {
Value::Sequence(ref mut vec) => vec.get_mut(*self),
Value::Mapping(ref mut vec) => vec.get_mut(&Value::I64(*self as i64)),
Value::Mapping(ref mut vec) => vec.get_mut(&Value::Number(Number::from(*self as i64))),
_ => None,
}
}
Expand All @@ -61,10 +62,10 @@ impl Index for usize {
Value::Mapping(ref mut map) => {
// TODO: use entry() once LinkedHashMap supports entry()
// https://github.com/contain-rs/linked-hash-map/issues/5
if !map.contains_key(&Value::I64(*self as i64)) {
map.insert(Value::I64(*self as i64), Value::Null);
if !map.contains_key(&Value::Number(Number::from(*self as i64))) {
map.insert(Value::Number(Number::from(*self as i64)), Value::Null);
}
map.get_mut(&Value::I64(*self as i64)).unwrap()
map.get_mut(&Value::Number(Number::from(*self as i64))).unwrap()
},
_ => panic!("cannot access index {} of YAML {}", self, Type(v)),
}
Expand Down Expand Up @@ -165,8 +166,7 @@ impl<'a> fmt::Display for Type<'a> {
match *self.0 {
Value::Null => formatter.write_str("null"),
Value::Bool(_) => formatter.write_str("boolean"),
Value::I64(_) => formatter.write_str("integer"),
Value::F64(_) => formatter.write_str("float"),
Value::Number(_) => formatter.write_str("number"),
Value::String(_) => formatter.write_str("string"),
Value::Sequence(_) => formatter.write_str("sequence"),
Value::Mapping(_) => formatter.write_str("mapping"),
Expand Down
102 changes: 70 additions & 32 deletions src/value/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use mapping::Mapping;
use ser::Serializer;

use self::index::Index;
pub use self::number::Number;

/// Represents any valid YAML value.
#[derive(Clone, PartialOrd, Debug)]
Expand All @@ -29,10 +30,8 @@ pub enum Value {
Null,
/// Represents a YAML boolean.
Bool(bool),
/// Represents a YAML integer value.
I64(i64),
/// Represents a YAML floating-point value.
F64(f64),
/// Represents a YAML numerical value
Number(Number),
/// Represents a YAML string.
String(String),
/// Represents a YAML sequence in which the elements are
Expand Down Expand Up @@ -98,13 +97,13 @@ impl Value {
///
/// ```rust
/// # extern crate serde_yaml;
/// # use serde_yaml::Value;
/// # use serde_yaml::{Value, Number};
/// #
/// # fn yaml(i: &str) -> serde_yaml::Value { serde_yaml::from_str(i).unwrap() }
/// # fn main() {
/// let object: Value = yaml(r#"{ A: 65, B: 66, C: 67 }"#);
/// let x = object.get("A").unwrap();
/// assert_eq!(x, &Value::I64(65));
/// assert_eq!(x, &Value::Number(Number::from(65)));
///
/// let sequence: Value = yaml(r#"[ "A", "B", "C" ]"#);
/// let x = sequence.get(2).unwrap();
Expand All @@ -120,7 +119,7 @@ impl Value {
///
/// ```rust
/// # extern crate serde_yaml;
/// # use serde_yaml::Value;
/// # use serde_yaml::{Value, Number};
/// #
/// # fn yaml(i: &str) -> serde_yaml::Value { serde_yaml::from_str(i).unwrap() }
/// # fn main() {
Expand All @@ -137,7 +136,7 @@ impl Value {
/// assert_eq!(object["D"], Value::Null);
/// assert_eq!(object[0]["x"]["y"]["z"], Value::Null);
///
/// assert_eq!(object[Value::I64(42)], Value::Bool(true));
/// assert_eq!(object[Value::Number(Number::from(42))], Value::Bool(true));
/// assert_eq!(object[42], Value::Bool(true));
/// # }
/// ```
Expand Down Expand Up @@ -263,7 +262,49 @@ impl Value {
/// ```
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should be an is_number method up here like what serde_json has.

pub fn as_i64(&self) -> Option<i64> {
match *self {
Value::I64(i) => Some(i),
Value::Number(ref i) => i.as_i64(),
_ => None,
}
}

/// Returns true if the `Value` is an integer between `u64::MIN` and
/// `u64::MAX`.
///
/// For any Value on which `is_u64` returns true, `as_u64` is guaranteed to
/// return the integer value.
///
/// ```rust
/// # use serde_yaml::Value;
/// let v: Value = serde_yaml::from_str("1337").unwrap();
/// assert!(v.is_u64());
/// ```
///
/// ```rust
/// # use serde_yaml::Value;
/// let v: Value = serde_yaml::from_str("null").unwrap();
/// assert!(!v.is_u64());
/// ```
pub fn is_u64(&self) -> bool {
self.as_u64().is_some()
}

/// If the `Value` is an integer, represent it as i64 if possible. Returns
/// None otherwise.
///
/// ```rust
/// # use serde_yaml::Value;
/// let v: Value = serde_yaml::from_str("1337").unwrap();
/// assert_eq!(v.as_u64(), Some(1337));
/// ```
///
/// ```rust
/// # use serde_yaml::Value;
/// let v: Value = serde_yaml::from_str("false").unwrap();
/// assert_eq!(v.as_u64(), None);
/// ```
pub fn as_u64(&self) -> Option<u64> {
match *self {
Value::Number(ref i) => i.as_u64(),
_ => None,
}
}
Expand Down Expand Up @@ -307,7 +348,7 @@ impl Value {
/// ```
pub fn as_f64(&self) -> Option<f64> {
match *self {
Value::F64(i) => Some(i),
Value::Number(ref i) => i.as_f64(),
_ => None,
}
}
Expand Down Expand Up @@ -374,9 +415,9 @@ impl Value {
/// Returns None otherwise.
///
/// ```rust
/// # use serde_yaml::Value;
/// # use serde_yaml::{Value, Number};
/// let v: Value = serde_yaml::from_str("[1, 2]").unwrap();
/// assert_eq!(v.as_sequence(), Some(&vec![Value::I64(1), Value::I64(2)]));
/// assert_eq!(v.as_sequence(), Some(&vec![Value::Number(Number::from(1)), Value::Number(Number::from(2))]));
/// ```
///
/// ```rust
Expand All @@ -395,11 +436,11 @@ impl Value {
/// possible. Returns None otherwise.
///
/// ```rust
/// # use serde_yaml::Value;
/// # use serde_yaml::{Value, Number};
/// let mut v: Value = serde_yaml::from_str("[1]").unwrap();
/// let s = v.as_sequence_mut().unwrap();
/// s.push(Value::I64(2));
/// assert_eq!(s, &vec![Value::I64(1), Value::I64(2)]);
/// s.push(Value::Number(Number::from(2)));
/// assert_eq!(s, &vec![Value::Number(Number::from(1)), Value::Number(Number::from(2))]);
/// ```
///
/// ```rust
Expand Down Expand Up @@ -435,11 +476,11 @@ impl Value {
/// Returns None otherwise.
///
/// ```rust
/// # use serde_yaml::{Value, Mapping};
/// # use serde_yaml::{Value, Mapping, Number};
/// let v: Value = serde_yaml::from_str("a: 42").unwrap();
///
/// let mut expected = Mapping::new();
/// expected.insert(Value::String("a".into()),Value::I64(42));
/// expected.insert(Value::String("a".into()),Value::Number(Number::from(42)));
///
/// assert_eq!(v.as_mapping(), Some(&expected));
/// ```
Expand All @@ -460,14 +501,14 @@ impl Value {
/// Returns None otherwise.
///
/// ```rust
/// # use serde_yaml::{Value, Mapping};
/// # use serde_yaml::{Value, Mapping, Number};
/// let mut v: Value = serde_yaml::from_str("a: 42").unwrap();
/// let m = v.as_mapping_mut().unwrap();
/// m.insert(Value::String("b".into()),Value::I64(21));
/// m.insert(Value::String("b".into()),Value::Number(Number::from(21)));
///
/// let mut expected = Mapping::new();
/// expected.insert(Value::String("a".into()),Value::I64(42));
/// expected.insert(Value::String("b".into()),Value::I64(21));
/// expected.insert(Value::String("a".into()),Value::Number(Number::from(42)));
/// expected.insert(Value::String("b".into()),Value::Number(Number::from(21)));
///
/// assert_eq!(m, &expected);
/// ```
Expand All @@ -489,11 +530,11 @@ fn yaml_to_value(yaml: Yaml) -> Value {
match yaml {
Yaml::Real(f) => {
match f.parse() {
Ok(f) => Value::F64(f),
Ok(f) => Value::Number(Number::from_f64(f).unwrap()),
Err(_) => Value::String(f),
}
}
Yaml::Integer(i) => Value::I64(i),
Yaml::Integer(i) => Value::Number(Number::from(i)),
Yaml::String(s) => Value::String(s),
Yaml::Boolean(b) => Value::Bool(b),
Yaml::Array(sequence) => Value::Sequence(sequence.into_iter().map(yaml_to_value).collect()),
Expand All @@ -513,21 +554,18 @@ impl Hash for Value {
match *self {
Value::Null => 0.hash(state),
Value::Bool(b) => (1, b).hash(state),
Value::I64(i) => (2, i).hash(state),
Value::F64(_) => {
// you should feel bad for using f64 as a map key
3.hash(state);
}
Value::String(ref s) => (4, s).hash(state),
Value::Sequence(ref seq) => (5, seq).hash(state),
Value::Mapping(ref map) => (6, map).hash(state),
Value::Number(ref i) => (2, i).hash(state),
Value::String(ref s) => (3, s).hash(state),
Value::Sequence(ref seq) => (4, seq).hash(state),
Value::Mapping(ref map) => (5, map).hash(state),
}
}
}

mod index;
mod partial_eq;
mod from;
mod number;

mod ser;
mod de;
mod de;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please set your editor to end every line of characters with a newline character. Otherwise this does not conform to the POSIX definition of a "text file" and will break some tools.