Remove a line feed for CI

Test all filters against random data to detect exceptions

.rubocop-https---shopify-github-io-ruby-style-guide-rubocop-yml

@@ -1,3 +1,5 @@
+# Recommended rubocop version: ~> 0.78.0
+
 AllCops:
   Exclude:
   - 'db/schema.rb'
@@ -20,7 +22,7 @@ Style/Alias:
   - prefer_alias
   - prefer_alias_method
 
-Layout/AlignHash:
+Layout/HashAlignment:
   EnforcedHashRocketStyle: key
   EnforcedColonStyle: key
   EnforcedLastArgumentHashStyle: ignore_implicit
@@ -30,7 +32,7 @@ Layout/AlignHash:
   - ignore_implicit
   - ignore_explicit
 
-Layout/AlignParameters:
+Layout/ParameterAlignment:
   EnforcedStyle: with_fixed_indentation
   SupportedStyles:
   - with_first_parameter
@@ -172,7 +174,7 @@ Naming/FileName:
   Regex:
   IgnoreExecutableScripts: true
 
-Layout/IndentFirstArgument:
+Layout/FirstArgumentIndentation:
   EnforcedStyle: consistent
   SupportedStyles:
   - consistent
@@ -225,7 +227,7 @@ Layout/IndentationConsistency:
 Layout/IndentationWidth:
   Width: 2
 
-Layout/IndentFirstArrayElement:
+Layout/FirstArrayElementIndentation:
   EnforcedStyle: consistent
   SupportedStyles:
   - special_inside_parentheses
@@ -233,10 +235,10 @@ Layout/IndentFirstArrayElement:
   - align_brackets
   IndentationWidth:
 
-Layout/IndentAssignment:
+Layout/AssignmentIndentation:
   IndentationWidth:
 
-Layout/IndentFirstHashElement:
+Layout/FirstHashElementIndentation:
   EnforcedStyle: consistent
   SupportedStyles:
   - special_inside_parentheses
@@ -340,9 +342,9 @@ Style/PercentQLiterals:
 Naming/PredicateName:
   NamePrefix:
   - is_
-  NamePrefixBlacklist:
+  ForbiddenPrefixes:
   - is_
-  NameWhitelist:
+  AllowedMethods:
   - is_a?
   Exclude:
   - 'spec/**/*'
@@ -467,7 +469,7 @@ Style/TernaryParentheses:
   - require_no_parentheses
   AllowSafeAssignment: true
 
-Layout/TrailingBlankLines:
+Layout/TrailingEmptyLines:
   EnforcedStyle: final_newline
   SupportedStyles:
   - final_newline
@@ -478,7 +480,7 @@ Style/TrivialAccessors:
   AllowPredicates: true
   AllowDSLWriters: false
   IgnoreClassMethods: false
-  Whitelist:
+  AllowedMethods:
   - to_ary
   - to_a
   - to_c
@@ -509,7 +511,7 @@ Style/WhileUntilModifier:
 Metrics/BlockNesting:
   Max: 3
 
-Metrics/LineLength:
+Layout/LineLength:
   Max: 120
   AllowHeredoc: true
   AllowURI: true
@@ -561,7 +563,7 @@ Lint/UnusedMethodArgument:
 Naming/AccessorMethodName:
   Enabled: true
 
-Layout/AlignArray:
+Layout/ArrayAlignment:
   Enabled: true
 
 Style/ArrayJoin:
@@ -819,13 +821,13 @@ Layout/TrailingWhitespace:
 Style/UnlessElse:
   Enabled: true
 
-Style/UnneededCapitalW:
+Style/RedundantCapitalW:
   Enabled: true
 
-Style/UnneededInterpolation:
+Style/RedundantInterpolation:
   Enabled: true
 
-Style/UnneededPercentQ:
+Style/RedundantPercentQ:
   Enabled: true
 
 Style/VariableInterpolation:
@@ -840,7 +842,7 @@ Style/WhileUntilDo:
 Style/ZeroLengthPredicate:
   Enabled: true
 
-Layout/IndentHeredoc:
+Layout/HeredocIndentation:
   EnforcedStyle: squiggly
 
 Lint/AmbiguousOperator:
@@ -864,7 +866,7 @@ Lint/DeprecatedClassMethods:
 Lint/DuplicateMethods:
   Enabled: true
 
-Lint/DuplicatedKey:
+Lint/DuplicateHashKey:
   Enabled: true
 
 Lint/EachWithObjectArgument:
@@ -891,7 +893,7 @@ Lint/FloatOutOfRange:
 Lint/FormatParameterMismatch:
   Enabled: true
 
-Lint/HandleExceptions:
+Lint/SuppressedException:
   AllowComments: true
 
 Lint/ImplicitStringConcatenation:
@@ -947,7 +949,7 @@ Lint/ShadowedException:
 Lint/ShadowingOuterLocalVariable:
   Enabled: true
 
-Lint/StringConversionInInterpolation:
+Lint/RedundantStringCoercion:
   Enabled: true
 
 Lint/UnderscorePrefixedVariableName:
@@ -956,13 +958,13 @@ Lint/UnderscorePrefixedVariableName:
 Lint/UnifiedInteger:
   Enabled: true
 
-Lint/UnneededCopDisableDirective:
+Lint/RedundantCopDisableDirective:
   Enabled: true
 
-Lint/UnneededCopEnableDirective:
+Lint/RedundantCopEnableDirective:
   Enabled: true
 
-Lint/UnneededSplatExpansion:
+Lint/RedundantSplatExpansion:
   Enabled: true
 
 Lint/UnreachableCode:

.rubocop_todo.yml

@@ -18,7 +18,7 @@ Lint/InheritException:
 # Cop supports --auto-correct.
 # Configuration parameters: AutoCorrect, AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns.
 # URISchemes: http, https
