diff --git a/Rakefile b/Rakefile index 51c22e04..82f50009 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 095b2b09..7834f49c 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 @@ module Liquid # 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 @@ module Liquid # @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 855acc64..8161a036 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 @@ module Liquid 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 @@ module Liquid 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 f49d14d9..5cca4e04 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 6bb03dfc..e638a01d 100644 --- a/lib/liquid/template.rb +++ b/lib/liquid/template.rb @@ -21,17 +21,6 @@ module Liquid 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 b61e9057..c8aa6769 100644 --- a/performance/benchmark.rb +++ b/performance/benchmark.rb @@ -4,7 +4,6 @@ require 'benchmark/ips' 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 e2934297..fb7312d5 100644 --- a/performance/memory_profile.rb +++ b/performance/memory_profile.rb @@ -53,8 +53,6 @@ class Profiler 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 70740778..f756fb20 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/spec/ruby_liquid_lax.rb b/spec/ruby_liquid_lax.rb deleted file mode 100644 index 4681ad41..00000000 --- a/spec/ruby_liquid_lax.rb +++ /dev/null @@ -1,34 +0,0 @@ -# frozen_string_literal: true - -# Liquid Spec Adapter for Shopify/liquid with lax parsing mode -# -# Run with: bundle exec liquid-spec run spec/ruby_liquid_lax.rb - -$LOAD_PATH.unshift(File.expand_path('../lib', __dir__)) -require 'liquid' - -LiquidSpec.configure do |config| - config.features = [:core, :lax_parsing] -end - -# Compile a template string into a Liquid::Template -LiquidSpec.compile do |ctx, source, options| - # Force lax mode - options = options.merge(error_mode: :lax) - ctx[:template] = Liquid::Template.parse(source, **options) -end - -# Render a compiled template with the given context -LiquidSpec.render do |ctx, assigns, options| - registers = Liquid::Registers.new(options[:registers] || {}) - - context = Liquid::Context.build( - static_environments: assigns, - registers: registers, - rethrow_errors: options[:strict_errors], - ) - - context.exception_renderer = options[:exception_renderer] if options[:exception_renderer] - - ctx[:template].render(context) -end diff --git a/spec/ruby_liquid_yjit.rb b/spec/ruby_liquid_yjit.rb index 3ff51d1f..2b3ed0b6 100644 --- a/spec/ruby_liquid_yjit.rb +++ b/spec/ruby_liquid_yjit.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Liquid Spec Adapter for Shopify/liquid with YJIT + strict mode + ActiveSupport +# Liquid Spec Adapter for Shopify/liquid with YJIT + ActiveSupport # # Run with: bundle exec liquid-spec run spec/ruby_liquid_yjit.rb @@ -20,8 +20,6 @@ end # Compile a template string into a Liquid::Template LiquidSpec.compile do |ctx, source, options| - # Force strict mode - options = { error_mode: :strict }.merge(options) ctx[:template] = Liquid::Template.parse(source, **options) end diff --git a/test/integration/assign_test.rb b/test/integration/assign_test.rb index a88941ae..69163ab9 100644 --- a/test/integration/assign_test.rb +++ b/test/integration/assign_test.rb @@ -39,11 +39,10 @@ class AssignTest < Minitest::Test 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 db68da45..e1952e63 100644 --- a/test/integration/context_test.rb +++ b/test/integration/context_test.rb @@ -632,11 +632,9 @@ class ContextTest < Minitest::Test 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 9f07b1f8..9de179f2 100644 --- a/test/integration/error_handling_test.rb +++ b/test/integration/error_handling_test.rb @@ -67,10 +67,8 @@ class ErrorHandlingTest < Minitest::Test 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 @@ class ErrorHandlingTest < Minitest::Test bla ', - error_mode: :rigid, line_numbers: true, ) end @@ -131,12 +128,12 @@ class ErrorHandlingTest < Minitest::Test 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 52351209..75954f93 100644 --- a/test/integration/parsing_quirks_test.rb +++ b/test/integration/parsing_quirks_test.rb @@ -31,31 +31,25 @@ class ParsingQuirksTest < Minitest::Test 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 cf3fdea1..9edd1995 100644 --- a/test/integration/tags/cycle_tag_test.rb +++ b/test/integration/tags/cycle_tag_test.rb @@ -91,23 +91,21 @@ class CycleTagTest < Minitest::Test 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 @@ class CycleTagTest < Minitest::Test 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 @@ class CycleTagTest < Minitest::Test {% 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 @@ class CycleTagTest < Minitest::Test {% 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 e35addcd..b911ace1 100644 --- a/test/integration/tags/include_tag_test.rb +++ b/test/integration/tags/include_tag_test.rb @@ -204,15 +204,13 @@ class IncludeTagTest < Minitest::Test ) 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 @@ class IncludeTagTest < Minitest::Test 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 @@ class IncludeTagTest < Minitest::Test 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 @@ class IncludeTagTest < Minitest::Test 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 440342c3..2c69ba54 100644 --- a/test/integration/tags/render_tag_test.rb +++ b/test/integration/tags/render_tag_test.rb @@ -105,15 +105,13 @@ class RenderTagTest < Minitest::Test 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 @@ class RenderTagTest < Minitest::Test 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 e8f7adb4..59b4cc25 100644 --- a/test/integration/tags/table_row_test.rb +++ b/test/integration/tags/table_row_test.rb @@ -236,7 +236,7 @@ class TableRowTest < Minitest::Test ) 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 @@ class TableRowTest < Minitest::Test 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 @@ class TableRowTest < Minitest::Test 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 @@ class TableRowTest < Minitest::Test 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 @@ class TableRowTest < Minitest::Test 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 @@ class TableRowTest < Minitest::Test 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 @@ class TableRowTest < Minitest::Test 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 @@ class TableRowTest < Minitest::Test 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 @@ class TableRowTest < Minitest::Test 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 @@ class TableRowTest < Minitest::Test 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 @@ class TableRowTest < Minitest::Test 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 ed86b3f9..d5c83cd0 100644 --- a/test/integration/template_test.rb +++ b/test/integration/template_test.rb @@ -259,7 +259,7 @@ class TemplateTest < Minitest::Test 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 82e7ac3e..38096e41 100644 --- a/test/integration/variable_test.rb +++ b/test/integration/variable_test.rb @@ -179,40 +179,30 @@ class VariableTest < Minitest::Test 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 201bdf26..f37d6db7 100755 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -8,13 +8,6 @@ $LOAD_PATH.unshift(File.join(File.expand_path(__dir__), '..', 'lib')) 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 Minitest 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 @@ module Minitest 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 67d6e262..3f35db59 100644 --- a/test/unit/condition_unit_test.rb +++ b/test/unit/condition_unit_test.rb @@ -166,8 +166,8 @@ class ConditionUnitTest < Minitest::Test 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 @@ class ConditionUnitTest < Minitest::Test 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 # Tests for blank? comparison without ActiveSupport diff --git a/test/unit/parse_context_unit_test.rb b/test/unit/parse_context_unit_test.rb index d1b32efc..f75a4804 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('') + parser = 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) - - 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 72555d52..cb8e2d35 100644 --- a/test/unit/partial_cache_unit_test.rb +++ b/test/unit/partial_cache_unit_test.rb @@ -175,7 +175,7 @@ class PartialCacheUnitTest < Minitest::Test 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 @@ class PartialCacheUnitTest < Minitest::Test }, ) - [: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 b7c562ba..79212dd6 100644 --- a/test/unit/tags/case_tag_unit_test.rb +++ b/test/unit/tags/case_tag_unit_test.rb @@ -20,11 +20,9 @@ class CaseTagUnitTest < Minitest::Test {%- 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 @@ class CaseTagUnitTest < Minitest::Test {%- 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 @@ class CaseTagUnitTest < Minitest::Test {%- 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 @@ class CaseTagUnitTest < Minitest::Test {%- endcase -%} LIQUID - with_error_modes(:strict2) do - assert_template_result("one", template) - end + assert_template_result("one", template) end def test_case_when_empty @@ -84,14 +76,12 @@ class CaseTagUnitTest < Minitest::Test {%- endcase -%} LIQUID - with_error_modes(:lax, :strict, :strict2) do - assert_template_result("2 or empty", template, { 'x' => 2 }) - assert_template_result("2 or empty", template, { 'x' => {} }) - assert_template_result("2 or empty", template, { 'x' => [] }) - assert_template_result("not 2 or empty", template, { 'x' => { 'a' => 'b' } }) - assert_template_result("not 2 or empty", template, { 'x' => ['a'] }) - assert_template_result("not 2 or empty", template, { 'x' => 4 }) - end + assert_template_result("2 or empty", template, { 'x' => 2 }) + assert_template_result("2 or empty", template, { 'x' => {} }) + assert_template_result("2 or empty", template, { 'x' => [] }) + assert_template_result("not 2 or empty", template, { 'x' => { 'a' => 'b' } }) + assert_template_result("not 2 or empty", template, { 'x' => ['a'] }) + assert_template_result("not 2 or empty", template, { 'x' => 4 }) end def test_case_with_invalid_expression @@ -105,11 +95,9 @@ class CaseTagUnitTest < Minitest::Test 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 @@ -123,10 +111,8 @@ class CaseTagUnitTest < Minitest::Test 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 590aa3f3..6379c6ec 100644 --- a/test/unit/variable_unit_test.rb +++ b/test/unit/variable_unit_test.rb @@ -102,11 +102,9 @@ class VariableUnitTest < Minitest::Test 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 @@ class VariableUnitTest < Minitest::Test 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