Liquid::Drop: Expect @context to always be set

.github/workflows/liquid.yml

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

History.md

@@ -1,14 +1,13 @@
 # Liquid Change Log
 
-## 5.3.0 (unreleased)
+## 5.2.1 (unreleased)
+
+### Features
 
 ### Fixes
-* StandardFilter: Fix missing @context on iterations (#1525) [Thierry Joyal]
+* Liquid::Drop: Expect @context to always be set (#1528) [Thierry Joyal]
 
-### Deprecation
-* Condition#evaluate to require mandatory context argument in Liquid 6.0.0 (#1527) [Thierry Joyal]
-
-## 5.2.0 2022-03-01
+## 5.2.0 2021-03-01
 
 ### Features
 * Add `remove_last`, and `replace_last` filters (#1422) [Anders Hagbard]
@@ -18,6 +17,7 @@
 * 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

lib/liquid/condition.rb

@@ -61,7 +61,7 @@ module Liquid
       @child_condition = nil
     end
 
-    def evaluate(context = deprecated_default_context)
+    def evaluate(context = Context.new)
       condition = self
       result = nil
       loop do
@@ -150,12 +150,6 @@ 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/drop.rb

@@ -27,7 +27,7 @@ module Liquid
 
     # Catch all for the method
     def liquid_method_missing(method)
-      return nil unless @context&.strict_variables
+      return nil unless @context.strict_variables
       raise Liquid::UndefinedDropMethod, "undefined method #{method}"
     end
 

lib/liquid/standardfilters.rb

@@ -95,32 +95,7 @@ module Liquid
       end
     end
 
-    # @public_docs
-    # @type filter
-    # @category string
-    # @summary
-    #   Truncates a string down to a specified number of characters.
-    # @description
-    #   Truncates a string down to a specified number of characters. By default, an ellipsis (`...`) 
-    #   is appended to the truncated string.
-    #
-    #   > Tip:
-    #   > The number of characters in both default and custom ellipses is included in the 
-    #   > character count for the truncated string. For example, if you want to truncate a string 
-    #   > down to ten characters and use the default ellipsis (`...`), then you should set the 
-    #   > character count parameter to `13`. 
-    #
-    #   ### Custom ellipsis
-    #
-    #   The `truncate` filter accepts an optional parameter to specify a custom ellipsis to be 
-    #   appended to the truncated string.
-    #
-    #   ### No ellipsis
-    #
-    #   If you don't want an ellipsis appended to your truncated string, then you can set the 
-    #   ellipsis parameter to a blank string (`''`).
-    # @syntax {{ string | truncate: character_count, ellipsis }}
-    # @return string
+    # Truncate a string down to x characters
     def truncate(input, length = 50, truncate_string = "...")
       return if input.nil?
       input_str = input.to_s
@@ -134,26 +109,6 @@ module Liquid
       input_str.length > length ? input_str[0...l].concat(truncate_string_str) : input_str
     end
 
-    # @public_docs
-    # @type filter
-    # @category string
-    # @summary
-    #   Truncates a string down to a specified number of words.
-    # @description
-    #   Truncates a string down to a specified number of words. By default, an ellipsis (`...`) 
-    #   is appended to the truncated string.
-    #
-    #   ### Custom ellipsis
-    #
-    #   The `truncate` filter accepts an optional parameter to specify a custom ellipsis to be 
-    #   appended to the truncated string.
-    #
-    #   ### No ellipsis
-    #
-    #   If you don't want an ellipsis appended to your truncated string, then you can set the 
-    #   ellipsis parameter to a blank string (`''`).
-    # @syntax {{ string | truncatewords: word_count, ellipsis }}
-    # @return string
     def truncatewords(input, words = 15, truncate_string = "...")
       return if input.nil?
       input = input.to_s
@@ -182,54 +137,18 @@ module Liquid
       input.to_s.split(pattern.to_s)
     end
 
-    # @public_docs
-    # @type filter
-    # @category string
-    # @summary
-    #   Strips all whitespace, such as tabs, spaces, and newlines, from the left and right sides of a string.
-    # @description
-    #   Strips all whitespace, such as tabs, spaces, and newlines, from the left and right sides of a string.
-    # @syntax {{ string | strip }}
-    # @return string
     def strip(input)
       input.to_s.strip
     end
 
-    # @public_docs
-    # @type filter
-    # @category string
-    # @summary
-    #   Strips all whitespace, such as tabs, spaces, and newlines, from the left side of a string.
-    # @description
-    #   Strips all whitespace, such as tabs, spaces, and newlines, from the left side of a string.
-    # @syntax {{ string | lstrip }}
-    # @return string
     def lstrip(input)
       input.to_s.lstrip
     end
 
-    # @public_docs
-    # @type filter
-    # @category string
-    # @summary
-    #   Strips all whitespace, such as tabs, spaces, and newlines, from the right side of a string.
-    # @description
-    #   Strips all whitespace, such as tabs, spaces, and newlines, from the right side of a string.
-    # @syntax {{ string | rstrip }}
-    # @return string
     def rstrip(input)
       input.to_s.rstrip
     end
 
-    # @public_docs
-    # @type filter
-    # @category string
-    # @summary
-    #   Strips all HTML tags from a string.
-    # @description
-    #   Strips all HTML tags from a string.
-    # @syntax {{ string | strip_html }}
-    # @return string
     def strip_html(input)
       empty  = ''
       result = input.to_s.gsub(STRIP_HTML_BLOCKS, empty)
@@ -237,15 +156,7 @@ module Liquid
       result
     end
 
-    # @public_docs
-    # @type filter
-    # @category string
-    # @summary
-    #   Strips all line breaks and newlines from a string.
-    # @description
-    #   Strips all line breaks and newlines from a string.
-    # @syntax {{ string | strip_newlines }}
-    # @return string
+    # Remove all newlines from the string
     def strip_newlines(input)
       input.to_s.gsub(/\r?\n/, '')
     end
@@ -671,9 +582,8 @@ module Liquid
 
       def each
         @input.each do |e|
-          e = e.respond_to?(:to_liquid) ? e.to_liquid : e
           e.context = @context if e.respond_to?(:context=)
-          yield(e)
+          yield(e.respond_to?(:to_liquid) ? e.to_liquid : e)
         end
       end
     end

lib/liquid/static_registers.rb

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

lib/liquid/version.rb

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

test/integration/context_test.rb

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

test/integration/profiler_test.rb

@@ -3,27 +3,6 @@
 require 'test_helper'
 
 class ProfilerTest < Minitest::Test
-  class TestDrop < Liquid::Drop
-    def initialize(value)
-      super()
-      @value = value
-    end
-
-    def to_s
-      artificial_execution_time
-
-      @value
-    end
-
-    private
-
-    # Monotonic clock precision fluctuate based on the operating system
-    # By introducing a small sleep we ensure ourselves to register a non zero unit of time
-    def artificial_execution_time
-      sleep(Process.clock_getres(Process::CLOCK_MONOTONIC))
-    end
-  end
-
   include Liquid
 
   class ProfilingFileSystem
@@ -219,22 +198,16 @@ class ProfilerTest < Minitest::Test
 
   def test_profiling_supports_self_time
     t = Template.parse("{% for item in collection %} {{ item }} {% endfor %}", profile: true)
-    collection = [
-      TestDrop.new("one"),
-      TestDrop.new("two"),
-    ]
-    output = t.render!("collection" => collection)
-    assert_equal(" one  two ", output)
-
+    t.render!("collection" => ["one", "two"])
     leaf = t.profiler[0].children[0]
-    assert_operator(leaf.self_time, :>, 0.0)
+
+    assert_operator(leaf.self_time, :>, 0)
   end
 
   def test_profiling_supports_total_time
-    t = Template.parse("{% if true %} {{ test }} {% endif %}", profile: true)
-    output = t.render!("test" => TestDrop.new("one"))
-    assert_equal(" one ", output)
+    t = Template.parse("{% if true %} {% increment test %} {{ test }} {% endif %}", profile: true)
+    t.render!
 
-    assert_operator(t.profiler[0].total_time, :>, 0.0)
+    assert_operator(t.profiler[0].total_time, :>, 0)
   end
 end

test/integration/standard_filter_test.rb

@@ -25,24 +25,8 @@ class TestThing
 end
 
 class TestDrop < Liquid::Drop
-  def initialize(value:)
-    @value = value
-  end
-
-  attr_reader :value
-
-  def registers
-    @context.registers
-  end
-end
-
-class TestModel
-  def initialize(value:)
-    @value = value
-  end
-
-  def to_liquid
-    TestDrop.new(value: @value)
+  def test
+    "testfoo"
   end
 end
 
@@ -378,9 +362,8 @@ class StandardFiltersTest < Minitest::Test
     assert_equal(["foo"], @filters.uniq("foo"))
     assert_equal([1, 3, 2, 4], @filters.uniq([1, 1, 3, 2, 3, 1, 4, 3, 2, 1]))
     assert_equal([{ "a" => 1 }, { "a" => 3 }, { "a" => 2 }], @filters.uniq([{ "a" => 1 }, { "a" => 3 }, { "a" => 1 }, { "a" => 2 }], "a"))
-    test_drop = TestDrop.new(value: "test")
-    test_drop_alternate = TestDrop.new(value: "test")
-    assert_equal([test_drop], @filters.uniq([test_drop, test_drop_alternate], 'value'))
+    testdrop = TestDrop.new
+    assert_equal([testdrop], @filters.uniq([testdrop, TestDrop.new], 'test'))
   end
 
   def test_uniq_empty_array
@@ -439,16 +422,6 @@ class StandardFiltersTest < Minitest::Test
     assert_template_result("woot: 1", '{{ foo | map: "whatever" }}', "foo" => [t])
   end
 
-  def test_map_calls_context=
-    model = TestModel.new(value: "test")
-
-    template = Template.parse('{{ foo | map: "registers" }}')
-    template.registers[:test] = 1234
-    template.assigns['foo'] = [model]
-
-    assert_template_result("{:test=>1234}", template.render!)
-  end
-
   def test_map_on_hashes
     assert_template_result("4217", '{{ thing | map: "foo" | map: "bar" }}',
       "thing" => { "foo" => [{ "bar" => 42 }, { "bar" => 17 }] })
@@ -467,9 +440,9 @@ class StandardFiltersTest < Minitest::Test
   end
 
   def test_map_over_proc
-    drop  = TestDrop.new(value: "testfoo")
+    drop  = TestDrop.new
     p     = proc { drop }
-    templ = '{{ procs | map: "value" }}'
+    templ = '{{ procs | map: "test" }}'
     assert_template_result("testfoo", templ, "procs" => [p])
   end
 
@@ -865,7 +838,7 @@ class StandardFiltersTest < Minitest::Test
   end
 
   def test_all_filters_never_raise_non_liquid_exception
-    test_drop = TestDrop.new(value: "test")
+    test_drop = TestDrop.new
     test_drop.context = Context.new
     test_enum = TestEnumerable.new
     test_enum.context = Context.new

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(Context.new))
-    assert_equal(true,  Condition.new(1, '==', 1).evaluate(Context.new))
+    assert_equal(false, Condition.new(1, '==', 2).evaluate)
+    assert_equal(true,  Condition.new(1, '==', 1).evaluate)
   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(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))
+    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)
   end
 
   def test_contains_works_on_arrays
@@ -106,29 +106,30 @@ class ConditionUnitTest < Minitest::Test
 
   def test_or_condition
     condition = Condition.new(1, '==', 2)
-    assert_equal(false, condition.evaluate(Context.new))
+
+    assert_equal(false, condition.evaluate)
 
     condition.or(Condition.new(2, '==', 1))
 
-    assert_equal(false, condition.evaluate(Context.new))
+    assert_equal(false, condition.evaluate)
 
     condition.or(Condition.new(1, '==', 1))
 
-    assert_equal(true, condition.evaluate(Context.new))
+    assert_equal(true, condition.evaluate)
   end
 
   def test_and_condition
     condition = Condition.new(1, '==', 1)
 
-    assert_equal(true, condition.evaluate(Context.new))
+    assert_equal(true, condition.evaluate)
 
     condition.and(Condition.new(2, '==', 2))
 
-    assert_equal(true, condition.evaluate(Context.new))
+    assert_equal(true, condition.evaluate)
 
     condition.and(Condition.new(2, '==', 1))
 
-    assert_equal(false, condition.evaluate(Context.new))
+    assert_equal(false, condition.evaluate)
   end
 
   def test_should_allow_custom_proc_operator
@@ -147,20 +148,6 @@ 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)