Refactor the Optimizer class no visit the AST

Just takes care of Variables for now.

Increases performance.

Gemfile

@@ -22,6 +22,6 @@ group :test do
   gem 'rubocop-performance', require: false
 
   platform :mri, :truffleruby do
-    gem 'liquid-c', github: 'Shopify/liquid-c', ref: 'optimize-invoke-filter-refactor'
+    gem 'liquid-c', github: 'Shopify/liquid-c', ref: 'master'
   end
 end

lib/liquid.rb

@@ -84,6 +84,7 @@ require 'liquid/usage'
 require 'liquid/register'
 require 'liquid/static_registers'
 require 'liquid/template_factory'
+require 'liquid/optimizer'
 
 # Load all the tags of the standard library
 #

lib/liquid/block.rb

@@ -10,7 +10,7 @@ module Liquid
     end
 
     def parse(tokens)
-      @body = BlockBody.new
+      @body = new_body
       while parse_body(@body, tokens)
       end
     end
@@ -28,7 +28,12 @@ module Liquid
       @body.nodelist
     end
 
-    def unknown_tag(tag, _params, _tokens)
+    def unknown_tag(tag_name, _markup, _tokenizer)
+      Block.raise_unknown_tag(tag_name, block_name, block_delimiter, parse_context)
+    end
+
+    # @api private
+    def self.raise_unknown_tag(tag, block_name, block_delimiter, parse_context)
       if tag == 'else'
         raise SyntaxError, parse_context.locale.t("errors.syntax.unexpected_else",
           block_name: block_name)
@@ -42,6 +47,10 @@ module Liquid
       end
     end
 
+    def raise_tag_never_closed(block_name)
+      raise SyntaxError, parse_context.locale.t("errors.syntax.tag_never_closed", block_name: block_name)
+    end
+
     def block_name
       @tag_name
     end
@@ -50,8 +59,14 @@ module Liquid
       @block_delimiter ||= "end#{block_name}"
     end
 
-    protected
+    private
 
+    # @api public
+    def new_body
+      parse_context.new_block_body
+    end
+
+    # @api public
     def parse_body(body, tokens)
       if parse_context.depth >= MAX_DEPTH
         raise StackLevelError, "Nesting too deep"
@@ -62,9 +77,7 @@ module Liquid
           @blank &&= body.blank?
 
           return false if end_tag_name == block_delimiter
-          unless end_tag_name
-            raise SyntaxError, parse_context.locale.t("errors.syntax.tag_never_closed", block_name: block_name)
-          end
+          raise_tag_never_closed(block_name) unless end_tag_name
 
           # this tag is not registered with the system
           # pass it to the current block for special handling or error reporting

lib/liquid/block_body.rb

@@ -54,28 +54,60 @@ module Liquid
     end
 
     # @api private
-    def self.unknown_tag_in_liquid_tag(end_tag_name, end_tag_markup)
-      yield end_tag_name, end_tag_markup
-    ensure
-      Usage.increment("liquid_tag_contains_outer_tag") unless $ERROR_INFO.is_a?(SyntaxError)
+    def self.unknown_tag_in_liquid_tag(tag, parse_context)
+      Block.raise_unknown_tag(tag, 'liquid', '%}', parse_context)
     end
 
-    private def parse_liquid_tag(markup, parse_context, &block)
-      liquid_tag_tokenizer = Tokenizer.new(markup, line_number: parse_context.line_number, for_liquid_tag: true)
-      parse_for_liquid_tag(liquid_tag_tokenizer, parse_context) do |end_tag_name, end_tag_markup|
-        next unless end_tag_name
-        self.class.unknown_tag_in_liquid_tag(end_tag_name, end_tag_markup, &block)
+    # @api private
+    def self.raise_missing_tag_terminator(token, parse_context)
+      raise SyntaxError, parse_context.locale.t("errors.syntax.tag_termination", token: token, tag_end: TagEnd.inspect)
+    end
+
+    # @api private
+    def self.raise_missing_variable_terminator(token, parse_context)
+      raise SyntaxError, parse_context.locale.t("errors.syntax.variable_termination", token: token, tag_end: VariableEnd.inspect)
+    end
+
+    # @api private
+    def self.render_node(context, output, node)
+      node.render_to_output_buffer(context, output)
+    rescue => exc
+      blank_tag = !node.instance_of?(Variable) && node.blank?
+      rescue_render_node(context, output, node.line_number, exc, blank_tag)
+    end
+
+    # @api private
+    def self.rescue_render_node(context, output, line_number, exc, blank_tag)
+      case exc
+      when MemoryError
+        raise
+      when UndefinedVariable, UndefinedDropMethod, UndefinedFilter
+        context.handle_error(exc, line_number)
+      else
+        error_message = context.handle_error(exc, line_number)
+        unless blank_tag # conditional for backwards compatibility
+          output << error_message
+        end
       end
     end
 
-    private def parse_for_document(tokenizer, parse_context, &block)
+    private def parse_liquid_tag(markup, parse_context)
+      liquid_tag_tokenizer = Tokenizer.new(markup, line_number: parse_context.line_number, for_liquid_tag: true)
+      parse_for_liquid_tag(liquid_tag_tokenizer, parse_context) do |end_tag_name, _end_tag_markup|
+        if end_tag_name
+          BlockBody.unknown_tag_in_liquid_tag(end_tag_name, parse_context)
+        end
+      end
+    end
+
+    private def parse_for_document(tokenizer, parse_context)
       while (token = tokenizer.shift)
         next if token.empty?
         case
         when token.start_with?(TAGSTART)
           whitespace_handler(token, parse_context)
           unless token =~ FullToken
-            raise_missing_tag_terminator(token, parse_context)
+            BlockBody.raise_missing_tag_terminator(token, parse_context)
           end
           tag_name = Regexp.last_match(2)
           markup   = Regexp.last_match(4)
@@ -87,7 +119,7 @@ module Liquid
           end
 
           if tag_name == 'liquid'
-            parse_liquid_tag(markup, parse_context, &block)
+            parse_liquid_tag(markup, parse_context)
             next
           end
 
@@ -121,7 +153,11 @@ module Liquid
       if token[2] == WhitespaceControl
         previous_token = @nodelist.last
         if previous_token.is_a?(String)
+          first_byte = previous_token.getbyte(0)
           previous_token.rstrip!
+          if previous_token.empty? && parse_context[:bug_compatible_whitespace_trimming] && first_byte
+            previous_token << first_byte
+          end
         end
       end
       parse_context.trim_whitespace = (token[-3] == WhitespaceControl)
@@ -131,38 +167,47 @@ module Liquid
       @blank
     end
 
+    # Remove blank strings in the block body for a control flow tag (e.g. `if`, `for`, `case`, `unless`)
+    # with a blank body.
+    #
+    # For example, in a conditional assignment like the following
+    #
+    # ```
+    # {% if size > max_size %}
+    #   {% assign size = max_size %}
+    # {% endif %}
+    # ```
+    #
+    # we assume the intention wasn't to output the blank spaces in the `if` tag's block body, so this method
+    # will remove them to reduce the render output size.
+    #
+    # Note that it is now preferred to use the `liquid` tag for this use case.
+    def remove_blank_strings
+      raise "remove_blank_strings only support being called on a blank block body" unless @blank
+      @nodelist.reject! { |node| node.instance_of?(String) }
+    end
+
     def render(context)
       render_to_output_buffer(context, +'')
     end
 
     def render_to_output_buffer(context, output)
-      context.resource_limits.render_score += @nodelist.length
+      context.resource_limits.increment_render_score(@nodelist.length)
 
       idx = 0
       while (node = @nodelist[idx])
-        previous_output_size = output.bytesize
-
-        case node
-        when String
+        if node.instance_of?(String)
           output << node
-        when Variable
+        else
           render_node(context, output, node)
-        when Block
-          render_node(context, node.blank? ? +'' : output, node)
-          break if context.interrupt? # might have happened in a for-block
-        when Continue, Break
           # If we get an Interrupt that means the block must stop processing. An
           # Interrupt is any command that stops block execution such as {% break %}
-          # or {% continue %}
-          context.push_interrupt(node.interrupt)
-          break
-        else # Other non-Block tags
-          render_node(context, output, node)
-          break if context.interrupt? # might have happened through an include
+          # or {% continue %}. These tags may also occur through Block or Include tags.
+          break if context.interrupt? # might have happened in a for-block
         end
         idx += 1
 
