Update rubocop-shopify

Drop support for Ruby 2.5 and 2.6

.github/workflows/liquid.yml

@@ -6,12 +6,12 @@ jobs:
     strategy:
       matrix:
         entry:
-          - { ruby: 2.5, allowed-failure: false } # minimum supported
+          - { ruby: 2.7, allowed-failure: false } # minimum supported
           - { ruby: 3.1, allowed-failure: false } # latest
           - { ruby: ruby-head, allowed-failure: true }
     name: test (${{ matrix.entry.ruby }})
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v3
       - uses: ruby/setup-ruby@v1
         with:
           ruby-version: ${{ matrix.entry.ruby }}
@@ -26,7 +26,7 @@ jobs:
   memory_profile:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v3
       - uses: ruby/setup-ruby@v1
         with:
           ruby-version: 2.7

.rubocop.yml

@@ -10,7 +10,7 @@ Performance:
   Enabled: true
 
 AllCops:
-  TargetRubyVersion: 2.5
+  TargetRubyVersion: 2.7
   NewCops: disable
   Exclude:
     - 'vendor/bundle/**/*'

.rubocop_todo.yml

@@ -1,27 +1,74 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2020-12-11 18:53:41 UTC using RuboCop version 1.6.1.
+# on 2022-05-26 17:08:23 UTC using RuboCop version 1.29.1.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new
 # versions of RuboCop, may require this file to be generated again.
 
-# Offense count: 2
-# Cop supports --auto-correct.
-# Configuration parameters: EnforcedStyle.
-# SupportedStyles: runtime_error, standard_error
-Lint/InheritException:
+# Offense count: 1
+# This cop supports safe auto-correction (--auto-correct).
+# Configuration parameters: TreatCommentsAsGroupSeparators, ConsiderPunctuation, Include.
+# Include: **/*.gemspec
+Gemspec/OrderedDependencies:
   Exclude:
-    - 'lib/liquid/interrupts.rb'
+    - 'liquid.gemspec'
 
-# Offense count: 113
-# Cop supports --auto-correct.
-# Configuration parameters: AutoCorrect, AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns.
-# URISchemes: http, https
-Layout/LineLength:
-  Max: 260
+# Offense count: 6
+# This cop supports safe auto-correction (--auto-correct).
+Layout/ClosingHeredocIndentation:
+  Exclude:
+    - 'test/integration/tags/for_tag_test.rb'
+
+# Offense count: 34
+# This cop supports safe auto-correction (--auto-correct).
+Layout/EmptyLineAfterGuardClause:
+  Exclude:
+    - 'lib/liquid/block.rb'
+    - 'lib/liquid/block_body.rb'
+    - 'lib/liquid/context.rb'
+    - 'lib/liquid/drop.rb'
+    - 'lib/liquid/lexer.rb'
+    - 'lib/liquid/parser.rb'
+    - 'lib/liquid/profiler/hooks.rb'
+    - 'lib/liquid/standardfilters.rb'
+    - 'lib/liquid/tags/for.rb'
+    - 'lib/liquid/tags/if.rb'
+    - 'lib/liquid/utils.rb'
+    - 'lib/liquid/variable.rb'
+    - 'lib/liquid/variable_lookup.rb'
+    - 'performance/shopify/money_filter.rb'
+    - 'performance/shopify/paginate.rb'
 
 # Offense count: 8
