From ba11b8587587727d15258af273d9e079dfce1752 Mon Sep 17 00:00:00 2001 From: Chris Pak Date: Sat, 4 Apr 2026 20:16:37 -0700 Subject: [PATCH] Decomposes, extracts, and names the important concepts Decomposes the 211-line try_fast_parse monolith in variable.rb into four named private methods -- fast_scan_name, fast_resolve_name, fast_scan_filters, fast_scan_filter_args -- and simplifies simple_variable_markup from a 55-line byte scanner to a 10-line regex with fast pre-checks. Generates invoke_single/invoke_two via module_eval in strainer_template.rb and context.rb instead of duplicating near-identical method bodies. The two arity variants differ only in parameter lists; generating them makes the pattern explicit and eliminates copy-paste drift. Extracts identical 4-line text-token handling block in block_body.rb into private append_text_token(token, parse_context). The block appeared in both the stray-{ fallback and the plain-text branch of parse_for_document. Extracts accessible?(object, key) predicate from the 4-line inline ternary in variable_lookup.rb that checked hash/array key presence; extracts liquidize(object, context) from the duplicated to_liquid + context= wiring that appeared in both the key-found and command-method branches. Extracts find_in_envs(envs, key, raise_on_not_found:) from try_variable_find_in_environments in context.rb, which looped over @environments then @static_environments with identical loop bodies. Adds 28 fast-path equivalence tests for Variable (variable_fast_parse_test.rb). --- lib/liquid/block_body.rb | 107 ++++---- lib/liquid/context.rb | 56 ++-- lib/liquid/strainer_template.rb | 49 ++-- lib/liquid/variable.rb | 367 +++++++++++++------------- lib/liquid/variable_lookup.rb | 44 +-- test/unit/variable_fast_parse_test.rb | 126 +++++++++ 6 files changed, 458 insertions(+), 291 deletions(-) create mode 100644 test/unit/variable_fast_parse_test.rb diff --git a/lib/liquid/block_body.rb b/lib/liquid/block_body.rb index dbca1382..6b7d92ab 100644 --- a/lib/liquid/block_body.rb +++ b/lib/liquid/block_body.rb @@ -52,8 +52,7 @@ module Liquid end unless (tag = parse_context.environment.tag_for_name(tag_name)) - # end parsing if we reach an unknown tag and let the caller decide - # determine how to proceed + # end parsing if we reach an unknown tag; let the caller determine how to proceed return yield tag_name, markup end new_tag = tag.parse(tag_name, markup, tokenizer, parse_context) @@ -124,11 +123,9 @@ module Liquid end - # Fast check if string is whitespace-only (replaces WhitespaceOrNothing regex) - BLANK_STRING_REGEX = /\A\s*\z/ def self.blank_string?(str) - str.match?(BLANK_STRING_REGEX) + str.match?(WhitespaceOrNothing) end private def parse_for_document(tokenizer, parse_context, &block) @@ -139,52 +136,26 @@ module Liquid if first_byte == Cursor::LCURLY second_byte = token.getbyte(1) if second_byte == Cursor::PCT - whitespace_handler(token, parse_context) - cursor = parse_context.cursor - tag_name = cursor.parse_tag_token(token) - unless tag_name - return handle_invalid_tag_token(token, parse_context, &block) - end - markup = cursor.tag_markup - - if parse_context.line_number - newlines = cursor.tag_newlines - parse_context.line_number += newlines if newlines > 0 - end - - if tag_name == 'liquid' - parse_liquid_tag(markup, parse_context) - next - end - - unless (tag = parse_context.environment.tag_for_name(tag_name)) - # end parsing if we reach an unknown tag and let the caller decide - # determine how to proceed - return yield tag_name, markup - end - new_tag = tag.parse(tag_name, markup, tokenizer, parse_context) - @blank &&= new_tag.blank? - @nodelist << new_tag + # handle_tag_token returns: + # nil — tag parsed normally, continue (update line number) + # :next — 'liquid' inline tag; skip line number update + # :unknown — end tag or unknown tag; yield to caller and return + # :invalid — malformed tag token; delegate to handle_invalid_tag_token + result = handle_tag_token(token, parse_context, tokenizer) + next unless result # nil: normal + next if result == :next # :next: 'liquid' + return yield(@_unknown_tag_name, parse_context.cursor.tag_markup) if result == :unknown + return handle_invalid_tag_token(token, parse_context, &block) # :invalid elsif second_byte == Cursor::LCURLY whitespace_handler(token, parse_context) @nodelist << create_variable(token, parse_context) @blank = false else # Fallback: text token starting with '{' - if parse_context.trim_whitespace - token.lstrip! - end - parse_context.trim_whitespace = false - @nodelist << token - @blank &&= BlockBody.blank_string?(token) + append_text_token(token, parse_context) end else - if parse_context.trim_whitespace - token.lstrip! - end - parse_context.trim_whitespace = false - @nodelist << token - @blank &&= BlockBody.blank_string?(token) + append_text_token(token, parse_context) end parse_context.line_number = tokenizer.line_number end @@ -192,8 +163,53 @@ module Liquid yield nil, nil end + # Handles a {%...%} tag token. Does not receive the outer block — callers handle + # yield/block passing themselves, keeping the Proc off the hot path. + # Returns: + # nil — tag parsed, caller continues the loop + # :next — 'liquid' inline tag; caller skips line number update + # :unknown — unknown/end tag; @_unknown_tag_name holds the tag name; + # markup is in parse_context.cursor.tag_markup + # :invalid — malformed token; caller delegates to handle_invalid_tag_token + private def handle_tag_token(token, parse_context, tokenizer) + whitespace_handler(token, parse_context) + cursor = parse_context.cursor + tag_name = cursor.parse_tag_token(token) + return :invalid unless tag_name - def whitespace_handler(token, parse_context) + markup = cursor.tag_markup + if parse_context.line_number + newlines = cursor.tag_newlines + parse_context.line_number += newlines if newlines > 0 + end + + if tag_name == 'liquid' + parse_liquid_tag(markup, parse_context) + return :next + end + + tag = parse_context.environment.tag_for_name(tag_name) + unless tag + # end parsing if we reach an unknown tag; let the caller determine how to proceed + @_unknown_tag_name = tag_name + return :unknown + end + + new_tag = tag.parse(tag_name, markup, tokenizer, parse_context) + @blank &&= new_tag.blank? + @nodelist << new_tag + nil + end + + def append_text_token(token, parse_context) + token.lstrip! if parse_context.trim_whitespace + parse_context.trim_whitespace = false + @nodelist << token + @blank &&= BlockBody.blank_string?(token) + end + private :append_text_token + + private def whitespace_handler(token, parse_context) if token.getbyte(2) == Cursor::DASH previous_token = @nodelist.last if previous_token.is_a?(String) @@ -236,7 +252,7 @@ module Liquid end def render_to_output_buffer(context, output) - freeze unless frozen? + freeze resource_limits = context.resource_limits resource_limits.increment_render_score(@nodelist.length) @@ -262,6 +278,7 @@ module Liquid private + # Indirection allows subclasses to intercept per-node rendering. def render_node(context, output, node) BlockBody.render_node(context, output, node) end diff --git a/lib/liquid/context.rb b/lib/liquid/context.rb index f982f2fa..76671909 100644 --- a/lib/liquid/context.rb +++ b/lib/liquid/context.rb @@ -77,7 +77,7 @@ module Liquid # Note that this does not register the filters with the main Template object. see Template.register_filter # for that def add_filters(filters) - filters = [filters].flatten.compact + filters = Array(filters).flatten.compact @filters += filters @strainer = nil end @@ -88,7 +88,7 @@ module Liquid # are there any not handled interrupts? def interrupt? - !@interrupts.frozen? && !@interrupts.empty? + !@interrupts.equal?(Const::EMPTY_ARRAY) && @interrupts.any? end # push an interrupt to the stack. this interrupt is considered not handled. @@ -114,15 +114,18 @@ module Liquid strainer.invoke(method, *args).to_liquid end - # Fast path for single-argument filter invocation (the most common case: - # {{ value | filter }}) — avoids *args splat allocation. - def invoke_single(method, input) - strainer.invoke_single(method, input).to_liquid - end - - # Fast path for two-argument filter invocation (e.g. {{ value | default: 'x' }}) - def invoke_two(method, input, arg1) - strainer.invoke_two(method, input, arg1).to_liquid + # Arity-specialized filter delegation — generated to match StrainerTemplate's specializations. + # The pattern (avoid *args splat) is the same for each arity; generating makes it explicit. + { + invoke_single: ['input'], + invoke_two: ['input', 'arg1'], + }.each do |method_name, params| + all_params = (["method"] + params).join(", ") + module_eval(<<~RUBY, __FILE__, __LINE__ + 1) + def #{method_name}(#{all_params}) + strainer.#{method_name}(#{all_params}).to_liquid + end + RUBY end # Push new local scope on the stack. use Context#stack instead @@ -200,7 +203,7 @@ module Liquid end def key?(key) - find_variable(key, raise_on_not_found: false) != nil + !find_variable(key, raise_on_not_found: false).nil? end def evaluate(object) @@ -218,10 +221,10 @@ module Liquid variable = try_variable_find_in_environments(key, raise_on_not_found: raise_on_not_found) else # Multiple scopes — search through all of them - index = @scopes.find_index { |s| s.key?(key) } + scope = @scopes.find { |s| s.key?(key) } - variable = if index - lookup_and_evaluate(@scopes[index], key, raise_on_not_found: raise_on_not_found) + variable = if scope + lookup_and_evaluate(scope, key, raise_on_not_found: raise_on_not_found) else try_variable_find_in_environments(key, raise_on_not_found: raise_on_not_found) end @@ -230,9 +233,7 @@ module Liquid # update variable's context before invoking #to_liquid # Fast path: primitive types don't need context= or to_liquid conversion case variable - when String, Integer, Float, NilClass, TrueClass, FalseClass - return variable - when Array, Hash, Time + when String, Integer, Float, NilClass, TrueClass, FalseClass, Array, Hash, Time return variable end @@ -286,17 +287,16 @@ module Liquid attr_reader :base_scope_depth def try_variable_find_in_environments(key, raise_on_not_found:) - @environments.each do |environment| + found = find_in_envs(@environments, key, raise_on_not_found: raise_on_not_found) + return found unless found.nil? && !(@strict_variables && raise_on_not_found) + + find_in_envs(@static_environments, key, raise_on_not_found: raise_on_not_found) + end + + def find_in_envs(envs, key, raise_on_not_found:) + envs.each do |environment| found_variable = lookup_and_evaluate(environment, key, raise_on_not_found: raise_on_not_found) - if !found_variable.nil? || @strict_variables && raise_on_not_found - return found_variable - end - end - @static_environments.each do |environment| - found_variable = lookup_and_evaluate(environment, key, raise_on_not_found: raise_on_not_found) - if !found_variable.nil? || @strict_variables && raise_on_not_found - return found_variable - end + return found_variable if !found_variable.nil? || (@strict_variables && raise_on_not_found) end nil end diff --git a/lib/liquid/strainer_template.rb b/lib/liquid/strainer_template.rb index d01c1381..67160d4c 100644 --- a/lib/liquid/strainer_template.rb +++ b/lib/liquid/strainer_template.rb @@ -59,31 +59,30 @@ module Liquid raise Liquid::ArgumentError, e.message, e.backtrace end - # Fast path for single-argument (no extra args) filter invocation. - # Avoids *args splat allocation for the common {{ value | filter }} case. - def invoke_single(method, input) - if self.class.invokable?(method) - send(method, input) - elsif @context.strict_filters - raise Liquid::UndefinedFilter, "undefined filter #{method}" - else - input - end - rescue ::ArgumentError => e - raise Liquid::ArgumentError, e.message, e.backtrace - end - - # Fast path for two-argument filter invocation (input + one arg). - def invoke_two(method, input, arg1) - if self.class.invokable?(method) - send(method, input, arg1) - elsif @context.strict_filters - raise Liquid::UndefinedFilter, "undefined filter #{method}" - else - input - end - rescue ::ArgumentError => e - raise Liquid::ArgumentError, e.message, e.backtrace + # Arity-specialized filter invocation. + # Avoids *args splat allocation for the common 0-arg and 1-arg cases. + # `invoke` (general case) still uses *args for 2+ extra arguments. + { + invoke_single: ['input'], + invoke_two: ['input', 'arg1'], + }.each do |method_name, params| + all_params = (["method"] + params).join(", ") + send_params = params.join(", ") + # __LINE__ + 1 is a parse-time constant; both generated methods will report + # the same file:line in backtraces. The method name in the trace distinguishes them. + module_eval(<<~RUBY, __FILE__, __LINE__ + 1) + def #{method_name}(#{all_params}) + if self.class.invokable?(method) + send(method, #{send_params}) + elsif @context.strict_filters + raise Liquid::UndefinedFilter, "undefined filter \#{method}" + else + input + end + rescue ::ArgumentError => e + raise Liquid::ArgumentError, e.message, e.backtrace + end + RUBY end end end diff --git a/lib/liquid/variable.rb b/lib/liquid/variable.rb index 34faa5fb..22615db1 100644 --- a/lib/liquid/variable.rb +++ b/lib/liquid/variable.rb @@ -14,66 +14,24 @@ module Liquid class Variable # Checks if markup is a simple "name.lookup.chain" with no filters/brackets/quotes. # Returns the trimmed markup string, or nil if not simple. - # Avoids regex MatchData allocation. def self.simple_variable_markup(markup) - len = markup.bytesize - return if len == 0 - - # Skip leading whitespace - pos = 0 - while pos < len - b = markup.getbyte(pos) - break unless b == 32 || b == 9 || b == 10 || b == 13 - pos += 1 - end - return if pos >= len - - start = pos - - # First char must be [a-zA-Z_] - b = markup.getbyte(pos) - return unless (b >= 97 && b <= 122) || (b >= 65 && b <= 90) || b == 95 - pos += 1 - - # Scan segments: [\w-]* (. [\w-]*)* - while pos < len - b = markup.getbyte(pos) - if (b >= 97 && b <= 122) || (b >= 65 && b <= 90) || (b >= 48 && b <= 57) || b == 95 || b == 45 - pos += 1 - elsif b == 46 # '.' - pos += 1 - # After dot, must have [a-zA-Z_] - return if pos >= len - b = markup.getbyte(pos) - return unless (b >= 97 && b <= 122) || (b >= 65 && b <= 90) || b == 95 - pos += 1 - else - break - end - end - - content_end = pos - - # Skip trailing whitespace - while pos < len - b = markup.getbyte(pos) - return unless b == 32 || b == 9 || b == 10 || b == 13 - pos += 1 - end - - # Must have consumed everything - return unless pos == len - - if start == 0 && content_end == len - markup - else - markup.byteslice(start, content_end - start) - end + return if markup.empty? + return unless markup.match?(SIMPLE_VARIABLE_RE) + # Avoid allocation when there's no surrounding whitespace (the common case) + first = markup.getbyte(0) + last = markup.getbyte(markup.bytesize - 1) + needs_strip = first == Cursor::SPACE || first == Cursor::TAB || first == Cursor::NL || first == Cursor::CR || + last == Cursor::SPACE || last == Cursor::TAB || last == Cursor::NL || last == Cursor::CR + needs_strip ? markup.strip : markup end # Cache for [filtername, EMPTY_ARRAY] tuples — avoids repeated array creation NO_ARG_FILTER_CACHE = Hash.new { |h, k| h[k] = [k, Const::EMPTY_ARRAY].freeze } + # Regex for a simple variable lookup with optional surrounding whitespace. + # Shares the identifier grammar with VariableLookup::SIMPLE_LOOKUP_RE. + SIMPLE_VARIABLE_RE = /\A\s*[\w-]+\??(?:\.[\w-]+\??)*\s*\z/ + FilterMarkupRegex = /#{FilterSeparator}\s*(.*)/om FilterParser = /(?:\s+|#{QuotedFragment}|#{ArgumentSeparator})+/o FilterArgsRegex = /(?:#{FilterArgumentSeparator}|#{ArgumentSeparator})\s*((?:\w+\s*\:\s*)?#{QuotedFragment})/o @@ -102,6 +60,34 @@ module Liquid end private def try_fast_parse(markup, parse_context) + pos = fast_scan_name(markup) + return false unless pos + + # fast_resolve_name calls VariableLookup.parse_simple / Expression::LITERALS — the + # only sites that can raise SyntaxError on malformed input. The byte scanners return + # false instead of raising. + begin + fast_resolve_name(markup, parse_context) + rescue SyntaxError + return false + end + + # End of markup — no filters + if pos >= markup.bytesize + @filters = Const::EMPTY_ARRAY + return true + end + + # Must be followed by a pipe filter separator + return false unless markup.getbyte(pos) == Cursor::PIPE + + fast_scan_filters(markup, pos, parse_context) + end + + # Scan the variable name (quoted string or identifier chain) at the start of markup. + # Returns the position after the name + trailing whitespace, or false on failure. + # Sets @_fast_name_start and @_fast_name_end for fast_resolve_name. + private def fast_scan_name(markup) len = markup.bytesize return false if len == 0 @@ -109,40 +95,40 @@ module Liquid pos = 0 while pos < len b = markup.getbyte(pos) - break unless b == 32 || b == 9 || b == 10 || b == 13 + break unless b == Cursor::SPACE || b == Cursor::TAB || b == Cursor::NL || b == Cursor::CR pos += 1 end return false if pos >= len b = markup.getbyte(pos) - if b == 39 || b == 34 # single or double quote + if b == Cursor::QUOTE_S || b == Cursor::QUOTE_D # Quoted string literal: scan to matching close quote quote = b - name_start = pos + @_fast_name_start = pos pos += 1 pos += 1 while pos < len && markup.getbyte(pos) != quote pos += 1 if pos < len # skip closing quote - name_end = pos - elsif (b >= 97 && b <= 122) || (b >= 65 && b <= 90) || b == 95 - # Identifier: scan [\w-]*(\.[\w-]*)* - name_start = pos + @_fast_name_end = pos + elsif ByteTables::IDENT_START[b] + # Identifier chain: [a-zA-Z_][a-zA-Z0-9_-]*(.[a-zA-Z_][a-zA-Z0-9_-]*)* + @_fast_name_start = pos pos += 1 while pos < len b = markup.getbyte(pos) - if (b >= 97 && b <= 122) || (b >= 65 && b <= 90) || (b >= 48 && b <= 57) || b == 95 || b == 45 + if ByteTables::IDENT_CONT[b] pos += 1 - elsif b == 46 # '.' + elsif b == Cursor::DOT pos += 1 return false if pos >= len b = markup.getbyte(pos) - return false unless (b >= 97 && b <= 122) || (b >= 65 && b <= 90) || b == 95 + return false unless ByteTables::IDENT_START[b] pos += 1 else break end end - name_end = pos + @_fast_name_end = pos else return false end @@ -150,22 +136,30 @@ module Liquid # Skip whitespace after name while pos < len b = markup.getbyte(pos) - break unless b == 32 || b == 9 || b == 10 || b == 13 + break unless b == Cursor::SPACE || b == Cursor::TAB || b == Cursor::NL || b == Cursor::CR pos += 1 end - # Resolve the name expression — avoid byteslice when markup is already the name - expr_markup = if name_start == 0 && name_end == len - markup # no whitespace, no filters — reuse the string - else - markup.byteslice(name_start, name_end - name_start) - end + pos + end + + # Resolve the scanned name bytes to a Liquid expression object. + # Reads @_fast_name_start / @_fast_name_end set by fast_scan_name. + # Sets @name. May raise SyntaxError (rescued in try_fast_parse). + private def fast_resolve_name(markup, parse_context) + name_start = @_fast_name_start + name_end = @_fast_name_end + len = markup.bytesize + + # Avoid byteslice when the name spans the whole markup (no surrounding whitespace/filters) + expr_markup = name_start == 0 && name_end == len ? markup : markup.byteslice(name_start, name_end - name_start) + cache = parse_context.expression_cache - ss = parse_context.string_scanner + ss = parse_context.string_scanner first_byte = expr_markup.getbyte(0) - @name = if first_byte == 39 || first_byte == 34 # quoted string - # Strip quotes for string literal + @name = if first_byte == Cursor::QUOTE_S || first_byte == Cursor::QUOTE_D + # String literal — strip enclosing quotes expr_markup.byteslice(1, expr_markup.bytesize - 2) elsif Expression::LITERALS.key?(expr_markup) Expression::LITERALS[expr_markup] @@ -174,145 +168,162 @@ module Liquid else VariableLookup.parse_simple(expr_markup, ss || StringScanner.new(""), nil).freeze end + end - # End of markup? No filters. - if pos >= len - @filters = Const::EMPTY_ARRAY - return true - end - - # Must be a pipe for filters - return false unless markup.getbyte(pos) == 124 # '|' - - # Try fast filter scanning first — handles no-arg and simple-arg filters - # Falls through to Lexer-based parsing for complex cases + # Scan the filter chain starting at `pos` (the first '|'). + # Returns true on success (sets @filters), false to fall back to the Lexer. + # Rescues SyntaxError from Expression.parse inside fast_scan_filter_args. + private def fast_scan_filters(markup, pos, parse_context) + len = markup.bytesize @filters = [] filter_pos = pos - while filter_pos < len && markup.getbyte(filter_pos) == 124 # '|' + while filter_pos < len && markup.getbyte(filter_pos) == Cursor::PIPE filter_pos += 1 - # Skip whitespace - filter_pos += 1 while filter_pos < len && markup.getbyte(filter_pos) == 32 + # Skip spaces after pipe (tabs/newlines handled in the between-filters skip below) + filter_pos += 1 while filter_pos < len && markup.getbyte(filter_pos) == Cursor::SPACE - # Scan filter name + # Scan filter name: must start with [a-zA-Z_] fname_start = filter_pos b = filter_pos < len ? markup.getbyte(filter_pos) : nil - break unless b && ((b >= 97 && b <= 122) || (b >= 65 && b <= 90) || b == 95) + break unless b && ByteTables::IDENT_START[b] filter_pos += 1 while filter_pos < len b = markup.getbyte(filter_pos) - break unless (b >= 97 && b <= 122) || (b >= 65 && b <= 90) || (b >= 48 && b <= 57) || b == 95 || b == 45 + break unless ByteTables::IDENT_CONT[b] filter_pos += 1 end filtername = markup.byteslice(fname_start, filter_pos - fname_start) - # Skip whitespace - filter_pos += 1 while filter_pos < len && markup.getbyte(filter_pos) == 32 + # Skip whitespace after filter name + filter_pos += 1 while filter_pos < len && markup.getbyte(filter_pos) == Cursor::SPACE - # Has arguments — try fast scanning for positional args - if filter_pos < len && markup.getbyte(filter_pos) == 58 # ':' + if filter_pos < len && markup.getbyte(filter_pos) == Cursor::COLON + # Has arguments — fast-scan positional args; fall to Lexer on keyword args filter_pos += 1 # skip ':' - filter_pos += 1 while filter_pos < len && markup.getbyte(filter_pos) == 32 + filter_pos += 1 while filter_pos < len && markup.getbyte(filter_pos) == Cursor::SPACE - filter_args = [] - fall_to_lexer = false - - loop do - arg_start = filter_pos - b = filter_pos < len ? markup.getbyte(filter_pos) : nil - - if b == 39 || b == 34 # quoted string - quote = b - filter_pos += 1 - filter_pos += 1 while filter_pos < len && markup.getbyte(filter_pos) != quote - filter_pos += 1 if filter_pos < len # skip closing quote - filter_args << markup.byteslice(arg_start + 1, filter_pos - arg_start - 2) - elsif b && ((b >= 48 && b <= 57) || (b == 45 && filter_pos + 1 < len && markup.getbyte(filter_pos + 1) >= 48 && markup.getbyte(filter_pos + 1) <= 57)) - # Number - filter_pos += 1 if b == 45 - filter_pos += 1 while filter_pos < len && markup.getbyte(filter_pos) >= 48 && markup.getbyte(filter_pos) <= 57 - if filter_pos < len && markup.getbyte(filter_pos) == 46 # float - filter_pos += 1 - filter_pos += 1 while filter_pos < len && markup.getbyte(filter_pos) >= 48 && markup.getbyte(filter_pos) <= 57 - end - num_str = markup.byteslice(arg_start, filter_pos - arg_start) - filter_args << (num_str.include?('.') ? num_str.to_f : num_str.to_i) - elsif b && ((b >= 97 && b <= 122) || (b >= 65 && b <= 90) || b == 95) - # Identifier - id_start = filter_pos - filter_pos += 1 - while filter_pos < len - b2 = markup.getbyte(filter_pos) - break unless (b2 >= 97 && b2 <= 122) || (b2 >= 65 && b2 <= 90) || (b2 >= 48 && b2 <= 57) || b2 == 95 || b2 == 45 || b2 == 46 - filter_pos += 1 - end - filter_pos += 1 if filter_pos < len && markup.getbyte(filter_pos) == 63 - - # Check if keyword arg (id followed by ':') - kw_check = filter_pos - kw_check += 1 while kw_check < len && markup.getbyte(kw_check) == 32 - if kw_check < len && markup.getbyte(kw_check) == 58 - fall_to_lexer = true - break - end - - id_markup = markup.byteslice(id_start, filter_pos - id_start) - filter_args << Expression.parse(id_markup, parse_context.string_scanner, parse_context.expression_cache) - else - fall_to_lexer = true - break - end - - # Skip whitespace after arg - filter_pos += 1 while filter_pos < len && markup.getbyte(filter_pos) == 32 - - # Comma = more args; pipe/end = done - if filter_pos < len && markup.getbyte(filter_pos) == 44 - filter_pos += 1 - filter_pos += 1 while filter_pos < len && markup.getbyte(filter_pos) == 32 - else - break - end - end - - if fall_to_lexer - # Complex filter — fall to Lexer for this and remaining filters - rest_start = fname_start - rest_start -= 1 while rest_start > pos && markup.getbyte(rest_start) != 124 - rest_markup = markup.byteslice(rest_start, len - rest_start) - p = parse_context.new_parser(rest_markup) - while p.consume?(:pipe) - fn = p.consume(:id) - fa = p.consume?(:colon) ? parse_filterargs(p) : Const::EMPTY_ARRAY - @filters << lax_parse_filter_expressions(fn, fa) - end - p.consume(:end_of_string) - @filters = Const::EMPTY_ARRAY if @filters.empty? - return true - end + result = fast_scan_filter_args(markup, filter_pos, parse_context) + return fall_to_lexer_filters(markup, pos, fname_start, len, parse_context) if result == :fall_to_lexer + filter_args, filter_pos = result @filters << [filtername, filter_args] else - # No args — add as simple filter + # No-arg filter — reuse the cached [name, EMPTY_ARRAY] tuple @filters << NO_ARG_FILTER_CACHE[filtername] end - # Skip whitespace between filters - filter_pos += 1 while filter_pos < len && (markup.getbyte(filter_pos) == 32 || markup.getbyte(filter_pos) == 9 || markup.getbyte(filter_pos) == 10 || markup.getbyte(filter_pos) == 13) + # Skip whitespace (including tabs and newlines) between filters + filter_pos += 1 while filter_pos < len && ( + markup.getbyte(filter_pos) == Cursor::SPACE || + markup.getbyte(filter_pos) == Cursor::TAB || + markup.getbyte(filter_pos) == Cursor::NL || + markup.getbyte(filter_pos) == Cursor::CR + ) end - # Must have consumed everything + # Trailing bytes that aren't a pipe mean something the fast path doesn't handle return false if filter_pos < len @filters = Const::EMPTY_ARRAY if @filters.empty? true rescue SyntaxError - # If fast parse fails, fall back to full parse + # Expression.parse (called inside fast_scan_filter_args for identifier args) can + # raise SyntaxError on malformed input. Fall back to full Lexer parse. @name = nil @filters = nil false end + # Called when fast_scan_filter_args encounters keyword args or an unrecognised + # token. Hands the remaining filter chain (from the pipe before fname_start) + # to the full Lexer-based parser, merges results into @filters, and returns true. + private def fall_to_lexer_filters(markup, pos, fname_start, len, parse_context) + # Walk back from fname_start to find the pipe that opened this filter. + # Equivalent to: markup.rindex('|', fname_start), bounded by pos. + rest_start = fname_start + rest_start -= 1 while rest_start > pos && markup.getbyte(rest_start) != Cursor::PIPE + rest_markup = markup.byteslice(rest_start, len - rest_start) + p = parse_context.new_parser(rest_markup) + while p.consume?(:pipe) + fn = p.consume(:id) + fa = p.consume?(:colon) ? parse_filterargs(p) : Const::EMPTY_ARRAY + @filters << lax_parse_filter_expressions(fn, fa) + end + p.consume(:end_of_string) + @filters = Const::EMPTY_ARRAY if @filters.empty? + true + end + + # Scan positional filter arguments starting at `filter_pos`. + # Returns [filter_args_array, new_filter_pos] on success, or :fall_to_lexer when + # keyword args or unrecognised tokens are encountered. + private def fast_scan_filter_args(markup, filter_pos, parse_context) + len = markup.bytesize + filter_args = [] + + loop do + arg_start = filter_pos + b = filter_pos < len ? markup.getbyte(filter_pos) : nil + + if b == Cursor::QUOTE_S || b == Cursor::QUOTE_D + # Quoted string argument + quote = b + filter_pos += 1 + filter_pos += 1 while filter_pos < len && markup.getbyte(filter_pos) != quote + filter_pos += 1 if filter_pos < len # skip closing quote + filter_args << markup.byteslice(arg_start + 1, filter_pos - arg_start - 2) + + elsif b && (ByteTables::DIGIT[b] || + (b == Cursor::DASH && filter_pos + 1 < len && ByteTables::DIGIT[markup.getbyte(filter_pos + 1)])) + # Numeric argument (integer or float, optionally negative) + filter_pos += 1 if b == Cursor::DASH + filter_pos += 1 while filter_pos < len && ByteTables::DIGIT[markup.getbyte(filter_pos)] + if filter_pos < len && markup.getbyte(filter_pos) == Cursor::DOT # float + filter_pos += 1 + filter_pos += 1 while filter_pos < len && ByteTables::DIGIT[markup.getbyte(filter_pos)] + end + num_str = markup.byteslice(arg_start, filter_pos - arg_start) + filter_args << (num_str.include?('.') ? num_str.to_f : num_str.to_i) + + elsif b && ByteTables::IDENT_START[b] + # Identifier argument — may be a variable lookup or keyword arg + id_start = filter_pos + filter_pos += 1 + while filter_pos < len + b2 = markup.getbyte(filter_pos) + break unless ByteTables::IDENT_CONT[b2] || b2 == Cursor::DOT + filter_pos += 1 + end + filter_pos += 1 if filter_pos < len && markup.getbyte(filter_pos) == Cursor::QMARK + + # Peek past whitespace: if followed by ':', this is a keyword arg → fall to Lexer + kw_check = filter_pos + kw_check += 1 while kw_check < len && markup.getbyte(kw_check) == Cursor::SPACE + return :fall_to_lexer if kw_check < len && markup.getbyte(kw_check) == Cursor::COLON + + id_markup = markup.byteslice(id_start, filter_pos - id_start) + filter_args << Expression.parse(id_markup, parse_context.string_scanner, parse_context.expression_cache) + + else + return :fall_to_lexer + end + + # Skip whitespace after argument + filter_pos += 1 while filter_pos < len && markup.getbyte(filter_pos) == Cursor::SPACE + + # Comma: more arguments follow; anything else: done with this filter's args + if filter_pos < len && markup.getbyte(filter_pos) == Cursor::COMMA + filter_pos += 1 + filter_pos += 1 while filter_pos < len && markup.getbyte(filter_pos) == Cursor::SPACE + else + break + end + end + + [filter_args, filter_pos] + end + def raw @markup end diff --git a/lib/liquid/variable_lookup.rb b/lib/liquid/variable_lookup.rb index 3f9b6cf3..3fca7b84 100644 --- a/lib/liquid/variable_lookup.rb +++ b/lib/liquid/variable_lookup.rb @@ -33,9 +33,9 @@ module Liquid pos += 1 while pos < len && depth > 0 b = markup.getbyte(pos) - if b == 91 + if b == 91 # rubocop:disable Metrics/BlockNesting depth += 1 - elsif b == 93 + elsif b == 93 # rubocop:disable Metrics/BlockNesting depth -= 1 end pos += 1 @@ -114,7 +114,7 @@ module Liquid lookups = self.class.scan_variable(markup) name = lookups.shift - if name&.start_with?('[') && name&.end_with?(']') + if name&.start_with?('[') && name.end_with?(']') name = Expression.parse( name[1..-2], string_scanner, @@ -126,9 +126,8 @@ module Liquid @lookups = lookups @command_flags = 0 - @lookups.each_index do |i| - lookup = lookups[i] - if lookup&.start_with?('[') && lookup&.end_with?(']') + @lookups.each_with_index do |lookup, i| + if lookup&.start_with?('[') && lookup.end_with?(']') lookups[i] = Expression.parse( lookup[1..-2], string_scanner, @@ -160,18 +159,13 @@ module Liquid # If object is a hash- or array-like object we look for the # presence of the key and if its available we return it - if object.instance_of?(Hash) ? object.key?(key) : - (object.respond_to?(:[]) && - ((object.respond_to?(:key?) && object.key?(key)) || - (object.respond_to?(:fetch) && key.is_a?(Integer)))) - + if accessible?(object, key) # if its a proc we will replace the entry with the proc object = context.lookup_and_evaluate(object, key) # Skip to_liquid for common primitive types (they return self) unless object.instance_of?(String) || object.instance_of?(Integer) || object.instance_of?(Float) || object.instance_of?(Array) || object.instance_of?(Hash) || object.nil? - object = object.to_liquid - object.context = context if object.respond_to?(:context=) + object = liquidize(object, context) end # Some special cases. If the part wasn't in square brackets and @@ -180,8 +174,7 @@ module Liquid elsif lookup_command?(i) && object.respond_to?(key) object = object.send(key) unless object.instance_of?(String) || object.instance_of?(Integer) || object.instance_of?(Array) || object.nil? - object = object.to_liquid - object.context = context if object.respond_to?(:context=) + object = liquidize(object, context) end # Handle string first/last like ActiveSupport does (returns first/last character) @@ -205,6 +198,27 @@ module Liquid self.class == other.class && state == other.state end + private + + # Returns true if +object+ has +key+ accessible via [] lookup. + def accessible?(object, key) + if object.instance_of?(Hash) + object.key?(key) + else + object.respond_to?(:[]) && + ((object.respond_to?(:key?) && object.key?(key)) || + (object.respond_to?(:fetch) && key.is_a?(Integer))) + end + end + + # Calls to_liquid on +object+ and wires up the context reference if needed. + # Skipped for primitive types that return self from to_liquid. + def liquidize(object, context) + object = object.to_liquid + object.context = context if object.respond_to?(:context=) + object + end + protected def state diff --git a/test/unit/variable_fast_parse_test.rb b/test/unit/variable_fast_parse_test.rb new file mode 100644 index 00000000..fd67c5ea --- /dev/null +++ b/test/unit/variable_fast_parse_test.rb @@ -0,0 +1,126 @@ +# frozen_string_literal: true + +require 'test_helper' + +# Tests that the fast-path parser (try_fast_parse) produces the same result as the +# full Lexer → Parser pipeline for every input we expect it to handle. +# +# This protects against silent regressions where a change to try_fast_parse causes it +# to produce different output from the slow path (the existing test suite would still +# pass because the slow path catches it, but correctness would be silently lost). +class VariableFastParseTest < Minitest::Test + include Liquid + + EQUIVALENCE_CASES = [ + # Simple lookups + "product", + "product.title", + "product.variants.first.title", + # Quoted string literals + "'hello'", + '"hello"', + # Variables with no-arg filters + "product | upcase", + "product | upcase | downcase", + "product | strip | upcase | downcase", + # Variables with single-arg filters + "product | truncate: 50", + "product | plus: 1", + "product | plus: -3", + "product | round: 2", + "product | append: ' world'", + # Variables with multi-arg filters + "product | replace: 'a', 'b'", + "product | pluralize: 'item', 'items'", + "product | slice: 0, 5", + # Chained mixed filters + "product.title | truncate: 50", + "'hello' | append: ' world' | upcase", + "name | prepend: 'Dr. ' | append: ' PhD' | upcase", + # Numeric args + "count | plus: 1.5", + "price | minus: 0.99", + # No whitespace around pipe + "x|upcase", + "x|replace:'a','b'|upcase", + # Leading/trailing whitespace + " product ", + " product.title | upcase ", + ].freeze + + EQUIVALENCE_CASES.each_with_index do |markup, i| + define_method(:"test_fast_parse_equivalence_#{i.to_s.rjust(2, "0")}") do + lax_ctx = Liquid::ParseContext.new(error_mode: :lax) + strict_ctx = Liquid::ParseContext.new(error_mode: :strict) + + lax_var = Liquid::Variable.new(markup, lax_ctx) + strict_var = Liquid::Variable.new(markup, strict_ctx) + + assert_equal strict_var.name, + lax_var.name, + "Name mismatch for #{markup.inspect}: " \ + "lax=#{lax_var.name.inspect} strict=#{strict_var.name.inspect}" + assert_equal strict_var.filters.length, + lax_var.filters.length, + "Filter count mismatch for #{markup.inspect}: " \ + "lax=#{lax_var.filters.inspect} strict=#{strict_var.filters.inspect}" + strict_var.filters.each_with_index do |(s_name, *), i| + l_name = lax_var.filters[i][0] + assert_equal s_name, + l_name, + "Filter name mismatch at index #{i} for #{markup.inspect}" + end + end + end + + # Verify the fast path is actually taken for simple variables (i.e. filters is the + # shared frozen EMPTY_ARRAY, not a newly allocated array). + def test_fast_path_taken_for_simple_variable + ctx = Liquid::ParseContext.new(error_mode: :lax) + var = Liquid::Variable.new("product.title", ctx) + assert_same( + Liquid::Const::EMPTY_ARRAY, + var.filters, + "Expected fast path (frozen EMPTY_ARRAY) for simple variable", + ) + end + + def test_fast_path_taken_for_no_arg_filter + ctx = Liquid::ParseContext.new(error_mode: :lax) + var = Liquid::Variable.new("product | upcase", ctx) + assert_equal(1, var.filters.length) + assert_equal("upcase", var.filters[0][0]) + # The no-arg filter tuple should come from NO_ARG_FILTER_CACHE (frozen) + assert_predicate(var.filters[0], :frozen?) + end + + def test_fast_path_taken_for_single_arg_filter + ctx = Liquid::ParseContext.new(error_mode: :lax) + var = Liquid::Variable.new("product | truncate: 50", ctx) + assert_equal(1, var.filters.length) + assert_equal("truncate", var.filters[0][0]) + assert_equal([50], var.filters[0][1]) + end + + # Keyword args must fall through to the Lexer — verify the result is still correct. + def test_keyword_arg_falls_to_lexer_and_parses_correctly + ctx = Liquid::ParseContext.new(error_mode: :lax) + var = Liquid::Variable.new("img | img_tag: class: 'hero'", ctx) + assert_equal(1, var.filters.length) + assert_equal("img_tag", var.filters[0][0]) + end + + # Numeric filter arguments: integers and floats + def test_numeric_filter_args + ctx = Liquid::ParseContext.new(error_mode: :lax) + + int_var = Liquid::Variable.new("price | plus: 3", ctx) + assert_equal([3], int_var.filters[0][1]) + + neg_var = Liquid::Variable.new("price | minus: -1", ctx) + assert_equal([-1], neg_var.filters[0][1]) + + float_var = Liquid::Variable.new("price | round: 2.5", ctx) + assert_equal([2.5], float_var.filters[0][1]) + end +end