* Move expression handling from variable.rb to expression.rb

* Update test suite to validate parity
* Remove parentheses handling
* Split boolean into comparison and logical expressions

Author: karreiro

lib/liquid.rb

@@ -80,6 +80,8 @@ module Liquid
 require 'liquid/range_lookup'
 require 'liquid/resource_limits'
 require 'liquid/expression'
+require 'liquid/expression/comparison_expression'
+require 'liquid/expression/logical_expression'
 require 'liquid/template'
 require 'liquid/condition'
 require 'liquid/utils'
@@ -89,4 +91,3 @@ module Liquid
 require 'liquid/usage'
 require 'liquid/registers'
 require 'liquid/template_factory'
-require 'liquid/boolean_expression'

lib/liquid/boolean_expression.rb

@@ -51,6 +51,9 @@ def parse(markup, ss = StringScanner.new(""), cache = nil)
       end
 
       def inner_parse(markup, ss, cache)
+        return LogicalExpression.parse(markup, ss, cache)    if LogicalExpression.logical?(markup)
+        return ComparisonExpression.parse(markup, ss, cache) if ComparisonExpression.comparison?(markup)
+
         if (markup.start_with?("(") && markup.end_with?(")")) && markup =~ RANGES_REGEX
           return RangeLookup.parse(
             Regexp.last_match(1),

lib/liquid/expression.rb

@@ -0,0 +1,29 @@
+# frozen_string_literal: true
+
+module Liquid
+  class Expression
+    class ComparisonExpression
+      COMPARISON_REGEX = /\A\s*(.+?)\s*(==|!=|<>|<=|>=|<|>|contains)\s*(.+)\s*\z/
+
+      class << self
+        def comparison?(markup)
+          markup.match(COMPARISON_REGEX)
+        end
+
+        def parse(markup, ss, cache)
+          match = comparison?(markup)
+
+          if match
+            left = Expression.parse(match[1].strip, ss, cache)
+            operator = match[2].strip
+            right = Expression.parse(match[3].strip, ss, cache)
+
+            return Condition.new(left, operator, right)
+          end
+
+          Condition.new(parse(markup, ss, cache), nil, nil)
+        end
+      end
+    end
+  end
+end

lib/liquid/expression/comparison_expression.rb

@@ -0,0 +1,60 @@
+# frozen_string_literal: true
+
+module Liquid
+  class Expression
+    class LogicalExpression
+      LOGICAL_REGEX = /\A\s*(.+?)\s+(and|or)\s+(.+)\s*\z/i
+      EXPRESSIONS_AND_OPERATORS = /(?:\b(?:\s?and\s?|\s?or\s?)\b|(?:\s*(?!\b(?:\s?and\s?|\s?or\s?)\b)(?:#{QuotedFragment}|\S+)\s*)+)/o
+      BOOLEAN_OPERATORS = ['and', 'or'].freeze
+
+      class << self
+        def logical?(markup)
+          markup.match(LOGICAL_REGEX)
+        end
+
+        def boolean_operator?(markup)
+          BOOLEAN_OPERATORS.include?(markup)
+        end
+
+        def parse(markup, ss, cache)
+          expressions = markup.scan(EXPRESSIONS_AND_OPERATORS)
+
+          last_expr = expressions.pop
+
+          condition = if ComparisonExpression.comparison?(last_expr)
+            ComparisonExpression.parse(last_expr, ss, cache)
+          elsif logical?(last_expr)
+            LogicalExpression.parse(last_expr, ss, cache)
+          else
+            Condition.new(Expression.parse(last_expr, ss, cache), nil, nil)
+          end
+
+          until expressions.empty?
+            operator = expressions.pop.to_s.strip
+            next unless boolean_operator?(operator)
+
+            expr = expressions.pop.to_s.strip
+
+            new_condition = if ComparisonExpression.comparison?(expr)
+              ComparisonExpression.parse(expr, ss, cache)
+            elsif logical?(expr)
+              LogicalExpression.parse(expr, ss, cache)
+            else
+              Condition.new(Expression.parse(expr, ss, cache), nil, nil)
+            end
+
+            if operator == 'and'
+              new_condition.and(condition)
+            else # operator == 'or'
+              new_condition.or(condition)
+            end
+
+            condition = new_condition
+          end
+
+          condition
+        end
+      end
+    end
+  end
+end

lib/liquid/expression/logical_expression.rb

@@ -17,8 +17,6 @@ class Variable
     FilterArgsRegex          = /(?:#{FilterArgumentSeparator}|#{ArgumentSeparator})\s*((?:\w+\s*\:\s*)?#{QuotedFragment})/o
     JustTagAttributes        = /\A#{TagAttributes}\z/o
     MarkupWithQuotedFragment = /(#{QuotedFragment})(.*)/om
-    ComparisonOperator       = /==|!=|<>|<=|>=|<|>|contains/o
-    LogicalOperator          = /\s+(and|or)\s+/i
 
     attr_accessor :filters, :name, :line_number
     attr_reader :parse_context
@@ -49,14 +47,7 @@ def lax_parse(markup)
 
       name_markup   = Regexp.last_match(1)
       filter_markup = Regexp.last_match(2)
-
-      # Check if name_markup contains a comparison operator or logical operator
-      @name = if name_markup =~ LogicalOperator || name_markup =~ ComparisonOperator
-        BooleanExpression.parse(name_markup)
-      else
-        parse_context.parse_expression(name_markup)
-      end
-
+      @name         = parse_context.parse_expression(name_markup)
       if filter_markup =~ FilterMarkupRegex
         filters = Regexp.last_match(1).scan(FilterParser)
         filters.each do |f|
@@ -74,18 +65,13 @@ def strict_parse(markup)
 
       return if p.look(:end_of_string)
 
-      # Check if markup contains a comparison operator or logical operator
-      if markup =~ LogicalOperator || markup =~ ComparisonOperator
-        @name = BooleanExpression.parse(markup)
-      else
-        @name = parse_context.parse_expression(p.expression)
-        while p.consume?(:pipe)
-          filtername = p.consume(:id)
-          filterargs = p.consume?(:colon) ? parse_filterargs(p) : Const::EMPTY_ARRAY
-          @filters << parse_filter_expressions(filtername, filterargs)
-        end
-        p.consume(:end_of_string)
+      @name = parse_context.parse_expression(p.expression)
+      while p.consume?(:pipe)
+        filtername = p.consume(:id)
+        filterargs = p.consume?(:colon) ? parse_filterargs(p) : Const::EMPTY_ARRAY
+        @filters << parse_filter_expressions(filtername, filterargs)
       end
+      p.consume(:end_of_string)
     end
 
     def parse_filterargs(p)