Improved calculation checking.

rsass/src/css/binop.rs

@@ -1,6 +1,6 @@
 use super::{is_function_name, InvalidCss, Value};
 use crate::output::{Format, Formatted};
-use crate::value::{CssDimension, Numeric, Operator};
+use crate::value::{CssDimensionSet, Numeric, Operator};
 use std::fmt::{self, Display, Write};
 
 /// A binary operation.
@@ -59,9 +59,7 @@ impl BinOp {
                 if self.a.is_calculation() || self.b.is_calculation() {
                     Err(InvalidCss::UndefOp(self.into()))
                 } else {
-                    fn cmp_dim(
-                        x: &Numeric,
-                    ) -> Option<Vec<(CssDimension, i8)>> {
+                    fn cmp_dim(x: &Numeric) -> Option<CssDimensionSet> {
                         let u = &x.unit;
                         if u.is_known() && !u.is_percent() {
                             Some(u.css_dimension())

rsass/src/parser/value.rs

@@ -479,6 +479,9 @@ fn literal_or_color(s: SassString) -> Value {
         if val == "infinity" {
             return Value::scalar(f64::INFINITY);
         }
+        if val == "NaN" {
+            return Value::scalar(f64::NAN);
+        }
         if let Some(rgba) = Rgba::from_name(val) {
             return Value::Color(rgba, Some(val.to_string()));
         }

rsass/src/sass/functions/call_error.rs

@@ -1,6 +1,7 @@
 use super::super::{ArgsError, Name};
-use crate::css::BadSelector;
+use crate::css::{BadSelector, Value};
 use crate::input::SourcePos;
+use crate::output::Format;
 use crate::{Error, Invalid, ScopeError};
 use std::fmt;
 
@@ -26,6 +27,15 @@ impl CallError {
         CallError::Invalid(Invalid::AtError(msg.to_string()))
     }
 
+    /// The values were expected to be compatible, but wasn't.
+    pub fn incompatible_values(a: &Value, b: &Value) -> Self {
+        Self::msg(format!(
+            "{} and {} are incompatible.",
+            a.format(Format::introspect()),
+            b.format(Format::introspect()),
+        ))
+    }
+
     /// Map this error to a [`crate::Error`].
     pub fn called_from(self, call_pos: &SourcePos, name: &str) -> Error {
         match self {

rsass/src/sass/functions/math.rs

@@ -1,5 +1,5 @@
 use super::{
-    check, expected_to, is_not, is_special, CallError, CheckedArg,
+    check, css_dim, expected_to, is_not, is_special, CallError, CheckedArg,
     FunctionMap, ResolvedArgs, Scope,
 };
 use crate::css::{BinOp, CallArgs, CssString, Value};
@@ -323,16 +323,21 @@ pub fn expose(m: &Scope, global: &mut FunctionMap) {
     def!(global, mod(y, x), |s| {
         fn real_mod(s: &ResolvedArgs) -> Result<Value, CallError> {
             let y: Numeric = s.get(name!(y))?;
-            let x = s.get_map(name!(x), |v| {
-                let v = Numeric::try_from(v)?;
-                v.as_unitset(&y.unit)
-                    .ok_or_else(|| diff_units_msg(&v, &y, name!(y)))
-            })?;
+            let x: Numeric = s.get(name!(x))?;
+            let x = x.as_unitset(&y.unit)
+                    .ok_or_else(|| CallError::msg(diff_units_msg2(&y, &x)))?;
             let unit = y.unit;
             let y = f64::from(y.value);
             let m = f64::from(x);
             let mut y = y.rem(m);
-            if dbg!(dbg!(dbg!(y) * m.signum()) < 0.) { y += m; }
+            if (y * m.signum()).is_sign_negative() {
+                if m.is_finite() {
+                    y += m;
+                    y = y.rem(m);
+                } else {
+                    y = f64::NAN;
+                }
+            }
             let y = y.abs() * m.signum();
             Ok(number(y, unit))
         }
@@ -419,7 +424,15 @@ fn fallback2a(
     a2: Name,
 ) -> Result<Value, ()> {
     let (a1, a2) = match (get_expr_a(s, a1), get_expr_a(s, a2)) {
-        (Ok(a1), Ok(a2)) => (a1, a2),
+        (Ok(a1), Ok(a2)) => {
+            let dim1 = css_dim(&a1);
+            let dim2 = css_dim(&a2);
+            if (dim1 == dim2) || dim1.is_none() || dim2.is_none() {
+                (a1, a2)
+            } else {
+                return Err(());
+            }
+        }
         _ => return Err(()),
     };
     Ok(Value::Call(name.into(), CallArgs::from_list(vec![a1, a2])))
@@ -578,6 +591,19 @@ fn diff_units_msg(
     )
 }
 
+fn diff_units_msg2(one: &Numeric, other: &Numeric) -> String {
+    format!(
+        "{} and {} are incompatible{}.",
+        one.format(Format::introspect()),
+        other.format(Format::introspect()),
+        if one.is_no_unit() || other.is_no_unit() {
+            " (one has units and the other doesn't)"
+        } else {
+            ""
+        }
+    )
+}
+
 pub(crate) fn clamp_fn(s: &ResolvedArgs) -> Result<Value, CallError> {
     let min_v = s.get::<Numeric>(name!(min))?;
     let check_numeric_compat_unit = |v: Value| -> Result<Numeric, String> {

rsass/src/sass/functions/mod.rs

@@ -2,7 +2,7 @@ use super::{Call, Closure, FormalArgs, Name};
 use crate::css::{self, is_not, BinOp, CallArgs, CssString, Value};
 use crate::input::SourcePos;
 use crate::output::{Format, Formatted};
-use crate::value::{CssDimension, Operator, Quotes};
+use crate::value::{CssDimensionSet, Operator, Quotes};
 use crate::{Scope, ScopeRef};
 use lazy_static::lazy_static;
 use std::collections::BTreeMap;
@@ -289,7 +289,10 @@ lazy_static! {
             } else {
                 let arg = match css_fn_arg(v)? {
                     Value::Paren(arg)
-                        if !matches!(arg.as_ref(), Value::Call(..)) =>
+                        if !matches!(
+                            arg.as_ref(),
+                            Value::Call(..) | Value::Literal(_)
+                        ) =>
                     {
                         *arg
                     }
@@ -300,13 +303,26 @@ lazy_static! {
         });
         def!(f, clamp(min, number = b"null", max = b"null"), |s| {
             self::math::clamp_fn(s).or_else(|_| {
-                let mut args = vec![s.get(name!(min))?];
+                let mut args = vec![s.get::<Value>(name!(min))?];
                 if let Some(b) = s.get_opt(name!(number))? {
                     args.push(b);
                 }
                 if let Some(c) = s.get_opt(name!(max))? {
                     args.push(c);
                 }
+                if let Some((a, rest)) = args.split_first() {
+                    if let Some(adim) = css_dim(a) {
+                        for b in rest {
+                            if let Some(bdim) = css_dim(b) {
+                                if adim != bdim {
+                                    return Err(
+                                        CallError::incompatible_values(a, b),
+                                    );
+                                }
+                            }
+                        }
+                    }
+                }
                 Ok(css::Value::Call(
                     "clamp".into(),
                     css::CallArgs::from_list(args),
@@ -368,7 +384,7 @@ fn css_fn_arg(v: Value) -> Result<Value, CallError> {
 }
 
 // Note: None here is for unknown, e.g. the dimension of something that is not a number.
-fn css_dim(v: &Value) -> Option<Vec<(CssDimension, i8)>> {
+fn css_dim(v: &Value) -> Option<CssDimensionSet> {
     match v {
         // TODO: Handle BinOp recursively (again) (or let in_calc return (Value, CssDimension)?)
         Value::Numeric(num, _) => {

rsass/src/value/mod.rs

@@ -16,5 +16,5 @@ pub use self::numeric::Numeric;
 pub use self::operator::{BadOp, Operator};
 pub use self::quotes::Quotes;
 pub use self::unit::{CssDimension, Dimension, Unit};
-pub use self::unitset::UnitSet;
+pub use self::unitset::{CssDimensionSet, UnitSet};
 pub(crate) use range::{RangeError, ValueRange};

rsass/src/value/unitset.rs

@@ -58,9 +58,10 @@ impl UnitSet {
             .collect::<Vec<_>>()
     }
 
-    pub(crate) fn css_dimension(&self) -> Vec<(CssDimension, i8)> {
+    pub(crate) fn css_dimension(&self) -> CssDimensionSet {
         use std::collections::BTreeMap;
-        self.units
+        let dim = self
+            .units
             .iter()
             .fold(BTreeMap::new(), |mut map, (unit, power)| {
                 let dim = CssDimension::from(unit.dimension());
@@ -71,15 +72,12 @@ impl UnitSet {
             })
             .into_iter()
             .filter(|(_d, power)| *power != 0)
-            .collect::<Vec<_>>()
+            .collect::<Vec<_>>();
+        CssDimensionSet { dim }
     }
+
     pub(crate) fn valid_in_css(&self) -> bool {
-        let dim = self.css_dimension();
-        match &dim[..] {
-            [] => true,
-            [(_d, p)] => *p == 1,
-            _ => false,
-        }
+        self.css_dimension().valid_in_css()
     }
 
     /// Get a scaling factor to convert this unit to another unit.
@@ -250,3 +248,31 @@ impl fmt::Debug for UnitSet {
         out.debug_list().entries(&self.units).finish()
     }
 }
+
+/// The dimension of a numeric value.
+///
+/// May be e.g. lenght, or something complex like length^17*angle*time^-3.
+#[derive(Debug, Default, PartialEq, Eq)]
+pub struct CssDimensionSet {
+    dim: Vec<(CssDimension, i8)>,
+}
+impl CssDimensionSet {
+    /// Return true for the empty dimension, i.e. the dimension of a unitless number.
+    pub fn is_empty(&self) -> bool {
+        self.dim.is_empty()
+    }
+    /// Return true if any unknown unit is part of the dimension.
+    pub fn is_unknown(&self) -> bool {
+        self.dim
+            .iter()
+            .any(|(dim, _)| matches!(dim, CssDimension::Unknown(_)))
+    }
+
+    pub(crate) fn valid_in_css(&self) -> bool {
+        match &self.dim[..] {
+            [] => true,
+            [(_d, p)] => *p == 1,
+            _ => false,
+        }
+    }
+}

rsass/tests/spec/values/calculation/atan2.rs

@@ -95,7 +95,7 @@ mod error {
     );
         }
         #[test]
-        #[ignore] // missing error
+        #[ignore] // wrong error
         fn known_incompatible() {
             assert_eq!(
                 runner().err("a {b: atan2(1deg, 1px)}\n"),
@@ -109,7 +109,7 @@ mod error {
             );
         }
         #[test]
-        #[ignore] // missing error
+        #[ignore] // wrong error
         fn unitless_and_real() {
             assert_eq!(
                 runner().err("a {b: atan2(1, 1px)}\n"),

rsass/tests/spec/values/calculation/calc/no_operator.rs

@@ -191,6 +191,7 @@ mod interpolation {
     use super::runner;
 
     #[test]
+    #[ignore] // wrong result
     fn nested() {
         assert_eq!(
             runner().ok("a {b: calc(calc(#{c}))}\n"),
@@ -213,7 +214,6 @@ mod interpolation {
     );
     }
     #[test]
-    #[ignore] // wrong result
     fn parens() {
         assert_eq!(
         runner().ok(

rsass/tests/spec/values/calculation/calc/operator.rs

@@ -178,7 +178,6 @@ mod precedence {
             use super::runner;
 
             #[test]
-            #[ignore] // wrong result
             fn asterisk() {
                 assert_eq!(
                     runner().ok("a {b: calc(calc(#{\"c*\"}))}\n"),
@@ -188,6 +187,7 @@ mod precedence {
                 );
             }
             #[test]
+            #[ignore] // wrong result
             fn plain() {
                 assert_eq!(
                     runner().ok("a {b: calc(calc(#{c}))}\n"),
@@ -197,7 +197,6 @@ mod precedence {
                 );
             }
             #[test]
-            #[ignore] // wrong result
             fn slash() {
                 assert_eq!(
                     runner().ok("a {b: calc(calc(#{\"c/\"}))}\n"),
@@ -207,7 +206,6 @@ mod precedence {
                 );
             }
             #[test]
-            #[ignore] // wrong result
             fn whitespace() {
                 assert_eq!(
                     runner().ok("a {b: calc(calc(#{\"c \"}))}\n"),
@@ -218,7 +216,6 @@ mod precedence {
             }
         }
         #[test]
-        #[ignore] // wrong result
         fn parens() {
             assert_eq!(
                 runner().ok("a {b: calc((#{c}))}\n"),

rsass/tests/spec/values/calculation/calc/parens.rs

@@ -25,7 +25,6 @@ fn double_preserved() {
     );
 }
 #[test]
-#[ignore] // wrong result
 fn identifier() {
     assert_eq!(
         runner().ok("a {b: calc((d))}\n"),
@@ -35,7 +34,6 @@ fn identifier() {
     );
 }
 #[test]
-#[ignore] // wrong result
 fn interpolation() {
     assert_eq!(
         runner().ok("a {b: calc((#{\"1 + 2\"}))}\n"),
@@ -87,7 +85,6 @@ mod var {
     }
 }
 #[test]
-#[ignore] // wrong result
 fn variable() {
     assert_eq!(
         runner().ok("$c: unquote(\"1 + 2\");\

rsass/tests/spec/values/calculation/clamp.rs

@@ -36,7 +36,7 @@ mod error {
         use super::runner;
 
         #[test]
-        #[ignore] // missing error
+        #[ignore] // wrong error
         fn first() {
             assert_eq!(
                 runner().err("a {b: clamp(1s, 2px, 3px)}\n"),
@@ -50,7 +50,7 @@ mod error {
             );
         }
         #[test]
-        #[ignore] // missing error
+        #[ignore] // wrong error
         fn second() {
             assert_eq!(
                 runner().err("a {b: clamp(1px, 2s, 3px)}\n"),
@@ -64,7 +64,7 @@ mod error {
             );
         }
         #[test]
-        #[ignore] // missing error
+        #[ignore] // wrong error
         fn third() {
             assert_eq!(
                 runner().err("a {b: clamp(1px, 2px, 3s)}\n"),

rsass/tests/spec/values/calculation/rem.rs

@@ -130,7 +130,7 @@ mod error {
     );
         }
         #[test]
-        #[ignore] // missing error
+        #[ignore] // wrong error
         fn incompatible() {
             assert_eq!(
                 runner().err("a {b: rem(16px, 5deg)}\n"),
@@ -144,7 +144,7 @@ mod error {
             );
         }
         #[test]
-        #[ignore] // missing error
+        #[ignore] // wrong error
         fn real_and_unitless() {
             assert_eq!(
                 runner().err("a {b: rem(16px, 5)}\n"),