Extract Parser#variable_lookup out of Expression.parse

Author: charlespwd

lib/liquid/parser.rb

@@ -50,6 +50,10 @@ def look(type, ahead = 0)
     def expression
       token = @tokens[@p]
       case token[0]
+      when :id
+        variable_lookup
+      when :open_square
+        unnamed_variable_lookup
       when :string
         string
       when :number
@@ -68,45 +72,33 @@ def string
       consume(:string)[1..-2]
     end
 
-    def argument_string
-      str = +""
-      # might be a keyword argument (identifier: expression)
-      if look(:id) && look(:colon, 1)
-        str << consume << consume << ' '
+    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
-
-      str << expression_string
-      str
     end
 
-    def variable_lookups
-      str = +""
-      loop do
-        if look(:open_square)
-          str << consume
-          str << expression_string
-          str << consume(:close_square)
-        elsif look(:dot)
-          str << consume
-          str << consume(:id)
-        else
-          break
-        end
-      end
-      str
+    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
+        str << variable_lookups_string
       when :open_square
         str = consume.dup
         str << expression_string
         str << consume(:close_square)
-        str << variable_lookups
+        str << variable_lookups_string
       when :string, :number
         consume
       when :open_round
@@ -121,6 +113,34 @@ def expression_string
       end
     end
 
+    def argument_string
+      str = +""
+      # might be a keyword argument (identifier: expression)
+      if look(:id) && look(:colon, 1)
+        str << consume << consume << ' '
+      end
+
+      str << expression_string
+      str
+    end
+
+    def variable_lookups_string
+      str = +""
+      loop do
+        if look(:open_square)
+          str << consume
+          str << expression_string
+          str << consume(:close_square)
+        elsif look(:dot)
+          str << consume
+          str << consume(:id)
+        else
+          break
+        end
+      end
+      str
+    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 @@ def unsafe_parse_expression(markup)
     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 @@ def test_expression_cache
     Liquid::Template.parse(template, expression_cache: cache).render
 
     assert_equal(
-      ["x", "y"],
+      [],
       cache.to_a.map { _1[0] }.sort,
     )
   end
@@ -91,7 +91,7 @@ def test_expression_cache_with_true_boolean
     cache = parse_context.instance_variable_get(:@expression_cache)
 
     assert_equal(
-      ["x", "y"],
+      [],
       cache.to_a.map { _1[0] }.sort,
     )
   end
@@ -112,7 +112,7 @@ def test_expression_cache_with_lru_redux
     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 @@ def test_look
     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 @@ def test_expressions
     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 @@ def test_string
     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)