WIP5

History.md

@@ -20,6 +20,8 @@
     * `{{ ((a > b) == (c < d)) or (e == f) }}`
 - Add support for parenthesized expressions
   * e.g. `(a or b) == c`
+* Add array literals wherever an expression is accepted
+  * e.g. `{% assign rows = ['top', product.title, [1, 2]] %}` or `{% for item in fallback or [] %}`
 
 ### Architectural changes
 * `parse_expression` and `safe_parse_expression` have been removed from `Tag` and `ParseContext`

lib/liquid.rb

@@ -81,6 +81,7 @@ module Liquid
 require 'liquid/variable'
 require 'liquid/variable_lookup'
 require 'liquid/range_lookup'
+require 'liquid/array_literal'
 require 'liquid/resource_limits'
 require 'liquid/expression'
 require 'liquid/template'

lib/liquid/array_literal.rb

@@ -0,0 +1,28 @@
+# frozen_string_literal: true
+
+module Liquid
+  # An array written in a template, such as `[1, product.title, [a, b]]`.
+  class ArrayLiteral
+    attr_reader :elements
+
+    def initialize(elements)
+      @elements = elements.freeze
+    end
+
+    # Returns a new Array on every evaluation, so a change to one result
+    # never reaches another render or a later evaluation in the same render.
+    def evaluate(context)
+      @elements.map { |element| context.evaluate(element) }
+    end
+
+    def ==(other)
+      self.class == other.class && elements == other.elements
+    end
+
+    class ParseTreeVisitor < Liquid::ParseTreeVisitor
+      def children
+        @node.elements
+      end
+    end
+  end
+end

lib/liquid/parse_context.rb

@@ -37,7 +37,7 @@ def new_block_body
 
     def new_parser(input)
       @string_scanner.string = input
-      Parser.new(@string_scanner, @expression_cache, reject_bare_brackets: true)
+      Parser.new(@string_scanner, @expression_cache)
     end
 
     def new_tokenizer(source, start_line_number: nil, for_liquid_tag: false)

lib/liquid/parser.rb

@@ -4,12 +4,11 @@ module Liquid
   class Parser
     OPERATOR_TOKEN_TYPES = [:logical, :equality, :comparison].freeze
 
-    def initialize(input, expression_cache = nil, reject_bare_brackets: false)
+    def initialize(input, expression_cache = nil)
       @ss = input.is_a?(StringScanner) ? input : StringScanner.new(input)
       @cache = expression_cache
       @tokens = Lexer.tokenize(@ss)
       @p      = 0 # pointer to current location
-      @reject_bare_brackets = reject_bare_brackets
     end
 
     # The current token position, which jump can return to.
@@ -62,7 +61,7 @@ def look(type, ahead = 0)
     # logical    := equality (("and" | "or") equality)*
     # equality   := comparison (("==" | "!=" | "<>") comparison)*
     # comparison := primary ((">=" | ">" | "<" | "<=" | ... ) primary)*
-    # primary    := string | number | variable_lookup | range | boolean | grouping
+    # primary    := string | number | variable_lookup | range | boolean | grouping | array_literal
     def expression
       logical
     end
@@ -101,14 +100,14 @@ def comparison
       expr
     end
 
-    # primary := string | number | variable_lookup | range | boolean | grouping
+    # primary := string | number | variable_lookup | range | boolean | grouping | array_literal
     def primary
       token = @tokens[@p]
       case token[0]
       when :id
         variable_lookup_or_literal
       when :open_square
-        unnamed_variable_lookup
+        array_literal
       when :string
         string
       when :number
@@ -150,12 +149,15 @@ def variable_lookup_or_literal
       end
     end
 
-    # unnamed_variable_lookup := indexed_lookup (lookup)*
-    def unnamed_variable_lookup
-      raise_bare_bracket_error if @reject_bare_brackets
-      name = indexed_lookup
-      lookups, command_flags = variable_lookups
-      VariableLookup.new(name, lookups, command_flags)
+    # array_literal := "[" (expression ("," expression)* ","?)? "]"
+    def array_literal
+      consume(:open_square)
+      elements = []
+      until consume?(:close_square)
+        elements << expression
+        consume(:comma) unless look(:close_square)
+      end
+      ArrayLiteral.new(elements)
     end
 
     # Parenthesized expressions are recursive
