Merge pull request #2034 from Shopify/fix-liquid-spec-without-activesupport

tobi · web-flow · commit a4a29f3e0836 · 2026-01-01T22:32:38.000-05:00
Require liquid-spec to be run on commit automatically and related fixes
diff --git a/.github/workflows/liquid.yml b/.github/workflows/liquid.yml
@@ -9,26 +9,29 @@ jobs:
   test:
     runs-on: ubuntu-latest
     strategy:
+      fail-fast: false
       matrix:
         entry:
-          - { ruby: 3.0, allowed-failure: false } # minimum supported
-          - { ruby: 3.2, allowed-failure: false }
-          - { ruby: 3.3, allowed-failure: false }
-          - { ruby: 3.3, allowed-failure: false }
-          - { ruby: 3.4, allowed-failure: false } # latest
+          - { ruby: 3.3, allowed-failure: false } # minimum supported
+          - { ruby: 3.4, allowed-failure: false, rubyopt: "--yjit" }
+          - { ruby: 4.0, allowed-failure: false } # latest stable
           - {
-              ruby: 3.4,
+              ruby: 4.0,
               allowed-failure: false,
               rubyopt: "--enable-frozen-string-literal",
             }
-          - { ruby: 3.4, allowed-failure: false, rubyopt: "--yjit" }
-          - { ruby: head, allowed-failure: false }
+          - { ruby: 4.0, allowed-failure: false, rubyopt: "--yjit" }
+          - { ruby: 4.0, allowed-failure: false, rubyopt: "--zjit" }
+
+          # Head can have failures due to being in development
+          - { ruby: head, allowed-failure: true }
           - {
               ruby: head,
-              allowed-failure: false,
+              allowed-failure: true,
               rubyopt: "--enable-frozen-string-literal",
             }
-          - { ruby: head, allowed-failure: false, rubyopt: "--yjit" }
+          - { ruby: head, allowed-failure: true, rubyopt: "--yjit" }
+          - { ruby: head, allowed-failure: true, rubyopt: "--zjit" }
     name: Test Ruby ${{ matrix.entry.ruby }}
     steps:
       - uses: actions/checkout@f43a0e5ff2bd294095638e18286ca9a3d1956744 # v3.6.0
@@ -42,6 +45,23 @@ jobs:
         env:
           RUBYOPT: ${{ matrix.entry.rubyopt }}
 
