From c688f705281bc26615a39d9312aa1e689e523ecd Mon Sep 17 00:00:00 2001 From: "Charles-P. Clermont" Date: Tue, 2 Dec 2025 11:03:26 -0500 Subject: [PATCH 01/14] Move `safe_parse_expression` into `Parser.expression_node` - Make `Parser` accept the expression cache - Remove `safe_parse_expression` from `ParseContext` - Replace all usage of `safe_parse_expression` with `parser.expression_node` --- History.md | 2 ++ lib/liquid/parse_context.rb | 6 +----- lib/liquid/parser.rb | 12 +++++++++--- lib/liquid/tag.rb | 4 ---- 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 +++--- test/unit/parse_context_unit_test.rb | 12 ++++++------ 11 files changed, 33 insertions(+), 33 deletions(-) diff --git a/History.md b/History.md index 8fd6ef372..cf6382eb8 100644 --- a/History.md +++ b/History.md @@ -28,10 +28,12 @@ * `: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` ### 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` ## 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/parse_context.rb b/lib/liquid/parse_context.rb index 96413eb61..e6279d767 100644 --- a/lib/liquid/parse_context.rb +++ b/lib/liquid/parse_context.rb @@ -37,7 +37,7 @@ def new_block_body def new_parser(input) @string_scanner.string = input - Parser.new(@string_scanner) + Parser.new(@string_scanner, @expression_cache) end def new_tokenizer(source, start_line_number: nil, for_liquid_tag: false) @@ -49,10 +49,6 @@ def new_tokenizer(source, start_line_number: nil, for_liquid_tag: false) ) end - def safe_parse_expression(parser) - Expression.safe_parse(parser, @string_scanner, @expression_cache) - end - def parse_expression(markup, safe: false) # markup MUST come from a string returned by the parser # (e.g., parser.expression). We're not calling the parser here to diff --git a/lib/liquid/parser.rb b/lib/liquid/parser.rb index 645dfa3a1..86de2eab3 100644 --- a/lib/liquid/parser.rb +++ b/lib/liquid/parser.rb @@ -2,9 +2,10 @@ module Liquid class Parser - def initialize(input) - ss = input.is_a?(StringScanner) ? input : StringScanner.new(input) - @tokens = Lexer.tokenize(ss) + def initialize(input, expression_cache = nil) + @ss = input.is_a?(StringScanner) ? input : StringScanner.new(input) + @cache = expression_cache + @tokens = Lexer.tokenize(@ss) @p = 0 # pointer to current location end @@ -71,6 +72,11 @@ def expression end end + def expression_node + expr = expression + Expression.parse(expr, @ss, @cache) + end + def argument str = +"" # might be a keyword argument (identifier: expression) diff --git a/lib/liquid/tag.rb b/lib/liquid/tag.rb index 374ee511e..501eec611 100644 --- a/lib/liquid/tag.rb +++ b/lib/liquid/tag.rb @@ -68,10 +68,6 @@ def blank? private - def safe_parse_expression(parser) - parse_context.safe_parse_expression(parser) - end - def parse_expression(markup, safe: false) parse_context.parse_expression(markup, safe: safe) end diff --git a/lib/liquid/tags/case.rb b/lib/liquid/tags/case.rb index 0a466c44c..6fd8586dc 100644 --- a/lib/liquid/tags/case.rb +++ b/lib/liquid/tags/case.rb @@ -85,7 +85,7 @@ def render_to_output_buffer(context, output) def parse_markup(markup) parser = @parse_context.new_parser(markup) - @left = safe_parse_expression(parser) + @left = parser.expression_node parser.consume(:end_of_string) end @@ -99,7 +99,7 @@ def parse_when(markup, body) parser = @parse_context.new_parser(markup) loop do - expr = safe_parse_expression(parser) + expr = parser.expression_node 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 a98d25392..4847824eb 100644 --- a/lib/liquid/tags/cycle.rb +++ b/lib/liquid/tags/cycle.rb @@ -61,14 +61,14 @@ def parse_markup(markup) raise SyntaxError, options[:locale].t("errors.syntax.cycle") if p.look(:end_of_string) - first_expression = safe_parse_expression(p) + first_expression = p.expression_node 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(safe_parse_expression(p)) + @variables << maybe_dup_lookup(p.expression_node) else # cycle expr1, expr2, ... @variables << maybe_dup_lookup(first_expression) @@ -78,7 +78,7 @@ def parse_markup(markup) while p.consume?(:comma) break if p.look(:end_of_string) - @variables << maybe_dup_lookup(safe_parse_expression(p)) + @variables << maybe_dup_lookup(p.expression_node) end p.consume(:end_of_string) diff --git a/lib/liquid/tags/include.rb b/lib/liquid/tags/include.rb index b50c68a66..02d8bf4e1 100644 --- a/lib/liquid/tags/include.rb +++ b/lib/liquid/tags/include.rb @@ -84,8 +84,8 @@ def render_to_output_buffer(context, output) def parse_markup(markup) p = @parse_context.new_parser(markup) - @template_name_expr = safe_parse_expression(p) - @variable_name_expr = safe_parse_expression(p) if p.id?("for") || p.id?("with") + @template_name_expr = p.expression_node + @variable_name_expr = p.expression_node if p.id?("for") || p.id?("with") @alias_name = p.consume(:id) if p.id?("as") p.consume?(:comma) @@ -94,7 +94,7 @@ def parse_markup(markup) while p.look(:id) key = p.consume p.consume(:colon) - @attributes[key] = safe_parse_expression(p) + @attributes[key] = p.expression_node p.consume?(:comma) end diff --git a/lib/liquid/tags/render.rb b/lib/liquid/tags/render.rb index 5c071c803..5a2ef0675 100644 --- a/lib/liquid/tags/render.rb +++ b/lib/liquid/tags/render.rb @@ -89,7 +89,7 @@ def parse_markup(markup) @template_name_expr = parse_expression(template_name(p), safe: true) with_or_for = p.id?("for") || p.id?("with") - @variable_name_expr = safe_parse_expression(p) if with_or_for + @variable_name_expr = p.expression_node if with_or_for @alias_name = p.consume(:id) if p.id?("as") @is_for_loop = (with_or_for == FOR) @@ -99,7 +99,7 @@ def parse_markup(markup) while p.look(:id) key = p.consume p.consume(:colon) - @attributes[key] = safe_parse_expression(p) + @attributes[key] = p.expression_node p.consume?(:comma) end diff --git a/lib/liquid/tags/table_row.rb b/lib/liquid/tags/table_row.rb index c91147541..3b4cc5866 100644 --- a/lib/liquid/tags/table_row.rb +++ b/lib/liquid/tags/table_row.rb @@ -42,7 +42,7 @@ def parse_markup(markup) raise SyntaxError, options[:locale].t("errors.syntax.for_invalid_in") end - @collection_name = safe_parse_expression(p) + @collection_name = p.expression_node p.consume?(:comma) @@ -54,7 +54,7 @@ def parse_markup(markup) end p.consume(:colon) - @attributes[key] = safe_parse_expression(p) + @attributes[key] = p.expression_node p.consume?(:comma) end diff --git a/lib/liquid/variable.rb b/lib/liquid/variable.rb index fcc723515..c73523587 100644 --- a/lib/liquid/variable.rb +++ b/lib/liquid/variable.rb @@ -47,7 +47,7 @@ def parse_markup(markup) return if p.look(:end_of_string) - @name = parse_context.safe_parse_expression(p) + @name = p.expression_node @filters << parse_filter_expressions(p) while p.consume?(:pipe) p.consume(:end_of_string) end @@ -121,10 +121,10 @@ def argument(p, positional_arguments, keyword_arguments) if p.look(:id) && p.look(:colon, 1) key = p.consume(:id) p.consume(:colon) - value = parse_context.safe_parse_expression(p) + value = p.expression_node keyword_arguments[key] = value else - positional_arguments << parse_context.safe_parse_expression(p) + positional_arguments << p.expression_node end end diff --git a/test/unit/parse_context_unit_test.rb b/test/unit/parse_context_unit_test.rb index f75a48045..89923a751 100644 --- a/test/unit/parse_context_unit_test.rb +++ b/test/unit/parse_context_unit_test.rb @@ -7,7 +7,7 @@ class ParseContextUnitTest < Minitest::Test def test_safe_parse_expression_with_variable_lookup parser = parse_context.new_parser('product.title') - result = parse_context.safe_parse_expression(parser) + result = parser.expression_node assert_instance_of(VariableLookup, result) assert_equal('product', result.name) @@ -18,7 +18,7 @@ def test_safe_parse_expression_raises_syntax_error_for_invalid_expression parser = parse_context.new_parser('') error = assert_raises(Liquid::SyntaxError) do - parse_context.safe_parse_expression(parser) + parser.expression_node end assert_match(/is not a valid expression/, error.message) @@ -56,15 +56,15 @@ def test_parse_expression_with_empty_string_and_safe_true def test_safe_parse_expression_advances_parser_pointer parser = parse_context.new_parser('foo, bar') - # safe_parse_expression consumes "foo" - first_result = parse_context.safe_parse_expression(parser) + # parser.expression_node consumes "foo" + first_result = parser.expression_node assert_instance_of(VariableLookup, first_result) assert_equal('foo', first_result.name) parser.consume(:comma) - # safe_parse_expression consumes "bar" - second_result = parse_context.safe_parse_expression(parser) + # parser.expression_node consumes "bar" + second_result = parser.expression_node assert_instance_of(VariableLookup, second_result) assert_equal('bar', second_result.name) From 158feb0d01367c074bf9bbe83a1a969d7e4f62d6 Mon Sep 17 00:00:00 2001 From: "Charles-P. Clermont" Date: Tue, 2 Dec 2025 11:36:44 -0500 Subject: [PATCH 02/14] Move parse_expression to Parser.unsafe_parse_expression - Add Parser#string - Add Parser#unsafe_parse_expression - Add private Parser#parse_expression - Remove ParseContext.parse_expression - Remove Tag.parse_expression - Condition.parse_expression now takes a parser as argument --- History.md | 2 + lib/liquid/condition.rb | 5 +- lib/liquid/parse_context.rb | 11 -- lib/liquid/parser.rb | 16 ++- lib/liquid/parser.rb.orig | 165 +++++++++++++++++++++++++++ lib/liquid/tag.rb | 6 - lib/liquid/tags/for.rb | 11 +- lib/liquid/tags/if.rb | 8 +- lib/liquid/tags/render.rb | 4 +- test/unit/condition_unit_test.rb | 15 ++- test/unit/parse_context_unit_test.rb | 34 +----- 11 files changed, 207 insertions(+), 70 deletions(-) create mode 100644 lib/liquid/parser.rb.orig diff --git a/History.md b/History.md index cf6382eb8..cf93d993b 100644 --- a/History.md +++ b/History.md @@ -29,6 +29,8 @@ * `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 ### Migrating from `^5.11.0` - In custom tags that include `ParserSwitching`, rename `strict2_parse` to `parse_markup` diff --git a/lib/liquid/condition.rb b/lib/liquid/condition.rb index 9ab350f07..ac1908773 100644 --- a/lib/liquid/condition.rb +++ b/lib/liquid/condition.rb @@ -48,8 +48,9 @@ def self.operators @@operators end - def self.parse_expression(parse_context, markup, safe: false) - @@method_literals[markup] || parse_context.parse_expression(markup, safe: safe) + def self.parse_expression(parser) + markup = parser.expression + @@method_literals[markup] || parser.unsafe_parse_expression(markup) end attr_reader :attachment, :child_condition diff --git a/lib/liquid/parse_context.rb b/lib/liquid/parse_context.rb index e6279d767..c8cc35e38 100644 --- a/lib/liquid/parse_context.rb +++ b/lib/liquid/parse_context.rb @@ -49,17 +49,6 @@ def new_tokenizer(source, start_line_number: nil, for_liquid_tag: false) ) end - def parse_expression(markup, safe: false) - # markup MUST come from a string returned by the parser - # (e.g., parser.expression). We're not calling the parser here to - # prevent redundant parser overhead. The `safe` opt-in - # exists to ensure it is not accidentally still called with - # the result of a regex. - raise Liquid::InternalError, "unsafe parse_expression cannot be used" unless safe - - Expression.parse(markup, @string_scanner, @expression_cache) - end - def partial=(value) @partial = value @options = value ? partial_options : @template_options diff --git a/lib/liquid/parser.rb b/lib/liquid/parser.rb index 86de2eab3..606644071 100644 --- a/lib/liquid/parser.rb +++ b/lib/liquid/parser.rb @@ -72,9 +72,9 @@ def expression end end + def expression_node - expr = expression - Expression.parse(expr, @ss, @cache) + parse_expression(expression) end def argument @@ -104,5 +104,17 @@ def variable_lookups 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) + parse_expression(markup) + end + + private + + def parse_expression(markup) + Expression.parse(markup, @ss, @cache) + end end end diff --git a/lib/liquid/parser.rb.orig b/lib/liquid/parser.rb.orig new file mode 100644 index 000000000..3e177e8ef --- /dev/null +++ b/lib/liquid/parser.rb.orig @@ -0,0 +1,165 @@ +# frozen_string_literal: true + +module Liquid + class Parser + def initialize(input, expression_cache = nil) + @ss = input.is_a?(StringScanner) ? input : StringScanner.new(input) + @cache = expression_cache + @tokens = Lexer.tokenize(@ss) + @p = 0 # pointer to current location + end + + def jump(point) + @p = point + end + + def consume(type = nil) + token = @tokens[@p] + if type && token[0] != type + raise SyntaxError, "Expected #{type} but found #{@tokens[@p].first}" + end + @p += 1 + token[1] + end + + # Only consumes the token if it matches the type + # Returns the token's contents if it was consumed + # or false otherwise. + def consume?(type) + token = @tokens[@p] + return false unless token && token[0] == type + @p += 1 + token[1] + end + + # Like consume? Except for an :id token of a certain name + def id?(str) + token = @tokens[@p] + return false unless token && token[0] == :id + return false unless token[1] == str + @p += 1 + token[1] + end + + def look(type, ahead = 0) + tok = @tokens[@p + ahead] + return false unless tok + tok[0] == type + end + + def expression + token = @tokens[@p] + case token[0] + when :id + str = consume + str << variable_lookups + when :open_square + str = consume.dup + str << expression + str << consume(:close_square) + str << variable_lookups + when :string, :number + consume + when :open_round + consume + first = expression + consume(:dotdot) + last = expression + consume(:close_round) + "(#{first}..#{last})" + else + raise SyntaxError, "#{token} is not a valid expression" + end + end + + def string + parse_expression(consume(:string)) + end + + def expression_node + parse_expression(expression) + end + +<<<<<<< HEAD + # 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) + parse_expression(markup) + end + + def argument +======= + def string + consume(:string)[1..-2] + end + + def argument_string +>>>>>>> 68476f39 (Move unsafe_parse_expression to the end) + str = +"" + # might be a keyword argument (identifier: expression) + if look(:id) && look(:colon, 1) + str << consume << consume << ' ' + end + + 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) + else + break + end + end + str + end + +<<<<<<< HEAD +======= + 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 range_lookup + consume(:open_round) + first = expression + consume(:dotdot) + last = expression + consume(:close_round) + RangeLookup.create(first, last) + 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) + parse_expression(markup) + end + +>>>>>>> 68476f39 (Move unsafe_parse_expression to the end) + private + + def parse_expression(markup) + Expression.parse(markup, @ss, @cache) + end + end +end diff --git a/lib/liquid/tag.rb b/lib/liquid/tag.rb index 501eec611..216a80bd5 100644 --- a/lib/liquid/tag.rb +++ b/lib/liquid/tag.rb @@ -65,11 +65,5 @@ def render_to_output_buffer(context, output) def blank? false end - - private - - def parse_expression(markup, safe: false) - parse_context.parse_expression(markup, safe: safe) - end end end diff --git a/lib/liquid/tags/for.rb b/lib/liquid/tags/for.rb index cac9d2393..ae507c25f 100644 --- a/lib/liquid/tags/for.rb +++ b/lib/liquid/tags/for.rb @@ -76,7 +76,7 @@ def parse_markup(markup) raise SyntaxError, options[:locale].t("errors.syntax.for_invalid_in") unless p.id?('in') collection_name = p.expression - @collection_name = parse_expression(collection_name, safe: true) + @collection_name = p.unsafe_parse_expression(collection_name) @name = "#{@variable_name}-#{collection_name}" @reversed = p.id?('reversed') @@ -87,7 +87,7 @@ def parse_markup(markup) raise SyntaxError, options[:locale].t("errors.syntax.for_invalid_attribute") end p.consume(:colon) - set_attribute(attribute, p.expression, safe: true) + set_attribute(attribute, p) end p.consume(:end_of_string) end @@ -157,16 +157,17 @@ def render_segment(context, output, segment) output end - def set_attribute(key, expr, safe: false) + def set_attribute(key, p) + expr = p.expression case key when 'offset' @from = if expr == 'continue' :continue else - parse_expression(expr, safe: safe) + p.unsafe_parse_expression(expr) end when 'limit' - @limit = parse_expression(expr, safe: safe) + @limit = p.unsafe_parse_expression(expr) end end diff --git a/lib/liquid/tags/if.rb b/lib/liquid/tags/if.rb index 06c752da7..2081c788f 100644 --- a/lib/liquid/tags/if.rb +++ b/lib/liquid/tags/if.rb @@ -73,8 +73,8 @@ def push_block(tag, markup) block.attach(new_body) end - def parse_expression(markup, safe: false) - Condition.parse_expression(parse_context, markup, safe: safe) + def parse_expression(parser) + Condition.parse_expression(parser) end def parse_markup(markup) @@ -96,9 +96,9 @@ def parse_binary_comparisons(p) end def parse_comparison(p) - a = parse_expression(p.expression, safe: true) + a = parse_expression(p) if (op = p.consume?(:comparison)) - b = parse_expression(p.expression, safe: true) + b = parse_expression(p) Condition.new(a, op, b) else Condition.new(a) diff --git a/lib/liquid/tags/render.rb b/lib/liquid/tags/render.rb index 5a2ef0675..fab99487e 100644 --- a/lib/liquid/tags/render.rb +++ b/lib/liquid/tags/render.rb @@ -87,7 +87,7 @@ def render_tag(context, output) def parse_markup(markup) p = @parse_context.new_parser(markup) - @template_name_expr = parse_expression(template_name(p), safe: true) + @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 @alias_name = p.consume(:id) if p.id?("as") @@ -107,7 +107,7 @@ def parse_markup(markup) end def template_name(p) - p.consume(:string) + p.string end class ParseTreeVisitor < Liquid::ParseTreeVisitor diff --git a/test/unit/condition_unit_test.rb b/test/unit/condition_unit_test.rb index 584951e21..7245e3b2d 100644 --- a/test/unit/condition_unit_test.rb +++ b/test/unit/condition_unit_test.rb @@ -166,25 +166,24 @@ def test_default_context_is_deprecated assert_includes(err.lines.map(&:strip), expected) end - def test_parse_expression_with_safe_true + def test_parse_expression environment = Environment.build parse_context = ParseContext.new(environment: environment) - result = Condition.parse_expression(parse_context, 'product.title', safe: true) + parser = parse_context.new_parser('product.title') + result = Condition.parse_expression(parser) assert_instance_of(VariableLookup, result) assert_equal('product', result.name) assert_equal(['title'], result.lookups) end - def test_parse_expression_raises_internal_error_if_not_safe + def test_parse_expression_returns_method_literal_for_blank_and_empty environment = Environment.build parse_context = ParseContext.new(environment: environment) + parser = parse_context.new_parser('blank') + result = Condition.parse_expression(parser) - error = assert_raises(Liquid::InternalError) do - Condition.parse_expression(parse_context, 'product.title') - end - - assert_match(/unsafe parse_expression cannot be used/, error.message) + assert_instance_of(Condition::MethodLiteral, result) end private diff --git a/test/unit/parse_context_unit_test.rb b/test/unit/parse_context_unit_test.rb index 89923a751..80b8ffc6d 100644 --- a/test/unit/parse_context_unit_test.rb +++ b/test/unit/parse_context_unit_test.rb @@ -5,7 +5,7 @@ class ParseContextUnitTest < Minitest::Test include Liquid - def test_safe_parse_expression_with_variable_lookup + def test_parser_expression_node_with_variable_lookup parser = parse_context.new_parser('product.title') result = parser.expression_node @@ -14,7 +14,7 @@ def test_safe_parse_expression_with_variable_lookup assert_equal(['title'], result.lookups) end - def test_safe_parse_expression_raises_syntax_error_for_invalid_expression + def test_parser_expression_node_raises_syntax_error_for_invalid_expression parser = parse_context.new_parser('') error = assert_raises(Liquid::SyntaxError) do @@ -25,35 +25,14 @@ def test_safe_parse_expression_raises_syntax_error_for_invalid_expression end def test_parse_expression_with_variable_lookup - error = assert_raises(Liquid::InternalError) do - parse_context.parse_expression('product.title') - end - - assert_match(/unsafe parse_expression cannot be used/, error.message) - end - - def test_parse_expression_with_safe_true - result = parse_context.parse_expression('product.title', safe: true) + result = parse_context.new_parser('product.title').expression_node assert_instance_of(VariableLookup, result) assert_equal('product', result.name) assert_equal(['title'], result.lookups) end - def test_parse_expression_with_empty_string - error = assert_raises(Liquid::InternalError) do - parse_context.parse_expression('') - end - - assert_match(/unsafe parse_expression cannot be used/, error.message) - end - - def test_parse_expression_with_empty_string_and_safe_true - result = parse_context.parse_expression('', safe: true) - assert_nil(result) - end - - def test_safe_parse_expression_advances_parser_pointer + def test_parser_expression_node_advances_parser_pointer parser = parse_context.new_parser('foo, bar') # parser.expression_node consumes "foo" @@ -71,11 +50,6 @@ def test_safe_parse_expression_advances_parser_pointer parser.consume(:end_of_string) end - def test_parse_expression_with_whitespace - result = parse_context.parse_expression(' ', safe: true) - assert_nil(result) - end - private def parse_context From 44106dc25023622c670d41e6e70aca1f54ec8875 Mon Sep 17 00:00:00 2001 From: "Charles-P. Clermont" Date: Tue, 2 Dec 2025 12:08:06 -0500 Subject: [PATCH 03/14] Rename Parser#expression -> Parser#expression_string --- lib/liquid/condition.rb | 2 +- lib/liquid/expression.rb | 2 +- lib/liquid/parser.rb | 57 +++++++++++++++++------------------ lib/liquid/tags/for.rb | 4 +-- test/unit/parser_unit_test.rb | 24 +++++++-------- 5 files changed, 44 insertions(+), 45 deletions(-) diff --git a/lib/liquid/condition.rb b/lib/liquid/condition.rb index ac1908773..8c4afc980 100644 --- a/lib/liquid/condition.rb +++ b/lib/liquid/condition.rb @@ -49,7 +49,7 @@ def self.operators end def self.parse_expression(parser) - markup = parser.expression + markup = parser.expression_string @@method_literals[markup] || parser.unsafe_parse_expression(markup) end diff --git a/lib/liquid/expression.rb b/lib/liquid/expression.rb index 3f9f08837..919cdf040 100644 --- a/lib/liquid/expression.rb +++ b/lib/liquid/expression.rb @@ -26,7 +26,7 @@ class Expression class << self def safe_parse(parser, ss = StringScanner.new(""), cache = nil) - parse(parser.expression, ss, cache) + parse(parser.expression_string, ss, cache) end def parse(markup, ss = StringScanner.new(""), cache = nil) diff --git a/lib/liquid/parser.rb b/lib/liquid/parser.rb index 606644071..881a24a37 100644 --- a/lib/liquid/parser.rb +++ b/lib/liquid/parser.rb @@ -47,34 +47,8 @@ def look(type, ahead = 0) tok[0] == type end - def expression - token = @tokens[@p] - case token[0] - when :id - str = consume - str << variable_lookups - when :open_square - str = consume.dup - str << expression - str << consume(:close_square) - str << variable_lookups - when :string, :number - consume - when :open_round - consume - first = expression - consume(:dotdot) - last = expression - consume(:close_round) - "(#{first}..#{last})" - else - raise SyntaxError, "#{token} is not a valid expression" - end - end - - def expression_node - parse_expression(expression) + parse_expression(expression_string) end def argument @@ -84,7 +58,7 @@ def argument str << consume << consume << ' ' end - str << expression + str << expression_string str end @@ -93,7 +67,7 @@ def variable_lookups loop do if look(:open_square) str << consume - str << expression + str << expression_string str << consume(:close_square) elsif look(:dot) str << consume @@ -105,6 +79,31 @@ def variable_lookups 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) diff --git a/lib/liquid/tags/for.rb b/lib/liquid/tags/for.rb index ae507c25f..3e23b2a14 100644 --- a/lib/liquid/tags/for.rb +++ b/lib/liquid/tags/for.rb @@ -75,7 +75,7 @@ def parse_markup(markup) @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 = p.expression_string @collection_name = p.unsafe_parse_expression(collection_name) @name = "#{@variable_name}-#{collection_name}" @@ -158,7 +158,7 @@ def render_segment(context, output, segment) end def set_attribute(key, p) - expr = p.expression + expr = p.expression_string case key when 'offset' @from = if expr == 'continue' diff --git a/test/unit/parser_unit_test.rb b/test/unit/parser_unit_test.rb index 2c6a3594e..453c10f80 100644 --- a/test/unit/parser_unit_test.rb +++ b/test/unit/parser_unit_test.rb @@ -47,23 +47,23 @@ def test_look def test_expressions p = new_parser("hi.there hi?[5].there? hi.there.bob") - assert_equal('hi.there', p.expression) - assert_equal('hi?[5].there?', p.expression) - assert_equal('hi.there.bob', p.expression) + assert_equal('hi.there', p.expression_string) + assert_equal('hi?[5].there?', p.expression_string) + assert_equal('hi.there.bob', p.expression_string) p = new_parser("567 6.0 'lol' \"wut\"") - assert_equal('567', p.expression) - assert_equal('6.0', p.expression) - assert_equal("'lol'", p.expression) - assert_equal('"wut"', p.expression) + assert_equal('567', p.expression_string) + assert_equal('6.0', p.expression_string) + assert_equal("'lol'", p.expression_string) + assert_equal('"wut"', p.expression_string) 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) - assert_equal('(1.5..9.6)', p.expression) - assert_equal('(young..old)', p.expression) - assert_equal('(hi[5].wat..old)', p.expression) + assert_equal('(5..7)', p.expression_string) + assert_equal('(1.5..9.6)', p.expression_string) + assert_equal('(young..old)', p.expression_string) + assert_equal('(hi[5].wat..old)', p.expression_string) end def test_arguments @@ -78,7 +78,7 @@ def test_arguments def test_invalid_expression assert_raises(SyntaxError) do p = new_parser("==") - p.expression + p.expression_string end end From 123c2e3ea8aa8f5cedbbd6d636c3af4970ef0b0c Mon Sep 17 00:00:00 2001 From: "Charles-P. Clermont" Date: Wed, 3 Dec 2025 09:55:15 -0500 Subject: [PATCH 04/14] Rename Parser#argument -> argument_string --- lib/liquid/parser.rb | 2 +- lib/liquid/parser.rb.orig | 165 ---------------------------------- test/unit/parser_unit_test.rb | 6 +- 3 files changed, 4 insertions(+), 169 deletions(-) delete mode 100644 lib/liquid/parser.rb.orig diff --git a/lib/liquid/parser.rb b/lib/liquid/parser.rb index 881a24a37..229a94bbe 100644 --- a/lib/liquid/parser.rb +++ b/lib/liquid/parser.rb @@ -51,7 +51,7 @@ def expression_node parse_expression(expression_string) end - def argument + def argument_string str = +"" # might be a keyword argument (identifier: expression) if look(:id) && look(:colon, 1) diff --git a/lib/liquid/parser.rb.orig b/lib/liquid/parser.rb.orig deleted file mode 100644 index 3e177e8ef..000000000 --- a/lib/liquid/parser.rb.orig +++ /dev/null @@ -1,165 +0,0 @@ -# frozen_string_literal: true - -module Liquid - class Parser - def initialize(input, expression_cache = nil) - @ss = input.is_a?(StringScanner) ? input : StringScanner.new(input) - @cache = expression_cache - @tokens = Lexer.tokenize(@ss) - @p = 0 # pointer to current location - end - - def jump(point) - @p = point - end - - def consume(type = nil) - token = @tokens[@p] - if type && token[0] != type - raise SyntaxError, "Expected #{type} but found #{@tokens[@p].first}" - end - @p += 1 - token[1] - end - - # Only consumes the token if it matches the type - # Returns the token's contents if it was consumed - # or false otherwise. - def consume?(type) - token = @tokens[@p] - return false unless token && token[0] == type - @p += 1 - token[1] - end - - # Like consume? Except for an :id token of a certain name - def id?(str) - token = @tokens[@p] - return false unless token && token[0] == :id - return false unless token[1] == str - @p += 1 - token[1] - end - - def look(type, ahead = 0) - tok = @tokens[@p + ahead] - return false unless tok - tok[0] == type - end - - def expression - token = @tokens[@p] - case token[0] - when :id - str = consume - str << variable_lookups - when :open_square - str = consume.dup - str << expression - str << consume(:close_square) - str << variable_lookups - when :string, :number - consume - when :open_round - consume - first = expression - consume(:dotdot) - last = expression - consume(:close_round) - "(#{first}..#{last})" - else - raise SyntaxError, "#{token} is not a valid expression" - end - end - - def string - parse_expression(consume(:string)) - end - - def expression_node - parse_expression(expression) - end - -<<<<<<< HEAD - # 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) - parse_expression(markup) - end - - def argument -======= - def string - consume(:string)[1..-2] - end - - def argument_string ->>>>>>> 68476f39 (Move unsafe_parse_expression to the end) - str = +"" - # might be a keyword argument (identifier: expression) - if look(:id) && look(:colon, 1) - str << consume << consume << ' ' - end - - 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) - else - break - end - end - str - end - -<<<<<<< HEAD -======= - 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 range_lookup - consume(:open_round) - first = expression - consume(:dotdot) - last = expression - consume(:close_round) - RangeLookup.create(first, last) - 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) - parse_expression(markup) - end - ->>>>>>> 68476f39 (Move unsafe_parse_expression to the end) - private - - def parse_expression(markup) - Expression.parse(markup, @ss, @cache) - end - end -end diff --git a/test/unit/parser_unit_test.rb b/test/unit/parser_unit_test.rb index 453c10f80..a800f90e3 100644 --- a/test/unit/parser_unit_test.rb +++ b/test/unit/parser_unit_test.rb @@ -66,13 +66,13 @@ def test_ranges assert_equal('(hi[5].wat..old)', p.expression_string) end - def test_arguments + def test_argument_string p = new_parser("filter: hi.there[5], keyarg: 7") assert_equal('filter', p.consume(:id)) assert_equal(':', p.consume(:colon)) - assert_equal('hi.there[5]', p.argument) + assert_equal('hi.there[5]', p.argument_string) assert_equal(',', p.consume(:comma)) - assert_equal('keyarg: 7', p.argument) + assert_equal('keyarg: 7', p.argument_string) end def test_invalid_expression From 8222a1ced60dbe2ebb95c4f027338ec68a411e34 Mon Sep 17 00:00:00 2001 From: "Charles-P. Clermont" Date: Tue, 2 Dec 2025 12:13:51 -0500 Subject: [PATCH 05/14] 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 | 49 +++++++++++++++--------- performance/unit/lexer_benchmark.rb | 7 ++-- test/unit/parse_context_unit_test.rb | 20 +++++----- 11 files changed, 71 insertions(+), 54 deletions(-) diff --git a/History.md b/History.md index cf93d993b..4337aeb98 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 229a94bbe..5709e1be1 100644 --- a/lib/liquid/parser.rb +++ b/lib/liquid/parser.rb @@ -47,7 +47,7 @@ def look(type, ahead = 0) 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 6fd8586dc..0f23ead1d 100644 --- a/lib/liquid/tags/case.rb +++ b/lib/liquid/tags/case.rb @@ -85,7 +85,7 @@ def render_to_output_buffer(context, output) 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 @@ def parse_when(markup, body) 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 4847824eb..1c166f555 100644 --- a/lib/liquid/tags/cycle.rb +++ b/lib/liquid/tags/cycle.rb @@ -61,14 +61,14 @@ def parse_markup(markup) 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 @@ def parse_markup(markup) 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 02d8bf4e1..1f193473b 100644 --- a/lib/liquid/tags/include.rb +++ b/lib/liquid/tags/include.rb @@ -84,8 +84,8 @@ def render_to_output_buffer(context, output) 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 @@ def parse_markup(markup) 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 fab99487e..a5babca28 100644 --- a/lib/liquid/tags/render.rb +++ b/lib/liquid/tags/render.rb @@ -89,7 +89,7 @@ def parse_markup(markup) @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 @@ def parse_markup(markup) 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 3b4cc5866..e6cfff040 100644 --- a/lib/liquid/tags/table_row.rb +++ b/lib/liquid/tags/table_row.rb @@ -42,7 +42,7 @@ def parse_markup(markup) 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 @@ def parse_markup(markup) 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 c73523587..9dd6eec56 100644 --- a/lib/liquid/variable.rb +++ b/lib/liquid/variable.rb @@ -47,7 +47,7 @@ def parse_markup(markup) 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 @@ def argument(p, positional_arguments, keyword_arguments) 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 8cd552595..eca9e825b 100644 --- a/performance/unit/expression_benchmark.rb +++ b/performance/unit/expression_benchmark.rb @@ -6,7 +6,7 @@ require 'liquid' -RubyVM::YJIT.enable +RubyVM::YJIT.enable if defined?(RubyVM::YJIT) STRING_MARKUPS = [ "\"foo\"", @@ -45,22 +45,14 @@ 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 @@ "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 - end - x.report("Liquid::Expression#parse: all") do - MARKUPS.values.flatten.each do |markup| - Liquid::Expression.parse(markup) + x.report("Liquid::Expression#parse: all") do + MARKUPS.values.flatten.each do |markup| + Expression.parse(markup) + end end end end diff --git a/performance/unit/lexer_benchmark.rb b/performance/unit/lexer_benchmark.rb index fda6ce1f6..261451527 100644 --- a/performance/unit/lexer_benchmark.rb +++ b/performance/unit/lexer_benchmark.rb @@ -6,7 +6,7 @@ require 'liquid' -RubyVM::YJIT.enable +RubyVM::YJIT.enable if defined?(RubyVM::YJIT) EXPRESSIONS = [ "foo[1..2].baz", @@ -31,11 +31,12 @@ 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 80b8ffc6d..42773ef7c 100644 --- a/test/unit/parse_context_unit_test.rb +++ b/test/unit/parse_context_unit_test.rb @@ -5,45 +5,45 @@ 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) From 58cb8858331e7a44d9c0898b6def975aafedac76 Mon Sep 17 00:00:00 2001 From: "Charles-P. Clermont" Date: Tue, 2 Dec 2025 12:21:26 -0500 Subject: [PATCH 06/14] Remove Expression#safe_parse --- lib/liquid/expression.rb | 4 ---- test/integration/expression_test.rb | 12 ++++++------ 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/lib/liquid/expression.rb b/lib/liquid/expression.rb index 919cdf040..f73fe8fac 100644 --- a/lib/liquid/expression.rb +++ b/lib/liquid/expression.rb @@ -25,10 +25,6 @@ class Expression FLOAT_REGEX = /\A(-?\d+)\.\d+\z/ class << self - def safe_parse(parser, ss = StringScanner.new(""), cache = nil) - parse(parser.expression_string, ss, cache) - end - def parse(markup, ss = StringScanner.new(""), cache = nil) return unless markup diff --git a/test/integration/expression_test.rb b/test/integration/expression_test.rb index 988f1d122..166df2507 100644 --- a/test/integration/expression_test.rb +++ b/test/integration/expression_test.rb @@ -134,30 +134,30 @@ def test_disable_expression_cache assert(parse_context.instance_variable_get(:@expression_cache).nil?) end - def test_safe_parse_with_variable_lookup + def test_parser_expression_with_variable_lookup parse_context = Liquid::ParseContext.new parser = parse_context.new_parser('product.title') - result = Liquid::Expression.safe_parse(parser) + result = parser.expression assert_instance_of(Liquid::VariableLookup, result) assert_equal('product', result.name) assert_equal(['title'], result.lookups) end - def test_safe_parse_with_number + def test_parser_expression_with_number parse_context = Liquid::ParseContext.new parser = parse_context.new_parser('42') - result = Liquid::Expression.safe_parse(parser) + result = parser.expression assert_equal(42, result) end - def test_safe_parse_raises_syntax_error_for_invalid_expression + def test_parser_expression_raises_syntax_error_for_invalid_expression parse_context = Liquid::ParseContext.new parser = parse_context.new_parser('') error = assert_raises(Liquid::SyntaxError) do - Liquid::Expression.safe_parse(parser) + parser.expression end assert_match(/is not a valid expression/, error.message) From 8848065cf971e5bf1a1c1c7d0bb9ca520c734af8 Mon Sep 17 00:00:00 2001 From: "Charles-P. Clermont" Date: Tue, 2 Dec 2025 14:39:11 -0500 Subject: [PATCH 07/14] Extract Parser#string out of Expression.parse --- lib/liquid/parser.rb | 12 +++++++++++- test/unit/parser_unit_test.rb | 8 ++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/lib/liquid/parser.rb b/lib/liquid/parser.rb index 5709e1be1..5c2613c89 100644 --- a/lib/liquid/parser.rb +++ b/lib/liquid/parser.rb @@ -48,7 +48,17 @@ def look(type, ahead = 0) end def expression - parse_expression(expression_string) + token = @tokens[@p] + case token[0] + when :string + string + else + parse_expression(expression_string) + end + end + + def string + consume(:string)[1..-2] end def argument_string diff --git a/test/unit/parser_unit_test.rb b/test/unit/parser_unit_test.rb index a800f90e3..9bf737f0a 100644 --- a/test/unit/parser_unit_test.rb +++ b/test/unit/parser_unit_test.rb @@ -58,6 +58,14 @@ def test_expressions assert_equal('"wut"', p.expression_string) end + def test_string + p = new_parser("'s1' \"s2\" 'this \"s3\"' \"that 's4'\"") + assert_equal('s1', p.string) + assert_equal('s2', p.string) + assert_equal('this "s3"', p.string) + assert_equal("that 's4'", p.string) + 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) From 96e1595886ca47fd69d5a345a40f39e72470a13a Mon Sep 17 00:00:00 2001 From: "Charles-P. Clermont" Date: Tue, 2 Dec 2025 15:58:42 -0500 Subject: [PATCH 08/14] Extract Parser#number parse out of Expression.parse --- lib/liquid/parser.rb | 7 +++++++ test/integration/expression_test.rb | 6 +++--- test/unit/parser_unit_test.rb | 8 ++++++++ 3 files changed, 18 insertions(+), 3 deletions(-) diff --git a/lib/liquid/parser.rb b/lib/liquid/parser.rb index 5c2613c89..ba4bad57c 100644 --- a/lib/liquid/parser.rb +++ b/lib/liquid/parser.rb @@ -52,11 +52,18 @@ def expression case token[0] when :string string + when :number + number else parse_expression(expression_string) end end + def number + num = consume(:number) + Expression.parse_number(num) + end + def string consume(:string)[1..-2] end diff --git a/test/integration/expression_test.rb b/test/integration/expression_test.rb index 166df2507..fd5273fa9 100644 --- a/test/integration/expression_test.rb +++ b/test/integration/expression_test.rb @@ -67,7 +67,7 @@ def test_expression_cache Liquid::Template.parse(template, expression_cache: cache).render assert_equal( - ["1", "2", "x", "y"], + ["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( - ["1", "2", "x", "y"], + ["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( - ["1", "2", "x", "y"], + ["x", "y"], cache.to_a.map { _1[0] }.sort, ) end diff --git a/test/unit/parser_unit_test.rb b/test/unit/parser_unit_test.rb index 9bf737f0a..117114bf9 100644 --- a/test/unit/parser_unit_test.rb +++ b/test/unit/parser_unit_test.rb @@ -58,6 +58,14 @@ def test_expressions assert_equal('"wut"', p.expression_string) end + def test_number + p = new_parser('-1 0 1 2.0') + assert_equal(-1, p.number) + assert_equal(0, p.number) + assert_equal(1, p.number) + assert_equal(2.0, p.number) + end + def test_string p = new_parser("'s1' \"s2\" 'this \"s3\"' \"that 's4'\"") assert_equal('s1', p.string) From 7b37ba61cbe3b0c715d980c9c48f43dfed2afefe Mon Sep 17 00:00:00 2001 From: "Charles-P. Clermont" Date: Tue, 2 Dec 2025 16:01:09 -0500 Subject: [PATCH 09/14] Simplify parse_number We don't need all the multi-dot logic in a world where number comes out of the Lexer. --- lib/liquid/expression.rb | 51 ++++------------------------------------ 1 file changed, 5 insertions(+), 46 deletions(-) diff --git a/lib/liquid/expression.rb b/lib/liquid/expression.rb index f73fe8fac..8c82b96be 100644 --- a/lib/liquid/expression.rb +++ b/lib/liquid/expression.rb @@ -57,63 +57,22 @@ def inner_parse(markup, ss, cache) ) end - if (num = parse_number(markup, ss)) + if (num = parse_number(markup)) num else VariableLookup.parse(markup, ss, cache) end end - def parse_number(markup, ss) + def parse_number(markup) # check if the markup is simple integer or float case markup when INTEGER_REGEX - return Integer(markup, 10) + Integer(markup, 10) when FLOAT_REGEX - return markup.to_f - end - - ss.string = markup - # the first byte must be a digit or a dash - byte = ss.scan_byte - - return false if byte != DASH && (byte < ZERO || byte > NINE) - - if byte == DASH - peek_byte = ss.peek_byte - - # if it starts with a dash, the next byte must be a digit - return false if peek_byte.nil? || !(peek_byte >= ZERO && peek_byte <= NINE) - end - - # The markup could be a float with multiple dots - first_dot_pos = nil - num_end_pos = nil - - while (byte = ss.scan_byte) - return false if byte != DOT && (byte < ZERO || byte > NINE) - - # we found our number and now we are just scanning the rest of the string - next if num_end_pos - - if byte == DOT - if first_dot_pos.nil? - first_dot_pos = ss.pos - else - # we found another dot, so we know that the number ends here - num_end_pos = ss.pos - 1 - end - end - end - - num_end_pos = markup.length if ss.eos? - - if num_end_pos - # number ends with a number "123.123" - markup.byteslice(0, num_end_pos).to_f + markup.to_f else - # number ends with a dot "123." - markup.byteslice(0, first_dot_pos).to_f + false end end end From 58ce63ebb3b27612c3e43cf5fe125dbc0a78a507 Mon Sep 17 00:00:00 2001 From: "Charles-P. Clermont" Date: Tue, 2 Dec 2025 16:14:22 -0500 Subject: [PATCH 10/14] Replace RangeLookup.parse with RangeLookup.create --- lib/liquid/expression.rb | 14 +++++++++----- lib/liquid/range_lookup.rb | 4 +--- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/lib/liquid/expression.rb b/lib/liquid/expression.rb index 8c82b96be..1bf7edcf6 100644 --- a/lib/liquid/expression.rb +++ b/lib/liquid/expression.rb @@ -49,11 +49,15 @@ def parse(markup, ss = StringScanner.new(""), cache = nil) def inner_parse(markup, ss, cache) if (markup.start_with?("(") && markup.end_with?(")")) && markup =~ RANGES_REGEX - return RangeLookup.parse( - Regexp.last_match(1), - Regexp.last_match(2), - ss, - cache, + start_markup = Regexp.last_match(1) + end_markup = Regexp.last_match(2) + start_obj = parse(start_markup, ss, cache) + end_obj = parse(end_markup, ss, cache) + return RangeLookup.create( + start_obj, + end_obj, + start_markup, + end_markup, ) end diff --git a/lib/liquid/range_lookup.rb b/lib/liquid/range_lookup.rb index bc316fe12..e4ce296cd 100644 --- a/lib/liquid/range_lookup.rb +++ b/lib/liquid/range_lookup.rb @@ -2,9 +2,7 @@ module Liquid class RangeLookup - def self.parse(start_markup, end_markup, string_scanner, cache = nil) - start_obj = Expression.parse(start_markup, string_scanner, cache) - end_obj = Expression.parse(end_markup, string_scanner, cache) + def self.create(start_obj, end_obj, start_markup, end_markup) if start_obj.respond_to?(:evaluate) || end_obj.respond_to?(:evaluate) new(start_obj, end_obj) else From 1023f3f41122615ffecac2076415cff4aa0140d2 Mon Sep 17 00:00:00 2001 From: "Charles-P. Clermont" Date: Tue, 2 Dec 2025 16:19:41 -0500 Subject: [PATCH 11/14] Move VariableLookup parsing logic to .parse instead of initializer Goal is to get rid of it entirely, but baby steps. --- lib/liquid/variable_lookup.rb | 20 +++++++++++--------- test/unit/condition_unit_test.rb | 8 ++++---- test/unit/variable_unit_test.rb | 28 ++++++++++++++-------------- 3 files changed, 29 insertions(+), 27 deletions(-) diff --git a/lib/liquid/variable_lookup.rb b/lib/liquid/variable_lookup.rb index 340c0b66d..d8c09423a 100644 --- a/lib/liquid/variable_lookup.rb +++ b/lib/liquid/variable_lookup.rb @@ -7,10 +7,6 @@ class VariableLookup attr_reader :name, :lookups def self.parse(markup, string_scanner = StringScanner.new(""), cache = nil) - new(markup, string_scanner, cache) - end - - def initialize(markup, string_scanner = StringScanner.new(""), cache = nil) lookups = markup.scan(VariableParser) name = lookups.shift @@ -21,12 +17,10 @@ def initialize(markup, string_scanner = StringScanner.new(""), cache = nil) cache, ) 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&.start_with?('[') && lookup&.end_with?(']') lookups[i] = Expression.parse( @@ -35,9 +29,17 @@ def initialize(markup, string_scanner = StringScanner.new(""), cache = nil) cache, ) elsif COMMAND_METHODS.include?(lookup) - @command_flags |= 1 << i + command_flags |= 1 << i end end + + new(name, lookups, command_flags) + end + + def initialize(name, lookups, command_flags) + @name = name + @lookups = lookups + @command_flags = command_flags end def lookup_command?(lookup_index) diff --git a/test/unit/condition_unit_test.rb b/test/unit/condition_unit_test.rb index 7245e3b2d..9123b5099 100644 --- a/test/unit/condition_unit_test.rb +++ b/test/unit/condition_unit_test.rb @@ -82,7 +82,7 @@ def test_hash_compare_backwards_compatibility def test_contains_works_on_arrays @context = Liquid::Context.new @context['array'] = [1, 2, 3, 4, 5] - array_expr = VariableLookup.new("array") + array_expr = VariableLookup.parse("array") assert_evaluates_false(array_expr, 'contains', 0) assert_evaluates_true(array_expr, 'contains', 1) @@ -96,8 +96,8 @@ def test_contains_works_on_arrays def test_contains_returns_false_for_nil_operands @context = Liquid::Context.new - assert_evaluates_false(VariableLookup.new('not_assigned'), 'contains', '0') - assert_evaluates_false(0, 'contains', VariableLookup.new('not_assigned')) + assert_evaluates_false(VariableLookup.parse('not_assigned'), 'contains', '0') + assert_evaluates_false(0, 'contains', VariableLookup.parse('not_assigned')) end def test_contains_return_false_on_wrong_data_type @@ -149,7 +149,7 @@ def test_left_or_right_may_contain_operators @context = Liquid::Context.new @context['one'] = @context['another'] = "gnomeslab-and-or-liquid" - assert_evaluates_true(VariableLookup.new("one"), '==', VariableLookup.new("another")) + assert_evaluates_true(VariableLookup.parse("one"), '==', VariableLookup.parse("another")) end def test_default_context_is_deprecated diff --git a/test/unit/variable_unit_test.rb b/test/unit/variable_unit_test.rb index 6379c6ece..b6f1c9aae 100644 --- a/test/unit/variable_unit_test.rb +++ b/test/unit/variable_unit_test.rb @@ -7,20 +7,20 @@ class VariableUnitTest < Minitest::Test def test_variable var = create_variable('hello') - assert_equal(VariableLookup.new('hello'), var.name) + assert_equal(VariableLookup.parse('hello'), var.name) end def test_filters var = create_variable('hello | textileze') - assert_equal(VariableLookup.new('hello'), var.name) + assert_equal(VariableLookup.parse('hello'), var.name) assert_equal([['textileze', []]], var.filters) var = create_variable('hello | textileze | paragraph') - assert_equal(VariableLookup.new('hello'), var.name) + assert_equal(VariableLookup.parse('hello'), var.name) assert_equal([['textileze', []], ['paragraph', []]], var.filters) var = create_variable(%( hello | strftime: '%Y')) - assert_equal(VariableLookup.new('hello'), var.name) + assert_equal(VariableLookup.parse('hello'), var.name) assert_equal([['strftime', ['%Y']]], var.filters) var = create_variable(%( 'typo' | link_to: 'Typo', true )) @@ -44,11 +44,11 @@ def test_filters assert_equal([['repeat', [3, 3, 3]]], var.filters) var = create_variable(%( hello | strftime: '%Y, okay?')) - assert_equal(VariableLookup.new('hello'), var.name) + assert_equal(VariableLookup.parse('hello'), var.name) assert_equal([['strftime', ['%Y, okay?']]], var.filters) var = create_variable(%( hello | things: "%Y, okay?", 'the other one')) - assert_equal(VariableLookup.new('hello'), var.name) + assert_equal(VariableLookup.parse('hello'), var.name) assert_equal([['things', ['%Y, okay?', 'the other one']]], var.filters) end @@ -60,15 +60,15 @@ def test_filter_with_date_parameter def test_filters_without_whitespace var = create_variable('hello | textileze | paragraph') - assert_equal(VariableLookup.new('hello'), var.name) + assert_equal(VariableLookup.parse('hello'), var.name) assert_equal([['textileze', []], ['paragraph', []]], var.filters) var = create_variable('hello|textileze|paragraph') - assert_equal(VariableLookup.new('hello'), var.name) + assert_equal(VariableLookup.parse('hello'), var.name) assert_equal([['textileze', []], ['paragraph', []]], var.filters) var = create_variable("hello|replace:'foo','bar'|textileze") - assert_equal(VariableLookup.new('hello'), var.name) + assert_equal(VariableLookup.parse('hello'), var.name) assert_equal([['replace', ['foo', 'bar']], ['textileze', []]], var.filters) end @@ -99,8 +99,8 @@ def test_float end def test_dashes - assert_equal(VariableLookup.new('foo-bar'), create_variable('foo-bar').name) - assert_equal(VariableLookup.new('foo-bar-2'), create_variable('foo-bar-2').name) + assert_equal(VariableLookup.parse('foo-bar'), create_variable('foo-bar').name) + assert_equal(VariableLookup.parse('foo-bar-2'), create_variable('foo-bar-2').name) assert_raises(Liquid::SyntaxError) { create_variable('foo - bar') } assert_raises(Liquid::SyntaxError) { create_variable('-foo') } @@ -114,12 +114,12 @@ def test_string_with_special_chars def test_string_dot var = create_variable(%( test.test )) - assert_equal(VariableLookup.new('test.test'), var.name) + assert_equal(VariableLookup.parse('test.test'), var.name) end def test_filter_with_keyword_arguments var = create_variable(%( hello | things: greeting: "world", farewell: 'goodbye')) - assert_equal(VariableLookup.new('hello'), var.name) + assert_equal(VariableLookup.parse('hello'), var.name) assert_equal([['things', [], { 'greeting' => 'world', 'farewell' => 'goodbye' }]], var.filters) end @@ -163,7 +163,7 @@ def test_output_raw_source_of_variable end def test_variable_lookup_interface - lookup = VariableLookup.new('a.b.c') + lookup = VariableLookup.parse('a.b.c') assert_equal('a', lookup.name) assert_equal(['b', 'c'], lookup.lookups) end From 0d8011729d777dc358c5c284052fad3132f45308 Mon Sep 17 00:00:00 2001 From: "Charles-P. Clermont" Date: Tue, 2 Dec 2025 16:37:44 -0500 Subject: [PATCH 12/14] Extract Parser#variable_lookup out of Expression.parse --- lib/liquid/parser.rb | 96 +++++++++++++++++++++-------- test/integration/expression_test.rb | 6 +- test/unit/parser_unit_test.rb | 30 ++++++++- 3 files changed, 103 insertions(+), 29 deletions(-) diff --git a/lib/liquid/parser.rb b/lib/liquid/parser.rb index ba4bad57c..72516c264 100644 --- a/lib/liquid/parser.rb +++ b/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,32 +72,20 @@ 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 @@ -101,12 +93,12 @@ def expression_string 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 diff --git a/test/integration/expression_test.rb b/test/integration/expression_test.rb index fd5273fa9..2f53596d8 100644 --- a/test/integration/expression_test.rb +++ b/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 diff --git a/test/unit/parser_unit_test.rb b/test/unit/parser_unit_test.rb index 117114bf9..1b3e142df 100644 --- a/test/unit/parser_unit_test.rb +++ b/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) From ab8928106ed1a66cb8e993eb5e7bcb4258bd63d9 Mon Sep 17 00:00:00 2001 From: "Charles-P. Clermont" Date: Wed, 3 Dec 2025 11:40:51 -0500 Subject: [PATCH 13/14] Extract Parser#range_lookup out of Expression.parse --- lib/liquid/parser.rb | 13 ++++++++++++- lib/liquid/range_lookup.rb | 4 +++- test/integration/expression_test.rb | 2 +- test/unit/parser_unit_test.rb | 9 +++++++++ 4 files changed, 25 insertions(+), 3 deletions(-) diff --git a/lib/liquid/parser.rb b/lib/liquid/parser.rb index 72516c264..7003af9be 100644 --- a/lib/liquid/parser.rb +++ b/lib/liquid/parser.rb @@ -58,8 +58,10 @@ def expression string when :number number + when :open_round + range_lookup else - parse_expression(expression_string) + raise SyntaxError, "#{token} is not a valid expression" end end @@ -88,6 +90,15 @@ def unnamed_variable_lookup VariableLookup.new(name, lookups, command_flags) end + def range_lookup + consume(:open_round) + first = expression + consume(:dotdot) + last = expression + consume(:close_round) + RangeLookup.create(first, last) + end + def expression_string token = @tokens[@p] case token[0] diff --git a/lib/liquid/range_lookup.rb b/lib/liquid/range_lookup.rb index e4ce296cd..8e617f733 100644 --- a/lib/liquid/range_lookup.rb +++ b/lib/liquid/range_lookup.rb @@ -2,13 +2,15 @@ module Liquid class RangeLookup - def self.create(start_obj, end_obj, start_markup, end_markup) + def self.create(start_obj, end_obj, start_markup = nil, end_markup = nil) if start_obj.respond_to?(:evaluate) || end_obj.respond_to?(:evaluate) new(start_obj, end_obj) else begin start_obj.to_i..end_obj.to_i rescue NoMethodError + start_markup = start_obj.to_s unless start_markup + end_markup = end_obj.to_s unless end_markup invalid_expr = start_markup unless start_obj.respond_to?(:to_i) invalid_expr ||= end_markup unless end_obj.respond_to?(:to_i) if invalid_expr diff --git a/test/integration/expression_test.rb b/test/integration/expression_test.rb index 2f53596d8..89dc9ab10 100644 --- a/test/integration/expression_test.rb +++ b/test/integration/expression_test.rb @@ -46,7 +46,7 @@ def test_range "{{ (false..true) }}", ) assert_match_syntax_error( - "Liquid syntax error (line 1): Invalid expression type '(1..2)' in range expression", + "Liquid syntax error (line 1): Invalid expression type '1..2' in range expression", "{{ ((1..2)..3) }}", ) end diff --git a/test/unit/parser_unit_test.rb b/test/unit/parser_unit_test.rb index 1b3e142df..cf0d8c7bb 100644 --- a/test/unit/parser_unit_test.rb +++ b/test/unit/parser_unit_test.rb @@ -102,6 +102,15 @@ def test_unnamed_variable_lookup assert_equal('title', v.lookups[0]) end + def test_range_lookup + p = new_parser('(0..5) (a..b)') + assert_equal((0..5), p.expression) + + r2 = p.expression + assert(r2.is_a?(RangeLookup)) + assert_equal((1..4), r2.evaluate(Context.new({ 'a' => 1, 'b' => 4 }))) + 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) From 5e3d8f4ad54ef9f23c95e7c3b111d693da87673f Mon Sep 17 00:00:00 2001 From: "Charles-P. Clermont" Date: Wed, 3 Dec 2025 12:00:17 -0500 Subject: [PATCH 14/14] Replace Context[]'s Expression.parse with Parser#expression --- lib/liquid/context.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/liquid/context.rb b/lib/liquid/context.rb index 1db6bb663..750749f1e 100644 --- a/lib/liquid/context.rb +++ b/lib/liquid/context.rb @@ -175,7 +175,8 @@ def []=(key, value) # Example: # products == empty #=> products.empty? def [](expression) - evaluate(Expression.parse(expression, @string_scanner)) + @string_scanner.string = expression + evaluate(Parser.new(@string_scanner).expression) end def key?(key)