From 1d2bee1f6053a03baeb888d333e01caed542c161 Mon Sep 17 00:00:00 2001
From: Thierry Joyal <thierry.joyal@gmail.com>
Date: Wed, 2 Mar 2022 16:14:55 +0000
Subject: [PATCH] Condition#evaluate to receive mandatory context argument

---
 History.md                       |  6 ++++-
 lib/liquid/condition.rb          |  8 ++++++-
 lib/liquid/version.rb            |  2 +-
 test/unit/condition_unit_test.rb | 41 +++++++++++++++++++++-----------
 4 files changed, 40 insertions(+), 17 deletions(-)

diff --git a/History.md b/History.md
index 07c09025..41203af5 100644
--- a/History.md
+++ b/History.md
@@ -1,5 +1,10 @@
 # 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
@@ -10,7 +15,6 @@
 * 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
diff --git a/lib/liquid/condition.rb b/lib/liquid/condition.rb
index 786f9bc9..595f1713 100644
--- a/lib/liquid/condition.rb
+++ b/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
         [
diff --git a/lib/liquid/version.rb b/lib/liquid/version.rb
index 6ab7b553..b164ba96 100644
--- a/lib/liquid/version.rb
+++ b/lib/liquid/version.rb
@@ -2,5 +2,5 @@
 # frozen_string_literal: true
 
 module Liquid
-  VERSION = "5.2.0"
+  VERSION = "5.3.0.alpha"
 end
diff --git a/test/unit/condition_unit_test.rb b/test/unit/condition_unit_test.rb
index 67f91627..8fcc4510 100644
--- a/test/unit/condition_unit_test.rb
+++ b/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)