-Metrics/LineLength:
+Layout/LineLength:
   Max: 294
 
 # Offense count: 44

.travis.yml

@@ -4,8 +4,8 @@ cache: bundler
 rvm:
   - 2.4
   - 2.5
-  - &latest_ruby 2.6
-  - 2.7
+  - 2.6
+  - &latest_ruby 2.7
   - ruby-head
 
 matrix:
@@ -13,8 +13,6 @@ matrix:
     - rvm: *latest_ruby
       script: bundle exec rake memory_profile:run
       name: Profiling Memory Usage
-  allow_failures:
-    - rvm: ruby-head
 
 branches:
   only:

Gemfile

@@ -18,7 +18,7 @@ group :benchmark, :test do
 end
 
 group :test do
-  gem 'rubocop', '~> 0.74.0', require: false
+  gem 'rubocop', '~> 0.78.0', require: false
   gem 'rubocop-performance', require: false
 
   platform :mri, :truffleruby do

History.md

@@ -1,5 +1,10 @@
 # Liquid Change Log
 
+### Unreleased
+
+* Split Strainer class as a factory and a template (#1208) [Thierry Joyal]
+* Remove handling of a nil context in the Strainer class (#1218) [Thierry Joyal]
+
 ## 4.0.3 / 2019-03-12
 
 ### Fixed

lib/liquid.rb

@@ -57,7 +57,10 @@ require 'liquid/forloop_drop'
 require 'liquid/extensions'
 require 'liquid/errors'
 require 'liquid/interrupts'
-require 'liquid/strainer'
+require 'liquid/filter'
+require 'liquid/filter_template'
+require 'liquid/strainer_factory'
+require 'liquid/strainer_template'
 require 'liquid/expression'
 require 'liquid/context'
 require 'liquid/parser_switching'
@@ -80,6 +83,7 @@ require 'liquid/partial_cache'
 require 'liquid/usage'
 require 'liquid/register'
 require 'liquid/static_registers'
+require 'liquid/template_factory'
 
 # Load all the tags of the standard library
 #

lib/liquid/block_body.rb

@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
+require 'English'
+
 module Liquid
   class BlockBody
     LiquidTagToken      = /\A\s*(\w+)\s*(.*?)\z/o
@@ -51,6 +53,21 @@ module Liquid
       yield nil, nil
     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)
+    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)
+      end
+    end
+
     private def parse_for_document(tokenizer, parse_context, &block)
       while (token = tokenizer.shift)
         next if token.empty?
@@ -70,8 +87,8 @@ module Liquid
           end
 
           if tag_name == 'liquid'
-            liquid_tag_tokenizer = Tokenizer.new(markup, line_number: parse_context.line_number, for_liquid_tag: true)
-            next parse_for_liquid_tag(liquid_tag_tokenizer, parse_context, &block)
+            parse_liquid_tag(markup, parse_context, &block)
+            next
           end
 
           unless (tag = registered_tags[tag_name])

lib/liquid/context.rb

@@ -52,7 +52,7 @@ module Liquid
     end
 
     def strainer
-      @strainer ||= Strainer.create(self, @filters)
+      @strainer ||= StrainerFactory.create(self, @filters)
     end
 
     # Adds filters to this context.

lib/liquid/filter.rb

@@ -0,0 +1,43 @@
+# frozen_string_literal: true
+
+require 'set'
+
+module Liquid
+  # A filter in liquid is a class which contain invokable logic from liquid templates.
+  #
+  # Public methods in filter classes are callable.
+  #
+  # The use for liquid filters is to make logic functions available to the web designers.
+  #
+  # Example:
+  #
+  #   class StringFilter < Liquid::Filter
+  #     def upcase(input)
+  #       input.upcase
+  #     end
+  #   end
+  #
+  #   tmpl = Liquid::Template.parse('Result: {{ "test" | upcase }}')
+  #   tmpl.render({}, filters: [StringFilter])
+  #   => "Result: TEST"
+  class Filter
+    class << self
+      def invokable_methods
+        @invokable_methods ||= begin
+          blacklist = Liquid::Filter.public_instance_methods
+          whitelist = public_instance_methods - blacklist
+
+          Set.new(whitelist.map(&:to_s))
+        end
+      end
+    end
+
+    def initialize(context)
+      @context = context
+    end
+
+    private
+
+    attr_reader :context
+  end
+end

lib/liquid/filter_template.rb

@@ -0,0 +1,40 @@
+# frozen_string_literal: true
+
+require 'set'
+
+module Liquid
+  # FilterTemplate is the computed class for the filters system.
+  #
+  # Historically Liquid used to include filters as Module to the context strainer.
+  # This lead to the absence of sandbox between filters (one filter could override private methods of another filter).
+  #
+  # With the implementation of Liquid::Filter, it is now possible for the modules from legacy code to be automatically
+  #   wrapped into a Liquid::Filter generated class.
+  #
+  # This should not be considered as the base behaviour, it is preferred to create filters going forward directly as
+  #   classes that are child of Liquid::Filter.
+  class FilterTemplate < Filter
+    class << self
+      def include(mod)
+        super
+
+        @init_module = mod
+      end
+
+      # Override of the `invokable_methods`.
+      # We can't rely on the parent logic as some modules might have been defining methods that shadow Class methods.
+      #
+      # Eg.:
+      # mod = Liquid::StandardFilters
+      # filter = Class.new(FilterTemplate)
+      # filter.include(mod)
+      # mod.public_instance_methods - (filter.public_instance_methods - Class.public_instance_methods)
+      # => [:prepend]
+      def invokable_methods
+        whitelist = @init_module.public_instance_methods
+
+        @invokable_methods ||= Set.new(whitelist.map(&:to_s))
+      end
+    end
+  end
+end

