remove Hash impl for Value

src/builtin/list.rs

@@ -245,7 +245,7 @@ fn index(mut args: CallArgs, parser: &mut Parser<'_>) -> SassResult<Value> {
     let value = parser.arg(&mut args, 1, "value")?;
     // TODO: find a way to propagate any errors here
     // Potential input to fuzz: index(1px 1in 1cm, 96px + 1rem)
-    let index = match list.into_iter().position(|v| v.equals(&value)) {
+    let index = match list.into_iter().position(|v| v == value) {
         Some(v) => Number::from(v + 1),
         None => return Ok(Value::Null),
     };

src/builtin/mod.rs

@@ -1,6 +1,5 @@
 use std::{
     collections::HashMap,
-    hash::{Hash, Hasher},
     sync::atomic::{AtomicUsize, Ordering},
 };
 
@@ -30,12 +29,6 @@ pub(crate) struct Builtin(
     usize,
 );
 
-impl Hash for Builtin {
-    fn hash<H: Hasher>(&self, state: &mut H) {
-        self.1.hash(state)
-    }
-}
-
 impl Builtin {
     pub fn new(body: fn(CallArgs, &mut Parser<'_>) -> SassResult<Value>) -> Builtin {
         let count = FUNCTION_COUNT.fetch_add(1, Ordering::Relaxed);

src/color/mod.rs

@@ -18,7 +18,6 @@
 use std::{
     cmp::{max, min},
     fmt::{self, Display},
-    hash::{Hash, Hasher},
 };
 
 use crate::value::Number;
@@ -43,12 +42,6 @@ impl PartialEq for Color {
 
 impl Eq for Color {}
 
-impl Hash for Color {
-    fn hash<H: Hasher>(&self, state: &mut H) {
-        self.rgba.hash(state)
-    }
-}
-
 impl Color {
     pub const fn new_rgba(
         red: Number,
@@ -116,34 +109,6 @@ impl PartialEq for Rgba {
 
 impl Eq for Rgba {}
 
-impl Hash for Rgba {
-    fn hash<H: Hasher>(&self, state: &mut H) {
-        if self.red > Number::from(255) {
-            255.hash(state);
-        } else {
-            self.red.hash(state)
-        }
-
-        if self.green > Number::from(255) {
-            255.hash(state);
-        } else {
-            self.green.hash(state)
-        }
-
-        if self.blue > Number::from(255) {
-            255.hash(state);
-        } else {
-            self.blue.hash(state)
-        }
-
-        if self.alpha > Number::one() {
-            1.hash(state);
-        } else {
-            self.alpha.hash(state)
-        }
-    }
-}
-
 impl Rgba {
     pub const fn new(red: Number, green: Number, blue: Number, alpha: Number) -> Self {
         Rgba {

src/parse/value/eval.rs

@@ -706,7 +706,7 @@ impl<'a, 'b: 'a> ValueVisitor<'a, 'b> {
             HigherIntermediateValue::Literal(v) => v,
             v => panic!("{:?}", v),
         };
-        Value::bool(left.equals(&right))
+        Value::bool(left == right)
     }
 
     fn not_equal(left: HigherIntermediateValue, right: HigherIntermediateValue) -> Value {

src/style.rs

@@ -3,7 +3,7 @@ use codemap::Spanned;
 use crate::{error::SassResult, interner::InternedString, value::Value};
 
 /// A style: `color: red`
-#[derive(Clone, Debug, Eq, PartialEq)]
+#[derive(Clone, Debug)]
 pub(crate) struct Style {
     pub property: InternedString,
     pub value: Box<Spanned<Value>>,

src/value/map.rs

@@ -1,9 +1,4 @@
-use std::{
+use std::{slice::Iter, vec::IntoIter};
-    collections::HashSet,
-    hash::{Hash, Hasher},
-    slice::Iter,
-    vec::IntoIter,
-};
 
 use crate::{
     common::{Brackets, ListSeparator},
@@ -16,28 +11,36 @@ pub(crate) struct SassMap(Vec<(Value, Value)>);
 
 impl PartialEq for SassMap {
     fn eq(&self, other: &Self) -> bool {
-        let set_one: HashSet<&(Value, Value)> = self.0.iter().collect();
+        if self.0.len() != other.0.len() {
-        let set_two: HashSet<&(Value, Value)> = other.0.iter().collect();
+            return false;
-        set_one == set_two
+        }
+        for (key, value) in &self.0 {
+            if !other
+                .0
+                .iter()
+                .any(|(key2, value2)| key == key2 && value == value2)
+            {
+                return false;
+            }
+        }
+        true
     }
 }
 
 impl Eq for SassMap {}
 
-impl Hash for SassMap {
-    fn hash<H: Hasher>(&self, state: &mut H) {
-        self.0.hash(state)
-    }
-}
-
 impl SassMap {
     pub const fn new() -> SassMap {
         SassMap(Vec::new())
     }
 
+    /// We take by value here (consuming the map) in order to
+    /// save a clone of the value, since the only place this
+    /// should be called is in a builtin function, which throws
+    /// away the map immediately anyway
     pub fn get(self, key: &Value) -> SassResult<Option<Value>> {
         for (k, v) in self.0 {
-            if k.equals(key) {
+            if &k == key {
                 return Ok(Some(v));
             }
         }
@@ -81,7 +84,7 @@ impl SassMap {
     /// Returns true if the key already exists
     pub fn insert(&mut self, key: Value, value: Value) -> bool {
         for (ref k, ref mut v) in &mut self.0 {
-            if k.equals(&key) {
+            if k == &key {
                 *v = value;
                 return true;
             }

src/value/mod.rs

@@ -23,8 +23,7 @@ mod map;
 mod number;
 mod sass_function;
 
-// todo: remove Eq and PartialEq impls
+#[derive(Debug, Clone)]
-#[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub(crate) enum Value {
     Important,
     True,
@@ -40,6 +39,83 @@ pub(crate) enum Value {
     FunctionRef(SassFunction),
 }
 
+impl PartialEq for Value {
+    fn eq(&self, other: &Self) -> bool {
+        match self {
+            Value::String(s1, ..) => match other {
+                Value::String(s2, ..) => s1 == s2,
+                _ => false,
+            },
+            Value::Dimension(n, unit, _) => match other {
+                Value::Dimension(n2, unit2, _) => {
+                    if !unit.comparable(unit2) {
+                        false
+                    } else if unit == unit2 {
+                        n == n2
+                    } else if unit == &Unit::None || unit2 == &Unit::None {
+                        false
+                    } else {
+                        n == &(n2.clone()
+                            * UNIT_CONVERSION_TABLE[unit.to_string().as_str()]
+                                [unit2.to_string().as_str()]
+                            .clone())
+                    }
+                }
+                _ => false,
+            },
+            Value::List(list1, sep1, brackets1) => match other {
+                Value::List(list2, sep2, brackets2) => {
+                    if sep1 != sep2 || brackets1 != brackets2 || list1.len() != list2.len() {
+                        false
+                    } else {
+                        for (a, b) in list1.iter().zip(list2) {
+                            if a != b {
+                                return false;
+                            }
+                        }
+                        true
+                    }
+                }
+                _ => false,
+            },
+            Value::Null => matches!(other, Value::Null),
+            Value::True => matches!(other, Value::True),
+            Value::False => matches!(other, Value::False),
+            Value::Important => matches!(other, Value::Important),
+            Value::FunctionRef(fn1) => {
+                if let Value::FunctionRef(fn2) = other {
+                    fn1 == fn2
+                } else {
+                    false
+                }
+            }
+            Value::Map(map1) => {
+                if let Value::Map(map2) = other {
+                    map1 == map2
+                } else {
+                    false
+                }
+            }
+            Value::Color(color1) => {
+                if let Value::Color(color2) = other {
+                    color1 == color2
+                } else {
+                    false
+                }
+            }
+            Value::ArgList(list1) => {
+                if let Value::ArgList(list2) = other {
+                    list1 == list2
+                } else {
+                    false
+                }
+            }
+        }
+    }
+}
+
+impl Eq for Value {}
+
 fn visit_quoted_string(buf: &mut String, force_double_quote: bool, string: &str) {
     let mut has_single_quote = false;
     let mut has_double_quote = false;
@@ -246,48 +322,6 @@ impl Value {
         }
     }
 
-    pub fn equals(&self, other: &Self) -> bool {
-        match self {
-            Value::String(s1, ..) => match other {
-                Value::String(s2, ..) => s1 == s2,
-                _ => false,
-            },
-            Value::Dimension(n, unit, _) => match other {
-                Value::Dimension(n2, unit2, _) => {
-                    if !unit.comparable(unit2) {
-                        false
-                    } else if unit == unit2 {
-                        n == n2
-                    } else if unit == &Unit::None || unit2 == &Unit::None {
-                        false
-                    } else {
-                        n == &(n2.clone()
-                            * UNIT_CONVERSION_TABLE[unit.to_string().as_str()]
-                                [unit2.to_string().as_str()]
-                            .clone())
-                    }
-                }
-                _ => false,
-            },
-            Value::List(list1, sep1, brackets1) => match other {
-                Value::List(list2, sep2, brackets2) => {
-                    if sep1 != sep2 || brackets1 != brackets2 || list1.len() != list2.len() {
-                        false
-                    } else {
-                        for (a, b) in list1.iter().zip(list2) {
-                            if !a.equals(b) {
-                                return false;
-                            }
-                        }
-                        true
-                    }
-                }
-                _ => false,
-            },
-            s => s == other,
-        }
-    }
-
     pub fn not_equals(&self, other: &Self) -> bool {
         match self {
             Value::String(s1, ..) => match other {

src/value/number/mod.rs

@@ -16,7 +16,7 @@ mod integer;
 
 const PRECISION: usize = 10;
 
-#[derive(Clone, Eq, PartialEq, Ord, Hash)]
+#[derive(Clone, Eq, PartialEq, Ord)]
 pub(crate) enum Number {
     Small(Rational64),
     Big(Box<BigRational>),

src/value/sass_function.rs

@@ -22,7 +22,7 @@ use crate::{
 ///
 /// The function name is stored in addition to the body
 /// for use in the builtin function `inspect()`
-#[derive(Clone, Eq, PartialEq, Hash)]
+#[derive(Clone, Eq, PartialEq)]
 pub(crate) enum SassFunction {
     Builtin(Builtin, Identifier),
     UserDefined(Box<Function>, Identifier),

tests/equality.rs

@@ -58,3 +58,8 @@ test!(
     "a {\n  color: (\"foo\",) != (foo,);\n}\n",
     "a {\n  color: false;\n}\n"
 );
+test!(
+    map_keys_equivalent,
+    "a {\n  color: (0mm: a)==(0cm: a);\n}\n",
+    "a {\n  color: true;\n}\n"
+);