Cleanup.

Raw f64 in some more places, use rust std f64 parsing for numbers.
Also fix some numeric comparisions and remove a duplicate test.

Author: kaj

rsass/src/css/value.rs

@@ -6,7 +6,7 @@ use crate::sass::Function;
 use crate::value::{Color, ListSeparator, Number, Numeric, Operator};
 
 /// A css value.
-#[derive(Clone, Debug, Eq, PartialOrd)]
+#[derive(Clone, Debug, PartialOrd)]
 pub enum Value {
     /// A special kind of escape.  Only really used for !important.
     Bang(String),
@@ -311,6 +311,7 @@ impl PartialEq for Value {
         }
     }
 }
+impl Eq for Value {}
 
 impl From<bool> for Value {
     fn from(v: bool) -> Self {

rsass/src/parser/value.rs

@@ -10,18 +10,18 @@ use super::{
     input_to_string, list_or_single, position, sass_string, PResult, Span,
 };
 use crate::sass::{BinOp, SassString, Value};
-use crate::value::{ListSeparator, Number, Numeric, Operator, Rgba};
+use crate::value::{ListSeparator, Numeric, Operator, Rgba};
 use nom::branch::alt;
 use nom::bytes::complete::{tag, tag_no_case};
 use nom::character::complete::{
     alphanumeric1, char, digit1, multispace0, multispace1, one_of,
 };
 use nom::combinator::{
-    cut, into, map, map_res, not, opt, peek, recognize, success, value,
+    cut, into, map, map_opt, map_res, not, opt, peek, recognize, value,
     verify,
 };
 use nom::error::context;
-use nom::multi::{fold_many0, fold_many1, many0, many_m_n, separated_list1};
+use nom::multi::{fold_many0, many0, many_m_n, separated_list1};
 use nom::sequence::{delimited, pair, preceded, terminated, tuple};
 use std::str::from_utf8;
 
@@ -332,68 +332,20 @@ pub fn numeric(input: Span) -> PResult<Numeric> {
     })(input)
 }
 