-        raise_if_resource_limits_reached(context, output.bytesize - previous_output_size)
+        context.resource_limits.increment_write_score(output)
       end
 
       output
@@ -171,34 +216,25 @@ module Liquid
     private
 
     def render_node(context, output, node)
-      node.render_to_output_buffer(context, output)
-    rescue UndefinedVariable, UndefinedDropMethod, UndefinedFilter => e
-      context.handle_error(e, node.line_number)
-    rescue ::StandardError => e
-      line_number = node.is_a?(String) ? nil : node.line_number
-      output << context.handle_error(e, line_number)
-    end
-
-    def raise_if_resource_limits_reached(context, length)
-      context.resource_limits.render_length += length
-      return unless context.resource_limits.reached?
-      raise MemoryError, "Memory limits exceeded"
+      BlockBody.render_node(context, output, node)
     end
 
     def create_variable(token, parse_context)
-      if token =~ ContentOfVariable
-        markup = Regexp.last_match(1)
+      token.scan(ContentOfVariable) do |content|
+        markup = content.first
         return Variable.new(markup, parse_context)
       end
-      raise_missing_variable_terminator(token, parse_context)
+      BlockBody.raise_missing_variable_terminator(token, parse_context)
     end
 
+    # @deprecated Use {.raise_missing_tag_terminator} instead
     def raise_missing_tag_terminator(token, parse_context)
-      raise SyntaxError, parse_context.locale.t("errors.syntax.tag_termination", token: token, tag_end: TagEnd.inspect)
+      BlockBody.raise_missing_tag_terminator(token, parse_context)
     end
 
+    # @deprecated Use {.raise_missing_variable_terminator} instead
     def raise_missing_variable_terminator(token, parse_context)
-      raise SyntaxError, parse_context.locale.t("errors.syntax.variable_termination", token: token, tag_end: VariableEnd.inspect)
+      BlockBody.raise_missing_variable_terminator(token, parse_context)
     end
 
     def registered_tags

lib/liquid/condition.rb

@@ -27,10 +27,28 @@ module Liquid
       end,
     }
 
+    class MethodLiteral
+      attr_reader :method_name, :to_s
+
+      def initialize(method_name, to_s)
+        @method_name = method_name
+        @to_s = to_s
+      end
+    end
+
+    @@method_literals = {
+      'blank' => MethodLiteral.new(:blank?, '').freeze,
+      'empty' => MethodLiteral.new(:empty?, '').freeze,
+    }
+
     def self.operators
       @@operators
     end
 
+    def self.parse_expression(markup)
+      @@method_literals[markup] || Expression.parse(markup)
+    end
+
     attr_reader :attachment, :child_condition
     attr_accessor :left, :operator, :right
 
@@ -91,7 +109,7 @@ module Liquid
     private
 
     def equal_variables(left, right)
-      if left.is_a?(Liquid::Expression::MethodLiteral)
+      if left.is_a?(MethodLiteral)
         if right.respond_to?(left.method_name)
           return right.send(left.method_name)
         else
@@ -99,7 +117,7 @@ module Liquid
         end
       end
 
-      if right.is_a?(Liquid::Expression::MethodLiteral)
+      if right.is_a?(MethodLiteral)
         if left.respond_to?(right.method_name)
           return left.send(right.method_name)
         else

lib/liquid/context.rb

@@ -34,17 +34,18 @@ module Liquid
       @strict_variables    = false
       @resource_limits     = resource_limits || ResourceLimits.new(Template.default_resource_limits)
       @base_scope_depth    = 0
-      squash_instance_assigns_with_environments
+      @interrupts          = []
+      @filters             = []
+      @global_filter       = nil
+      @disabled_tags       = {}
 
       self.exception_renderer = Template.default_exception_renderer
       if rethrow_errors
         self.exception_renderer = ->(_e) { raise }
       end
 
-      @interrupts    = []
-      @filters       = []
-      @global_filter = nil
-      @disabled_tags = {}
+      # Do this last, since it could result in this object being passed to a Proc in the environment
+      squash_instance_assigns_with_environments
     end
     # rubocop:enable Metrics/ParameterLists
 
@@ -93,13 +94,8 @@ module Liquid
       exception_renderer.call(e).to_s
     end
 
-    def invoke(method, input, *args)
-      strainer.invoke(method, input, *args).to_liquid
-    end
-
-    # @api private
-    def invoke_filter(method, input, args)
-      strainer.invoke_filter(method, input, args).to_liquid
+    def invoke(method, *args)
+      strainer.invoke(method, *args).to_liquid
     end
 
     # Push new local scope on the stack. use <tt>Context#stack</tt> instead

lib/liquid/document.rb

@@ -1,23 +1,33 @@
 # frozen_string_literal: true
 
 module Liquid
-  class Document < BlockBody
+  class Document
     def self.parse(tokens, parse_context)
-      doc = new
+      doc = new(parse_context)
       doc.parse(tokens, parse_context)
       doc
     end
 
-    def parse(tokens, parse_context)
-      super do |end_tag_name, _end_tag_params|
-        unknown_tag(end_tag_name, parse_context) if end_tag_name
+    attr_reader :parse_context, :body
+
+    def initialize(parse_context)
+      @parse_context = parse_context
+      @body = new_body
+    end
+
+    def nodelist
+      @body.nodelist
+    end
+
+    def parse(tokenizer, parse_context)
+      while parse_body(tokenizer)
       end
     rescue SyntaxError => e
       e.line_number ||= parse_context.line_number
       raise
     end
 
-    def unknown_tag(tag, parse_context)
+    def unknown_tag(tag, _markup, _tokenizer)
       case tag
       when 'else', 'end'
         raise SyntaxError, parse_context.locale.t("errors.syntax.unexpected_outer_tag", tag: tag)
@@ -25,5 +35,30 @@ module Liquid
         raise SyntaxError, parse_context.locale.t("errors.syntax.unknown_tag", tag: tag)
       end
     end
+
+    def render_to_output_buffer(context, output)
+      @body.render_to_output_buffer(context, output)
+    end
+
+    def render(context)
+      @body.render(context)
+    end
+
+    private
+
+    def new_body
+      parse_context.new_block_body
+    end
+
+    def parse_body(tokenizer)
+      @body.parse(tokenizer, parse_context) do |unknown_tag_name, unknown_tag_markup|
+        if unknown_tag_name
+          unknown_tag(unknown_tag_name, unknown_tag_markup, tokenizer)
+          true
+        else
+          false
+        end
+      end
+    end
   end
 end

lib/liquid/expression.rb

@@ -2,25 +2,12 @@
 
 module Liquid
   class Expression
-    class MethodLiteral
-      attr_reader :method_name, :to_s
-
-      def initialize(method_name, to_s)
-        @method_name = method_name
-        @to_s = to_s
-      end
-
-      def to_liquid
-        to_s
-      end
-    end
-
     LITERALS = {
       nil => nil, 'nil' => nil, 'null' => nil, '' => nil,
       'true' => true,
       'false' => false,
-      'blank' => MethodLiteral.new(:blank?, '').freeze,
-      'empty' => MethodLiteral.new(:empty?, '').freeze
+      'blank' => '',
+      'empty' => ''
     }.freeze
 
     SINGLE_QUOTED_STRING = /\A'(.*)'\z/m

lib/liquid/optimizer.rb

@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+
+module Liquid
+  class Optimizer
+    class << self
+      attr_accessor :enabled
+
+      def optimize_variable(node)
+        return unless enabled
+
+        # Turn chained `| append: "..."| append: "..."`, into a single `append_all: [...]`
+        if node.filters.size > 1 && node.filters.all? { |f, _| f == "append" }
+          node.filters = [["append_all", node.filters.map { |f, (arg)| arg }]]
+        end
+      end
+    end
+    self.enabled = true
+  end
+end

lib/liquid/parse_context.rb

@@ -19,6 +19,10 @@ module Liquid
       @options[option_key]
     end
 
+    def new_block_body
+      Liquid::BlockBody.new
+    end
+
     def partial=(value)
       @partial = value
       @options = value ? partial_options : @template_options

lib/liquid/resource_limits.rb

@@ -2,8 +2,8 @@
 
 module Liquid
   class ResourceLimits
-    attr_accessor :render_length, :render_score, :assign_score,
-      :render_length_limit, :render_score_limit, :assign_score_limit
+    attr_accessor :render_length_limit, :render_score_limit, :assign_score_limit
+    attr_reader :render_score, :assign_score
 
     def initialize(limits)
       @render_length_limit = limits[:render_length_limit]
