WIP2

History.md

@@ -2,8 +2,6 @@
 
 ## 6.0.0
 
-### Architectural changes
-
 ### Features
 * (TODO) Add support for boolean expressions everywhere
   * As variable output `{{ a or b }}`
@@ -21,17 +19,28 @@
 - (TODO) Add support for parenthesized expressions
   * e.g. `(a or b) and c`
 
+### Architectural changes
+* `parse_expression` and `safe_parse_expression` have been removed from `Tag` and `ParseContext`
+* `Parser` methods now produce AST nodes instead of strings
+  * `Parser#expression` produces a value,
+  * `Parser#string` produces a string,
+  * etc.
+
 ### Breaking changes
 * The Environment's `error_mode` option has been removed.
   * `:warn` is no longer supported
   * `:lax` and `lax_parse` is no longer supported
   * `:strict` and `strict_parse` is no longer supported
   * `strict2_parse` is renamed to `parse_markup`
 * The `warnings` system has been removed.
+* `Parser#expression` is renamed to `Parser#expression_string`
+* `safe_parse_expression` methods are replaced by `Parser#expression`
+* `parse_expression` methods are replaced by `Parser#unsafe_parse_expression`
 
 ### Migrating from `^5.11.0`
 - In custom tags that include `ParserSwitching`, rename `strict2_parse` to `parse_markup`
 - Remove code depending on `:error_mode`
