diff --git a/lib/liquid/expression.rb b/lib/liquid/expression.rb index 5b3f0cd1..f2abb2fc 100644 --- a/lib/liquid/expression.rb +++ b/lib/liquid/expression.rb @@ -3,52 +3,7 @@ require "lru_redux" module Liquid - class Expression1 - LITERALS = { - nil => nil, - 'nil' => nil, - 'null' => nil, - '' => nil, - 'true' => true, - 'false' => false, - 'blank' => '', - 'empty' => '' - }.freeze - - INTEGERS_REGEX = /\A(-?\d+)\z/ - FLOATS_REGEX = /\A(-?\d[\d\.]+)\z/ - - # Use an atomic group (?>...) to avoid pathological backtracing from - # malicious input as described in https://github.com/Shopify/liquid/issues/1357 - RANGES_REGEX = /\A\(\s*(?>(\S+)\s*\.\.)\s*(\S+)\s*\)\z/ - - def self.parse(markup, _ss = nil, _cache = nil) - return nil unless markup - - markup = markup.strip - if (markup.start_with?('"') && markup.end_with?('"')) || - (markup.start_with?("'") && markup.end_with?("'")) - return markup[1..-2] - end - - case markup - when INTEGERS_REGEX - Regexp.last_match(1).to_i - when RANGES_REGEX - RangeLookup.parse(Regexp.last_match(1), Regexp.last_match(2), nil) - when FLOATS_REGEX - Regexp.last_match(1).to_f - else - if LITERALS.key?(markup) - LITERALS[markup] - else - VariableLookup.parse(markup, nil) - end - end - end - end - - class Expression2 + class Expression LITERALS = { nil => nil, 'nil' => nil, @@ -161,7 +116,4 @@ module Liquid end end end - - # Remove this once we can depend on strscan >= 3.1.1 - Expression = HAS_STRING_SCANNER_SCAN_BYTE ? Expression2 : Expression1 end diff --git a/performance/unit/expression_benchmark.rb b/performance/unit/expression_benchmark.rb index ae4eeaa5..8cd55259 100644 --- a/performance/unit/expression_benchmark.rb +++ b/performance/unit/expression_benchmark.rb @@ -75,109 +75,20 @@ MARKUPS = { "range" => RANGE_MARKUPS, } -def compare_objects(object_1, object_2) - if object_1.is_a?(Liquid::VariableLookup) && object_2.is_a?(Liquid::VariableLookup) - return false if object_1.name != object_2.name - elsif object_1 != object_2 - return false - end - - true -end - -def compare_range_lookup(expression_1_result, expression_2_result) - return false unless expression_1_result.is_a?(Liquid::RangeLookup) && expression_2_result.is_a?(Liquid::RangeLookup) - - start_obj_1 = expression_1_result.start_obj - start_obj_2 = expression_2_result.start_obj - - return false unless compare_objects(start_obj_1, start_obj_2) - - end_obj_1 = expression_1_result.end_obj - end_obj_2 = expression_2_result.end_obj - - return false unless compare_objects(end_obj_1, end_obj_2) - - true -end - -MARKUPS.values.flatten.each do |markup| - expression_1_result = Liquid::Expression1.parse(markup) - expression_2_result = Liquid::Expression2.parse(markup) - - next if expression_1_result == expression_2_result - - if expression_1_result.is_a?(Liquid::RangeLookup) && expression_2_result.is_a?(Liquid::RangeLookup) - next if compare_range_lookup(expression_1_result, expression_2_result) - end - - warn "Expression1 and Expression2 results are different for markup: #{markup}" - warn "expected: #{expression_1_result}" - warn "got: #{expression_2_result}" - abort -end - -warmed_up = false - -MARKUPS.each do |type, markups| - Benchmark.ips do |x| - if warmed_up - x.config(time: 10, warmup: 5) - warmed_up = true - else - x.config(time: 10) - end - - x.report("Liquid::Expression1#parse: #{type}") do - if Liquid::Expression != Liquid::Expression1 - Liquid.send(:remove_const, :Expression) - Liquid.const_set(:Expression, Liquid::Expression1) - end - - markups.each do |markup| - Liquid::Expression1.parse(markup) - end - end - - x.report("Liquid::Expression2#parse: #{type}") do - if Liquid::Expression != Liquid::Expression2 - Liquid.send(:remove_const, :Expression) - Liquid.const_set(:Expression, Liquid::Expression2) - end - - markups.each do |markup| - Liquid::Expression2.parse(markup) - end - end - - x.compare! - end -end - Benchmark.ips do |x| - x.config(time: 10) + x.config(time: 5, warmup: 5) - x.report("Liquid::Expression1#parse: all") do - if Liquid::Expression != Liquid::Expression1 - Liquid.send(:remove_const, :Expression) - Liquid.const_set(:Expression, Liquid::Expression1) - end - - MARKUPS.values.flatten.each do |markup| - Liquid::Expression1.parse(markup) + MARKUPS.each do |type, markups| + x.report("Liquid::Expression#parse: #{type}") do + markups.each do |markup| + Liquid::Expression.parse(markup) + end end end - x.report("Liquid::Expression2#parse: all") do - if Liquid::Expression != Liquid::Expression2 - Liquid.send(:remove_const, :Expression) - Liquid.const_set(:Expression, Liquid::Expression2) - end - + x.report("Liquid::Expression#parse: all") do MARKUPS.values.flatten.each do |markup| - Liquid::Expression2.parse(markup) + Liquid::Expression.parse(markup) end end - - x.compare! end diff --git a/performance/unit/lexer_benchmark.rb b/performance/unit/lexer_benchmark.rb index 2faa1f68..fda6ce1f 100644 --- a/performance/unit/lexer_benchmark.rb +++ b/performance/unit/lexer_benchmark.rb @@ -29,31 +29,12 @@ EXPRESSIONS = [ "foo | default: -1", ] -EXPRESSIONS.each do |expr| - lexer_1_result = Liquid::Lexer1.new(expr).tokenize - lexer_2_result = Liquid::Lexer2.new(expr).tokenize - - next if lexer_1_result == lexer_2_result - - warn "Lexer1 and Lexer2 results are different for expression: #{expr}" - warn "expected: #{lexer_1_result}" - warn "got: #{lexer_2_result}" - abort -end - Benchmark.ips do |x| x.config(time: 10, warmup: 5) - x.report("Liquid::Lexer1#tokenize") do + x.report("Liquid::Lexer#tokenize") do EXPRESSIONS.each do |expr| - l = Liquid::Lexer1.new(expr) - l.tokenize - end - end - - x.report("Liquid::Lexer2#tokenize") do - EXPRESSIONS.each do |expr| - l = Liquid::Lexer2.new(expr) + l = Liquid::Lexer.new(expr) l.tokenize end end diff --git a/test/integration/expression_test.rb b/test/integration/expression_test.rb index bb2c02e0..190a6bec 100644 --- a/test/integration/expression_test.rb +++ b/test/integration/expression_test.rb @@ -57,7 +57,6 @@ class ExpressionTest < Minitest::Test def test_expression_cache skip("Liquid-C does not support Expression caching") if defined?(Liquid::C) && Liquid::C.enabled - skip("Expression Caching is only available with Expression2") if Liquid::Expression != Liquid::Expression2 cache = LruRedux::Cache.new(10) template = <<~LIQUID @@ -79,7 +78,6 @@ class ExpressionTest < Minitest::Test def test_disable_expression_cache skip("Liquid-C does not support Expression caching") if defined?(Liquid::C) && Liquid::C.enabled - skip("Expression Caching is only available with Expression2") if Liquid::Expression != Liquid::Expression2 template = <<~LIQUID {% assign x = 1 %}