remove Expression2 strscan workaround

lib/liquid/expression.rb

@@ -3,52 +3,7 @@
 require "lru_redux"
 
 module Liquid
-  class Expression1
+  class Expression
-    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
     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

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
+  MARKUPS.each do |type, markups|
-    if Liquid::Expression != Liquid::Expression1
+    x.report("Liquid::Expression#parse: #{type}") do
-      Liquid.send(:remove_const, :Expression)
+      markups.each do |markup|
-      Liquid.const_set(:Expression, Liquid::Expression1)
+        Liquid::Expression.parse(markup)
+      end
+    end
   end
 
+  x.report("Liquid::Expression#parse: all") do
     MARKUPS.values.flatten.each do |markup|
-      Liquid::Expression1.parse(markup)
+      Liquid::Expression.parse(markup)
     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
-
-    MARKUPS.values.flatten.each do |markup|
-      Liquid::Expression2.parse(markup)
-    end
-  end
-
-  x.compare!
 end

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 = Liquid::Lexer.new(expr)
-      l.tokenize
-    end
-  end
-
-  x.report("Liquid::Lexer2#tokenize") do
-    EXPRESSIONS.each do |expr|
-      l = Liquid::Lexer2.new(expr)
       l.tokenize
     end
   end

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 %}