lib/liquid/parser_switching.rb

@@ -8,10 +8,10 @@ module Liquid
       when :lax    then lax_parse(markup)
       when :warn
         begin
-          return strict_parse_with_error_context(markup)
+          strict_parse_with_error_context(markup)
         rescue SyntaxError => e
           parse_context.warnings << e
-          return lax_parse(markup)
+          lax_parse(markup)
         end
       end
     end

lib/liquid/partial_cache.rb

@@ -12,7 +12,10 @@ module Liquid
 
       parse_context.partial = true
 
-      partial = Liquid::Template.parse(source, parse_context)
+      template_factory = (context.registers[:template_factory] ||= Liquid::TemplateFactory.new)
+      template = template_factory.for(template_name)
+
+      partial = template.parse(source, parse_context)
       cached_partials[template_name] = partial
     ensure
       parse_context.partial = false

lib/liquid/strainer.rb

@@ -1,68 +0,0 @@
-# frozen_string_literal: true
-
-require 'set'
-
-module Liquid
-  # Strainer is the parent class for the filters system.
-  # New filters are mixed into the strainer class which is then instantiated for each liquid template render run.
-  #
-  # The Strainer only allows method calls defined in filters given to it via Strainer.global_filter,
-  # Context#add_filters or Template.register_filter
-  class Strainer #:nodoc:
-    @@global_strainer = Class.new(Strainer) do
-      @filter_methods = Set.new
-    end
-    @@strainer_class_cache = Hash.new do |hash, filters|
-      hash[filters] = Class.new(@@global_strainer) do
-        @filter_methods = @@global_strainer.filter_methods.dup
-        filters.each { |f| add_filter(f) }
-      end
-    end
-
-    def initialize(context)
-      @context = context
-    end
-
-    class << self
-      attr_reader :filter_methods
-    end
-
-    def self.add_filter(filter)
-      raise ArgumentError, "Expected module but got: #{filter.class}" unless filter.is_a?(Module)
-      unless include?(filter)
-        invokable_non_public_methods = (filter.private_instance_methods + filter.protected_instance_methods).select { |m| invokable?(m) }
-        if invokable_non_public_methods.any?
-          raise MethodOverrideError, "Filter overrides registered public methods as non public: #{invokable_non_public_methods.join(', ')}"
-        else
-          send(:include, filter)
-          @filter_methods.merge(filter.public_instance_methods.map(&:to_s))
-        end
-      end
-    end
-
-    def self.global_filter(filter)
-      @@strainer_class_cache.clear
-      @@global_strainer.add_filter(filter)
-    end
-
-    def self.invokable?(method)
-      @filter_methods.include?(method.to_s)
-    end
-
-    def self.create(context, filters = [])
-      @@strainer_class_cache[filters].new(context)
-    end
-
-    def invoke(method, *args)
-      if self.class.invokable?(method)
-        send(method, *args)
-      elsif @context&.strict_filters
-        raise Liquid::UndefinedFilter, "undefined filter #{method}"
-      else
-        args.first
-      end
-    rescue ::ArgumentError => e
-      raise Liquid::ArgumentError, e.message, e.backtrace
-    end
-  end
-end

lib/liquid/strainer_factory.rb

@@ -0,0 +1,36 @@
+# frozen_string_literal: true
+
+module Liquid
+  # StrainerFactory is the factory for the filters system.
+  module StrainerFactory
+    extend self
+
+    def add_global_filter(filter)
+      strainer_class_cache.clear
+      global_filters << filter
+    end
+
+    def create(context, filters = [])
+      strainer_from_cache(filters).new(context)
+    end
+
+    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
+      end
+    end
+
+    def strainer_class_cache
+      @strainer_class_cache ||= {}
+    end
+  end
+end

lib/liquid/strainer_template.rb

@@ -0,0 +1,82 @@
+# frozen_string_literal: true
+
+require 'set'
+
+module Liquid
+  # StrainerTemplate is the computed class for the filters system.
+  # New filters are mixed into the strainer class which is then instantiated for each liquid template render run.
+  #
+  # The Strainer only allows method calls defined in filters given to it via StrainerFactory.add_global_filter,
+  # Context#add_filters or Template.register_filter
+  class StrainerTemplate
+    def initialize(context)
+      @context = context
+    end
+
+    class << self
+      def add_filter(mod)
+        filter = if mod.is_a?(Class) && mod.ancestors.include?(Liquid::Filter)
+          mod
+        elsif mod.instance_of?(Module)
+          convert_mod_to_filter(mod)
+        else
+          raise(ArgumentError, "wrong argument type Proc (expected Liquid::Filter)")
+        end
+
+        filter.invokable_methods.each do |method|
+          filter_method_map[method] = filter
+        end
+      end
+
+      def filter_for(method)
+        filter_method_map[method]
+      end
+
+      def invokable?(method)
+        filter_method_map.key?(method)
+      end
+
+      private
+
+      def filter_method_map
+        @filter_method_map ||= {}
+      end
+
+      def convert_mod_to_filter(mod)
+        @filter_classes ||= {}
+        @filter_classes[mod] ||= begin
+          klass = Class.new(FilterTemplate)
+          klass.include(mod)
+          klass
+        end
+      end
+    end
+
+    def invoke(method, *args)
+      if self.class.invokable?(method)
+        begin
+          instance = filter_instance_for(method)
+          instance.public_send(method, *args)
+        rescue ::ArgumentError => e
+          raise Liquid::ArgumentError, e.message, e.backtrace
+        end
+      elsif context.strict_filters
+        raise(Liquid::UndefinedFilter, "undefined filter #{method}")
+      else
+        args.first
+      end
+    end
+
+    private
+
+    def filter_instance_for(method)
+      @filter_instances ||= {}
+      @filter_instances.fetch(method) do
+        klass = self.class.filter_for(method)
+        klass.new(context)
+      end
+    end
+
+    attr_reader :context
+  end
+end

