[WIP] Add support for migrating liquid code away from several lax parse quirks

It transforms liquid code to equivalently parsed code, that doesn't depend
on lax parsing quirks.

Note that this transformation can hide the intent of the original code, so
it is best to review the changes, since removed syntax often would indicate
a bug in the original liquid code.

lib/liquid.rb

@@ -68,6 +68,7 @@ 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'
@@ -81,7 +82,6 @@ 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,6 +60,21 @@ 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

@@ -4,8 +4,8 @@ require 'English'
 
 module Liquid
   class BlockBody
-    LiquidTagToken      = /\A\s*(#{TagName})\s*(.*?)\z/o
-    FullToken           = /\A#{TagStart}#{WhitespaceControl}?(\s*)(#{TagName})(\s*)(.*?)#{WhitespaceControl}?#{TagEnd}\z/om
+    LiquidTagToken      = /\A\s*(#{TagName})\s*(.*?)\s*\z/o
+    FullToken           = /\A#{TagStart}#{WhitespaceControl}?(\s*)(#{TagName})(\s*)(.*?)\s*?#{WhitespaceControl}?#{TagEnd}\z/om
     ContentOfVariable   = /\A#{VariableStart}#{WhitespaceControl}?(.*?)#{WhitespaceControl}?#{VariableEnd}\z/om
     WhitespaceOrNothing = /\A\s*\z/
     TAGSTART            = "{%"
@@ -30,6 +30,16 @@ 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
@@ -60,6 +70,56 @@ 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 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)
+        if token.empty? || token.match?(WhitespaceOrNothing)
+          result << token
+          result << "\n" if tokenizer.more?
+        else
+          match = token.match(LiquidTagToken)
+          unless match
+            # Missing tag name, which was allowed in comment tags through its
+            # unknown tag handling
+            raise NotImplementedError, "TODO"
+          end
+          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
+          has_more_tokens = tokenizer.more?
+          new_markup, new_tag_body = tag.migrate(tag_name, markup, tokenizer, parse_context)
+          result << Utils.match_capture_replace(match, 2, new_markup)
+          result << "\n" if has_more_tokens
+          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)
@@ -109,6 +169,15 @@ 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
@@ -166,6 +235,56 @@ 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?('%}')
+          match = token.match(FullToken)
+          unless match
+            # Missing tag name, which was allowed in comment tags through its
+            # unknown tag handling
+            raise NotImplementedError, "TODO"
+          end
+          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 = migrate_liquid_tag(markup, parse_context)
+            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_markup, new_tag_body = tag.migrate(tag_name, markup, tokenizer, parse_context)
+          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
@@ -246,6 +365,17 @@ 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,6 +28,16 @@ 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/expression.rb

@@ -41,5 +41,36 @@ 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/parser_switching.rb

@@ -2,6 +2,30 @@
 
 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(markup)
+          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,6 +22,37 @@ 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/tag.rb

@@ -7,6 +7,8 @@ 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,6 +21,18 @@ 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(/\s*#{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/if.rb

@@ -20,6 +20,39 @@ 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 end 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 = []
@@ -81,6 +114,10 @@ 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
@@ -101,6 +138,40 @@ 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/template.rb

@@ -96,6 +96,11 @@ 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
@@ -112,6 +117,12 @@ module Liquid
       self
     end
 
+    def migrate(source, parse_options = {})
+      parse_context = configure_options(parse_options)
+      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,6 +25,10 @@ module Liquid
       token
     end
 
+    def more?
+      @offset < @tokens.length
+    end
+
     private
 
     def tokenize

lib/liquid/utils.rb

@@ -89,5 +89,33 @@ 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
+
+    # @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,6 +41,77 @@ module Liquid
       "in \"{{#{markup}}}\""
     end
 
+    STRICT_PARSE_CONTEXT = ParseContext.new(error_mode: :strict).freeze
+    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
+
+      Utils.match_captures_replace(markup_match, 1 => new_name_markup, 2 => new_filter_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,6 +10,41 @@ 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
+        if lookup&.start_with?('[') && lookup&.end_with?(']')
+          new_markup ||= +""
+          new_markup << "[" << Expression.lax_migrate(lookup[1..-2]) << "]"
+        else
+          new_markup << "." if new_markup
+          new_markup ||= +""
+          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)