From 33bac87a5cf4192ce478162849465cbde1f15bc5 Mon Sep 17 00:00:00 2001 From: Tobi Lutke Date: Thu, 1 Jan 2026 20:14:21 -0500 Subject: [PATCH] Address liquid-spec issues without ActiveSupport loaded Implement ActiveSupport-compatible behaviors internally so Liquid works correctly without ActiveSupport being loaded: 1. String first/last via property access (name.first, name.last) - VariableLookup now handles string[0] and string[-1] for first/last 2. String first/last via filters (name | first, name | last) - StandardFilters#first and #last now handle strings 3. blank?/empty? comparisons for types without these methods - Condition now implements liquid_blank? and liquid_empty? internally - blank? matches ActiveSupport: nil, false, empty/whitespace strings, empty arrays/hashes are all blank - empty? checks length == 0 only (whitespace is NOT empty) This fixes spec failures for templates like: - {{ name.first }} / {{ name | first }} on strings - {% if x == blank %} for whitespace strings, empty hashes/arrays - {% case ' ' %}{% when blank %} matching whitespace --- lib/liquid/condition.rb | 62 +++++++-- lib/liquid/standardfilters.rb | 2 + lib/liquid/variable_lookup.rb | 4 + test/integration/standard_filter_test.rb | 32 +++++ test/unit/condition_unit_test.rb | 156 +++++++++++++++++++++++ 5 files changed, 246 insertions(+), 10 deletions(-) diff --git a/lib/liquid/condition.rb b/lib/liquid/condition.rb index 9ab350f0..2acf5ae3 100644 --- a/lib/liquid/condition.rb +++ b/lib/liquid/condition.rb @@ -113,24 +113,66 @@ module Liquid def equal_variables(left, right) if left.is_a?(MethodLiteral) - if right.respond_to?(left.method_name) - return right.send(left.method_name) - else - return nil - end + return call_method_literal(left, right) end if right.is_a?(MethodLiteral) - if left.respond_to?(right.method_name) - return left.send(right.method_name) - else - return nil - end + return call_method_literal(right, left) end left == right end + def call_method_literal(literal, value) + method_name = literal.method_name + + # If the object responds to the method, use it + if value.respond_to?(method_name) + return value.send(method_name) + end + + # Implement blank?/empty? for common types that don't have it + # (ActiveSupport adds these, but Liquid should work without it) + case method_name + when :blank? + liquid_blank?(value) + when :empty? + liquid_empty?(value) + else + nil + end + end + + # Implement blank? semantics matching ActiveSupport + def liquid_blank?(value) + case value + when NilClass, FalseClass + true + when TrueClass, Numeric + false + when String + # Blank if empty or whitespace only + value.empty? || value.match?(/\A\s*\z/) + when Array, Hash + value.empty? + else + # Fall back to empty? if available, otherwise false + value.respond_to?(:empty?) ? value.empty? : false + end + end + + # Implement empty? semantics + def liquid_empty?(value) + case value + when NilClass + true + when String, Array, Hash + value.empty? + else + value.respond_to?(:empty?) ? value.empty? : false + end + end + def interpret_condition(left, right, op, context) # If the operator is empty this means that the decision statement is just # a single variable. We can just poll this variable from the context and diff --git a/lib/liquid/standardfilters.rb b/lib/liquid/standardfilters.rb index 6e072fcf..d17a699c 100644 --- a/lib/liquid/standardfilters.rb +++ b/lib/liquid/standardfilters.rb @@ -768,6 +768,7 @@ module Liquid # @liquid_syntax array | first # @liquid_return [untyped] def first(array) + return array[0] if array.is_a?(String) array.first if array.respond_to?(:first) end @@ -779,6 +780,7 @@ module Liquid # @liquid_syntax array | last # @liquid_return [untyped] def last(array) + return array[-1] if array.is_a?(String) array.last if array.respond_to?(:last) end diff --git a/lib/liquid/variable_lookup.rb b/lib/liquid/variable_lookup.rb index 340c0b66..bc9bbd97 100644 --- a/lib/liquid/variable_lookup.rb +++ b/lib/liquid/variable_lookup.rb @@ -70,6 +70,10 @@ module Liquid elsif lookup_command?(i) && object.respond_to?(key) object = object.send(key).to_liquid + # Handle string first/last like ActiveSupport does (returns first/last character) + elsif lookup_command?(i) && object.is_a?(String) && (key == "first" || key == "last") + object = key == "first" ? object[0] : object[-1] + # No key was present with the desired value and it wasn't one of the directly supported # keywords either. The only thing we got left is to return nil or # raise an exception if `strict_variables` option is set to true diff --git a/test/integration/standard_filter_test.rb b/test/integration/standard_filter_test.rb index e27605ff..eb95276c 100644 --- a/test/integration/standard_filter_test.rb +++ b/test/integration/standard_filter_test.rb @@ -627,6 +627,38 @@ class StandardFiltersTest < Minitest::Test assert_nil(@filters.last([])) end + def test_first_last_on_strings + # Ruby's String class does not have first/last methods by default. + # ActiveSupport adds String#first and String#last to return the first/last character. + # Liquid must work without ActiveSupport, so the first/last filters handle strings specially. + # + # This enables template patterns like: + # {{ product.title | first }} => "S" (for "Snowboard") + # {{ customer.name | last }} => "h" (for "Smith") + assert_equal('f', @filters.first('foo')) + assert_equal('o', @filters.last('foo')) + assert_nil(@filters.first('')) + assert_nil(@filters.last('')) + end + + def test_first_last_on_unicode_strings + # Unicode strings should return the first/last grapheme cluster (character), + # not the first/last byte. Ruby's String#[] handles this correctly with index 0/-1. + # This ensures international text works properly: + # {{ korean_name | first }} => "고" (not a partial byte sequence) + assert_equal('고', @filters.first('고스트빈')) + assert_equal('빈', @filters.last('고스트빈')) + end + + def test_first_last_on_strings_via_template + # Integration test to verify the filter works end-to-end in templates. + # Empty strings return empty output (nil renders as empty string). + assert_template_result('f', '{{ name | first }}', { 'name' => 'foo' }) + assert_template_result('o', '{{ name | last }}', { 'name' => 'foo' }) + assert_template_result('', '{{ name | first }}', { 'name' => '' }) + assert_template_result('', '{{ name | last }}', { 'name' => '' }) + end + def test_replace assert_equal('b b b b', @filters.replace('a a a a', 'a', 'b')) assert_equal('2 2 2 2', @filters.replace('1 1 1 1', 1, 2)) diff --git a/test/unit/condition_unit_test.rb b/test/unit/condition_unit_test.rb index eb466f7a..e250ce0f 100644 --- a/test/unit/condition_unit_test.rb +++ b/test/unit/condition_unit_test.rb @@ -197,6 +197,162 @@ class ConditionUnitTest < Minitest::Test assert_equal(['title'], result.lookups) end + # Tests for blank? comparison without ActiveSupport + # + # Ruby's standard library does not include blank? on String, Array, Hash, etc. + # ActiveSupport adds blank? but Liquid must work without it. These tests verify + # that Liquid implements blank? semantics internally for use in templates like: + # {% if x == blank %}...{% endif %} + # + # The blank? semantics match ActiveSupport's behavior: + # - nil and false are blank + # - Strings are blank if empty or contain only whitespace + # - Arrays and Hashes are blank if empty + # - true and numbers are never blank + + def test_blank_with_whitespace_string + # Template authors expect " " to be blank since it has no visible content. + # This matches ActiveSupport's String#blank? which returns true for whitespace-only strings. + @context['whitespace'] = ' ' + blank_literal = Condition.class_variable_get(:@@method_literals)['blank'] + + assert_evaluates_true(VariableLookup.new('whitespace'), '==', blank_literal) + end + + def test_blank_with_empty_string + # An empty string has no content, so it should be considered blank. + # This is the most basic case of a blank string. + @context['empty_string'] = '' + blank_literal = Condition.class_variable_get(:@@method_literals)['blank'] + + assert_evaluates_true(VariableLookup.new('empty_string'), '==', blank_literal) + end + + def test_blank_with_empty_array + # Empty arrays have no elements, so they are blank. + # Useful for checking if a collection has items: {% if products == blank %} + @context['empty_array'] = [] + blank_literal = Condition.class_variable_get(:@@method_literals)['blank'] + + assert_evaluates_true(VariableLookup.new('empty_array'), '==', blank_literal) + end + + def test_blank_with_empty_hash + # Empty hashes have no key-value pairs, so they are blank. + # Useful for checking if settings/options exist: {% if settings == blank %} + @context['empty_hash'] = {} + blank_literal = Condition.class_variable_get(:@@method_literals)['blank'] + + assert_evaluates_true(VariableLookup.new('empty_hash'), '==', blank_literal) + end + + def test_blank_with_nil + # nil represents "nothing" and is the canonical blank value. + # Unassigned variables resolve to nil, so this enables: {% if missing_var == blank %} + @context['nil_value'] = nil + blank_literal = Condition.class_variable_get(:@@method_literals)['blank'] + + assert_evaluates_true(VariableLookup.new('nil_value'), '==', blank_literal) + end + + def test_blank_with_false + # false is considered blank to match ActiveSupport semantics. + # This allows {% if some_flag == blank %} to work when flag is false. + @context['false_value'] = false + blank_literal = Condition.class_variable_get(:@@method_literals)['blank'] + + assert_evaluates_true(VariableLookup.new('false_value'), '==', blank_literal) + end + + def test_not_blank_with_true + # true is a definite value, not blank. + # Ensures {% if flag == blank %} works correctly for boolean flags. + @context['true_value'] = true + blank_literal = Condition.class_variable_get(:@@method_literals)['blank'] + + assert_evaluates_false(VariableLookup.new('true_value'), '==', blank_literal) + end + + def test_not_blank_with_number + # Numbers (including zero) are never blank - they represent actual values. + # 0 is a valid quantity, not the absence of a value. + @context['number'] = 42 + blank_literal = Condition.class_variable_get(:@@method_literals)['blank'] + + assert_evaluates_false(VariableLookup.new('number'), '==', blank_literal) + end + + def test_not_blank_with_string_content + # A string with actual content is not blank. + # This is the expected behavior for most template string comparisons. + @context['string'] = 'hello' + blank_literal = Condition.class_variable_get(:@@method_literals)['blank'] + + assert_evaluates_false(VariableLookup.new('string'), '==', blank_literal) + end + + def test_not_blank_with_non_empty_array + # An array with elements has content, so it's not blank. + # Enables patterns like {% unless products == blank %}Show products{% endunless %} + @context['array'] = [1, 2, 3] + blank_literal = Condition.class_variable_get(:@@method_literals)['blank'] + + assert_evaluates_false(VariableLookup.new('array'), '==', blank_literal) + end + + def test_not_blank_with_non_empty_hash + # A hash with key-value pairs has content, so it's not blank. + # Useful for checking if configuration exists: {% if config != blank %} + @context['hash'] = { 'a' => 1 } + blank_literal = Condition.class_variable_get(:@@method_literals)['blank'] + + assert_evaluates_false(VariableLookup.new('hash'), '==', blank_literal) + end + + # Tests for empty? comparison without ActiveSupport + # + # empty? is distinct from blank? - it only checks if a collection has zero elements. + # For strings, empty? checks length == 0, NOT whitespace content. + # Ruby's standard library has empty? on String, Array, and Hash, but Liquid + # provides a fallback implementation for consistency. + + def test_empty_with_empty_string + # An empty string ("") has length 0, so it's empty. + # Different from blank - empty is a stricter check. + @context['empty_string'] = '' + empty_literal = Condition.class_variable_get(:@@method_literals)['empty'] + + assert_evaluates_true(VariableLookup.new('empty_string'), '==', empty_literal) + end + + def test_empty_with_whitespace_string_not_empty + # Whitespace strings have length > 0, so they are NOT empty. + # This is the key difference between empty and blank: + # " ".empty? => false, but " ".blank? => true + @context['whitespace'] = ' ' + empty_literal = Condition.class_variable_get(:@@method_literals)['empty'] + + assert_evaluates_false(VariableLookup.new('whitespace'), '==', empty_literal) + end + + def test_empty_with_empty_array + # An array with no elements is empty. + # [].empty? => true + @context['empty_array'] = [] + empty_literal = Condition.class_variable_get(:@@method_literals)['empty'] + + assert_evaluates_true(VariableLookup.new('empty_array'), '==', empty_literal) + end + + def test_empty_with_empty_hash + # A hash with no key-value pairs is empty. + # {}.empty? => true + @context['empty_hash'] = {} + empty_literal = Condition.class_variable_get(:@@method_literals)['empty'] + + assert_evaluates_true(VariableLookup.new('empty_hash'), '==', empty_literal) + end + private def assert_evaluates_true(left, op, right)