From c07bb7eccef21afd4fd2d3424a527658f032e199 Mon Sep 17 00:00:00 2001 From: ConnorSkees <39542938+ConnorSkees@users.noreply.github.com> Date: Sat, 16 May 2020 22:43:13 -0400 Subject: [PATCH] refactor == and != order of operations --- src/builtin/list.rs | 10 +++-- src/value/map.rs | 2 +- src/value/mod.rs | 81 ++++++++++++++++++++++++++++++++++-- src/value/parse.rs | 1 + tests/order-of-operations.rs | 12 +++++- 5 files changed, 96 insertions(+), 10 deletions(-) diff --git a/src/builtin/list.rs b/src/builtin/list.rs index c9af8e1..4873f6a 100644 --- a/src/builtin/list.rs +++ b/src/builtin/list.rs @@ -272,10 +272,12 @@ fn index(mut args: CallArgs, scope: &Scope, super_selector: &Selector) -> SassRe // evaluated prior to checking equality, but // it is still dirty. // Potential input to fuzz: index(1px 1in 1cm, 96px + 1rem) - let index = match list - .into_iter() - .position(|v| v.equals(value.clone(), args.span()).unwrap()) - { + let index = match list.into_iter().position(|v| { + v.equals(value.clone(), args.span()) + .unwrap() + .is_true(args.span()) + .unwrap() + }) { Some(v) => Number::from(v + 1), None => return Ok(Value::Null), }; diff --git a/src/value/map.rs b/src/value/map.rs index 7052014..6dd1f28 100644 --- a/src/value/map.rs +++ b/src/value/map.rs @@ -17,7 +17,7 @@ impl SassMap { pub fn get(self, key: &Value, span: Span) -> SassResult> { for (k, v) in self.0 { - if k.equals(key.clone(), span)? { + if k.equals(key.clone(), span)?.node.is_true(span)? { return Ok(Some(v)); } } diff --git a/src/value/mod.rs b/src/value/mod.rs index d151f08..d681a89 100644 --- a/src/value/mod.rs +++ b/src/value/mod.rs @@ -313,8 +313,14 @@ impl Value { }) } - pub fn equals(self, other: Value, span: Span) -> SassResult { - Ok(match self.eval(span)?.node { + pub fn equals(self, mut other: Value, span: Span) -> SassResult> { + if let Self::Paren(..) = other { + other = other.eval(span)?.node + } + + let precedence = Op::Equal.precedence(); + + Ok(Value::bool(match self { Self::Ident(s1, ..) => match other { Self::Ident(s2, ..) => s1 == s2, _ => false, @@ -336,8 +342,75 @@ impl Value { } _ => false, }, + Self::BinaryOp(left, op2, right) => { + if op2.precedence() >= precedence { + Self::BinaryOp(left, op2, right).eval(span)?.node == other + } else { + return Self::BinaryOp( + left, + op2, + Box::new( + Self::BinaryOp(right, Op::Equal, Box::new(other)) + .eval(span)? + .node, + ), + ) + .eval(span); + } + } s => s == other.eval(span)?.node, }) + .span(span)) + } + + pub fn not_equals(self, mut other: Value, span: Span) -> SassResult> { + if let Self::Paren(..) = other { + other = other.eval(span)?.node + } + + let precedence = Op::Equal.precedence(); + + Ok(Value::bool(match self { + Self::Ident(s1, ..) => match other { + Self::Ident(s2, ..) => s1 != s2, + _ => true, + }, + Self::Dimension(n, unit) => match other { + Self::Dimension(n2, unit2) => { + if !unit.comparable(&unit2) { + true + } else if unit == unit2 { + n != n2 + } else if unit == Unit::None || unit2 == Unit::None { + true + } else { + n != (n2 + * UNIT_CONVERSION_TABLE[unit.to_string().as_str()] + [unit2.to_string().as_str()] + .clone()) + } + } + _ => true, + }, + Self::BinaryOp(left, op2, right) => { + if op2.precedence() >= precedence { + Self::BinaryOp(left, op2, right).eval(span)?.node != other + } else { + return Self::BinaryOp( + left, + op2, + Box::new( + Self::BinaryOp(right, Op::NotEqual, Box::new(other)) + .eval(span)? + .node, + ), + ) + .eval(span); + } + } + s => s != other.eval(span)?.node, + }) + .span(span)) } pub fn unary_op_plus(self, span: Span) -> SassResult { @@ -352,8 +425,8 @@ impl Value { Self::BinaryOp(lhs, op, rhs) => match op { Op::Plus => lhs.add(*rhs, span)?, Op::Minus => lhs.sub(*rhs, span)?, - Op::Equal => Self::bool(lhs.equals(*rhs, span)?), - Op::NotEqual => Self::bool(!lhs.equals(*rhs, span)?), + Op::Equal => lhs.equals(*rhs, span)?.node, + Op::NotEqual => lhs.not_equals(*rhs, span)?.node, Op::Mul => lhs.mul(*rhs, span)?, Op::Div => lhs.div(*rhs, span)?, Op::Rem => lhs.rem(*rhs, span)?, diff --git a/src/value/parse.rs b/src/value/parse.rs index b5c52bb..1ced72d 100644 --- a/src/value/parse.rs +++ b/src/value/parse.rs @@ -301,6 +301,7 @@ fn eat_op>( } Op::And | Op::Or => { devour_whitespace(iter); + // special case when the value is literally "and" or "or" if iter.peek().is_none() { space_separated.push(Value::Ident(op.to_string(), QuoteKind::None).span(op.span)); } else if let Some(left) = space_separated.pop() { diff --git a/tests/order-of-operations.rs b/tests/order-of-operations.rs index 72a4949..a3e08c5 100644 --- a/tests/order-of-operations.rs +++ b/tests/order-of-operations.rs @@ -25,6 +25,16 @@ test!( ); test!( comparison, - "a {\n color: 1 < 1 and 1 < 1;;\n}\n", + "a {\n color: 1 < 1 and 1 < 1;\n}\n", "a {\n color: false;\n}\n" ); +test!( + equals_then_or, + "a {\n color: a or b==c;\n}\n", + "a {\n color: a;\n}\n" +); +test!( + not_equals_then_or, + "a {\n color: a or b !=c;\n}\n", + "a {\n color: a;\n}\n" +);