Bring comparison in line with Ruby Sass.

3 files changed, 75 insertions, 60 deletions

diff --git a/scss/tests/functions/test_core.py b/scss/tests/functions/test_core.py
index 820126a..306f4a6 100644
--- a/scss/tests/functions/test_core.py
+++ b/scss/tests/functions/test_core.py
@@ -36,13 +36,13 @@ def test_rgba(calc):
     assert calc('rgba(red, 0.4)') == ColorValue.from_rgb(1., 0., 0., 0.4)
 
 def test_red(calc):
-    assert calc('red(orange)') == 255
+    assert calc('red(orange)') == NumberValue(255)
 
 def test_green(calc):
-    assert calc('green(orange)') == 165
+    assert calc('green(orange)') == NumberValue(165)
 
 def test_blue(calc):
-    assert calc('blue(orange)') == 0
+    assert calc('blue(orange)') == NumberValue(0)
 
 @xfail(reason="difference in rounding; ruby floors, we round")
 def test_mix(calc):
@@ -71,14 +71,13 @@ def test_hsla(calc):
     assert calc('hsla(30, 100%, 50%, 0.1)') == ColorValue.from_rgb(1., 0.5, 0., 0.1)
 
 def test_hue(calc):
-    # TODO deg; this shouldn't really pass
-    assert calc('hue(yellow)') == 60
+    assert calc('hue(yellow)') == NumberValue(60, type='deg')
 
 def test_saturation(calc):
-    assert calc('saturation(yellow)') == 1.
+    assert calc('saturation(yellow)') == NumberValue(100, type='%')
 
 def test_lightness(calc):
-    assert calc('lightness(yellow)') == 0.5
+    assert calc('lightness(yellow)') == NumberValue(50, type='%')
 
 # HSL manipulation functions
 
diff --git a/scss/tests/test_expression.py b/scss/tests/test_expression.py
index e240610..c1084c3 100644
--- a/scss/tests/test_expression.py
+++ b/scss/tests/test_expression.py
@@ -3,6 +3,8 @@ from scss.rule import Namespace
 #from scss.types import Number
 from scss.util import to_str
 
+import pytest
+
 # TODO fix constructors for various types, and stop using to_str in here
 
 def test_reference_operations():
@@ -103,24 +105,25 @@ def test_comparison_numeric():
 def test_comparison_stringerific():
     calc = Calculator(Namespace()).calculate
 
-    # TODO whoops these aren't supposed to work for strings
-    assert calc('"abc" < "xyz"')
-    assert calc('"abc" <= "xyz"')
-    assert calc('"abc" <= "abc"')
-    assert calc('"xyz" > "abc"')
-    assert calc('"xyz" >= "abc"')
-    assert calc('"xyz" >= "xyz"')
     assert calc('"abc" == "abc"')
     assert calc('"abc" != "xyz"')
 
     # Same tests, negated
-    assert not calc('"abc" > "xyz"')
-    assert not calc('"abc" >= "xyz"')
-    assert not calc('"xyz" < "abc"')
-    assert not calc('"xyz" <= "abc"')
     assert not calc('"abc" != "abc"')
     assert not calc('"abc" == "xyz"')
 
+    # Sass strings don't support ordering
+    for expression in (
+            '"abc" < "xyz"',
+            '"abc" <= "xyz"',
+            '"abc" <= "abc"',
+            '"xyz" > "abc"',
+            '"xyz" >= "abc"',
+            '"xyz" >= "xyz"'):
+
+        with pytest.raises(TypeError):
+            calc(expression)
+
 
 
 # TODO write more!  i'm lazy.
diff --git a/scss/types.py b/scss/types.py
index 77e6b5c..c574338 100644
--- a/scss/types.py
+++ b/scss/types.py
@@ -21,28 +21,35 @@ class Value(object):
     def __repr__(self):
         return '<%s: %s>' % (self.__class__.__name__, repr(self.value))
 
-    def __lt__(self, other):
-        return self._do_cmps(self, other, operator.__lt__)
+    # Sass values are all true, except for booleans and nulls
+    def __nonzero__(self):
+        return True
 
-    def __le__(self, other):
-        return self._do_cmps(self, other, operator.__le__)
+    ### NOTE: From here on down, the operators are exposed to Sass code and
+    ### thus should ONLY return Sass types
 
+    # Reasonable default for equality
     def __eq__(self, other):
-        return self._do_cmps(self, other, operator.__eq__)
+        return BooleanValue(
+            type(self) == type(other) and self.value == other.value)
 
     def __ne__(self, other):
-        return self._do_cmps(self, other, operator.__ne__)
+        return BooleanValue(not self.__eq__(other))
+
+    # Only numbers support ordering
+    def __lt__(self, other):
+        raise TypeError("Can't compare %r with %r" % (self, other))
+
+    def __le__(self, other):
+        raise TypeError("Can't compare %r with %r" % (self, other))
 
     def __gt__(self, other):
