From aa11d345b7df4058c4507de4c3867a9ac89b39f4 Mon Sep 17 00:00:00 2001 From: "Charles-P. Clermont" Date: Thu, 6 Nov 2025 14:58:31 -0500 Subject: [PATCH 1/5] Remove :lax, :strict and :warn error modes --- README.md | 25 ------- Rakefile | 37 ++-------- lib/liquid/environment.rb | 4 +- lib/liquid/expression.rb | 3 - lib/liquid/parser_switching.rb | 57 +--------------- lib/liquid/tags/case.rb | 29 -------- lib/liquid/tags/cycle.rb | 31 --------- lib/liquid/tags/for.rb | 23 +------ lib/liquid/tags/if.rb | 30 +------- lib/liquid/tags/include.rb | 26 ------- lib/liquid/tags/render.rb | 23 ------- lib/liquid/tags/table_row.rb | 18 ----- lib/liquid/template.rb | 3 - lib/liquid/variable.rb | 59 +--------------- test/integration/assign_test.rb | 5 +- test/integration/context_test.rb | 2 +- test/integration/error_handling_test.rb | 52 ++------------ test/integration/expression_test.rb | 18 ----- test/integration/parsing_quirks_test.rb | 83 ++--------------------- test/integration/tags/cycle_tag_test.rb | 21 ------ test/integration/tags/include_tag_test.rb | 30 +------- test/integration/tags/render_tag_test.rb | 16 ----- test/integration/tags/table_row_test.rb | 35 ---------- test/integration/template_test.rb | 2 +- test/integration/variable_test.rb | 59 ---------------- test/test_helper.rb | 2 +- test/unit/condition_unit_test.rb | 4 +- test/unit/parse_context_unit_test.rb | 18 ----- test/unit/partial_cache_unit_test.rb | 4 +- test/unit/tags/case_tag_unit_test.rb | 20 +----- test/unit/variable_unit_test.rb | 38 +---------- 31 files changed, 35 insertions(+), 742 deletions(-) diff --git a/README.md b/README.md index 5066dc659..b3745e024 100644 --- a/README.md +++ b/README.md @@ -93,31 +93,6 @@ LIQUID By using Environments, you ensure that custom tags and filters are only available in the contexts where they are needed, making your Liquid templates more robust and easier to manage. For smaller projects, a global environment is available via `Liquid::Environment.default`. -### Error Modes - -Setting the error mode of Liquid lets you specify how strictly you want your templates to be interpreted. -Normally the parser is very lax and will accept almost anything without error. Unfortunately this can make -it very hard to debug and can lead to unexpected behaviour. - -Liquid also comes with different parsers that can be used when editing templates to give better error messages -when templates are invalid. You can enable this new parser like this: - -```ruby -Liquid::Environment.default.error_mode = :strict2 # Raises a SyntaxError when invalid syntax is used in all tags -Liquid::Environment.default.error_mode = :strict # Raises a SyntaxError when invalid syntax is used in some tags -Liquid::Environment.default.error_mode = :warn # Adds strict errors to template.errors but continues as normal -Liquid::Environment.default.error_mode = :lax # The default mode, accepts almost anything. -``` - -If you want to set the error mode only on specific templates you can pass `:error_mode` as an option to `parse`: -```ruby -Liquid::Template.parse(source, error_mode: :strict) -``` -This is useful for doing things like enabling strict mode only in the theme editor. - -It is recommended that you enable `:strict` or `:warn` mode on new apps to stop invalid templates from being created. -It is also recommended that you use it in the template editors of existing apps to give editors better error messages. - ### Undefined variables and filters By default, the renderer doesn't raise or in any other way notify you if some variables or filters are missing, i.e. not passed to the `render` method. diff --git a/Rakefile b/Rakefile index 83afcbfa0..b8939316b 100755 --- a/Rakefile +++ b/Rakefile @@ -33,28 +33,13 @@ task :rubocop do end end -desc('runs test suite with lax, strict, and strict2 parsers') +desc('runs test suite with strict2 parser') task :test do - ENV['LIQUID_PARSER_MODE'] = 'lax' - Rake::Task['base_test'].invoke - - ENV['LIQUID_PARSER_MODE'] = 'strict' - Rake::Task['base_test'].reenable - Rake::Task['base_test'].invoke - ENV['LIQUID_PARSER_MODE'] = 'strict2' Rake::Task['base_test'].reenable Rake::Task['base_test'].invoke if RUBY_ENGINE == 'ruby' || RUBY_ENGINE == 'truffleruby' - ENV['LIQUID_PARSER_MODE'] = 'lax' - Rake::Task['integration_test'].reenable - Rake::Task['integration_test'].invoke - - ENV['LIQUID_PARSER_MODE'] = 'strict' - Rake::Task['integration_test'].reenable - Rake::Task['integration_test'].invoke - ENV['LIQUID_PARSER_MODE'] = 'strict2' Rake::Task['integration_test'].reenable Rake::Task['integration_test'].invoke @@ -78,23 +63,13 @@ task release: :build do end namespace :benchmark do - desc "Run the liquid benchmark with lax parsing" - task :lax do - ruby "./performance/benchmark.rb lax" - end - - desc "Run the liquid benchmark with strict parsing" - task :strict do - ruby "./performance/benchmark.rb strict" - end - desc "Run the liquid benchmark with strict2 parsing" task :strict2 do ruby "./performance/benchmark.rb strict2" end - desc "Run the liquid benchmark with lax, strict, and strict2 parsing" - task run: [:lax, :strict, :strict2] + desc "Run the liquid benchmark" + task run: [:strict2] desc "Run unit benchmarks" namespace :unit do @@ -127,9 +102,9 @@ namespace :profile do ruby "./performance/profile.rb" end - desc "Run the liquid profile/performance coverage with strict parsing" - task :strict do - ruby "./performance/profile.rb strict" + desc "Run the liquid profile/performance coverage with strict2 parsing" + task :strict2 do + ruby "./performance/profile.rb strict2" end end diff --git a/lib/liquid/environment.rb b/lib/liquid/environment.rb index 100bd0bbd..095b2b091 100644 --- a/lib/liquid/environment.rb +++ b/lib/liquid/environment.rb @@ -34,7 +34,7 @@ class << self # @param file_system The default file system that is used # to load templates from. # @param error_mode [Symbol] The default error mode for all templates - # (either :strict2, :strict, :warn, or :lax). + # (:strict2). # @param exception_renderer [Proc] The exception renderer that is used to # render exceptions. # @yieldparam environment [Environment] The environment instance that is being built. @@ -75,7 +75,7 @@ def dangerously_override(environment) # @api private def initialize @tags = Tags::STANDARD_TAGS.dup - @error_mode = :lax + @error_mode = :strict2 @strainer_template = Class.new(StrainerTemplate).tap do |klass| klass.add_filter(StandardFilters) end diff --git a/lib/liquid/expression.rb b/lib/liquid/expression.rb index 2605c5576..3f9f08837 100644 --- a/lib/liquid/expression.rb +++ b/lib/liquid/expression.rb @@ -11,9 +11,6 @@ class Expression 'false' => false, 'blank' => '', 'empty' => '', - # in lax mode, minus sign can be a VariableLookup - # For simplicity and performace, we treat it like a literal - '-' => VariableLookup.parse("-", nil).freeze, }.freeze DOT = ".".ord diff --git a/lib/liquid/parser_switching.rb b/lib/liquid/parser_switching.rb index e419dc999..4b35e2139 100644 --- a/lib/liquid/parser_switching.rb +++ b/lib/liquid/parser_switching.rb @@ -2,59 +2,12 @@ module Liquid module ParserSwitching - # Do not use this. - # - # It's basically doing the same thing the {#parse_with_selected_parser}, - # except this will try the strict parser regardless of the error mode, - # and fall back to the lax parser if the error mode is lax or warn, - # except when in strict2 mode where it uses the strict2 parser. - # - # @deprecated Use {#parse_with_selected_parser} instead. - def strict_parse_with_error_mode_fallback(markup) - return strict2_parse_with_error_context(markup) if strict2_mode? - - strict_parse_with_error_context(markup) - rescue SyntaxError => e - case parse_context.error_mode - when :rigid - rigid_warn - raise - when :strict2 - raise - when :strict - raise - when :warn - parse_context.warnings << e - end - lax_parse(markup) - end - def parse_with_selected_parser(markup) - case parse_context.error_mode - when :rigid then rigid_warn && strict2_parse_with_error_context(markup) - when :strict2 then strict2_parse_with_error_context(markup) - when :strict then strict_parse_with_error_context(markup) - when :lax then lax_parse(markup) - when :warn - begin - strict2_parse_with_error_context(markup) - rescue SyntaxError => e - parse_context.warnings << e - lax_parse(markup) - end - end - end - - def strict2_mode? - parse_context.error_mode == :strict2 || parse_context.error_mode == :rigid + strict2_parse_with_error_context(markup) end private - def rigid_warn - Deprecations.warn(':rigid', ':strict2') - end - def strict2_parse_with_error_context(markup) strict2_parse(markup) rescue SyntaxError => e @@ -63,14 +16,6 @@ def strict2_parse_with_error_context(markup) raise e end - def strict_parse_with_error_context(markup) - strict_parse(markup) - rescue SyntaxError => e - e.line_number = line_number - e.markup_context = markup_context(markup) - raise e - end - def markup_context(markup) "in \"#{markup.strip}\"" end diff --git a/lib/liquid/tags/case.rb b/lib/liquid/tags/case.rb index b32ea1ae7..e24cf5a1b 100644 --- a/lib/liquid/tags/case.rb +++ b/lib/liquid/tags/case.rb @@ -23,9 +23,6 @@ module Liquid # @liquid_syntax_keyword second_expression An expression to be rendered when the variable's value matches `second_value`. # @liquid_syntax_keyword third_expression An expression to be rendered when the variable's value has no match. class Case < Block - Syntax = /(#{QuotedFragment})/o - WhenSyntax = /(#{QuotedFragment})(?:(?:\s+or\s+|\s*\,\s*)(#{QuotedFragment}.*))?/om - attr_reader :blocks, :left def initialize(tag_name, markup, options) @@ -92,18 +89,6 @@ def strict2_parse(markup) parser.consume(:end_of_string) end - def strict_parse(markup) - lax_parse(markup) - end - - def lax_parse(markup) - if markup =~ Syntax - @left = parse_expression(Regexp.last_match(1)) - else - raise SyntaxError, options[:locale].t("errors.syntax.case") - end - end - def record_when_condition(markup) body = new_body @@ -129,20 +114,6 @@ def parse_strict2_when(markup, body) parser.consume(:end_of_string) end - def parse_lax_when(markup, body) - while markup - unless markup =~ WhenSyntax - raise SyntaxError, options[:locale].t("errors.syntax.case_invalid_when") - end - - markup = Regexp.last_match(2) - - block = Condition.new(@left, '==', Condition.parse_expression(parse_context, Regexp.last_match(1))) - block.attach(body) - @blocks << block - end - end - def record_else_condition(markup) unless markup.strip.empty? raise SyntaxError, options[:locale].t("errors.syntax.case_invalid_else") diff --git a/lib/liquid/tags/cycle.rb b/lib/liquid/tags/cycle.rb index b7d3069c8..2d372cee5 100644 --- a/lib/liquid/tags/cycle.rb +++ b/lib/liquid/tags/cycle.rb @@ -15,8 +15,6 @@ module Liquid # @liquid_syntax # {% cycle string, string, ... %} class Cycle < Tag - SimpleSyntax = /\A#{QuotedFragment}+/o - NamedSyntax = /\A(#{QuotedFragment})\s*\:\s*(.*)/om UNNAMED_CYCLE_PATTERN = /\w+:0x\h{8}/ attr_reader :variables @@ -91,35 +89,6 @@ def strict2_parse(markup) end end - def strict_parse(markup) - lax_parse(markup) - end - - def lax_parse(markup) - case markup - when NamedSyntax - @variables = variables_from_string(Regexp.last_match(2)) - @name = parse_expression(Regexp.last_match(1)) - @is_named = true - when SimpleSyntax - @variables = variables_from_string(markup) - @name = @variables.to_s - @is_named = !@name.match?(UNNAMED_CYCLE_PATTERN) - else - raise SyntaxError, options[:locale].t("errors.syntax.cycle") - end - end - - def variables_from_string(markup) - markup.split(',').collect do |var| - var =~ /\s*(#{QuotedFragment})\s*/o - next unless Regexp.last_match(1) - - var = parse_expression(Regexp.last_match(1)) - maybe_dup_lookup(var) - end.compact - end - # For backwards compatibility, whenever a lookup is used in an unnamed cycle, # we make it so that the @variables.to_s produces different strings for cycles # called with the same arguments (since @variables.to_s is used as the cycle counter key) diff --git a/lib/liquid/tags/for.rb b/lib/liquid/tags/for.rb index cbea85bcb..4b89b7972 100644 --- a/lib/liquid/tags/for.rb +++ b/lib/liquid/tags/for.rb @@ -25,8 +25,6 @@ module Liquid # @liquid_optional_param range [untyped] A custom numeric range to iterate over. # @liquid_optional_param reversed [untyped] Iterate in reverse order. class For < Block - Syntax = /\A(#{VariableSegment}+)\s+in\s+(#{QuotedFragment}+)\s*(reversed)?/o - attr_reader :collection_name, :variable_name, :limit, :from def initialize(tag_name, markup, options) @@ -72,22 +70,7 @@ def render_to_output_buffer(context, output) protected - def lax_parse(markup) - if markup =~ Syntax - @variable_name = Regexp.last_match(1) - collection_name = Regexp.last_match(2) - @reversed = !!Regexp.last_match(3) - @name = "#{@variable_name}-#{collection_name}" - @collection_name = parse_expression(collection_name) - markup.scan(TagAttributes) do |key, value| - set_attribute(key, value) - end - else - raise SyntaxError, options[:locale].t("errors.syntax.for") - end - end - - def strict_parse(markup) + def strict2_parse(markup) p = @parse_context.new_parser(markup) @variable_name = p.consume(:id) raise SyntaxError, options[:locale].t("errors.syntax.for_invalid_in") unless p.id?('in') @@ -111,10 +94,6 @@ def strict_parse(markup) private - def strict2_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 c423c1e84..287fe78ae 100644 --- a/lib/liquid/tags/if.rb +++ b/lib/liquid/tags/if.rb @@ -14,10 +14,6 @@ module Liquid # @liquid_syntax_keyword condition The condition to evaluate. # @liquid_syntax_keyword expression The expression to render if the condition is met. class If < Block - Syntax = /(#{QuotedFragment})\s*([=!<>a-z_]+)?\s*(#{QuotedFragment})?/o - ExpressionsAndOperators = /(?:\b(?:\s?and\s?|\s?or\s?)\b|(?:\s*(?!\b(?:\s?and\s?|\s?or\s?)\b)(?:#{QuotedFragment}|\S+)\s*)+)/o - BOOLEAN_OPERATORS = %w(and or).freeze - attr_reader :blocks def initialize(tag_name, markup, options) @@ -66,10 +62,6 @@ def render_to_output_buffer(context, output) private - def strict2_parse(markup) - strict_parse(markup) - end - def push_block(tag, markup) block = if tag == 'else' ElseCondition.new @@ -85,27 +77,7 @@ def parse_expression(markup, safe: false) Condition.parse_expression(parse_context, markup, safe: safe) end - def lax_parse(markup) - expressions = markup.scan(ExpressionsAndOperators) - raise SyntaxError, options[:locale].t("errors.syntax.if") unless expressions.pop =~ Syntax - - condition = Condition.new(parse_expression(Regexp.last_match(1)), Regexp.last_match(2), parse_expression(Regexp.last_match(3))) - - until expressions.empty? - operator = expressions.pop.to_s.strip - - raise SyntaxError, options[:locale].t("errors.syntax.if") unless expressions.pop.to_s =~ Syntax - - new_condition = Condition.new(parse_expression(Regexp.last_match(1)), Regexp.last_match(2), parse_expression(Regexp.last_match(3))) - raise SyntaxError, options[:locale].t("errors.syntax.if") unless BOOLEAN_OPERATORS.include?(operator) - new_condition.send(operator, condition) - condition = new_condition - end - - condition - end - - def strict_parse(markup) + def strict2_parse(markup) p = @parse_context.new_parser(markup) condition = parse_binary_comparisons(p) p.consume(:end_of_string) diff --git a/lib/liquid/tags/include.rb b/lib/liquid/tags/include.rb index 969482d49..49bc032f6 100644 --- a/lib/liquid/tags/include.rb +++ b/lib/liquid/tags/include.rb @@ -20,9 +20,6 @@ module Liquid class Include < Tag prepend Tag::Disableable - SYNTAX = /(#{QuotedFragment}+)(\s+(?:with|for)\s+(#{QuotedFragment}+))?(\s+(?:as)\s+(#{VariableSegment}+))?/o - Syntax = SYNTAX - attr_reader :template_name_expr, :variable_name_expr, :attributes def initialize(tag_name, markup, options) @@ -104,29 +101,6 @@ def strict2_parse(markup) p.consume(:end_of_string) end - def strict_parse(markup) - lax_parse(markup) - end - - def lax_parse(markup) - if markup =~ SYNTAX - template_name = Regexp.last_match(1) - variable_name = Regexp.last_match(3) - - @alias_name = Regexp.last_match(5) - @variable_name_expr = variable_name ? parse_expression(variable_name) : nil - @template_name_expr = parse_expression(template_name) - @attributes = {} - - markup.scan(TagAttributes) do |key, value| - @attributes[key] = parse_expression(value) - end - - else - raise SyntaxError, options[:locale].t("errors.syntax.include") - end - end - class ParseTreeVisitor < Liquid::ParseTreeVisitor def children [ diff --git a/lib/liquid/tags/render.rb b/lib/liquid/tags/render.rb index 6e1559cc2..43d0bca82 100644 --- a/lib/liquid/tags/render.rb +++ b/lib/liquid/tags/render.rb @@ -27,7 +27,6 @@ module Liquid # @liquid_syntax_keyword filename The name of the snippet to render, without the `.liquid` extension. class Render < Tag FOR = 'for' - SYNTAX = /(#{QuotedString}+)(\s+(with|#{FOR})\s+(#{QuotedFragment}+))?(\s+(?:as)\s+(#{VariableSegment}+))?/o disable_tags "include" @@ -111,28 +110,6 @@ def strict2_template_name(p) p.consume(:string) end - def strict_parse(markup) - lax_parse(markup) - end - - def lax_parse(markup) - raise SyntaxError, options[:locale].t("errors.syntax.render") unless markup =~ SYNTAX - - template_name = Regexp.last_match(1) - with_or_for = Regexp.last_match(3) - variable_name = Regexp.last_match(4) - - @alias_name = Regexp.last_match(6) - @variable_name_expr = variable_name ? parse_expression(variable_name) : nil - @template_name_expr = parse_expression(template_name) - @is_for_loop = (with_or_for == FOR) - - @attributes = {} - markup.scan(TagAttributes) do |key, value| - @attributes[key] = parse_expression(value) - end - end - class ParseTreeVisitor < Liquid::ParseTreeVisitor def children [ diff --git a/lib/liquid/tags/table_row.rb b/lib/liquid/tags/table_row.rb index b69f91486..07c6da23a 100644 --- a/lib/liquid/tags/table_row.rb +++ b/lib/liquid/tags/table_row.rb @@ -24,7 +24,6 @@ module Liquid # @liquid_optional_param offset: [number] The 1-based index to start iterating at. # @liquid_optional_param range [untyped] A custom numeric range to iterate over. class TableRow < Block - Syntax = /(\w+)\s+in\s+(#{QuotedFragment}+)/o ALLOWED_ATTRIBUTES = ['cols', 'limit', 'offset', 'range'].freeze attr_reader :variable_name, :collection_name, :attributes @@ -62,23 +61,6 @@ def strict2_parse(markup) p.consume(:end_of_string) end - def strict_parse(markup) - lax_parse(markup) - end - - def lax_parse(markup) - if markup =~ Syntax - @variable_name = Regexp.last_match(1) - @collection_name = parse_expression(Regexp.last_match(2)) - @attributes = {} - markup.scan(TagAttributes) do |key, value| - @attributes[key] = parse_expression(value) - end - else - raise SyntaxError, options[:locale].t("errors.syntax.table_row") - end - end - def render_to_output_buffer(context, output) (collection = context.evaluate(@collection_name)) || (return '') diff --git a/lib/liquid/template.rb b/lib/liquid/template.rb index b007765ce..6bb03dfc3 100644 --- a/lib/liquid/template.rb +++ b/lib/liquid/template.rb @@ -22,9 +22,6 @@ class Template class << self # Sets how strict the parser should be. - # :lax acts like liquid 2.5 and silently ignores malformed tags in most cases. - # :warn is the default and will give deprecation warnings when invalid syntax is used. - # :strict enforces correct syntax for most tags # :strict2 enforces correct syntax for all tags def error_mode=(mode) Deprecations.warn("Template.error_mode=", "Environment#error_mode=") diff --git a/lib/liquid/variable.rb b/lib/liquid/variable.rb index 6b5fb412b..0a0ad21ec 100644 --- a/lib/liquid/variable.rb +++ b/lib/liquid/variable.rb @@ -30,7 +30,7 @@ def initialize(markup, parse_context) @parse_context = parse_context @line_number = parse_context.line_number - strict_parse_with_error_mode_fallback(markup) + parse_with_selected_parser(markup) end def raw @@ -41,39 +41,6 @@ def markup_context(markup) "in \"{{#{markup}}}\"" end - def lax_parse(markup) - @filters = [] - return unless markup =~ MarkupWithQuotedFragment - - name_markup = Regexp.last_match(1) - filter_markup = Regexp.last_match(2) - @name = parse_context.parse_expression(name_markup) - if filter_markup =~ FilterMarkupRegex - filters = Regexp.last_match(1).scan(FilterParser) - filters.each do |f| - next unless f =~ /\w+/ - filtername = Regexp.last_match(0) - filterargs = f.scan(FilterArgsRegex).flatten - @filters << lax_parse_filter_expressions(filtername, filterargs) - end - end - end - - def strict_parse(markup) - @filters = [] - p = @parse_context.new_parser(markup) - - return if p.look(:end_of_string) - - @name = parse_context.safe_parse_expression(p) - while p.consume?(:pipe) - filtername = p.consume(:id) - filterargs = p.consume?(:colon) ? parse_filterargs(p) : Const::EMPTY_ARRAY - @filters << lax_parse_filter_expressions(filtername, filterargs) - end - p.consume(:end_of_string) - end - def strict2_parse(markup) @filters = [] p = @parse_context.new_parser(markup) @@ -85,14 +52,6 @@ def strict2_parse(markup) p.consume(:end_of_string) end - def parse_filterargs(p) - # first argument - filterargs = [p.argument] - # followed by comma separated others - filterargs << p.argument while p.consume?(:comma) - filterargs - end - def render(context) obj = context.evaluate(@name) @@ -133,22 +92,6 @@ def disabled_tags private - def lax_parse_filter_expressions(filter_name, unparsed_args) - filter_args = [] - keyword_args = nil - unparsed_args.each do |a| - if (matches = a.match(JustTagAttributes)) - keyword_args ||= {} - keyword_args[matches[1]] = parse_context.parse_expression(matches[2]) - else - filter_args << parse_context.parse_expression(a) - end - end - result = [filter_name, filter_args] - result << keyword_args if keyword_args - result - end - # Surprisingly, positional and keyword arguments can be mixed. # # filter = filtername [":" filterargs?] diff --git a/test/integration/assign_test.rb b/test/integration/assign_test.rb index fdb6c99ca..a88941ae2 100644 --- a/test/integration/assign_test.rb +++ b/test/integration/assign_test.rb @@ -41,11 +41,10 @@ def test_assign_syntax_error def test_assign_uses_error_mode assert_match_syntax_error( - "Expected dotdot but found pipe in ", + "Expected dotdot but found pipe", "{% assign foo = ('X' | downcase) %}", - error_mode: :strict, + error_mode: :rigid, ) - assert_template_result("", "{% assign foo = ('X' | downcase) %}", error_mode: :lax) end def test_expression_with_whitespace_in_square_brackets diff --git a/test/integration/context_test.rb b/test/integration/context_test.rb index d230734f6..db68da45f 100644 --- a/test/integration/context_test.rb +++ b/test/integration/context_test.rb @@ -632,7 +632,7 @@ def notice(output) end def test_has_key_will_not_add_an_error_for_missing_keys - with_error_modes(:strict) do + with_error_modes(:rigid) do context = Context.new context.key?('unknown') assert_empty(context.errors) diff --git a/test/integration/error_handling_test.rb b/test/integration/error_handling_test.rb index 0fda83ca5..9f07b1f87 100644 --- a/test/integration/error_handling_test.rb +++ b/test/integration/error_handling_test.rb @@ -67,20 +67,13 @@ def test_missing_endtag_parse_time_error end def test_unrecognized_operator - with_error_modes(:strict) do + with_error_modes(:rigid) do assert_raises(SyntaxError) do Liquid::Template.parse(' {% if 1 =! 2 %}ok{% endif %} ') end end end - def test_lax_unrecognized_operator - template = Liquid::Template.parse(' {% if 1 =! 2 %}ok{% endif %} ', error_mode: :lax) - assert_equal(' Liquid error: Unknown operator =! ', template.render) - assert_equal(1, template.errors.size) - assert_equal(Liquid::ArgumentError, template.errors.first.class) - end - def test_with_line_numbers_adds_numbers_to_parser_errors source = <<~LIQUID foobar @@ -104,25 +97,6 @@ def test_with_line_numbers_adds_numbers_to_parser_errors_with_whitespace_trim assert_match_syntax_error(/Liquid syntax error \(line 3\)/, source) end - def test_parsing_warn_with_line_numbers_adds_numbers_to_lexer_errors - template = Liquid::Template.parse( - ' - foobar - - {% if 1 =! 2 %}ok{% endif %} - - bla - ', - error_mode: :warn, - line_numbers: true, - ) - - assert_equal( - ['Liquid syntax error (line 4): Unexpected character = in "1 =! 2"'], - template.warnings.map(&:message), - ) - end - def test_parsing_strict_with_line_numbers_adds_numbers_to_lexer_errors err = assert_raises(SyntaxError) do Liquid::Template.parse( @@ -133,7 +107,7 @@ def test_parsing_strict_with_line_numbers_adds_numbers_to_lexer_errors bla ', - error_mode: :strict, + error_mode: :rigid, line_numbers: true, ) end @@ -157,34 +131,16 @@ def test_syntax_errors_in_nested_blocks_have_correct_line_number def test_strict_error_messages err = assert_raises(SyntaxError) do - Liquid::Template.parse(' {% if 1 =! 2 %}ok{% endif %} ', error_mode: :strict) + Liquid::Template.parse(' {% if 1 =! 2 %}ok{% endif %} ', error_mode: :rigid) end assert_equal('Liquid syntax error: Unexpected character = in "1 =! 2"', err.message) err = assert_raises(SyntaxError) do - Liquid::Template.parse('{{%%%}}', error_mode: :strict) + Liquid::Template.parse('{{%%%}}', error_mode: :rigid) end assert_equal('Liquid syntax error: Unexpected character % in "{{%%%}}"', err.message) end - def test_warnings - template = Liquid::Template.parse('{% if ~~~ %}{{%%%}}{% else %}{{ hello. }}{% endif %}', error_mode: :warn) - assert_equal(3, template.warnings.size) - assert_equal('Unexpected character ~ in "~~~"', template.warnings[0].to_s(false)) - assert_equal('Unexpected character % in "{{%%%}}"', template.warnings[1].to_s(false)) - assert_equal('Expected id but found end_of_string in "{{ hello. }}"', template.warnings[2].to_s(false)) - assert_equal('', template.render) - end - - def test_warning_line_numbers - template = Liquid::Template.parse("{% if ~~~ %}\n{{%%%}}{% else %}\n{{ hello. }}{% endif %}", error_mode: :warn, line_numbers: true) - assert_equal('Liquid syntax error (line 1): Unexpected character ~ in "~~~"', template.warnings[0].message) - assert_equal('Liquid syntax error (line 2): Unexpected character % in "{{%%%}}"', template.warnings[1].message) - assert_equal('Liquid syntax error (line 3): Expected id but found end_of_string in "{{ hello. }}"', template.warnings[2].message) - assert_equal(3, template.warnings.size) - assert_equal([1, 2, 3], template.warnings.map(&:line_number)) - end - # Liquid should not catch Exceptions that are not subclasses of StandardError, like Interrupt and NoMemoryError def test_exceptions_propagate assert_raises(Exception) do diff --git a/test/integration/expression_test.rb b/test/integration/expression_test.rb index ae84fa36d..988f1d122 100644 --- a/test/integration/expression_test.rb +++ b/test/integration/expression_test.rb @@ -27,11 +27,6 @@ def test_float assert_template_result("-17.42", "{{ -17.42 }}") assert_template_result("2.5", "{{ 2.5 }}") - with_error_modes(:lax) do - assert_expression_result(0.0, "0.....5") - assert_expression_result(0.0, "-0..1") - end - assert_expression_result(1.5, "1.5") # this is a unfortunate quirky behavior of Liquid @@ -56,19 +51,6 @@ def test_range ) end - def test_quirky_negative_sign_expression_markup - result = Expression.parse("-", nil) - assert(result.is_a?(VariableLookup)) - assert_equal("-", result.name) - - # for this template, the expression markup is "-" - assert_template_result( - "", - "{{ - 'theme.css' - }}", - error_mode: :lax, - ) - end - def test_expression_cache skip("Liquid-C does not support Expression caching") if defined?(Liquid::C) && Liquid::C.enabled diff --git a/test/integration/parsing_quirks_test.rb b/test/integration/parsing_quirks_test.rb index b82ce86c5..523512098 100644 --- a/test/integration/parsing_quirks_test.rb +++ b/test/integration/parsing_quirks_test.rb @@ -31,18 +31,14 @@ def test_raise_on_label_and_no_close_bracets_percent def test_error_on_empty_filter assert(Template.parse("{{test}}")) - with_error_modes(:lax) do - assert(Template.parse("{{|test}}")) - end - - with_error_modes(:strict) do - assert_raises(SyntaxError) { Template.parse("{{|test}}") } - assert_raises(SyntaxError) { Template.parse("{{test |a|b|}}") } + with_error_modes(:rigid) do + assert_raises(Liquid::SyntaxError) { Template.parse("{{|test}}") } + assert_raises(Liquid::SyntaxError) { Template.parse("{{test |a|b|}}") } end end def test_meaningless_parens_error - with_error_modes(:strict) do + with_error_modes(:rigid) do assert_raises(SyntaxError) do markup = "a == 'foo' or (b == 'bar' and c == 'baz') or false" Template.parse("{% if #{markup} %} YES {% endif %}") @@ -51,7 +47,7 @@ def test_meaningless_parens_error end def test_unexpected_characters_syntax_error - with_error_modes(:strict) do + with_error_modes(:rigid) do assert_raises(SyntaxError) do markup = "true && false" Template.parse("{% if #{markup} %} YES {% endif %}") @@ -63,61 +59,12 @@ def test_unexpected_characters_syntax_error end end - def test_no_error_on_lax_empty_filter - assert(Template.parse("{{test |a|b|}}", error_mode: :lax)) - assert(Template.parse("{{test}}", error_mode: :lax)) - assert(Template.parse("{{|test|}}", error_mode: :lax)) - end - - def test_meaningless_parens_lax - with_error_modes(:lax) do - assigns = { 'b' => 'bar', 'c' => 'baz' } - markup = "a == 'foo' or (b == 'bar' and c == 'baz') or false" - assert_template_result(' YES ', "{% if #{markup} %} YES {% endif %}", assigns) - end - end - - def test_unexpected_characters_silently_eat_logic_lax - with_error_modes(:lax) do - markup = "true && false" - assert_template_result(' YES ', "{% if #{markup} %} YES {% endif %}") - markup = "false || true" - assert_template_result('', "{% if #{markup} %} YES {% endif %}") - end - end - def test_raise_on_invalid_tag_delimiter assert_raises(Liquid::SyntaxError) do Template.new.parse('{% end %}') end end - def test_unanchored_filter_arguments - with_error_modes(:lax) do - assert_template_result('hi', "{{ 'hi there' | split$$$:' ' | first }}") - - assert_template_result('x', "{{ 'X' | downcase) }}") - - # After the messed up quotes a filter without parameters (reverse) should work - # but one with parameters (remove) shouldn't be detected. - assert_template_result('here', "{{ 'hi there' | split:\"t\"\" | reverse | first}}") - assert_template_result('hi ', "{{ 'hi there' | split:\"t\"\" | remove:\"i\" | first}}") - end - end - - def test_invalid_variables_work - with_error_modes(:lax) do - assert_template_result('bar', "{% assign 123foo = 'bar' %}{{ 123foo }}") - assert_template_result('123', "{% assign 123 = 'bar' %}{{ 123 }}") - end - end - - def test_extra_dots_in_ranges - with_error_modes(:lax) do - assert_template_result('12345', "{% for i in (1...5) %}{{ i }}{% endfor %}") - end - end - def test_blank_variable_markup assert_template_result('', "{{}}") end @@ -131,24 +78,4 @@ def test_lookup_on_var_with_literal_name def test_contains_in_id assert_template_result(' YES ', '{% if containsallshipments == true %} YES {% endif %}', { 'containsallshipments' => true }) end - - def test_incomplete_expression - with_error_modes(:lax) do - assert_template_result("false", "{{ false - }}") - assert_template_result("false", "{{ false > }}") - assert_template_result("false", "{{ false < }}") - assert_template_result("false", "{{ false = }}") - assert_template_result("false", "{{ false ! }}") - assert_template_result("false", "{{ false 1 }}") - assert_template_result("false", "{{ false a }}") - - assert_template_result("false", "{% liquid assign foo = false -\n%}{{ foo }}") - assert_template_result("false", "{% liquid assign foo = false >\n%}{{ foo }}") - assert_template_result("false", "{% liquid assign foo = false <\n%}{{ foo }}") - assert_template_result("false", "{% liquid assign foo = false =\n%}{{ foo }}") - assert_template_result("false", "{% liquid assign foo = false !\n%}{{ foo }}") - assert_template_result("false", "{% liquid assign foo = false 1\n%}{{ foo }}") - assert_template_result("false", "{% liquid assign foo = false a\n%}{{ foo }}") - end - end end # ParsingQuirksTest diff --git a/test/integration/tags/cycle_tag_test.rb b/test/integration/tags/cycle_tag_test.rb index dfb5984d8..cf3fdea18 100644 --- a/test/integration/tags/cycle_tag_test.rb +++ b/test/integration/tags/cycle_tag_test.rb @@ -96,11 +96,6 @@ def test_cycle_tag_with_error_mode template1 = "{% assign 5 = 'b' %}{% cycle .5, .4 %}" template2 = "{% cycle .5: 'a', 'b' %}" - with_error_modes(:lax, :strict) do - assert_template_result("b", template1) - assert_template_result("a", template2) - end - with_error_modes(:strict2) do error1 = assert_raises(Liquid::SyntaxError) { Template.parse(template1) } error2 = assert_raises(Liquid::SyntaxError) { Template.parse(template2) } @@ -121,14 +116,6 @@ def test_cycle_with_trailing_elements template4 = "#{assignments}{% cycle n e: 'a', 'b', 'c' %}" template5 = "#{assignments}{% cycle n e 'a', 'b', 'c' %}" - with_error_modes(:lax, :strict) do - assert_template_result("a", template1) - assert_template_result("a", template2) - assert_template_result("a", template3) - assert_template_result("N", template4) - assert_template_result("N", template5) - end - with_error_modes(:strict2) do error1 = assert_raises(Liquid::SyntaxError) { Template.parse(template1) } error2 = assert_raises(Liquid::SyntaxError) { Template.parse(template2) } @@ -153,10 +140,6 @@ def test_cycle_name_with_invalid_expression {% endfor %} LIQUID - with_error_modes(:lax, :strict) do - refute_nil(Template.parse(template)) - end - with_error_modes(:strict2) do error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } assert_match(/Unexpected character =/, error.message) @@ -170,10 +153,6 @@ def test_cycle_variable_with_invalid_expression {% endfor %} LIQUID - with_error_modes(:lax, :strict) do - refute_nil(Template.parse(template)) - end - with_error_modes(:strict2) do error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } assert_match(/Unexpected character =/, error.message) diff --git a/test/integration/tags/include_tag_test.rb b/test/integration/tags/include_tag_test.rb index 44c0dcda7..e35addcd9 100644 --- a/test/integration/tags/include_tag_test.rb +++ b/test/integration/tags/include_tag_test.rb @@ -205,14 +205,6 @@ def test_dynamically_choosen_template end def test_strict2_parsing_errors - with_error_modes(:lax, :strict) do - assert_template_result( - 'hello value1 value2', - '{% include "snippet" !!! arg1: "value1" ~~~ arg2: "value2" %}', - partials: { 'snippet' => 'hello {{ arg1 }} {{ arg2 }}' }, - ) - end - with_error_modes(:strict2) do assert_syntax_error( '{% include "snippet" !!! arg1: "value1" ~~~ arg2: "value2" %}', @@ -301,16 +293,10 @@ def test_passing_options_to_included_templates env = Liquid::Environment.build(file_system: TestFileSystem.new) assert_raises(Liquid::SyntaxError) do - Template.parse("{% include template %}", error_mode: :strict, environment: env).render!("template" => '{{ "X" || downcase }}') - end - with_error_modes(:lax) do - assert_equal('x', Template.parse("{% include template %}", error_mode: :strict, include_options_blacklist: true, environment: env).render!("template" => '{{ "X" || downcase }}')) + Template.parse("{% include template %}", error_mode: :rigid, environment: env).render!("template" => '{{ "X" || downcase }}') end assert_raises(Liquid::SyntaxError) do - Template.parse("{% include template %}", error_mode: :strict, include_options_blacklist: [:locale], environment: env).render!("template" => '{{ "X" || downcase }}') - end - with_error_modes(:lax) do - assert_equal('x', Template.parse("{% include template %}", error_mode: :strict, include_options_blacklist: [:error_mode], environment: env).render!("template" => '{{ "X" || downcase }}')) + Template.parse("{% include template %}", error_mode: :rigid, include_options_blacklist: [:locale], environment: env).render!("template" => '{{ "X" || downcase }}') end end @@ -404,10 +390,6 @@ def test_render_tag_renders_error_with_template_name_from_template_factory def test_include_template_with_invalid_expression template = "{% include foo=>bar %}" - with_error_modes(:lax, :strict) do - refute_nil(Template.parse(template)) - end - with_error_modes(:strict2) do error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } assert_match(/Unexpected character =/, error.message) @@ -417,10 +399,6 @@ def test_include_template_with_invalid_expression def test_include_with_invalid_expression template = '{% include "snippet" with foo=>bar %}' - with_error_modes(:lax, :strict) do - refute_nil(Template.parse(template)) - end - with_error_modes(:strict2) do error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } assert_match(/Unexpected character =/, error.message) @@ -430,10 +408,6 @@ def test_include_with_invalid_expression def test_include_attribute_with_invalid_expression template = '{% include "snippet", key: foo=>bar %}' - with_error_modes(:lax, :strict) do - refute_nil(Template.parse(template)) - end - with_error_modes(:strict2) do error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } assert_match(/Unexpected character =/, error.message) diff --git a/test/integration/tags/render_tag_test.rb b/test/integration/tags/render_tag_test.rb index 0bd09d088..440342c33 100644 --- a/test/integration/tags/render_tag_test.rb +++ b/test/integration/tags/render_tag_test.rb @@ -106,14 +106,6 @@ def test_dynamically_choosen_templates_are_not_allowed end def test_strict2_parsing_errors - with_error_modes(:lax, :strict) do - assert_template_result( - 'hello value1 value2', - '{% render "snippet" !!! arg1: "value1" ~~~ arg2: "value2" %}', - partials: { 'snippet' => 'hello {{ arg1 }} {{ arg2 }}' }, - ) - end - with_error_modes(:strict2) do assert_syntax_error( '{% render "snippet" !!! arg1: "value1" ~~~ arg2: "value2" %}', @@ -318,10 +310,6 @@ def test_render_tag_renders_error_with_template_name_from_template_factory def test_render_with_invalid_expression template = '{% render "snippet" with foo=>bar %}' - with_error_modes(:lax, :strict) do - refute_nil(Template.parse(template)) - end - with_error_modes(:strict2) do error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } assert_match(/Unexpected character =/, error.message) @@ -331,10 +319,6 @@ def test_render_with_invalid_expression def test_render_attribute_with_invalid_expression template = '{% render "snippet", key: foo=>bar %}' - with_error_modes(:lax, :strict) do - refute_nil(Template.parse(template)) - end - with_error_modes(:strict2) do error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } assert_match(/Unexpected character =/, error.message) diff --git a/test/integration/tags/table_row_test.rb b/test/integration/tags/table_row_test.rb index 45628ce9d..e8f7adb4f 100644 --- a/test/integration/tags/table_row_test.rb +++ b/test/integration/tags/table_row_test.rb @@ -188,29 +188,6 @@ def test_tablerow_loop_drop_attributes assert_template_result(expected_output, template) end - def test_table_row_renders_correct_error_message_for_invalid_parameters - assert_template_result( - "Liquid error (line 1): invalid integer", - '{% tablerow n in (1...10) limit:true %} {{n}} {% endtablerow %}', - error_mode: :warn, - render_errors: true, - ) - - assert_template_result( - "Liquid error (line 1): invalid integer", - '{% tablerow n in (1...10) offset:true %} {{n}} {% endtablerow %}', - error_mode: :warn, - render_errors: true, - ) - - assert_template_result( - "Liquid error (line 1): invalid integer", - '{% tablerow n in (1...10) cols:true %} {{n}} {% endtablerow %}', - render_errors: true, - error_mode: :warn, - ) - end - def test_table_row_handles_interrupts assert_template_result( "\n 1 \n", @@ -439,10 +416,6 @@ def test_tablerow_with_invalid_attribute_strict_vs_strict2 12345 OUTPUT - with_error_modes(:lax, :strict) do - assert_template_result(expected, template) - end - with_error_modes(:strict2) do error = assert_raises(SyntaxError) { Template.parse(template) } assert_match(/Invalid attribute 'invalid_attr'/, error.message) @@ -452,14 +425,6 @@ def test_tablerow_with_invalid_attribute_strict_vs_strict2 def test_tablerow_with_invalid_expression_strict_vs_strict2 template = '{% tablerow i in (1..5) limit: foo=>bar %}{{ i }}{% endtablerow %}' - with_error_modes(:lax, :strict) do - expected = <<~OUTPUT - - - OUTPUT - assert_template_result(expected, template) - end - with_error_modes(:strict2) do error = assert_raises(SyntaxError) { Template.parse(template) } assert_match(/Unexpected character =/, error.message) diff --git a/test/integration/template_test.rb b/test/integration/template_test.rb index 92ece017c..2a90d34f7 100644 --- a/test/integration/template_test.rb +++ b/test/integration/template_test.rb @@ -259,7 +259,7 @@ def test_undefined_variables end def test_nil_value_does_not_raise - t = Template.parse("some{{x}}thing", error_mode: :strict) + t = Template.parse("some{{x}}thing", error_mode: :rigid) result = t.render!({ 'x' => nil }, strict_variables: true) assert_equal(0, t.errors.count) diff --git a/test/integration/variable_test.rb b/test/integration/variable_test.rb index e272b96b9..82e7ac3e2 100644 --- a/test/integration/variable_test.rb +++ b/test/integration/variable_test.rb @@ -176,48 +176,9 @@ def test_double_nested_variable_lookup ) end - def test_variable_lookup_should_not_hang_with_invalid_syntax - Timeout.timeout(1) do - assert_template_result( - 'bar', - "{{['foo'}}", - { - 'foo' => 'bar', - }, - error_mode: :lax, - ) - end - - very_long_key = "1234567890" * 100 - - template_list = [ - "{{['#{very_long_key}']}}", # valid - "{{['#{very_long_key}'}}", # missing closing bracket - "{{[['#{very_long_key}']}}", # extra open bracket - ] - - template_list.each do |template| - Timeout.timeout(1) do - assert_template_result( - 'bar', - template, - { - very_long_key => 'bar', - }, - error_mode: :lax, - ) - end - end - end - def test_filter_with_single_trailing_comma template = '{{ "hello" | append: "world", }}' - with_error_modes(:strict) do - error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } - assert_match(/is not a valid expression/, error.message) - end - with_error_modes(:strict2) do assert_template_result('helloworld', template) end @@ -226,11 +187,6 @@ def test_filter_with_single_trailing_comma def test_multiple_filters_with_trailing_commas template = '{{ "hello" | append: "1", | append: "2", }}' - with_error_modes(:strict) do - error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } - assert_match(/is not a valid expression/, error.message) - end - with_error_modes(:strict2) do assert_template_result('hello12', template) end @@ -239,11 +195,6 @@ def test_multiple_filters_with_trailing_commas def test_filter_with_colon_but_no_arguments template = '{{ "test" | upcase: }}' - with_error_modes(:strict) do - error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } - assert_match(/is not a valid expression/, error.message) - end - with_error_modes(:strict2) do assert_template_result('TEST', template) end @@ -252,11 +203,6 @@ def test_filter_with_colon_but_no_arguments def test_filter_chain_with_colon_no_args template = '{{ "test" | append: "x" | upcase: }}' - with_error_modes(:strict) do - error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } - assert_match(/is not a valid expression/, error.message) - end - with_error_modes(:strict2) do assert_template_result('TESTX', template) end @@ -265,11 +211,6 @@ def test_filter_chain_with_colon_no_args def test_combining_trailing_comma_and_empty_args template = '{{ "test" | append: "x", | upcase: }}' - with_error_modes(:strict) do - error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } - assert_match(/is not a valid expression/, error.message) - end - with_error_modes(:strict2) do assert_template_result('TESTX', template) end diff --git a/test/test_helper.rb b/test/test_helper.rb index 293ea4766..70ba9b694 100755 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -8,7 +8,7 @@ require 'liquid.rb' require 'liquid/profiler' -mode = :strict +mode = :rigid if (env_mode = ENV['LIQUID_PARSER_MODE']) puts "-- #{env_mode.upcase} ERROR MODE" mode = env_mode.to_sym diff --git a/test/unit/condition_unit_test.rb b/test/unit/condition_unit_test.rb index eb466f7a9..b77eed9c4 100644 --- a/test/unit/condition_unit_test.rb +++ b/test/unit/condition_unit_test.rb @@ -167,9 +167,9 @@ def test_default_context_is_deprecated end def test_parse_expression_in_strict_mode - environment = Environment.build(error_mode: :strict) + environment = Environment.build(error_mode: :rigid) parse_context = ParseContext.new(environment: environment) - result = Condition.parse_expression(parse_context, 'product.title') + result = Condition.parse_expression(parse_context, 'product.title', safe: true) assert_instance_of(VariableLookup, result) assert_equal('product', result.name) diff --git a/test/unit/parse_context_unit_test.rb b/test/unit/parse_context_unit_test.rb index f53cd1148..d1b32efcd 100644 --- a/test/unit/parse_context_unit_test.rb +++ b/test/unit/parse_context_unit_test.rb @@ -38,12 +38,6 @@ def test_safe_parse_expression_raises_syntax_error_for_invalid_expression end def test_parse_expression_with_variable_lookup - result_strict = strict_parse_context.parse_expression('product.title') - - assert_instance_of(VariableLookup, result_strict) - assert_equal('product', result_strict.name) - assert_equal(['title'], result_strict.lookups) - error = assert_raises(Liquid::InternalError) do strict2_parse_context.parse_expression('product.title') end @@ -66,9 +60,6 @@ def test_parse_expression_with_safe_true end def test_parse_expression_with_empty_string - result_strict = strict_parse_context.parse_expression('') - assert_nil(result_strict) - error = assert_raises(Liquid::InternalError) do strict2_parse_context.parse_expression('') end @@ -77,9 +68,6 @@ def test_parse_expression_with_empty_string end def test_parse_expression_with_empty_string_and_safe_true - result_strict = strict_parse_context.parse_expression('', safe: true) - assert_nil(result_strict) - result_strict2 = strict2_parse_context.parse_expression('', safe: true) assert_nil(result_strict2) end @@ -109,12 +97,6 @@ def test_parse_expression_with_whitespace_in_strict2_mode private - def strict_parse_context - @strict_parse_context ||= ParseContext.new( - environment: Environment.build(error_mode: :strict), - ) - end - def strict2_parse_context @strict2_parse_context ||= ParseContext.new( environment: Environment.build(error_mode: :strict2), diff --git a/test/unit/partial_cache_unit_test.rb b/test/unit/partial_cache_unit_test.rb index 623241b40..72555d52b 100644 --- a/test/unit/partial_cache_unit_test.rb +++ b/test/unit/partial_cache_unit_test.rb @@ -184,7 +184,7 @@ def test_includes_error_mode_into_template_cache }, ) - [:lax, :warn, :strict, :strict2].each do |error_mode| + [:strict2].each do |error_mode| Liquid::PartialCache.load( 'my_partial', context: context, @@ -193,7 +193,7 @@ def test_includes_error_mode_into_template_cache end assert_equal( - ["my_partial:lax", "my_partial:warn", "my_partial:strict", "my_partial:strict2"], + ["my_partial:strict2"], context.registers[:cached_partials].keys, ) end diff --git a/test/unit/tags/case_tag_unit_test.rb b/test/unit/tags/case_tag_unit_test.rb index 9d7d34a39..183071c49 100644 --- a/test/unit/tags/case_tag_unit_test.rb +++ b/test/unit/tags/case_tag_unit_test.rb @@ -20,10 +20,6 @@ def test_case_with_trailing_element {%- endcase -%} LIQUID - with_error_modes(:lax, :strict) do - assert_template_result("one", template) - end - with_error_modes(:strict2) do error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } @@ -41,10 +37,6 @@ def test_case_when_with_trailing_element {%- endcase -%} LIQUID - with_error_modes(:lax, :strict) do - assert_template_result("one", template) - end - with_error_modes(:strict2) do error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } @@ -62,7 +54,7 @@ def test_case_when_with_comma {%- endcase -%} LIQUID - with_error_modes(:lax, :strict, :strict2) do + with_error_modes(:strict2) do assert_template_result("one", template) end end @@ -77,7 +69,7 @@ def test_case_when_with_or {%- endcase -%} LIQUID - with_error_modes(:lax, :strict, :strict2) do + with_error_modes(:strict2) do assert_template_result("one", template) end end @@ -93,10 +85,6 @@ def test_case_with_invalid_expression LIQUID assigns = { 'foo' => { 'bar' => 'baz' } } - with_error_modes(:lax, :strict) do - assert_template_result("one", template, assigns) - end - with_error_modes(:strict2) do error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } @@ -115,10 +103,6 @@ def test_case_when_with_invalid_expression LIQUID assigns = { 'foo' => { 'bar' => 'baz' } } - with_error_modes(:lax, :strict) do - assert_template_result("one", template, assigns) - end - with_error_modes(:strict2) do error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } diff --git a/test/unit/variable_unit_test.rb b/test/unit/variable_unit_test.rb index 03f5862aa..590aa3f38 100644 --- a/test/unit/variable_unit_test.rb +++ b/test/unit/variable_unit_test.rb @@ -72,12 +72,6 @@ def test_filters_without_whitespace assert_equal([['replace', ['foo', 'bar']], ['textileze', []]], var.filters) end - def test_symbol - var = create_variable("http://disney.com/logo.gif | image: 'med' ", error_mode: :lax) - assert_equal(VariableLookup.new('http://disney.com/logo.gif'), var.name) - assert_equal([['image', ['med']]], var.filters) - end - def test_string_to_filter var = create_variable("'http://disney.com/logo.gif' | image: 'med' ") assert_equal('http://disney.com/logo.gif', var.name) @@ -108,7 +102,7 @@ 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) - with_error_modes(:strict) do + with_error_modes(:strict2) do assert_raises(Liquid::SyntaxError) { create_variable('foo - bar') } assert_raises(Liquid::SyntaxError) { create_variable('-foo') } assert_raises(Liquid::SyntaxError) { create_variable('2foo') } @@ -131,36 +125,6 @@ def test_filter_with_keyword_arguments assert_equal([['things', [], { 'greeting' => 'world', 'farewell' => 'goodbye' }]], var.filters) end - def test_lax_filter_argument_parsing - var = create_variable(%( number_of_comments | pluralize: 'comment': 'comments' ), error_mode: :lax) - assert_equal(VariableLookup.new('number_of_comments'), var.name) - assert_equal([['pluralize', ['comment', 'comments']]], var.filters) - - # missing does not throws error - create_variable(%(n | f1: ,), error_mode: :lax) - create_variable(%(n | f1: ,| f2), error_mode: :lax) - - # arg does not require colon, but ignores args :O, also ignores first kwarg since it splits on ':' - var = create_variable(%(n | f1 1 | f2 k1: v1), error_mode: :lax) - assert_equal([['f1', []], ['f2', [VariableLookup.new('v1')]]], var.filters) - - # positional and kwargs parsing - var = create_variable(%(n | filter: 1, 2, 3 | filter2: k1: 1, k2: 2), error_mode: :lax) - assert_equal([['filter', [1, 2, 3]], ['filter2', [], { "k1" => 1, "k2" => 2 }]], var.filters) - - # positional and kwargs intermixed (pos1, key1: val1, pos2) - var = create_variable(%(n | link_to: class: "black", "https://example.com", title: "title"), error_mode: :lax) - assert_equal([['link_to', ["https://example.com"], { "class" => "black", "title" => "title" }]], var.filters) - end - - def test_strict_filter_argument_parsing - with_error_modes(:strict) do - assert_raises(SyntaxError) do - create_variable(%( number_of_comments | pluralize: 'comment': 'comments' )) - end - end - end - def test_strict2_filter_argument_parsing with_error_modes(:strict2) do # optional colon From db61632e0f88c8fc967f10002d6a87dc6eb9a8f0 Mon Sep 17 00:00:00 2001 From: "Charles-P. Clermont" Date: Thu, 6 Nov 2025 15:14:05 -0500 Subject: [PATCH 2/5] Rename strict2_parse to parse_markup --- lib/liquid/parser_switching.rb | 10 +++------- lib/liquid/tags/case.rb | 10 +++------- lib/liquid/tags/cycle.rb | 2 +- lib/liquid/tags/for.rb | 2 +- lib/liquid/tags/if.rb | 2 +- lib/liquid/tags/include.rb | 2 +- lib/liquid/tags/render.rb | 6 +++--- lib/liquid/tags/table_row.rb | 2 +- lib/liquid/variable.rb | 6 +++--- 9 files changed, 17 insertions(+), 25 deletions(-) diff --git a/lib/liquid/parser_switching.rb b/lib/liquid/parser_switching.rb index 4b35e2139..ca4267668 100644 --- a/lib/liquid/parser_switching.rb +++ b/lib/liquid/parser_switching.rb @@ -3,19 +3,15 @@ module Liquid module ParserSwitching def parse_with_selected_parser(markup) - strict2_parse_with_error_context(markup) - end - - private - - def strict2_parse_with_error_context(markup) - strict2_parse(markup) + parse_markup(markup) rescue SyntaxError => e e.line_number = line_number e.markup_context = markup_context(markup) raise e end + private + def markup_context(markup) "in \"#{markup.strip}\"" end diff --git a/lib/liquid/tags/case.rb b/lib/liquid/tags/case.rb index e24cf5a1b..0a466c44c 100644 --- a/lib/liquid/tags/case.rb +++ b/lib/liquid/tags/case.rb @@ -83,7 +83,7 @@ def render_to_output_buffer(context, output) private - def strict2_parse(markup) + def parse_markup(markup) parser = @parse_context.new_parser(markup) @left = safe_parse_expression(parser) parser.consume(:end_of_string) @@ -92,14 +92,10 @@ def strict2_parse(markup) def record_when_condition(markup) body = new_body - if strict2_mode? - parse_strict2_when(markup, body) - else - parse_lax_when(markup, body) - end + parse_when(markup, body) end - def parse_strict2_when(markup, body) + def parse_when(markup, body) parser = @parse_context.new_parser(markup) loop do diff --git a/lib/liquid/tags/cycle.rb b/lib/liquid/tags/cycle.rb index 2d372cee5..a98d25392 100644 --- a/lib/liquid/tags/cycle.rb +++ b/lib/liquid/tags/cycle.rb @@ -54,7 +54,7 @@ def render_to_output_buffer(context, output) private # cycle [name:] expression(, expression)* - def strict2_parse(markup) + def parse_markup(markup) p = @parse_context.new_parser(markup) @variables = [] diff --git a/lib/liquid/tags/for.rb b/lib/liquid/tags/for.rb index 4b89b7972..cac9d2393 100644 --- a/lib/liquid/tags/for.rb +++ b/lib/liquid/tags/for.rb @@ -70,7 +70,7 @@ def render_to_output_buffer(context, output) protected - def strict2_parse(markup) + def parse_markup(markup) p = @parse_context.new_parser(markup) @variable_name = p.consume(:id) raise SyntaxError, options[:locale].t("errors.syntax.for_invalid_in") unless p.id?('in') diff --git a/lib/liquid/tags/if.rb b/lib/liquid/tags/if.rb index 287fe78ae..06c752da7 100644 --- a/lib/liquid/tags/if.rb +++ b/lib/liquid/tags/if.rb @@ -77,7 +77,7 @@ def parse_expression(markup, safe: false) Condition.parse_expression(parse_context, markup, safe: safe) end - def strict2_parse(markup) + def parse_markup(markup) p = @parse_context.new_parser(markup) condition = parse_binary_comparisons(p) p.consume(:end_of_string) diff --git a/lib/liquid/tags/include.rb b/lib/liquid/tags/include.rb index 49bc032f6..b50c68a66 100644 --- a/lib/liquid/tags/include.rb +++ b/lib/liquid/tags/include.rb @@ -81,7 +81,7 @@ def render_to_output_buffer(context, output) alias_method :parse_context, :options private :parse_context - def strict2_parse(markup) + def parse_markup(markup) p = @parse_context.new_parser(markup) @template_name_expr = safe_parse_expression(p) diff --git a/lib/liquid/tags/render.rb b/lib/liquid/tags/render.rb index 43d0bca82..5c071c803 100644 --- a/lib/liquid/tags/render.rb +++ b/lib/liquid/tags/render.rb @@ -84,10 +84,10 @@ def render_tag(context, output) end # render (string) (with|for expression)? (as id)? (key: value)* - def strict2_parse(markup) + def parse_markup(markup) p = @parse_context.new_parser(markup) - @template_name_expr = parse_expression(strict2_template_name(p), safe: true) + @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 @alias_name = p.consume(:id) if p.id?("as") @@ -106,7 +106,7 @@ def strict2_parse(markup) p.consume(:end_of_string) end - def strict2_template_name(p) + def template_name(p) p.consume(:string) end diff --git a/lib/liquid/tags/table_row.rb b/lib/liquid/tags/table_row.rb index 07c6da23a..c91147541 100644 --- a/lib/liquid/tags/table_row.rb +++ b/lib/liquid/tags/table_row.rb @@ -33,7 +33,7 @@ def initialize(tag_name, markup, options) parse_with_selected_parser(markup) end - def strict2_parse(markup) + def parse_markup(markup) p = @parse_context.new_parser(markup) @variable_name = p.consume(:id) diff --git a/lib/liquid/variable.rb b/lib/liquid/variable.rb index 0a0ad21ec..fcc723515 100644 --- a/lib/liquid/variable.rb +++ b/lib/liquid/variable.rb @@ -41,14 +41,14 @@ def markup_context(markup) "in \"{{#{markup}}}\"" end - def strict2_parse(markup) + def parse_markup(markup) @filters = [] p = @parse_context.new_parser(markup) return if p.look(:end_of_string) @name = parse_context.safe_parse_expression(p) - @filters << strict2_parse_filter_expressions(p) while p.consume?(:pipe) + @filters << parse_filter_expressions(p) while p.consume?(:pipe) p.consume(:end_of_string) end @@ -99,7 +99,7 @@ def disabled_tags # argument = (positional_argument | keyword_argument) # positional_argument = expression # keyword_argument = id ":" expression - def strict2_parse_filter_expressions(p) + def parse_filter_expressions(p) filtername = p.consume(:id) filter_args = [] keyword_args = {} From f0f63d44c026288cf941bf6c95ff0270c13c64d7 Mon Sep 17 00:00:00 2001 From: "Charles-P. Clermont" Date: Tue, 2 Dec 2025 09:20:03 -0500 Subject: [PATCH 3/5] Remove :error_mode --- Rakefile | 18 +---- lib/liquid/environment.rb | 10 +-- lib/liquid/parse_context.rb | 20 ++--- lib/liquid/partial_cache.rb | 2 +- lib/liquid/template.rb | 11 --- performance/benchmark.rb | 1 - performance/memory_profile.rb | 2 - performance/profile.rb | 1 - test/integration/assign_test.rb | 3 +- test/integration/context_test.rb | 8 +- test/integration/error_handling_test.rb | 11 +-- test/integration/parsing_quirks_test.rb | 30 +++---- test/integration/tags/cycle_tag_test.rb | 52 +++++------- test/integration/tags/include_tag_test.rb | 40 ++++----- test/integration/tags/render_tag_test.rb | 30 +++---- test/integration/tags/table_row_test.rb | 99 ++++++++--------------- test/integration/template_test.rb | 2 +- test/integration/variable_test.rb | 20 ++--- test/test_helper.rb | 29 ++----- test/unit/condition_unit_test.rb | 20 ++--- test/unit/parse_context_unit_test.rb | 73 +++++++---------- test/unit/partial_cache_unit_test.rb | 16 ++-- test/unit/tags/case_tag_unit_test.rb | 32 +++----- test/unit/variable_unit_test.rb | 58 +++++++------ 24 files changed, 202 insertions(+), 386 deletions(-) diff --git a/Rakefile b/Rakefile index b8939316b..086a5a81e 100755 --- a/Rakefile +++ b/Rakefile @@ -33,14 +33,12 @@ task :rubocop do end end -desc('runs test suite with strict2 parser') +desc('runs test suite') task :test do - ENV['LIQUID_PARSER_MODE'] = 'strict2' Rake::Task['base_test'].reenable Rake::Task['base_test'].invoke if RUBY_ENGINE == 'ruby' || RUBY_ENGINE == 'truffleruby' - ENV['LIQUID_PARSER_MODE'] = 'strict2' Rake::Task['integration_test'].reenable Rake::Task['integration_test'].invoke end @@ -63,13 +61,10 @@ task release: :build do end namespace :benchmark do - desc "Run the liquid benchmark with strict2 parsing" - task :strict2 do - ruby "./performance/benchmark.rb strict2" - end - desc "Run the liquid benchmark" - task run: [:strict2] + task :run do + ruby "./performance/benchmark.rb" + end desc "Run unit benchmarks" namespace :unit do @@ -101,11 +96,6 @@ namespace :profile do task :run do ruby "./performance/profile.rb" end - - desc "Run the liquid profile/performance coverage with strict2 parsing" - task :strict2 do - ruby "./performance/profile.rb strict2" - end end namespace :memory_profile do diff --git a/lib/liquid/environment.rb b/lib/liquid/environment.rb index 095b2b091..7834f49c3 100644 --- a/lib/liquid/environment.rb +++ b/lib/liquid/environment.rb @@ -4,10 +4,6 @@ module Liquid # The Environment is the container for all configuration options of Liquid, such as # the registered tags, filters, and the default error mode. class Environment - # The default error mode for all templates. This can be overridden on a - # per-template basis. - attr_accessor :error_mode - # The tags that are available to use in the template. attr_accessor :tags @@ -33,17 +29,14 @@ class << self # the template. # @param file_system The default file system that is used # to load templates from. - # @param error_mode [Symbol] The default error mode for all templates - # (:strict2). # @param exception_renderer [Proc] The exception renderer that is used to # render exceptions. # @yieldparam environment [Environment] The environment instance that is being built. # @return [Environment] The new environment instance. - def build(tags: nil, file_system: nil, error_mode: nil, exception_renderer: nil) + def build(tags: nil, file_system: nil, exception_renderer: nil) ret = new ret.tags = tags if tags ret.file_system = file_system if file_system - ret.error_mode = error_mode if error_mode ret.exception_renderer = exception_renderer if exception_renderer yield ret if block_given? ret.freeze @@ -75,7 +68,6 @@ def dangerously_override(environment) # @api private def initialize @tags = Tags::STANDARD_TAGS.dup - @error_mode = :strict2 @strainer_template = Class.new(StrainerTemplate).tap do |klass| klass.add_filter(StandardFilters) end diff --git a/lib/liquid/parse_context.rb b/lib/liquid/parse_context.rb index 855acc64e..8161a0369 100644 --- a/lib/liquid/parse_context.rb +++ b/lib/liquid/parse_context.rb @@ -3,7 +3,7 @@ module Liquid class ParseContext attr_accessor :locale, :line_number, :trim_whitespace, :depth - attr_reader :partial, :warnings, :error_mode, :environment + attr_reader :partial, :warnings, :environment def initialize(options = Const::EMPTY_HASH) @environment = options.fetch(:environment, Environment.default) @@ -55,16 +55,12 @@ def safe_parse_expression(parser) end def parse_expression(markup, safe: false) - if !safe && @error_mode == :strict2 - # parse_expression is a widely used API. To maintain backward - # compatibility while raising awareness about strict2 parser standards, - # the safe flag supports API users make a deliberate decision. - # - # In strict2 mode, 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. - raise Liquid::InternalError, "unsafe parse_expression cannot be used in strict2 mode" - end + # 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 @@ -72,8 +68,6 @@ def parse_expression(markup, safe: false) def partial=(value) @partial = value @options = value ? partial_options : @template_options - - @error_mode = @options[:error_mode] || @environment.error_mode end def partial_options diff --git a/lib/liquid/partial_cache.rb b/lib/liquid/partial_cache.rb index f49d14d90..5cca4e04b 100644 --- a/lib/liquid/partial_cache.rb +++ b/lib/liquid/partial_cache.rb @@ -4,7 +4,7 @@ module Liquid class PartialCache def self.load(template_name, context:, parse_context:) cached_partials = context.registers[:cached_partials] - cache_key = "#{template_name}:#{parse_context.error_mode}" + cache_key = template_name.to_s cached = cached_partials[cache_key] return cached if cached diff --git a/lib/liquid/template.rb b/lib/liquid/template.rb index 6bb03dfc3..e638a01d7 100644 --- a/lib/liquid/template.rb +++ b/lib/liquid/template.rb @@ -21,17 +21,6 @@ class Template attr_reader :profiler class << self - # Sets how strict the parser should be. - # :strict2 enforces correct syntax for all tags - def error_mode=(mode) - Deprecations.warn("Template.error_mode=", "Environment#error_mode=") - Environment.default.error_mode = mode - end - - def error_mode - Environment.default.error_mode - end - def default_exception_renderer=(renderer) Deprecations.warn("Template.default_exception_renderer=", "Environment#exception_renderer=") Environment.default.exception_renderer = renderer diff --git a/performance/benchmark.rb b/performance/benchmark.rb index b61e9057c..c8aa6769d 100644 --- a/performance/benchmark.rb +++ b/performance/benchmark.rb @@ -4,7 +4,6 @@ require_relative 'theme_runner' RubyVM::YJIT.enable if defined?(RubyVM::YJIT) -Liquid::Environment.default.error_mode = ARGV.first.to_sym if ARGV.first profiler = ThemeRunner.new diff --git a/performance/memory_profile.rb b/performance/memory_profile.rb index e2934297a..fb7312d55 100644 --- a/performance/memory_profile.rb +++ b/performance/memory_profile.rb @@ -53,8 +53,6 @@ def sanitize(string) end end -Liquid::Template.error_mode = ARGV.first.to_sym if ARGV.first - runner = ThemeRunner.new Profiler.run do |x| x.profile('parse') { runner.compile } diff --git a/performance/profile.rb b/performance/profile.rb index 70740778d..f756fb202 100644 --- a/performance/profile.rb +++ b/performance/profile.rb @@ -3,7 +3,6 @@ require 'stackprof' require_relative 'theme_runner' -Liquid::Template.error_mode = ARGV.first.to_sym if ARGV.first profiler = ThemeRunner.new profiler.run diff --git a/test/integration/assign_test.rb b/test/integration/assign_test.rb index a88941ae2..69163ab96 100644 --- a/test/integration/assign_test.rb +++ b/test/integration/assign_test.rb @@ -39,11 +39,10 @@ def test_assign_syntax_error assert_match_syntax_error(/assign/, '{% assign foo not values %}.') end - def test_assign_uses_error_mode + def test_assign_throws_on_unsupported_syntax assert_match_syntax_error( "Expected dotdot but found pipe", "{% assign foo = ('X' | downcase) %}", - error_mode: :rigid, ) end diff --git a/test/integration/context_test.rb b/test/integration/context_test.rb index db68da45f..e1952e634 100644 --- a/test/integration/context_test.rb +++ b/test/integration/context_test.rb @@ -632,11 +632,9 @@ def notice(output) end def test_has_key_will_not_add_an_error_for_missing_keys - with_error_modes(:rigid) do - context = Context.new - context.key?('unknown') - assert_empty(context.errors) - end + context = Context.new + context.key?('unknown') + assert_empty(context.errors) end def test_key_lookup_will_raise_for_missing_keys_when_strict_variables_is_enabled diff --git a/test/integration/error_handling_test.rb b/test/integration/error_handling_test.rb index 9f07b1f87..9de179f2d 100644 --- a/test/integration/error_handling_test.rb +++ b/test/integration/error_handling_test.rb @@ -67,10 +67,8 @@ def test_missing_endtag_parse_time_error end def test_unrecognized_operator - with_error_modes(:rigid) do - assert_raises(SyntaxError) do - Liquid::Template.parse(' {% if 1 =! 2 %}ok{% endif %} ') - end + assert_raises(SyntaxError) do + Liquid::Template.parse(' {% if 1 =! 2 %}ok{% endif %} ') end end @@ -107,7 +105,6 @@ def test_parsing_strict_with_line_numbers_adds_numbers_to_lexer_errors bla ', - error_mode: :rigid, line_numbers: true, ) end @@ -131,12 +128,12 @@ def test_syntax_errors_in_nested_blocks_have_correct_line_number def test_strict_error_messages err = assert_raises(SyntaxError) do - Liquid::Template.parse(' {% if 1 =! 2 %}ok{% endif %} ', error_mode: :rigid) + Liquid::Template.parse(' {% if 1 =! 2 %}ok{% endif %} ') end assert_equal('Liquid syntax error: Unexpected character = in "1 =! 2"', err.message) err = assert_raises(SyntaxError) do - Liquid::Template.parse('{{%%%}}', error_mode: :rigid) + Liquid::Template.parse('{{%%%}}') end assert_equal('Liquid syntax error: Unexpected character % in "{{%%%}}"', err.message) end diff --git a/test/integration/parsing_quirks_test.rb b/test/integration/parsing_quirks_test.rb index 523512098..75954f93f 100644 --- a/test/integration/parsing_quirks_test.rb +++ b/test/integration/parsing_quirks_test.rb @@ -31,31 +31,25 @@ def test_raise_on_label_and_no_close_bracets_percent def test_error_on_empty_filter assert(Template.parse("{{test}}")) - with_error_modes(:rigid) do - assert_raises(Liquid::SyntaxError) { Template.parse("{{|test}}") } - assert_raises(Liquid::SyntaxError) { Template.parse("{{test |a|b|}}") } - end + assert_raises(Liquid::SyntaxError) { Template.parse("{{|test}}") } + assert_raises(Liquid::SyntaxError) { Template.parse("{{test |a|b|}}") } end def test_meaningless_parens_error - with_error_modes(:rigid) do - assert_raises(SyntaxError) do - markup = "a == 'foo' or (b == 'bar' and c == 'baz') or false" - Template.parse("{% if #{markup} %} YES {% endif %}") - end + assert_raises(SyntaxError) do + markup = "a == 'foo' or (b == 'bar' and c == 'baz') or false" + Template.parse("{% if #{markup} %} YES {% endif %}") end end def test_unexpected_characters_syntax_error - with_error_modes(:rigid) do - assert_raises(SyntaxError) do - markup = "true && false" - Template.parse("{% if #{markup} %} YES {% endif %}") - end - assert_raises(SyntaxError) do - markup = "false || true" - Template.parse("{% if #{markup} %} YES {% endif %}") - end + assert_raises(SyntaxError) do + markup = "true && false" + Template.parse("{% if #{markup} %} YES {% endif %}") + end + assert_raises(SyntaxError) do + markup = "false || true" + Template.parse("{% if #{markup} %} YES {% endif %}") end end diff --git a/test/integration/tags/cycle_tag_test.rb b/test/integration/tags/cycle_tag_test.rb index cf3fdea18..9edd19958 100644 --- a/test/integration/tags/cycle_tag_test.rb +++ b/test/integration/tags/cycle_tag_test.rb @@ -91,23 +91,21 @@ def test_cycle_tag_without_arguments assert_match(/Syntax Error in 'cycle' - Valid syntax: cycle \[name :\] var/, error.message) end - def test_cycle_tag_with_error_mode + def test_cycle_tag_unsupported_legacy_quirk # QuotedFragment is more permissive than what Parser#expression allows. template1 = "{% assign 5 = 'b' %}{% cycle .5, .4 %}" template2 = "{% cycle .5: 'a', 'b' %}" - with_error_modes(:strict2) do - error1 = assert_raises(Liquid::SyntaxError) { Template.parse(template1) } - error2 = assert_raises(Liquid::SyntaxError) { Template.parse(template2) } + error1 = assert_raises(Liquid::SyntaxError) { Template.parse(template1) } + error2 = assert_raises(Liquid::SyntaxError) { Template.parse(template2) } - expected_error = /Liquid syntax error: \[:dot, "."\] is not a valid expression/ + expected_error = /Liquid syntax error: \[:dot, "."\] is not a valid expression/ - assert_match(expected_error, error1.message) - assert_match(expected_error, error2.message) - end + assert_match(expected_error, error1.message) + assert_match(expected_error, error2.message) end - def test_cycle_with_trailing_elements + def test_cycle_with_trailing_elements_legacy_syntax assignments = "{% assign a = 'A' %}{% assign n = 'N' %}" template1 = "#{assignments}{% cycle 'a' 'b', 'c' %}" @@ -116,21 +114,19 @@ def test_cycle_with_trailing_elements template4 = "#{assignments}{% cycle n e: 'a', 'b', 'c' %}" template5 = "#{assignments}{% cycle n e 'a', 'b', 'c' %}" - with_error_modes(:strict2) do - error1 = assert_raises(Liquid::SyntaxError) { Template.parse(template1) } - error2 = assert_raises(Liquid::SyntaxError) { Template.parse(template2) } - error3 = assert_raises(Liquid::SyntaxError) { Template.parse(template3) } - error4 = assert_raises(Liquid::SyntaxError) { Template.parse(template4) } - error5 = assert_raises(Liquid::SyntaxError) { Template.parse(template5) } + error1 = assert_raises(Liquid::SyntaxError) { Template.parse(template1) } + error2 = assert_raises(Liquid::SyntaxError) { Template.parse(template2) } + error3 = assert_raises(Liquid::SyntaxError) { Template.parse(template3) } + error4 = assert_raises(Liquid::SyntaxError) { Template.parse(template4) } + error5 = assert_raises(Liquid::SyntaxError) { Template.parse(template5) } - expected_error = /Expected end_of_string but found/ + expected_error = /Expected end_of_string but found/ - assert_match(expected_error, error1.message) - assert_match(expected_error, error2.message) - assert_match(expected_error, error3.message) - assert_match(expected_error, error4.message) - assert_match(expected_error, error5.message) - end + assert_match(expected_error, error1.message) + assert_match(expected_error, error2.message) + assert_match(expected_error, error3.message) + assert_match(expected_error, error4.message) + assert_match(expected_error, error5.message) end def test_cycle_name_with_invalid_expression @@ -140,10 +136,8 @@ def test_cycle_name_with_invalid_expression {% endfor %} LIQUID - with_error_modes(:strict2) do - error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } - assert_match(/Unexpected character =/, error.message) - end + error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } + assert_match(/Unexpected character =/, error.message) end def test_cycle_variable_with_invalid_expression @@ -153,9 +147,7 @@ def test_cycle_variable_with_invalid_expression {% endfor %} LIQUID - with_error_modes(:strict2) do - error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } - assert_match(/Unexpected character =/, error.message) - end + error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } + assert_match(/Unexpected character =/, error.message) end end diff --git a/test/integration/tags/include_tag_test.rb b/test/integration/tags/include_tag_test.rb index e35addcd9..b911ace16 100644 --- a/test/integration/tags/include_tag_test.rb +++ b/test/integration/tags/include_tag_test.rb @@ -204,15 +204,13 @@ def test_dynamically_choosen_template ) end - def test_strict2_parsing_errors - with_error_modes(:strict2) do - assert_syntax_error( - '{% include "snippet" !!! arg1: "value1" ~~~ arg2: "value2" %}', - ) - assert_syntax_error( - '{% include "snippet" | filter %}', - ) - end + def test_parsing_errors_for_legacy_quirk + assert_syntax_error( + '{% include "snippet" !!! arg1: "value1" ~~~ arg2: "value2" %}', + ) + assert_syntax_error( + '{% include "snippet" | filter %}', + ) end def test_optional_commas @@ -293,10 +291,10 @@ def test_passing_options_to_included_templates env = Liquid::Environment.build(file_system: TestFileSystem.new) assert_raises(Liquid::SyntaxError) do - Template.parse("{% include template %}", error_mode: :rigid, environment: env).render!("template" => '{{ "X" || downcase }}') + Template.parse("{% include template %}", environment: env).render!("template" => '{{ "X" || downcase }}') end assert_raises(Liquid::SyntaxError) do - Template.parse("{% include template %}", error_mode: :rigid, include_options_blacklist: [:locale], environment: env).render!("template" => '{{ "X" || downcase }}') + Template.parse("{% include template %}", include_options_blacklist: [:locale], environment: env).render!("template" => '{{ "X" || downcase }}') end end @@ -351,7 +349,7 @@ def test_including_with_strict_variables file_system: StubFileSystem.new('simple' => 'simple'), ) - template = Liquid::Template.parse("{% include 'simple' %}", error_mode: :warn, environment: env) + template = Liquid::Template.parse("{% include 'simple' %}", environment: env) template.render(nil, strict_variables: true) assert_equal([], template.errors) @@ -390,27 +388,21 @@ def test_render_tag_renders_error_with_template_name_from_template_factory def test_include_template_with_invalid_expression template = "{% include foo=>bar %}" - with_error_modes(:strict2) do - error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } - assert_match(/Unexpected character =/, error.message) - end + error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } + assert_match(/Unexpected character =/, error.message) end def test_include_with_invalid_expression template = '{% include "snippet" with foo=>bar %}' - with_error_modes(:strict2) do - error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } - assert_match(/Unexpected character =/, error.message) - end + error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } + assert_match(/Unexpected character =/, error.message) end def test_include_attribute_with_invalid_expression template = '{% include "snippet", key: foo=>bar %}' - with_error_modes(:strict2) do - error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } - assert_match(/Unexpected character =/, error.message) - end + error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } + assert_match(/Unexpected character =/, error.message) end end # IncludeTagTest diff --git a/test/integration/tags/render_tag_test.rb b/test/integration/tags/render_tag_test.rb index 440342c33..2c69ba547 100644 --- a/test/integration/tags/render_tag_test.rb +++ b/test/integration/tags/render_tag_test.rb @@ -105,15 +105,13 @@ def test_dynamically_choosen_templates_are_not_allowed assert_syntax_error("{% assign name = 'snippet' %}{% render name %}") end - def test_strict2_parsing_errors - with_error_modes(:strict2) do - assert_syntax_error( - '{% render "snippet" !!! arg1: "value1" ~~~ arg2: "value2" %}', - ) - assert_syntax_error( - '{% render "snippet" | filter %}', - ) - end + def test_parsing_errors_legacy_syntax + assert_syntax_error( + '{% render "snippet" !!! arg1: "value1" ~~~ arg2: "value2" %}', + ) + assert_syntax_error( + '{% render "snippet" | filter %}', + ) end def test_optional_commas @@ -309,19 +307,13 @@ def test_render_tag_renders_error_with_template_name_from_template_factory def test_render_with_invalid_expression template = '{% render "snippet" with foo=>bar %}' - - with_error_modes(:strict2) do - error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } - assert_match(/Unexpected character =/, error.message) - end + error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } + assert_match(/Unexpected character =/, error.message) end def test_render_attribute_with_invalid_expression template = '{% render "snippet", key: foo=>bar %}' - - with_error_modes(:strict2) do - error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } - assert_match(/Unexpected character =/, error.message) - end + error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } + assert_match(/Unexpected character =/, error.message) end end diff --git a/test/integration/tags/table_row_test.rb b/test/integration/tags/table_row_test.rb index e8f7adb4f..59b4cc25e 100644 --- a/test/integration/tags/table_row_test.rb +++ b/test/integration/tags/table_row_test.rb @@ -236,7 +236,7 @@ def test_table_row_does_not_leak_interrupts ) end - def test_tablerow_with_cols_attribute_in_strict2_mode + def test_tablerow_with_cols_attribute template = <<~LIQUID.chomp {% tablerow i in (1..6) cols: 3 %}{{ i }}{% endtablerow %} LIQUID @@ -247,12 +247,10 @@ def test_tablerow_with_cols_attribute_in_strict2_mode 456 OUTPUT - with_error_modes(:strict2) do - assert_template_result(expected, template) - end + assert_template_result(expected, template) end - def test_tablerow_with_limit_attribute_in_strict2_mode + def test_tablerow_with_limit_attribute template = <<~LIQUID.chomp {% tablerow i in (1..10) limit: 3 %}{{ i }}{% endtablerow %} LIQUID @@ -262,12 +260,10 @@ def test_tablerow_with_limit_attribute_in_strict2_mode 123 OUTPUT - with_error_modes(:strict2) do - assert_template_result(expected, template) - end + assert_template_result(expected, template) end - def test_tablerow_with_offset_attribute_in_strict2_mode + def test_tablerow_with_offset_attribute template = <<~LIQUID.chomp {% tablerow i in (1..5) offset: 2 %}{{ i }}{% endtablerow %} LIQUID @@ -277,12 +273,10 @@ def test_tablerow_with_offset_attribute_in_strict2_mode 345 OUTPUT - with_error_modes(:strict2) do - assert_template_result(expected, template) - end + assert_template_result(expected, template) end - def test_tablerow_with_range_attribute_in_strict2_mode + def test_tablerow_with_range_attribute template = <<~LIQUID.chomp {% tablerow i in (1..3) range: (1..10) %}{{ i }}{% endtablerow %} LIQUID @@ -292,12 +286,10 @@ def test_tablerow_with_range_attribute_in_strict2_mode 123 OUTPUT - with_error_modes(:strict2) do - assert_template_result(expected, template) - end + assert_template_result(expected, template) end - def test_tablerow_with_multiple_attributes_in_strict2_mode + def test_tablerow_with_multiple_attributes template = <<~LIQUID.chomp {% tablerow i in (1..10) cols: 2, limit: 4, offset: 1 %}{{ i }}{% endtablerow %} LIQUID @@ -308,12 +300,10 @@ def test_tablerow_with_multiple_attributes_in_strict2_mode 45 OUTPUT - with_error_modes(:strict2) do - assert_template_result(expected, template) - end + assert_template_result(expected, template) end - def test_tablerow_with_variable_collection_in_strict2_mode + def test_tablerow_with_variable_collection template = <<~LIQUID.chomp {% tablerow n in numbers cols: 2 %}{{ n }}{% endtablerow %} LIQUID @@ -324,12 +314,10 @@ def test_tablerow_with_variable_collection_in_strict2_mode 34 OUTPUT - with_error_modes(:strict2) do - assert_template_result(expected, template, { 'numbers' => [1, 2, 3, 4] }) - end + assert_template_result(expected, template, { 'numbers' => [1, 2, 3, 4] }) end - def test_tablerow_with_dotted_access_in_strict2_mode + def test_tablerow_with_dotted_access template = <<~LIQUID.chomp {% tablerow n in obj.numbers cols: 2 %}{{ n }}{% endtablerow %} LIQUID @@ -340,12 +328,10 @@ def test_tablerow_with_dotted_access_in_strict2_mode 34 OUTPUT - with_error_modes(:strict2) do - assert_template_result(expected, template, { 'obj' => { 'numbers' => [1, 2, 3, 4] } }) - end + assert_template_result(expected, template, { 'obj' => { 'numbers' => [1, 2, 3, 4] } }) end - def test_tablerow_with_bracketed_access_in_strict2_mode + def test_tablerow_with_bracketed_access template = <<~LIQUID.chomp {% tablerow n in obj["numbers"] cols: 2 %}{{ n }}{% endtablerow %} LIQUID @@ -355,12 +341,10 @@ def test_tablerow_with_bracketed_access_in_strict2_mode 1020 OUTPUT - with_error_modes(:strict2) do - assert_template_result(expected, template, { 'obj' => { 'numbers' => [10, 20] } }) - end + assert_template_result(expected, template, { 'obj' => { 'numbers' => [10, 20] } }) end - def test_tablerow_without_attributes_in_strict2_mode + def test_tablerow_without_attributes template = <<~LIQUID.chomp {% tablerow i in (1..3) %}{{ i }}{% endtablerow %} LIQUID @@ -370,30 +354,24 @@ def test_tablerow_without_attributes_in_strict2_mode 123 OUTPUT - with_error_modes(:strict2) do - assert_template_result(expected, template) - end + assert_template_result(expected, template) end - def test_tablerow_without_in_keyword_in_strict2_mode + def test_tablerow_without_in_keyword template = '{% tablerow i (1..10) %}{{ i }}{% endtablerow %}' - with_error_modes(:strict2) do - error = assert_raises(SyntaxError) { Template.parse(template) } - assert_equal("Liquid syntax error: For loops require an 'in' clause in \"i (1..10)\"", error.message) - end + error = assert_raises(SyntaxError) { Template.parse(template) } + assert_equal("Liquid syntax error: For loops require an 'in' clause in \"i (1..10)\"", error.message) end - def test_tablerow_with_multiple_invalid_attributes_reports_first_in_strict2_mode + def test_tablerow_with_multiple_invalid_attributes_reports_first template = '{% tablerow i in (1..10) invalid1: 5, invalid2: 10 %}{{ i }}{% endtablerow %}' - with_error_modes(:strict2) do - error = assert_raises(SyntaxError) { Template.parse(template) } - assert_equal("Liquid syntax error: Invalid attribute 'invalid1' in tablerow loop. Valid attributes are cols, limit, offset, and range in \"i in (1..10) invalid1: 5, invalid2: 10\"", error.message) - end + error = assert_raises(SyntaxError) { Template.parse(template) } + assert_equal("Liquid syntax error: Invalid attribute 'invalid1' in tablerow loop. Valid attributes are cols, limit, offset, and range in \"i in (1..10) invalid1: 5, invalid2: 10\"", error.message) end - def test_tablerow_with_empty_collection_in_strict2_mode + def test_tablerow_with_empty_collection template = <<~LIQUID.chomp {% tablerow i in empty_array cols: 2 %}{{ i }}{% endtablerow %} LIQUID @@ -403,31 +381,18 @@ def test_tablerow_with_empty_collection_in_strict2_mode OUTPUT - with_error_modes(:strict2) do - assert_template_result(expected, template, { 'empty_array' => [] }) - end + assert_template_result(expected, template, { 'empty_array' => [] }) end - def test_tablerow_with_invalid_attribute_strict_vs_strict2 + def test_tablerow_with_invalid_attribute template = '{% tablerow i in (1..5) invalid_attr: 10 %}{{ i }}{% endtablerow %}' - - expected = <<~OUTPUT - - 12345 - OUTPUT - - with_error_modes(:strict2) do - error = assert_raises(SyntaxError) { Template.parse(template) } - assert_match(/Invalid attribute 'invalid_attr'/, error.message) - end + error = assert_raises(SyntaxError) { Template.parse(template) } + assert_match(/Invalid attribute 'invalid_attr'/, error.message) end - def test_tablerow_with_invalid_expression_strict_vs_strict2 + def test_tablerow_with_invalid_expression template = '{% tablerow i in (1..5) limit: foo=>bar %}{{ i }}{% endtablerow %}' - - with_error_modes(:strict2) do - error = assert_raises(SyntaxError) { Template.parse(template) } - assert_match(/Unexpected character =/, error.message) - end + error = assert_raises(SyntaxError) { Template.parse(template) } + assert_match(/Unexpected character =/, error.message) end end diff --git a/test/integration/template_test.rb b/test/integration/template_test.rb index 2a90d34f7..b09f2584a 100644 --- a/test/integration/template_test.rb +++ b/test/integration/template_test.rb @@ -259,7 +259,7 @@ def test_undefined_variables end def test_nil_value_does_not_raise - t = Template.parse("some{{x}}thing", error_mode: :rigid) + t = Template.parse("some{{x}}thing") result = t.render!({ 'x' => nil }, strict_variables: true) assert_equal(0, t.errors.count) diff --git a/test/integration/variable_test.rb b/test/integration/variable_test.rb index 82e7ac3e2..38096e41d 100644 --- a/test/integration/variable_test.rb +++ b/test/integration/variable_test.rb @@ -179,40 +179,30 @@ def test_double_nested_variable_lookup def test_filter_with_single_trailing_comma template = '{{ "hello" | append: "world", }}' - with_error_modes(:strict2) do - assert_template_result('helloworld', template) - end + assert_template_result('helloworld', template) end def test_multiple_filters_with_trailing_commas template = '{{ "hello" | append: "1", | append: "2", }}' - with_error_modes(:strict2) do - assert_template_result('hello12', template) - end + assert_template_result('hello12', template) end def test_filter_with_colon_but_no_arguments template = '{{ "test" | upcase: }}' - with_error_modes(:strict2) do - assert_template_result('TEST', template) - end + assert_template_result('TEST', template) end def test_filter_chain_with_colon_no_args template = '{{ "test" | append: "x" | upcase: }}' - with_error_modes(:strict2) do - assert_template_result('TESTX', template) - end + assert_template_result('TESTX', template) end def test_combining_trailing_comma_and_empty_args template = '{{ "test" | append: "x", | upcase: }}' - with_error_modes(:strict2) do - assert_template_result('TESTX', template) - end + assert_template_result('TESTX', template) end end diff --git a/test/test_helper.rb b/test/test_helper.rb index 70ba9b694..0c9596f4e 100755 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -8,13 +8,6 @@ require 'liquid.rb' require 'liquid/profiler' -mode = :rigid -if (env_mode = ENV['LIQUID_PARSER_MODE']) - puts "-- #{env_mode.upcase} ERROR MODE" - mode = env_mode.to_sym -end -Liquid::Environment.default.error_mode = mode - if Minitest.const_defined?('Test') # We're on Minitest 5+. Nothing to do here. else @@ -34,27 +27,27 @@ module Assertions def assert_template_result( expected, template, assigns = {}, - message: nil, partials: nil, error_mode: Liquid::Environment.default.error_mode, render_errors: false, + message: nil, partials: nil, render_errors: false, template_factory: nil ) file_system = StubFileSystem.new(partials || {}) environment = Liquid::Environment.build(file_system: file_system) - template = Liquid::Template.parse(template, line_numbers: true, error_mode: error_mode&.to_sym, environment: environment) + template = Liquid::Template.parse(template, line_numbers: true, environment: environment) registers = Liquid::Registers.new(file_system: file_system, template_factory: template_factory) context = Liquid::Context.build(static_environments: assigns, rethrow_errors: !render_errors, registers: registers, environment: environment) output = template.render(context) assert_equal(expected, output, message) end - def assert_match_syntax_error(match, template, error_mode: nil) + def assert_match_syntax_error(match, template) exception = assert_raises(Liquid::SyntaxError) do - Template.parse(template, line_numbers: true, error_mode: error_mode&.to_sym).render + Template.parse(template, line_numbers: true).render end assert_match(match, exception.message) end - def assert_syntax_error(template, error_mode: nil) - assert_match_syntax_error("", template, error_mode: error_mode) + def assert_syntax_error(template) + assert_match_syntax_error("", template) end def assert_usage_increment(name, times: 1) @@ -82,16 +75,6 @@ def with_global_filter(*globals, &blk) Environment.dangerously_override(environment, &blk) end - def with_error_modes(*modes) - old_mode = Liquid::Environment.default.error_mode - modes.each do |mode| - Liquid::Environment.default.error_mode = mode - yield - end - ensure - Liquid::Environment.default.error_mode = old_mode - end - def with_custom_tag(tag_name, tag_class, &block) environment = Liquid::Environment.default.dup environment.register_tag(tag_name, tag_class) diff --git a/test/unit/condition_unit_test.rb b/test/unit/condition_unit_test.rb index b77eed9c4..584951e21 100644 --- a/test/unit/condition_unit_test.rb +++ b/test/unit/condition_unit_test.rb @@ -166,8 +166,8 @@ def test_default_context_is_deprecated assert_includes(err.lines.map(&:strip), expected) end - def test_parse_expression_in_strict_mode - environment = Environment.build(error_mode: :rigid) + def test_parse_expression_with_safe_true + environment = Environment.build parse_context = ParseContext.new(environment: environment) result = Condition.parse_expression(parse_context, 'product.title', safe: true) @@ -176,25 +176,15 @@ def test_parse_expression_in_strict_mode assert_equal(['title'], result.lookups) end - def test_parse_expression_in_strict2_mode_raises_internal_error - environment = Environment.build(error_mode: :strict2) + def test_parse_expression_raises_internal_error_if_not_safe + environment = Environment.build parse_context = ParseContext.new(environment: environment) error = assert_raises(Liquid::InternalError) do Condition.parse_expression(parse_context, 'product.title') end - assert_match(/unsafe parse_expression cannot be used in strict2 mode/, error.message) - end - - def test_parse_expression_with_safe_true_in_strict2_mode - environment = Environment.build(error_mode: :strict2) - parse_context = ParseContext.new(environment: environment) - result = Condition.parse_expression(parse_context, 'product.title', safe: true) - - assert_instance_of(VariableLookup, result) - assert_equal('product', result.name) - assert_equal(['title'], result.lookups) + assert_match(/unsafe parse_expression cannot be used/, error.message) end private diff --git a/test/unit/parse_context_unit_test.rb b/test/unit/parse_context_unit_test.rb index d1b32efcd..f75a48045 100644 --- a/test/unit/parse_context_unit_test.rb +++ b/test/unit/parse_context_unit_test.rb @@ -6,100 +6,81 @@ class ParseContextUnitTest < Minitest::Test include Liquid def test_safe_parse_expression_with_variable_lookup - parser_strict = strict_parse_context.new_parser('product.title') - result_strict = strict_parse_context.safe_parse_expression(parser_strict) + parser = parse_context.new_parser('product.title') + result = parse_context.safe_parse_expression(parser) - parser_strict2 = strict2_parse_context.new_parser('product.title') - result_strict2 = strict2_parse_context.safe_parse_expression(parser_strict2) - - assert_instance_of(VariableLookup, result_strict) - assert_equal('product', result_strict.name) - assert_equal(['title'], result_strict.lookups) - - assert_instance_of(VariableLookup, result_strict2) - assert_equal('product', result_strict2.name) - assert_equal(['title'], result_strict2.lookups) + assert_instance_of(VariableLookup, result) + assert_equal('product', result.name) + assert_equal(['title'], result.lookups) end def test_safe_parse_expression_raises_syntax_error_for_invalid_expression - parser_strict = strict_parse_context.new_parser('') - parser_strict2 = strict2_parse_context.new_parser('') - - error_strict = assert_raises(Liquid::SyntaxError) do - strict_parse_context.safe_parse_expression(parser_strict) - end - assert_match(/is not a valid expression/, error_strict.message) + parser = parse_context.new_parser('') - error_strict2 = assert_raises(Liquid::SyntaxError) do - strict2_parse_context.safe_parse_expression(parser_strict2) + error = assert_raises(Liquid::SyntaxError) do + parse_context.safe_parse_expression(parser) end - assert_match(/is not a valid expression/, error_strict2.message) + assert_match(/is not a valid expression/, error.message) end def test_parse_expression_with_variable_lookup error = assert_raises(Liquid::InternalError) do - strict2_parse_context.parse_expression('product.title') + parse_context.parse_expression('product.title') end - assert_match(/unsafe parse_expression cannot be used in strict2 mode/, error.message) + assert_match(/unsafe parse_expression cannot be used/, error.message) end def test_parse_expression_with_safe_true - result_strict = strict_parse_context.parse_expression('product.title', safe: true) + result = parse_context.parse_expression('product.title', safe: true) - assert_instance_of(VariableLookup, result_strict) - assert_equal('product', result_strict.name) - assert_equal(['title'], result_strict.lookups) - - result_strict2 = strict2_parse_context.parse_expression('product.title', safe: true) - - assert_instance_of(VariableLookup, result_strict2) - assert_equal('product', result_strict2.name) - assert_equal(['title'], result_strict2.lookups) + 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 - strict2_parse_context.parse_expression('') + parse_context.parse_expression('') end - assert_match(/unsafe parse_expression cannot be used in strict2 mode/, error.message) + assert_match(/unsafe parse_expression cannot be used/, error.message) end def test_parse_expression_with_empty_string_and_safe_true - result_strict2 = strict2_parse_context.parse_expression('', safe: true) - assert_nil(result_strict2) + result = parse_context.parse_expression('', safe: true) + assert_nil(result) end def test_safe_parse_expression_advances_parser_pointer - parser = strict2_parse_context.new_parser('foo, bar') + parser = parse_context.new_parser('foo, bar') # safe_parse_expression consumes "foo" - first_result = strict2_parse_context.safe_parse_expression(parser) + first_result = parse_context.safe_parse_expression(parser) assert_instance_of(VariableLookup, first_result) assert_equal('foo', first_result.name) parser.consume(:comma) # safe_parse_expression consumes "bar" - second_result = strict2_parse_context.safe_parse_expression(parser) + second_result = parse_context.safe_parse_expression(parser) assert_instance_of(VariableLookup, second_result) assert_equal('bar', second_result.name) parser.consume(:end_of_string) end - def test_parse_expression_with_whitespace_in_strict2_mode - result = strict2_parse_context.parse_expression(' ', safe: true) + def test_parse_expression_with_whitespace + result = parse_context.parse_expression(' ', safe: true) assert_nil(result) end private - def strict2_parse_context - @strict2_parse_context ||= ParseContext.new( - environment: Environment.build(error_mode: :strict2), + def parse_context + @parse_context ||= ParseContext.new( + environment: Environment.build, ) end end diff --git a/test/unit/partial_cache_unit_test.rb b/test/unit/partial_cache_unit_test.rb index 72555d52b..cb8e2d354 100644 --- a/test/unit/partial_cache_unit_test.rb +++ b/test/unit/partial_cache_unit_test.rb @@ -175,7 +175,7 @@ def test_uses_template_name_from_template_factory assert_equal('some/path/my_partial', partial.name) end - def test_includes_error_mode_into_template_cache + def test_cache_key template_factory = StubTemplateFactory.new context = Liquid::Context.build( registers: { @@ -184,16 +184,14 @@ def test_includes_error_mode_into_template_cache }, ) - [:strict2].each do |error_mode| - Liquid::PartialCache.load( - 'my_partial', - context: context, - parse_context: Liquid::ParseContext.new(error_mode: error_mode), - ) - end + Liquid::PartialCache.load( + 'my_partial', + context: context, + parse_context: Liquid::ParseContext.new, + ) assert_equal( - ["my_partial:strict2"], + ["my_partial"], context.registers[:cached_partials].keys, ) end diff --git a/test/unit/tags/case_tag_unit_test.rb b/test/unit/tags/case_tag_unit_test.rb index 183071c49..0d70c6c0a 100644 --- a/test/unit/tags/case_tag_unit_test.rb +++ b/test/unit/tags/case_tag_unit_test.rb @@ -20,11 +20,9 @@ def test_case_with_trailing_element {%- endcase -%} LIQUID - with_error_modes(:strict2) do - error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } + error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } - assert_match(/Expected end_of_string but found/, error.message) - end + assert_match(/Expected end_of_string but found/, error.message) end def test_case_when_with_trailing_element @@ -37,11 +35,9 @@ def test_case_when_with_trailing_element {%- endcase -%} LIQUID - with_error_modes(:strict2) do - error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } + error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } - assert_match(/Expected end_of_string but found/, error.message) - end + assert_match(/Expected end_of_string but found/, error.message) end def test_case_when_with_comma @@ -54,9 +50,7 @@ def test_case_when_with_comma {%- endcase -%} LIQUID - with_error_modes(:strict2) do - assert_template_result("one", template) - end + assert_template_result("one", template) end def test_case_when_with_or @@ -69,9 +63,7 @@ def test_case_when_with_or {%- endcase -%} LIQUID - with_error_modes(:strict2) do - assert_template_result("one", template) - end + assert_template_result("one", template) end def test_case_with_invalid_expression @@ -85,11 +77,9 @@ def test_case_with_invalid_expression LIQUID assigns = { 'foo' => { 'bar' => 'baz' } } - with_error_modes(:strict2) do - error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } + error = assert_raises(Liquid::SyntaxError) { Template.parse(template, assigns) } - assert_match(/Unexpected character =/, error.message) - end + assert_match(/Unexpected character =/, error.message) end def test_case_when_with_invalid_expression @@ -103,10 +93,8 @@ def test_case_when_with_invalid_expression LIQUID assigns = { 'foo' => { 'bar' => 'baz' } } - with_error_modes(:strict2) do - error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } + error = assert_raises(Liquid::SyntaxError) { Template.parse(template, assigns) } - assert_match(/Unexpected character =/, error.message) - end + assert_match(/Unexpected character =/, error.message) end end diff --git a/test/unit/variable_unit_test.rb b/test/unit/variable_unit_test.rb index 590aa3f38..6379c6ece 100644 --- a/test/unit/variable_unit_test.rb +++ b/test/unit/variable_unit_test.rb @@ -102,11 +102,9 @@ 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) - with_error_modes(:strict2) do - assert_raises(Liquid::SyntaxError) { create_variable('foo - bar') } - assert_raises(Liquid::SyntaxError) { create_variable('-foo') } - assert_raises(Liquid::SyntaxError) { create_variable('2foo') } - end + assert_raises(Liquid::SyntaxError) { create_variable('foo - bar') } + assert_raises(Liquid::SyntaxError) { create_variable('-foo') } + assert_raises(Liquid::SyntaxError) { create_variable('2foo') } end def test_string_with_special_chars @@ -125,40 +123,38 @@ def test_filter_with_keyword_arguments assert_equal([['things', [], { 'greeting' => 'world', 'farewell' => 'goodbye' }]], var.filters) end - def test_strict2_filter_argument_parsing - with_error_modes(:strict2) do - # optional colon - var = create_variable(%(n | f1 | f2:)) - assert_equal([['f1', []], ['f2', []]], var.filters) + def test_filter_argument_parsing + # optional colon + var = create_variable(%(n | f1 | f2:)) + assert_equal([['f1', []], ['f2', []]], var.filters) - # missing argument throws error - assert_raises(SyntaxError) { create_variable(%(n | f1: ,)) } - assert_raises(SyntaxError) { create_variable(%(n | f1: ,| f2)) } + # missing argument throws error + assert_raises(SyntaxError) { create_variable(%(n | f1: ,)) } + assert_raises(SyntaxError) { create_variable(%(n | f1: ,| f2)) } - # arg requires colon - assert_raises(SyntaxError) { create_variable(%(n | f1 1)) } + # arg requires colon + assert_raises(SyntaxError) { create_variable(%(n | f1 1)) } - # trailing comma doesn't throw - create_variable(%(n | f1: 1, 2, 3, | f2:)) + # trailing comma doesn't throw + create_variable(%(n | f1: 1, 2, 3, | f2:)) - # missing comma throws error - assert_raises(SyntaxError) { create_variable(%(n | filter: 1 2, 3)) } + # missing comma throws error + assert_raises(SyntaxError) { create_variable(%(n | filter: 1 2, 3)) } - # positional and kwargs parsing - var = create_variable(%(n | filter: 1, 2, 3 | filter2: k1: 1, k2: 2)) - assert_equal([['filter', [1, 2, 3]], ['filter2', [], { "k1" => 1, "k2" => 2 }]], var.filters) + # positional and kwargs parsing + var = create_variable(%(n | filter: 1, 2, 3 | filter2: k1: 1, k2: 2)) + assert_equal([['filter', [1, 2, 3]], ['filter2', [], { "k1" => 1, "k2" => 2 }]], var.filters) - # positional and kwargs mixed - var = create_variable(%(n | filter: 'a', 'b', key1: 1, key2: 2, 'c')) - assert_equal([["filter", ["a", "b", "c"], { "key1" => 1, "key2" => 2 }]], var.filters) + # positional and kwargs mixed + var = create_variable(%(n | filter: 'a', 'b', key1: 1, key2: 2, 'c')) + assert_equal([["filter", ["a", "b", "c"], { "key1" => 1, "key2" => 2 }]], var.filters) - # positional and kwargs intermixed (pos1, key1: val1, pos2) - var = create_variable(%(n | link_to: class: "black", "https://example.com", title: "title")) - assert_equal([['link_to', ["https://example.com"], { "class" => "black", "title" => "title" }]], var.filters) + # positional and kwargs intermixed (pos1, key1: val1, pos2) + var = create_variable(%(n | link_to: class: "black", "https://example.com", title: "title")) + assert_equal([['link_to', ["https://example.com"], { "class" => "black", "title" => "title" }]], var.filters) - # string key throws - assert_raises(SyntaxError) { create_variable(%(n | pluralize: 'comment': 'comments')) } - end + # string key throws + assert_raises(SyntaxError) { create_variable(%(n | pluralize: 'comment': 'comments')) } end def test_output_raw_source_of_variable From c12fc1b1bc97a4313cfa6bb0dbef9b0c83b38bb4 Mon Sep 17 00:00:00 2001 From: "Charles-P. Clermont" Date: Mon, 1 Dec 2025 10:19:34 -0500 Subject: [PATCH 4/5] Update changelog with planned changes for 6.0.0 --- History.md | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/History.md b/History.md index 94feaabfd..2bb2bbb3b 100644 --- a/History.md +++ b/History.md @@ -1,5 +1,37 @@ # Liquid Change Log +## 6.0.0 + +### Architectural changes + +### Features +* (TODO) Add support for boolean expressions everywhere + * As variable output `{{ a or b }}` + * As filter argument `{{ collection | where: 'prop', a or b }}` + * As tag argument `{% render 'snip', enabled: a or b %}` + * As conditional tag argument `{% if cond %}` (extending previous behaviour) +* (TODO) Add support for subexpression prioritization and associativity + * In ascending order of priority: + * Logical: `and`, `or` (right to left) + * Equality: `==`, `!=`, `<>` (left to right) + * Comparison: `>`, `>=`, `<`, `<=`, `contains` (left to right) + - For example, this is now supported + * `{{ a > b == c < d or e == f }}` which is equivalent to + * `{{ ((a > b) == (c < d)) or (e == f) }}` +- (TODO) Add support for parenthesized expressions + * e.g. `(a or b) and c` + +### Breaking changes +* We are removing the Environment's `error_mode` option. + * `:warn` is no longer supported + * `:lax` and `lax_parse` is no longer supported + * `:strict` and `strict_parse` is no longer supported + * `strict2_parse` is renamed to `parse_markup` + +### Migrating from `^5.11.0` +- In custom tags that include `ParserSwitching`, rename `strict2_parse` to `parse_markup` +- Remove code depending on `:error_mode` + ## 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] * Rename the `:rigid` error mode to `:strict2` and display a warning when users attempt to use the `:rigid` mode [Guilherme Carreiro] From d22ead1dae90db1ada8ff243125797694ee4adfa Mon Sep 17 00:00:00 2001 From: "Charles-P. Clermont" Date: Wed, 3 Dec 2025 10:04:10 -0500 Subject: [PATCH 5/5] Remove warnings system --- History.md | 3 ++- lib/liquid/context.rb | 7 +------ lib/liquid/parse_context.rb | 5 ++--- lib/liquid/template.rb | 3 +-- test/integration/template_test.rb | 10 ---------- 5 files changed, 6 insertions(+), 22 deletions(-) diff --git a/History.md b/History.md index 2bb2bbb3b..8fd6ef372 100644 --- a/History.md +++ b/History.md @@ -22,11 +22,12 @@ * e.g. `(a or b) and c` ### Breaking changes -* We are removing the Environment's `error_mode` option. +* The Environment's `error_mode` option has been removed. * `:warn` is no longer supported * `:lax` and `lax_parse` is no longer supported * `:strict` and `strict_parse` is no longer supported * `strict2_parse` is renamed to `parse_markup` +* The `warnings` system has been removed. ### Migrating from `^5.11.0` - In custom tags that include `ParserSwitching`, rename `strict2_parse` to `parse_markup` diff --git a/lib/liquid/context.rb b/lib/liquid/context.rb index 433b6d003..1db6bb663 100644 --- a/lib/liquid/context.rb +++ b/lib/liquid/context.rb @@ -60,10 +60,6 @@ def initialize(environments = {}, outer_scope = {}, registers = {}, rethrow_erro end # rubocop:enable Metrics/ParameterLists - def warnings - @warnings ||= [] - end - def strainer @strainer ||= @environment.create_strainer(self, @filters) end @@ -157,7 +153,6 @@ def new_isolated_subcontext subcontext.filters = @filters subcontext.strainer = nil subcontext.errors = errors - subcontext.warnings = warnings subcontext.disabled_tags = @disabled_tags end end @@ -244,7 +239,7 @@ def tag_disabled?(tag_name) protected - attr_writer :base_scope_depth, :warnings, :errors, :strainer, :filters, :disabled_tags + attr_writer :base_scope_depth, :errors, :strainer, :filters, :disabled_tags private diff --git a/lib/liquid/parse_context.rb b/lib/liquid/parse_context.rb index 8161a0369..96413eb61 100644 --- a/lib/liquid/parse_context.rb +++ b/lib/liquid/parse_context.rb @@ -3,14 +3,13 @@ module Liquid class ParseContext attr_accessor :locale, :line_number, :trim_whitespace, :depth - attr_reader :partial, :warnings, :environment + attr_reader :partial, :environment def initialize(options = Const::EMPTY_HASH) @environment = options.fetch(:environment, Environment.default) @template_options = options ? options.dup : {} - @locale = @template_options[:locale] ||= I18n.new - @warnings = [] + @locale = @template_options[:locale] ||= I18n.new # constructing new StringScanner in Lexer, Tokenizer, etc is expensive # This StringScanner will be shared by all of them diff --git a/lib/liquid/template.rb b/lib/liquid/template.rb index e638a01d7..a81fec423 100644 --- a/lib/liquid/template.rb +++ b/lib/liquid/template.rb @@ -16,7 +16,7 @@ module Liquid # class Template attr_accessor :root, :name - attr_reader :resource_limits, :warnings + attr_reader :resource_limits attr_reader :profiler @@ -209,7 +209,6 @@ def configure_options(options) ParseContext.new(opts) end - @warnings = parse_context.warnings parse_context end diff --git a/test/integration/template_test.rb b/test/integration/template_test.rb index b09f2584a..e29ca3b6b 100644 --- a/test/integration/template_test.rb +++ b/test/integration/template_test.rb @@ -44,16 +44,6 @@ def test_instance_assigns_persist_on_same_template_object_between_parses assert_equal('from instance assigns', t.parse("{{ foo }}").render!) end - def test_warnings_is_not_exponential_time - str = "false" - 100.times do - str = "{% if true %}true{% else %}#{str}{% endif %}" - end - - t = Template.parse(str) - assert_equal([], Timeout.timeout(1) { t.warnings }) - end - def test_instance_assigns_persist_on_same_template_parsing_between_renders t = Template.new.parse("{{ foo }}{% assign foo = 'foo' %}{{ foo }}") assert_equal('foo', t.render!)