From 327790cdce43b30411635583f0bd841e33379b68 Mon Sep 17 00:00:00 2001 From: Guilherme Carreiro Date: Thu, 2 Oct 2025 11:29:49 +0200 Subject: [PATCH] Fix an int the `cycle` tag, add extra unit tests, and updated parser switcher: - Fixed NoMethod error with .peek (using look instead) - Add friendlier error message when {% cycle %} --- lib/liquid/parser_switching.rb | 13 ++++++++----- lib/liquid/tags/cycle.rb | 15 +++++++++++---- lib/liquid/tags/for.rb | 4 ++++ lib/liquid/tags/if.rb | 4 ++++ test/integration/tags/cycle_tag_test.rb | 24 ++++++++++++++++++++---- 5 files changed, 47 insertions(+), 13 deletions(-) diff --git a/lib/liquid/parser_switching.rb b/lib/liquid/parser_switching.rb index a978dc43..78b86b23 100644 --- a/lib/liquid/parser_switching.rb +++ b/lib/liquid/parser_switching.rb @@ -2,9 +2,12 @@ module Liquid module ParserSwitching - # Do not use this. Use parse_with_selected_parser instead. - # It's basically doing the same thing, except this will use strict_parse regardless - # of the error mode and fallback only if strict throws. + # Do not use this. + # + # It's basically doing the same thing the {#parse_with_selected_parser}, + # except this will use the strict parser, instead of the rigid parser. + # + # @deprecated Use {#parse_with_selected_parser} instead. def strict_parse_with_error_mode_fallback(markup) strict_parse_with_error_context(markup) rescue SyntaxError => e @@ -26,7 +29,7 @@ module Liquid when :lax then lax_parse(markup) when :warn begin - strict_parse_with_error_context(markup) + rigid_parse_with_error_context(markup) rescue SyntaxError => e parse_context.warnings << e lax_parse(markup) @@ -37,7 +40,7 @@ module Liquid private def rigid_parse_with_error_context(markup) - respond_to?(:rigid_parse) ? rigid_parse(markup) : strict_parse(markup) + rigid_parse(markup) rescue SyntaxError => e e.line_number = line_number e.markup_context = markup_context(markup) diff --git a/lib/liquid/tags/cycle.rb b/lib/liquid/tags/cycle.rb index 2df55bfb..d4f7166a 100644 --- a/lib/liquid/tags/cycle.rb +++ b/lib/liquid/tags/cycle.rb @@ -52,27 +52,34 @@ module Liquid output end + private + # cycle [name:] expression(, expression)* def rigid_parse(markup) p = @parse_context.new_parser(markup) - if p.look(:id) && p.peek(1) == :colon + if p.look(:id) && p.look(:colon, 1) @name = p.consume(:id) @is_named = true p.consume(:colon) end @variables = [] + + raise SyntaxError, options[:locale].t("errors.syntax.cycle") if p.look(:end_of_string) + while (var = p.expression) + var = parse_expression(var) @variables << var break unless p.consume?(:comma) end - raise_syntax_error(options) if @variables.empty? + unless @is_named + @name = @variables.to_s + @is_named = !@name.match?(/\w+:0x\h{8}/) + end end - private - # Temporarily until we migrate def strict_parse(markup) lax_parse(markup) diff --git a/lib/liquid/tags/for.rb b/lib/liquid/tags/for.rb index 6aa308f1..c2be5db1 100644 --- a/lib/liquid/tags/for.rb +++ b/lib/liquid/tags/for.rb @@ -111,6 +111,10 @@ module Liquid private + def rigid_parse(markup) + strict_parse(markup) + end + def collection_segment(context) offsets = context.registers[:for] ||= {} diff --git a/lib/liquid/tags/if.rb b/lib/liquid/tags/if.rb index 040fecb8..342374f1 100644 --- a/lib/liquid/tags/if.rb +++ b/lib/liquid/tags/if.rb @@ -66,6 +66,10 @@ module Liquid private + def rigid_parse(markup) + strict_parse(markup) + end + def push_block(tag, markup) block = if tag == 'else' ElseCondition.new diff --git a/test/integration/tags/cycle_tag_test.rb b/test/integration/tags/cycle_tag_test.rb index 8cd6daac..a4042479 100644 --- a/test/integration/tags/cycle_tag_test.rb +++ b/test/integration/tags/cycle_tag_test.rb @@ -46,18 +46,34 @@ class CycleTagTest < Minitest::Test assert_template_result("11", template) end + def test_cycle_tag_without_arguments + error = assert_raises(Liquid::SyntaxError) do + Template.parse("{% cycle %}") + end + + assert_match(/Syntax Error in 'cycle' - Valid syntax: cycle \[name :\] var/, error.message) + end + def test_cycle_tag_with_error_mode # QuotedFragment is more permissive than what Parser#expression allows. + temlate1 = "{% assign 5 = 'b' %}{% cycle .5, .4 %}" + temlate2 = "{% cycle .5: 'a', 'b' %}" + [:lax, :strict].each do |mode| with_error_mode(mode) do - assert_template_result("a", "{% cycle .5: 'a', 'b' %}") - assert_template_result("b", "{% assign 5 = 'b' %}{% cycle .5, .4 %}") + assert_template_result("b", temlate1) + assert_template_result("a", temlate2) end end with_error_mode(:rigid) do - assert_raises(Liquid::SyntaxError) { Template.parse("{% cycle .5: 'a', 'b' %}") } - assert_raises(Liquid::SyntaxError) { Template.parse("{% cycle .5, .4 %}") } + error1 = assert_raises(Liquid::SyntaxError) { Template.parse(temlate1) } + error2 = assert_raises(Liquid::SyntaxError) { Template.parse(temlate2) } + + expected_error = /Liquid syntax error: \[:dot, "."\] is not a valid expression/ + + assert_match(expected_error, error1.message) + assert_match(expected_error, error2.message) end end end