diff --git a/.github/workflows/continuous_integration.yml b/.github/workflows/continuous_integration.yml index f50886d7..c273531b 100644 --- a/.github/workflows/continuous_integration.yml +++ b/.github/workflows/continuous_integration.yml @@ -32,7 +32,7 @@ jobs: - name: Setup Ruby uses: ruby/setup-ruby@v1 with: - ruby-version: 3.1.4 + ruby-version: 3.3.5 bundler-cache: true cache-version: 1 diff --git a/Gemfile b/Gemfile index 2e0158dd..9b5f48f5 100644 --- a/Gemfile +++ b/Gemfile @@ -32,5 +32,5 @@ group :test do gem 'rack-test', '~> 2.1.0' - gem 'simplecov', '~> 0.22.0', require: false + gem 'simplecov', '~> 0.22.0', require: false end diff --git a/Gemfile.lock b/Gemfile.lock index 334e353c..7101d0f5 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -18,6 +18,7 @@ PATH morphine (~> 0.1.1) multi_json (~> 1.15.0) nokogiri (~> 1.15.6) + parser (~> 3.3) pony (~> 1.12) rack-cache (>= 1.7, < 2) rack-rewrite (~> 1.5.1) @@ -41,6 +42,7 @@ GEM tzinfo (~> 2.0) addressable (2.8.7) public_suffix (>= 2.0.2, < 7.0) + ast (2.4.2) attr_extras (7.1.0) base64 (0.2.0) bcrypt (3.1.20) @@ -120,9 +122,10 @@ GEM nokogiri (1.15.6) mini_portile2 (~> 2.8.2) racc (~> 1.4) - nokogiri (1.15.6-x86_64-darwin) - racc (~> 1.4) origin (2.3.1) + parser (3.3.5.0) + ast (~> 2.4.1) + racc pony (1.13.1) mail (>= 2.0) public_suffix (6.0.0) diff --git a/lib/locomotive/steam/liquid.rb b/lib/locomotive/steam/liquid.rb index a41d0cb7..5d31bf51 100644 --- a/lib/locomotive/steam/liquid.rb +++ b/lib/locomotive/steam/liquid.rb @@ -1,4 +1,6 @@ require 'liquid' +require 'parser/current' +require 'ast' require_relative 'liquid/errors' require_relative 'liquid/patches' @@ -7,6 +9,6 @@ require_relative 'liquid/drops/i18n_base' require_relative 'liquid/tags/hybrid' require_relative 'liquid/tags/concerns/section' -require_relative 'liquid/tags/concerns/attributes' +require_relative 'liquid/tags/concerns/simple_attributes_parser' require_relative 'liquid/tags/section' require_relative_all %w(. drops filters tags/concerns tags), 'liquid' diff --git a/lib/locomotive/steam/liquid/tags/concerns/attributes_parser.rb b/lib/locomotive/steam/liquid/tags/concerns/attributes_parser.rb new file mode 100644 index 00000000..2f570610 --- /dev/null +++ b/lib/locomotive/steam/liquid/tags/concerns/attributes_parser.rb @@ -0,0 +1,142 @@ +module Locomotive + module Steam + module Liquid + module Tags + module Concerns + + # The with_scope liquid tag lets the developer use a Ruby syntax to + # pass options which is difficult to implement with the Liquid parsing + # approach (see the SimpleAttributesParser for instance) + module AttributesParser + extend ActiveSupport::Concern + + def parse_markup(markup) + parser = self.class.current_parser + + # 'liquid_code.rb' is purely arbitrary + source_buffer = ::Parser::Source::Buffer.new('liquid_code.rb') + source_buffer.source = "{%s}" % markup + + ast = parser.parse(source_buffer) + AstProcessor.new.process(ast) + end + + class_methods do + def current_parser + (@current_parser ||= build_parser).tap do |parser| + parser.reset + end + end + + def build_parser + ::Parser::CurrentRuby.new.tap do |parser| + # Silent the error instead of logging them to STDERR (default behavior of the parser) + parser.diagnostics.consumer = ->(message) { true } + end + end + end + + class AstProcessor + include AST::Processor::Mixin + + def on_hash(node) + nodes = process_all(node) + nodes.inject({}) { |memo, sub_hash| memo.merge(sub_hash) } + end + + def on_pair(node) + key_expr, right_expr = *node + { process(key_expr) => process(right_expr) } + end + + def on_sym(node) + node.children.first.to_sym + end + + def on_array(node) + process_all(node) + end + + def on_int(node) + node.children.first.to_i + end + + def on_float(node) + node.children.first.to_f + end + + def on_str(node) + node.children.first.to_s + end + + def on_true(node) + true + end + + def on_false(node) + false + end + + def on_regexp(node) + regexp_expr, opts_expr = *node + Regexp.new(process(regexp_expr), process(opts_expr)) + end + + def on_regopt(node) + node.children ? node.children.join('') : nil + end + + def on_deep_send(node) + source_expr, name_expr = *node + + if source_expr.nil? + [name_expr.to_s] + elsif source_expr.type == :send + process(source_expr.updated(:deep_send, nil)) << name_expr.to_s + else + raise 'NOT IMPLEMENTED [DEEP_SEND]' # TODO + end + end + + def on_send(node) + source_expr, name_expr = *node + + if source_expr.nil? + ::Liquid::Expression.parse(name_expr.to_s) + elsif name_expr == :+ + process(source_expr) + elsif source_expr.type == :send + ::Liquid::Expression.parse( + (process(source_expr.updated(:deep_send, nil)) << name_expr.to_s).join('.') + ) + else + raise 'NOT IMPLEMENTED [SEND]' # TODO + end + end + + # HACK: override the default process implementation + def process(node) + return if node.nil? + + node = node.to_ast + + # Invoke a specific handler + on_handler = :"on_#{node.type}" + if respond_to? on_handler + new_node = send on_handler, node + else + new_node = handler_missing(node) + end + + # fix: the original method considered false as nil which is incorrect + node = new_node unless new_node.nil? + + node + end + end + end + end + end + end + end +end \ No newline at end of file diff --git a/lib/locomotive/steam/liquid/tags/concerns/attributes.rb b/lib/locomotive/steam/liquid/tags/concerns/simple_attributes_parser.rb similarity index 96% rename from lib/locomotive/steam/liquid/tags/concerns/attributes.rb rename to lib/locomotive/steam/liquid/tags/concerns/simple_attributes_parser.rb index f85b63a6..29e04937 100644 --- a/lib/locomotive/steam/liquid/tags/concerns/attributes.rb +++ b/lib/locomotive/steam/liquid/tags/concerns/simple_attributes_parser.rb @@ -7,8 +7,7 @@ module Concerns # Many of Liquid tags have attributes (like options) # This module makes sure we use the same reliable way to # extract and evaluate them. - - module Attributes + module SimpleAttributesParser attr_reader :attributes, :raw_attributes diff --git a/lib/locomotive/steam/liquid/tags/consume.rb b/lib/locomotive/steam/liquid/tags/consume.rb index 90fc1be7..7946ab63 100644 --- a/lib/locomotive/steam/liquid/tags/consume.rb +++ b/lib/locomotive/steam/liquid/tags/consume.rb @@ -15,7 +15,7 @@ module Tags # class Consume < ::Liquid::Block - include Concerns::Attributes + include Concerns::SimpleAttributesParser Syntax = /(#{::Liquid::VariableSignature}+)\s*from\s*(#{::Liquid::QuotedFragment}+),?(.+)?/o.freeze diff --git a/lib/locomotive/steam/liquid/tags/editable/base.rb b/lib/locomotive/steam/liquid/tags/editable/base.rb index 9f02a9d0..ce2e3aa5 100644 --- a/lib/locomotive/steam/liquid/tags/editable/base.rb +++ b/lib/locomotive/steam/liquid/tags/editable/base.rb @@ -5,7 +5,7 @@ module Tags module Editable class Base < ::Liquid::Block - include Concerns::Attributes + include Concerns::SimpleAttributesParser Syntax = /(#{::Liquid::QuotedFragment})(\s*,\s*#{::Liquid::Expression}+)?/o diff --git a/lib/locomotive/steam/liquid/tags/inherited_block.rb b/lib/locomotive/steam/liquid/tags/inherited_block.rb index b2476bfe..2209566a 100644 --- a/lib/locomotive/steam/liquid/tags/inherited_block.rb +++ b/lib/locomotive/steam/liquid/tags/inherited_block.rb @@ -15,7 +15,7 @@ module Tags # class InheritedBlock < ::Liquid::Block - include Concerns::Attributes + include Concerns::SimpleAttributesParser SYNTAX = /(#{::Liquid::QuotedFragment}+)/o diff --git a/lib/locomotive/steam/liquid/tags/link_to.rb b/lib/locomotive/steam/liquid/tags/link_to.rb index d07f5a34..f21d026a 100644 --- a/lib/locomotive/steam/liquid/tags/link_to.rb +++ b/lib/locomotive/steam/liquid/tags/link_to.rb @@ -4,7 +4,7 @@ module Liquid module Tags class LinkTo < Hybrid - include Concerns::Attributes + include Concerns::SimpleAttributesParser include Concerns::I18nPage include Concerns::Path diff --git a/lib/locomotive/steam/liquid/tags/locale_switcher.rb b/lib/locomotive/steam/liquid/tags/locale_switcher.rb index 66065285..e9fd46ec 100644 --- a/lib/locomotive/steam/liquid/tags/locale_switcher.rb +++ b/lib/locomotive/steam/liquid/tags/locale_switcher.rb @@ -19,10 +19,9 @@ module Tags # - "iso" is the default choice for label # - " | " is the default separating code # - class LocaleSwitcher < ::Liquid::Tag - include Concerns::Attributes + include Concerns::SimpleAttributesParser include Concerns::I18nPage attr_reader :attributes, :site, :page, :current_locale, :url_builder diff --git a/lib/locomotive/steam/liquid/tags/model_form.rb b/lib/locomotive/steam/liquid/tags/model_form.rb index 3ca90c34..3f5a939d 100644 --- a/lib/locomotive/steam/liquid/tags/model_form.rb +++ b/lib/locomotive/steam/liquid/tags/model_form.rb @@ -18,7 +18,7 @@ module Tags # class ModelForm < ::Liquid::Block - include Concerns::Attributes + include Concerns::SimpleAttributesParser Syntax = /(#{::Liquid::QuotedFragment})\s*,*(.*)?/o.freeze diff --git a/lib/locomotive/steam/liquid/tags/paginate.rb b/lib/locomotive/steam/liquid/tags/paginate.rb index 6ec5863d..e0bb4283 100644 --- a/lib/locomotive/steam/liquid/tags/paginate.rb +++ b/lib/locomotive/steam/liquid/tags/paginate.rb @@ -15,7 +15,7 @@ module Tags # class Paginate < ::Liquid::Block - include Concerns::Attributes + include Concerns::SimpleAttributesParser Syntax = /(#{::Liquid::QuotedFragment}+)\s+by\s+(#{::Liquid::QuotedFragment}+)/o diff --git a/lib/locomotive/steam/liquid/tags/path_to.rb b/lib/locomotive/steam/liquid/tags/path_to.rb index 03f71eff..16af0af3 100644 --- a/lib/locomotive/steam/liquid/tags/path_to.rb +++ b/lib/locomotive/steam/liquid/tags/path_to.rb @@ -4,7 +4,7 @@ module Liquid module Tags class PathTo < ::Liquid::Tag - include Concerns::Attributes + include Concerns::SimpleAttributesParser include Concerns::I18nPage include Concerns::Path diff --git a/lib/locomotive/steam/liquid/tags/redirect_to.rb b/lib/locomotive/steam/liquid/tags/redirect_to.rb index 2b1040e3..188530b9 100644 --- a/lib/locomotive/steam/liquid/tags/redirect_to.rb +++ b/lib/locomotive/steam/liquid/tags/redirect_to.rb @@ -5,7 +5,7 @@ module Tags class RedirectTo < ::Liquid::Tag - include Concerns::Attributes + include Concerns::SimpleAttributesParser include Concerns::I18nPage include Concerns::Path diff --git a/lib/locomotive/steam/liquid/tags/section.rb b/lib/locomotive/steam/liquid/tags/section.rb index c3c28da4..4c92f70e 100644 --- a/lib/locomotive/steam/liquid/tags/section.rb +++ b/lib/locomotive/steam/liquid/tags/section.rb @@ -5,7 +5,7 @@ module Tags class Section < ::Liquid::Include include Concerns::Section - include Concerns::Attributes + include Concerns::SimpleAttributesParser Syntax = /(#{::Liquid::QuotedString}|#{::Liquid::VariableSignature}+)\s*,*(.*)?/o.freeze diff --git a/lib/locomotive/steam/liquid/tags/with_scope.rb b/lib/locomotive/steam/liquid/tags/with_scope.rb index 8d2a300d..82ac9a0f 100644 --- a/lib/locomotive/steam/liquid/tags/with_scope.rb +++ b/lib/locomotive/steam/liquid/tags/with_scope.rb @@ -12,20 +12,16 @@ module Tags # {{ project.name }} # {% endfor %} # {% endwith_scope %} - # - + # class WithScope < ::Liquid::Block - include Concerns::Attributes + include Concerns::AttributesParser - # Regexps and Arrays are allowed - ArrayFragment = /\[(\s*(#{::Liquid::QuotedFragment},\s*)*#{::Liquid::QuotedFragment}\s*)\]/o.freeze + # Regexps are allowed as strings RegexpFragment = /\/([^\/]+)\/([imx]+)?/o.freeze StrictRegexpFragment = /\A#{RegexpFragment}\z/o.freeze - # a slight different from the Shopify implementation because we allow stuff like `started_at.le` - TagAttributes = /([a-zA-Z_0-9\.]+)\s*\:\s*(#{ArrayFragment}|#{RegexpFragment}|#{::Liquid::QuotedFragment})/o.freeze - SingleVariable = /(#{::Liquid::VariableSignature}+)/om.freeze + SingleVariable = /\A\s*([a-zA-Z_0-9]+)\s*\z/om.freeze REGEX_OPTIONS = { 'i' => Regexp::IGNORECASE, @@ -33,21 +29,28 @@ class WithScope < ::Liquid::Block 'x' => Regexp::EXTENDED }.freeze + OPERATORS = %w(all exists gt gte in lt lte ne nin size near within) + + SYMBOL_OPERATORS_REGEXP = /(\w+\.(#{OPERATORS.join('|')})){1}\s*\:/o + attr_reader :attributes, :attributes_var_name def initialize(tag_name, markup, options) super - # simple hash? - parse_attributes(markup) { |value| parse_attribute(value) } + # convert symbol operators into valid ruby code + markup.gsub!(SYMBOL_OPERATORS_REGEXP, ':"\1" =>') - if attributes.empty? && markup =~ SingleVariable + if markup =~ SingleVariable # alright, maybe we'vot got a single variable built # with the Action liquid tag instead? @attributes_var_name = Regexp.last_match(1) + elsif markup.present? + # use our own Ruby parser + @attributes = parse_markup(markup) end - if attributes.empty? && attributes_var_name.blank? + if attributes.blank? && attributes_var_name.blank? raise ::Liquid::SyntaxError.new("Syntax Error in 'with_scope' - Valid syntax: with_scope : , ..., : ") end end @@ -65,41 +68,30 @@ def render(context) protected - def parse_attribute(value) - case value - when StrictRegexpFragment - # let the cast_value attribute create the Regexp (done during the rendering phase) - value - when ArrayFragment - $1.split(',').map { |_value| parse_attribute(_value) } - else - ::Liquid::Expression.parse(value) - end - end - def evaluate_attributes(context) @attributes = context[attributes_var_name] || {} if attributes_var_name.present? - HashWithIndifferentAccess.new.tap do |hash| - attributes.each do |key, value| - # _slug instead of _permalink - _key = key.to_s == '_permalink' ? '_slug' : key.to_s + attributes.inject({}) do |memo, (key, value)| + # _slug instead of _permalink + _key = key.to_s == '_permalink' ? '_slug' : key.to_s - # evaluate the value if possible before casting it - _value = value.is_a?(::Liquid::VariableLookup) ? context.evaluate(value) : value - - hash[_key] = cast_value(context, _value) - end + memo.merge({ _key => evaluate_attribute(context, value) }) end end - def cast_value(context, value) + def evaluate_attribute(context, value) case value - when Array then value.map { |_value| cast_value(context, _value) } - when StrictRegexpFragment then create_regexp($1, $2) - else - _value = context.evaluate(value) - _value.respond_to?(:_id) ? _value.send(:_source) : _value + when Array + value.map { |v| evaluate_attribute(context, v) } + when Hash + Hash[value.map { |k, v| [k.to_s, evaluate_attribute(context, v)] }] + when StrictRegexpFragment + create_regexp($1, $2) + when ::Liquid::VariableLookup + evaluated_value = context.evaluate(value) + evaluated_value.respond_to?(:_id) ? evaluated_value.send(:_source) : evaluate_attribute(context, evaluated_value) + else + value end end @@ -109,11 +101,6 @@ def create_regexp(value, unparsed_options) end Regexp.new(value, options) end - - def tag_attributes_regexp - TagAttributes - end - end ::Liquid::Template.register_tag('with_scope'.freeze, WithScope) diff --git a/locomotivecms_steam.gemspec b/locomotivecms_steam.gemspec index 9f948dfa..37c82e54 100644 --- a/locomotivecms_steam.gemspec +++ b/locomotivecms_steam.gemspec @@ -42,8 +42,9 @@ Gem::Specification.new do |spec| spec.add_dependency 'mime-types', '~> 3.5.0' spec.add_dependency 'duktape', '~> 2.0.1.1' spec.add_dependency 'pony', '~> 1.12' + spec.add_dependency 'parser', '~> 3.3' spec.add_dependency 'locomotivecms_common', '~> 0.6.0.alpha1' - spec.required_ruby_version = ['>= 3.0'] + spec.required_ruby_version = ['>= 3.2'] end diff --git a/spec/unit/liquid/tags/with_scope_spec.rb b/spec/unit/liquid/tags/with_scope_spec.rb index 296b29c2..a1482522 100644 --- a/spec/unit/liquid/tags/with_scope_spec.rb +++ b/spec/unit/liquid/tags/with_scope_spec.rb @@ -52,6 +52,17 @@ end + describe 'don\'t decode numeric operations' do + let(:source) { "{% with_scope price: 41 + 1 %}{% assign conditions = with_scope %}{% endwith_scope %}" } + it { expect(conditions['price']).to eq 41 } + + context 'the operation calls a variable' do + let(:assigns) { { 'prices' => { 'low' => 41 } } } + let(:source) { "{% with_scope price: prices.low + 1 %}{% assign conditions = with_scope %}{% endwith_scope %}" } + it { expect(conditions['price']).to eq 41 } + end + end + describe 'decode basic options (boolean, integer, ...)' do let(:source) { "{% with_scope active: true, price: 42, title: 'foo', hidden: false %}{% assign conditions = with_scope %}{% endwith_scope %}" } @@ -138,7 +149,7 @@ describe 'decode criteria with gt and lt' do - let(:source) { "{% with_scope price.gt:42.0, price.lt:50, published_at.lte: '2019-09-10 00:00:00', published_at.gte: '2019/09/09 00:00:00' %}{% assign conditions = with_scope %}{% endwith_scope %}" } + let(:source) { "{% with_scope price.gt: 42.0, price.lt:50, published_at.lte: '2019-09-10 00:00:00', published_at.gte: '2019/09/09 00:00:00' %}{% assign conditions = with_scope %}{% endwith_scope %}" } it { expect(conditions['price.gt']).to eq 42.0 } it { expect(conditions['price.lt']).to eq 50 } it { expect(conditions['published_at.lte']).to eq '2019-09-10 00:00:00' } @@ -147,9 +158,8 @@ end describe 'In a loop context, each scope should be evaluated correctly' do - let(:assigns) { {'list' => ['A', 'B', 'C']} } - - let(:source) { "{% for key in list %}{% with_scope foo: key %}{% assign conditions = with_scope %}{% endwith_scope %}{{ conditions }}{% endfor %}" } + let(:assigns) { {'list' => ['A', 'B', 'C']} } + let(:source) { "{% for key in list %}{% with_scope foo: key %}{% assign conditions = with_scope %}{% endwith_scope %}{{ conditions }}{% endfor %}" } it { expect(output).to eq '{"foo"=>"A"}{"foo"=>"B"}{"foo"=>"C"}' } @@ -157,4 +167,56 @@ end + describe 'decode advanced options' do + let(:options) { "" } + let(:source) { "{% with_scope key: #{options} %}{% assign conditions = with_scope %}{% endwith_scope %}" } + + before { output } + + context "Array" do + context "of Integer" do + let(:options) { "[1, 2, 3]" } + it { expect(conditions['key']).to eq [1, 2, 3] } + end + + context "of String" do + let(:options) { "['a', 'b', 'c']" } + it { expect(conditions['key']).to eq ['a', 'b', 'c'] } + end + + context "With variable" do + let(:assigns) { {'a' => 1, 'c' => 3} } + let(:options) { "[a, 2, c, 'd']" } + it { expect(conditions['key']).to eq [1, 2, 3, 'd'] } + end + end + + context "Hash" do + context "With key value" do + let(:options) { "{a: 1, b: 2, c: 3, d: 'foo'}" } + it { expect(conditions['key'].keys).to eq(%w(a b c d)) } + it { expect(conditions['key']['a']).to eq 1 } + it { expect(conditions['key']['b']).to eq 2 } + it { expect(conditions['key']['c']).to eq 3 } + it { expect(conditions['key']['d']).to eq 'foo' } + end + + context "With key variable" do + let(:assigns) { {'a' => 1, 'c' => 3} } + let(:options) { "{a: a, b: 2, c: c, d: 'foo'}" } + it { expect(conditions['key'].keys).to eq(%w(a b c d)) } + it { expect(conditions['key']['a']).to eq 1 } + it { expect(conditions['key']['b']).to eq 2 } + it { expect(conditions['key']['c']).to eq 3 } + it { expect(conditions['key']['d']).to eq 'foo' } + end + + context "With params" do + let(:assigns) { { 'params' => Locomotive::Steam::Liquid::Drops::Params.new({ foo: 'bar' }) } } + let(:options) { "{'a': params.foo}" } + it { expect(conditions['key'].keys).to eq(%w(a)) } + it { expect(conditions['key']['a']).to eq 'bar' } + end + end + end end