-        return self._do_cmps(self, other, operator.__gt__)
+        raise TypeError("Can't compare %r with %r" % (self, other))
 
     def __ge__(self, other):
-        return self._do_cmps(self, other, operator.__ge__)
-
-    def __nonzero__(self):
-        # Sass values are all true, except for booleans and nulls
-        return True
+        raise TypeError("Can't compare %r with %r" % (self, other))
 
+    # Math ops
     def __add__(self, other):
         return self._do_op(self, other, operator.__add__)
 
@@ -87,6 +94,9 @@ class NullValue(Value):
     def __nonzero__(self):
         return False
 
+    def __eq__(self, other):
+        return isinstance(other, NullValue)
+
 
 class BooleanValue(Value):
     def __init__(self, value):
@@ -102,10 +112,6 @@ class BooleanValue(Value):
         return self.value
 
     @classmethod
-    def _do_cmps(cls, first, second, op):
-        return op(bool(first), bool(second))
-
-    @classmethod
     def _do_op(cls, first, second, op):
         if isinstance(first, ListValue) and isinstance(second, ListValue):
             ret = ListValue(first)
@@ -186,8 +192,31 @@ class NumberValue(Value):
         val = to_str(val) + unit
         return val
 
-    @classmethod
-    def _do_cmps(cls, first, second, op):
+    def __eq__(self, other):
+        if not isinstance(other, NumberValue):
+            return BooleanValue(False)
+
+        return BooleanValue(self.value == other.value and self.unit == other.unit)
+
+    def __lt__(self, other):
+        return self._compare(other, operator.__lt__)
+
+    def __le__(self, other):
+        return self._compare(other, operator.__le__)
+
+    def __gt__(self, other):
+        return self._compare(other, operator.__gt__)
+
+    def __ge__(self, other):
+        return self._compare(other, operator.__ge__)
+
+    def _compare(self, other, op):
+        if not isinstance(other, NumberValue):
+            raise TypeError("Can't compare %r and %r" % (self, other))
+
+        # TODO this will need to get more complicated for full unit support
+        first = self
+        second = other
         try:
             first = NumberValue(first)
             second = NumberValue(second)
@@ -354,15 +383,6 @@ class ListValue(Value):
         return hash((frozenset(self.value.items())))
 
     @classmethod
-    def _do_cmps(cls, first, second, op):
-        try:
-            first = ListValue(first)
-            second = ListValue(second)
-        except ValueError:
-            return op(getattr(first, 'value', first), getattr(second, 'value', second))
-        return op(first.value, second.value)
-
-    @classmethod
     def _do_op(cls, first, second, op):
         if isinstance(first, ListValue) and isinstance(second, ListValue):
             ret = ListValue(first)
@@ -553,20 +573,16 @@ class ColorValue(Value):
             return 'rgba(%s%%, %s%%, %s%%, %s)' % (to_str(c[0] * 100.0 / 255.0), to_str(c[1] * 100.0 / 255.0), to_str(c[2] * 100.0 / 255.0), to_str(c[3]))
         return 'rgba(%d, %d, %d, %s)' % (round(c[0]), round(c[1]), round(c[2]), to_str(c[3]))
 
-    @classmethod
-    def _do_cmps(cls, first, second, op):
-        try:
-            first = ColorValue(first)
-            second = ColorValue(second)
-        except ValueError:
-            return op(getattr(first, 'value', first), getattr(second, 'value', second))
+    def __eq__(self, other):
+        if not isinstance(other, ColorValue):
+            return BooleanValue(False)
 
         # Round to the nearest 5 digits for comparisons; corresponds roughly to
         # 16 bits per channel, the most that generally matters.  Otherwise
         # float errors make equality fail for HSL colors.
-        first_rounded = tuple(round(n, 5) for n in first.value)
-        second_rounded = tuple(round(n, 5) for n in second.value)
-        return op(first_rounded, second_rounded)
+        left = tuple(round(n, 5) for n in self.value)
+        right = tuple(round(n, 5) for n in other.value)
+        return left == right
 
     @classmethod
     def _do_op(cls, first, second, op):
@@ -641,11 +657,8 @@ class QuotedStringValue(Value):
     def __unicode__(self):
         return '"%s"' % escape(self.value)
 
-    @classmethod
-    def _do_cmps(cls, first, second, op):
-        first = QuotedStringValue(first)
-        second = QuotedStringValue(second)
-        return op(first.value, second.value)
+    def __eq__(self, other):
+        return BooleanValue(isinstance(other, QuotedStringValue) and self.value == other.value)
 
     @classmethod
     def _do_op(cls, first, second, op):