@@ -12,14 +12,51 @@ module Liquid
       reset
     end
 
+    def increment_render_score(amount)
+      @render_score += amount
+      raise_limits_reached if @render_score_limit && @render_score > @render_score_limit
+    end
+
+    def increment_assign_score(amount)
+      @assign_score += amount
+      raise_limits_reached if @assign_score_limit && @assign_score > @assign_score_limit
+    end
+
+    # update either render_length or assign_score based on whether or not the writes are captured
+    def increment_write_score(output)
+      if (last_captured = @last_capture_length)
+        captured = output.bytesize
+        increment = captured - last_captured
+        @last_capture_length = captured
+        increment_assign_score(increment)
+      elsif @render_length_limit && output.bytesize > @render_length_limit
+        raise_limits_reached
+      end
+    end
+
+    def raise_limits_reached
+      @reached_limit = true
+      raise MemoryError, "Memory limits exceeded"
+    end
+
     def reached?
-      (@render_length_limit && @render_length > @render_length_limit) ||
-        (@render_score_limit && @render_score > @render_score_limit) ||
-        (@assign_score_limit && @assign_score > @assign_score_limit)
+      @reached_limit
     end
 
     def reset
-      @render_length = @render_score = @assign_score = 0
+      @reached_limit = false
+      @last_capture_length = nil
+      @render_score = @assign_score = 0
+    end
+
+    def with_capture
+      old_capture_length = @last_capture_length
+      begin
+        @last_capture_length = 0
+        yield
+      ensure
+        @last_capture_length = old_capture_length
+      end
     end
   end
 end

lib/liquid/standardfilters.rb

@@ -281,6 +281,10 @@ module Liquid
       input.to_s + string.to_s
     end
 
+    def append_all(input, *items)
+      input.to_s + items.join
+    end
+
     def concat(input, array)
       unless array.respond_to?(:to_ary)
         raise ArgumentError, "concat filter requires an array argument"

lib/liquid/strainer_template.rb

@@ -24,11 +24,11 @@ module Liquid
 
         include(filter)
 
-        filter_methods.merge(filter.public_instance_methods)
+        filter_methods.merge(filter.public_instance_methods.map(&:to_s))
       end
 
       def invokable?(method)
-        filter_methods.include?(method.to_sym)
+        filter_methods.include?(method.to_s)
       end
 
       private
@@ -38,18 +38,13 @@ module Liquid
       end
     end
 
-    def invoke(method, input, *args)
-      invoke_filter(method.to_sym, input, args)
-    end
-
-    # @api private
-    def invoke_filter(method, input, args)
+    def invoke(method, *args)
       if self.class.invokable?(method)
-        send(method, input, *args)
+        send(method, *args)
       elsif @context.strict_filters
         raise Liquid::UndefinedFilter, "undefined filter #{method}"
       else
-        input
+        args.first
       end
     rescue ::ArgumentError => e
       raise Liquid::ArgumentError, e.message, e.backtrace

lib/liquid/tags/assign.rb

@@ -27,7 +27,7 @@ module Liquid
     def render_to_output_buffer(context, output)
       val = @from.render(context)
       context.scopes.last[@to] = val
-      context.resource_limits.assign_score += assign_score_of(val)
+      context.resource_limits.increment_assign_score(assign_score_of(val))
       output
     end
 

lib/liquid/tags/break.rb

@@ -11,8 +11,11 @@ module Liquid
   #    {% endfor %}
   #
   class Break < Tag
-    def interrupt
-      BreakInterrupt.new
+    INTERRUPT = BreakInterrupt.new.freeze
+
+    def render_to_output_buffer(context, output)
+      context.push_interrupt(INTERRUPT)
+      output
     end
   end
 

lib/liquid/tags/capture.rb

@@ -25,10 +25,10 @@ module Liquid
     end
 
     def render_to_output_buffer(context, output)
-      previous_output_size = output.bytesize
-      super
-      context.scopes.last[@to] = output
-      context.resource_limits.assign_score += (output.bytesize - previous_output_size)
+      context.resource_limits.with_capture do
+        capture_output = render(context)
+        context.scopes.last[@to] = capture_output
+      end
       output
     end
 

lib/liquid/tags/case.rb

@@ -19,8 +19,11 @@ module Liquid
     end
 
     def parse(tokens)
-      body = BlockBody.new
+      body = new_body
       body = @blocks.last.attachment while parse_body(body, tokens)
+      if blank?
+        @blocks.each { |condition| condition.attachment.remove_blank_strings }
+      end
     end
 
     def nodelist
@@ -56,7 +59,7 @@ module Liquid
     private
 
     def record_when_condition(markup)
-      body = BlockBody.new
+      body = new_body
 
       while markup
         unless markup =~ WhenSyntax
@@ -65,7 +68,7 @@ module Liquid
 
         markup = Regexp.last_match(2)
 
-        block = Condition.new(@left, '==', Expression.parse(Regexp.last_match(1)))
+        block = Condition.new(@left, '==', Condition.parse_expression(Regexp.last_match(1)))
         block.attach(body)
         @blocks << block
       end
@@ -77,7 +80,7 @@ module Liquid
       end
 
       block = ElseCondition.new
-      block.attach(BlockBody.new)
+      block.attach(new_body)
       @blocks << block
     end
 

lib/liquid/tags/continue.rb

@@ -11,8 +11,11 @@ module Liquid
   #    {% endfor %}
   #
   class Continue < Tag
-    def interrupt
-      ContinueInterrupt.new
+    INTERRUPT = ContinueInterrupt.new.freeze
+
+    def render_to_output_buffer(context, output)
+      context.push_interrupt(INTERRUPT)
+      output
     end
   end
 

lib/liquid/tags/for.rb

@@ -54,13 +54,18 @@ module Liquid
       super
       @from = @limit = nil
       parse_with_selected_parser(markup)
-      @for_block = BlockBody.new
+      @for_block = new_body
       @else_block = nil
     end
 
     def parse(tokens)
-      return unless parse_body(@for_block, tokens)
-      parse_body(@else_block, tokens)
+      if parse_body(@for_block, tokens)
+        parse_body(@else_block, tokens)
+      end
+      if blank?
+        @for_block.remove_blank_strings
+        @else_block&.remove_blank_strings
+      end
     end
 
     def nodelist
@@ -69,7 +74,7 @@ module Liquid
 
     def unknown_tag(tag, markup, tokens)
       return super unless tag == 'else'
-      @else_block = BlockBody.new
+      @else_block = new_body
     end
 
     def render_to_output_buffer(context, output)

lib/liquid/tags/if.rb

@@ -31,6 +31,9 @@ module Liquid
     def parse(tokens)
       while parse_body(@blocks.last.attachment, tokens)
       end
+      if blank?
+        @blocks.each { |condition| condition.attachment.remove_blank_strings }
+      end
     end
 
     def unknown_tag(tag, markup, tokens)
@@ -61,21 +64,25 @@ module Liquid
       end
 
       @blocks.push(block)
-      block.attach(BlockBody.new)
+      block.attach(new_body)
+    end
+
+    def parse_expression(markup)
+      Condition.parse_expression(markup)
     end
 
     def lax_parse(markup)
       expressions = markup.scan(ExpressionsAndOperators)
       raise SyntaxError, options[:locale].t("errors.syntax.if") unless expressions.pop =~ Syntax
 
-      condition = Condition.new(Expression.parse(Regexp.last_match(1)), Regexp.last_match(2), Expression.parse(Regexp.last_match(3)))
+      condition = Condition.new(parse_expression(Regexp.last_match(1)), Regexp.last_match(2), parse_expression(Regexp.last_match(3)))
 
       until expressions.empty?
         operator = expressions.pop.to_s.strip
 
         raise SyntaxError, options[:locale].t("errors.syntax.if") unless expressions.pop.to_s =~ Syntax
 
-        new_condition = Condition.new(Expression.parse(Regexp.last_match(1)), Regexp.last_match(2), Expression.parse(Regexp.last_match(3)))
+        new_condition = Condition.new(parse_expression(Regexp.last_match(1)), Regexp.last_match(2), parse_expression(Regexp.last_match(3)))
         raise SyntaxError, options[:locale].t("errors.syntax.if") unless BOOLEAN_OPERATORS.include?(operator)
         new_condition.send(operator, condition)
         condition = new_condition
