From c9ae1283548d8f53a5e8790b21a0f52d4f029c83 Mon Sep 17 00:00:00 2001 From: "Charles-P. Clermont" Date: Tue, 2 Dec 2025 12:13:51 -0500 Subject: [PATCH] Rename Parser#expression_node -> Parser#expression --- History.md | 17 +++++--- lib/liquid/parser.rb | 2 +- lib/liquid/tags/case.rb | 4 +- lib/liquid/tags/cycle.rb | 6 +-- lib/liquid/tags/include.rb | 6 +-- lib/liquid/tags/render.rb | 4 +- lib/liquid/tags/table_row.rb | 4 +- lib/liquid/variable.rb | 6 +-- performance/unit/expression_benchmark.rb | 53 ++++++++++++++---------- performance/unit/lexer_benchmark.rb | 7 ++-- test/unit/parse_context_unit_test.rb | 20 ++++----- 11 files changed, 73 insertions(+), 56 deletions(-) diff --git a/History.md b/History.md index cf93d993..4337aeb9 100644 --- a/History.md +++ b/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] diff --git a/lib/liquid/parser.rb b/lib/liquid/parser.rb index 229a94bb..5709e1be 100644 --- a/lib/liquid/parser.rb +++ b/lib/liquid/parser.rb @@ -47,7 +47,7 @@ module Liquid tok[0] == type end - def expression_node + def expression parse_expression(expression_string) end diff --git a/lib/liquid/tags/case.rb b/lib/liquid/tags/case.rb index 6fd8586d..0f23ead1 100644 --- a/lib/liquid/tags/case.rb +++ b/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 diff --git a/lib/liquid/tags/cycle.rb b/lib/liquid/tags/cycle.rb index 4847824e..1c166f55 100644 --- a/lib/liquid/tags/cycle.rb +++ b/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) diff --git a/lib/liquid/tags/include.rb b/lib/liquid/tags/include.rb index 02d8bf4e..1f193473 100644 --- a/lib/liquid/tags/include.rb +++ b/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 diff --git a/lib/liquid/tags/render.rb b/lib/liquid/tags/render.rb index fab99487..a5babca2 100644 --- a/lib/liquid/tags/render.rb +++ b/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 diff --git a/lib/liquid/tags/table_row.rb b/lib/liquid/tags/table_row.rb index 3b4cc586..e6cfff04 100644 --- a/lib/liquid/tags/table_row.rb +++ b/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 diff --git a/lib/liquid/variable.rb b/lib/liquid/variable.rb index c7352358..9dd6eec5 100644 --- a/lib/liquid/variable.rb +++ b/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 diff --git a/performance/unit/expression_benchmark.rb b/performance/unit/expression_benchmark.rb index 8cd55259..eca9e825 100644 --- a/performance/unit/expression_benchmark.rb +++ b/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 diff --git a/performance/unit/lexer_benchmark.rb b/performance/unit/lexer_benchmark.rb index fda6ce1f..26145152 100644 --- a/performance/unit/lexer_benchmark.rb +++ b/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 diff --git a/test/unit/parse_context_unit_test.rb b/test/unit/parse_context_unit_test.rb index 80b8ffc6..42773ef7 100644 --- a/test/unit/parse_context_unit_test.rb +++ b/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)