Rename Parser#expression_node -> Parser#expression

History.md

@@ -2,8 +2,6 @@
 
 ## 6.0.0
 
-### Architectural changes
-
 ### Features
 * (TODO) Add support for boolean expressions everywhere
   * As variable output `{{ a or b }}`
@@ -21,6 +19,13 @@
 - (TODO) Add support for parenthesized expressions
   * e.g. `(a or b) and c`
 
+### Architectural changes
+* `parse_expression` and `safe_parse_expression` have been removed from `Tag` and `ParseContext`
+* `Parser` methods now produce AST nodes instead of strings
+  * `Parser#expression` produces a value,
+  * `Parser#string` produces a string,
+  * etc.
+
 ### Breaking changes
 * The Environment's `error_mode` option has been removed.
   * `:warn` is no longer supported
@@ -28,14 +33,14 @@
   * `:strict` and `strict_parse` is no longer supported
   * `strict2_parse` is renamed to `parse_markup`
 * The `warnings` system has been removed.
-* `safe_parse_expression` has been moved to `Parser.expression_node`
-* `parse_expression` methods have been moved to `Parser#unsafe_parse_expression`
-  * Use `Parser#expression_node`, `Parser#string`, etc. instead
+* `Parser#expression` is renamed to `Parser#expression_string`
+* `safe_parse_expression` methods are replaced by `Parser#expression`
+* `parse_expression` methods are replaced by `Parser#unsafe_parse_expression`
 
 ### Migrating from `^5.11.0`
 - In custom tags that include `ParserSwitching`, rename `strict2_parse` to `parse_markup`
 - Remove code depending on `:error_mode`