@@ -103,9 +110,9 @@ module Liquid
     end
 
     def parse_comparison(p)
-      a = Expression.parse(p.expression)
+      a = parse_expression(p.expression)
       if (op = p.consume?(:comparison))
-        b = Expression.parse(p.expression)
+        b = parse_expression(p.expression)
         Condition.new(a, op, b)
       else
         Condition.new(a)

lib/liquid/tags/raw.rb

@@ -14,14 +14,14 @@ module Liquid
     def parse(tokens)
       @body = +''
       while (token = tokens.shift)
-        if token =~ FullTokenPossiblyInvalid
+        if token =~ FullTokenPossiblyInvalid && block_delimiter == Regexp.last_match(2)
           @body << Regexp.last_match(1) if Regexp.last_match(1) != ""
-          return if block_delimiter == Regexp.last_match(2)
+          return
         end
         @body << token unless token.empty?
       end
 
-      raise SyntaxError, parse_context.locale.t("errors.syntax.tag_never_closed", block_name: block_name)
+      raise_tag_never_closed(block_name)
     end
 
     def render_to_output_buffer(_context, output)

lib/liquid/variable.rb

@@ -18,7 +18,7 @@ module Liquid
     JustTagAttributes        = /\A#{TagAttributes}\z/o
     MarkupWithQuotedFragment = /(#{QuotedFragment})(.*)/om
 
-    attr_accessor :name, :line_number
+    attr_accessor :filters, :name, :line_number
     attr_reader :parse_context
     alias_method :options, :parse_context
 
@@ -31,6 +31,8 @@ module Liquid
       @line_number   = parse_context.line_number
 
       parse_with_selected_parser(markup)
+
+      Optimizer.optimize_variable(self)
     end
 
     def raw
@@ -81,15 +83,12 @@ module Liquid
     end
 
     def render(context)
-      result = context.evaluate(@name)
-      @filters.each do |filter_name, constant_args, filter_args, filter_kwargs|
-        unless constant_args
-          filter_args = evaluate_filter_expressions(context, filter_args, filter_kwargs)
-        end
-        result = context.invoke_filter(filter_name, result, filter_args)
+      obj = @filters.inject(context.evaluate(@name)) do |output, (filter_name, filter_args, filter_kwargs)|
+        filter_args = evaluate_filter_expressions(context, filter_args, filter_kwargs)
+        context.invoke(filter_name, output, *filter_args)
       end
 
-      context.apply_global_filter(result)
+      context.apply_global_filter(obj)
     end
 
     def render_to_output_buffer(context, output)
@@ -113,49 +112,21 @@ module Liquid
       []
     end
 
-    def filters
-      @filters.map do |filter_name, constant_args, *filter_args_and_kwargs|
-        filter_name = filter_name.to_s
-        if constant_args
-          filter_args = filter_args_and_kwargs.first
-          if filter_args.last.is_a?(Hash)
-            filter_args = filter_args.dup
-            [filter_name, filter_args, filter_args.pop]
-          else
-            [filter_name, *filter_args_and_kwargs]
-          end
-        else
-          [filter_name, *filter_args_and_kwargs]
-        end
-      end
-    end
-
     private
 
     def parse_filter_expressions(filter_name, unparsed_args)
-      constant_args = true
-      filter_args = []
+      filter_args  = []
       keyword_args = nil
       unparsed_args.each do |a|
         if (matches = a.match(JustTagAttributes))
-          keyword_args ||= {}
-          expression = Expression.parse(matches[2])
-          constant_args &&= !expression.is_a?(VariableLookup)
-          keyword_args[matches[1]] = expression
+          keyword_args           ||= {}
+          keyword_args[matches[1]] = Expression.parse(matches[2])
         else
-          expression = Expression.parse(a)
-          constant_args &&= !expression.is_a?(VariableLookup)
-          filter_args << expression
-        end
-      end
-      result = [filter_name.to_sym, constant_args, filter_args]
-      if keyword_args
-        if constant_args
-          filter_args << keyword_args
-        else
-          result << keyword_args
+          filter_args << Expression.parse(a)
         end
       end
+      result = [filter_name, filter_args]
+      result << keyword_args if keyword_args
       result
     end
 

test/integration/block_test.rb

@@ -11,4 +11,48 @@ class BlockTest < Minitest::Test
     end
     assert_equal(exc.message, "Liquid syntax error: 'endunless' is not a valid delimiter for if tags. use endif")
   end
+
+  def test_with_custom_tag
+    with_custom_tag('testtag', Block) do
+      assert Liquid::Template.parse("{% testtag %} {% endtesttag %}")
+    end
+  end
+
+  def test_custom_block_tags_have_a_default_render_to_output_buffer_method_for_backwards_compatibility
+    klass1 = Class.new(Block) do
+      def render(*)
+        'hello'
+      end
+    end
+
+    with_custom_tag('blabla', klass1) do
+      template = Liquid::Template.parse("{% blabla %} bla {% endblabla %}")
+
+      assert_equal 'hello', template.render
+
+      buf    = +''
+      output = template.render({}, output: buf)
+      assert_equal 'hello', output
+      assert_equal 'hello', buf
+      assert_equal buf.object_id, output.object_id
+    end
+
+    klass2 = Class.new(klass1) do
+      def render(*)
+        'foo' + super + 'bar'
+      end
+    end
+
+    with_custom_tag('blabla', klass2) do
+      template = Liquid::Template.parse("{% blabla %} foo {% endblabla %}")
+
+      assert_equal 'foohellobar', template.render
+
+      buf    = +''
+      output = template.render({}, output: buf)
+      assert_equal 'foohellobar', output
+      assert_equal 'foohellobar', buf
+      assert_equal buf.object_id, output.object_id
+    end
+  end
 end

test/integration/context_test

@@ -65,7 +65,7 @@ class ArrayLike
   end
 end
 
-class ContextUnitTest < Minitest::Test
+class ContextTest < Minitest::Test
   include Liquid
 
   def setup

test/integration/context_test.rb

@@ -2,9 +2,596 @@
 
 require 'test_helper'
 
+class HundredCentes
+  def to_liquid
+    100
+  end
+end
+
+class CentsDrop < Liquid::Drop
+  def amount
+    HundredCentes.new
+  end
+
+  def non_zero?
+    true
+  end
+end
+
+class ContextSensitiveDrop < Liquid::Drop
+  def test
+    @context['test']
+  end
+end
+
+class Category < Liquid::Drop
+  attr_accessor :name
+
+  def initialize(name)
+    @name = name
+  end
+
+  def to_liquid
+    CategoryDrop.new(self)
+  end
+end
+
+class CategoryDrop
+  attr_accessor :category, :context
+  def initialize(category)
+    @category = category
+  end
+end
+
+class CounterDrop < Liquid::Drop
+  def count
+    @count ||= 0
+    @count  += 1
+  end
+end
+
+class ArrayLike
+  def fetch(index)
+  end
+
+  def [](index)
+    @counts        ||= []
+    @counts[index] ||= 0
+    @counts[index]  += 1
+  end
+
+  def to_liquid
+    self
+  end
+end
+
 class ContextTest < Minitest::Test
   include Liquid
 
+  def setup
+    @context = Liquid::Context.new
+  end
+
+  def test_variables
+    @context['string'] = 'string'
+    assert_equal('string', @context['string'])
+
+    @context['num'] = 5
+    assert_equal(5, @context['num'])
+
+    @context['time'] = Time.parse('2006-06-06 12:00:00')
+    assert_equal(Time.parse('2006-06-06 12:00:00'), @context['time'])
+
+    @context['date'] = Date.today
+    assert_equal(Date.today, @context['date'])
+
+    now = Time.now
+    @context['datetime'] = now
+    assert_equal(now, @context['datetime'])
+
+    @context['bool'] = true
+    assert_equal(true, @context['bool'])
+
+    @context['bool'] = false
+    assert_equal(false, @context['bool'])
+
+    @context['nil'] = nil
+    assert_nil(@context['nil'])
+    assert_nil(@context['nil'])
+  end
+
+  def test_variables_not_existing
+    assert_nil(@context['does_not_exist'])
+  end
+
+  def test_scoping
+    @context.push
+    @context.pop
+
+    assert_raises(Liquid::ContextError) do
+      @context.pop
+    end
+
+    assert_raises(Liquid::ContextError) do
+      @context.push
+      @context.pop
+      @context.pop
+    end
+  end
+
+  def test_length_query
+    @context['numbers'] = [1, 2, 3, 4]
+
+    assert_equal(4, @context['numbers.size'])
+
+    @context['numbers'] = { 1 => 1, 2 => 2, 3 => 3, 4 => 4 }
+
+    assert_equal(4, @context['numbers.size'])
+
+    @context['numbers'] = { 1 => 1, 2 => 2, 3 => 3, 4 => 4, 'size' => 1000 }
+
+    assert_equal(1000, @context['numbers.size'])
+  end
+
+  def test_hyphenated_variable
+    @context['oh-my'] = 'godz'
+    assert_equal('godz', @context['oh-my'])
+  end
+
+  def test_add_filter
+    filter = Module.new do
+      def hi(output)
+        output + ' hi!'
+      end
+    end
+
+    context = Context.new
+    context.add_filters(filter)
+    assert_equal('hi? hi!', context.invoke(:hi, 'hi?'))
+
+    context = Context.new
+    assert_equal('hi?', context.invoke(:hi, 'hi?'))
+
+    context.add_filters(filter)
+    assert_equal('hi? hi!', context.invoke(:hi, 'hi?'))
+  end
+
+  def test_only_intended_filters_make_it_there
+    filter = Module.new do
+      def hi(output)
+        output + ' hi!'
+      end
+    end
+
+    context = Context.new
+    assert_equal("Wookie", context.invoke("hi", "Wookie"))
+
+    context.add_filters(filter)
+    assert_equal("Wookie hi!", context.invoke("hi", "Wookie"))
+  end
+
+  def test_add_item_in_outer_scope
+    @context['test'] = 'test'
+    @context.push
+    assert_equal('test', @context['test'])
+    @context.pop
+    assert_equal('test', @context['test'])
+  end
+
+  def test_add_item_in_inner_scope
+    @context.push
+    @context['test'] = 'test'
+    assert_equal('test', @context['test'])
+    @context.pop
+    assert_nil(@context['test'])
+  end
+
+  def test_hierachical_data
+    @context['hash'] = { "name" => 'tobi' }
+    assert_equal('tobi', @context['hash.name'])
+    assert_equal('tobi', @context['hash["name"]'])
+  end
+
+  def test_keywords
+    assert_equal(true, @context['true'])
+    assert_equal(false, @context['false'])
+  end
+
+  def test_digits
+    assert_equal(100, @context['100'])
+    assert_equal(100.00, @context['100.00'])
+  end
+
+  def test_strings
+    assert_equal("hello!", @context['"hello!"'])
+    assert_equal("hello!", @context["'hello!'"])
+  end
+
+  def test_merge
+    @context.merge("test" => "test")
+    assert_equal('test', @context['test'])
+    @context.merge("test" => "newvalue", "foo" => "bar")
+    assert_equal('newvalue', @context['test'])
+    assert_equal('bar', @context['foo'])
+  end
+
+  def test_array_notation
+    @context['test'] = [1, 2, 3, 4, 5]
+
+    assert_equal(1, @context['test[0]'])
+    assert_equal(2, @context['test[1]'])
+    assert_equal(3, @context['test[2]'])
+    assert_equal(4, @context['test[3]'])
+    assert_equal(5, @context['test[4]'])
+  end
+
+  def test_recoursive_array_notation
+    @context['test'] = { 'test' => [1, 2, 3, 4, 5] }
+
+    assert_equal(1, @context['test.test[0]'])
+
+    @context['test'] = [{ 'test' => 'worked' }]
+
+    assert_equal('worked', @context['test[0].test'])
+  end
+
+  def test_hash_to_array_transition
+    @context['colors'] = {
+      'Blue' => ['003366', '336699', '6699CC', '99CCFF'],
+      'Green' => ['003300', '336633', '669966', '99CC99'],
+      'Yellow' => ['CC9900', 'FFCC00', 'FFFF99', 'FFFFCC'],
+      'Red' => ['660000', '993333', 'CC6666', 'FF9999'],
+    }
+
+    assert_equal('003366', @context['colors.Blue[0]'])
+    assert_equal('FF9999', @context['colors.Red[3]'])
+  end
+
+  def test_try_first
+    @context['test'] = [1, 2, 3, 4, 5]
+
+    assert_equal(1, @context['test.first'])
+    assert_equal(5, @context['test.last'])
+
+    @context['test'] = { 'test' => [1, 2, 3, 4, 5] }
+
+    assert_equal(1, @context['test.test.first'])
+    assert_equal(5, @context['test.test.last'])
+
+    @context['test'] = [1]
+    assert_equal(1, @context['test.first'])
+    assert_equal(1, @context['test.last'])
+  end
+
+  def test_access_hashes_with_hash_notation
+    @context['products'] = { 'count' => 5, 'tags' => ['deepsnow', 'freestyle'] }
+    @context['product']  = { 'variants' => [{ 'title' => 'draft151cm' }, { 'title' => 'element151cm' }] }
+
+    assert_equal(5, @context['products["count"]'])
+    assert_equal('deepsnow', @context['products["tags"][0]'])
+    assert_equal('deepsnow', @context['products["tags"].first'])
+    assert_equal('draft151cm', @context['product["variants"][0]["title"]'])
+    assert_equal('element151cm', @context['product["variants"][1]["title"]'])
+    assert_equal('draft151cm', @context['product["variants"][0]["title"]'])
+    assert_equal('element151cm', @context['product["variants"].last["title"]'])
+  end
+
+  def test_access_variable_with_hash_notation
+    @context['foo'] = 'baz'
+    @context['bar'] = 'foo'
+
+    assert_equal('baz', @context['["foo"]'])
+    assert_equal('baz', @context['[bar]'])
+  end
+
+  def test_access_hashes_with_hash_access_variables
+    @context['var']      = 'tags'
+    @context['nested']   = { 'var' => 'tags' }
+    @context['products'] = { 'count' => 5, 'tags' => ['deepsnow', 'freestyle'] }
+
+    assert_equal('deepsnow', @context['products[var].first'])
+    assert_equal('freestyle', @context['products[nested.var].last'])
+  end
+
+  def test_hash_notation_only_for_hash_access
+    @context['array'] = [1, 2, 3, 4, 5]
+    @context['hash']  = { 'first' => 'Hello' }
+
+    assert_equal(1, @context['array.first'])
+    assert_nil(@context['array["first"]'])
+    assert_equal('Hello', @context['hash["first"]'])
+  end
+
+  def test_first_can_appear_in_middle_of_callchain
+    @context['product'] = { 'variants' => [{ 'title' => 'draft151cm' }, { 'title' => 'element151cm' }] }
+
+    assert_equal('draft151cm', @context['product.variants[0].title'])
+    assert_equal('element151cm', @context['product.variants[1].title'])
+    assert_equal('draft151cm', @context['product.variants.first.title'])
+    assert_equal('element151cm', @context['product.variants.last.title'])
+  end
+
+  def test_cents
+    @context.merge("cents" => HundredCentes.new)
+    assert_equal(100, @context['cents'])
+  end
+
+  def test_nested_cents
+    @context.merge("cents" => { 'amount' => HundredCentes.new })
+    assert_equal(100, @context['cents.amount'])
+
+    @context.merge("cents" => { 'cents' => { 'amount' => HundredCentes.new } })
+    assert_equal(100, @context['cents.cents.amount'])
+  end
+
+  def test_cents_through_drop
+    @context.merge("cents" => CentsDrop.new)
+    assert_equal(100, @context['cents.amount'])
+  end
+
+  def test_nested_cents_through_drop
+    @context.merge("vars" => { "cents" => CentsDrop.new })
+    assert_equal(100, @context['vars.cents.amount'])
+  end
+
+  def test_drop_methods_with_question_marks
+    @context.merge("cents" => CentsDrop.new)
+    assert(@context['cents.non_zero?'])
+  end
+
+  def test_context_from_within_drop
+    @context.merge("test" => '123', "vars" => ContextSensitiveDrop.new)
+    assert_equal('123', @context['vars.test'])
+  end
+
+  def test_nested_context_from_within_drop
+    @context.merge("test" => '123', "vars" => { "local" => ContextSensitiveDrop.new })
+    assert_equal('123', @context['vars.local.test'])
+  end
+
+  def test_ranges
+    @context.merge("test" => '5')
+    assert_equal((1..5), @context['(1..5)'])
+    assert_equal((1..5), @context['(1..test)'])
+    assert_equal((5..5), @context['(test..test)'])
+  end
+
+  def test_cents_through_drop_nestedly
+    @context.merge("cents" => { "cents" => CentsDrop.new })
+    assert_equal(100, @context['cents.cents.amount'])
+
+    @context.merge("cents" => { "cents" => { "cents" => CentsDrop.new } })
+    assert_equal(100, @context['cents.cents.cents.amount'])
+  end
+
+  def test_drop_with_variable_called_only_once
+    @context['counter'] = CounterDrop.new
+
+    assert_equal(1, @context['counter.count'])
+    assert_equal(2, @context['counter.count'])
+    assert_equal(3, @context['counter.count'])
+  end
+
+  def test_drop_with_key_called_only_once
+    @context['counter'] = CounterDrop.new
+
+    assert_equal(1, @context['counter["count"]'])
+    assert_equal(2, @context['counter["count"]'])
+    assert_equal(3, @context['counter["count"]'])
+  end
+
+  def test_proc_as_variable
+    @context['dynamic'] = proc { 'Hello' }
+
+    assert_equal('Hello', @context['dynamic'])
+  end
+
+  def test_lambda_as_variable
+    @context['dynamic'] = proc { 'Hello' }
+
+    assert_equal('Hello', @context['dynamic'])
+  end
+
+  def test_nested_lambda_as_variable
+    @context['dynamic'] = { "lambda" => proc { 'Hello' } }
+
+    assert_equal('Hello', @context['dynamic.lambda'])
+  end
+
+  def test_array_containing_lambda_as_variable
+    @context['dynamic'] = [1, 2, proc { 'Hello' }, 4, 5]
+
+    assert_equal('Hello', @context['dynamic[2]'])
+  end
+
+  def test_lambda_is_called_once
+    @context['callcount'] = proc {
+      @global ||= 0
+      @global  += 1
+      @global.to_s
+    }
+
+    assert_equal('1', @context['callcount'])
+    assert_equal('1', @context['callcount'])
+    assert_equal('1', @context['callcount'])
+
+    @global = nil
+  end
+
+  def test_nested_lambda_is_called_once
+    @context['callcount'] = { "lambda" => proc {
+                                            @global ||= 0
+                                            @global  += 1
+                                            @global.to_s
+                                          } }
+
+    assert_equal('1', @context['callcount.lambda'])
+    assert_equal('1', @context['callcount.lambda'])
+    assert_equal('1', @context['callcount.lambda'])
+
+    @global = nil
+  end
+
+  def test_lambda_in_array_is_called_once
+    @context['callcount'] = [1, 2, proc {
+                                     @global ||= 0
+                                     @global  += 1
+                                     @global.to_s
+                                   }, 4, 5]
+
+    assert_equal('1', @context['callcount[2]'])
+    assert_equal('1', @context['callcount[2]'])
+    assert_equal('1', @context['callcount[2]'])
+
+    @global = nil
+  end
+
+  def test_access_to_context_from_proc
+    @context.registers[:magic] = 345392
+
+    @context['magic'] = proc { @context.registers[:magic] }
+
+    assert_equal(345392, @context['magic'])
+  end
+
+  def test_to_liquid_and_context_at_first_level
+    @context['category'] = Category.new("foobar")
+    assert_kind_of(CategoryDrop, @context['category'])
+    assert_equal(@context, @context['category'].context)
+  end
+
+  def test_interrupt_avoids_object_allocations
+    assert_no_object_allocations do
+      @context.interrupt?
+    end
+  end
+
+  def test_context_initialization_with_a_proc_in_environment
+    contx = Context.new([test: ->(c) { c['poutine'] }], test: :foo)
+
+    assert(contx)
+    assert_nil(contx['poutine'])
+  end
+
+  def test_apply_global_filter
+    global_filter_proc = ->(output) { "#{output} filtered" }
+
+    context = Context.new
+    context.global_filter = global_filter_proc
+
+    assert_equal('hi filtered', context.apply_global_filter('hi'))
+  end
+
+  def test_static_environments_are_read_with_lower_priority_than_environments
+    context = Context.build(
+      static_environments: { 'shadowed' => 'static', 'unshadowed' => 'static' },
+      environments: { 'shadowed' => 'dynamic' }
+    )
+
+    assert_equal('dynamic', context['shadowed'])
+    assert_equal('static', context['unshadowed'])
+  end
+
+  def test_apply_global_filter_when_no_global_filter_exist
+    context = Context.new
+    assert_equal('hi', context.apply_global_filter('hi'))
+  end
+
+  def test_new_isolated_subcontext_does_not_inherit_variables
+    super_context = Context.new
+    super_context['my_variable'] = 'some value'
+    subcontext = super_context.new_isolated_subcontext
+
+    assert_nil(subcontext['my_variable'])
+  end
+
+  def test_new_isolated_subcontext_inherits_static_environment
+    super_context = Context.build(static_environments: { 'my_environment_value' => 'my value' })
+    subcontext    = super_context.new_isolated_subcontext
+
+    assert_equal('my value', subcontext['my_environment_value'])
+  end
+
+  def test_new_isolated_subcontext_inherits_resource_limits
+    resource_limits = ResourceLimits.new({})
+    super_context   = Context.new({}, {}, {}, false, resource_limits)
+    subcontext      = super_context.new_isolated_subcontext
+    assert_equal(resource_limits, subcontext.resource_limits)
+  end
+
+  def test_new_isolated_subcontext_inherits_exception_renderer
+    super_context = Context.new
+    super_context.exception_renderer = ->(_e) { 'my exception message' }
+    subcontext = super_context.new_isolated_subcontext
+    assert_equal('my exception message', subcontext.handle_error(Liquid::Error.new))
+  end
+
+  def test_new_isolated_subcontext_does_not_inherit_non_static_registers
+    registers = {
+      my_register: :my_value,
+    }
+    super_context = Context.new({}, {}, StaticRegisters.new(registers))
+    super_context.registers[:my_register] = :my_alt_value
+    subcontext                            = super_context.new_isolated_subcontext
+    assert_equal(:my_value, subcontext.registers[:my_register])
+  end
+
+  def test_new_isolated_subcontext_inherits_static_registers
+    super_context = Context.build(registers: { my_register: :my_value })
+    subcontext    = super_context.new_isolated_subcontext
+    assert_equal(:my_value, subcontext.registers[:my_register])
+  end
+
+  def test_new_isolated_subcontext_registers_do_not_pollute_context
+    super_context                      = Context.build(registers: { my_register: :my_value })
+    subcontext                         = super_context.new_isolated_subcontext
+    subcontext.registers[:my_register] = :my_alt_value
+    assert_equal(:my_value, super_context.registers[:my_register])
+  end
+
+  def test_new_isolated_subcontext_inherits_filters
+    my_filter = Module.new do
+      def my_filter(*)
+        'my filter result'
+      end
+    end
+
+    super_context = Context.new
+    super_context.add_filters([my_filter])
+    subcontext    = super_context.new_isolated_subcontext
+    template      = Template.parse('{{ 123 | my_filter }}')
+    assert_equal('my filter result', template.render(subcontext))
+  end
+
+  def test_disables_tag_specified
+    context = Context.new
+    context.with_disabled_tags(%w(foo bar)) do
+      assert_equal true, context.tag_disabled?("foo")
+      assert_equal true, context.tag_disabled?("bar")
+      assert_equal false, context.tag_disabled?("unknown")
+    end
+  end
+
+  def test_disables_nested_tags
+    context = Context.new
+    context.with_disabled_tags(["foo"]) do
+      context.with_disabled_tags(["foo"]) do
+        assert_equal true, context.tag_disabled?("foo")
+        assert_equal false, context.tag_disabled?("bar")
+      end
+      context.with_disabled_tags(["bar"]) do
+        assert_equal true, context.tag_disabled?("foo")
+        assert_equal true, context.tag_disabled?("bar")
+        context.with_disabled_tags(["foo"]) do
+          assert_equal true, context.tag_disabled?("foo")
+          assert_equal true, context.tag_disabled?("bar")
+        end
+      end
+      assert_equal true, context.tag_disabled?("foo")
+      assert_equal false, context.tag_disabled?("bar")
+    end
+  end
+
   def test_override_global_filter
     global = Module.new do
       def notice(output)
@@ -31,4 +618,18 @@ class ContextTest < Minitest::Test
       assert_empty context.errors
     end
   end
-end
+
+  private
+
+  def assert_no_object_allocations
+    unless RUBY_ENGINE == 'ruby'
+      skip("stackprof needed to count object allocations")
+    end
+    require 'stackprof'
+
+    profile = StackProf.run(mode: :object) do
+      yield
+    end
+    assert_equal(0, profile[:samples])
+  end
+end # ContextTest

test/integration/error_handling_test.rb

@@ -261,4 +261,12 @@ class ErrorHandlingTest < Minitest::Test
     assert_equal("Argument error:\nLiquid error (product line 1): argument error", page)
     assert_equal("product", template.errors.first.template_name)
   end
+
+  def test_bug_compatible_silencing_of_errors_in_blank_nodes
+    output = Liquid::Template.parse("{% assign x = 0 %}{% if 1 < '2' %}not blank{% assign x = 3 %}{% endif %}{{ x }}").render
+    assert_equal("Liquid error: comparison of Integer with String failed0", output)
+
+    output = Liquid::Template.parse("{% assign x = 0 %}{% if 1 < '2' %}{% assign x = 3 %}{% endif %}{{ x }}").render
+    assert_equal("0", output)
+  end
 end

test/integration/security_test.rb

@@ -43,14 +43,20 @@ class SecurityTest < Minitest::Test
     assert_equal(expected, Template.parse(text).render!(@assigns, filters: SecurityFilter))
   end
 
-  def test_does_not_add_filters_to_symbol_table
+  def test_does_not_permanently_add_filters_to_symbol_table
     current_symbols = Symbol.all_symbols
 
-    test = %( {{ "some_string" | a_bad_filter }} )
+    # MRI imprecisely marks objects found on the C stack, which can result
+    # in uninitialized memory being marked. This can even result in the test failing
+    # deterministically for a given compilation of ruby. Using a separate thread will
+    # keep these writes of the symbol pointer on a separate stack that will be garbage
+    # collected after Thread#join.
+    Thread.new do
+      test = %( {{ "some_string" | a_bad_filter }} )
+      Template.parse(test).render!
+      nil
+    end.join
 
-    Template.parse(test).render!
-
-    GC.start
     GC.start
 
     assert_equal([], (Symbol.all_symbols - current_symbols))

test/integration/tag_test.rb

@@ -0,0 +1,45 @@
+# frozen_string_literal: true
+
+require 'test_helper'
+
+class TagTest < Minitest::Test
+  include Liquid
+
+  def test_custom_tags_have_a_default_render_to_output_buffer_method_for_backwards_compatibility
+    klass1 = Class.new(Tag) do
+      def render(*)
+        'hello'
+      end
+    end
+
+    with_custom_tag('blabla', klass1) do
+      template = Liquid::Template.parse("{% blabla %}")
+
+      assert_equal 'hello', template.render
+
+      buf    = +''
+      output = template.render({}, output: buf)
+      assert_equal 'hello', output
+      assert_equal 'hello', buf
+      assert_equal buf.object_id, output.object_id
+    end
+
+    klass2 = Class.new(klass1) do
+      def render(*)
+        'foo' + super + 'bar'
+      end
+    end
+
+    with_custom_tag('blabla', klass2) do
+      template = Liquid::Template.parse("{% blabla %}")
+
+      assert_equal 'foohellobar', template.render
+
+      buf    = +''
+      output = template.render({}, output: buf)
+      assert_equal 'foohellobar', output
+      assert_equal 'foohellobar', buf
+      assert_equal buf.object_id, output.object_id
+    end
+  end
+end

test/integration/tags/liquid_tag_test.rb

@@ -82,15 +82,13 @@ class LiquidTagTest < Minitest::Test
   end
 
   def test_nested_liquid_tag
-    assert_usage_increment("liquid_tag_contains_outer_tag", times: 0) do
-      assert_template_result('good', <<~LIQUID)
-        {%- if true %}
-          {%- liquid
-            echo "good"
-          %}
-        {%- endif -%}
-      LIQUID
-    end
+    assert_template_result('good', <<~LIQUID)
+      {%- if true %}
+        {%- liquid
+          echo "good"
+        %}
+      {%- endif -%}
+    LIQUID
   end
 
   def test_cannot_open_blocks_living_past_a_liquid_tag
@@ -102,14 +100,12 @@ class LiquidTagTest < Minitest::Test
     LIQUID
   end
 
-  def test_quirk_can_close_blocks_created_before_a_liquid_tag
-    assert_usage_increment("liquid_tag_contains_outer_tag") do
-      assert_template_result("42", <<~LIQUID)
-        {%- if true -%}
-        42
-        {%- liquid endif -%}
-      LIQUID
-    end
+  def test_cannot_close_blocks_created_before_a_liquid_tag
+    assert_match_syntax_error("syntax error (line 3): 'endif' is not a valid delimiter for liquid tags. use %}", <<~LIQUID)
+      {%- if true -%}
+      42
+      {%- liquid endif -%}
+    LIQUID
   end
 
   def test_liquid_tag_in_raw

test/integration/tags/raw_tag_test.rb

@@ -23,6 +23,7 @@ class RawTagTest < Minitest::Test
     assert_template_result(' Foobar {% {% {% ', '{% raw %} Foobar {% {% {% {% endraw %}')
     assert_template_result(' test {% raw %} {% endraw %}', '{% raw %} test {% raw %} {% {% endraw %}endraw %}')
     assert_template_result(' Foobar {{ invalid 1', '{% raw %} Foobar {{ invalid {% endraw %}{{ 1 }}')
+    assert_template_result(' Foobar {% foo {% bar %}', '{% raw %} Foobar {% foo {% bar %}{% endraw %}')
   end
 
   def test_invalid_raw

test/integration/template_test.rb

@@ -111,13 +111,12 @@ class TemplateTest < Minitest::Test
 
   def test_resource_limits_render_length
     t = Template.parse("0123456789")
-    t.resource_limits.render_length_limit = 5
+    t.resource_limits.render_length_limit = 9
     assert_equal("Liquid error: Memory limits exceeded", t.render)
     assert(t.resource_limits.reached?)
 
     t.resource_limits.render_length_limit = 10
     assert_equal("0123456789", t.render!)
-    refute_nil(t.resource_limits.render_length)
   end
 
   def test_resource_limits_render_score
@@ -176,50 +175,46 @@ class TemplateTest < Minitest::Test
   end
 
   def test_resource_limits_hash_in_template_gets_updated_even_if_no_limits_are_set
-    t = Template.parse("{% for a in (1..100) %} {% assign foo = 1 %} {% endfor %}")
+    t = Template.parse("{% for a in (1..100) %}x{% assign foo = 1 %} {% endfor %}")
     t.render!
     assert(t.resource_limits.assign_score > 0)
     assert(t.resource_limits.render_score > 0)
-    assert(t.resource_limits.render_length > 0)
   end
 
   def test_render_length_persists_between_blocks
     t = Template.parse("{% if true %}aaaa{% endif %}")
-    t.resource_limits.render_length_limit = 7
+    t.resource_limits.render_length_limit = 3
     assert_equal("Liquid error: Memory limits exceeded", t.render)
-    t.resource_limits.render_length_limit = 8
+    t.resource_limits.render_length_limit = 4
     assert_equal("aaaa", t.render)
 
     t = Template.parse("{% if true %}aaaa{% endif %}{% if true %}bbb{% endif %}")
-    t.resource_limits.render_length_limit = 13
+    t.resource_limits.render_length_limit = 6
     assert_equal("Liquid error: Memory limits exceeded", t.render)
-    t.resource_limits.render_length_limit = 14
+    t.resource_limits.render_length_limit = 7
     assert_equal("aaaabbb", t.render)
 
     t = Template.parse("{% if true %}a{% endif %}{% if true %}b{% endif %}{% if true %}a{% endif %}{% if true %}b{% endif %}{% if true %}a{% endif %}{% if true %}b{% endif %}")
     t.resource_limits.render_length_limit = 5
     assert_equal("Liquid error: Memory limits exceeded", t.render)
-    t.resource_limits.render_length_limit = 11
-    assert_equal("Liquid error: Memory limits exceeded", t.render)
-    t.resource_limits.render_length_limit = 12
+    t.resource_limits.render_length_limit = 6
     assert_equal("ababab", t.render)
   end
 
   def test_render_length_uses_number_of_bytes_not_characters
     t = Template.parse("{% if true %}すごい{% endif %}")
-    t.resource_limits.render_length_limit = 10
+    t.resource_limits.render_length_limit = 8
     assert_equal("Liquid error: Memory limits exceeded", t.render)
-    t.resource_limits.render_length_limit = 18
+    t.resource_limits.render_length_limit = 9
     assert_equal("すごい", t.render)
   end
 
   def test_default_resource_limits_unaffected_by_render_with_context
     context = Context.new
-    t = Template.parse("{% for a in (1..100) %} {% assign foo = 1 %} {% endfor %}")
+    t = Template.parse("{% for a in (1..100) %}x{% assign foo = 1 %} {% endfor %}")
     t.render!(context)
     assert(context.resource_limits.assign_score > 0)
     assert(context.resource_limits.render_score > 0)
-    assert(context.resource_limits.render_length > 0)
   end
 
   def test_can_use_drop_as_context

test/integration/trim_mode_test.rb

@@ -528,4 +528,32 @@ class TrimModeTest < Minitest::Test
     END_EXPECTED
     assert_template_result(expected, text)
   end
+
+  def test_pre_trim_blank_preceding_text
+    template = Liquid::Template.parse("\n{%- raw %}{% endraw %}")
+    assert_equal("", template.render)
+
+    template = Liquid::Template.parse("\n{%- if true %}{% endif %}")
+    assert_equal("", template.render)
+
+    template = Liquid::Template.parse("{{ 'B' }} \n{%- if true %}C{% endif %}")
+    assert_equal("BC", template.render)
+  end
+
+  def test_bug_compatible_pre_trim
+    template = Liquid::Template.parse("\n {%- raw %}{% endraw %}", bug_compatible_whitespace_trimming: true)
+    assert_equal("\n", template.render)
+
+    template = Liquid::Template.parse("\n {%- if true %}{% endif %}", bug_compatible_whitespace_trimming: true)
+    assert_equal("\n", template.render)
+
+    template = Liquid::Template.parse("{{ 'B' }} \n{%- if true %}C{% endif %}", bug_compatible_whitespace_trimming: true)
+    assert_equal("B C", template.render)
+
+    template = Liquid::Template.parse("B\n {%- raw %}{% endraw %}", bug_compatible_whitespace_trimming: true)
+    assert_equal("B", template.render)
+
+    template = Liquid::Template.parse("B\n {%- if true %}{% endif %}", bug_compatible_whitespace_trimming: true)
+    assert_equal("B", template.render)
+  end
 end # TrimModeTest

test/unit/block_unit_test.rb

@@ -45,53 +45,9 @@ class BlockUnitTest < Minitest::Test
     assert_equal(3, template.root.nodelist.size)
   end
 
-  def test_with_custom_tag
-    with_custom_tag('testtag', Block) do
-      assert Liquid::Template.parse("{% testtag %} {% endtesttag %}")
-    end
-  end
-
-  def test_custom_block_tags_have_a_default_render_to_output_buffer_method_for_backwards_compatibility
-    klass1 = Class.new(Block) do
-      def render(*)
-        'hello'
-      end
-    end
-
-    with_custom_tag('blabla', klass1) do
-      template = Liquid::Template.parse("{% blabla %} bla {% endblabla %}")
-
-      assert_equal 'hello', template.render
-
-      buf    = +''
-      output = template.render({}, output: buf)
-      assert_equal 'hello', output
-      assert_equal 'hello', buf
-      assert_equal buf.object_id, output.object_id
-    end
-
-    klass2 = Class.new(klass1) do
-      def render(*)
-        'foo' + super + 'bar'
-      end
-    end
-
-    with_custom_tag('blabla', klass2) do
-      template = Liquid::Template.parse("{% blabla %} foo {% endblabla %}")
-
-      assert_equal 'foohellobar', template.render
-
-      buf    = +''
-      output = template.render({}, output: buf)
-      assert_equal 'foohellobar', output
-      assert_equal 'foohellobar', buf
-      assert_equal buf.object_id, output.object_id
-    end
-  end
-
   private
 
   def block_types(nodelist)
     nodelist.collect(&:class)
   end
-end # VariableTest
+end

test/unit/optimizer_test.rb

@@ -0,0 +1,16 @@
+# frozen_string_literal: true
+
+require 'test_helper'
+
+class OptimizerUnitTest < Minitest::Test
+  include Liquid
+
+  def test_combines_append_filters
+    optimizer = Optimizer.new
+    var = Variable.new('hello | append: "a" | append: b', ParseContext.new)
+    var = optimizer.optimize(var)
+    assert_equal([
+      ['append_all', ["a", VariableLookup.new("b")]]
+    ], var.filters)
+  end
+end

test/unit/strainer_factory_unit_test.rb

@@ -6,11 +6,11 @@ class StrainerFactoryUnitTest < Minitest::Test
   include Liquid
 
   module AccessScopeFilters
-    def public_filter(_input)
+    def public_filter
       "public"
     end
 
-    def private_filter(_input)
+    def private_filter
       "private"
     end
     private :private_filter
@@ -31,13 +31,13 @@ class StrainerFactoryUnitTest < Minitest::Test
   def test_strainer
     strainer = StrainerFactory.create(@context)
     assert_equal(5, strainer.invoke('size', 'input'))
-    assert_equal("public", strainer.invoke("public_filter", 'input'))
+    assert_equal("public", strainer.invoke("public_filter"))
   end
 
   def test_stainer_raises_argument_error
     strainer = StrainerFactory.create(@context)
     assert_raises(Liquid::ArgumentError) do
-      strainer.invoke("public_filter", 'input', 1)
+      strainer.invoke("public_filter", 1)
     end
   end
 
@@ -45,11 +45,11 @@ class StrainerFactoryUnitTest < Minitest::Test
     strainer = StrainerFactory.create(@context)
 
     exception = assert_raises(Liquid::ArgumentError) do
-      strainer.invoke("public_filter", 'input', 1)
+      strainer.invoke("public_filter", 1)
     end
 
     assert_match(
-      /\ALiquid error: wrong number of arguments \((2 for 1|given 2, expected 1)\)\z/,
+      /\ALiquid error: wrong number of arguments \((1 for 0|given 1, expected 0)\)\z/,
       exception.message
     )
     assert_equal(exception.backtrace[0].split(':')[0], __FILE__)
@@ -66,8 +66,8 @@ class StrainerFactoryUnitTest < Minitest::Test
 
   def test_strainer_returns_nil_if_no_filter_method_found
     strainer = StrainerFactory.create(@context)
-    assert_nil(strainer.invoke("private_filter", nil))
-    assert_nil(strainer.invoke("undef_the_filter", nil))
+    assert_nil(strainer.invoke("private_filter"))
+    assert_nil(strainer.invoke("undef_the_filter"))
   end
 
   def test_strainer_returns_first_argument_if_no_method_and_arguments_given

test/unit/strainer_template_unit_test.rb

@@ -60,7 +60,7 @@ class StrainerTemplateUnitTest < Minitest::Test
     strainer = Context.new.strainer
     with_global_filter do
       strainer.class.add_filter(PublicMethodOverrideFilter)
-      assert(strainer.class.send(:filter_methods).include?(:public_filter))
+      assert(strainer.class.send(:filter_methods).include?('public_filter'))
     end
   end
 

test/unit/tag_unit_test.rb

@@ -20,42 +20,4 @@ class TagUnitTest < Minitest::Test
     tag = Tag.parse("some_tag", "", Tokenizer.new(""), ParseContext.new)
     assert_equal('some_tag', tag.tag_name)
   end
-
-  def test_custom_tags_have_a_default_render_to_output_buffer_method_for_backwards_compatibility
-    klass1 = Class.new(Tag) do
-      def render(*)
-        'hello'
-      end
-    end
-
-    with_custom_tag('blabla', klass1) do
-      template = Liquid::Template.parse("{% blabla %}")
-
-      assert_equal 'hello', template.render
-
-      buf    = +''
-      output = template.render({}, output: buf)
-      assert_equal 'hello', output
-      assert_equal 'hello', buf
-      assert_equal buf.object_id, output.object_id
-    end
-
-    klass2 = Class.new(klass1) do
-      def render(*)
-        'foo' + super + 'bar'
-      end
-    end
-
-    with_custom_tag('blabla', klass2) do
-      template = Liquid::Template.parse("{% blabla %}")
-
-      assert_equal 'foohellobar', template.render
-
-      buf    = +''
-      output = template.render({}, output: buf)
-      assert_equal 'foohellobar', output
-      assert_equal 'foohellobar', buf
-      assert_equal buf.object_id, output.object_id
-    end
-  end
 end