Change Parser#expression to build the parsed expression

lib/liquid/expression.rb

@@ -33,7 +33,7 @@ module Liquid
         if LITERALS.key?(markup)
           LITERALS[markup]
         else
-          VariableLookup.parse(markup)
+          VariableLookup.lax_parse(markup)
         end
       end
     end

lib/liquid/parser.rb

@@ -50,53 +50,55 @@ module Liquid
       token = @tokens[@p]
       case token[0]
       when :id
-        str = consume
-        str << variable_lookups
+        if Expression::LITERALS.key?(token[1]) && !look(:dot, 1) && !look(:open_square, 1)
+          Expression::LITERALS[consume]
+        else
+          VariableLookup.strict_parse(self)
+        end
       when :open_square
-        str = consume
-        str << expression
-        str << consume(:close_square)
-        str << variable_lookups
-      when :string, :number
-        consume
+        VariableLookup.strict_parse(self)
+      when :string
+        consume[1..-2]
+      when :number
+        Expression.parse(consume)
       when :open_round
         consume
         first = expression
         consume(:dotdot)
         last = expression
         consume(:close_round)
-        "(#{first}..#{last})"
+        if first.respond_to?(:evaluate) || last.respond_to?(:evaluate)
+          RangeLookup.new(first, last)
+        else
+          first.to_i..last.to_i
+        end
       else
         raise SyntaxError, "#{token} is not a valid expression"
       end
     end
 
-    def argument
-      str = +""
-      # might be a keyword argument (identifier: expression)
-      if look(:id) && look(:colon, 1)
-        str << consume << consume << ' '
-      end
+    def arguments
+      filter_args = []
+      keyword_args = nil
 
-      str << expression
-      str
-    end
-
-    def variable_lookups
-      str = +""
       loop do
-        if look(:open_square)
-          str << consume
-          str << expression
-          str << consume(:close_square)
-        elsif look(:dot)
-          str << consume
-          str << consume(:id)
+        # keyword argument (identifier: expression)
+        if look(:colon, 1)
+          keyword_args ||= {}
+          k = consume(:id)
+          consume
+          v = expression
+          keyword_args[k] = v
         else
-          break
+          filter_args << expression
         end
+
+        break unless consume?(:comma)
       end
-      str
+
+      result = [filter_args]
+      result << keyword_args if keyword_args
+      result
     end
   end
 end

lib/liquid/range_lookup.rb

@@ -12,6 +12,8 @@ module Liquid
       end
     end
 
+    attr_reader :start_obj, :end_obj
+
     def initialize(start_obj, end_obj)
       @start_obj = start_obj
       @end_obj   = end_obj
@@ -23,6 +25,10 @@ module Liquid
       start_int..end_int
     end
 
+    def ==(other)
+      self.class == other.class && start_obj == other.start_obj && end_obj == other.end_obj
+    end
+
     private
 
     def to_integer(input)

lib/liquid/tags/for.rb

@@ -113,10 +113,9 @@ module Liquid
       @variable_name = p.consume(:id)
       raise SyntaxError, options[:locale].t("errors.syntax.for_invalid_in") unless p.id?('in')
 
-      collection_name  = p.expression
-      @collection_name = parse_expression(collection_name)
+      @collection_name = p.expression
 
-      @name     = "#{@variable_name}-#{collection_name}"
+      @name     = "#{@variable_name}-#{@collection_name}"
       @reversed = p.id?('reversed')
 
       while p.look(:id) && p.look(:colon, 1)
@@ -124,7 +123,17 @@ module Liquid
           raise SyntaxError, options[:locale].t("errors.syntax.for_invalid_attribute")
         end
         p.consume
-        set_attribute(attribute, p.expression)
+        case attribute
+        when 'offset'
+          @from =
+            if p.id?('continue')
+              :continue
+            else
+              p.expression
+            end
+        when 'limit'
+          @limit = p.expression
+        end
       end
       p.consume(:end_of_string)
     end