-pub fn number(input: Span) -> PResult<Number> {
-    map(
-        tuple((
-            sign_neg,
+pub fn number(input: Span) -> PResult<f64> {
+    map_opt(
+        recognize(delimited(
+            opt(one_of("+-")),
             alt((
-                map(pair(decimal_integer, decimal_decimals), |(n, d)| n + d),
-                decimal_decimals,
-                decimal_integer,
-            )),
-            opt(preceded(
-                alt((tag("e"), tag("E"))),
-                tuple((sign_neg, decimal_i32)),
+                terminated(digit1, opt(terminated(char('.'), digit1))),
+                preceded(char('.'), digit1),
             )),
+            opt(delimited(one_of("eE"), opt(one_of("+-")), digit1)),
         )),
-        |(is_neg, num, exp)| {
-            let value = if is_neg { -num } else { num };
-            Number::from(if let Some((e_neg, e_val)) = exp {
-                let e_val = if e_neg { -e_val } else { e_val };
-                // Note: powi sounds right, but looses some precision.
-                value * 10f64.powf(e_val.into())
-            } else {
-                value
-            })
-        },
-    )(input)
-}
-
-/// Parse true on `-` and false on `+` or no sign.
-fn sign_neg(input: Span) -> PResult<bool> {
-    alt((
-        value(true, char('-')),
-        value(false, char('+')),
-        success(false),
-    ))(input)
-}
-
-pub fn decimal_integer(input: Span) -> PResult<f64> {
-    map(digit1, |s: Span| {
-        s.fragment()
-            .iter()
-            .fold(0.0, |r, d| (r * 10.) + f64::from(d - b'0'))
-    })(input)
-}
-pub fn decimal_i32(input: Span) -> PResult<i32> {
-    fold_many1(
-        // Note: We should use bytes directly, one_of returns a char.
-        one_of("0123456789"),
-        || 0,
-        |r, d| (r * 10) + i32::from(d as u8 - b'0'),
+        |s: Span| from_utf8(s.fragment()).ok()?.parse().ok(),
     )(input)
 }
 
-pub fn decimal_decimals(input: Span) -> PResult<f64> {
-    map(preceded(char('.'), digit1), |s: Span| {
-        let digits = s.fragment();
-        digits
-            .iter()
-            .fold(0.0, |r, d| (r * 10.) + f64::from(d - b'0'))
-            * (10f64).powf(-(digits.len() as f64))
-    })(input)
-}
-
 pub fn variable_nomod(input: Span) -> PResult<Value> {
     let (rest, name) = preceded(char('$'), identifier)(input)?;
     let pos = input.up_to(&rest).to_owned();
@@ -584,12 +536,12 @@ mod test {
 
     #[test]
     fn simple_number() {
-        check_expr("4;", number(4.))
+        check_expr("4;", Value::scalar(4.))
     }
 
     #[test]
     fn simple_number_neg() {
-        check_expr("-4;", number(-4.))
+        check_expr("-4;", Value::scalar(-4.))
     }
 
     #[test]
@@ -599,23 +551,19 @@ mod test {
 
     #[test]
     fn simple_number_dec() {
-        check_expr("4.34;", number(4.34))
+        check_expr("4.34;", Value::scalar(4.34))
     }
     #[test]
     fn simple_number_onlydec() {
-        check_expr(".34;", number(0.34))
+        check_expr(".34;", Value::scalar(0.34))
     }
     #[test]
     fn simple_number_onlydec_neg() {
-        check_expr("-.34;", number(-0.34))
+        check_expr("-.34;", Value::scalar(-0.34))
     }
     #[test]
     fn simple_number_onlydec_pos() {
-        check_expr("+.34;", number(0.34))
-    }
-
-    fn number(value: f64) -> Value {
-        Value::scalar(value)
+        check_expr("+.34;", Value::scalar(0.34))
     }
 
     #[test]

rsass/src/value/number.rs

@@ -6,12 +6,27 @@ use std::ops::{Add, Div, Mul, Neg, Rem, Sub};
 ///
 /// Only the actual numeric value is included, not any unit.
 /// Internally, a number is represented as a floating-point value.
-#[derive(Clone, PartialEq, PartialOrd)]
+#[derive(Clone)]
 pub struct Number {
     value: f64,
 }
+impl PartialEq for Number {
+    fn eq(&self, other: &Self) -> bool {
+        (self.value - other.value).abs() / self.value.abs() <= f64::EPSILON
+    }
+}
 impl Eq for Number {}
 
+impl PartialOrd for Number {
+    fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
+        if self == other {
+            Some(std::cmp::Ordering::Equal)
+        } else {
+            self.value.partial_cmp(&other.value)
+        }
+    }
+}
+
 impl Neg for Number {
     type Output = Self;
     fn neg(self) -> Self {
@@ -88,28 +103,17 @@ impl Number {
         self.value.is_finite()
     }
 
-    /// Returns true if the number is an integer.
+    /// Convert this number to an `i64` if it is (very close to)
+    /// integer and in range for `i64`, otherwise return `Err(self)`.
     pub fn into_integer(self) -> Result<i64, Self> {
-        let sr = self.value.round();
-        if (sr - self.value).abs() <= f32::EPSILON.into() {
-            Ok(sr as i64)
+        let int = self.value.round() as i64;
+        if ((int as f64) - self.value).abs() <= f32::EPSILON.into() {
+            Ok(int)
         } else {
             Err(self)
         }
     }
 
-    /// Converts to an integer, rounding towards zero.
-    ///
-    /// An integer that is too big to fit in an i64 returns `None`.
-    pub fn to_integer(&self) -> Option<i64> {
-        let i = self.value.ceil() as i64;
-        let ia = i.abs() as f64;
-        if ia <= self.value.abs() && ia + 1. >= self.value.abs() {
-            Some(i)
-        } else {
-            None
-        }
-    }
     /// Computes self^p.
     pub fn powi(self, p: i32) -> Self {
         self.value.powi(p).into()
@@ -131,7 +135,7 @@ impl From<i64> for Number {
 }
 impl From<i32> for Number {
     fn from(value: i32) -> Self {
-        Self::from(i64::from(value))
+        Self::from(f64::from(value))
     }
 }
 impl From<usize> for Number {
@@ -305,30 +309,30 @@ impl<'a> fmt::Display for Formatted<'a, Number> {
 
 impl fmt::Debug for Number {
     fn fmt(&self, out: &mut fmt::Formatter) -> fmt::Result {
-        write!(out, "Number {}", self.value)
+        write!(out, "Number({:?})", self.value)
     }
 }
 
 #[test]
 fn debug_integer() {
-    assert_eq!(format!("{:?}", Number::from(17)), "Number 17",);
+    assert_eq!(format!("{:?}", Number::from(17)), "Number(17.0)",);
 }
 #[test]
 fn debug_long_integer() {
-    assert_eq!(format!("{:#?}", Number::from(17)), "Number 17",);
+    assert_eq!(format!("{:#?}", Number::from(17)), "Number(17.0)",);
 }
 
 #[test]
 fn debug_float() {
-    assert_eq!(format!("{:?}", Number::from(17.5)), "Number 17.5",);
+    assert_eq!(format!("{:?}", Number::from(17.5)), "Number(17.5)",);
 }
 
 #[test]
 fn debug_int_value() {
     assert_eq!(
         format!("{:#?}", crate::sass::Value::scalar(17)),
         "Numeric(\
-         \n    Number 17; UnitSet [],\
+         \n    Number(17.0); UnitSet [],\
          \n)",
     );
 }

rsass/src/value/numeric.rs

@@ -5,7 +5,7 @@ use std::ops::{Div, Mul, Neg};
 
 /// A Numeric value is a [`Number`] with a [`Unit`] (which may be
 /// `Unit::None`).
-#[derive(Clone, Eq)]
+#[derive(Clone)]
 pub struct Numeric {
     /// The number value of this numeric.
     pub value: Number,
@@ -55,7 +55,7 @@ impl Numeric {
     pub fn as_unit(&self, unit: Unit) -> Option<Number> {
         self.unit
             .scale_to_unit(&unit)
-            .map(|scale| &self.value * &scale)
+            .map(|scale| &self.value * &Number::from(scale))
     }
     /// Convert this numeric value to a given unit, if possible.
     ///
@@ -67,7 +67,9 @@ impl Numeric {
     /// assert_eq!(inch.as_unit(Unit::Deg), None);
     /// ```
     pub fn as_unitset(&self, unit: &UnitSet) -> Option<Number> {
-        self.unit.scale_to(unit).map(|scale| &self.value * &scale)
+        self.unit
+            .scale_to(unit)
+            .map(|scale| &self.value * &Number::from(scale))
     }
 
     /// Convert this numeric value to a given unit, if possible.
@@ -101,6 +103,7 @@ impl PartialEq for Numeric {
         self.partial_cmp(other) == Some(std::cmp::Ordering::Equal)
     }
 }
+impl Eq for Numeric {}
 
 impl PartialOrd for Numeric {
     fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {

rsass/src/value/unit.rs

@@ -1,6 +1,5 @@
 //! The Unit enum defines css units
 
-use crate::value::Number;
 use std::f64::consts::FRAC_1_PI;
 use std::fmt;
 
@@ -150,9 +149,9 @@ impl Unit {
     /// Get a scaling factor to convert this unit to another unit.
     ///
     /// Returns None if the units are of different dimension.
-    pub fn scale_to(&self, other: &Self) -> Option<Number> {
+    pub fn scale_to(&self, other: &Self) -> Option<f64> {
         if self == other {
-            Some(1.into())
+            Some(1.)
         } else if self.dimension() == other.dimension() {
             Some(self.scale_factor() / other.scale_factor())
         } else {
@@ -163,12 +162,12 @@ impl Unit {
     /// Some of these are exact and correct, others are more arbitrary.
     /// When comparing 10cm to 4in, these factors will give correct results.
     /// When comparing rems to vw, who can say?
-    pub(crate) fn scale_factor(&self) -> Number {
+    pub(crate) fn scale_factor(&self) -> f64 {
         #[allow(clippy::match_same_arms)]
-        Number::from(match *self {
-            Self::Em | Self::Rem => 10. / 2.,
-            Self::Ex => 10. / 3.,
-            Self::Ch => 10. / 4.,
+        match *self {
+            Self::Em | Self::Rem => 5.,
+            Self::Ex => 3.,
+            Self::Ch => 2.,
             Self::Vw | Self::Vh | Self::Vmin | Self::Vmax => 1.,
             Self::Cm => 10.,
             Self::Mm => 1.,
@@ -198,7 +197,7 @@ impl Unit {
             Self::None => 1.,
 
             Self::Unknown(_) => 1.,
-        })
+        }
     }
 }