+# This cop supports safe auto-correction (--auto-correct).
+# Configuration parameters: AllowAliasSyntax, AllowedMethods.
+# AllowedMethods: alias_method, public, protected, private
+Layout/EmptyLinesAroundAttributeAccessor:
+  Exclude:
+    - 'lib/liquid/template.rb'
+    - 'test/integration/filter_test.rb'
+    - 'test/integration/tags/include_tag_test.rb'
+    - 'test/unit/strainer_template_unit_test.rb'
+
+# Offense count: 17
+# This cop supports safe auto-correction (--auto-correct).
+# Configuration parameters: EnforcedStyle, IndentationWidth.
+# SupportedStyles: aligned, indented
+Layout/LineEndStringConcatenationIndentation:
+  Exclude:
+    - 'test/integration/tags/for_tag_test.rb'
+    - 'test/integration/tags/increment_tag_test.rb'
+
+# Offense count: 1
+# This cop supports safe auto-correction (--auto-correct).
+# Configuration parameters: EnforcedStyle, IndentationWidth.
+# SupportedStyles: aligned, indented
+Layout/MultilineOperationIndentation:
+  Exclude:
+    - 'lib/liquid/expression.rb'
+
+# Offense count: 9
 Lint/MissingSuper:
   Exclude:
     - 'lib/liquid/forloop_drop.rb'
@@ -33,7 +80,7 @@ Lint/MissingSuper:
     - 'test/integration/tags/for_tag_test.rb'
     - 'test/integration/tags/table_row_test.rb'
 
-# Offense count: 43
+# Offense count: 44
 Naming/ConstantName:
   Exclude:
     - 'lib/liquid.rb'
@@ -51,7 +98,83 @@ Naming/ConstantName:
     - 'performance/shopify/paginate.rb'
     - 'test/integration/tags/include_tag_test.rb'
 
+# Offense count: 9
+# Configuration parameters: CheckIdentifiers, CheckConstants, CheckVariables, CheckStrings, CheckSymbols, CheckComments, CheckFilepaths, FlaggedTerms.
+Naming/InclusiveLanguage:
+  Exclude:
+    - 'lib/liquid/drop.rb'
+    - 'lib/liquid/parse_context.rb'
+    - 'test/integration/drop_test.rb'
+    - 'test/integration/tags/if_else_tag_test.rb'
+
 # Offense count: 2
 Style/ClassVars:
   Exclude:
     - 'lib/liquid/condition.rb'
+
+# Offense count: 3
+# This cop supports safe auto-correction (--auto-correct).
+Style/ExplicitBlockArgument:
+  Exclude:
+    - 'test/integration/context_test.rb'
+    - 'test/integration/tag/disableable_test.rb'
+    - 'test/integration/tags/for_tag_test.rb'
+
+# Offense count: 2982
+# This cop supports safe auto-correction (--auto-correct).
+# Configuration parameters: EnforcedStyle, ConsistentQuotesInMultiline.
+# SupportedStyles: single_quotes, double_quotes
+Style/StringLiterals:
+  Enabled: false
+
+# Offense count: 20
+# This cop supports safe auto-correction (--auto-correct).
+# Configuration parameters: EnforcedStyle.
+# SupportedStyles: single_quotes, double_quotes
+Style/StringLiteralsInInterpolation:
+  Exclude:
+    - 'lib/liquid/condition.rb'
+    - 'lib/liquid/strainer_template.rb'
+    - 'lib/liquid/tag/disableable.rb'
+    - 'performance/shopify/shop_filter.rb'
+    - 'performance/shopify/tag_filter.rb'
+
+# Offense count: 6
+# This cop supports safe auto-correction (--auto-correct).
+# Configuration parameters: EnforcedStyleForMultiline.
+# SupportedStylesForMultiline: comma, consistent_comma, no_comma
+Style/TrailingCommaInArrayLiteral:
+  Exclude:
+    - 'example/server/example_servlet.rb'
+    - 'lib/liquid/condition.rb'
+    - 'test/integration/context_test.rb'
+    - 'test/integration/standard_filter_test.rb'
+    - 'test/unit/parse_tree_visitor_test.rb'
+
+# Offense count: 1
+# This cop supports safe auto-correction (--auto-correct).
+# Configuration parameters: EnforcedStyleForMultiline.
+# SupportedStylesForMultiline: comma, consistent_comma, no_comma
+Style/TrailingCommaInHashLiteral:
+  Exclude:
+    - 'lib/liquid/expression.rb'
+
+# Offense count: 19
+# This cop supports safe auto-correction (--auto-correct).
+# Configuration parameters: EnforcedStyle, MinSize, WordRegex.
+# SupportedStyles: percent, brackets
+Style/WordArray:
+  Exclude:
+    - 'lib/liquid/tags/if.rb'
+    - 'liquid.gemspec'
+    - 'test/integration/assign_test.rb'
+    - 'test/integration/context_test.rb'
+    - 'test/integration/drop_test.rb'
+    - 'test/integration/standard_filter_test.rb'
+
+# Offense count: 117
+# This cop supports safe auto-correction (--auto-correct).
+# Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, AllowedPatterns, IgnoredPatterns.
+# URISchemes: http, https
+Layout/LineLength:
+  Max: 260

