diff --git a/src/builtin/list.rs b/src/builtin/list.rs index e3edbc7..0474e32 100644 --- a/src/builtin/list.rs +++ b/src/builtin/list.rs @@ -245,7 +245,7 @@ fn index(mut args: CallArgs, parser: &mut Parser<'_>) -> SassResult { 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), }; diff --git a/src/builtin/mod.rs b/src/builtin/mod.rs index f8a4de7..8d78dea 100644 --- a/src/builtin/mod.rs +++ b/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(&self, state: &mut H) { - self.1.hash(state) - } -} - impl Builtin { pub fn new(body: fn(CallArgs, &mut Parser<'_>) -> SassResult) -> Builtin { let count = FUNCTION_COUNT.fetch_add(1, Ordering::Relaxed); diff --git a/src/color/mod.rs b/src/color/mod.rs index 9de0297..8c3da1a 100644 --- a/src/color/mod.rs +++ b/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(&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(&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 { diff --git a/src/parse/value/eval.rs b/src/parse/value/eval.rs index 781e228..94c5101 100644 --- a/src/parse/value/eval.rs +++ b/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 { diff --git a/src/style.rs b/src/style.rs index 03f9662..4c0e263 100644 --- a/src/style.rs +++ b/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>, diff --git a/src/value/map.rs b/src/value/map.rs index 0a69ab6..e5071c6 100644 --- a/src/value/map.rs +++ b/src/value/map.rs @@ -1,9 +1,4 @@ -use std::{ - collections::HashSet, - hash::{Hash, Hasher}, - slice::Iter, - vec::IntoIter, -}; +use std::{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(); - let set_two: HashSet<&(Value, Value)> = other.0.iter().collect(); - set_one == set_two + if self.0.len() != other.0.len() { + return false; + } + 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(&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> { 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; } diff --git a/src/value/mod.rs b/src/value/mod.rs index 1e5f123..780f957 100644 --- a/src/value/mod.rs +++ b/src/value/mod.rs @@ -23,8 +23,7 @@ mod map; mod number; mod sass_function; -// todo: remove Eq and PartialEq impls -#[derive(Debug, Clone, PartialEq, Eq, Hash)] +#[derive(Debug, Clone)] 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 { diff --git a/src/value/number/mod.rs b/src/value/number/mod.rs index d3bfcfe..00152b5 100644 --- a/src/value/number/mod.rs +++ b/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), diff --git a/src/value/sass_function.rs b/src/value/sass_function.rs index 1b8cc2b..9535933 100644 --- a/src/value/sass_function.rs +++ b/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, Identifier), diff --git a/tests/equality.rs b/tests/equality.rs index b68bc52..fe0a33e 100644 --- a/tests/equality.rs +++ b/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" +);