Optimize filter invocation

lib/liquid/block_body.rb

@@ -186,8 +186,8 @@ module Liquid
     end
 
     def create_variable(token, parse_context)
-      token.scan(ContentOfVariable) do |content|
+      if token =~ ContentOfVariable
-        markup = content.first
+        markup = Regexp.last_match(1)
         return Variable.new(markup, parse_context)
       end
       raise_missing_variable_terminator(token, parse_context)

lib/liquid/context.rb

@@ -93,8 +93,13 @@ module Liquid
       exception_renderer.call(e).to_s
     end
 
-    def invoke(method, *args)
+    def invoke(method, input, *args)
-      strainer.invoke(method, *args).to_liquid
+      strainer.invoke(method, input, *args).to_liquid
+    end
+
+    # @api private
+    def invoke_filter(method, input, args)
+      strainer.invoke_filter(method, input, args).to_liquid
     end
 
     # Push new local scope on the stack. use <tt>Context#stack</tt> instead

lib/liquid/strainer_template.rb

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

lib/liquid/variable.rb

@@ -18,7 +18,7 @@ module Liquid
     JustTagAttributes        = /\A#{TagAttributes}\z/o
     MarkupWithQuotedFragment = /(#{QuotedFragment})(.*)/om
 
-    attr_accessor :filters, :name, :line_number
+    attr_accessor :name, :line_number
     attr_reader :parse_context
     alias_method :options, :parse_context
 
@@ -81,12 +81,15 @@ module Liquid
     end
 
     def render(context)
-      obj = @filters.inject(context.evaluate(@name)) do |output, (filter_name, filter_args, filter_kwargs)|
+      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)
-        context.invoke(filter_name, output, *filter_args)
+        end
+        result = context.invoke_filter(filter_name, result, filter_args)
       end
 
-      context.apply_global_filter(obj)
+      context.apply_global_filter(result)
     end
 
     def render_to_output_buffer(context, output)
@@ -110,21 +113,49 @@ 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 = []
       keyword_args = nil
       unparsed_args.each do |a|
         if (matches = a.match(JustTagAttributes))
           keyword_args ||= {}
-          keyword_args[matches[1]] = Expression.parse(matches[2])
+          expression = Expression.parse(matches[2])
+          constant_args &&= !expression.is_a?(VariableLookup)
+          keyword_args[matches[1]] = expression
         else
-          filter_args << Expression.parse(a)
+          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
         end
       end
-      result = [filter_name, filter_args]
-      result << keyword_args if keyword_args
       result
     end
 

test/integration/security_test.rb

@@ -48,10 +48,11 @@ class SecurityTest < Minitest::Test
 
     test = %( {{ "some_string" | a_bad_filter }} )
 
-    template = Template.parse(test)
+    Template.parse(test).render!
-    assert_equal([], (Symbol.all_symbols - current_symbols))
+
+    GC.start
+    GC.start
 
-    template.render!
     assert_equal([], (Symbol.all_symbols - current_symbols))
   end
 

test/unit/strainer_factory_unit_test.rb

@@ -6,11 +6,11 @@ class StrainerFactoryUnitTest < Minitest::Test
   include Liquid
 
   module AccessScopeFilters
-    def public_filter
+    def public_filter(_input)
       "public"
     end
 
-    def private_filter
+    def private_filter(_input)
       "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"))
+    assert_equal("public", strainer.invoke("public_filter", 'input'))
   end
 
   def test_stainer_raises_argument_error
     strainer = StrainerFactory.create(@context)
     assert_raises(Liquid::ArgumentError) do
-      strainer.invoke("public_filter", 1)
+      strainer.invoke("public_filter", 'input', 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", 1)
+      strainer.invoke("public_filter", 'input', 1)
     end
 
     assert_match(
-      /\ALiquid error: wrong number of arguments \((1 for 0|given 1, expected 0)\)\z/,
+      /\ALiquid error: wrong number of arguments \((2 for 1|given 2, expected 1)\)\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"))
+    assert_nil(strainer.invoke("private_filter", nil))
-    assert_nil(strainer.invoke("undef_the_filter"))
+    assert_nil(strainer.invoke("undef_the_filter", nil))
   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