Use parentheses when joining with AND potentially-complex expressions

Author: Gallaecio

cssselect/xpath.py

@@ -56,7 +56,7 @@ def __repr__(self):
 
     def add_condition(self, condition):
         if self.condition:
-            self.condition = '%s and (%s)' % (self.condition, condition)
+            self.condition = '(%s) and (%s)' % (self.condition, condition)
         else:
             self.condition = condition
         return self
@@ -457,19 +457,19 @@ def xpath_nth_child_function(self, xpath, function, last=False,
         if a == 0:
             return xpath.add_condition('%s = %s' % (siblings_count, b_min_1))
 
-        expr = []
+        expressions = []
 
         if a > 0:
             # siblings count, an+b-1, is always >= 0,
             # so if a>0, and (b-1)<=0, an "n" exists to satisfy this,
             # therefore, the predicate is only interesting if (b-1)>0
             if b_min_1 > 0:
-                expr.append('%s >= %s' % (siblings_count, b_min_1))
+                expressions.append('%s >= %s' % (siblings_count, b_min_1))
         else:
             # if a<0, and (b-1)<0, no "n" satisfies this,
             # this is tested above as an early exist condition
             # otherwise,
-            expr.append('%s <= %s' % (siblings_count, b_min_1))
+            expressions.append('%s <= %s' % (siblings_count, b_min_1))
 
         # operations modulo 1 or -1 are simpler, one only needs to verify:
         #
@@ -495,9 +495,14 @@ def xpath_nth_child_function(self, xpath, function, last=False,
                 b_neg = '+%s' % b_neg
                 left = '(%s %s)' % (left, b_neg)
 
-            expr.append('%s mod %s = 0' % (left, a))
+            expressions.append('%s mod %s = 0' % (left, a))
 
-        xpath.add_condition(' and '.join(expr))
+        if len(expressions) > 1:
+            template = '(%s)'
+        else:
+            template = '%s'
+        xpath.add_condition(' and '.join(template % expression
+                                         for expression in expressions))
         return xpath
 
     def xpath_nth_last_child_function(self, xpath, function):

tests/test_cssselect.py

@@ -428,8 +428,8 @@ def xpath(css):
             "e[count(preceding-sibling::*) <= 0]")
 
         assert xpath('e:nth-child(3n+2)') == (
-            "e[count(preceding-sibling::*) >= 1 and "
-              "(count(preceding-sibling::*) +2) mod 3 = 0]")
+            "e[(count(preceding-sibling::*) >= 1) and "
+              "((count(preceding-sibling::*) +2) mod 3 = 0)]")
         assert xpath('e:nth-child(3n-2)') == (
             "e[count(preceding-sibling::*) mod 3 = 0]")
         assert xpath('e:nth-child(-n+6)') == (
@@ -442,8 +442,8 @@ def xpath(css):
         assert xpath('e:nth-last-child(2n+1)') == (
             "e[count(following-sibling::*) mod 2 = 0]")
         assert xpath('e:nth-last-child(2n+2)') == (
-            "e[count(following-sibling::*) >= 1 and "
-              "(count(following-sibling::*) +1) mod 2 = 0]")
+            "e[(count(following-sibling::*) >= 1) and "
+              "((count(following-sibling::*) +1) mod 2 = 0)]")
         assert xpath('e:nth-last-child(3n+1)') == (
             "e[count(following-sibling::*) mod 3 = 0]")
         # represents the two last e elements
@@ -497,7 +497,7 @@ def xpath(css):
         assert xpath('e > f') == (
             "e/f")
         assert xpath('e + f') == (
-            "e/following-sibling::*[name() = 'f' and (position() = 1)]")
+            "e/following-sibling::*[(name() = 'f') and (position() = 1)]")
         assert xpath('e ~ f') == (
             "e/following-sibling::f")
         assert xpath('e ~ f:nth-child(3)') == (
@@ -642,7 +642,7 @@ def xpath(css):
             "descendant-or-self::*[(@id = 'first' or @id = 'second') "
             "and (@href)]")
 
-        assert str(XPathExpr('', '', condition='@href')) == "[(@href)]"
+        assert str(XPathExpr('', '', condition='@href')) == "[@href]"
 
         document = etree.fromstring(OPERATOR_PRECEDENCE_IDS)
         sort_key = dict(