From c8f116c80ad37151a0bd0aa7d730ee57a341a1ca Mon Sep 17 00:00:00 2001
From: Dylan Thacker-Smith <Dylan.Smith@shopify.com>
Date: Tue, 27 Oct 2020 09:23:49 -0400
Subject: [PATCH] Respect disable_liquid_c_nodes when parsing expressions

---
 ext/liquid_c/expression.c    |  3 +++
 lib/liquid/c.rb              | 34 ++++++++++++++++++++++------------
 test/unit/expression_test.rb | 12 ++++++++++++
 3 files changed, 37 insertions(+), 12 deletions(-)

diff --git a/ext/liquid_c/expression.c b/ext/liquid_c/expression.c
index f7d3f69e..e0a6ac42 100644
--- a/ext/liquid_c/expression.c
+++ b/ext/liquid_c/expression.c
@@ -62,6 +62,9 @@ static VALUE internal_expression_parse(parser_t *p)
 
 static VALUE expression_strict_parse(VALUE klass, VALUE markup)
 {
+    if (NIL_P(markup))
+        return Qnil;
+
     StringValue(markup);
     char *start = RSTRING_PTR(markup);
 
diff --git a/lib/liquid/c.rb b/lib/liquid/c.rb
index 5e6dc6ef..5afb2469 100644
--- a/lib/liquid/c.rb
+++ b/lib/liquid/c.rb
@@ -70,7 +70,6 @@ def new_block_body
   end
 
   alias_method :ruby_new_tokenizer, :new_tokenizer
-
   def new_tokenizer(source, start_line_number: nil, for_liquid_tag: false)
     unless liquid_c_nodes_disabled?
       source = source.to_s.to_str
@@ -85,6 +84,14 @@ def new_tokenizer(source, start_line_number: nil, for_liquid_tag: false)
     ruby_new_tokenizer(source, start_line_number: start_line_number, for_liquid_tag: for_liquid_tag)
   end
 
+  def parse_expression(markup)
+    if liquid_c_nodes_disabled?
+      Liquid::Expression.ruby_parse(markup)
+    else
+      Liquid::C::Expression.lax_parse(markup)
+    end
+  end
+
   # @api private
   def liquid_c_nodes_disabled?
     # Liquid::Profiler exposes the internal parse tree that we don't want to build when
@@ -195,21 +202,22 @@ def arg_exc_to_liquid_exc(argument_error)
   end
 end
 
+Liquid::C::Expression.class_eval do
+  class << self
+    def lax_parse(markup)
+      strict_parse(markup)
+    rescue Liquid::SyntaxError
+      Liquid::Expression.ruby_parse(markup)
+    end
+  end
+end
+
 Liquid::Expression.class_eval do
   class << self
     alias_method :ruby_parse, :parse
 
-    def parse(markup)
-      return nil unless markup
-
-      if Liquid::C.enabled
-        begin
-          return Liquid::C::Expression.strict_parse(markup)
-        rescue Liquid::SyntaxError
-          # no-op
-        end
-      end
-      ruby_parse(markup)
+    def c_parse(markup)
+      Liquid::C::Expression.lax_parse(markup)
     end
   end
 end
@@ -251,10 +259,12 @@ def enabled=(value)
           Liquid::Context.send(:alias_method, :evaluate, :c_evaluate)
           Liquid::Context.send(:alias_method, :find_variable, :c_find_variable_kwarg)
           Liquid::Context.send(:alias_method, :strict_variables=, :c_strict_variables=)
+          Liquid::Expression.singleton_class.send(:alias_method, :parse, :c_parse)
         else
           Liquid::Context.send(:alias_method, :evaluate, :ruby_evaluate)
           Liquid::Context.send(:alias_method, :find_variable, :ruby_find_variable)
           Liquid::Context.send(:alias_method, :strict_variables=, :ruby_strict_variables=)
+          Liquid::Expression.singleton_class.send(:alias_method, :parse, :ruby_parse)
         end
       end
     end
diff --git a/test/unit/expression_test.rb b/test/unit/expression_test.rb
index 73e09a68..b520cec8 100644
--- a/test/unit/expression_test.rb
+++ b/test/unit/expression_test.rb
@@ -157,6 +157,18 @@ def test_disassemble_int16
     ASM
   end
 
+  def test_disable_c_nodes
+    context = Liquid::Context.new({ "x" => 123 })
+
+    expr = Liquid::ParseContext.new.parse_expression('x')
+    assert_instance_of(Liquid::C::Expression, expr)
+    assert_equal(123, context.evaluate(expr))
+
+    expr = Liquid::ParseContext.new(disable_liquid_c_nodes: true).parse_expression('x')
+    assert_instance_of(Liquid::VariableLookup, expr)
+    assert_equal(123, context.evaluate(expr))
+  end
+
   private
 
   class ReturnKeyDrop < Liquid::Drop