@@ -188,12 +190,6 @@ def expression_string
       when :id
         str = consume.dup
         str << variable_lookups_string
-      when :open_square
-        raise_bare_bracket_error if @reject_bare_brackets
-        str = consume.dup
-        str << expression_string
-        str << consume(:close_square)
-        str << variable_lookups_string
       when :string, :number
         consume
       when :open_round
@@ -285,9 +281,5 @@ def indexed_lookup
       consume(:close_square)
       expr
     end
-
-    def raise_bare_bracket_error
-      raise SyntaxError, "Bare bracket access is not allowed. Use #{Expression::SELF}['...'] instead"
-    end
   end
 end

test/integration/array_literal_test.rb

@@ -0,0 +1,152 @@
+# frozen_string_literal: true
+
+require 'test_helper'
+
+class ArrayLiteralTest < Minitest::Test
+  include Liquid
+
+  module AppendFilter
+    def append_item(input, item)
+      input << item
+    end
+  end
+
+  def test_empty_and_fixed_literals
+    assert_template_result('0|', "{% assign rows = [] %}{{ rows.size }}|{{ rows | join: ',' }}")
+    assert_template_result('top,bottom|2', "{% assign rows = ['top', 'bottom'] %}{{ rows | join: ',' }}|{{ rows.size }}")
+  end
+
+  def test_elements_are_full_expressions
+    assert_template_result(
+      '1|6|Shirt|lookup|B|true|1,2',
+      "{% assign v = [1, values[1], product['title'], self['a'], missing or b, x == y, (1..2)] %}" \
+        "{{ v | first }}|{{ v[1] }}|{{ v[2] }}|{{ v[3] }}|{{ v[4] }}|{{ v[5] }}|{{ v[6] | join: ',' }}",
+      { 'values' => [5, 6], 'product' => { 'title' => 'Shirt' }, 'a' => 'lookup', 'b' => 'B', 'x' => 1, 'y' => 1 },
+    )
+  end
+
+  def test_nested_literals_with_trailing_commas_and_whitespace
+    template = "{% assign n = [\n\t[1,],\n  [ 2 , [] ,],\n] %}{{ n.size }}|{{ n[0].size }}|{{ n[1][0] }}|{{ n[1][1].size }}"
+    assert_template_result('2|1|2|0', template)
+  end
+
+  def test_leading_bracket_constructs_a_literal_instead_of_a_bare_lookup
+    assert_template_result('a|k', "{{ ['a'] }}|{{ [key] }}", { 'a' => 'lookup', 'key' => 'k', 'k' => 'v' })
+  end
+
+  def test_bracket_after_an_expression_stays_an_index_lookup
+    assert_template_result('6|5|Shirt', "{{ values[1] }}|{{ values [0] }}|{{ product['title'] }}", {
+      'values' => [5, 6],
+      'product' => { 'title' => 'Shirt' },
+    })
+  end
+
+  def test_malformed_literals_are_syntax_errors
+    ['[1, , 2]', '[,]', '[1, 2,,]', '[1 2]', '[1, 2', '[1, 2][0]', '[1, 2].size', '[a: 1]'].each do |markup|
+      assert_match_syntax_error(/\ALiquid syntax error/, "{{ #{markup} }}")
+    end
+  end
+
+  def test_literals_work_in_expression_positions
+    assert_template_result('a-b', "{% echo ['a', 'b'] | join: '-' %}")
+    assert_template_result('1,2,3,4', "{{ list | concat: [3, 4], | join: ',' }}", { 'list' => [1, 2] })
+    assert_template_result('false', "{{ flag | default: 'd', allow_false: [] }}", { 'flag' => false })
+    assert_template_result('yes', "{% if [1, 2] contains 2 and [] %}yes{% endif %}")
+    assert_template_result('match', "{% case list %}{% when [1, 2], [3] %}match{% endcase %}", { 'list' => [3] })
+    assert_template_result('0=a;1=b;', "{% for item in fallback or ['a', 'b'] %}{{ forloop.index0 }}={{ item }};{% endfor %}")
+    assert_template_result('12', "{% cycle [1, 2], 'b' %}")
+  end
+
+  def test_render_named_argument
+    assert_template_result(
+      '1,2,3',
+      "{% render 'snip', coll: [a, b, c] %}",
+      { 'a' => 1, 'b' => 2, 'c' => 3 },
+      partials: { 'snip' => "{{ coll | join: ',' }}" },
+    )
+  end
+
+  def test_render_for
+    assert_template_result(
+      '1,2,',
+      "{% render 'snip' for [1, 2] as item %}",
+      partials: { 'snip' => "{{ item }}," },
+    )
+  end
+
+  def test_render_with
+    assert_template_result(
+      '1-2',
+      "{% render 'snip' with [1, 2] as items %}",
+      partials: { 'snip' => "{{ items | join: '-' }}" },
+    )
+  end
+
+  def test_include_named_argument
+    assert_template_result(
+      '1,2',
+      "{% include 'snip', coll: [1, 2] %}",
+      partials: { 'snip' => "{{ coll | join: ',' }}" },
+    )
+  end
+
+  def test_assign_score_matches_an_environment_array
+    literal = Template.parse("{% assign a = [1, 2] %}")
+    literal.render!
+    environment_array = Template.parse("{% assign a = list %}")
+    environment_array.render!({ 'list' => [1, 2] })
+
+    assert_equal(3, literal.resource_limits.assign_score)
+    assert_equal(environment_array.resource_limits.assign_score, literal.resource_limits.assign_score)
+  end
+
+  def test_assign_score_limit_applies_to_a_literal
+    template = Template.parse("{% assign a = [1, 2] %}")
+    template.resource_limits.assign_score_limit = 2
+    assert_equal("Liquid error: Memory limits exceeded", template.render)
+    assert(template.resource_limits.reached?)
+  end
+
+  def test_elements_evaluate_in_each_render_context
+    template = Template.parse("{{ [a, [a]] | join: ',' }}", expression_cache: {})
+    assert_equal('1,1', template.render!({ 'a' => 1 }))
+    assert_equal('2,2', template.render!({ 'a' => 2 }))
+  end
+
+  def test_each_evaluation_returns_a_new_array
+    environment = Environment.build { |env| env.register_filter(AppendFilter) }
+    template = Template.parse(
+      "{% for n in (1..2) %}{{ [1] | append_item: n | join: ',' }};{% endfor %}",
+      environment: environment,
+    )
+    assert_equal('1,1;1,2;', template.render!)
+    assert_equal('1,1;1,2;', template.render!)
+  end
+
+  def test_reversed_loop_leaves_the_literal_unchanged
+    assert_template_result('321;321;', "{% for n in (1..2) %}{% for i in [1, 2, 3] reversed %}{{ i }}{% endfor %};{% endfor %}")
+  end
+
+  def test_host_expression_with_a_leading_bracket_is_an_array
+    context = Context.new({ 'key' => 'name', 'name' => 'value' })
+    assert_equal(['name'], context['[key]'])
+    assert_equal([], context['[]'])
+    assert_equal([1, ['name']], context['[1, [key]]'])
+  end
+
+  def test_host_self_lookup_reads_a_variable_by_name
+    context = Context.new({ 'key' => 'name', 'name' => 'value' })
+    assert_equal('value', context['self[key]'])
+    assert_equal('value', context["self['name']"])
+  end
+
+  def test_for_single_element_literals
+    assigns = { 'x' => 'name', 'name' => 'value', 'list' => [5, 6] }
+    assert_template_result('1;', "{% for i in [1] %}{{ i }};{% endfor %}")
+    assert_template_result('name;', "{% for i in [x] %}{{ i }};{% endfor %}", assigns)
+    assert_template_result('', "{% for i in [] %}{{ i }};{% endfor %}")
+    assert_template_result('1,2;3;', "{% for i in [[1, 2], 3] %}{{ i | join: ',' }};{% endfor %}")
+    assert_template_result('6;', "{% for i in [list[1]] %}{{ i }};{% endfor %}", assigns)
+    assert_template_result('7;8;', "{% for i in table[[1]] %}{{ i }};{% endfor %}", { 'table' => { [1] => [7, 8] } })
+  end
+end

