Extract Parser#variable_lookup out of Expression.parse

lib/liquid/parser.rb

@@ -50,6 +50,10 @@ module Liquid
     def expression
       token = @tokens[@p]
       case token[0]
+      when :id
+        variable_lookup
+      when :open_square
+        unnamed_variable_lookup
       when :string
         string
       when :number
@@ -68,6 +72,47 @@ module Liquid
       consume(:string)[1..-2]
     end
 
+    def variable_lookup
+      name = consume(:id)
+      lookups, command_flags = variable_lookups
+      if Expression::LITERALS.key?(name) && lookups.empty?
+        Expression::LITERALS[name]
+      else
+        VariableLookup.new(name, lookups, command_flags)
+      end
+    end
+
+    def unnamed_variable_lookup
+      name = indexed_lookup
+      lookups, command_flags = variable_lookups
+      VariableLookup.new(name, lookups, command_flags)
+    end
+
+    def expression_string
+      token = @tokens[@p]
+      case token[0]
+      when :id
+        str = consume
+        str << variable_lookups_string
+      when :open_square
+        str = consume.dup
+        str << expression_string
+        str << consume(:close_square)
+        str << variable_lookups_string
+      when :string, :number
+        consume
+      when :open_round
+        consume
+        first = expression_string
+        consume(:dotdot)
+        last = expression_string
+        consume(:close_round)
+        "(#{first}..#{last})"
+      else
+        raise SyntaxError, "#{token} is not a valid expression"
+      end
+    end
+
     def argument_string
       str = +""
       # might be a keyword argument (identifier: expression)
@@ -79,7 +124,7 @@ module Liquid
       str
     end
 
-    def variable_lookups
+    def variable_lookups_string
       str = +""
       loop do
         if look(:open_square)
@@ -96,31 +141,6 @@ module Liquid
       str
     end
 
-    def expression_string
-      token = @tokens[@p]
-      case token[0]
-      when :id
-        str = consume
-        str << variable_lookups
-      when :open_square
-        str = consume.dup
-        str << expression_string
-        str << consume(:close_square)
-        str << variable_lookups
-      when :string, :number
-        consume
-      when :open_round
-        consume
-        first = expression_string
-        consume(:dotdot)
-        last = expression_string
-        consume(:close_round)
-        "(#{first}..#{last})"
-      else
-        raise SyntaxError, "#{token} is not a valid expression"
-      end
-    end
-
     # Assumes safe input. For cases where you need the string.
     # Don't use this unless you're sure about what you're doing.
     def unsafe_parse_expression(markup)
@@ -132,5 +152,31 @@ module Liquid
     def parse_expression(markup)
       Expression.parse(markup, @ss, @cache)
     end
+
+    def variable_lookups
+      lookups = []
+      command_flags = 0
+      i = -1
+      loop do
+        i += 1
+        if look(:open_square)
+          lookups << indexed_lookup
+        elsif consume?(:dot)
+          lookup = consume(:id)
+          lookups << lookup
+          command_flags |= 1 << i if VariableLookup::COMMAND_METHODS.include?(lookup)
+        else
+          break
+        end
+      end
+      [lookups, command_flags]
+    end
+
+    def indexed_lookup
+      consume(:open_square)
+      expr = expression
+      consume(:close_square)
+      expr
+    end
   end
 end

test/integration/expression_test.rb

@@ -67,7 +67,7 @@ class ExpressionTest < Minitest::Test
     Liquid::Template.parse(template, expression_cache: cache).render
 
     assert_equal(
-      ["x", "y"],
+      [],
       cache.to_a.map { _1[0] }.sort,
     )
   end
@@ -91,7 +91,7 @@ class ExpressionTest < Minitest::Test
     cache = parse_context.instance_variable_get(:@expression_cache)
 
     assert_equal(
-      ["x", "y"],
+      [],
       cache.to_a.map { _1[0] }.sort,
     )
   end
@@ -112,7 +112,7 @@ class ExpressionTest < Minitest::Test
     Liquid::Template.parse(template, expression_cache: cache).render
 
     assert_equal(
-      ["x", "y"],
+      [],
       cache.to_a.map { _1[0] }.sort,
     )
   end

test/unit/parser_unit_test.rb

@@ -45,7 +45,7 @@ class ParserUnitTest < Minitest::Test
     assert_equal(false, p.look(:number, 1))
   end
 
-  def test_expressions
+  def test_expression_string
     p = new_parser("hi.there hi?[5].there? hi.there.bob")
     assert_equal('hi.there', p.expression_string)
     assert_equal('hi?[5].there?', p.expression_string)
@@ -58,6 +58,25 @@ class ParserUnitTest < Minitest::Test
     assert_equal('"wut"', p.expression_string)
   end
 
+  def test_expression
+    p = new_parser("hi.there hi?[5].there? hi.there.bob")
+    v1 = p.expression
+    v2 = p.expression
+    v3 = p.expression
+    assert(v1.is_a?(VariableLookup) && v1.name == 'hi' && v1.lookups[0] == 'there')
+    assert(v2.is_a?(VariableLookup) && v2.name == 'hi?' && v2.lookups[0] == 5)
+    assert(v3.is_a?(VariableLookup) && v3.name == 'hi' && v3.lookups[0] == 'there')
+
+    p = new_parser("567 6.0 'lol' \"wut\" true false (0..5)")
+    assert_equal(567, p.expression)
+    assert_equal(6.0, p.expression)
+    assert_equal('lol', p.expression)
+    assert_equal('wut', p.expression)
+    assert_equal(true, p.expression)
+    assert_equal(false, p.expression)
+    assert_equal((0..5), p.expression)
+  end
+
   def test_number
     p = new_parser('-1 0 1 2.0')
     assert_equal(-1, p.number)
@@ -74,6 +93,15 @@ class ParserUnitTest < Minitest::Test
     assert_equal("that 's4'", p.string)
   end
 
+  def test_unnamed_variable_lookup
+    p = new_parser('[key].title')
+    v = p.expression
+    assert(v.is_a?(VariableLookup))
+    assert(v.name.is_a?(VariableLookup))
+    assert_equal('key', v.name.name)
+    assert_equal('title', v.lookups[0])
+  end
+
   def test_ranges
     p = new_parser("(5..7) (1.5..9.6) (young..old) (hi[5].wat..old)")
     assert_equal('(5..7)', p.expression_string)