Fix bug when parsing negative numbers

lib/liquid.rb

@@ -52,6 +52,7 @@ end
 require "liquid/version"
 require "liquid/deprecations"
 require "liquid/const"
+require "liquid/template/tag_registry"
 require 'liquid/standardfilters'
 require 'liquid/file_system'
 require 'liquid/parser_switching'

lib/liquid/block_body.rb

@@ -246,17 +246,10 @@ module Liquid
     end
 
     def create_variable(token, parse_context)
-      if token.end_with?("}}")
-        i = 2
-        i = 3 if token[i] == "-"
-        parse_end = token.length - 3
-        parse_end -= 1 if token[parse_end] == "-"
-        markup_end = parse_end - i + 1
-        markup = markup_end <= 0 ? "" : token.slice(i, markup_end)
-
+      if token =~ ContentOfVariable
+        markup = Regexp.last_match(1)
         return Variable.new(markup, parse_context)
       end
-
       BlockBody.raise_missing_variable_terminator(token, parse_context)
     end
 

lib/liquid/context.rb

@@ -19,7 +19,7 @@ module Liquid
 
     # rubocop:disable Metrics/ParameterLists
     def self.build(environment: Environment.default, environments: {}, outer_scope: {}, registers: {}, rethrow_errors: false, resource_limits: nil, static_environments: {}, &block)
-      new(environments, outer_scope, registers, rethrow_errors, resource_limits, static_environments, environment, &block)
+      new(environments, outer_scope, registers, rethrow_errors, resource_limits, static_environments, &block)
     end
 
     def initialize(environments = {}, outer_scope = {}, registers = {}, rethrow_errors = false, resource_limits = nil, static_environments = {}, environment = Environment.default)