Gemfile

@@ -18,8 +18,7 @@ group :benchmark, :test do
 end
 
 group :test do
-  gem 'rubocop', '~> 1.4', require: false
-  gem 'rubocop-shopify', '~> 1.0.7', require: false
+  gem 'rubocop-shopify', '~> 2.6', require: false
   gem 'rubocop-performance', require: false
 
   platform :mri, :truffleruby do

History.md

@@ -1,5 +1,20 @@
 # Liquid Change Log
 
+## 5.4.0 (unreleased)
+
+### Breaking Changes
+* Drop support for end-of-life Ruby versions (2.5 and 2.6) (#1578) [Andy Waite]
+
+### Features
+* Allow `#` to be used as an inline comment tag (#1498) [CP Clermont]
+
+### Fixes
+* `PartialCache` now shares snippet cache with subcontexts by default (#1553) [Chris AtLee]
+* Hash registers no longer leak into subcontexts as static registers (#1564) [Chris AtLee]
+
+### Changed
+* Liquid::Context#registers now always returns a Liquid::StaticRegisters object, though supports the most used Hash functions for compatibility (#1553)
+
 ## 5.3.0 2022-03-22
 
 ### Fixes

README.md

@@ -63,13 +63,13 @@ when templates are invalid. You can enable this new parser like this:
 
 ```ruby
 Liquid::Template.error_mode = :strict # Raises a SyntaxError when invalid syntax is used
-Liquid::Template.error_mode = :warn # Adds errors to template.errors but continues as normal
+Liquid::Template.error_mode = :warn # Adds strict errors to template.errors but continues as normal
 Liquid::Template.error_mode = :lax # The default mode, accepts almost anything.
 ```
 
 If you want to set the error mode only on specific templates you can pass `:error_mode` as an option to `parse`:
 ```ruby
-Liquid::Template.parse(source, :error_mode => :strict)
+Liquid::Template.parse(source, error_mode: :strict)
 ```
 This is useful for doing things like enabling strict mode only in the theme editor.
 

lib/liquid.rb

@@ -29,6 +29,7 @@ module Liquid
   WhitespaceControl           = '-'
   TagStart                    = /\{\%/
   TagEnd                      = /\%\}/
+  TagName                     = /#|\w+/
   VariableSignature           = /\(?[\w\-\.\[\]]\)?/
   VariableSegment             = /[\w\-]/
   VariableStart               = /\{\{/

lib/liquid/block_body.rb

@@ -4,8 +4,8 @@ require 'English'
 
 module Liquid
   class BlockBody
-    LiquidTagToken      = /\A\s*(\w+)\s*(.*?)\z/o
-    FullToken           = /\A#{TagStart}#{WhitespaceControl}?(\s*)(\w+)(\s*)(.*?)#{WhitespaceControl}?#{TagEnd}\z/om
+    LiquidTagToken      = /\A\s*(#{TagName})\s*(.*?)\z/o
+    FullToken           = /\A#{TagStart}#{WhitespaceControl}?(\s*)(#{TagName})(\s*)(.*?)#{WhitespaceControl}?#{TagEnd}\z/om
     ContentOfVariable   = /\A#{VariableStart}#{WhitespaceControl}?(.*?)#{WhitespaceControl}?#{VariableEnd}\z/om
     WhitespaceOrNothing = /\A\s*\z/
     TAGSTART            = "{%"

lib/liquid/context.rb

@@ -28,7 +28,7 @@ module Liquid
 
       @static_environments = [static_environments].flat_map(&:freeze).freeze
       @scopes              = [(outer_scope || {})]
-      @registers           = registers
+      @registers           = registers.is_a?(StaticRegisters) ? registers : StaticRegisters.new(registers)
       @errors              = []
       @partial             = false
       @strict_variables    = false
@@ -39,6 +39,10 @@ module Liquid
       @global_filter       = nil
       @disabled_tags       = {}
 
+      @registers.static[:cached_partials] ||= {}
+      @registers.static[:file_system] ||= Liquid::Template.file_system
+      @registers.static[:template_factory] ||= Liquid::TemplateFactory.new
+
       self.exception_renderer = Template.default_exception_renderer
       if rethrow_errors
         self.exception_renderer = Liquid::RAISE_EXCEPTION_LAMBDA

lib/liquid/locales/en.yml

@@ -13,15 +13,16 @@
       for_invalid_attribute: "Invalid attribute in for loop. Valid attributes are limit and offset"
       if: "Syntax Error in tag 'if' - Valid syntax: if [expression]"
       include: "Error in tag 'include' - Valid syntax: include '[template]' (with|for) [object|collection]"
-      unknown_tag: "Unknown tag '%{tag}'"
+      inline_comment_invalid: "Syntax error in tag '#' - Each line of comments must be prefixed by the '#' character"
       invalid_delimiter: "'%{tag}' is not a valid delimiter for %{block_name} tags. use %{block_delimiter}"
+      render: "Syntax error in tag 'render' - Template name must be a quoted string"
+      table_row: "Syntax Error in 'table_row loop' - Valid syntax: table_row [item] in [collection] cols=3"
+      tag_never_closed: "'%{block_name}' tag was never closed"
+      tag_termination: "Tag '%{token}' was not properly terminated with regexp: %{tag_end}"
       unexpected_else: "%{block_name} tag does not expect 'else' tag"
       unexpected_outer_tag: "Unexpected outer '%{tag}' tag"
-      tag_termination: "Tag '%{token}' was not properly terminated with regexp: %{tag_end}"
+      unknown_tag: "Unknown tag '%{tag}'"
       variable_termination: "Variable '%{token}' was not properly terminated with regexp: %{tag_end}"
-      tag_never_closed: "'%{block_name}' tag was never closed"
-      table_row: "Syntax Error in 'table_row loop' - Valid syntax: table_row [item] in [collection] cols=3"
-      render: "Syntax error in tag 'render' - Template name must be a quoted string"
     argument:
       include: "Argument error in tag 'include' - Illegal template name"
     disabled:

lib/liquid/partial_cache.rb

@@ -3,16 +3,16 @@
 module Liquid
   class PartialCache
     def self.load(template_name, context:, parse_context:)
-      cached_partials = (context.registers[:cached_partials] ||= {})
+      cached_partials = context.registers[:cached_partials]
       cached = cached_partials[template_name]
       return cached if cached
 
-      file_system = (context.registers[:file_system] ||= Liquid::Template.file_system)
+      file_system = context.registers[:file_system]
       source      = file_system.read_template_file(template_name)
 
       parse_context.partial = true
 
-      template_factory = (context.registers[:template_factory] ||= Liquid::TemplateFactory.new)
+      template_factory = context.registers[:template_factory]
       template = template_factory.for(template_name)
 
       partial = template.parse(source, parse_context)

lib/liquid/tags/inline_comment.rb

@@ -0,0 +1,30 @@
+# frozen_string_literal: true
+
+module Liquid
+  class InlineComment < Tag
+    def initialize(tag_name, markup, options)
+      super
+
+      # Semantically, a comment should only ignore everything after it on the line.
+      # Currently, this implementation doesn't support mixing a comment with another tag
+      # but we need to reserve future support for this and prevent the introduction
+      # of inline comments from being backward incompatible change.
+      #
+      # As such, we're forcing users to put a # symbol on every line otherwise this
+      # tag will throw an error.
+      if markup.match?(/\n\s*[^#\s]/)
+        raise SyntaxError, options[:locale].t("errors.syntax.inline_comment_invalid")
+      end
+    end
+
+    def render_to_output_buffer(_context, output)
+      output
+    end
+
+    def blank?
+      true
+    end
+  end
+
+  Template.register_tag('#', InlineComment)
+end

lib/liquid/tags/render.rb

@@ -34,9 +34,9 @@ module Liquid
     end
 
     def render_tag(context, output)
-      # Though we evaluate this here we will only ever parse it as a string literal.
-      template_name = context.evaluate(@template_name_expr)
-      raise ArgumentError, options[:locale].t("errors.argument.include") unless template_name
+      # The expression should be a String literal, which parses to a String object
+      template_name = @template_name_expr
+      raise ::ArgumentError unless template_name.is_a?(String)
 
       partial = PartialCache.load(
         template_name,

lib/liquid/template.rb

@@ -167,15 +167,14 @@ module Liquid
 
       output = nil
 
-      context_register = context.registers.is_a?(StaticRegisters) ? context.registers.static : context.registers
-
       case args.last
       when Hash
         options = args.pop
         output  = options[:output] if options[:output]
+        static_registers = context.registers.static
 
         options[:registers]&.each do |key, register|
-          context_register[key] = register
+          static_registers[key] = register
         end
 
         apply_options_to_context(context, options)

lib/liquid/version.rb

@@ -2,5 +2,5 @@
 # frozen_string_literal: true
 
 module Liquid
-  VERSION = "5.3.0"
+  VERSION = "5.4.0.alpha"
 end

liquid.gemspec

@@ -17,7 +17,7 @@ Gem::Specification.new do |s|
   s.license     = "MIT"
   # s.description = "A secure, non-evaling end user template engine with aesthetic markup."
 
-  s.required_ruby_version     = ">= 2.5.0"
+  s.required_ruby_version     = ">= 2.7.0"
   s.required_rubygems_version = ">= 1.3.7"
 
   s.metadata['allowed_push_host'] = 'https://rubygems.org'

performance/theme_runner.rb

@@ -3,7 +3,7 @@
 # This profiler run simulates Shopify.
 # We are looking in the tests directory for liquid files and render them within the designated layout file.
 # We will also export a substantial database to liquid which the templates can render values of.
-# All this is to make the benchmark as non syntetic as possible. All templates and tests are lifted from
+# All this is to make the benchmark as non synthetic as possible. All templates and tests are lifted from
 # direct real-world usage and the profiler measures code that looks very similar to the way it looks in
 # Shopify which is likely the biggest user of liquid in the world which something to the tune of several
 # million Template#render calls a day.

test/integration/context_test.rb

@@ -618,6 +618,20 @@ class ContextTest < Minitest::Test
     end
   end
 
+  def test_context_always_uses_static_registers
+    registers = {
+      my_register: :my_value,
+    }
+    c = Context.new({}, {}, registers)
+    assert_instance_of(StaticRegisters, c.registers)
+    assert_equal(:my_value, c.registers[:my_register])
+
+    r = StaticRegisters.new(registers)
+    c = Context.new({}, {}, r)
+    assert_instance_of(StaticRegisters, c.registers)
+    assert_equal(:my_value, c.registers[:my_register])
+  end
+
   private
 
   def assert_no_object_allocations

test/integration/standard_filter_test.rb

@@ -32,7 +32,7 @@ class TestDrop < Liquid::Drop
   attr_reader :value
 
   def registers
-    @context.registers
+    { @value => @context.registers[@value] }
   end
 end
 
@@ -440,7 +440,7 @@ class StandardFiltersTest < Minitest::Test
   end
 
   def test_map_calls_context=
-    model = TestModel.new(value: "test")
+    model = TestModel.new(value: :test)
 
     template = Template.parse('{{ foo | map: "registers" }}')
     template.registers[:test] = 1234

test/integration/tags/inline_comment_test.rb

@@ -0,0 +1,69 @@
+# frozen_string_literal: true
+
+require 'test_helper'
+
+class InlineCommentTest < Minitest::Test
+  include Liquid
+
+  def test_inline_comment_returns_nothing
+    assert_template_result('', '{%- # this is an inline comment -%}')
+    assert_template_result('', '{%-# this is an inline comment -%}')
+    assert_template_result('', '{% # this is an inline comment %}')
+    assert_template_result('', '{%# this is an inline comment %}')
+  end
+
+  def test_inline_comment_does_not_require_a_space_after_the_pound_sign
+    assert_template_result('', '{%#this is an inline comment%}')
+  end
+
+  def test_liquid_inline_comment_returns_nothing
+    assert_template_result('Hey there, how are you doing today?', <<~LIQUID)
+      {%- liquid
+        # This is how you'd write a block comment in a liquid tag.
+        # It looks a lot like what you'd have in ruby.
+
+        # You can use it as inline documentation in your
+        # liquid blocks to explain why you're doing something.
+        echo "Hey there, "
+
+        # It won't affect the output.
+        echo "how are you doing today?"
+      -%}
+    LIQUID
+  end
+
+  def test_inline_comment_can_be_written_on_multiple_lines
+    assert_template_result('', <<~LIQUID)
+      {%-
+        # That kind of block comment is also allowed.
+        # It would only be a stylistic difference.
+
+        # Much like JavaScript's /* */ comments and their
+        # leading * on new lines.
+      -%}
+    LIQUID
+  end
+
+  def test_inline_comment_multiple_pound_signs
+    assert_template_result('', <<~LIQUID)
+      {%- liquid
+        ######################################
+        # We support comments like this too. #
+        ######################################
+      -%}
+    LIQUID
+  end
+
+  def test_inline_comments_require_the_pound_sign_on_every_new_line
+    assert_match_syntax_error("Each line of comments must be prefixed by the '#' character", <<~LIQUID)
+      {%-
+        # some comment
+        echo 'hello world'
+      -%}
+    LIQUID
+  end
+
+  def test_inline_comment_does_not_support_nested_tags
+    assert_template_result(' -%}', "{%- # {% echo 'hello world' %} -%}")
+  end
+end

test/unit/partial_cache_unit_test.rb

@@ -125,4 +125,35 @@ class PartialCacheUnitTest < Minitest::Test
     assert_equal('my partial body', partial.render)
     assert_equal(1, template_factory.count)
   end
+
+  def test_cache_state_is_shared_for_subcontexts
+    parse_context      = Liquid::ParseContext.new
+    shared_file_system = StubFileSystem.new(
+      'my_partial' => 'my shared value'
+    )
+    context = Liquid::Context.build(
+      registers: Liquid::StaticRegisters.new(
+        file_system: shared_file_system,
+      )
+    )
+    subcontext = context.new_isolated_subcontext
+
+    assert_equal(subcontext.registers[:cached_partials].object_id, context.registers[:cached_partials].object_id)
+
+    2.times do
+      Liquid::PartialCache.load(
+        'my_partial',
+        context: context,
+        parse_context: parse_context
+      )
+
+      Liquid::PartialCache.load(
+        'my_partial',
+        context: subcontext,
+        parse_context: parse_context
+      )
+    end
+
+    assert_equal(1, shared_file_system.file_read_count)
+  end
 end