+- Replace `safe_parse_expression` calls with `Parser#expression`
 
 ## 5.11.0
 * Revert the Inline Snippets tag (#2001), treat its inclusion in the latest Liquid release as a bug, and allow for feedback on RFC#1916 to better support Liquid developers [Guilherme Carreiro]

lib/liquid/condition.rb

@@ -48,8 +48,9 @@ def self.operators
       @@operators
     end
 
-    def self.parse_expression(parse_context, markup, safe: false)
-      @@method_literals[markup] || parse_context.parse_expression(markup, safe: safe)
+    def self.parse_expression(parser)
+      markup = parser.expression_string
+      @@method_literals[markup] || parser.unsafe_parse_expression(markup)
     end
 
     attr_reader :attachment, :child_condition

lib/liquid/context.rb

@@ -175,7 +175,8 @@ def []=(key, value)
     # Example:
     #   products == empty #=> products.empty?
     def [](expression)
-      evaluate(Expression.parse(expression, @string_scanner))
+      @string_scanner.string = expression
+      evaluate(Parser.new(@string_scanner).expression)
     end
 
     def key?(key)

lib/liquid/expression.rb

@@ -25,10 +25,6 @@ class Expression
     FLOAT_REGEX = /\A(-?\d+)\.\d+\z/
 
     class << self
-      def safe_parse(parser, ss = StringScanner.new(""), cache = nil)
-        parse(parser.expression, ss, cache)
-      end
-
       def parse(markup, ss = StringScanner.new(""), cache = nil)
         return unless markup
 
@@ -53,71 +49,34 @@ def parse(markup, ss = StringScanner.new(""), cache = nil)
 
       def inner_parse(markup, ss, cache)
         if (markup.start_with?("(") && markup.end_with?(")")) && markup =~ RANGES_REGEX
-          return RangeLookup.parse(
-            Regexp.last_match(1),
-            Regexp.last_match(2),
-            ss,
-            cache,
+          start_markup = Regexp.last_match(1)
+          end_markup = Regexp.last_match(2)
+          start_obj = parse(start_markup, ss, cache)
+          end_obj = parse(end_markup, ss, cache)
+          return RangeLookup.create(
+            start_obj,
+            end_obj,
+            start_markup,
+            end_markup,
           )
         end
 
-        if (num = parse_number(markup, ss))
+        if (num = parse_number(markup))
           num
         else
           VariableLookup.parse(markup, ss, cache)
         end
       end
 
-      def parse_number(markup, ss)
+      def parse_number(markup)
         # check if the markup is simple integer or float
         case markup
         when INTEGER_REGEX
-          return Integer(markup, 10)
+          Integer(markup, 10)
         when FLOAT_REGEX
-          return markup.to_f
-        end
-
-        ss.string = markup
-        # the first byte must be a digit or  a dash
-        byte = ss.scan_byte
-
-        return false if byte != DASH && (byte < ZERO || byte > NINE)
-
-        if byte == DASH
-          peek_byte = ss.peek_byte
-
-          # if it starts with a dash, the next byte must be a digit
-          return false if peek_byte.nil? || !(peek_byte >= ZERO && peek_byte <= NINE)
-        end
-
-        # The markup could be a float with multiple dots
-        first_dot_pos = nil
-        num_end_pos = nil
-
-        while (byte = ss.scan_byte)
-          return false if byte != DOT && (byte < ZERO || byte > NINE)
-
-          # we found our number and now we are just scanning the rest of the string
-          next if num_end_pos
-
-          if byte == DOT
-            if first_dot_pos.nil?
-              first_dot_pos = ss.pos
-            else
-              # we found another dot, so we know that the number ends here
-              num_end_pos = ss.pos - 1
-            end
-          end
-        end
-
-        num_end_pos = markup.length if ss.eos?
-
-        if num_end_pos
-          # number ends with a number "123.123"
-          markup.byteslice(0, num_end_pos).to_f
+          markup.to_f
         else
-          # number ends with a dot "123."
-          markup.byteslice(0, first_dot_pos).to_f
+          false
         end
       end
     end

lib/liquid/parse_context.rb

@@ -37,7 +37,7 @@ def new_block_body
 
     def new_parser(input)
       @string_scanner.string = input
-      Parser.new(@string_scanner)
+      Parser.new(@string_scanner, @expression_cache)
     end
 
     def new_tokenizer(source, start_line_number: nil, for_liquid_tag: false)
@@ -49,21 +49,6 @@ def new_tokenizer(source, start_line_number: nil, for_liquid_tag: false)
       )
     end
 
-    def safe_parse_expression(parser)
-      Expression.safe_parse(parser, @string_scanner, @expression_cache)
-    end
-
-    def parse_expression(markup, safe: false)
-      # markup MUST come from a string returned by the parser
-      # (e.g., parser.expression). We're not calling the parser here to
-      # prevent redundant parser overhead. The `safe` opt-in
-      # exists to ensure it is not accidentally still called with
-      # the result of a regex.
-      raise Liquid::InternalError, "unsafe parse_expression cannot be used" unless safe
-
-      Expression.parse(markup, @string_scanner, @expression_cache)
-    end
-
     def partial=(value)
       @partial = value
       @options = value ? partial_options : @template_options

lib/liquid/parser.rb

@@ -2,9 +2,10 @@
 
 module Liquid
   class Parser
-    def initialize(input)
-      ss = input.is_a?(StringScanner) ? input : StringScanner.new(input)
-      @tokens = Lexer.tokenize(ss)
+    def initialize(input, expression_cache = nil)
+      @ss = input.is_a?(StringScanner) ? input : StringScanner.new(input)
+      @cache = expression_cache
+      @tokens = Lexer.tokenize(@ss)
       @p      = 0 # pointer to current location
     end
 
@@ -47,47 +48,99 @@ def look(type, ahead = 0)
     end
 
     def expression
+      token = @tokens[@p]
+      case token[0]
+      when :id
+        variable_lookup
+      when :open_square
+        unnamed_variable_lookup
+      when :string
+        string
+      when :number
+        number
+      when :open_round
+        range_lookup
+      else
+        raise SyntaxError, "#{token} is not a valid expression"
+      end
+    end
+
+    def number
+      num = consume(:number)
+      Expression.parse_number(num)
+    end
+
+    def string
+      consume(:string)[1..-2]
+    end
+
+    def variable_lookup
+      name = consume(:id)
+      lookups, command_flags = variable_lookups
+      if Expression::LITERALS.key?(name) && lookups.empty?
+        Expression::LITERALS[name]
+      else
+        VariableLookup.new(name, lookups, command_flags)
+      end
+    end
+
+    def unnamed_variable_lookup
+      name = indexed_lookup
+      lookups, command_flags = variable_lookups
+      VariableLookup.new(name, lookups, command_flags)
+    end
+
+    def range_lookup
+      consume(:open_round)
+      first = expression
+      consume(:dotdot)
+      last = expression
+      consume(:close_round)
+      RangeLookup.create(first, last)
+    end
+
+    def expression_string
       token = @tokens[@p]
       case token[0]
       when :id
         str = consume
-        str << variable_lookups
+        str << variable_lookups_string
       when :open_square
         str = consume.dup
-        str << expression
+        str << expression_string
         str << consume(:close_square)
-        str << variable_lookups
+        str << variable_lookups_string
       when :string, :number
         consume
       when :open_round
         consume
-        first = expression
+        first = expression_string
         consume(:dotdot)
-        last = expression
+        last = expression_string
         consume(:close_round)
         "(#{first}..#{last})"
       else
         raise SyntaxError, "#{token} is not a valid expression"
       end
     end
 
-    def argument
+    def argument_string
       str = +""
       # might be a keyword argument (identifier: expression)
       if look(:id) && look(:colon, 1)
         str << consume << consume << ' '
       end
 
-      str << expression
+      str << expression_string
       str
     end
 
-    def variable_lookups
+    def variable_lookups_string
       str = +""
       loop do
         if look(:open_square)
           str << consume
-          str << expression
+          str << expression_string
           str << consume(:close_square)
         elsif look(:dot)
           str << consume
@@ -98,5 +151,43 @@ def variable_lookups
       end
       str
     end
+
+    # Assumes safe input. For cases where you need the string.
+    # Don't use this unless you're sure about what you're doing.
+    def unsafe_parse_expression(markup)
+      parse_expression(markup)
+    end
+
+    private
+
+    def parse_expression(markup)
+      Expression.parse(markup, @ss, @cache)
+    end
+
+    def variable_lookups
+      lookups = []
+      command_flags = 0
+      i = -1
+      loop do
+        i += 1
+        if look(:open_square)
+          lookups << indexed_lookup
+        elsif consume?(:dot)
+          lookup = consume(:id)
+          lookups << lookup
+          command_flags |= 1 << i if VariableLookup::COMMAND_METHODS.include?(lookup)
+        else
+          break
+        end
+      end
+      [lookups, command_flags]
+    end
+
+    def indexed_lookup
+      consume(:open_square)
+      expr = expression
+      consume(:close_square)
+      expr
+    end
   end
 end

lib/liquid/range_lookup.rb

@@ -2,15 +2,15 @@
 
 module Liquid
   class RangeLookup
-    def self.parse(start_markup, end_markup, string_scanner, cache = nil)
-      start_obj = Expression.parse(start_markup, string_scanner, cache)
-      end_obj   = Expression.parse(end_markup, string_scanner, cache)
+    def self.create(start_obj, end_obj, start_markup = nil, end_markup = nil)
       if start_obj.respond_to?(:evaluate) || end_obj.respond_to?(:evaluate)
         new(start_obj, end_obj)
       else
         begin
           start_obj.to_i..end_obj.to_i
         rescue NoMethodError
+          start_markup = start_obj.to_s unless start_markup
+          end_markup = end_obj.to_s unless end_markup
           invalid_expr = start_markup unless start_obj.respond_to?(:to_i)
           invalid_expr ||= end_markup unless end_obj.respond_to?(:to_i)
           if invalid_expr

lib/liquid/tag.rb

@@ -65,15 +65,5 @@ def render_to_output_buffer(context, output)
     def blank?
       false
     end
-
-    private
-
-    def safe_parse_expression(parser)
-      parse_context.safe_parse_expression(parser)
-    end
-
-    def parse_expression(markup, safe: false)
-      parse_context.parse_expression(markup, safe: safe)
-    end
   end
 end