-- Replace `safe_parse_expression` calls with `Parser.expression_node`
+- Replace `safe_parse_expression` calls with `Parser#expression`
 
 ## 5.11.0
 * Revert the Inline Snippets tag (#2001), treat its inclusion in the latest Liquid release as a bug, and allow for feedback on RFC#1916 to better support Liquid developers [Guilherme Carreiro]

lib/liquid/parser.rb

@@ -47,7 +47,7 @@ module Liquid
       tok[0] == type
     end
 
-    def expression_node
+    def expression
       parse_expression(expression_string)
     end
 

lib/liquid/tags/case.rb

@@ -85,7 +85,7 @@ module Liquid
 
     def parse_markup(markup)
       parser = @parse_context.new_parser(markup)
-      @left = parser.expression_node
+      @left = parser.expression
       parser.consume(:end_of_string)
     end
 
@@ -99,7 +99,7 @@ module Liquid
       parser = @parse_context.new_parser(markup)
 
       loop do
-        expr = parser.expression_node
+        expr = parser.expression
         block = Condition.new(@left, '==', expr)
         block.attach(body)
         @blocks << block

lib/liquid/tags/cycle.rb

@@ -61,14 +61,14 @@ module Liquid
 
       raise SyntaxError, options[:locale].t("errors.syntax.cycle") if p.look(:end_of_string)
 
-      first_expression = p.expression_node
+      first_expression = p.expression
       if p.look(:colon)
         # cycle name: expr1, expr2, ...
         @name = first_expression
         @is_named = true
         p.consume(:colon)
         # After the colon, parse the first variable (required for named cycles)
-        @variables << maybe_dup_lookup(p.expression_node)
+        @variables << maybe_dup_lookup(p.expression)
       else
         # cycle expr1, expr2, ...
         @variables << maybe_dup_lookup(first_expression)
@@ -78,7 +78,7 @@ module Liquid
       while p.consume?(:comma)
         break if p.look(:end_of_string)
 
-        @variables << maybe_dup_lookup(p.expression_node)
+        @variables << maybe_dup_lookup(p.expression)
       end
 
       p.consume(:end_of_string)

lib/liquid/tags/include.rb

@@ -84,8 +84,8 @@ module Liquid
     def parse_markup(markup)
       p = @parse_context.new_parser(markup)
 
-      @template_name_expr = p.expression_node
-      @variable_name_expr = p.expression_node if p.id?("for") || p.id?("with")
+      @template_name_expr = p.expression
+      @variable_name_expr = p.expression if p.id?("for") || p.id?("with")
       @alias_name         = p.consume(:id) if p.id?("as")
 
       p.consume?(:comma)
@@ -94,7 +94,7 @@ module Liquid
       while p.look(:id)
         key = p.consume
         p.consume(:colon)
-        @attributes[key] = p.expression_node
+        @attributes[key] = p.expression
         p.consume?(:comma)
       end
 

lib/liquid/tags/render.rb

@@ -89,7 +89,7 @@ module Liquid
 
       @template_name_expr = template_name(p)
       with_or_for         = p.id?("for") || p.id?("with")
-      @variable_name_expr = p.expression_node if with_or_for
+      @variable_name_expr = p.expression if with_or_for
       @alias_name         = p.consume(:id) if p.id?("as")
       @is_for_loop        = (with_or_for == FOR)
 
@@ -99,7 +99,7 @@ module Liquid
       while p.look(:id)
         key = p.consume
         p.consume(:colon)
-        @attributes[key] = p.expression_node
+        @attributes[key] = p.expression
         p.consume?(:comma)
       end
 

lib/liquid/tags/table_row.rb

@@ -42,7 +42,7 @@ module Liquid
         raise SyntaxError, options[:locale].t("errors.syntax.for_invalid_in")
       end
 
-      @collection_name = p.expression_node
+      @collection_name = p.expression
 
       p.consume?(:comma)
 
@@ -54,7 +54,7 @@ module Liquid
         end
 
         p.consume(:colon)
-        @attributes[key] = p.expression_node
+        @attributes[key] = p.expression
         p.consume?(:comma)
       end
 

lib/liquid/variable.rb

@@ -47,7 +47,7 @@ module Liquid
 
       return if p.look(:end_of_string)
 
-      @name = p.expression_node
+      @name = p.expression
       @filters << parse_filter_expressions(p) while p.consume?(:pipe)
       p.consume(:end_of_string)
     end
@@ -121,10 +121,10 @@ module Liquid
       if p.look(:id) && p.look(:colon, 1)
         key = p.consume(:id)
         p.consume(:colon)
-        value = p.expression_node
+        value = p.expression
         keyword_arguments[key] = value
       else
-        positional_arguments << p.expression_node
+        positional_arguments << p.expression
       end
     end
 

performance/unit/expression_benchmark.rb

@@ -6,7 +6,7 @@ require "benchmark/ips"
 
 require 'liquid'
 
-RubyVM::YJIT.enable
+RubyVM::YJIT.enable if defined?(RubyVM::YJIT)
 
 STRING_MARKUPS = [
   "\"foo\"",
@@ -45,22 +45,14 @@ NUMBER_MARKUPS = [
 
 RANGE_MARKUPS = [
   "(1..30)",
-  "(1...30)",
-  "(1..30..5)",
-  "(1.0...30.0)",
-  "(1.........30)",
   "(1..foo)",
   "(foo..30)",
   "(foo..bar)",
-  "(foo...bar...100)",
-  "(foo...bar...100.0)",
 ]
 
 LITERAL_MARKUPS = [
-  nil,
   'nil',
   'null',
-  '',
   'true',
   'false',
   'blank',
@@ -75,20 +67,39 @@ MARKUPS = {
   "range" => RANGE_MARKUPS,
 }
 
-Benchmark.ips do |x|
-  x.config(time: 5, warmup: 5)
+module Liquid
+  Benchmark.ips do |x|
+    x.config(time: 5, warmup: 5)
 
-  MARKUPS.each do |type, markups|
-    x.report("Liquid::Expression#parse: #{type}") do
-      markups.each do |markup|
-        Liquid::Expression.parse(markup)
+    ss = StringScanner.new('')
+
+    MARKUPS.each do |type, markups|
+      x.report("#{type} - Liquid::Expression#parse") do
+        markups.each do |markup|
+          ss.string = markup
+          Expression.parse(markup, ss)
+        end
+      end
+
+      x.report("#{type} - Liquid::Parser#expression") do
+        markups.each do |markup|
+          ss.string = markup
+          Parser.new(ss).expression
+        end
+      end
+
+      x.report("#{type} - Liquid::Expression.parse(Parser#expression_string)") do
+        markups.each do |markup|
+          ss.string = markup
+          Expression.parse(Parser.new(ss).expression_string, ss)
+        end
+      end
+    end
+
+    x.report("Liquid::Expression#parse: all") do
+      MARKUPS.values.flatten.each do |markup|
+        Expression.parse(markup)
       end
     end
   end
-
-  x.report("Liquid::Expression#parse: all") do
-    MARKUPS.values.flatten.each do |markup|
-      Liquid::Expression.parse(markup)
-    end
-  end
 end

performance/unit/lexer_benchmark.rb

@@ -6,7 +6,7 @@ require "benchmark/ips"
 
 require 'liquid'
 
-RubyVM::YJIT.enable
+RubyVM::YJIT.enable if defined?(RubyVM::YJIT)
 
 EXPRESSIONS = [
   "foo[1..2].baz",
@@ -31,11 +31,12 @@ EXPRESSIONS = [
 
 Benchmark.ips do |x|
   x.config(time: 10, warmup: 5)
+  ss = StringScanner.new('')
 
   x.report("Liquid::Lexer#tokenize") do
     EXPRESSIONS.each do |expr|
-      l = Liquid::Lexer.new(expr)
-      l.tokenize
+      ss.string = expr
+      Liquid::Lexer.tokenize(ss)
     end
   end
 

test/unit/parse_context_unit_test.rb

@@ -5,45 +5,45 @@ require 'test_helper'
 class ParseContextUnitTest < Minitest::Test
   include Liquid
 
-  def test_parser_expression_node_with_variable_lookup
+  def test_parser_expression_with_variable_lookup
     parser = parse_context.new_parser('product.title')
-    result = parser.expression_node
+    result = parser.expression
 
     assert_instance_of(VariableLookup, result)
     assert_equal('product', result.name)
     assert_equal(['title'], result.lookups)
   end
 
-  def test_parser_expression_node_raises_syntax_error_for_invalid_expression
+  def test_parser_expression_raises_syntax_error_for_invalid_expression
     parser = parse_context.new_parser('')
 
     error = assert_raises(Liquid::SyntaxError) do
-      parser.expression_node
+      parser.expression
     end
 
     assert_match(/is not a valid expression/, error.message)
   end
 
   def test_parse_expression_with_variable_lookup
-    result = parse_context.new_parser('product.title').expression_node
+    result = parse_context.new_parser('product.title').expression
 
     assert_instance_of(VariableLookup, result)
     assert_equal('product', result.name)
     assert_equal(['title'], result.lookups)
   end
 
-  def test_parser_expression_node_advances_parser_pointer
+  def test_parser_expression_advances_parser_pointer
     parser = parse_context.new_parser('foo, bar')
 
-    # parser.expression_node consumes "foo"
-    first_result = parser.expression_node
+    # parser.expression consumes "foo"
+    first_result = parser.expression
     assert_instance_of(VariableLookup, first_result)
     assert_equal('foo', first_result.name)
 
     parser.consume(:comma)
 
-    # parser.expression_node consumes "bar"
-    second_result = parser.expression_node
+    # parser.expression consumes "bar"
+    second_result = parser.expression
     assert_instance_of(VariableLookup, second_result)
     assert_equal('bar', second_result.name)