test/unit/parse_tree_visitor_test.rb

@@ -173,6 +173,20 @@ def test_for_expressions
     )
   end
 
+  def test_array_literal_elements
+    assert_equal(
+      ["a", "b", "c"],
+      visit(%({{ [a, [b], 1, (c or 'x')] }})),
+    )
+  end
+
+  def test_for_array_literal
+    assert_equal(
+      ["test", "a"],
+      visit(%({% for x in [a] %}{{ test }}{% endfor %})),
+    )
+  end
+
   def test_tablerow_in
     assert_equal(
       ["test"],

test/unit/parser_unit_test.rb

@@ -195,13 +195,17 @@ def test_string
     assert_equal("that 's4'", p.string)
   end
 
-  def test_unnamed_variable_lookup
+  def test_leading_bracket_is_an_array_literal
     p = new_parser('[key].title')
-    v = p.expression
-    assert(v.is_a?(VariableLookup))
-    assert(v.name.is_a?(VariableLookup))
-    assert_equal('key', v.name.name)
-    assert_equal('title', v.lookups[0])
+    assert_equal(ArrayLiteral.new([VariableLookup.new('key', [], 0)]), p.expression)
+    assert(p.look(:dot))
+  end
+
+  def test_self_lookup_replaces_bare_brackets
+    v = new_parser('self[key].title').expression
+    assert_instance_of(VariableLookup, v)
+    assert_equal('self', v.name)
+    assert_equal([VariableLookup.new('key', [], 0), 'title'], v.lookups)
   end
 
   def test_range_lookup
@@ -259,12 +263,46 @@ def test_invalid_expression
     end
   end
 
-  def test_rejects_bare_brackets_when_requested
-    ['[key].title', 'a[[key]]'].each do |markup|
-      assert_raises(SyntaxError) { Parser.new(markup, reject_bare_brackets: true).expression }
-      assert_raises(SyntaxError) { Parser.new(markup, reject_bare_brackets: true).expression_string }
+  def test_expression_string_rejects_leading_brackets
+    ['[1]', '[key]', '[key].title', 'a[[key]]', '[]', '[[1], 2]'].each do |markup|
+      assert_raises(SyntaxError, markup) { new_parser(markup).expression_string }
+    end
+  end
+
+  def test_array_literals
+    p = new_parser("[] [1, 'a', key, [nil,],] values[0] values[[0]]")
+    assert_equal(ArrayLiteral.new([]), p.expression)
+    assert_equal(
+      ArrayLiteral.new([1, 'a', VariableLookup.new('key', [], 0), ArrayLiteral.new([nil])]),
+      p.expression,
+    )
+    assert_equal(VariableLookup.new('values', [0], 0), p.expression)
+    assert_equal(VariableLookup.new('values', [ArrayLiteral.new([0])], 0), p.expression)
+    assert(p.look(:end_of_string))
+  end
+
+  def test_parse_context_parser_matches_default_parser
+    ["[1]", "[key]", "[]", "[[1], key, 'a']", "values[[0]]"].each do |markup|
+      assert_equal(new_parser(markup).expression, ParseContext.new.new_parser(markup).expression, markup)
+    end
+  end
+
+  def test_array_literal_elements_are_full_expressions
+    literal = new_parser('[a or b, x == y, (1..2)]').expression
+    assert_equal(['or', '=='], literal.elements.first(2).map(&:operator))
+    assert_equal(1..2, literal.elements.last)
+  end
+
+  def test_array_literal_is_an_operand
+    expr = new_parser("fallback or ['a']").expression
+    assert_equal('or', expr.operator)
+    assert_equal(ArrayLiteral.new(['a']), expr.right_node)
+  end
+
+  def test_malformed_array_literals
+    ['[1, , 2]', '[,]', '[1, 2,,]', '[1 2]', '[1, 2', '[a: 1]'].each do |markup|
+      assert_raises(SyntaxError, markup) { new_parser(markup).expression }
     end
-    assert_instance_of(VariableLookup, Parser.new('self[key].title', reject_bare_brackets: true).expression)
   end
 
   private