From 933a1f1e7ec57ce6b4d2bcb52e8d02a0f6a602cb Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Wed, 11 Nov 2020 14:19:31 -0500 Subject: [PATCH] Change Parser#expression to build the parsed expression --- lib/liquid/expression.rb | 2 +- lib/liquid/parser.rb | 62 ++++++++++++++-------------- lib/liquid/range_lookup.rb | 6 +++ lib/liquid/tags/for.rb | 17 ++++++-- lib/liquid/tags/if.rb | 12 +++++- lib/liquid/variable.rb | 14 ++----- lib/liquid/variable_lookup.rb | 64 ++++++++++++++++++++++++----- test/integration/expression_test.rb | 10 ++--- test/unit/condition_unit_test.rb | 17 ++++++-- test/unit/parser_unit_test.rb | 31 +++++++------- test/unit/variable_unit_test.rb | 47 +++++++++++++-------- 11 files changed, 183 insertions(+), 99 deletions(-) diff --git a/lib/liquid/expression.rb b/lib/liquid/expression.rb index 69c7472..53ea9fd 100644 --- a/lib/liquid/expression.rb +++ b/lib/liquid/expression.rb @@ -33,7 +33,7 @@ module Liquid if LITERALS.key?(markup) LITERALS[markup] else - VariableLookup.parse(markup) + VariableLookup.lax_parse(markup) end end end diff --git a/lib/liquid/parser.rb b/lib/liquid/parser.rb index 609601a..1656d9d 100644 --- a/lib/liquid/parser.rb +++ b/lib/liquid/parser.rb @@ -50,53 +50,55 @@ module Liquid token = @tokens[@p] case token[0] when :id - str = consume - str << variable_lookups + if Expression::LITERALS.key?(token[1]) && !look(:dot, 1) && !look(:open_square, 1) + Expression::LITERALS[consume] + else + VariableLookup.strict_parse(self) + end when :open_square - str = consume - str << expression - str << consume(:close_square) - str << variable_lookups - when :string, :number - consume + VariableLookup.strict_parse(self) + when :string + consume[1..-2] + when :number + Expression.parse(consume) when :open_round consume first = expression consume(:dotdot) last = expression consume(:close_round) - "(#{first}..#{last})" + if first.respond_to?(:evaluate) || last.respond_to?(:evaluate) + RangeLookup.new(first, last) + else + first.to_i..last.to_i + end else raise SyntaxError, "#{token} is not a valid expression" end end - def argument - str = +"" - # might be a keyword argument (identifier: expression) - if look(:id) && look(:colon, 1) - str << consume << consume << ' ' - end + def arguments + filter_args = [] + keyword_args = nil - str << expression - str - end - - def variable_lookups - str = +"" loop do - if look(:open_square) - str << consume - str << expression - str << consume(:close_square) - elsif look(:dot) - str << consume - str << consume(:id) + # keyword argument (identifier: expression) + if look(:colon, 1) + keyword_args ||= {} + k = consume(:id) + consume + v = expression + keyword_args[k] = v else - break + filter_args << expression end + + break unless consume?(:comma) end - str + + result = [filter_args] + result << keyword_args if keyword_args + result end end end diff --git a/lib/liquid/range_lookup.rb b/lib/liquid/range_lookup.rb index 57bccd0..fc8c8e6 100644 --- a/lib/liquid/range_lookup.rb +++ b/lib/liquid/range_lookup.rb @@ -12,6 +12,8 @@ module Liquid end end + attr_reader :start_obj, :end_obj + def initialize(start_obj, end_obj) @start_obj = start_obj @end_obj = end_obj @@ -23,6 +25,10 @@ module Liquid start_int..end_int end + def ==(other) + self.class == other.class && start_obj == other.start_obj && end_obj == other.end_obj + end + private def to_integer(input) diff --git a/lib/liquid/tags/for.rb b/lib/liquid/tags/for.rb index 2c80a58..cb0fe81 100644 --- a/lib/liquid/tags/for.rb +++ b/lib/liquid/tags/for.rb @@ -113,10 +113,9 @@ module Liquid @variable_name = p.consume(:id) raise SyntaxError, options[:locale].t("errors.syntax.for_invalid_in") unless p.id?('in') - collection_name = p.expression - @collection_name = parse_expression(collection_name) + @collection_name = p.expression - @name = "#{@variable_name}-#{collection_name}" + @name = "#{@variable_name}-#{@collection_name}" @reversed = p.id?('reversed') while p.look(:id) && p.look(:colon, 1) @@ -124,7 +123,17 @@ module Liquid raise SyntaxError, options[:locale].t("errors.syntax.for_invalid_attribute") end p.consume - set_attribute(attribute, p.expression) + case attribute + when 'offset' + @from = + if p.id?('continue') + :continue + else + p.expression + end + when 'limit' + @limit = p.expression + end end p.consume(:end_of_string) end diff --git a/lib/liquid/tags/if.rb b/lib/liquid/tags/if.rb index b4a680a..932a547 100644 --- a/lib/liquid/tags/if.rb +++ b/lib/liquid/tags/if.rb @@ -114,15 +114,23 @@ module Liquid end def parse_comparison(p) - a = parse_expression(p.expression) + a = parse_operand_expression(p) if (op = p.consume?(:comparison)) - b = parse_expression(p.expression) + b = parse_operand_expression(p) Condition.new(a, op, b) else Condition.new(a) end end + def parse_operand_expression(p) + if p.look(:id) && !p.look(:dot, 1) && !p.look(:open_square, 1) + parse_expression(p.consume) + else + p.expression + end + end + class ParseTreeVisitor < Liquid::ParseTreeVisitor def children @node.blocks diff --git a/lib/liquid/variable.rb b/lib/liquid/variable.rb index 95446e1..4d31b6e 100644 --- a/lib/liquid/variable.rb +++ b/lib/liquid/variable.rb @@ -65,23 +65,15 @@ module Liquid return if p.look(:end_of_string) - @name = Expression.parse(p.expression) + @name = p.expression while p.consume?(:pipe) filtername = p.consume(:id) - filterargs = p.consume?(:colon) ? parse_filterargs(p) : [] - @filters << parse_filter_expressions(filtername, filterargs) + filterargs = p.consume?(:colon) ? p.arguments : [[]] + @filters << [filtername] + filterargs end p.consume(:end_of_string) end - def parse_filterargs(p) - # first argument - filterargs = [p.argument] - # followed by comma separated others - filterargs << p.argument while p.consume?(:comma) - filterargs - end - def render(context) obj = context.evaluate(@name) diff --git a/lib/liquid/variable_lookup.rb b/lib/liquid/variable_lookup.rb index 48c7d98..95ab6ca 100644 --- a/lib/liquid/variable_lookup.rb +++ b/lib/liquid/variable_lookup.rb @@ -7,30 +7,62 @@ module Liquid attr_reader :name, :lookups - def self.parse(markup) - new(markup) - end - - def initialize(markup) + def self.lax_parse(markup) lookups = markup.scan(VariableParser) name = lookups.shift if name =~ SQUARE_BRACKETED name = Expression.parse(Regexp.last_match(1)) end - @name = name - @lookups = lookups - @command_flags = 0 + command_flags = 0 - @lookups.each_index do |i| + lookups.each_index do |i| lookup = lookups[i] if lookup =~ SQUARE_BRACKETED lookups[i] = Expression.parse(Regexp.last_match(1)) elsif COMMAND_METHODS.include?(lookup) - @command_flags |= 1 << i + command_flags |= 1 << i end end + + new(name, lookups, command_flags) + end + + def self.strict_parse(p) + if p.look(:id) + name = p.consume + else + p.consume(:open_square) + name = p.expression + p.consume(:close_square) + end + + lookups = [] + command_flags = 0 + + loop do + if p.consume?(:open_square) + lookups << p.expression + p.consume(:close_square) + elsif p.consume?(:dot) + lookup = p.consume(:id) + lookups << lookup + if COMMAND_METHODS.include?(lookup) + command_flags |= 1 << (lookups.length - 1) + end + else + break + end + end + + new(name, lookups, command_flags) + end + + def initialize(name, lookups, command_flags) + @name = name + @lookups = lookups + @command_flags = command_flags end def evaluate(context) @@ -75,6 +107,18 @@ module Liquid self.class == other.class && state == other.state end + def to_s + str = name.dup + lookups.each do |lookup| + if lookup.instance_of?(String) + str += '.' + lookup + else + str += '[' + lookup.to_s + ']' + end + end + str + end + protected def state diff --git a/test/integration/expression_test.rb b/test/integration/expression_test.rb index 91a0494..2496fee 100644 --- a/test/integration/expression_test.rb +++ b/test/integration/expression_test.rb @@ -36,11 +36,11 @@ class ExpressionTest < Minitest::Test def parse_and_eval(markup, **assigns) if Liquid::Template.error_mode == :strict p = Liquid::Parser.new(markup) - markup = p.expression - p.consume(:end_of_string) + p.expression + else + expression = Liquid::Expression.parse(markup) + context = Liquid::Context.new(assigns) + context.evaluate(expression) end - expression = Liquid::Expression.parse(markup) - context = Liquid::Context.new(assigns) - context.evaluate(expression) end end diff --git a/test/unit/condition_unit_test.rb b/test/unit/condition_unit_test.rb index 67f9162..e03189c 100644 --- a/test/unit/condition_unit_test.rb +++ b/test/unit/condition_unit_test.rb @@ -77,7 +77,7 @@ class ConditionUnitTest < Minitest::Test def test_contains_works_on_arrays @context = Liquid::Context.new @context['array'] = [1, 2, 3, 4, 5] - array_expr = VariableLookup.new("array") + array_expr = parse_variable_lookup("array") assert_evaluates_false(array_expr, 'contains', 0) assert_evaluates_true(array_expr, 'contains', 1) @@ -91,8 +91,8 @@ class ConditionUnitTest < Minitest::Test def test_contains_returns_false_for_nil_operands @context = Liquid::Context.new - assert_evaluates_false(VariableLookup.new('not_assigned'), 'contains', '0') - assert_evaluates_false(0, 'contains', VariableLookup.new('not_assigned')) + assert_evaluates_false(parse_variable_lookup('not_assigned'), 'contains', '0') + assert_evaluates_false(0, 'contains', parse_variable_lookup('not_assigned')) end def test_contains_return_false_on_wrong_data_type @@ -145,11 +145,20 @@ class ConditionUnitTest < Minitest::Test @context = Liquid::Context.new @context['one'] = @context['another'] = "gnomeslab-and-or-liquid" - assert_evaluates_true(VariableLookup.new("one"), '==', VariableLookup.new("another")) + assert_evaluates_true(parse_variable_lookup("one"), '==', parse_variable_lookup("another")) end private + def parse_variable_lookup(markup) + if Liquid::Template.error_mode == :strict + p = Liquid::Parser.new(markup) + VariableLookup.strict_parse(p) + else + VariableLookup.lax_parse(markup) + end + end + def assert_evaluates_true(left, op, right) assert(Condition.new(left, op, right).evaluate(@context), "Evaluated false: #{left} #{op} #{right}") diff --git a/test/unit/parser_unit_test.rb b/test/unit/parser_unit_test.rb index 130b8b7..d2e5e49 100644 --- a/test/unit/parser_unit_test.rb +++ b/test/unit/parser_unit_test.rb @@ -47,32 +47,35 @@ class ParserUnitTest < Minitest::Test def test_expressions p = Parser.new("hi.there hi?[5].there? hi.there.bob") - assert_equal('hi.there', p.expression) - assert_equal('hi?[5].there?', p.expression) - assert_equal('hi.there.bob', p.expression) + assert_equal(VariableLookup.new('hi', ['there'], 0), p.expression) + assert_equal(VariableLookup.new('hi?', [5, 'there?'], 0), p.expression) + assert_equal(VariableLookup.new('hi', ['there', 'bob'], 0), p.expression) + + p = Parser.new("nil true false") + assert_nil(p.expression) + assert_equal(true, p.expression) + assert_equal(false, p.expression) p = Parser.new("567 6.0 'lol' \"wut\"") - assert_equal('567', p.expression) - assert_equal('6.0', p.expression) - assert_equal("'lol'", p.expression) - assert_equal('"wut"', p.expression) + assert_equal(567, p.expression) + assert_equal(6.0, p.expression) + assert_equal('lol', p.expression) + assert_equal('wut', p.expression) end def test_ranges p = Parser.new("(5..7) (1.5..9.6) (young..old) (hi[5].wat..old)") - assert_equal('(5..7)', p.expression) - assert_equal('(1.5..9.6)', p.expression) - assert_equal('(young..old)', p.expression) - assert_equal('(hi[5].wat..old)', p.expression) + assert_equal(5..7, p.expression) + assert_equal(1..9, p.expression) + assert_equal(RangeLookup.new(VariableLookup.new('young', [], 0), VariableLookup.new('old', [], 0)), p.expression) + assert_equal(RangeLookup.new(VariableLookup.new('hi', [5, "wat"], 0), VariableLookup.new('old', [], 0)), p.expression) end def test_arguments p = Parser.new("filter: hi.there[5], keyarg: 7") assert_equal('filter', p.consume(:id)) assert_equal(':', p.consume(:colon)) - assert_equal('hi.there[5]', p.argument) - assert_equal(',', p.consume(:comma)) - assert_equal('keyarg: 7', p.argument) + assert_equal([[VariableLookup.new("hi", ["there", 5], 0)], { "keyarg" => 7 }], p.arguments) end def test_invalid_expression diff --git a/test/unit/variable_unit_test.rb b/test/unit/variable_unit_test.rb index bf8e42a..2833ffe 100644 --- a/test/unit/variable_unit_test.rb +++ b/test/unit/variable_unit_test.rb @@ -7,20 +7,20 @@ class VariableUnitTest < Minitest::Test def test_variable var = create_variable('hello') - assert_equal(VariableLookup.new('hello'), var.name) + assert_equal(parse_variable_lookup('hello'), var.name) end def test_filters var = create_variable('hello | textileze') - assert_equal(VariableLookup.new('hello'), var.name) + assert_equal(parse_variable_lookup('hello'), var.name) assert_equal([['textileze', []]], var.filters) var = create_variable('hello | textileze | paragraph') - assert_equal(VariableLookup.new('hello'), var.name) + assert_equal(parse_variable_lookup('hello'), var.name) assert_equal([['textileze', []], ['paragraph', []]], var.filters) var = create_variable(%( hello | strftime: '%Y')) - assert_equal(VariableLookup.new('hello'), var.name) + assert_equal(parse_variable_lookup('hello'), var.name) assert_equal([['strftime', ['%Y']]], var.filters) var = create_variable(%( 'typo' | link_to: 'Typo', true )) @@ -44,11 +44,11 @@ class VariableUnitTest < Minitest::Test assert_equal([['repeat', [3, 3, 3]]], var.filters) var = create_variable(%( hello | strftime: '%Y, okay?')) - assert_equal(VariableLookup.new('hello'), var.name) + assert_equal(parse_variable_lookup('hello'), var.name) assert_equal([['strftime', ['%Y, okay?']]], var.filters) var = create_variable(%( hello | things: "%Y, okay?", 'the other one')) - assert_equal(VariableLookup.new('hello'), var.name) + assert_equal(parse_variable_lookup('hello'), var.name) assert_equal([['things', ['%Y, okay?', 'the other one']]], var.filters) end @@ -60,22 +60,24 @@ class VariableUnitTest < Minitest::Test def test_filters_without_whitespace var = create_variable('hello | textileze | paragraph') - assert_equal(VariableLookup.new('hello'), var.name) + assert_equal(parse_variable_lookup('hello'), var.name) assert_equal([['textileze', []], ['paragraph', []]], var.filters) var = create_variable('hello|textileze|paragraph') - assert_equal(VariableLookup.new('hello'), var.name) + assert_equal(parse_variable_lookup('hello'), var.name) assert_equal([['textileze', []], ['paragraph', []]], var.filters) var = create_variable("hello|replace:'foo','bar'|textileze") - assert_equal(VariableLookup.new('hello'), var.name) + assert_equal(parse_variable_lookup('hello'), var.name) assert_equal([['replace', ['foo', 'bar']], ['textileze', []]], var.filters) end def test_symbol - var = create_variable("http://disney.com/logo.gif | image: 'med' ", error_mode: :lax) - assert_equal(VariableLookup.new('http://disney.com/logo.gif'), var.name) - assert_equal([['image', ['med']]], var.filters) + with_error_mode(:lax) do + var = create_variable("http://disney.com/logo.gif | image: 'med' ", error_mode: :lax) + assert_equal(parse_variable_lookup('http://disney.com/logo.gif'), var.name) + assert_equal([['image', ['med']]], var.filters) + end end def test_string_to_filter @@ -105,8 +107,8 @@ class VariableUnitTest < Minitest::Test end def test_dashes - assert_equal(VariableLookup.new('foo-bar'), create_variable('foo-bar').name) - assert_equal(VariableLookup.new('foo-bar-2'), create_variable('foo-bar-2').name) + assert_equal(parse_variable_lookup('foo-bar'), create_variable('foo-bar').name) + assert_equal(parse_variable_lookup('foo-bar-2'), create_variable('foo-bar-2').name) with_error_mode :strict do assert_raises(Liquid::SyntaxError) { create_variable('foo - bar') } @@ -122,18 +124,18 @@ class VariableUnitTest < Minitest::Test def test_string_dot var = create_variable(%( test.test )) - assert_equal(VariableLookup.new('test.test'), var.name) + assert_equal(parse_variable_lookup('test.test'), var.name) end def test_filter_with_keyword_arguments var = create_variable(%( hello | things: greeting: "world", farewell: 'goodbye')) - assert_equal(VariableLookup.new('hello'), var.name) + assert_equal(parse_variable_lookup('hello'), var.name) assert_equal([['things', [], { 'greeting' => 'world', 'farewell' => 'goodbye' }]], var.filters) end def test_lax_filter_argument_parsing var = create_variable(%( number_of_comments | pluralize: 'comment': 'comments' ), error_mode: :lax) - assert_equal(VariableLookup.new('number_of_comments'), var.name) + assert_equal(parse_variable_lookup('number_of_comments'), var.name) assert_equal([['pluralize', ['comment', 'comments']]], var.filters) end @@ -151,13 +153,22 @@ class VariableUnitTest < Minitest::Test end def test_variable_lookup_interface - lookup = VariableLookup.new('a.b.c') + lookup = parse_variable_lookup('a.b.c') assert_equal('a', lookup.name) assert_equal(['b', 'c'], lookup.lookups) end private + def parse_variable_lookup(markup) + if Liquid::Template.error_mode == :strict + p = Liquid::Parser.new(markup) + VariableLookup.strict_parse(p) + else + VariableLookup.lax_parse(markup) + end + end + def create_variable(markup, options = {}) Variable.new(markup, ParseContext.new(options)) end