+  spec:
+    runs-on: ubuntu-latest
+    env:
+      BUNDLE_WITH: spec
+    steps:
+      - uses: actions/checkout@f43a0e5ff2bd294095638e18286ca9a3d1956744 # v3.6.0
+      - uses: ruby/setup-ruby@a25f1e45f0e65a92fcb1e95e8847f78fb0a7197a # v1.273.0
+        with:
+          bundler-cache: true
+          bundler: latest
+      - name: Run liquid-spec for all adapters
+        run: |
+          for adapter in spec/*.rb; do
+            echo "=== Running $adapter ==="
+            bundle exec liquid-spec run "$adapter" --no-max-failures
+          done
+
   memory_profile:
     runs-on: ubuntu-latest
     steps:
diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
@@ -174,7 +174,16 @@ Style/WordArray:
 
 # Offense count: 117
 # This cop supports safe auto-correction (--auto-correct).
-# Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, AllowedPatterns, IgnoredPatterns.
+# Configuration parameters: AllowHeredoc, AllowURI, URISchemes, AllowCopDirectives, AllowedPatterns.
 # URISchemes: http, https
 Layout/LineLength:
   Max: 260
+
+Naming/PredicatePrefix:
+  Enabled: false
+
+# Offense count: 1
+# This is intentional - early return from begin/rescue in assignment context
+Lint/NoReturnInBeginEndBlocks:
+  Exclude:
+    - 'lib/liquid/standardfilters.rb'
diff --git a/Gemfile b/Gemfile
@@ -25,7 +25,14 @@ group :development do
 end
 
 group :test do
-  gem 'rubocop', '~> 1.61.0'
-  gem 'rubocop-shopify', '~> 2.12.0', require: false
+  gem 'benchmark'
+  gem 'rubocop', '~> 1.82.0'
+  gem 'rubocop-shopify', '~> 2.18.0', require: false
   gem 'rubocop-performance', require: false
 end
+
+group :spec do
+  # Using feature branch until https://github.com/Shopify/liquid-spec/pull/97 is merged
+  gem 'liquid-spec', github: 'Shopify/liquid-spec', branch: 'add-per-spec-required-features'
+  gem 'activesupport', require: false
+end
diff --git a/lib/liquid/condition.rb b/lib/liquid/condition.rb
@@ -113,24 +113,63 @@ def inspect
 
     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)
+      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
+    # Note: nil is NOT empty (but IS blank). empty? checks if a collection has zero elements.
+    def liquid_empty?(value)
+      case value
+      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
@@ -154,8 +193,8 @@ def interpret_condition(left, right, op, context)
     end
 
     def deprecated_default_context
-      warn("DEPRECATION WARNING: Condition#evaluate without a context argument is deprecated" \
-        " and will be removed from Liquid 6.0.0.")
+      warn("DEPRECATION WARNING: Condition#evaluate without a context argument is deprecated " \
+        "and will be removed from Liquid 6.0.0.")
       Context.new
     end
 
diff --git a/lib/liquid/drop.rb b/lib/liquid/drop.rb
@@ -31,7 +31,7 @@ def initialize
 
     # Catch all for the method
     def liquid_method_missing(method)
-      return nil unless @context&.strict_variables
+      return unless @context&.strict_variables
       raise Liquid::UndefinedDropMethod, "undefined method #{method}"
     end
 
diff --git a/lib/liquid/expression.rb b/lib/liquid/expression.rb
@@ -55,7 +55,7 @@ def parse(markup, ss = StringScanner.new(""), cache = nil)
       end
 
       def inner_parse(markup, ss, cache)
-        if (markup.start_with?("(") && markup.end_with?(")")) && markup =~ RANGES_REGEX
+        if markup.start_with?("(") && markup.end_with?(")") && markup =~ RANGES_REGEX
           return RangeLookup.parse(
             Regexp.last_match(1),
             Regexp.last_match(2),
diff --git a/lib/liquid/i18n.rb b/lib/liquid/i18n.rb
@@ -28,7 +28,7 @@ def locale
     def interpolate(name, vars)
       name.gsub(/%\{(\w+)\}/) do
         # raise TranslationError, "Undefined key #{$1} for interpolation in translation #{name}"  unless vars[$1.to_sym]
-        (vars[Regexp.last_match(1).to_sym]).to_s
+        vars[Regexp.last_match(1).to_sym].to_s
       end
     end
 
diff --git a/lib/liquid/standardfilters.rb b/lib/liquid/standardfilters.rb
@@ -768,6 +768,8 @@ def date(input, format)
     # @liquid_syntax array | first
     # @liquid_return [untyped]
     def first(array)
+      # ActiveSupport returns "" for empty strings, not nil
+      return array[0] || "" if array.is_a?(String)
       array.first if array.respond_to?(:first)
     end
 
@@ -779,6 +781,8 @@ def first(array)
     # @liquid_syntax array | last
     # @liquid_return [untyped]
     def last(array)
+      # ActiveSupport returns "" for empty strings, not nil
+      return array[-1] || "" if array.is_a?(String)
       array.last if array.respond_to?(:last)
     end
 
diff --git a/lib/liquid/tokenizer.rb b/lib/liquid/tokenizer.rb
@@ -117,7 +117,7 @@ def next_variable_token
       byte_a = byte_b = @ss.scan_byte
 
       while byte_b
-        byte_a = @ss.scan_byte while byte_a && (byte_a != CLOSE_CURLEY && byte_a != OPEN_CURLEY)
+        byte_a = @ss.scan_byte while byte_a && byte_a != CLOSE_CURLEY && byte_a != OPEN_CURLEY
 
         break unless byte_a
 
diff --git a/lib/liquid/utils.rb b/lib/liquid/utils.rb
@@ -69,7 +69,7 @@ def self.to_date(obj)
       return obj if obj.respond_to?(:strftime)
 
       if obj.is_a?(String)
-        return nil if obj.empty?
+        return if obj.empty?
         obj = obj.downcase
       end
 
diff --git a/lib/liquid/variable_lookup.rb b/lib/liquid/variable_lookup.rb
@@ -70,6 +70,11 @@ def evaluate(context)
         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)
+        # ActiveSupport returns "" for empty strings, not nil
+        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/spec/ruby_liquid.rb b/spec/ruby_liquid.rb
@@ -0,0 +1,36 @@
+# frozen_string_literal: true
+
+# Liquid Spec Adapter for Shopify/liquid (Ruby reference implementation)
+#
+# Run with: bundle exec liquid-spec run spec/ruby_liquid.rb
+
+$LOAD_PATH.unshift(File.expand_path('../lib', __dir__))
+require 'liquid'
+
+LiquidSpec.configure do |config|
+  # Run core Liquid specs
+  config.features = [:core]
+end
+
+# Compile a template string into a Liquid::Template
+LiquidSpec.compile do |source, options|
+  Liquid::Template.parse(source, **options)
+end
+
+# Render a compiled template with the given context
+# @param template [Liquid::Template] compiled template
+# @param assigns [Hash] environment variables
+# @param options [Hash] :registers, :strict_errors, :exception_renderer
+LiquidSpec.render do |template, assigns, options|
+  registers = Liquid::Registers.new(options[:registers] || {})
+
+  context = Liquid::Context.build(
+    static_environments: assigns,
+    registers: registers,
+    rethrow_errors: options[:strict_errors],
+  )
+
+  context.exception_renderer = options[:exception_renderer] if options[:exception_renderer]
+
+  template.render(context)
+end
diff --git a/spec/ruby_liquid_with_active_support.rb b/spec/ruby_liquid_with_active_support.rb
@@ -0,0 +1,37 @@
+# frozen_string_literal: true
+
+# Liquid Spec Adapter for Shopify/liquid with ActiveSupport loaded
+#
+# Run with: bundle exec liquid-spec run spec/ruby_liquid_with_active_support.rb
+
+$LOAD_PATH.unshift(File.expand_path('../lib', __dir__))
+require 'active_support/all'
+require 'liquid'
+
+LiquidSpec.configure do |config|
+  # Run core Liquid specs plus ActiveSupport SafeBuffer tests
+  config.features = [:core, :activesupport]
+end
+
+# Compile a template string into a Liquid::Template
+LiquidSpec.compile do |source, options|
+  Liquid::Template.parse(source, **options)
+end
+
+# Render a compiled template with the given context
+# @param template [Liquid::Template] compiled template
+# @param assigns [Hash] environment variables
+# @param options [Hash] :registers, :strict_errors, :exception_renderer
+LiquidSpec.render do |template, assigns, options|
+  registers = Liquid::Registers.new(options[:registers] || {})
+
+  context = Liquid::Context.build(
+    static_environments: assigns,
+    registers: registers,
+    rethrow_errors: options[:strict_errors],
+  )
+
+  context.exception_renderer = options[:exception_renderer] if options[:exception_renderer]
+
+  template.render(context)
+end
diff --git a/test/integration/security_test.rb b/test/integration/security_test.rb
@@ -59,7 +59,7 @@ def test_does_not_permanently_add_filters_to_symbol_table
 
     GC.start
 
-    assert_equal([], (Symbol.all_symbols - current_symbols))
+    assert_equal([], Symbol.all_symbols - current_symbols)
   end
 
   def test_does_not_add_drop_methods_to_symbol_table
@@ -70,7 +70,7 @@ def test_does_not_add_drop_methods_to_symbol_table
     assert_equal("", Template.parse("{{ drop.custom_method_2 }}", assigns).render!)
     assert_equal("", Template.parse("{{ drop.custom_method_3 }}", assigns).render!)
 
-    assert_equal([], (Symbol.all_symbols - current_symbols))
+    assert_equal([], Symbol.all_symbols - current_symbols)
   end
 
   def test_max_depth_nested_blocks_does_not_raise_exception
diff --git a/test/integration/standard_filter_test.rb b/test/integration/standard_filter_test.rb
diff --git a/test/integration/tags/standard_tag_test.rb b/test/integration/tags/standard_tag_test.rb
diff --git a/test/integration/template_test.rb b/test/integration/template_test.rb
diff --git a/test/unit/condition_unit_test.rb b/test/unit/condition_unit_test.rb
diff --git a/test/unit/strainer_template_unit_test.rb b/test/unit/strainer_template_unit_test.rb
