Remove to_liquid conversion from StandardFilters

Co-authored-by: Dylan Thacker-Smith <[email protected]>

.github/workflows/liquid.yml

@@ -7,7 +7,7 @@ jobs:
       matrix:
         entry:
           - { ruby: 2.5, allowed-failure: false } # minimum supported
-          - { ruby: 3.0, allowed-failure: false } # latest
+          - { ruby: 3.1, allowed-failure: false } # latest
           - { ruby: ruby-head, allowed-failure: true }
     name: test (${{ matrix.entry.ruby }})
     steps:

History.md

@@ -1,5 +1,20 @@
 # Liquid Change Log
 
+## 5.3.0 (unreleased)
+
+### Deprecation
+* Condition#evaluate to require mandatory context argument in Liquid 6.0.0 (#1527) [Thierry Joyal]
+
+## 5.2.0 2022-03-01
+
+### Features
+* Add `remove_last`, and `replace_last` filters (#1422) [Anders Hagbard]
+* Eagerly cache global filters (#1524) [Jean Boussier]
+
+### Fixes
+* Fix some internal errors in filters from invalid input (#1476) [Dylan Thacker-Smith]
+* Allow dash in filter kwarg name for consistency with Liquid::C (#1518) [CP Clermont]
+
 ## 5.1.0 / 2021-09-09
 
 ### Features

README.md

@@ -5,7 +5,7 @@
 
 * [Contributing guidelines](CONTRIBUTING.md)
 * [Version history](History.md)
-* [Liquid documentation from Shopify](http://docs.shopify.com/themes/liquid-basics)
+* [Liquid documentation from Shopify](https://shopify.dev/api/liquid)
 * [Liquid Wiki at GitHub](https://github.com/Shopify/liquid/wiki)
 * [Website](http://liquidmarkup.org/)
 
@@ -56,7 +56,7 @@ For standard use you can just pass it the content of a file and call render with
 
 Setting the error mode of Liquid lets you specify how strictly you want your templates to be interpreted.
 Normally the parser is very lax and will accept almost anything without error. Unfortunately this can make
-it very hard to debug and can lead to unexpected behaviour. 
+it very hard to debug and can lead to unexpected behaviour.
 
 Liquid also comes with a stricter parser that can be used when editing templates to give better error messages
 when templates are invalid. You can enable this new parser like this:

lib/liquid.rb

@@ -36,7 +36,7 @@ module Liquid
   VariableIncompleteEnd       = /\}\}?/
   QuotedString                = /"[^"]*"|'[^']*'/
   QuotedFragment              = /#{QuotedString}|(?:[^\s,\|'"]|#{QuotedString})+/o
-  TagAttributes               = /(\w+)\s*\:\s*(#{QuotedFragment})/o
+  TagAttributes               = /(\w[\w-]*)\s*\:\s*(#{QuotedFragment})/o
   AnyStartingTag              = /#{TagStart}|#{VariableStart}/o
   PartialTemplateParser       = /#{TagStart}.*?#{TagEnd}|#{VariableStart}.*?#{VariableIncompleteEnd}/om
   TemplateParser              = /(#{PartialTemplateParser}|#{AnyStartingTag})/om
@@ -59,8 +59,8 @@ require 'liquid/forloop_drop'
 require 'liquid/extensions'
 require 'liquid/errors'
 require 'liquid/interrupts'
-require 'liquid/strainer_factory'
 require 'liquid/strainer_template'
+require 'liquid/strainer_factory'
 require 'liquid/expression'
 require 'liquid/context'
 require 'liquid/parser_switching'

lib/liquid/block_body.rb

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

lib/liquid/condition.rb

@@ -61,7 +61,7 @@ module Liquid
       @child_condition = nil
     end
 
-    def evaluate(context = Context.new)
+    def evaluate(context = deprecated_default_context)
       condition = self
       result = nil
       loop do
@@ -150,6 +150,12 @@ module Liquid
       end
     end
 
+    def deprecated_default_context
+      warn("DEPRECATION WARNING: Condition#evaluate without a context argument is deprecated" \
+        " and will be removed from Liquid 6.0.0.")
+      Context.new
+    end
+
     class ParseTreeVisitor < Liquid::ParseTreeVisitor
       def children
         [

lib/liquid/context.rb

@@ -187,16 +187,11 @@ module Liquid
       # path and find_index() is optimized in MRI to reduce object allocation
       index = @scopes.find_index { |s| s.key?(key) }
 
-      variable = if index
+      if index
         lookup_and_evaluate(@scopes[index], key, raise_on_not_found: raise_on_not_found)
       else
         try_variable_find_in_environments(key, raise_on_not_found: raise_on_not_found)
       end
-
-      variable         = variable.to_liquid
-      variable.context = self if variable.respond_to?(:context=)
-
-      variable
     end
 
     def lookup_and_evaluate(obj, key, raise_on_not_found: true)
@@ -206,11 +201,17 @@ module Liquid
 
       value = obj[key]
 
-      if value.is_a?(Proc) && obj.respond_to?(:[]=)
-        obj[key] = value.arity == 0 ? value.call : value.call(self)
-      else
-        value
+      # Skip contextualization and memoization when not found
+      return if value.nil?
+
+      value = contextualize(value)
+
+      # Memoization layer: Proc resolution and other to_liquid computations are persisted
+      if obj.respond_to?(:[]=) && !obj.frozen?
+        obj[key] = value
       end
+
+      value
     end
 
     def with_disabled_tags(tag_names)
@@ -228,6 +229,58 @@ module Liquid
       @disabled_tags.fetch(tag_name, 0) > 0
     end
 
+    # Convert input objects into liquid aware representations
+    # Also assigns the context (self) through context=
+    def contextualize(object)
+      if object.is_a?(Proc)
+        object = object.arity == 0 ? object.call : object.call(self)
+      end
+
+      # TODO: This is a block of code that needs some extra polish
+      # My goal is to centralize as much as possible the contextualization/sanitization of objects being exchanged
+      #   between the inputs given by the caller and templates being executed.
+      # I desire for Filters to not have to worry about object conversion (eg.: StandardFilters#each)
+      #   Filters implemented outside Shopify/Liquid shouldn't have to worry about this layer of internals
+      #   Using a filter shouldn't create a worry for data leak and missing context
+      # Running `ruby -I test` with the following implementation is successful
+      # Running `rake test` which will also run tests with the liquid-c gem will lead to errors
+      #   This is due to liquid-c optimizing some code paths
+      #   Eg.: https://github.com/Shopify/liquid-c/blame/master/ext/liquid_c/context.h#L44-L45
+      #   We would need to also add the following code in liquid-c
+      # If we were to only consider Array and Hash as special use cases, this might be worth the effort
+      #   Alternatively I think considering Array and Hash as the only two cases of nested objects is not quite right
+      #   It might be better for extension.rb to be responsible for this. Array#to_liquid to return self is somewhat be wrong
+      #   One does not prevent the other, need to make sure the generic implementation works and we can optimize over it
+      # Note: Moving this logic to the different patches in extensions.rb
+      #   Some changes in liquid-c is most likely required
+      #   Liquid-c has early return optimization I have yet to track all code paths it relates to
+      #   if (klass == rb_cString || klass == rb_cArray || klass == rb_cHash)
+      #         return value;
+      if object.is_a?(Array)
+        object = object.map do |obj|
+          contextualize(obj)
+        end
+      elsif object.is_a?(Hash)
+        new_obj = {}
+        object.map do |k, obj|
+          new_obj[k] = contextualize(obj)
+        end
+        object = new_obj
+      else
+        object = object.to_liquid
+
+        # TODO: Ideally all contextualized object would define "context=" even if they perform a noop
+        # We want to ensure non-liquid objects aren't leaked in the context visible to the templates
+        # Having a standard interface is a direction we can take.
+        #   Alternatively, we could impose only a pre-determine array of available type is allowed
+        #   Eg.: String, Integer, Symbol, Liquid::Drop not SomeCustomModelFromTheHostApplication
+        # For now this might not be as pressing issue to deal with
+        object.context = self if object.respond_to?(:context=)
+      end
+
+      object
+    end
+
     protected
 
     attr_writer :base_scope_depth, :warnings, :errors, :strainer, :filters, :disabled_tags

lib/liquid/expression.rb

@@ -10,21 +10,23 @@ module Liquid
       'empty' => ''
     }.freeze
 
-    SINGLE_QUOTED_STRING = /\A\s*'(.*)'\s*\z/m
-    DOUBLE_QUOTED_STRING = /\A\s*"(.*)"\s*\z/m
-    INTEGERS_REGEX       = /\A\s*(-?\d+)\s*\z/
-    FLOATS_REGEX         = /\A\s*(-?\d[\d\.]+)\s*\z/
+    INTEGERS_REGEX       = /\A(-?\d+)\z/
+    FLOATS_REGEX         = /\A(-?\d[\d\.]+)\z/
 
     # Use an atomic group (?>...) to avoid pathological backtracing from
     # malicious input as described in https://github.com/Shopify/liquid/issues/1357
-    RANGES_REGEX         = /\A\s*\(\s*(?>(\S+)\s*\.\.)\s*(\S+)\s*\)\s*\z/
+    RANGES_REGEX         = /\A\(\s*(?>(\S+)\s*\.\.)\s*(\S+)\s*\)\z/
 
     def self.parse(markup)
+      return nil unless markup
+
+      markup = markup.strip
+      if (markup.start_with?('"') && markup.end_with?('"')) ||
+         (markup.start_with?("'") && markup.end_with?("'"))
+        return markup[1..-2]
+      end
+
       case markup
-      when nil
-        nil
-      when SINGLE_QUOTED_STRING, DOUBLE_QUOTED_STRING
-        Regexp.last_match(1)
       when INTEGERS_REGEX
         Regexp.last_match(1).to_i
       when RANGES_REGEX
@@ -32,7 +34,6 @@ module Liquid
       when FLOATS_REGEX
         Regexp.last_match(1).to_f
       else
-        markup = markup.strip
         if LITERALS.key?(markup)
           LITERALS[markup]
         else

lib/liquid/standardfilters.rb

@@ -213,17 +213,23 @@ module Liquid
 
       if ary.empty?
         []
-      elsif ary.first.respond_to?(:[]) && target_value.nil?
-        begin
-          ary.select { |item| item[property] }
+      elsif target_value.nil?
+        ary.select do |item|
+          item[property]
         rescue TypeError
           raise_property_error(property)
+        rescue NoMethodError
+          return nil unless item.respond_to?(:[])
+          raise
         end
-      elsif ary.first.respond_to?(:[])
-        begin
-          ary.select { |item| item[property] == target_value }
+      else
+        ary.select do |item|
+          item[property] == target_value
         rescue TypeError
           raise_property_error(property)
+        rescue NoMethodError
+          return nil unless item.respond_to?(:[])
+          raise
         end
       end
     end
@@ -237,11 +243,14 @@ module Liquid
         ary.uniq
       elsif ary.empty? # The next two cases assume a non-empty array.
         []
-      elsif ary.first.respond_to?(:[])
-        begin
-          ary.uniq { |a| a[property] }
+      else
+        ary.uniq do |item|
+          item[property]
         rescue TypeError
           raise_property_error(property)
+        rescue NoMethodError
+          return nil unless item.respond_to?(:[])
+          raise
         end
       end
     end
@@ -277,11 +286,14 @@ module Liquid
         ary.compact
       elsif ary.empty? # The next two cases assume a non-empty array.
         []
-      elsif ary.first.respond_to?(:[])
-        begin
-          ary.reject { |a| a[property].nil? }
+      else
+        ary.reject do |item|
+          item[property].nil?
         rescue TypeError
           raise_property_error(property)
+        rescue NoMethodError
+          return nil unless item.respond_to?(:[])
+          raise
         end
       end
     end
@@ -296,14 +308,34 @@ module Liquid
       input.to_s.sub(string.to_s, replacement.to_s)
     end
 
+    # Replace the last occurrences of a string with another
+    def replace_last(input, string, replacement)
+      input = input.to_s
+      string = string.to_s
+      replacement = replacement.to_s
+
+      start_index = input.rindex(string)
+
+      return input unless start_index
+
+      output = input.dup
+      output[start_index, string.length] = replacement
+      output
+    end
+
     # remove a substring
     def remove(input, string)
-      input.to_s.gsub(string.to_s, '')
+      replace(input, string, '')
     end
 
     # remove the first occurrences of a substring
     def remove_first(input, string)
-      input.to_s.sub(string.to_s, '')
+      replace_first(input, string, '')
+    end
+
+    # remove the last occurences of a substring
+    def remove_last(input, string)
+      replace_last(input, string, '')
     end
 
     # add one string to another
@@ -486,10 +518,16 @@ module Liquid
     end
 
     def nil_safe_compare(a, b)
-      if !a.nil? && !b.nil?
-        a <=> b
+      result = a <=> b
+
+      if result
+        result
+      elsif a.nil?
+        1
+      elsif b.nil?
+        -1
       else
-        a.nil? ? 1 : -1
+        raise Liquid::ArgumentError, "cannot sort values of incompatible types"
       end
     end
 
@@ -544,8 +582,7 @@ module Liquid
 
       def each
         @input.each do |e|
-          e.context = @context if e.respond_to?(:context=)
-          yield(e.respond_to?(:to_liquid) ? e.to_liquid : e)
+          yield(e)
         end
       end
     end

lib/liquid/static_registers.rb

@@ -31,7 +31,11 @@ module Liquid
       if @registers.key?(key)
         @registers.fetch(key)
       elsif default != UNDEFINED
-        @static.fetch(key, default, &block)
+        if block_given?
+          @static.fetch(key, &block)
+        else
+          @static.fetch(key, default)
+        end
       else
         @static.fetch(key, &block)
       end

lib/liquid/strainer_factory.rb

@@ -7,25 +7,26 @@ module Liquid
 
     def add_global_filter(filter)
       strainer_class_cache.clear
-      global_filters << filter
+      GlobalCache.add_filter(filter)
     end
 
     def create(context, filters = [])
       strainer_from_cache(filters).new(context)
     end
 
+    GlobalCache = Class.new(StrainerTemplate)
+
     private
 
-    def global_filters
-      @global_filters ||= []
-    end
-
     def strainer_from_cache(filters)
-      strainer_class_cache[filters] ||= begin
-        klass = Class.new(StrainerTemplate)
-        global_filters.each { |f| klass.add_filter(f) }
-        filters.each { |f| klass.add_filter(f) }
-        klass
+      if filters.empty?
+        GlobalCache
+      else
+        strainer_class_cache[filters] ||= begin
+          klass = Class.new(GlobalCache)
+          filters.each { |f| klass.add_filter(f) }
+          klass
+        end
       end
     end
 

lib/liquid/strainer_template.rb

@@ -31,6 +31,11 @@ module Liquid
         filter_methods.include?(method.to_s)
       end
 
+      def inherited(subclass)
+        super
+        subclass.instance_variable_set(:@filter_methods, @filter_methods.dup)
+      end
+
       private
 
       def filter_methods

lib/liquid/variable_lookup.rb

@@ -49,15 +49,17 @@ module Liquid
             ((object.respond_to?(:key?) && object.key?(key)) ||
              (object.respond_to?(:fetch) && key.is_a?(Integer)))
 
-          # if its a proc we will replace the entry with the proc
-          res    = context.lookup_and_evaluate(object, key)
-          object = res.to_liquid
+          object = context.lookup_and_evaluate(object, key)
 
           # Some special cases. If the part wasn't in square brackets and
           # no key with the same name was found we interpret following calls
           # as commands and call them on the current object
         elsif @command_flags & (1 << i) != 0 && object.respond_to?(key)
-          object = object.send(key).to_liquid
+
+          # TODO: These do not go through lookup_and_evaluate.
+          # Let's see if we can move the conversion back to Context
+          object = object.send(key)
+          object = context.contextualize(object)
 
           # No key was present with the desired value and it wasn't one of the directly supported
           # keywords either. The only thing we got left is to return nil or
@@ -66,9 +68,6 @@ module Liquid
           return nil unless context.strict_variables
           raise Liquid::UndefinedVariable, "undefined variable #{key}"
         end
-
-        # If we are dealing with a drop here we have to
-        object.context = context if object.respond_to?(:context=)
       end
 
       object

lib/liquid/version.rb

@@ -2,5 +2,5 @@
 # frozen_string_literal: true
 
 module Liquid
-  VERSION = "5.1.0"
+  VERSION = "5.3.0.alpha"
 end

test/integration/context_test.rb

@@ -24,7 +24,7 @@ class ContextSensitiveDrop < Liquid::Drop
   end
 end
 
-class Category < Liquid::Drop
+class Category
   attr_accessor :name
 
   def initialize(name)
@@ -36,8 +36,9 @@ class Category < Liquid::Drop
   end
 end
 
-class CategoryDrop
+class CategoryDrop < Liquid::Drop
   attr_accessor :category, :context
+
   def initialize(category)
     @category = category
   end
@@ -405,45 +406,42 @@ class ContextTest < Minitest::Test
   end
 
   def test_lambda_is_called_once
+    @global = 0
+
     @context['callcount'] = proc {
-      @global ||= 0
-      @global  += 1
+      @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
+    @global = 0
+
     @context['callcount'] = { "lambda" => proc {
-                                            @global ||= 0
-                                            @global  += 1
+                                            @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
+    @global = 0
+
     @context['callcount'] = [1, 2, proc {
-                                     @global ||= 0
-                                     @global  += 1
+                                     @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

test/integration/drop_test.rb

@@ -149,6 +149,8 @@ class DropsTest < Minitest::Test
     assert_equal(' carrot ', output)
   end
 
+  # This test succeed in the ruby implementation, but not in liquid-c
+  # See Context#contextualize
   def test_context_drop_array_with_map
     output = Liquid::Template.parse(' {{ contexts | map: "bar" }} ').render!('contexts' => [ContextDrop.new, ContextDrop.new], 'bar' => "carrot")
     assert_equal(' carrotcarrot ', output)

test/integration/filter_kwarg_test.rb

@@ -0,0 +1,24 @@
+# frozen_string_literal: true
+
+require 'test_helper'
+
+class FilterKwargTest < Minitest::Test
+  module KwargFilter
+    def html_tag(_tag, attributes)
+      attributes
+        .map { |key, value| "#{key}='#{value}'" }
+        .join(' ')
+    end
+  end
+
+  include Liquid
+
+  def test_can_parse_data_kwargs
+    with_global_filter(KwargFilter) do
+      assert_equal(
+        "data-src='src' data-widths='100, 200'",
+        Template.parse("{{ 'img' | html_tag: data-src: 'src', data-widths: '100, 200' }}").render(nil, nil)
+      )
+    end
+  end
+end

test/integration/standard_filter_test.rb

@@ -3,10 +3,6 @@
 
 require 'test_helper'
 
-class Filters
-  include Liquid::StandardFilters
-end
-
 class TestThing
   attr_reader :foo
 
@@ -53,10 +49,13 @@ class NumberLikeThing < Liquid::Drop
 end
 
 class StandardFiltersTest < Minitest::Test
+  Filters = Class.new(Liquid::StrainerTemplate)
+  Filters.add_filter(Liquid::StandardFilters)
+
   include Liquid
 
   def setup
-    @filters = Filters.new
+    @filters = Filters.new(Context.new)
   end
 
   def test_size
@@ -259,8 +258,8 @@ class StandardFiltersTest < Minitest::Test
       { "price" => 1, "handle" => "gamma" },
       { "price" => 2, "handle" => "epsilon" },
       { "price" => 4, "handle" => "alpha" },
-      { "handle" => "delta" },
       { "handle" => "beta" },
+      { "handle" => "delta" },
     ]
     assert_equal(expectation, @filters.sort(input, "price"))
   end
@@ -418,6 +417,8 @@ class StandardFiltersTest < Minitest::Test
     assert_template_result("", '{{ "foo" | map: "inspect" }}')
   end
 
+  # This test succeed in the ruby implementation, but not in liquid-c
+  # See Context#contextualize
   def test_map_calls_to_liquid
     t = TestThing.new
     assert_template_result("woot: 1", '{{ foo | map: "whatever" }}', "foo" => [t])
@@ -434,6 +435,8 @@ class StandardFiltersTest < Minitest::Test
     assert_template_result("42", template, "thing" => hash)
   end
 
+  # This test succeed in the ruby implementation, but not in liquid-c
+  # See Context#contextualize
   def test_sort_calls_to_liquid
     t = TestThing.new
     Liquid::Template.parse('{{ foo | sort: "whatever" }}').render("foo" => [t])
@@ -539,19 +542,31 @@ class StandardFiltersTest < Minitest::Test
   end
 
   def test_replace
-    assert_equal('2 2 2 2', @filters.replace('1 1 1 1', '1', 2))
+    assert_equal('b b b b', @filters.replace('a a a a', 'a', 'b'))
     assert_equal('2 2 2 2', @filters.replace('1 1 1 1', 1, 2))
-    assert_equal('2 1 1 1', @filters.replace_first('1 1 1 1', '1', 2))
+    assert_equal('1 1 1 1', @filters.replace('1 1 1 1', 2, 3))
+    assert_template_result('2 2 2 2', "{{ '1 1 1 1' | replace: '1', 2 }}")
+
+    assert_equal('b a a a', @filters.replace_first('a a a a', 'a', 'b'))
     assert_equal('2 1 1 1', @filters.replace_first('1 1 1 1', 1, 2))
+    assert_equal('1 1 1 1', @filters.replace_first('1 1 1 1', 2, 3))
     assert_template_result('2 1 1 1', "{{ '1 1 1 1' | replace_first: '1', 2 }}")
+
+    assert_equal('a a a b', @filters.replace_last('a a a a', 'a', 'b'))
+    assert_equal('1 1 1 2', @filters.replace_last('1 1 1 1', 1, 2))
+    assert_equal('1 1 1 1', @filters.replace_last('1 1 1 1', 2, 3))
+    assert_template_result('1 1 1 2', "{{ '1 1 1 1' | replace_last: '1', 2 }}")
   end
 
   def test_remove
     assert_equal('   ', @filters.remove("a a a a", 'a'))
-    assert_equal('   ', @filters.remove("1 1 1 1", 1))
-    assert_equal('a a a', @filters.remove_first("a a a a", 'a '))
-    assert_equal(' 1 1 1', @filters.remove_first("1 1 1 1", 1))
-    assert_template_result('a a a', "{{ 'a a a a' | remove_first: 'a ' }}")
+    assert_template_result('   ', "{{ '1 1 1 1' | remove: 1 }}")
+
+    assert_equal('b a a', @filters.remove_first("a b a a", 'a '))
+    assert_template_result(' 1 1 1', "{{ '1 1 1 1' | remove_first: 1 }}")
+
+    assert_equal('a a b', @filters.remove_last("a a b a", ' a'))
+    assert_template_result('1 1 1 ', "{{ '1 1 1 1' | remove_last: 1 }}")
   end
 
   def test_pipes_in_string_arguments
@@ -850,21 +865,16 @@ class StandardFiltersTest < Minitest::Test
       { foo: "bar" },
       [{ "foo" => "bar" }, { "foo" => 123 }, { "foo" => nil }, { "foo" => true }, { "foo" => ["foo", "bar"] }],
       { 1 => "bar" },
-      ["foo", 123, nil, true, false, Drop, ["foo"], { foo: "bar" }],
+      ["foo", 123, nil, true, false, test_drop, test_enum, ["foo"], { foo: "bar" }],
     ]
-    test_types.each do |first|
-      test_types.each do |other|
-        (@filters.methods - Object.methods).each do |method|
-          arg_count = @filters.method(method).arity
-          arg_count *= -1 if arg_count < 0
-          inputs = [first]
-          inputs << ([other] * (arg_count - 1)) if arg_count > 1
-          begin
-            @filters.send(method, *inputs)
-          rescue Liquid::ArgumentError, Liquid::ZeroDivisionError
-            nil
-          end
-        end
+    StandardFilters.public_instance_methods(false).each do |method|
+      arg_count = @filters.method(method).arity
+      arg_count *= -1 if arg_count < 0
+
+      test_types.repeated_permutation(arg_count) do |args|
+        @filters.send(method, *args)
+      rescue Liquid::Error
+        nil
       end
     end
   end

test/test_helper.rb

@@ -72,21 +72,21 @@ module Minitest
     end
 
     def with_global_filter(*globals)
-      original_global_filters = Liquid::StrainerFactory.instance_variable_get(:@global_filters)
-      Liquid::StrainerFactory.instance_variable_set(:@global_filters, [])
-      globals.each do |global|
-        Liquid::StrainerFactory.add_global_filter(global)
-      end
-
-      Liquid::StrainerFactory.send(:strainer_class_cache).clear
+      original_global_cache = Liquid::StrainerFactory::GlobalCache
+      Liquid::StrainerFactory.send(:remove_const, :GlobalCache)
+      Liquid::StrainerFactory.const_set(:GlobalCache, Class.new(Liquid::StrainerTemplate))
 
       globals.each do |global|
         Liquid::Template.register_filter(global)
       end
-      yield
-    ensure
       Liquid::StrainerFactory.send(:strainer_class_cache).clear
-      Liquid::StrainerFactory.instance_variable_set(:@global_filters, original_global_filters)
+      begin
+        yield
+      ensure
+        Liquid::StrainerFactory.send(:remove_const, :GlobalCache)
+        Liquid::StrainerFactory.const_set(:GlobalCache, original_global_cache)
+        Liquid::StrainerFactory.send(:strainer_class_cache).clear
+      end
     end
 
     def with_error_mode(mode)

test/unit/condition_unit_test.rb

@@ -10,8 +10,8 @@ class ConditionUnitTest < Minitest::Test
   end
 
   def test_basic_condition
-    assert_equal(false, Condition.new(1, '==', 2).evaluate)
-    assert_equal(true,  Condition.new(1, '==', 1).evaluate)
+    assert_equal(false, Condition.new(1, '==', 2).evaluate(Context.new))
+    assert_equal(true,  Condition.new(1, '==', 1).evaluate(Context.new))
   end
 
   def test_default_operators_evalute_true
@@ -67,11 +67,11 @@ class ConditionUnitTest < Minitest::Test
   end
 
   def test_hash_compare_backwards_compatibility
-    assert_nil(Condition.new({}, '>', 2).evaluate)
-    assert_nil(Condition.new(2, '>', {}).evaluate)
-    assert_equal(false, Condition.new({}, '==', 2).evaluate)
-    assert_equal(true, Condition.new({ 'a' => 1 }, '==', 'a' => 1).evaluate)
-    assert_equal(true, Condition.new({ 'a' => 2 }, 'contains', 'a').evaluate)
+    assert_nil(Condition.new({}, '>', 2).evaluate(Context.new))
+    assert_nil(Condition.new(2, '>', {}).evaluate(Context.new))
+    assert_equal(false, Condition.new({}, '==', 2).evaluate(Context.new))
+    assert_equal(true, Condition.new({ 'a' => 1 }, '==', 'a' => 1).evaluate(Context.new))
+    assert_equal(true, Condition.new({ 'a' => 2 }, 'contains', 'a').evaluate(Context.new))
   end
 
   def test_contains_works_on_arrays
@@ -106,30 +106,29 @@ class ConditionUnitTest < Minitest::Test
 
   def test_or_condition
     condition = Condition.new(1, '==', 2)
-
-    assert_equal(false, condition.evaluate)
+    assert_equal(false, condition.evaluate(Context.new))
 
     condition.or(Condition.new(2, '==', 1))
 
-    assert_equal(false, condition.evaluate)
+    assert_equal(false, condition.evaluate(Context.new))
 
     condition.or(Condition.new(1, '==', 1))
 
-    assert_equal(true, condition.evaluate)
+    assert_equal(true, condition.evaluate(Context.new))
   end
 
   def test_and_condition
     condition = Condition.new(1, '==', 1)
 
-    assert_equal(true, condition.evaluate)
+    assert_equal(true, condition.evaluate(Context.new))
 
     condition.and(Condition.new(2, '==', 2))
 
-    assert_equal(true, condition.evaluate)
+    assert_equal(true, condition.evaluate(Context.new))
 
     condition.and(Condition.new(2, '==', 1))
 
-    assert_equal(false, condition.evaluate)
+    assert_equal(false, condition.evaluate(Context.new))
   end
 
   def test_should_allow_custom_proc_operator
@@ -148,6 +147,20 @@ class ConditionUnitTest < Minitest::Test
     assert_evaluates_true(VariableLookup.new("one"), '==', VariableLookup.new("another"))
   end
 
+  def test_default_context_is_deprecated
+    if Gem::Version.new(Liquid::VERSION) >= Gem::Version.new('6.0.0')
+      flunk("Condition#evaluate without a context argument is to be removed")
+    end
+
+    _out, err = capture_io do
+      assert_equal(true, Condition.new(1, '==', 1).evaluate)
+    end
+
+    expected = "DEPRECATION WARNING: Condition#evaluate without a context argument is deprecated" \
+      " and will be removed from Liquid 6.0.0."
+    assert_includes(err.lines.map(&:strip), expected)
+  end
+
   private
 
   def assert_evaluates_true(left, op, right)

test/unit/strainer_factory_unit_test.rb

@@ -52,7 +52,8 @@ class StrainerFactoryUnitTest < Minitest::Test
       /\ALiquid error: wrong number of arguments \((1 for 0|given 1, expected 0)\)\z/,
       exception.message
     )
-    assert_equal(exception.backtrace[0].split(':')[0], __FILE__)
+    source = AccessScopeFilters.instance_method(:public_filter).source_location
+    assert_equal(source.map(&:to_s), exception.backtrace[0].split(':')[0..1])
   end
 
   def test_strainer_only_invokes_public_filter_methods

test/unit/strainer_template_unit_test.rb

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