@@ -143,7 +143,6 @@ module Liquid
       check_overflow
 
       self.class.build(
-        environment: @environment,
         resource_limits: resource_limits,
         static_environments: static_environments,
         registers: Registers.new(registers),

lib/liquid/environment.rb

@@ -41,7 +41,7 @@ module Liquid
       # @return [Environment] The new environment instance.
       def build(tags: nil, file_system: nil, error_mode: nil, exception_renderer: nil)
         ret = new
-        ret.tags = tags if tags
+        ret.tags = Template::TagRegistry.new(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
@@ -74,7 +74,7 @@ module Liquid
     # Initializes a new environment instance.
     # @api private
     def initialize
-      @tags = Tags::STANDARD_TAGS.dup
+      @tags = Template::TagRegistry.new(Tags::STANDARD_TAGS)
       @error_mode = :lax
       @strainer_template = Class.new(StrainerTemplate).tap do |klass|
         klass.add_filter(StandardFilters)

lib/liquid/lexer.rb

@@ -73,6 +73,7 @@ module Liquid
     COMPARISON_LESS_THAN = [:comparison, "<"].freeze
     COMPARISON_LESS_THAN_OR_EQUAL = [:comparison, "<="].freeze
     COMPARISON_NOT_EQUAL_ALT = [:comparison, "<>"].freeze
+    CONTAINS = /contains(?=\s)/
     DASH = [:dash, "-"].freeze
     DOT = [:dot, "."].freeze
     DOTDOT = [:dotdot, ".."].freeze
@@ -89,12 +90,7 @@ module Liquid
     SINGLE_STRING_LITERAL = /'[^\']*'/
     WHITESPACE_OR_NOTHING = /\s*/
 
-    SINGLE_COMPARISON_TOKENS = [].tap do |table|
-      table["<".ord] = COMPARISON_LESS_THAN
-      table[">".ord] = COMPARISON_GREATER_THAN
-    end
-
-    TWO_CHARS_COMPARISON_JUMP_TABLE = [].tap do |table|
+    COMPARISON_JUMP_TABLE = [].tap do |table|
       table["=".ord] = [].tap do |sub_table|
         sub_table["=".ord] = COMPARISON_EQUAL
         sub_table.freeze
@@ -103,9 +99,6 @@ module Liquid
         sub_table["=".ord] = COMPARISION_NOT_EQUAL
         sub_table.freeze
       end
-    end
-
-    COMPARISON_JUMP_TABLE = [].tap do |table|
       table["<".ord] = [].tap do |sub_table|
         sub_table["=".ord] = COMPARISON_LESS_THAN_OR_EQUAL
         sub_table[">".ord] = COMPARISON_NOT_EQUAL_ALT
@@ -170,7 +163,6 @@ module Liquid
 
         break if @ss.eos?
 
-        start_pos = @ss.pos
         peeked = @ss.peek_byte
 
         if (special = SPECIAL_TABLE[peeked])
@@ -181,7 +173,7 @@ module Liquid
             @output << DOTDOT
           elsif special == DASH
             # Special case for negative numbers
-            if (peeked_byte = @ss.peek_byte) && NUMBER_TABLE[peeked_byte]
+            if NUMBER_TABLE[@ss.peek_byte]
               @ss.pos -= 1
               @output << [:number, @ss.scan(NUMBER_LITERAL)]
             else
@@ -190,34 +182,26 @@ module Liquid
           else
             @output << special
           end
-        elsif (sub_table = TWO_CHARS_COMPARISON_JUMP_TABLE[peeked])
-          @ss.scan_byte
-          if (peeked_byte = @ss.peek_byte) && (found = sub_table[peeked_byte])
-            @output << found
-            @ss.scan_byte
-          else
-            raise_syntax_error(start_pos)
-          end
         elsif (sub_table = COMPARISON_JUMP_TABLE[peeked])
           @ss.scan_byte
-          if (peeked_byte = @ss.peek_byte) && (found = sub_table[peeked_byte])
+          if (found = sub_table[@ss.peek_byte])
             @output << found
             @ss.scan_byte
           else
-            @output << SINGLE_COMPARISON_TOKENS[peeked]
+            raise SyntaxError, "Unexpected character #{peeked.chr}"
           end
         else
           type, pattern = NEXT_MATCHER_JUMP_TABLE[peeked]
 
           if type && (t = @ss.scan(pattern))
             # Special case for "contains"
-            @output << if type == :id && t == "contains" && @output.last&.first != :dot
+            @output << if type == :id && t == "contains"
               COMPARISON_CONTAINS
             else
               [type, t]
             end
           else
-            raise_syntax_error(start_pos)
+            raise SyntaxError, "Unexpected character #{peeked.chr}"
           end
         end
       end
@@ -225,12 +209,6 @@ module Liquid
 
       @output << EOS
     end
-
-    def raise_syntax_error(start_pos)
-      @ss.pos = start_pos
-      # the character could be a UTF-8 character, use getch to get all the bytes
-      raise SyntaxError, "Unexpected character #{@ss.getch}"
-    end
   end
 
   Lexer = StringScanner.instance_methods.include?(:scan_byte) ? Lexer2 : Lexer1

lib/liquid/parse_tree_visitor.rb

@@ -36,7 +36,7 @@ module Liquid
     protected
 
     def children
-      @node.respond_to?(:nodelist) ? Array(@node.nodelist) : Const::EMPTY_ARRAY
+      @node.respond_to?(:nodelist) ? Array(@node.nodelist) : []
     end
   end
 end

lib/liquid/standardfilters.rb

@@ -877,7 +877,7 @@ module Liquid
     #   - [`nil`](/docs/api/liquid/basics#nil)
     # @liquid_syntax variable | default: variable
     # @liquid_return [untyped]
-    # @liquid_optional_param allow_false: [boolean] Whether to use false values instead of the default.
+    # @liquid_optional_param allow_false [boolean] Whether to use false values instead of the default.
     def default(input, default_value = '', options = {})
       options = {} unless options.is_a?(Hash)
       false_check = options['allow_false'] ? input.nil? : !Liquid::Utils.to_liquid_value(input)

lib/liquid/template/tag_registry.rb

@@ -0,0 +1,44 @@
+# frozen_string_literal: true
+
+module Liquid
+  class Template
+    class TagRegistry
+      include Enumerable
+
+      def initialize(tags = nil)
+        @tags  = {}
+        @cache = {}
+        tags.each { |tag_name, klass| self[tag_name] = klass }
+        Deprecations.warn("Template::TagRegistry", "Use a Environment instance with zeitwerk")
+      end
+
+      def [](tag_name)
+        return nil unless @tags.key?(tag_name)
+        return @cache[tag_name] if Liquid.cache_classes
+
+        lookup_class(@tags[tag_name]).tap { |o| @cache[tag_name] = o }
+      end
+
+      def delete(tag_name)
+        Deprecations.warn("Template::TagRegistry#delete", "Use a Environment instance with immutable tags")
+        @tags.delete(tag_name)
+        @cache.delete(tag_name)
+      end
+
+      def []=(tag_name, klass)
+        @tags[tag_name]  = klass.name
+        @cache[tag_name] = klass
+      end
+
+      def each(&block)
+        @tags.each(&block)
+      end
+
+      private
+
+      def lookup_class(name)
+        Object.const_get(name)
+      end
+    end
+  end
+end

lib/liquid/variable.rb

@@ -68,7 +68,7 @@ module Liquid
       @name = parse_context.parse_expression(p.expression)
       while p.consume?(:pipe)
         filtername = p.consume(:id)
-        filterargs = p.consume?(:colon) ? parse_filterargs(p) : Const::EMPTY_ARRAY
+        filterargs = p.consume?(:colon) ? parse_filterargs(p) : []
         @filters << parse_filter_expressions(filtername, filterargs)
       end
       p.consume(:end_of_string)

test/integration/context_test.rb

@@ -672,21 +672,6 @@ class ContextTest < Minitest::Test
     assert_includes(result, "unscoped_products_count: 5")
   end
 
-  def test_new_isolated_context_inherits_parent_environment
-    global_environment = Liquid::Environment.build(tags: {})
-    context = Context.build(environment: global_environment)
-
-    subcontext = context.new_isolated_subcontext
-    assert_equal(global_environment, subcontext.environment)
-  end
-
-  def test_newly_built_context_inherits_parent_environment
-    global_environment = Liquid::Environment.build(tags: {})
-    context = Context.build(environment: global_environment)
-    assert_equal(global_environment, context.environment)
-    assert(context.environment.tags.each.to_a.empty?)
-  end
-
   private
 
   def assert_no_object_allocations

test/integration/parsing_quirks_test.rb

@@ -131,16 +131,4 @@ class ParsingQuirksTest < Minitest::Test
   def test_contains_in_id
     assert_template_result(' YES ', '{% if containsallshipments == true %} YES {% endif %}', { 'containsallshipments' => true })
   end
-
-  def test_incomplete_expression
-    with_error_mode(:lax) do
-      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

test/unit/block_unit_test.rb

@@ -32,12 +32,6 @@ class BlockUnitTest < Minitest::Test
     assert_equal(String, template.root.nodelist[2].class)
   end
 
-  def test_variable_with_multibyte_character
-    template = Liquid::Template.parse("{{ '❤️' }}")
-    assert_equal(1, template.root.nodelist.size)
-    assert_equal(Variable, template.root.nodelist[0].class)
-  end
-
   def test_variable_many_embedded_fragments
     template = Liquid::Template.parse("  {{funk}} {{so}} {{brother}} ")
     assert_equal(7, template.root.nodelist.size)

test/unit/lexer_unit_test.rb

@@ -6,134 +6,58 @@ class LexerUnitTest < Minitest::Test
   include Liquid
 
   def test_strings
-    assert_equal(
-      [[:string, %('this is a test""')], [:string, %("wat 'lol'")], [:end_of_string]],
-      tokenize(%( 'this is a test""' "wat 'lol'")),
-    )
+    tokens = Lexer.new(%( 'this is a test""' "wat 'lol'")).tokenize
+    assert_equal([[:string, %('this is a test""')], [:string, %("wat 'lol'")], [:end_of_string]], tokens)
   end
 
   def test_integer
-    assert_equal(
-      [[:id, 'hi'], [:number, '50'], [:end_of_string]],
-      tokenize('hi 50'),
-    )
+    tokens = Lexer.new('hi 50').tokenize
+    assert_equal([[:id, 'hi'], [:number, '50'], [:end_of_string]], tokens)
   end
 
   def test_float
-    assert_equal(
-      [[:id, 'hi'], [:number, '5.0'], [:end_of_string]],
-      tokenize('hi 5.0'),
-    )
+    tokens = Lexer.new('hi 5.0').tokenize
+    assert_equal([[:id, 'hi'], [:number, '5.0'], [:end_of_string]], tokens)
   end
 
   def test_comparison
-    assert_equal(
-      [[:comparison, '=='], [:comparison, '<>'], [:comparison, 'contains'], [:end_of_string]],
-      tokenize('== <> contains '),
-    )
-  end
-
-  def test_comparison_without_whitespace
-    assert_equal(
-      [[:number, '1'], [:comparison, '>'], [:number, '0'], [:end_of_string]],
-      tokenize('1>0'),
-    )
-  end
-
-  def test_comparison_with_negative_number
-    assert_equal(
-      [[:number, '1'], [:comparison, '>'], [:number, '-1'], [:end_of_string]],
-      tokenize('1>-1'),
-    )
-  end
-
-  def test_raise_for_invalid_comparison
-    assert_raises(SyntaxError) do
-      tokenize('1>!1')
-    end
-
-    assert_raises(SyntaxError) do
-      tokenize('1=<1')
-    end
-
-    assert_raises(SyntaxError) do
-      tokenize('1!!1')
-    end
+    tokens = Lexer.new('== <> contains ').tokenize
+    assert_equal([[:comparison, '=='], [:comparison, '<>'], [:comparison, 'contains'], [:end_of_string]], tokens)
   end
 
   def test_specials
-    assert_equal(
-      [[:pipe, '|'], [:dot, '.'], [:colon, ':'], [:end_of_string]],
-      tokenize('| .:'),
-    )
-
-    assert_equal(
-      [[:open_square, '['], [:comma, ','], [:close_square, ']'], [:end_of_string]],
-      tokenize('[,]'),
-    )
+    tokens = Lexer.new('| .:').tokenize
+    assert_equal([[:pipe, '|'], [:dot, '.'], [:colon, ':'], [:end_of_string]], tokens)
+    tokens = Lexer.new('[,]').tokenize
+    assert_equal([[:open_square, '['], [:comma, ','], [:close_square, ']'], [:end_of_string]], tokens)
   end
 
   def test_fancy_identifiers
-    assert_equal([[:id, 'hi'], [:id, 'five?'], [:end_of_string]], tokenize('hi five?'))
+    tokens = Lexer.new('hi five?').tokenize
+    assert_equal([[:id, 'hi'], [:id, 'five?'], [:end_of_string]], tokens)
 
-    assert_equal([[:number, '2'], [:id, 'foo'], [:end_of_string]], tokenize('2foo'))
+    tokens = Lexer.new('2foo').tokenize
+    assert_equal([[:number, '2'], [:id, 'foo'], [:end_of_string]], tokens)
   end
 
   def test_whitespace
-    assert_equal(
-      [[:id, 'five'], [:pipe, '|'], [:comparison, '=='], [:end_of_string]],
-      tokenize("five|\n\t =="),
-    )
+    tokens = Lexer.new("five|\n\t ==").tokenize
+    assert_equal([[:id, 'five'], [:pipe, '|'], [:comparison, '=='], [:end_of_string]], tokens)
   end
 
   def test_unexpected_character
     assert_raises(SyntaxError) do
-      tokenize("%")
+      Lexer.new("%").tokenize
     end
   end
 
   def test_negative_numbers
-    assert_equal(
-      [[:id, 'foo'], [:pipe, '|'], [:id, 'default'], [:colon, ":"], [:number, '-1'], [:end_of_string]],
-      tokenize("foo | default: -1"),
-    )
+    tokens = Lexer.new("foo | default: -1").tokenize
+    assert_equal([[:id, 'foo'], [:pipe, '|'], [:id, 'default'], [:colon, ":"], [:number, '-1'], [:end_of_string]], tokens)
   end
 
   def test_greater_than_two_digits
-    assert_equal(
-      [[:id, 'foo'], [:comparison, '>'], [:number, '12'], [:end_of_string]],
-      tokenize("foo > 12"),
-    )
-  end
-
-  def test_error_with_utf8_character
-    error = assert_raises(SyntaxError) do
-      tokenize("1 < 1Ø")
-    end
-
-    assert_equal(
-      'Liquid syntax error: Unexpected character Ø',
-      error.message,
-    )
-  end
-
-  def test_contains_as_attribute_name
-    assert_equal(
-      [[:id, "a"], [:dot, "."], [:id, "contains"], [:dot, "."], [:id, "b"], [:end_of_string]],
-      tokenize("a.contains.b"),
-    )
-  end
-
-  def test_tokenize_incomplete_expression
-    assert_equal([[:id, "false"], [:dash, "-"], [:end_of_string]], tokenize("false -"))
-    assert_equal([[:id, "false"], [:comparison, "<"], [:end_of_string]], tokenize("false <"))
-    assert_equal([[:id, "false"], [:comparison, ">"], [:end_of_string]], tokenize("false >"))
-    assert_equal([[:id, "false"], [:number, "1"], [:end_of_string]], tokenize("false 1"))
-  end
-
-  private
-
-  def tokenize(input)
-    Lexer.new(input).tokenize
+    tokens = Lexer.new("foo > 12").tokenize
+    assert_equal([[:id, 'foo'], [:comparison, '>'], [:number, '12'], [:end_of_string]], tokens)
   end
 end

test/unit/template_unit_test.rb

@@ -39,12 +39,31 @@ class TemplateUnitTest < Minitest::Test
     Liquid.cache_classes = original_cache_setting
   end
 
+  def test_without_cache_classes_tags_reloads_the_class
+    original_cache_setting = Liquid.cache_classes
+    Liquid.cache_classes   = false
+
+    original_klass = Class.new
+    Object.send(:const_set, :CustomTag, original_klass)
+    with_custom_tag('custom', CustomTag) do
+      Object.send(:remove_const, :CustomTag)
+
+      new_klass = Class.new
+      Object.send(:const_set, :CustomTag, new_klass)
+
+      assert(Template.tags['custom'].equal?(new_klass))
+    end
+  ensure
+    Object.send(:remove_const, :CustomTag)
+    Liquid.cache_classes = original_cache_setting
+  end
+
   class FakeTag; end
 
   def test_tags_can_be_looped_over
     with_custom_tag('fake', FakeTag) do
       result = Template.tags.map { |name, klass| [name, klass] }
-      assert(result.include?(["fake", TemplateUnitTest::FakeTag]))
+      assert(result.include?(["fake", "TemplateUnitTest::FakeTag"]))
     end
   end