From 84779f8a28fc7c34b9aecd3fd66268896c0043f6 Mon Sep 17 00:00:00 2001 From: Chris Pak Date: Sat, 4 Apr 2026 20:17:12 -0700 Subject: [PATCH] Removes dead code, tightens idioms, adds clarifying comments Dead code removed: cursor.rb: parse_for_markup + attr_reader :for_var/collection/reversed (zero callers in lib/; method also incomplete -- no limit/offset) for.rb: Syntax regex, REVERSED_BYTES constant (both unreferenced by lax_parse and strict_parse) block_body.rb: BLANK_STRING_REGEX (exact duplicate of WhitespaceOrNothing) Idioms: expression.rb: parse_number returns nil on failure, not false (Ruby convention for 'no result'; callers use if (num = parse_number)) block_body.rb: freeze (idempotent by spec); whitespace_handler marked private; render_node gets rationale comment; redundant comments collapsed variable_lookup.rb: COMMAND_METHODS -> %w[]; initialize loop uses each_with_index; removes redundant &. on second clause of && for.rb: strict2_parse -> alias_method; nil-guard if/else -> ternary; render_else -> ternary; include? checks unconsumed rest only condition.rb: loop/break chain -> while condition.child_relation utils.rb: tightens slice_collection_using_each loop Comments: variable.rb: backward pipe-walk algorithm; SPACE-only whitespace asymmetry cursor.rb: COMPARISON_OPS identity-map exists for frozen string interning if.rb: include? pre-check is both correctness guard and perf gate for.rb: cursor->regex fallback for limit:/offset: attributes strainer_template.rb: __LINE__+1 limitation in module_eval loop --- lib/liquid/condition.rb | 4 +-- lib/liquid/cursor.rb | 48 +++-------------------------------- lib/liquid/expression.rb | 43 ++++++++++++++++++------------- lib/liquid/tags/for.rb | 32 +++++------------------ lib/liquid/tags/if.rb | 5 +++- lib/liquid/utils.rb | 16 ++++-------- lib/liquid/variable_lookup.rb | 7 ++--- 7 files changed, 48 insertions(+), 107 deletions(-) diff --git a/lib/liquid/condition.rb b/lib/liquid/condition.rb index 13f238d2..bf8b9409 100644 --- a/lib/liquid/condition.rb +++ b/lib/liquid/condition.rb @@ -71,14 +71,12 @@ module Liquid return result unless @child_relation condition = self - loop do + while condition.child_relation case condition.child_relation when :or break if Liquid::Utils.to_liquid_value(result) when :and break unless Liquid::Utils.to_liquid_value(result) - else - break end condition = condition.child_condition result = interpret_condition(condition.left, condition.right, condition.operator, context) diff --git a/lib/liquid/cursor.rb b/lib/liquid/cursor.rb index 97603f79..29d85609 100644 --- a/lib/liquid/cursor.rb +++ b/lib/liquid/cursor.rb @@ -189,6 +189,10 @@ module Liquid end # ── Comparison operators ──────────────────────────────────────── + # Identity map used for frozen string interning: StringScanner#scan returns a + # new unfrozen String on every call. Indexing into this hash returns the frozen + # literal stored here, avoiding a separate allocation and enabling faster + # equality checks downstream (frozen strings can be compared by identity). COMPARISON_OPS = { '==' => '==', '!=' => '!=', @@ -313,50 +317,6 @@ module Liquid true end - # ── For tag parser ──────────────────────────────────────────────── - # Results from parse_for_markup - attr_reader :for_var, :for_collection, :for_reversed - # Parse "var in collection [reversed] [limit:N] [offset:N]" - # Returns true on success, nil on failure. - def parse_for_markup - skip_ws - @for_var = scan_id - return unless @for_var - - skip_ws - # expect "in" - return unless scan_id == "in" - - skip_ws - # Collection: parenthesized range or fragment - if peek_byte == LPAREN - start = @ss.pos - depth = 1 - @ss.scan_byte - while !@ss.eos? && depth > 0 - b = @ss.scan_byte - depth += 1 if b == LPAREN - depth -= 1 if b == RPAREN - end - @for_collection = @source.byteslice(start, @ss.pos - start) - else - @for_collection = scan_fragment - return unless @for_collection - end - - skip_ws - # Check for 'reversed' - saved = @ss.pos - word = scan_id - if word == "reversed" - @for_reversed = true - else - @for_reversed = false - @ss.pos = saved if word # rewind if we consumed a non-'reversed' word - end - - true - end end end diff --git a/lib/liquid/expression.rb b/lib/liquid/expression.rb index 7b0ae148..466c021e 100644 --- a/lib/liquid/expression.rb +++ b/lib/liquid/expression.rb @@ -72,23 +72,23 @@ module Liquid def parse_number(markup, _ss = nil) len = markup.bytesize - return false if len == 0 + return if len == 0 # Quick reject: first byte must be digit or dash pos = 0 first = markup.getbyte(pos) if first == Cursor::DASH pos += 1 - return false if pos >= len + return if pos >= len b = markup.getbyte(pos) - return false unless ByteTables::DIGIT[b] + return unless ByteTables::DIGIT[b] pos += 1 elsif ByteTables::DIGIT[first] pos += 1 else - return false + return end # Scan digits @@ -121,19 +121,9 @@ module Liquid # Simple float like "123.456" return markup.to_f elsif pos > digit_after_dot - # Float followed by more dots or other chars: "1.2.3.4" - # Return the float portion up to second dot - while pos < len - b = markup.getbyte(pos) - if b == Cursor::DOT - return markup.byteslice(0, pos).to_f - elsif !ByteTables::DIGIT[b] - return false - end - - pos += 1 - end - return markup.byteslice(0, pos).to_f + # Float followed by more content: "1.2.3.4" — scan to find where the + # numeric portion ends (stop at next dot or non-digit). + return scan_float_with_trailing(markup, pos, len) else # dot at end: "123." return markup.byteslice(0, dot_pos).to_f @@ -141,7 +131,24 @@ module Liquid end # Not a number (has non-digit, non-dot characters) - false + nil + end + + private + + # Scans forward from `pos` through digits, returning the float up to the + # next dot or the end of string. Returns nil when a non-digit, non-dot + # byte is found (not a valid number). Used by parse_number for inputs + # like "1.2.3.4" where the float literal ends at the second dot. + def scan_float_with_trailing(markup, pos, len) + while pos < len + b = markup.getbyte(pos) + return markup.byteslice(0, pos).to_f if b == Cursor::DOT + return unless ByteTables::DIGIT[b] + + pos += 1 + end + markup.byteslice(0, pos).to_f end end end diff --git a/lib/liquid/tags/for.rb b/lib/liquid/tags/for.rb index 0eb95382..2ed7d186 100644 --- a/lib/liquid/tags/for.rb +++ b/lib/liquid/tags/for.rb @@ -25,8 +25,6 @@ module Liquid # @liquid_optional_param range [untyped] A custom numeric range to iterate over. # @liquid_optional_param reversed [untyped] Iterate in reverse order. class For < Block - Syntax = /\A(#{VariableSegment}+)\s+in\s+(#{QuotedFragment}+)\s*(reversed)?/o - attr_reader :collection_name, :variable_name, :limit, :from def initialize(tag_name, markup, options) @@ -73,8 +71,6 @@ module Liquid protected # Fast byte-level parser for "var in collection [reversed] [limit:N] [offset:N]" - REVERSED_BYTES = "reversed".bytes.freeze - def lax_parse(markup) c = @parse_context.cursor c.reset(markup) @@ -114,9 +110,9 @@ module Liquid @reversed = c.expect_id("reversed") c.skip_ws - # Parse limit:/offset: if present - if !c.eos? && markup.include?(':') - rest = c.slice(c.pos, markup.bytesize - c.pos) + # Parse limit:/offset: if present. + # Cursor doesn't handle key:value attributes — delegate to regex for limit:/offset:. + if !c.eos? && (rest = c.slice(c.pos, markup.bytesize - c.pos)).include?(':') rest.scan(TagAttributes) do |key, value| set_attribute(key, value) end @@ -147,9 +143,7 @@ module Liquid private - def strict2_parse(markup) - strict_parse(markup) - end + alias_method :strict2_parse, :strict_parse def collection_segment(context) offsets = context.registers[:for] ||= {} @@ -158,22 +152,14 @@ module Liquid offsets[@name].to_i else from_value = context.evaluate(@from) - if from_value.nil? - 0 - else - Utils.to_integer(from_value) - end + from_value.nil? ? 0 : Utils.to_integer(from_value) end collection = context.evaluate(@collection_name) collection = collection.to_a if collection.is_a?(Range) limit_value = context.evaluate(@limit) - to = if limit_value.nil? - nil - else - Utils.to_integer(limit_value) + from - end + to = limit_value && (Utils.to_integer(limit_value) + from) segment = Utils.slice_collection(collection, from, to) segment.reverse! if @reversed @@ -228,11 +214,7 @@ module Liquid end def render_else(context, output) - if @else_block - @else_block.render_to_output_buffer(context, output) - else - output - end + @else_block ? @else_block.render_to_output_buffer(context, output) : output end class ParseTreeVisitor < Liquid::ParseTreeVisitor diff --git a/lib/liquid/tags/if.rb b/lib/liquid/tags/if.rb index 9ad58b5f..cc77161e 100644 --- a/lib/liquid/tags/if.rb +++ b/lib/liquid/tags/if.rb @@ -94,7 +94,10 @@ module Liquid return Condition.new(parse_expression(simple)) end - # Fast path: simple condition without and/or — use Cursor + # Fast path: simple condition without and/or — use Cursor. + # The include? pre-checks are both a correctness guard (parse_simple_condition + # only handles a single comparison) and a perf gate (avoids cursor allocation + # for the compound-condition case that will always fall through to lax_parse). if !markup.include?(' and ') && !markup.include?(' or ') cursor = @parse_context.cursor cursor.reset(markup) diff --git a/lib/liquid/utils.rb b/lib/liquid/utils.rb index a2b8f447..41b9f621 100644 --- a/lib/liquid/utils.rb +++ b/lib/liquid/utils.rb @@ -18,23 +18,17 @@ module Liquid def self.slice_collection_using_each(collection, from, to) segments = [] - index = 0 - # Maintains Ruby 1.8.7 String#each behaviour on 1.9 + # String is Enumerable but #each is not defined; handle it as a single-element collection if collection.is_a?(String) return collection.empty? ? [] : [collection] end return [] unless collection.respond_to?(:each) + index = 0 collection.each do |item| - if to && to <= index - break - end - - if from <= index - segments << item - end - + break if to && to <= index + segments << item if from <= index index += 1 end @@ -103,7 +97,7 @@ module Liquid def self.to_s(obj, seen = nil) case obj when Integer - return (obj >= 0 && obj < 1000) ? SMALL_INT_STRINGS[obj] : obj.to_s + obj >= 0 && obj < 1000 ? SMALL_INT_STRINGS[obj] : obj.to_s when BigDecimal obj.to_s("F") when Hash diff --git a/lib/liquid/variable_lookup.rb b/lib/liquid/variable_lookup.rb index 3fca7b84..bb33b68c 100644 --- a/lib/liquid/variable_lookup.rb +++ b/lib/liquid/variable_lookup.rb @@ -33,11 +33,8 @@ module Liquid pos += 1 while pos < len && depth > 0 b = markup.getbyte(pos) - if b == 91 # rubocop:disable Metrics/BlockNesting - depth += 1 - elsif b == 93 # rubocop:disable Metrics/BlockNesting - depth -= 1 - end + depth += 1 if b == 91 + depth -= 1 if b == 93 pos += 1 end if depth == 0