diff --git a/History.md b/History.md index fab3b49c5..bbf6a9bfe 100644 --- a/History.md +++ b/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` diff --git a/lib/liquid.rb b/lib/liquid.rb index 633cf63d1..3248bdacd 100644 --- a/lib/liquid.rb +++ b/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' diff --git a/lib/liquid/array_literal.rb b/lib/liquid/array_literal.rb new file mode 100644 index 000000000..97b40f9c8 --- /dev/null +++ b/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 diff --git a/lib/liquid/parse_context.rb b/lib/liquid/parse_context.rb index 07bc3fa20..c8cc35e38 100644 --- a/lib/liquid/parse_context.rb +++ b/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) diff --git a/lib/liquid/parser.rb b/lib/liquid/parser.rb index f1822fdfa..de2d0fd65 100644 --- a/lib/liquid/parser.rb +++ b/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 diff --git a/test/integration/array_literal_test.rb b/test/integration/array_literal_test.rb new file mode 100644 index 000000000..3aefc3eeb --- /dev/null +++ b/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 diff --git a/test/unit/parse_tree_visitor_test.rb b/test/unit/parse_tree_visitor_test.rb index 0ea16d0ce..a1c41018f 100644 --- a/test/unit/parse_tree_visitor_test.rb +++ b/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"], diff --git a/test/unit/parser_unit_test.rb b/test/unit/parser_unit_test.rb index caed3062c..7aa643c53 100644 --- a/test/unit/parser_unit_test.rb +++ b/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