add an unit test for double nest variable lookup

raise invalid integer argument error from tablerow

.github/workflows/liquid.yml

@@ -1,5 +1,10 @@
 name: Liquid
 on: [push, pull_request]
+
+env:
+  BUNDLE_JOBS: 4
+  BUNDLE_RETRY: 3
+
 jobs:
   test:
     runs-on: ubuntu-latest
@@ -7,22 +12,18 @@ jobs:
       matrix:
         entry:
           - { ruby: 2.7, allowed-failure: false } # minimum supported
-          - { ruby: 3.1, allowed-failure: false } # latest
+          - { ruby: 3.2, allowed-failure: false } # latest
           - { ruby: ruby-head, allowed-failure: true }
-    name: test (${{ matrix.entry.ruby }})
+    name: Test Ruby ${{ matrix.entry.ruby }}
     steps:
       - uses: actions/checkout@v3
       - uses: ruby/setup-ruby@v1
         with:
           ruby-version: ${{ matrix.entry.ruby }}
-      - uses: actions/cache@v1
-        with:
-          path: vendor/bundle
-          key: ${{ runner.os }}-gems-${{ hashFiles('Gemfile') }}
-          restore-keys: ${{ runner.os }}-gems-
-      - run: bundle install --jobs=3 --retry=3 --path=vendor/bundle
+          bundler-cache: true
       - run: bundle exec rake
         continue-on-error: ${{ matrix.entry.allowed-failure }}
+
   memory_profile:
     runs-on: ubuntu-latest
     steps:
@@ -30,10 +31,5 @@ jobs:
       - uses: ruby/setup-ruby@v1
         with:
           ruby-version: 2.7
-      - uses: actions/cache@v1
-        with:
-          path: vendor/bundle
-          key: ${{ runner.os }}-gems-${{ hashFiles('Gemfile') }}
-          restore-keys: ${{ runner.os }}-gems-
-      - run: bundle install --jobs=3 --retry=3 --path=vendor/bundle
+          bundler-cache: true
       - run: bundle exec rake memory_profile:run

Rakefile

@@ -5,7 +5,7 @@ require 'rake/testtask'
 $LOAD_PATH.unshift(File.expand_path("../lib", __FILE__))
 require "liquid/version"
 
-task(default: [:test, "test:migrator_integration", :rubocop])
+task(default: [:test, :rubocop])
 
 desc('run test suite with default parser')
 Rake::TestTask.new(:base_test) do |t|
@@ -33,23 +33,6 @@ task :rubocop do
   end
 end
 
-namespace :test do
-  task :migrator_integration do
-    ENV['LIQUID_MIGRATOR'] = '1'
-    original_parse_mode = ENV['LIQUID_PARSER_MODE']
-    begin
-      Rake::Task['integration_test'].reenable
-      ["lax", "strict"].each do |parse_mode|
-        ENV['LIQUID_PARSER_MODE'] = parse_mode
-        Rake::Task['integration_test'].invoke
-        Rake::Task['integration_test'].reenable
-      end
-    ensure
-      ENV['LIQUID_PARSER_MODE'] = original_parse_mode
-    end
-  end
-end
-
 desc('runs test suite with both strict and lax parsers')
 task :test do
   ENV['LIQUID_PARSER_MODE'] = 'lax'

lib/liquid.rb

@@ -41,7 +41,7 @@ module Liquid
   AnyStartingTag              = /#{TagStart}|#{VariableStart}/o
   PartialTemplateParser       = /#{TagStart}.*?#{TagEnd}|#{VariableStart}.*?#{VariableIncompleteEnd}/om
   TemplateParser              = /(#{PartialTemplateParser}|#{AnyStartingTag})/om
-  VariableParser              = /\[[^\]]+\]|#{VariableSegment}+\??/o
+  VariableParser              = /\[(?:[^\[\]]+|\g<0>)*\]|#{VariableSegment}+\??/o
 
   RAISE_EXCEPTION_LAMBDA = ->(_e) { raise }
 
@@ -68,7 +68,6 @@ require 'liquid/parser_switching'
 require 'liquid/tag'
 require 'liquid/tag/disabler'
 require 'liquid/tag/disableable'
-require 'liquid/parse_context'
 require 'liquid/block'
 require 'liquid/block_body'
 require 'liquid/document'
@@ -82,6 +81,7 @@ require 'liquid/standardfilters'
 require 'liquid/condition'
 require 'liquid/utils'
 require 'liquid/tokenizer'
+require 'liquid/parse_context'
 require 'liquid/partial_cache'
 require 'liquid/usage'
 require 'liquid/registers'

lib/liquid/block.rb

@@ -60,21 +60,6 @@ module Liquid
       @block_delimiter ||= "end#{block_name}"
     end
 
-    def self.migrate_body(start_tag_name, tokenizer, parse_context)
-      new_body, unknown_tag = BlockBody.migrate(tokenizer, parse_context)
-
-      raise SyntaxError unless unknown_tag
-
-      block_delimiter = "end#{start_tag_name}"
-      if unknown_tag.tag_name == block_delimiter
-        new_body << unknown_tag.replaced_markup("") # markup was ignored on end tags
-        return [new_body, nil]
-      end
-
-      # handle the delimiter tag in the caller
-      [new_body, unknown_tag]
-    end
-
     private
 
     # @api public

lib/liquid/block_body.rb

@@ -30,16 +30,6 @@ module Liquid
       end
     end
 
-    def self.migrate(tokenizer, parse_context, &block)
-      parse_context.line_number = tokenizer.line_number
-
-      if tokenizer.for_liquid_tag
-        migrate_for_liquid_tag(tokenizer, parse_context, &block)
-      else
-        migrate_for_document(tokenizer, parse_context, &block)
-      end
-    end
-
     def freeze
       @nodelist.freeze
       super
@@ -70,57 +60,6 @@ module Liquid
       yield nil, nil
     end
 