lib/liquid/tags/if.rb

@@ -114,15 +114,23 @@ module Liquid
     end
 
     def parse_comparison(p)
-      a = parse_expression(p.expression)
+      a = parse_operand_expression(p)
       if (op = p.consume?(:comparison))
-        b = parse_expression(p.expression)
+        b = parse_operand_expression(p)
         Condition.new(a, op, b)
       else
         Condition.new(a)
       end
     end
 
+    def parse_operand_expression(p)
+      if p.look(:id) && !p.look(:dot, 1) && !p.look(:open_square, 1)
+        parse_expression(p.consume)
+      else
+        p.expression
+      end
+    end
+
     class ParseTreeVisitor < Liquid::ParseTreeVisitor
       def children
         @node.blocks

lib/liquid/variable.rb

@@ -65,23 +65,15 @@ module Liquid
 
       return if p.look(:end_of_string)
 
-      @name = Expression.parse(p.expression)
+      @name = p.expression
       while p.consume?(:pipe)
         filtername = p.consume(:id)
-        filterargs = p.consume?(:colon) ? parse_filterargs(p) : []
-        @filters << parse_filter_expressions(filtername, filterargs)
+        filterargs = p.consume?(:colon) ? p.arguments : [[]]
+        @filters << [filtername] + filterargs
       end
       p.consume(:end_of_string)
     end
 
-    def parse_filterargs(p)
-      # first argument
-      filterargs = [p.argument]
-      # followed by comma separated others
-      filterargs << p.argument while p.consume?(:comma)
-      filterargs
-    end
-
     def render(context)
       obj = context.evaluate(@name)
 

lib/liquid/variable_lookup.rb

@@ -7,30 +7,62 @@ module Liquid
 
     attr_reader :name, :lookups
 
-    def self.parse(markup)
-      new(markup)
-    end
-
-    def initialize(markup)
+    def self.lax_parse(markup)
       lookups = markup.scan(VariableParser)
 
       name = lookups.shift
       if name =~ SQUARE_BRACKETED
         name = Expression.parse(Regexp.last_match(1))
       end
-      @name = name
 
-      @lookups       = lookups
-      @command_flags = 0
+      command_flags = 0
 
-      @lookups.each_index do |i|
+      lookups.each_index do |i|
         lookup = lookups[i]
         if lookup =~ SQUARE_BRACKETED
           lookups[i] = Expression.parse(Regexp.last_match(1))
         elsif COMMAND_METHODS.include?(lookup)
-          @command_flags |= 1 << i
+          command_flags |= 1 << i
         end
       end
+
+      new(name, lookups, command_flags)
+    end
+
+    def self.strict_parse(p)
+      if p.look(:id)
+        name = p.consume
+      else
+        p.consume(:open_square)
+        name = p.expression
+        p.consume(:close_square)
+      end
+
+      lookups = []
+      command_flags = 0
+
+      loop do
+        if p.consume?(:open_square)
+          lookups << p.expression
+          p.consume(:close_square)
+        elsif p.consume?(:dot)
+          lookup = p.consume(:id)
+          lookups << lookup
+          if COMMAND_METHODS.include?(lookup)
+            command_flags |= 1 << (lookups.length - 1)
+          end
+        else
+          break
+        end
+      end
+
+      new(name, lookups, command_flags)
+    end
+
+    def initialize(name, lookups, command_flags)
+      @name = name
+      @lookups = lookups
+      @command_flags = command_flags
     end
 
     def evaluate(context)
@@ -75,6 +107,18 @@ module Liquid
       self.class == other.class && state == other.state
     end
 
+    def to_s
+      str = name.dup
+      lookups.each do |lookup|
+        if lookup.instance_of?(String)
+          str += '.' + lookup
+        else
+          str += '[' + lookup.to_s + ']'
+        end
+      end
+      str
+    end
+
     protected
 
     def state