lib/liquid/tags/include.rb

@@ -50,7 +50,19 @@ module Liquid
       template_name = context.evaluate(@template_name_expr)
       raise ArgumentError, options[:locale].t("errors.argument.include") unless template_name
 
-      partial = load_partial(template_name, context, parse_context)
+      partial = PartialCache.load(
+        template_name,
+        context: context,
+        parse_context: parse_context
+      )
+
+      context_variable_name = @alias_name || template_name.split('/').last
+
+      variable = if @variable_name_expr
+        context.evaluate(@variable_name_expr)
+      else
+        context.find_variable(template_name, raise_on_not_found: false)
+      end
 
       old_template_name = context.template_name
       old_partial       = context.partial
@@ -59,15 +71,7 @@ module Liquid
         context.partial       = true
         context.stack do
           @attributes.each do |key, value|
-            context[key] = evaluate(context, value)
-          end
-
-          context_variable_name = @alias_name || template_name.split('/').last
-
-          variable = if @variable_name_expr
-            evaluate(context, @variable_name_expr)
-          else
-            find_variable(context, template_name, raise_on_not_found: false)
+            context[key] = context.evaluate(value)
           end
 
           if variable.is_a?(Array)
@@ -99,24 +103,6 @@ module Liquid
         ] + @node.attributes.values
       end
     end
-
-    private
-
-    def evaluate(context, value)
-      context.evaluate(value)
-    end
-
-    def find_variable(context, *args)
-      context.find_variable(*args)
-    end
-
-    def load_partial(template_name, context, parse_context)
-      PartialCache.load(
-        template_name,
-        context: context,
-        parse_context: parse_context
-      )
-    end
   end
 
   Template.register_tag('include', Include)

lib/liquid/tags/render.rb

@@ -38,7 +38,11 @@ module Liquid
       template_name = context.evaluate(@template_name_expr)
       raise ArgumentError, options[:locale].t("errors.argument.include") unless template_name
 
-      partial = load_partial(template_name, context, parse_context)
+      partial = PartialCache.load(
+        template_name,
+        context: context,
+        parse_context: parse_context
+      )
 
       context_variable_name = @alias_name || template_name.split('/').last
 
@@ -49,14 +53,14 @@ module Liquid
         inner_context['forloop']    = forloop if forloop
 
         @attributes.each do |key, value|
-          inner_context[key] = evaluate(context, value)
+          inner_context[key] = context.evaluate(value)
         end
         inner_context[context_variable_name] = var unless var.nil?
         partial.render_to_output_buffer(inner_context, output)
         forloop&.send(:increment!)
       }
 
-      variable = @variable_name_expr ? evaluate(context, @variable_name_expr) : nil
+      variable = @variable_name_expr ? context.evaluate(@variable_name_expr) : nil
       if @for && variable.respond_to?(:each) && variable.respond_to?(:count)
         forloop = Liquid::ForloopDrop.new(template_name, variable.count, nil)
         variable.each { |var| render_partial_func.call(var, forloop) }
@@ -74,20 +78,6 @@ module Liquid
         ] + @node.attributes.values
       end
     end
-
-    private
-
-    def evaluate(context, value)
-      context.evaluate(value)
-    end
-
-    def load_partial(template_name, context, parse_context)
-      PartialCache.load(
-        template_name,
-        context: context,
-        parse_context: parse_context
-      )
-    end
   end
 
   Template.register_tag('render', Render)

lib/liquid/template.rb

@@ -18,8 +18,6 @@ module Liquid
     attr_accessor :root
     attr_reader :resource_limits, :warnings
 
-    @@file_system = BlankFileSystem.new
-
     class TagRegistry
       include Enumerable
 
@@ -63,60 +61,59 @@ module Liquid
       # :lax acts like liquid 2.5 and silently ignores malformed tags in most cases.
       # :warn is the default and will give deprecation warnings when invalid syntax is used.
       # :strict will enforce correct syntax.
-      attr_writer :error_mode
+      attr_accessor :error_mode
+      Template.error_mode = :lax
+
+      attr_reader :taint_mode
 
       # Sets how strict the taint checker should be.
       # :lax is the default, and ignores the taint flag completely
       # :warn adds a warning, but does not interrupt the rendering
       # :error raises an error when tainted output is used
-      attr_writer :taint_mode
+      # @deprecated Since it is being deprecated in ruby itself.
+      def taint_mode=(mode)
+        taint_supported = Object.new.taint.tainted?
+        if mode != :lax && !taint_supported
+          raise NotImplementedError, "#{RUBY_ENGINE} #{RUBY_VERSION} doesn't support taint checking"
+        end
+        @taint_mode = mode
+      end
+
+      Template.taint_mode = :lax
 
       attr_accessor :default_exception_renderer
       Template.default_exception_renderer = lambda do |exception|
         exception
       end
 
-      def file_system
-        @@file_system
-      end
+      attr_accessor :file_system
+      Template.file_system = BlankFileSystem.new
 
-      def file_system=(obj)
-        @@file_system = obj
-      end
+      attr_accessor :tags
+      Template.tags = TagRegistry.new
+      private :tags=
 
       def register_tag(name, klass)
         tags[name.to_s] = klass
       end
 
-      def tags
-        @tags ||= TagRegistry.new
-      end
+      attr_accessor :registers
+      Template.registers = {}
+      private :registers=
 
       def add_register(name, klass)
         registers[name.to_sym] = klass
       end
 
