Only use MethodLiteral in condition expressions (#1300)

Shopify · Sep 25, 2020 · a03f027 · a03f027
1 parent ca4b9b4
commit a03f027
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 22 deletions.
diff --git a/lib/liquid/condition.rb b/lib/liquid/condition.rb
@@ -27,10 +27,28 @@ class Condition #:nodoc:
       end,
     }
 
+    class MethodLiteral
+      attr_reader :method_name, :to_s
+
+      def initialize(method_name, to_s)
+        @method_name = method_name
+        @to_s = to_s
+      end
+    end
+
+    @@method_literals = {
+      'blank' => MethodLiteral.new(:blank?, '').freeze,
+      'empty' => MethodLiteral.new(:empty?, '').freeze,
+    }
+
     def self.operators
       @@operators
     end
 
+    def self.parse_expression(markup)
+      @@method_literals[markup] || Expression.parse(markup)
+    end
+
     attr_reader :attachment, :child_condition
     attr_accessor :left, :operator, :right
 
@@ -91,15 +109,15 @@ def inspect
     private
 
     def equal_variables(left, right)
-      if left.is_a?(Liquid::Expression::MethodLiteral)
+      if left.is_a?(MethodLiteral)
         if right.respond_to?(left.method_name)
           return right.send(left.method_name)
         else
           return nil
         end
       end
 
-      if right.is_a?(Liquid::Expression::MethodLiteral)
+      if right.is_a?(MethodLiteral)
         if left.respond_to?(right.method_name)
           return left.send(right.method_name)
         else

diff --git a/lib/liquid/expression.rb b/lib/liquid/expression.rb
@@ -2,25 +2,12 @@
 
 module Liquid
   class Expression
-    class MethodLiteral
-      attr_reader :method_name, :to_s
-
-      def initialize(method_name, to_s)
-        @method_name = method_name
-        @to_s = to_s
-      end
-
-      def to_liquid
-        to_s
-      end
-    end
-
     LITERALS = {
       nil => nil, 'nil' => nil, 'null' => nil, '' => nil,
       'true' => true,
       'false' => false,
-      'blank' => MethodLiteral.new(:blank?, '').freeze,
-      'empty' => MethodLiteral.new(:empty?, '').freeze
+      'blank' => '',
+      'empty' => ''
     }.freeze
 
     SINGLE_QUOTED_STRING = /\A'(.*)'\z/m

diff --git a/lib/liquid/tags/case.rb b/lib/liquid/tags/case.rb
@@ -68,7 +68,7 @@ def record_when_condition(markup)
 
         markup = Regexp.last_match(2)
 
-        block = Condition.new(@left, '==', Expression.parse(Regexp.last_match(1)))
+        block = Condition.new(@left, '==', Condition.parse_expression(Regexp.last_match(1)))
         block.attach(body)
         @blocks << block
       end

diff --git a/lib/liquid/tags/if.rb b/lib/liquid/tags/if.rb
@@ -67,18 +67,22 @@ def push_block(tag, markup)
       block.attach(new_body)
     end
 
+    def parse_expression(markup)
+      Condition.parse_expression(markup)
+    end
+
     def lax_parse(markup)
       expressions = markup.scan(ExpressionsAndOperators)
       raise SyntaxError, options[:locale].t("errors.syntax.if") unless expressions.pop =~ Syntax
 
-      condition = Condition.new(Expression.parse(Regexp.last_match(1)), Regexp.last_match(2), Expression.parse(Regexp.last_match(3)))
+      condition = Condition.new(parse_expression(Regexp.last_match(1)), Regexp.last_match(2), parse_expression(Regexp.last_match(3)))
 
       until expressions.empty?
         operator = expressions.pop.to_s.strip
 
         raise SyntaxError, options[:locale].t("errors.syntax.if") unless expressions.pop.to_s =~ Syntax
 
-        new_condition = Condition.new(Expression.parse(Regexp.last_match(1)), Regexp.last_match(2), Expression.parse(Regexp.last_match(3)))
+        new_condition = Condition.new(parse_expression(Regexp.last_match(1)), Regexp.last_match(2), parse_expression(Regexp.last_match(3)))
         raise SyntaxError, options[:locale].t("errors.syntax.if") unless BOOLEAN_OPERATORS.include?(operator)
         new_condition.send(operator, condition)
         condition = new_condition
@@ -106,9 +110,9 @@ def parse_binary_comparisons(p)
     end
 
     def parse_comparison(p)
-      a = Expression.parse(p.expression)
+      a = parse_expression(p.expression)
       if (op = p.consume?(:comparison))
-        b = Expression.parse(p.expression)
+        b = parse_expression(p.expression)
         Condition.new(a, op, b)
       else
         Condition.new(a)