-    class UnknownTagMigrator
-      attr_reader :tag_name, :markup
-
-      def initialize(match:, markup_capture_number:, tag_name:, markup:)
-        @match = match
-        @tag_name = tag_name
-        @markup = markup
-        @markup_capture_number = markup_capture_number
-      end
-
-      def original_tag_string
-        @match[0]
-      end
-
-      def replaced_markup(new_markup)
-        Utils.match_capture_replace(@match, @markup_capture_number, new_markup)
-      end
-    end
-
-    private_class_method def self.migrate_for_liquid_tag(tokenizer, parse_context)
-      result = +""
-      while (token = tokenizer.shift)
-        token += "\n" if tokenizer.more?
-        if token.empty? || token.match?(WhitespaceOrNothing)
-          result << token
-        else
-          # modified version of LiquidTagToken with following changes:
-          # * TagName is optional, to continue supporting its absence in the comment tag
-          # * trailing spaces is allowed to support the newline appended above and so the tag
-          #   migrate method doesn't have to handle trailing whitespace
-          match = token.match(/\A\s*(#{TagName})?\s*(.*?)\s*\z/o)
-          tag_name = match[1]
-          markup   = match[2]
-          unless (tag = Template.tags[tag_name])
-            # delegate handling of unknown tags to the caller, where a block tag may treat
-            # it as an end tag or body delimiter.
-            unknown_tag = UnknownTagMigrator.new(
-              match: match, markup_capture_number: 2, tag_name: tag_name, markup: markup
-            )
-            return [result, unknown_tag]
-          end
-          new_markup, new_tag_body = tag.migrate(tag_name, markup, tokenizer, parse_context)
-          result << Utils.match_capture_replace(match, 2, new_markup)
-          result << new_tag_body.to_s
-        end
-        parse_context.line_number = tokenizer.line_number
-      end
-
-      [result, nil]
-    end
-
     # @api private
     def self.unknown_tag_in_liquid_tag(tag, parse_context)
       Block.raise_unknown_tag(tag, 'liquid', '%}', parse_context)
@@ -170,15 +109,6 @@ module Liquid
       end
     end
 
-    private_class_method def self.migrate_liquid_tag(markup, parse_context)
-      liquid_tag_tokenizer = parse_context.new_tokenizer(
-        markup, start_line_number: parse_context.line_number, for_liquid_tag: true
-      )
-      result, unknown_tag = migrate_for_liquid_tag(liquid_tag_tokenizer, parse_context)
-      raise SyntaxError if unknown_tag
-      result
-    end
-
     private def handle_invalid_tag_token(token, parse_context)
       if token.end_with?('%}')
         yield token, token
@@ -236,58 +166,6 @@ module Liquid
       yield nil, nil
     end
 
-    private_class_method def self.migrate_for_document(tokenizer, parse_context, &block)
-      result = +""
-      while (token = tokenizer.shift)
-        next if token.empty?
-
-        case
-        when token.start_with?(TAGSTART)
-          raise SyntaxError unless token.end_with?('%}')
-          # modified FullToken regex with optional tag name, to allow its absence in a comment tag
-          match = token.match(/\A#{TagStart}#{WhitespaceControl}?(\s*)(#{TagName})?(\s*)(.*?)#{WhitespaceControl}?#{TagEnd}\z/om)
-          tag_name = match[2]
-          markup   = match[4]
-
-          if parse_context.line_number
-            # newlines inside the tag should increase the line number,
-            # particularly important for multiline {% liquid %} tags
-            parse_context.line_number += Regexp.last_match(1).count("\n") + Regexp.last_match(3).count("\n")
-          end
-
-          if tag_name == 'liquid'
-            new_markup = Utils.migrate_stripped(markup) do |stripped_markup|
-              migrate_liquid_tag(stripped_markup, parse_context)
-            end
-            result << Utils.match_capture_replace(match, 4, new_markup)
-            next
-          end
-
-          unless (tag = Template.tags[tag_name])
-            # delegate handling of unknown tags to the caller, where a block tag may treat
-            # it as an end tag or body delimiter.
-            unknown_tag = UnknownTagMigrator.new(
-              match: match, markup_capture_number: 4, tag_name: tag_name, markup: markup
-            )
-            return [result, unknown_tag]
-          end
-          new_tag_body = nil
-          new_markup = Utils.migrate_stripped(markup) do |stripped_markup|
-            new_stripped_markup, new_tag_body = tag.migrate(tag_name, stripped_markup, tokenizer, parse_context)
-            new_stripped_markup
-          end
-          result << Utils.match_capture_replace(match, 4, new_markup) << new_tag_body.to_s
-        when token.start_with?(VARSTART)
-          result << migrate_variable(token, parse_context)
-        else
-          result << token
-        end
-        parse_context.line_number = tokenizer.line_number
-      end
-
-      [result, nil]
-    end
-
     def whitespace_handler(token, parse_context)
       if token[2] == WhitespaceControl
         previous_token = @nodelist.last
@@ -368,17 +246,6 @@ module Liquid
       BlockBody.raise_missing_variable_terminator(token, parse_context)
     end
 
-    private_class_method def self.migrate_variable(token, parse_context)
-      match = token.match(ContentOfVariable)
-      if match
-        new_markup = Utils.migrate_stripped(match[1]) do |markup|
-          Variable.migrate(markup, parse_context)
-        end
-        return Utils.match_capture_replace(match, 1, new_markup)
-      end
-      BlockBody.raise_missing_variable_terminator(token, parse_context)
-    end
-
     # @deprecated Use {.raise_missing_tag_terminator} instead
     def raise_missing_tag_terminator(token, parse_context)
       BlockBody.raise_missing_tag_terminator(token, parse_context)

lib/liquid/document.rb

@@ -28,16 +28,6 @@ module Liquid
       raise
     end
 
-    def self.migrate(tokenizer, parse_context)
-      new_body, unknown_tag = BlockBody.migrate(tokenizer, parse_context)
-      raise SyntaxError if unknown_tag
-
-      new_body
-    rescue SyntaxError => e
-      e.line_number ||= parse_context.line_number
-      raise
-    end
-
     def unknown_tag(tag, _markup, _tokenizer)
       case tag
       when 'else', 'end'

lib/liquid/drop.rb

@@ -25,6 +25,10 @@ module Liquid
   class Drop
     attr_writer :context
 
+    def initialize
+      @context = nil
+    end
+
     # Catch all for the method
     def liquid_method_missing(method)
       return nil unless @context&.strict_variables

lib/liquid/expression.rb

@@ -41,36 +41,5 @@ module Liquid
         end
       end
     end
-
-    def self.lax_migrate(markup)
-      Utils.migrate_stripped(markup) do |markup|
-        raise ArgumentError, "unexpected empty expression" if markup.empty?
-
-        if (markup.start_with?('"') && markup.end_with?('"')) ||
-           (markup.start_with?("'") && markup.end_with?("'"))
-          markup
-        else
-          case markup
-          when INTEGERS_REGEX
-            markup
-          when RANGES_REGEX
-            match = Regexp.last_match
-            new_start, new_end = RangeLookup.lax_migrate(match[1], match[2])
-            Utils.match_captures_replace(match, 1 => new_start, 2 => new_end)
-          when FLOATS_REGEX
-            # lax parser allowed multiple periods, but the second period and following characters were ignored
-            new_markup = markup.slice(/\A(-?\d+\.\d*)/)
-            new_markup << "0" if new_markup.end_with?(".")
-            new_markup
-          else
-            if LITERALS.key?(markup)
-              markup
-            else
-              VariableLookup.lax_migrate(markup)
-            end
-          end
-        end
-      end
-    end
   end
 end

lib/liquid/parse_context.rb

@@ -5,9 +5,6 @@ module Liquid
     attr_accessor :locale, :line_number, :trim_whitespace, :depth
     attr_reader :partial, :warnings, :error_mode
 
-    # @api private
-    attr_writer :error_mode
-
     def initialize(options = {})
       @template_options = options ? options.dup : {}
 

lib/liquid/parser_switching.rb

@@ -2,30 +2,6 @@
 
 module Liquid
   module ParserSwitching
-    module ClassMethods
-      def migrate_with_selected_parser(tag_name, markup, tokenizer, parse_context)
-        case parse_context.error_mode
-        when :strict then strict_migrate(tag_name, markup, tokenizer, parse_context)
-        when :lax    then lax_migrate(tag_name, markup, tokenizer, parse_context)
-        when :warn
-          begin
-            parse_context.error_mode = :strict
-            begin
-              # Use exception side effect to conditionally branch to lax migration
-              parse(tag_name, markup, tokenizer, parse_context)
-            ensure
-              parse_context.error_mode = :warn
-            end
-
-            strict_migrate(tag_name, markup, tokenizer, parse_context)
-          rescue SyntaxError => e
-            parse_context.warnings << e
-            lax_migrate(tag_name, markup, tokenizer, parse_context)
-          end
-        end
-      end
-    end
-
     def strict_parse_with_error_mode_fallback(markup)
       strict_parse_with_error_context(markup)
     rescue SyntaxError => e

lib/liquid/range_lookup.rb

@@ -22,37 +22,6 @@ module Liquid
       end
     end
 
-    def self.lax_migrate(start_markup, end_markup)
-      new_start = Expression.lax_migrate(start_markup)
-      new_end = Expression.lax_migrate(end_markup)
-
-      # cast literals
-      start_obj = Expression.parse(new_start)
-      end_obj   = Expression.parse(new_end)
-      if start_obj.respond_to?(:evaluate) || end_obj.respond_to?(:evaluate)
-        new_start = lax_migrate_range_expression(new_start, start_obj)
-        new_end = lax_migrate_range_expression(new_end, end_obj)
-      else
-        new_start = start_obj.to_i.to_s unless start_obj.is_a?(Integer)
-        new_end = end_obj.to_i.to_s unless end_obj.is_a?(Integer)
-      end
-
-      [new_start, new_end]
-    end
-
-    def self.lax_migrate_range_expression(markup, expression)
-      return markup if expression.respond_to?(:evaluate)
-
-      case expression
-      when Integer
-        markup
-      when NilClass, String
-        expression.to_i.to_s
-      else
-        Utils.to_integer(input).to_s
-      end
-    end
-
     attr_reader :start_obj, :end_obj
 
     def initialize(start_obj, end_obj)

lib/liquid/standardfilters.rb

@@ -757,7 +757,7 @@ module Liquid
     # @liquid_type filter
     # @liquid_category math
     # @liquid_summary
-    #   Divides a number by a given number.
+    #   Divides a number by a given number. The `divided_by` filter produces a result of the same type as the divisor. This means if you divide by an integer, the result will be an integer, and if you divide by a float, the result will be a float.
     # @liquid_syntax number | divided_by: number
     # @liquid_return [number]
     def divided_by(input, operand)

lib/liquid/tag.rb

@@ -7,8 +7,6 @@ module Liquid
     include ParserSwitching
 
     class << self
-      include ParserSwitching::ClassMethods
-
       def parse(tag_name, markup, tokenizer, parse_context)
         tag = new(tag_name, markup, parse_context)
         tag.parse(tokenizer)

lib/liquid/tags/assign.rb

@@ -21,18 +21,6 @@ module Liquid
       raise Liquid::SyntaxError, parse_context.locale.t('errors.syntax.assign')
     end
 
-    def self.migrate(tag_name, markup, tokenizer, parse_context)
-      match = markup.match(Syntax)
-      new_variable_markup = Variable.migrate(match[2], parse_context)
-      new_markup = Utils.match_captures_replace(match, 2 => new_variable_markup)
-
-      # replace scanned over characters with a space to ensure there is a space
-      # to separate the tag name and the variable name
-      new_markup.prepend(" ") if match.begin(0) > 0
-
-      new_markup
-    end
-
     attr_reader :to, :from
 
     def initialize(tag_name, markup, parse_context)

lib/liquid/tags/break.rb

@@ -21,10 +21,6 @@ module Liquid
   class Break < Tag
     INTERRUPT = BreakInterrupt.new.freeze
 
-    def self.migrate(_tag_name, _markup, _tokenizer, _parse_context)
-      "" # markup was ignored
-    end
-
     def render_to_output_buffer(context, output)
       context.push_interrupt(INTERRUPT)
       output

lib/liquid/tags/capture.rb

@@ -18,21 +18,6 @@ module Liquid
   class Capture < Block
     Syntax = /(#{VariableSignature}+)/o
 
-    def self.migrate(tag_name, markup, tokenizer, parse_context)
-      match = markup.match(Syntax)
-
-      new_markup = match[1]
-
-      # replace scanned over characters with a space to ensure there is a space
-      # to separate the tag name and the variable name
-      new_markup.prepend(" ") if match.begin(0) > 0
-
-      new_body, unknown_tag = migrate_body(tag_name, tokenizer, parse_context)
-      raise SyntaxError if unknown_tag
-
-      [new_markup, new_body]
-    end
-
     def initialize(tag_name, markup, options)
       super
       if markup =~ Syntax

lib/liquid/tags/case.rb

@@ -28,20 +28,6 @@ module Liquid
 
     attr_reader :blocks, :left
 
-    def self.migrate(tag_name, markup, tokenizer, parse_context)
-      match = markup.match(/\s*#{Syntax}\s*/o) || raise(SyntaxError)
-
-      new_expression = Expression.lax_migrate(match[1])
-      new_markup = Utils.match_captures_replace(match, { 1 => new_expression })
-
-      # replace scanned over characters with a space to ensure there is a space
-      # to separate the tag name and the variable name
-      new_markup.prepend(" ") if match.begin(0) > 0
-
-      new_body = migrate_body(tag_name, tokenizer, parse_context)
-      [new_markup, new_body]
-    end
-
     def initialize(tag_name, markup, options)
       super
       @blocks = []
@@ -53,37 +39,6 @@ module Liquid
       end
     end
 
-    def self.migrate_body(start_tag_name, tokenizer, parse_context)
-      result = +""
-
-      # body before first `when` delimiter tag is ignored
-      unused_body, delimiter_tag = super(start_tag_name, tokenizer, parse_context)
-      unless delimiter_tag
-        raise NotImplementedError, "TODO: migrate `case` tag with no `when` or `else` tags"
-      end
-
-      result << Utils.migrate_stripped(unused_body) { "" } # just keep whitespace (e.g. newline and indent)
-
-      while delimiter_tag
-        break unless delimiter_tag
-
-        case delimiter_tag.tag_name
-        when "when"
-          new_markup = migrate_when_markup(delimiter_tag.markup)
-          result << delimiter_tag.replaced_markup(new_markup)
-        when "else"
-          result << delimiter_tag.original_tag_string
-        else
-          raise SyntaxError
-        end
-
-        new_body, delimiter_tag = super(start_tag_name, tokenizer, parse_context)
-        result << new_body
-      end
-
-      result
-    end
-
     def parse(tokens)
       body = case_body = new_body
       body = @blocks.last.attachment while parse_body(body, tokens)
@@ -136,25 +91,6 @@ module Liquid
 
     private
 
-    private_class_method def self.migrate_when_markup(unstripped_markup)
-      Utils.migrate_stripped(unstripped_markup) do |markup|
-        match = markup.match(WhenSyntax) || raise(SyntaxError)
-
-        replacements = { 1 => Expression.lax_migrate(match[1]) }
-        if (right = match[2])
-          replacements[2] = migrate_when_markup(right)
-        end
-
-        new_markup = Utils.match_captures_replace(match, replacements)
-
-        # replace scanned over characters with a space to ensure there is a space
-        # to separate the tag name and the variable name
-        new_markup.prepend(" ") if match.begin(0) > 0
-
-        new_markup
-      end
-    end
-
     def record_when_condition(markup)
       body = new_body
 

lib/liquid/tags/comment.rb

@@ -15,24 +15,6 @@ module Liquid
   #   {% endcomment %}
   # @liquid_syntax_keyword content The content of the comment.
   class Comment < Block
-    def self.migrate(tag_name, _markup, tokenizer, parse_context)
-      new_markup = "" # markup was ignored
-      new_body = migrate_body(tag_name, tokenizer, parse_context)
-      [new_markup, new_body]
-    end
-
-    def self.migrate_body(start_tag_name, tokenizer, parse_context)
-      result = +""
-      loop do
-        new_body, delimiter_tag = super(start_tag_name, tokenizer, parse_context)
-        result << new_body
-        break unless delimiter_tag
-
-        result << delimiter_tag.original_tag_string # unknown tags allowed
-      end
-      result
-    end
-
     def render_to_output_buffer(_context, output)
       output
     end

lib/liquid/tags/continue.rb

@@ -12,10 +12,6 @@ module Liquid
   class Continue < Tag
     INTERRUPT = ContinueInterrupt.new.freeze
 
-    def self.migrate(_tag_name, _markup, _tokenizer, _parse_context)
-      "" # markup was ignored
-    end
-
     def render_to_output_buffer(context, output)
       context.push_interrupt(INTERRUPT)
       output

lib/liquid/tags/cycle.rb

@@ -18,43 +18,8 @@ module Liquid
     SimpleSyntax = /\A#{QuotedFragment}+/o
     NamedSyntax  = /\A(#{QuotedFragment})\s*\:\s*(.*)/om
 
-    def self.migrate(tag_name, markup, tokenizer, parse_context)
-      new_markup = case markup
-      when NamedSyntax
-        match = Regexp.last_match
-
-        new_name_syntax = Expression.lax_migrate(match[1])
-        new_variables_markup = migrate_variables_from_string(match[2])
-
-        Utils.match_captures_replace(match, 1 => new_name_syntax, 2 => new_variables_markup)
-      when SimpleSyntax
-        match = Regexp.last_match
-        migrate_variables_from_string(markup)
-      else
-        raise SyntaxError
-      end
-
-      # replace scanned over characters with a space to ensure there is a space
-      # to separate the tag name and the variable name
-      new_markup.prepend(" ") if match.begin(0) > 0
-
-      new_markup
-    end
-
-    def self.migrate_variables_from_string(markup)
-      markup.split(',').collect do |var|
-        match = var.match(/\s*(#{QuotedFragment})\s*/o)
-        if match
-          Utils.match_captures_replace(match, 1 => Expression.lax_migrate(match[1]))
-        end
-      end.compact.join(",")
-    end
-
     attr_reader :variables
 
-    # @api private
-    attr_reader :name
-
     def initialize(tag_name, markup, options)
       super
       case markup

lib/liquid/tags/decrement.rb

@@ -21,10 +21,6 @@ module Liquid
   class Decrement < Tag
     attr_reader :variable_name
 
-    def self.migrate(_tag_name, markup, _tokenizer, _parse_context)
-      markup # no characters ignored, it just uses anything for the variable name
-    end
-
     def initialize(tag_name, markup, options)
       super
       @variable_name = markup.strip

lib/liquid/tags/echo.rb

@@ -21,10 +21,6 @@ module Liquid
   class Echo < Tag
     attr_reader :variable
 
-    def self.migrate(tag_name, markup, tokenizer, parse_context)
-      Variable.migrate(markup, parse_context)
-    end
-
     def initialize(tag_name, markup, parse_context)
       super
       @variable = Variable.new(markup, parse_context)

lib/liquid/tags/for.rb

@@ -25,16 +25,10 @@ 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
+    Syntax = /\A(#{VariableSegment}+)\s+in\s+(#{QuotedFragment}+)\s*(reversed)?/o
 
     attr_reader :collection_name, :variable_name, :limit, :from
 
-    def self.migrate(tag_name, markup, tokenizer, parse_context)
-      new_markup = migrate_with_selected_parser(tag_name, markup, tokenizer, parse_context)
-      new_body = migrate_body(tag_name, tokenizer, parse_context)
-      [new_markup, new_body]
-    end
-
     def initialize(tag_name, markup, options)
       super
       @from = @limit = nil
@@ -59,27 +53,6 @@ module Liquid
       @else_block ? [@for_block, @else_block] : [@for_block]
     end
 
-    def self.migrate_body(start_tag_name, tokenizer, parse_context)
-      result = +""
-
-      new_body, delimiter_tag = super(start_tag_name, tokenizer, parse_context)
-      result << new_body
-
-      else_tag = delimiter_tag
-      else_body = nil
-      while delimiter_tag
-        raise SyntaxError unless delimiter_tag.tag_name == 'else'
-        else_tag = delimiter_tag
-        else_body, delimiter_tag = super(start_tag_name, tokenizer, parse_context)
-      end
-      if else_tag
-        result << else_tag.replaced_markup("") # markup was ignored in else tags
-        result << else_body
-      end
-
-      result
-    end
-
     def unknown_tag(tag, markup, tokens)
       return super unless tag == 'else'
       @else_block = new_body
@@ -99,13 +72,6 @@ module Liquid
 
     protected
 
-    private_class_method def self.lax_migrate(tag_name, markup, tokenizer, parse_context)
-      match = markup.match(Syntax) || raise(SyntaxError)
-      new_collection_name = Expression.lax_migrate(match[2])
-      new_markup = Utils.match_captures_replace(match, { 2 => new_collection_name }.compact)
-      new_markup << Utils.migrate_tag_attributes(markup)
-    end
-
     def lax_parse(markup)
       if markup =~ Syntax
         @variable_name   = Regexp.last_match(1)
@@ -121,10 +87,6 @@ module Liquid
       end
     end
 
-    private_class_method def self.strict_migrate(tag_name, markup, tokenizer, parse_context)
-      markup
-    end
-
     def strict_parse(markup)
       p = Parser.new(markup)
       @variable_name = p.consume(:id)

lib/liquid/tags/if.rb

@@ -20,39 +20,6 @@ module Liquid
 
     attr_reader :blocks
 
-    def self.migrate(tag_name, markup, tokenizer, parse_context)
-      new_markup = migrate_with_selected_parser(tag_name, markup, tokenizer, parse_context)
-      new_body = migrate_body(tag_name, tokenizer, parse_context)
-      [new_markup, new_body]
-    end
-
-    def self.migrate_body(start_tag_name, tokenizer, parse_context)
-      result = +""
-
-      loop do
-        new_body, delimiter_tag = super(start_tag_name, tokenizer, parse_context)
-        result << new_body
-
-        break unless delimiter_tag
-
-        case delimiter_tag.tag_name
-        when "else"
-          result << delimiter_tag.replaced_markup("") # markup was ignored on else tags
-        when "elsif"
-          new_markup = migrate_with_selected_parser(delimiter_tag.tag_name, delimiter_tag.markup, tokenizer, parse_context)
-          result << delimiter_tag.replaced_markup(new_markup)
-        else
-          raise SyntaxError
-        end
-      end
-
-      result
-    end
-
-    private_class_method def self.strict_migrate(tag_name, markup, tokenizer, parse_context)
-      markup
-    end
-
     def initialize(tag_name, markup, options)
       super
       @blocks = []
@@ -114,10 +81,6 @@ module Liquid
       Condition.parse_expression(parse_context, markup)
     end
 
-    private_class_method def self.lax_migrate_expression(markup)
-      Expression.lax_migrate(markup)
-    end
-
     def lax_parse(markup)
       expressions = markup.scan(ExpressionsAndOperators)
       raise SyntaxError, options[:locale].t("errors.syntax.if") unless expressions.pop =~ Syntax
@@ -138,40 +101,6 @@ module Liquid
       condition
     end
 
-    private_class_method def self.lax_migrate(tag_name, markup, tokenizer, parse_context)
-      expressions = markup.scan(ExpressionsAndOperators)
-
-      new_markup = lax_migrate_condition(expressions.pop)
-      until expressions.empty?
-        operator = expressions.pop
-        new_left_markup = lax_migrate_condition(expressions.pop)
-        new_markup = new_left_markup << operator << new_markup
-      end
-
-      new_markup
-    end
-
-    private_class_method def self.lax_migrate_condition(markup)
-      Utils.migrate_stripped(markup) do |markup|
-        match = markup.match(Syntax)
-        left = lax_migrate_expression(match[1])
-        op = match[2]
-        right_capture = match[3]
-        if op
-          right = lax_migrate_expression(right_capture) if right_capture
-        elsif right_capture
-          right = "" # remove right capture, since it is ignored with no operator
-        end
-        new_markup = Utils.match_captures_replace(match, { 1 => left, 2 => op, 3 => right }.compact)
-        new_markup.prepend(' ') if match.begin(0) > 0
-        new_markup << ' ' if match.end(0) < markup.length
-        if op && !right # missing right operand missing
-          new_markup << " nil" # replace with nil, which it was semantically treated as
-        end
-        new_markup
-      end
-    end
-
     def strict_parse(markup)
       p = Parser.new(markup)
       condition = parse_binary_comparisons(p)

lib/liquid/tags/ifchanged.rb

@@ -2,15 +2,6 @@
 
 module Liquid
   class Ifchanged < Block
-    def self.migrate(tag_name, _markup, tokenizer, parse_context)
-      new_markup = "" # markup was ignored
-
-      new_body, unknown_tag = migrate_body(tag_name, tokenizer, parse_context)
-      raise SyntaxError if unknown_tag
-
-      [new_markup, new_body]
-    end
-
     def render_to_output_buffer(context, output)
       block_output = +''
       super(context, block_output)

lib/liquid/tags/include.rb

@@ -25,22 +25,6 @@ module Liquid
 
     attr_reader :template_name_expr, :variable_name_expr, :attributes
 
-    def self.migrate(_tag_name, markup, _tokenizer, parse_context)
-      match = markup.match(SYNTAX) || raise(SyntaxError)
-
-      template_name = Expression.lax_migrate(match[1])
-      variable_name = Expression.lax_migrate(match[3]) if match[3]
-
-      new_markup = Utils.match_captures_replace(match, { 1 => template_name, 3 => variable_name }.compact)
-      new_markup << Utils.migrate_tag_attributes(markup)
-
-      # replace scanned over characters with a space to ensure there is a space
-      # to separate the tag name and the variable name
-      new_markup.prepend(" ") if match.begin(0) > 0
-
-      new_markup
-    end
-
     def initialize(tag_name, markup, options)
       super
 

lib/liquid/tags/increment.rb

@@ -21,10 +21,6 @@ module Liquid
   class Increment < Tag
     attr_reader :variable_name
 
-    def self.migrate(_tag_name, markup, _tokenizer, _parse_context)
-      markup # no characters ignored, it just uses anything for the variable name
-    end
-
     def initialize(tag_name, markup, options)
       super
       @variable_name = markup.strip

lib/liquid/tags/inline_comment.rb

@@ -2,10 +2,6 @@
 
 module Liquid
   class InlineComment < Tag
-    def self.migrate(_tag_name, markup, _tokenizer, _parse_context)
-      markup
-    end
-
     def initialize(tag_name, markup, options)
       super
 

lib/liquid/tags/raw.rb

@@ -16,31 +16,6 @@ module Liquid
     Syntax = /\A\s*\z/
     FullTokenPossiblyInvalid = /\A(.*)#{TagStart}\s*(\w+)\s*(.*)?#{TagEnd}\z/om
 
-    def self.migrate(tag_name, markup, tokenizer, parse_context)
-      raise SyntaxError unless Syntax.match?(markup)
-
-      new_body = migrate_body(tag_name, tokenizer, parse_context)
-      [markup, new_body]
-    end
-
-    def self.migrate_body(start_tag_name, tokenizer, parse_context)
-      block_delimiter = "end#{start_tag_name}"
-
-      body = +''
-      while (token = tokenizer.shift)
-        match = token.match(FullTokenPossiblyInvalid)
-        if match && block_delimiter == match[2]
-          body << Utils.match_captures_replace(match, { 3 => "" })
-          return body
-        end
-        body << token unless token.empty?
-      end
-
-      raise SyntaxError
-    end
-
-    attr_reader :body
-
     def initialize(tag_name, markup, parse_context)
       super
 

lib/liquid/tags/render.rb

@@ -29,22 +29,6 @@ module Liquid
     FOR = 'for'
     SYNTAX = /(#{QuotedString}+)(\s+(with|#{FOR})\s+(#{QuotedFragment}+))?(\s+(?:as)\s+(#{VariableSegment}+))?/o
 
-    def self.migrate(_tag_name, markup, _tokenizer, parse_context)
-      match = markup.match(SYNTAX)
-
-      template_name = Expression.lax_migrate(match[1])
-      variable_name = Expression.lax_migrate(match[4]) if match[4]
-
-      new_markup = Utils.match_captures_replace(match, { 1 => template_name, 4 => variable_name }.compact)
-      new_markup << Utils.migrate_tag_attributes(markup)
-
-      # replace scanned over characters with a space to ensure there is a space
-      # to separate the tag name and the variable name
-      new_markup.prepend(" ") if match.begin(0) > 0
-
-      new_markup
-    end
-
     disable_tags "include"
 
     attr_reader :template_name_expr, :variable_name_expr, :attributes, :alias_name

lib/liquid/tags/table_row.rb

@@ -26,20 +26,6 @@ module Liquid
   class TableRow < Block
     Syntax = /(\w+)\s+in\s+(#{QuotedFragment}+)/o
 
-    def self.migrate(tag_name, markup, tokenizer, parse_context)
-      match = markup.match(Syntax) || raise(SyntaxError)
-
-      new_collection_name = Expression.lax_migrate(match[2])
-
-      new_markup = Utils.match_captures_replace(match, { 2 => new_collection_name }.compact)
-      new_markup << Utils.migrate_tag_attributes(markup)
-
-      new_body, unknown_tag = migrate_body(tag_name, tokenizer, parse_context)
-      raise SyntaxError if unknown_tag
-
-      [new_markup, new_body]
-    end
-
     attr_reader :variable_name, :collection_name, :attributes
 
     def initialize(tag_name, markup, options)
@@ -59,13 +45,13 @@ module Liquid
     def render_to_output_buffer(context, output)
       (collection = context.evaluate(@collection_name)) || (return '')
 
-      from = @attributes.key?('offset') ? context.evaluate(@attributes['offset']).to_i : 0
-      to   = @attributes.key?('limit')  ? from + context.evaluate(@attributes['limit']).to_i : nil
+      from = @attributes.key?('offset') ? to_integer(context.evaluate(@attributes['offset'])) : 0
+      to = @attributes.key?('limit') ? from + to_integer(context.evaluate(@attributes['limit'])) : nil
 
       collection = Utils.slice_collection(collection, from, to)
       length     = collection.length
 
-      cols = @attributes.key?('cols') ? context.evaluate(@attributes['cols']).to_i : length
+      cols = @attributes.key?('cols') ? to_integer(context.evaluate(@attributes['cols'])) : length
 
       output << "<tr class=\"row1\">\n"
       context.stack do
@@ -96,6 +82,14 @@ module Liquid
         super + @node.attributes.values + [@node.collection_name]
       end
     end
+
+    private
+
+    def to_integer(value)
+      value.to_i
+    rescue NoMethodError
+      raise Liquid::ArgumentError, "invalid integer"
+    end
   end
 
   Template.register_tag('tablerow', TableRow)

lib/liquid/template.rb

@@ -96,11 +96,6 @@ module Liquid
       def parse(source, options = {})
         new.parse(source, options)
       end
-
-      def migrate(source, parse_options = {})
-        parse(source, parse_options) # raise if source has syntax errors
-        new.migrate(source, parse_options)
-      end
     end
 
     def initialize
@@ -117,12 +112,6 @@ module Liquid
       self
     end
 
-    def migrate(source, parse_options = {})
-      parse_context = configure_options(parse_options.merge(disable_liquid_c_nodes: true))
-      tokenizer     = parse_context.new_tokenizer(source, start_line_number: @line_numbers && 1)
-      Document.migrate(tokenizer, parse_context)
-    end
-
     def registers
       @registers ||= {}
     end

lib/liquid/tokenizer.rb

@@ -25,10 +25,6 @@ module Liquid
       token
     end
 
-    def more?
-      @offset < @tokens.length
-    end
-
     private
 
     def tokenize

lib/liquid/utils.rb

@@ -16,7 +16,11 @@ module Liquid
 
       # Maintains Ruby 1.8.7 String#each behaviour on 1.9
       if collection.is_a?(String)
-        return collection.empty? ? [] : [collection]
+        return [] if collection.empty?
+        if from > 0 || to == 0
+          Usage.increment("string_slice_bug")
+        end
+        return [collection]
       end
       return [] unless collection.respond_to?(:each)
 
@@ -89,48 +93,5 @@ module Liquid
       # Otherwise return the object itself
       obj
     end
-
-    def self.migrate_stripped(markup)
-      match = markup.match(/\A\s*(.*?)\s*\z/m)
-      new_markup = yield match[1]
-      Utils.match_captures_replace(match, 1 => new_markup)
-    end
-
-    def self.migrate_tag_attributes(markup)
-      attributes = []
-      markup.scan(/\s*,?\s*#{TagAttributes}/) do
-        tag_match = Regexp.last_match
-        new_value_markup = Expression.lax_migrate(tag_match[2])
-        attribute_markup = Utils.match_captures_replace(tag_match, { 2 => new_value_markup })
-        unless attribute_markup.match?(/\A[,\s]/)
-          attribute_markup.prepend(", ")
-        end
-        attributes << attribute_markup
-      end
-      return "" if attributes.empty?
-      attributes.join
-    end
-
-    # @api private
-    def self.match_capture_replace(match, capture_number, replacement_string)
-      match_captures_replace(match, { capture_number => replacement_string })
-    end
-
-    def self.match_captures_replace(match, replacements = {})
-      new_string = match[0].dup
-      capture_numbers = replacements.keys
-      unless capture_numbers.all?(Integer)
-        raise TypeError, "Currently, only numbered captures are supported"
-      end
-      # replace from later captures first, to avoid affecting the position for following replacements
-      match_begin = match.begin(0)
-      capture_numbers.sort.reverse_each do |capture_number|
-        replacement_string = replacements.fetch(capture_number)
-        capture_start = match.begin(capture_number)
-        capture_length = match.end(capture_number) - capture_start
-        new_string[capture_start - match_begin, capture_length] = replacement_string
-      end
-      new_string
-    end
   end
 end

lib/liquid/variable.rb

@@ -41,79 +41,6 @@ module Liquid
       "in \"{{#{markup}}}\""
     end
 
-    STRICT_PARSE_CONTEXT = ParseContext.new(error_mode: :strict)
-    private_constant :STRICT_PARSE_CONTEXT
-
-    def self.migrate(markup, parse_context)
-      new(markup, STRICT_PARSE_CONTEXT)
-      # TODO: migrate non-integer range expression literals
-      markup
-    rescue Liquid::SyntaxError
-      raise if parse_context.error_mode == :strict
-      lax_migrate(markup, parse_context)
-    end
-
-    def self.lax_migrate(markup, parse_context)
-      # unanchored match that may skip over characters preceding the name expression
-      markup_match = markup.match(MarkupWithQuotedFragment)
-      unless markup_match
-        # Treated as a blank variable (e.g. `{{ -}}`), which outputs nothing
-        # but may still have an effect on whitespace trimming
-        return ""
-      end
-
-      name_markup = markup_match[1]
-      filters_markup = markup_match[2]
-
-      new_name_markup = Expression.lax_migrate(name_markup)
-
-      new_filter_markup = ""
-      # unanchored match that may skip over characters preceding the pipe for the first filter
-      if (filters_match = filters_markup.match(/\s*#{FilterMarkupRegex}/o))
-        filters = filters_match[1].scan(FilterParser) # may skip over unterminated quote characters
-        filters.map! do |f|
-          filter_match = f.match(/\A(\s*)\W*(\w+)(\s*)/)
-          next unless filter_match
-
-          # omit non-word characters preceding the filter name that the lax parser skips over
-          transformed_filter = +"#{filter_match[1]}#{filter_match[2]}#{filter_match[3]}"
-
-          filter_args = []
-          f.scan(/#{FilterArgsRegex}\s*/o) do # may skip over characters before the argument separator
-            filter_arg_match = Regexp.last_match
-            new_filter_arg = lax_migrate_filter_argument(filter_arg_match[1])
-            filter_arg_string = Utils.match_captures_replace(filter_arg_match, 1 => new_filter_arg)
-            filter_arg_string = filter_arg_string[1...] # remove separator character
-            filter_args << filter_arg_string
-          end
-
-          unless filter_args.empty?
-            transformed_filter << ":" << filter_args.join(",")
-          end
-
-          transformed_filter
-        end
-        filters.compact!
-        new_filters_markup = filters.join('|')
-
-        # include pipe separator along with whitespace surrounding it
-        new_filter_markup = Utils.match_captures_replace(filters_match, 1 => new_filters_markup)
-      end
-
-      new_markup = Utils.match_captures_replace(markup_match, 1 => new_name_markup, 2 => new_filter_markup)
-      new_markup.prepend(" ") if markup_match.begin(0) > 0
-      new_markup
-    end
-
-    def self.lax_migrate_filter_argument(unparsed_arg)
-      if (match = unparsed_arg.match(JustTagAttributes))
-        new_value_markup = Expression.lax_migrate(match[2])
-        Utils.match_captures_replace(match, 2 => new_value_markup)
-      else
-        Expression.lax_migrate(unparsed_arg)
-      end
-    end
-
     def lax_parse(markup)
       @filters = []
       return unless markup =~ MarkupWithQuotedFragment

lib/liquid/variable_lookup.rb

@@ -10,43 +10,6 @@ module Liquid
       new(markup)
     end
 
-    LITERALS = Expression::LITERALS.keys.freeze
-    private_constant :LITERALS
-
-    def self.lax_migrate(markup)
-      new_markup = nil
-      last_match = nil
-      first_match = nil
-      markup.scan(VariableParser) do |lookup|
-        last_match = Regexp.last_match
-        first_match ||= last_match
-        new_markup ||= +""
-        if lookup&.start_with?('[') && lookup&.end_with?(']')
-          new_markup << "[" << Expression.lax_migrate(lookup[1..-2]) << "]"
-        elsif !lookup.match?(/\A#{Liquid::Lexer::IDENTIFIER}\z/)
-          # quote non-strictly valid identifiers
-          new_markup << "['" << lookup << "']"
-        else
-          new_markup << "." unless new_markup.empty?
-          new_markup << lookup
-        end
-      end
-
-      case new_markup
-      when nil
-        # `markup.scan(VariableParser)` may skip over all characters
-        new_markup ||= " nil "
-      when Expression::INTEGERS_REGEX, Expression::RANGES_REGEX, Expression::FLOATS_REGEX, *LITERALS
-        # Quote variable lookups that match literals after characters are skipped by regex scanning
-        new_markup = "['#{new_markup}']"
-      else
-        new_markup.prepend(" ") if first_match.begin(0) > 0
-        new_markup << ' ' if last_match.end(0) < markup.length
-      end
-
-      new_markup
-    end
-
     def initialize(markup)
       lookups = markup.scan(VariableParser)
 

test/integration/tags/for_tag_test.rb

@@ -103,6 +103,7 @@ HERE
     assert_template_result('1234', '{%for i in array limit:4 %}{{ i }}{%endfor%}', assigns)
     assert_template_result('3456', '{%for i in array limit:4 offset:2 %}{{ i }}{%endfor%}', assigns)
     assert_template_result('3456', '{%for i in array limit: 4 offset: 2 %}{{ i }}{%endfor%}', assigns)
+    assert_template_result('3456', '{%for i in array, limit: 4, offset: 2 %}{{ i }}{%endfor%}', assigns)
   end
 
   def test_limiting_with_invalid_limit

test/integration/tags/table_row_test.rb

@@ -80,6 +80,32 @@ class TableRowTest < Minitest::Test
       { "var" => nil })
   end
 
+  def test_nil_limit_is_treated_as_zero
+    expect = "<tr class=\"row1\">\n" \
+      "</tr>\n"
+
+    assert_template_result(expect,
+      "{% tablerow i in (1..2) limit:nil %}{{ i }}{% endtablerow %}")
+
+    assert_template_result(expect,
+      "{% tablerow i in (1..2) limit:var %}{{ i }}{% endtablerow %}",
+      { "var" => nil })
+  end
+
+  def test_nil_offset_is_treated_as_zero
+    expect = "<tr class=\"row1\">\n" \
+      "<td class=\"col1\">1:false</td>" \
+      "<td class=\"col2\">2:true</td>" \
+      "</tr>\n"
+
+    assert_template_result(expect,
+      "{% tablerow i in (1..2) offset:nil %}{{ i }}:{{ tablerowloop.col_last }}{% endtablerow %}")
+
+    assert_template_result(expect,
+      "{% tablerow i in (1..2) offset:var %}{{ i }}:{{ tablerowloop.col_last }}{% endtablerow %}",
+      { "var" => nil })
+  end
+
   def test_tablerow_loop_drop_attributes
     template = <<~LIQUID.chomp
       {% tablerow i in (1...2) %}
@@ -131,4 +157,24 @@ class TableRowTest < Minitest::Test
 
     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 %}',
+      render_errors: true,
+    )
+
+    assert_template_result(
+      "Liquid error (line 1): invalid integer",
+      '{% tablerow n in (1...10) offset:true %} {{n}} {% endtablerow %}',
+      render_errors: true,
+    )
+
+    assert_template_result(
+      "Liquid error (line 1): invalid integer",
+      '{% tablerow n in (1...10) cols:true %} {{n}} {% endtablerow %}',
+      render_errors: true,
+    )
+  end
 end

test/integration/variable_test.rb

@@ -24,17 +24,22 @@ class VariableTest < Minitest::Test
 
   def test_if_tag_calls_to_liquid_value
     assert_template_result('one', '{% if foo == 1 %}one{% endif %}', { 'foo' => IntegerDrop.new('1') })
+    assert_template_result('one', '{% if foo == eqv %}one{% endif %}', { 'foo' => IntegerDrop.new(1), 'eqv' => IntegerDrop.new(1) })
     assert_template_result('one', '{% if 0 < foo %}one{% endif %}', { 'foo' => IntegerDrop.new('1') })
     assert_template_result('one', '{% if foo > 0 %}one{% endif %}', { 'foo' => IntegerDrop.new('1') })
+    assert_template_result('one', '{% if b > a %}one{% endif %}', { 'b' => IntegerDrop.new(1), 'a' => IntegerDrop.new(0) })
     assert_template_result('true', '{% if foo == true %}true{% endif %}', { 'foo' => BooleanDrop.new(true) })
     assert_template_result('true', '{% if foo %}true{% endif %}', { 'foo' => BooleanDrop.new(true) })
 
     assert_template_result('', '{% if foo %}true{% endif %}', { 'foo' => BooleanDrop.new(false) })
     assert_template_result('', '{% if foo == true %}True{% endif %}', { 'foo' => BooleanDrop.new(false) })
+
+    assert_template_result('one', '{% if a contains x %}one{% endif %}', { 'a' => [1], 'x' => IntegerDrop.new(1) })
   end
 
   def test_unless_tag_calls_to_liquid_value
     assert_template_result('', '{% unless foo %}true{% endunless %}', { 'foo' => BooleanDrop.new(true) })
+    assert_template_result('true', '{% unless foo %}true{% endunless %}', { 'foo' => BooleanDrop.new(false) })
   end
 
   def test_case_tag_calls_to_liquid_value
@@ -130,4 +135,38 @@ class VariableTest < Minitest::Test
   def test_raw_value_variable
     assert_template_result('bar', '{{ [key] }}', { 'key' => 'foo', 'foo' => 'bar' })
   end
+
+  def test_dynamic_find_var_with_drop
+    assert_template_result(
+      'bar',
+      '{{ [list[settings.zero]] }}',
+      {
+        'list' => ['foo'],
+        'settings' => SettingsDrop.new("zero" => 0),
+        'foo' => 'bar',
+      }
+    )
+
+    assert_template_result(
+      'foo',
+      '{{ [list[settings.zero]["foo"]] }}',
+      {
+        'list' => [{ 'foo' => 'bar' }],
+        'settings' => SettingsDrop.new("zero" => 0),
+        'bar' => 'foo',
+      }
+    )
+  end
+
+  def test_double_nested_variable_lookup
+    assert_template_result(
+      'bar',
+      '{{ list[list[settings.zero]]["foo"] }}',
+      {
+        'list' => [1, { 'foo' => 'bar' }],
+        'settings' => SettingsDrop.new("zero" => 0),
+        'bar' => 'foo',
+      }
+    )
+  end
 end

test/test_helper.rb

@@ -49,27 +49,6 @@ module Minitest
       assert_equal(expected, output, message)
     end
 
-    if ENV['LIQUID_MIGRATOR']
-      puts "-- Liquid Migrator Enabled"
-
-      alias_method(:assert_template_result_without_migrator, :assert_template_result)
-
-      def assert_template_result(expected, source, assigns = {}, error_mode: nil, partials: nil, **kwargs)
-        migrated_source = Liquid::Template.migrate(source, line_numbers: true, error_mode: error_mode&.to_sym)
-        assert_no_migration(migrated_source)
-        if partials
-          migrated_partials = {}
-          partials.each do |name, partial|
-            new_partial = Liquid::Template.migrate(partial, line_numbers: true, error_mode: error_mode&.to_sym)
-            assert_no_migration(new_partial)
-            migrated_partials[name] = new_partial
-          end
-        end
-        assert_template_result_without_migrator(expected, migrated_source, assigns,
-          error_mode: 'strict', partials: migrated_partials, **kwargs)
-      end
-    end
-
     def assert_match_syntax_error(match, template, error_mode: nil)
       exception = assert_raises(Liquid::SyntaxError) do
         Template.parse(template, line_numbers: true, error_mode: error_mode&.to_sym).render
@@ -81,10 +60,6 @@ module Minitest
       assert_match_syntax_error("", template, error_mode: error_mode)
     end
 
-    def assert_no_migration(source)
-      assert_equal(source, Liquid::Template.migrate(source))
-    end
-
     def assert_usage_increment(name, times: 1)
       old_method = Liquid::Usage.method(:increment)
       calls = 0
@@ -150,16 +125,23 @@ class ThingWithToLiquid
   end
 end
 
+class SettingsDrop < Liquid::Drop
+  def initialize(settings)
+    super()
+    @settings = settings
+  end
+
+  def liquid_method_missing(key)
+    @settings[key]
+  end
+end
+
 class IntegerDrop < Liquid::Drop
   def initialize(value)
     super()
     @value = value.to_i
   end
 
-  def ==(other)
-    @value == other
-  end
-
   def to_s
     @value.to_s
   end
@@ -175,10 +157,6 @@ class BooleanDrop < Liquid::Drop
     @value = value
   end
 
-  def ==(other)
-    @value == other
-  end
-
   def to_liquid_value
     @value
   end

test/unit/migrate_unit_test.rb

@@ -1,133 +0,0 @@
-# frozen_string_literal: true
-
-require 'test_helper'
-
-class MigrateUnitTest < Minitest::Test
-  def test_migrate_preserves_valid_markup
-    [
-      "{{a}}",
-      " {{- \ta\n -}} ",
-      "{{a.b['c'].d[5]|default:6,allow_false:true|truncate:7,'..'}}",
-      "{{ a . b [ 'c' ] . d [ 5 ] | default : 6 , allow_false : true | truncate :  4 , '..'  }}",
-      "{%assign x=a.b['c'].d[5]|default:6,allow_false:true|truncate:7,'..'%}",
-      "{% assign x =\na . b [ 'c' ] . d [ 5 ] | default : 6 , allow_false : true | truncate :  4 , '..'  %}",
-      "{% if a and b > c %}A{% elsif d or f contains g %}B{% else %}C{% endif %}",
-      <<~LIQUID,
-        {% liquid
-          if x > 0
-            assign x = x | plus: 1
-
-          endif
-        %}
-      LIQUID
-    ].each do |source|
-      assert_no_migration(source)
-    end
-  end
-
-  def test_migrate_variable
-    with_error_mode(:lax) do
-      assert_migration({
-        %({{ ,|"' }}) => "{{  }}", # no MarkupWithQuotedFragment match, skipping characters
-        %({{ ,|"' 123 }}) => "{{  123 }}", # MarkupWithQuotedFragment skipped characters
-        "{{,-2}}" => "{{ -2}}", # preserve separators when removing ignored characters
-        "{{  12 34  }}" => "{{  12  }}", # no FilterMarkupRegex match, skipping characters
-        "{{ -12 34 | abs }}" => "{{ -12 | abs }}", # FilterMarkupRegex skipped characters
-        %({{ -12 | '" abs }}) => "{{ -12 |  abs }}", # FilterParser skipped characters
-        "{{ -1 | abs ' plus: 1 }}" => "{{ -1 | abs | plus: 1 }}", # FilterParser unexpected separator
-        "{{ -1 | ! abs }}" => "{{ -1 | abs }}", # ignored non-word characters preceding filter name
-        "{{ 'a' | append WAT: 'b' }}" => "{{ 'a' | append : 'b' }}", # FilterArgsRegex skipped characters
-        "{{ '!' | replace, '!': '?' }}" => "{{ '!' | replace: '!', '?' }}", # FilterArgsRegex unexpected separators
-        "{{ -a.1b }}" => "{{ ['-a']['1b'] }}", # quote separators when removing ignored characters
-      })
-    end
-  end
-
-  def test_migrate_expression
-    with_error_mode(:lax) do
-      assert_migration({
-        "{{ (1.9...2.8) }}" => "{{ (1..2) }}", # apply constant range coercion
-        "{{ 1.2.3.4 }}" => "{{ 1.2 }}", # multiple periods allowed by FLOATS_REGEX, truncated by to_f
-        "{{ 1. }}" => "{{ 1.0 }}", # FLOATS_REGEX didn't require digits after the period
-        "{{ .empty }}" => "{{ ['empty'] }}", # skipped character prevents exact literal lookup
-      })
-    end
-  end
-
-  def test_migrate_variable_lookup
-    with_error_mode(:lax) do
-      assert_migration({
-        "{{@a[b].c@}}" => "{{ a[b].c }}", # VariableParser skipped characters
-        "{{ a!b$c }}" => "{{ a.b.c }}", # VariableParser unexpected separators
-      })
-    end
-  end
-
-  def test_migrate_assign
-    with_error_mode(:lax) do
-      assert_migration({
-        "{% assign!a = b!%}" => "{% assign a = b %}", # Syntax skipped characters
-        "{% assign a = @b ! %}" => "{% assign a =  b %}", # Variable skipped characters
-        "{% assign|x=1 %}" => "{% assign x=1 %}", # ensure tag name separated from markup
-      })
-    end
-  end
-
-  def test_lax_migrate_if
-    with_error_mode(:lax) do
-      assert_migration({
-        "{% if@a@%}Y{% endif %}" => "{% if a %}Y{% endif %}", # Syntax skipped character
-        "{% if &a contains^b and *c %}A{% elsif %d or$e %}B{% endif %}" =>
-          "{% if  a contains b and  c %}A{% elsif  d or e %}B{% endif %}", # test more expressions
-        "{% if b 1 %}Y{% endif %}" => "{% if b  %}Y{% endif %}", # missing operator with right operand
-        "{% if c == %}Y{% endif %}" => "{% if c == nil %}Y{% endif %}", # operator with missing right operand
-        "{% if!a!%}T{% endif %}" => "{% if a %}T{% endif %}", # VariableParser skipped characters
-        "{% if!%}T{% endif %}" => "{% if nil %}T{% endif %}", # VariableParser skipping all characters
-      })
-    end
-  end
-
-  def test_migrate_liquid_tag
-    with_error_mode(:lax) do
-      source = <<~LIQUID
-        {% liquid
-          assign ! a = 1
-          assign a = @b !
-        %}
-      LIQUID
-      expect = <<~LIQUID
-        {% liquid
-          assign  a = 1
-          assign a =  b
-        %}
-      LIQUID
-      assert_migration({ source => expect })
-    end
-  end
-
-  def test_migrate_for
-    with_error_mode(:lax) do
-      assert_migration({
-        # VariableLookup ignored character
-        "{% for i in !array %}x{% endfor %}" => "{% for i in  array %}x{% endfor %}",
-
-        # TagAttributes scanned over separators
-        "{% for i in array|offset: 1|limit: 5 %}x{% endfor %}" => "{% for i in array, offset: 1, limit: 5 %}x{% endfor %}",
-
-        # TagAttributes scans all the markup, which can overlap with what Syntax already matched
-        "{% for i in foo.offset: wat %}x{% endfor %}" => "{% for i in foo.offset , offset: wat %}x{% endfor %}",
-      })
-    end
-  end
-
-  private
-
-  def assert_migration(source_to_expected_output_hash)
-    source_to_expected_output_hash.each do |source, expect|
-      message = "source: #{source.inspect}"
-      assert_equal(expect, Liquid::Template.migrate(source), message)
-      assert_no_migration(expect)
-      assert_equal(Liquid::Template.parse(expect, parse_mode: :strict).render!, Liquid::Template.parse(source).render!, message)
-    end
-  end
-end

test/unit/tags/for_tag_unit_test.rb

@@ -12,4 +12,34 @@ class ForTagUnitTest < Minitest::Test
     template = Liquid::Template.parse('{% for item in items %}FOR{% else %}ELSE{% endfor %}')
     assert_equal(['FOR', 'ELSE'], template.root.nodelist[0].nodelist.map(&:nodelist).flatten)
   end
+
+  def test_for_string_slice_bug_usage
+    template = Liquid::Template.parse("{% for x in str, offset: 1 %}{{ x }},{% endfor %}")
+    assert_usage("string_slice_bug") do
+      assert_equal("abc,", template.render({ "str" => "abc" }))
+    end
+  end
+
+  def test_for_string_0_limit_usage
+    template = Liquid::Template.parse("{% for x in str, limit: 0 %}{{ x }},{% endfor %}")
+    assert_usage("string_slice_bug") do
+      assert_equal("abc,", template.render({ "str" => "abc" }))
+    end
+  end
+
+  def test_for_string_no_slice_usage
+    template = Liquid::Template.parse("{% for x in str, offset: 0, limit: 1 %}{{ x }},{% endfor %}")
+    assert_usage("string_slice_bug", times: 0) do
+      assert_equal("abc,", template.render({ "str" => "abc" }))
+    end
+  end
+
+  private
+
+  def assert_usage(name, times: 1, &block)
+    count = 0
+    result = Liquid::Usage.stub(:increment, ->(n) { count += 1 if n == name }, &block)
+    assert_equal(times, count)
+    result
+  end
 end