-      def registers
-        @registers ||= {}
-      end
-
-      def error_mode
-        @error_mode ||= :lax
-      end
-
-      def taint_mode
-        @taint_mode ||= :lax
-      end
-
       # Pass a module with filter methods which should be available
       # to all liquid views. Good for registering the standard library
       def register_filter(mod)
-        Strainer.global_filter(mod)
+        StrainerFactory.add_global_filter(mod)
       end
 
-      def default_resource_limits
-        @default_resource_limits ||= {}
-      end
+      attr_accessor :default_resource_limits
+      Template.default_resource_limits = {}
+      private :default_resource_limits=
 
       # creates a new <tt>Template</tt> object from liquid source code
       # To enable profiling, pass in <tt>profile: true</tt> as an option.

lib/liquid/template_factory.rb

@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+module Liquid
+  class TemplateFactory
+    def for(_template_name)
+      Liquid::Template.new
+    end
+  end
+end

lib/liquid/variable.rb

@@ -143,8 +143,8 @@ module Liquid
     end
 
     def taint_check(context, obj)
-      return unless obj.tainted?
       return if Template.taint_mode == :lax
+      return unless obj.tainted?
 
       @markup =~ QuotedFragment
       name = Regexp.last_match(0)

performance/shopify/shop_filter.rb

@@ -60,7 +60,7 @@ module ShopFilter
 
     case style
     when 'original'
-      return '/files/shops/random_number/' + url
+      '/files/shops/random_number/' + url
     when 'grande', 'large', 'medium', 'compact', 'small', 'thumb', 'icon'
       "/files/shops/random_number/products/#{Regexp.last_match(1)}_#{style}.#{Regexp.last_match(2)}"
     else

test/integration/drop_test.rb

@@ -114,29 +114,31 @@ class DropsTest < Minitest::Test
     assert_equal('  ', tpl.render!('product' => ProductDrop.new))
   end
 
-  def test_rendering_raises_on_tainted_attr
-    with_taint_mode(:error) do
-      tpl = Liquid::Template.parse('{{ product.user_input }}')
-      assert_raises TaintedError do
-        tpl.render!('product' => ProductDrop.new)
+  if taint_supported?
+    def test_rendering_raises_on_tainted_attr
+      with_taint_mode(:error) do
+        tpl = Liquid::Template.parse('{{ product.user_input }}')
+        assert_raises TaintedError do
+          tpl.render!('product' => ProductDrop.new)
+        end
       end
     end
-  end
 
-  def test_rendering_warns_on_tainted_attr
-    with_taint_mode(:warn) do
-      tpl     = Liquid::Template.parse('{{ product.user_input }}')
-      context = Context.new('product' => ProductDrop.new)
-      tpl.render!(context)
-      assert_equal [Liquid::TaintedError], context.warnings.map(&:class)
-      assert_equal "variable 'product.user_input' is tainted and was not escaped", context.warnings.first.to_s(false)
+    def test_rendering_warns_on_tainted_attr
+      with_taint_mode(:warn) do
+        tpl     = Liquid::Template.parse('{{ product.user_input }}')
+        context = Context.new('product' => ProductDrop.new)
+        tpl.render!(context)
+        assert_equal [Liquid::TaintedError], context.warnings.map(&:class)
+        assert_equal "variable 'product.user_input' is tainted and was not escaped", context.warnings.first.to_s(false)
+      end
     end
-  end
 
-  def test_rendering_doesnt_raise_on_escaped_tainted_attr
-    with_taint_mode(:error) do
-      tpl = Liquid::Template.parse('{{ product.user_input | escape }}')
-      tpl.render!('product' => ProductDrop.new)
+    def test_rendering_doesnt_raise_on_escaped_tainted_attr
+      with_taint_mode(:error) do
+        tpl = Liquid::Template.parse('{{ product.user_input | escape }}')
+        tpl.render!('product' => ProductDrop.new)
+      end
     end
   end
 

test/integration/render_profiling_test.rb

@@ -159,13 +159,13 @@ class RenderProfilingTest < Minitest::Test
     t.render!("collection" => ["one", "two"])
     leaf = t.profiler[0].children[0]
 
-    assert_operator leaf.self_time, :>, 0
+    assert_operator(leaf.self_time, :>, 0)
   end
 
   def test_profiling_supports_total_time
     t = Template.parse("{% if true %} {% increment test %} {{ test }} {% endif %}", profile: true)
     t.render!
 
-    assert_operator t.profiler[0].total_time, :>, 0
+    assert_operator(t.profiler[0].total_time, :>, 0)
   end
 end

test/integration/standard_filter_test.rb

@@ -768,6 +768,49 @@ class StandardFiltersTest < Minitest::Test
     assert_nil(@filters.where([nil], "ok"))
   end
 
+  def test_all_filters_never_raise_non_liquid_exception
+    test_drop = TestDrop.new
+    test_drop.context = Context.new
+    test_enum = TestEnumerable.new
+    test_enum.context = Context.new
+    test_types = [
+      "foo",
+      123,
+      0,
+      0.0,
+      -1234.003030303,
+      -99999999,
+      1234.38383000383830003838300,
+      nil,
+      true,
+      false,
+      TestThing.new,
+      test_drop,
+      test_enum,
+      ["foo", "bar"],
+      { "foo" => "bar" },
+      { foo: "bar" },
+      [{ "foo" => "bar" }, { "foo" => 123 }, { "foo" => nil }, { "foo" => true }, { "foo" => ["foo", "bar"] }],
+      { 1 => "bar" },
+      ["foo", 123, nil, true, false, Drop, ["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
+      end
+    end
+  end
+
   def test_where_no_target_value
     input = [
       { "foo" => false },

test/integration/tags/liquid_tag_test.rb

@@ -81,6 +81,18 @@ class LiquidTagTest < Minitest::Test
     assert_match_syntax_error("syntax error (line 3): Unknown tag 'error'", "{% liquid echo ''\n  \n error %}")
   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
+  end
+
   def test_cannot_open_blocks_living_past_a_liquid_tag
     assert_match_syntax_error("syntax error (line 3): 'if' tag was never closed", <<~LIQUID)
       {%- liquid
@@ -91,11 +103,13 @@ class LiquidTagTest < Minitest::Test
   end
 
   def test_quirk_can_close_blocks_created_before_a_liquid_tag
-    assert_template_result("42", <<~LIQUID)
-      {%- if true -%}
-      42
-      {%- liquid endif -%}
-    LIQUID
+    assert_usage_increment("liquid_tag_contains_outer_tag") do
+      assert_template_result("42", <<~LIQUID)
+        {%- if true -%}
+        42
+        {%- liquid endif -%}
+      LIQUID
+    end
   end
 
   def test_liquid_tag_in_raw

test/integration/tags/render_tag_test.rb

@@ -42,29 +42,31 @@ class RenderTagTest < Minitest::Test
     assert_template_result('', "{% assign snippet = 'should not be visible' %}{% render 'snippet' %}")
   end
 
-  def test_render_sets_the_correct_template_name_for_errors
-    Liquid::Template.file_system = StubFileSystem.new('snippet' => '{{ unsafe }}')
+  if taint_supported?
+    def test_render_sets_the_correct_template_name_for_errors
+      Liquid::Template.file_system = StubFileSystem.new('snippet' => '{{ unsafe }}')
 
-    with_taint_mode :error do
-      template = Liquid::Template.parse('{% render "snippet", unsafe: unsafe %}')
-      context  = Context.new('unsafe' => (+'unsafe').tap(&:taint))
-      template.render(context)
+      with_taint_mode :error do
+        template = Liquid::Template.parse('{% render "snippet", unsafe: unsafe %}')
+        context  = Context.new('unsafe' => (+'unsafe').tap(&:taint))
+        template.render(context)
 
-      assert_equal [Liquid::TaintedError], template.errors.map(&:class)
-      assert_equal 'snippet', template.errors.first.template_name
+        assert_equal [Liquid::TaintedError], template.errors.map(&:class)
+        assert_equal 'snippet', template.errors.first.template_name
+      end
     end
-  end
 
-  def test_render_sets_the_correct_template_name_for_warnings
-    Liquid::Template.file_system = StubFileSystem.new('snippet' => '{{ unsafe }}')
+    def test_render_sets_the_correct_template_name_for_warnings
+      Liquid::Template.file_system = StubFileSystem.new('snippet' => '{{ unsafe }}')
 
-    with_taint_mode :warn do
-      template = Liquid::Template.parse('{% render "snippet", unsafe: unsafe %}')
-      context  = Context.new('unsafe' => (+'unsafe').tap(&:taint))
-      template.render(context)
+      with_taint_mode :warn do
+        template = Liquid::Template.parse('{% render "snippet", unsafe: unsafe %}')
+        context  = Context.new('unsafe' => (+'unsafe').tap(&:taint))
+        template.render(context)
 
-      assert_equal [Liquid::TaintedError], context.warnings.map(&:class)
-      assert_equal 'snippet', context.warnings.first.template_name
+        assert_equal [Liquid::TaintedError], context.warnings.map(&:class)
+        assert_equal 'snippet', context.warnings.first.template_name
+      end
     end
   end
 

test/integration/template_test.rb

@@ -361,4 +361,12 @@ class TemplateTest < Minitest::Test
     result = t.render('x' => 1, 'y' => 5)
     assert_equal('12345', result)
   end
+
+  unless taint_supported?
+    def test_taint_mode
+      assert_raises(NotImplementedError) do
+        Template.taint_mode = :warn
+      end
+    end
+  end
 end

test/test_helper.rb

@@ -32,6 +32,10 @@ module Minitest
     def fixture(name)
       File.join(File.expand_path(__dir__), "fixtures", name)
     end
+
+    def self.taint_supported?
+      Object.new.taint.tainted?
+    end
   end
 
   module Assertions
@@ -54,20 +58,39 @@ module Minitest
       assert_match(match, exception.message)
     end
 
+    def assert_usage_increment(name, times: 1)
+      old_method = Liquid::Usage.method(:increment)
+      calls = 0
+      begin
+        Liquid::Usage.singleton_class.send(:remove_method, :increment)
+        Liquid::Usage.define_singleton_method(:increment) do |got_name|
+          calls += 1 if got_name == name
+          old_method.call(got_name)
+        end
+        yield
+      ensure
+        Liquid::Usage.singleton_class.send(:remove_method, :increment)
+        Liquid::Usage.define_singleton_method(:increment, old_method)
+      end
+      assert_equal(times, calls, "Number of calls to Usage.increment with #{name.inspect}")
+    end
+
     def with_global_filter(*globals)
-      original_global_strainer = Liquid::Strainer.class_variable_get(:@@global_strainer)
-      Liquid::Strainer.class_variable_set(:@@global_strainer, Class.new(Liquid::Strainer) do
-        @filter_methods = Set.new
-      end)
-      Liquid::Strainer.class_variable_get(:@@strainer_class_cache).clear
+      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
 
       globals.each do |global|
         Liquid::Template.register_filter(global)
       end
       yield
     ensure
-      Liquid::Strainer.class_variable_get(:@@strainer_class_cache).clear
-      Liquid::Strainer.class_variable_set(:@@global_strainer, original_global_strainer)
+      Liquid::StrainerFactory.send(:strainer_class_cache).clear
+      Liquid::StrainerFactory.instance_variable_set(:@global_filters, original_global_filters)
     end
 
     def with_taint_mode(mode)
@@ -136,3 +159,16 @@ class StubFileSystem
     @values.fetch(template_path)
   end
 end
+
+class StubTemplateFactory
+  attr_reader :count
+
+  def initialize
+    @count = 0
+  end
+
+  def for(_template_name)
+    @count += 1
+    Liquid::Template.new
+  end
+end

test/unit/context_unit_test.rb

@@ -147,13 +147,13 @@ class ContextUnitTest < Minitest::Test
 
     context = Context.new
     context.add_filters(filter)
-    assert_equal('hi? hi!', context.invoke(:hi, 'hi?'))
+    assert_equal('hi? hi!', context.invoke('hi', 'hi?'))
 
     context = Context.new
-    assert_equal('hi?', context.invoke(:hi, 'hi?'))
+    assert_equal('hi?', context.invoke('hi', 'hi?'))
 
     context.add_filters(filter)
-    assert_equal('hi? hi!', context.invoke(:hi, 'hi?'))
+    assert_equal('hi? hi!', context.invoke('hi', 'hi?'))
   end
 
   def test_only_intended_filters_make_it_there

test/unit/partial_cache_unit_test.rb

@@ -90,4 +90,39 @@ class PartialCacheUnitTest < Minitest::Test
     # but measuring file reads is an OK proxy for this.
     assert_equal(1, file_system.file_read_count)
   end
+
+  def test_uses_default_template_factory_when_no_template_factory_found_in_register
+    context = Liquid::Context.build(
+      registers: {
+        file_system: StubFileSystem.new('my_partial' => 'my partial body'),
+      }
+    )
+
+    partial = Liquid::PartialCache.load(
+      'my_partial',
+      context: context,
+      parse_context: Liquid::ParseContext.new
+    )
+
+    assert_equal('my partial body', partial.render)
+  end
+
+  def test_uses_template_factory_register_if_present
+    template_factory = StubTemplateFactory.new
+    context = Liquid::Context.build(
+      registers: {
+        file_system: StubFileSystem.new('my_partial' => 'my partial body'),
+        template_factory: template_factory,
+      }
+    )
+
+    partial = Liquid::PartialCache.load(
+      'my_partial',
+      context: context,
+      parse_context: Liquid::ParseContext.new
+    )
+
+    assert_equal('my partial body', partial.render)
+    assert_equal(1, template_factory.count)
+  end
 end

test/unit/strainer_factory_unit_test.rb

@@ -0,0 +1,90 @@
+# frozen_string_literal: true
+
+require 'test_helper'
+
+class StrainerFactoryUnitTest < Minitest::Test
+  include Liquid
+
+  module AccessScopeFilters
+    def public_filter
+      "public"
+    end
+
+    def private_filter
+      "private"
+    end
+    private :private_filter
+  end
+
+  StrainerFactory.add_global_filter(AccessScopeFilters)
+
+  module LateAddedFilter
+    def late_added_filter(_input)
+      "filtered"
+    end
+  end
+
+  def setup
+    @context = Context.build
+  end
+
+  def test_strainer
+    strainer = StrainerFactory.create(@context)
+    assert_equal(5, strainer.invoke('size', '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", 1)
+    end
+  end
+
+  def test_stainer_argument_error_contains_backtrace
+    strainer = StrainerFactory.create(@context)
+
+    exception = assert_raises(Liquid::ArgumentError) do
+      strainer.invoke("public_filter", 1)
+    end
+
+    assert_match(
+      /\ALiquid error: wrong number of arguments \((1 for 0|given 1, expected 0)\)\z/,
+      exception.message
+    )
+    assert_equal(exception.backtrace[0].split(':')[0], __FILE__)
+  end
+
+  def test_strainer_only_invokes_public_filter_methods
+    strainer = StrainerFactory.create(@context)
+    assert_equal(false, strainer.class.invokable?('__test__'))
+    assert_equal(false, strainer.class.invokable?('test'))
+    assert_equal(false, strainer.class.invokable?('instance_eval'))
+    assert_equal(false, strainer.class.invokable?('__send__'))
+    assert_equal(true, strainer.class.invokable?('size')) # from the standard lib
+  end
+
+  def test_strainer_returns_nil_if_no_filter_method_found
+    strainer = StrainerFactory.create(@context)
+    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
+    strainer = StrainerFactory.create(@context)
+    assert_equal("password", strainer.invoke("undef_the_method", "password"))
+  end
+
+  def test_strainer_only_allows_methods_defined_in_filters
+    strainer = StrainerFactory.create(@context)
+    assert_equal("1 + 1", strainer.invoke("instance_eval", "1 + 1"))
+    assert_equal("puts",  strainer.invoke("__send__", "puts", "Hi Mom"))
+    assert_equal("has_method?", strainer.invoke("invoke", "has_method?", "invoke"))
+  end
+
+  def test_add_global_filter_clears_cache
+    assert_equal('input', StrainerFactory.create(@context).invoke('late_added_filter', 'input'))
+    StrainerFactory.add_global_filter(LateAddedFilter)
+    assert_equal('filtered', StrainerFactory.create(nil).invoke('late_added_filter', 'input'))
+  end
+end

test/unit/strainer_template_unit_test.rb

@@ -0,0 +1,18 @@
+# frozen_string_literal: true
+
+require 'test_helper'
+
+class StrainerTemplateUnitTest < Minitest::Test
+  include Liquid
+
+  def test_add_filter_when_wrong_filter_class
+    c = Context.new
+    s = c.strainer
+    wrong_filter = ->(v) { v.reverse }
+
+    exception = assert_raises(ArgumentError) do
+      s.class.add_filter(wrong_filter)
+    end
+    assert_equal(exception.message, "Liquid error: wrong argument type Proc (expected Liquid::Filter)")
+  end
+end

test/unit/strainer_unit_test.rb

@@ -1,167 +0,0 @@
-# frozen_string_literal: true
-
-require 'test_helper'
-
-class StrainerUnitTest < Minitest::Test
-  include Liquid
-
-  module AccessScopeFilters
-    def public_filter
-      "public"
-    end
-
-    def private_filter
-      "private"
-    end
-    private :private_filter
-  end
-
-  Strainer.global_filter(AccessScopeFilters)
-
-  def test_strainer
-    strainer = Strainer.create(nil)
-    assert_equal(5, strainer.invoke('size', 'input'))
-    assert_equal("public", strainer.invoke("public_filter"))
-  end
-
-  def test_stainer_raises_argument_error
-    strainer = Strainer.create(nil)
-    assert_raises(Liquid::ArgumentError) do
-      strainer.invoke("public_filter", 1)
-    end
-  end
-
-  def test_stainer_argument_error_contains_backtrace
-    strainer = Strainer.create(nil)
-    begin
-      strainer.invoke("public_filter", 1)
-    rescue Liquid::ArgumentError => e
-      assert_match(
-        /\ALiquid error: wrong number of arguments \((1 for 0|given 1, expected 0)\)\z/,
-        e.message
-      )
-      assert_equal(e.backtrace[0].split(':')[0], __FILE__)
-    end
-  end
-
-  def test_strainer_only_invokes_public_filter_methods
-    strainer = Strainer.create(nil)
-    assert_equal(false, strainer.class.invokable?('__test__'))
-    assert_equal(false, strainer.class.invokable?('test'))
-    assert_equal(false, strainer.class.invokable?('instance_eval'))
-    assert_equal(false, strainer.class.invokable?('__send__'))
-    assert_equal(true, strainer.class.invokable?('size')) # from the standard lib
-  end
-
-  def test_strainer_returns_nil_if_no_filter_method_found
-    strainer = Strainer.create(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
-    strainer = Strainer.create(nil)
-    assert_equal("password", strainer.invoke("undef_the_method", "password"))
-  end
-
-  def test_strainer_only_allows_methods_defined_in_filters
-    strainer = Strainer.create(nil)
-    assert_equal("1 + 1", strainer.invoke("instance_eval", "1 + 1"))
-    assert_equal("puts",  strainer.invoke("__send__", "puts", "Hi Mom"))
-    assert_equal("has_method?", strainer.invoke("invoke", "has_method?", "invoke"))
-  end
-
-  def test_strainer_uses_a_class_cache_to_avoid_method_cache_invalidation
-    a = Module.new
-    b = Module.new
-    strainer = Strainer.create(nil, [a, b])
-    assert_kind_of(Strainer, strainer)
-    assert_kind_of(a, strainer)
-    assert_kind_of(b, strainer)
-    assert_kind_of(Liquid::StandardFilters, strainer)
-  end
-
-  def test_add_filter_when_wrong_filter_class
-    c = Context.new
-    s = c.strainer
-    wrong_filter = ->(v) { v.reverse }
-
-    assert_raises ArgumentError do
-      s.class.add_filter(wrong_filter)
-    end
-  end
-
-  module PrivateMethodOverrideFilter
-    private
-
-    def public_filter
-      "overriden as private"
-    end
-  end
-
-  def test_add_filter_raises_when_module_privately_overrides_registered_public_methods
-    strainer = Context.new.strainer
-
-    error = assert_raises(Liquid::MethodOverrideError) do
-      strainer.class.add_filter(PrivateMethodOverrideFilter)
-    end
-    assert_equal('Liquid error: Filter overrides registered public methods as non public: public_filter', error.message)
-  end
-
-  module ProtectedMethodOverrideFilter
-    protected
-
-    def public_filter
-      "overriden as protected"
-    end
-  end
-
-  def test_add_filter_raises_when_module_overrides_registered_public_method_as_protected
-    strainer = Context.new.strainer
-
-    error = assert_raises(Liquid::MethodOverrideError) do
-      strainer.class.add_filter(ProtectedMethodOverrideFilter)
-    end
-    assert_equal('Liquid error: Filter overrides registered public methods as non public: public_filter', error.message)
-  end
-
-  module PublicMethodOverrideFilter
-    def public_filter
-      "public"
-    end
-  end
-
-  def test_add_filter_does_not_raise_when_module_overrides_previously_registered_method
-    strainer = Context.new.strainer
-    strainer.class.add_filter(PublicMethodOverrideFilter)
-    assert(strainer.class.filter_methods.include?('public_filter'))
-  end
-
-  module LateAddedFilter
-    def late_added_filter(_input)
-      "filtered"
-    end
-  end
-
-  def test_global_filter_clears_cache
-    assert_equal('input', Strainer.create(nil).invoke('late_added_filter', 'input'))
-    Strainer.global_filter(LateAddedFilter)
-    assert_equal('filtered', Strainer.create(nil).invoke('late_added_filter', 'input'))
-  end
-
-  def test_add_filter_does_not_include_already_included_module
-    mod = Module.new do
-      class << self
-        attr_accessor :include_count
-        def included(_mod)
-          self.include_count += 1
-        end
-      end
-      self.include_count = 0
-    end
-    strainer = Context.new.strainer
-    strainer.class.add_filter(mod)
-    strainer.class.add_filter(mod)
-    assert_equal(1, mod.include_count)
-  end
-end # StrainerTest

test/unit/template_factory_unit_test.rb

@@ -0,0 +1,12 @@
+# frozen_string_literal: true
+
+require 'test_helper'
+
+class TemplateFactoryUnitTest < Minitest::Test
+  include Liquid
+
+  def test_for_returns_liquid_template_instance
+    template = TemplateFactory.new.for("anything")
+    assert_instance_of(Liquid::Template, template)
+  end
+end