From a6c93c35f8caae6034b045ceb7096a7bbd4ad954 Mon Sep 17 00:00:00 2001 From: Didier Lafforgue Date: Tue, 17 Sep 2024 11:21:08 +0200 Subject: [PATCH 1/4] working POC --- Gemfile | 2 + Gemfile.lock | 5 + lib/locomotive/steam/liquid.rb | 2 + .../steam/liquid/tags/with_scope.rb | 233 +++++++++++++++--- spec/unit/liquid/tags/with_scope_spec.rb | 59 ++++- 5 files changed, 265 insertions(+), 36 deletions(-) diff --git a/Gemfile b/Gemfile index 2e0158dd..5fff49ef 100644 --- a/Gemfile +++ b/Gemfile @@ -2,6 +2,8 @@ source 'https://rubygems.org' gemspec +gem 'parser' + group :development do # gem 'locomotivecms_common', github: 'locomotivecms/common', ref: '4d1bd56' # gem 'locomotivecms_common', path: '../common' diff --git a/Gemfile.lock b/Gemfile.lock index 334e353c..b5cd0b3a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -41,6 +41,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) @@ -123,6 +124,9 @@ GEM 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) @@ -190,6 +194,7 @@ DEPENDENCIES memory_profiler mongo (~> 2.18.2) origin (~> 2.3.1) + parser puma (~> 6.4.0) rack (~> 3.0) rack-mini-profiler (~> 0.10.1) diff --git a/lib/locomotive/steam/liquid.rb b/lib/locomotive/steam/liquid.rb index a41d0cb7..52f13b61 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' diff --git a/lib/locomotive/steam/liquid/tags/with_scope.rb b/lib/locomotive/steam/liquid/tags/with_scope.rb index 8d2a300d..1680d787 100644 --- a/lib/locomotive/steam/liquid/tags/with_scope.rb +++ b/lib/locomotive/steam/liquid/tags/with_scope.rb @@ -12,10 +12,120 @@ module Tags # {{ project.name }} # {% endfor %} # {% endwith_scope %} - # + # + # s(:hash, + # s(:pair, + # s(:sym, :key), + # s(:array, + # s(:int, 1), + # s(:int, 2), + # s(:int, 3)))) class WithScope < ::Liquid::Block + class HashProcessor + 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) + pp node.location + pp node.location.expression.source + + ::Liquid::Expression.parse(node.location.expression.source) + + # raise 'TODO' + + # source_expr, name_expr = *node + + # if source_expr.nil? + # ::Liquid::Expression.parse(name_expr.to_s) + # 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 + + # TODO: create our own mixin + 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 + + node = new_node unless new_node.nil? + + node + end + end + include Concerns::Attributes # Regexps and Arrays are allowed @@ -25,7 +135,8 @@ class WithScope < ::Liquid::Block # 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 = /(#{::Liquid::VariableSignature}+)/om.freeze + SingleVariable = /\A\w*([a-zA-Z_0-9]+)\w*\z/om.freeze REGEX_OPTIONS = { 'i' => Regexp::IGNORECASE, @@ -33,23 +144,51 @@ class WithScope < ::Liquid::Block 'x' => Regexp::EXTENDED }.freeze - attr_reader :attributes, :attributes_var_name + 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, :ast 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 + pp markup + + if markup =~ SingleVariable + puts "HERE?" # 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? + ast = Parser::CurrentRuby.parse("{%s}" % markup) + pp ast + @attributes = HashProcessor.new.process(ast) + puts "-----" + pp @attributes 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 + + # raise 'TODO' + + # # simple hash? + # parse_attributes(markup) { |value| parse_attribute(value) } + + # if attributes.empty? && markup =~ SingleVariable + # # alright, maybe we'vot got a single variable built + # # with the Action liquid tag instead? + # @attributes_var_name = Regexp.last_match(1) + # end + + # if attributes.empty? && attributes_var_name.blank? + # raise ::Liquid::SyntaxError.new("Syntax Error in 'with_scope' - Valid syntax: with_scope : , ..., : ") + # end end def render(context) @@ -65,44 +204,74 @@ 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 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 + # puts [_key, evaluate_attribute(context, 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) + pp "evaluate_attribute = #{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 ::Liquid::VariableLookup + evaluated_value = context.evaluate(value) + evaluated_value.respond_to?(:_id) ? evaluated_value.send(:_source) : evaluate_attribute(context, evaluated_value) + when StrictRegexpFragment + create_regexp($1, $2) + else + 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 + + # # 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 + # end + # end + + # def cast_value(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 + # end + # end + def create_regexp(value, unparsed_options) options = unparsed_options.blank? ? nil : unparsed_options.split('').uniq.inject(0) do |_options, letter| _options |= REGEX_OPTIONS[letter] diff --git a/spec/unit/liquid/tags/with_scope_spec.rb b/spec/unit/liquid/tags/with_scope_spec.rb index 296b29c2..1fbf7c2d 100644 --- a/spec/unit/liquid/tags/with_scope_spec.rb +++ b/spec/unit/liquid/tags/with_scope_spec.rb @@ -54,7 +54,7 @@ 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 %}" } + let(:source) { "{% with_scope active: true, price: 41 + 1, title: 'foo', hidden: false %}{% assign conditions = with_scope %}{% endwith_scope %}" } it { expect(conditions['active']).to eq true } it { expect(conditions['price']).to eq 42 } @@ -147,9 +147,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 +156,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 From 724994559f732b3296b4ca3d550ee92e20dd51a0 Mon Sep 17 00:00:00 2001 From: Didier Lafforgue Date: Fri, 20 Sep 2024 14:59:13 +0200 Subject: [PATCH 2/4] pass all the with_scope specs + benchmarking (WIP) in order to improve parsing performances --- .../steam/liquid/tags/with_scope.rb | 58 +++++++++++++++---- 1 file changed, 46 insertions(+), 12 deletions(-) diff --git a/lib/locomotive/steam/liquid/tags/with_scope.rb b/lib/locomotive/steam/liquid/tags/with_scope.rb index 1680d787..ec6d0a2a 100644 --- a/lib/locomotive/steam/liquid/tags/with_scope.rb +++ b/lib/locomotive/steam/liquid/tags/with_scope.rb @@ -86,8 +86,8 @@ def on_deep_send(node) end def on_send(node) - pp node.location - pp node.location.expression.source + # pp node.location + # pp node.location.expression.source ::Liquid::Expression.parse(node.location.expression.source) @@ -136,7 +136,7 @@ def process(node) # 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\w*([a-zA-Z_0-9]+)\w*\z/om.freeze + SingleVariable = /\A\s*([a-zA-Z_0-9]+)\s*\z/om.freeze REGEX_OPTIONS = { 'i' => Regexp::IGNORECASE, @@ -151,24 +151,26 @@ def process(node) attr_reader :attributes, :attributes_var_name, :ast def initialize(tag_name, markup, options) + super # convert symbol operators into valid ruby code markup.gsub!(SYMBOL_OPERATORS_REGEXP, ':"\1" =>') - pp markup + # pp markup if markup =~ SingleVariable - puts "HERE?" + # puts "HERE?" # 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? - ast = Parser::CurrentRuby.parse("{%s}" % markup) - pp ast - @attributes = HashProcessor.new.process(ast) - puts "-----" - pp @attributes + # ast = Parser::CurrentRuby.parse("{%s}" % markup) + # pp ast + # @attributes = HashProcessor.new.process(ast) + # puts "-----" + # pp @attributes + @attributes = parse_markup(markup) end if attributes.blank? && attributes_var_name.blank? @@ -192,6 +194,7 @@ def initialize(tag_name, markup, options) end def render(context) + pp attributes if ENV['WITH_SCOPE_DEBUG'] context.stack do context['with_scope'] = self.evaluate_attributes(context) @@ -204,6 +207,38 @@ def render(context) protected + def parse_markup(markup) + # begin + parser = nil + ast = nil + source_buffer = nil + + puts "init:" + puts Benchmark.measure { parser = Parser::CurrentRuby.new } + # Silent the error instead of logging them to STDERR (default behavior of the parser) + + puts "consumer:" + puts Benchmark.measure { parser.diagnostics.consumer = ->(message) { true } } + + # 'with_scope.rb' is purely arbitrary + puts "source_buffer:" + puts Benchmark.measure { source_buffer = Parser::Source::Buffer.new('with_scope.rb') } + + source_buffer.source = "{%s}" % markup + + puts "ast: " + puts Benchmark.measure { ast = parser.parse(source_buffer) } + + foo = nil + puts "visit ast: " + puts Benchmark.measure { foo = HashProcessor.new.process(ast) } + foo + # rescue StandardError => e + # TODO: log something??? + # {} + # end + end + # def parse_attribute(value) # case value # when StrictRegexpFragment @@ -230,7 +265,7 @@ def evaluate_attributes(context) end def evaluate_attribute(context, value) - pp "evaluate_attribute = #{value}" + # pp "evaluate_attribute = #{value}" case value when Array value.map { |v| evaluate_attribute(context, v) } @@ -282,7 +317,6 @@ def create_regexp(value, unparsed_options) def tag_attributes_regexp TagAttributes end - end ::Liquid::Template.register_tag('with_scope'.freeze, WithScope) From 4e54eec4b70a7065eda5bc1103b2cebc1e28eeb7 Mon Sep 17 00:00:00 2001 From: Didier Lafforgue Date: Fri, 20 Sep 2024 19:27:49 +0200 Subject: [PATCH 3/4] improve the performance of the with_scope attributes parsing + use a different name of the simple version of the parser --- lib/locomotive/steam/liquid.rb | 2 +- .../liquid/tags/concerns/attributes_parser.rb | 142 +++++++++++ ...ributes.rb => simple_attributes_parser.rb} | 3 +- lib/locomotive/steam/liquid/tags/consume.rb | 2 +- .../steam/liquid/tags/editable/base.rb | 2 +- .../steam/liquid/tags/inherited_block.rb | 2 +- lib/locomotive/steam/liquid/tags/link_to.rb | 2 +- .../steam/liquid/tags/locale_switcher.rb | 3 +- .../steam/liquid/tags/model_form.rb | 2 +- lib/locomotive/steam/liquid/tags/paginate.rb | 2 +- lib/locomotive/steam/liquid/tags/path_to.rb | 2 +- .../steam/liquid/tags/redirect_to.rb | 2 +- lib/locomotive/steam/liquid/tags/section.rb | 2 +- .../steam/liquid/tags/with_scope.rb | 228 +----------------- spec/unit/liquid/tags/with_scope_spec.rb | 15 +- 15 files changed, 173 insertions(+), 238 deletions(-) create mode 100644 lib/locomotive/steam/liquid/tags/concerns/attributes_parser.rb rename lib/locomotive/steam/liquid/tags/concerns/{attributes.rb => simple_attributes_parser.rb} (96%) diff --git a/lib/locomotive/steam/liquid.rb b/lib/locomotive/steam/liquid.rb index 52f13b61..5d31bf51 100644 --- a/lib/locomotive/steam/liquid.rb +++ b/lib/locomotive/steam/liquid.rb @@ -9,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 ec6d0a2a..82ac9a0f 100644 --- a/lib/locomotive/steam/liquid/tags/with_scope.rb +++ b/lib/locomotive/steam/liquid/tags/with_scope.rb @@ -13,129 +13,14 @@ module Tags # {% endfor %} # {% endwith_scope %} # - - # s(:hash, - # s(:pair, - # s(:sym, :key), - # s(:array, - # s(:int, 1), - # s(:int, 2), - # s(:int, 3)))) class WithScope < ::Liquid::Block - class HashProcessor - 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) - # pp node.location - # pp node.location.expression.source - - ::Liquid::Expression.parse(node.location.expression.source) - - # raise 'TODO' - - # source_expr, name_expr = *node - - # if source_expr.nil? - # ::Liquid::Expression.parse(name_expr.to_s) - # 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 - - # TODO: create our own mixin - def process(node) - return if node.nil? + include Concerns::AttributesParser - 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 - - node = new_node unless new_node.nil? - - node - end - end - - include Concerns::Attributes - - # 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 = { @@ -148,53 +33,29 @@ def process(node) SYMBOL_OPERATORS_REGEXP = /(\w+\.(#{OPERATORS.join('|')})){1}\s*\:/o - attr_reader :attributes, :attributes_var_name, :ast + attr_reader :attributes, :attributes_var_name def initialize(tag_name, markup, options) - super # convert symbol operators into valid ruby code markup.gsub!(SYMBOL_OPERATORS_REGEXP, ':"\1" =>') - # pp markup - if markup =~ SingleVariable - # puts "HERE?" # 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? - # ast = Parser::CurrentRuby.parse("{%s}" % markup) - # pp ast - # @attributes = HashProcessor.new.process(ast) - # puts "-----" - # pp @attributes + # use our own Ruby parser @attributes = parse_markup(markup) end if attributes.blank? && attributes_var_name.blank? raise ::Liquid::SyntaxError.new("Syntax Error in 'with_scope' - Valid syntax: with_scope : , ..., : ") end - - # raise 'TODO' - - # # simple hash? - # parse_attributes(markup) { |value| parse_attribute(value) } - - # if attributes.empty? && markup =~ SingleVariable - # # alright, maybe we'vot got a single variable built - # # with the Action liquid tag instead? - # @attributes_var_name = Regexp.last_match(1) - # end - - # if attributes.empty? && attributes_var_name.blank? - # raise ::Liquid::SyntaxError.new("Syntax Error in 'with_scope' - Valid syntax: with_scope : , ..., : ") - # end end def render(context) - pp attributes if ENV['WITH_SCOPE_DEBUG'] context.stack do context['with_scope'] = self.evaluate_attributes(context) @@ -207,50 +68,6 @@ def render(context) protected - def parse_markup(markup) - # begin - parser = nil - ast = nil - source_buffer = nil - - puts "init:" - puts Benchmark.measure { parser = Parser::CurrentRuby.new } - # Silent the error instead of logging them to STDERR (default behavior of the parser) - - puts "consumer:" - puts Benchmark.measure { parser.diagnostics.consumer = ->(message) { true } } - - # 'with_scope.rb' is purely arbitrary - puts "source_buffer:" - puts Benchmark.measure { source_buffer = Parser::Source::Buffer.new('with_scope.rb') } - - source_buffer.source = "{%s}" % markup - - puts "ast: " - puts Benchmark.measure { ast = parser.parse(source_buffer) } - - foo = nil - puts "visit ast: " - puts Benchmark.measure { foo = HashProcessor.new.process(ast) } - foo - # rescue StandardError => e - # TODO: log something??? - # {} - # end - end - - # 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? @@ -258,65 +75,32 @@ def evaluate_attributes(context) # _slug instead of _permalink _key = key.to_s == '_permalink' ? '_slug' : key.to_s - # puts [_key, evaluate_attribute(context, value)] - memo.merge({ _key => evaluate_attribute(context, value) }) end end def evaluate_attribute(context, value) - # pp "evaluate_attribute = #{value}" case 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) - when StrictRegexpFragment - create_regexp($1, $2) else 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 - - # # 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 - # end - # end - - # def cast_value(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 - # end - # end - def create_regexp(value, unparsed_options) options = unparsed_options.blank? ? nil : unparsed_options.split('').uniq.inject(0) do |_options, letter| _options |= REGEX_OPTIONS[letter] end Regexp.new(value, options) end - - def tag_attributes_regexp - TagAttributes - end end ::Liquid::Template.register_tag('with_scope'.freeze, WithScope) diff --git a/spec/unit/liquid/tags/with_scope_spec.rb b/spec/unit/liquid/tags/with_scope_spec.rb index 1fbf7c2d..a1482522 100644 --- a/spec/unit/liquid/tags/with_scope_spec.rb +++ b/spec/unit/liquid/tags/with_scope_spec.rb @@ -52,9 +52,20 @@ 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: 41 + 1, title: 'foo', hidden: false %}{% assign conditions = with_scope %}{% endwith_scope %}" } + let(:source) { "{% with_scope active: true, price: 42, title: 'foo', hidden: false %}{% assign conditions = with_scope %}{% endwith_scope %}" } it { expect(conditions['active']).to eq true } it { expect(conditions['price']).to eq 42 } @@ -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' } From 50c068b493042179e4ae5203d678b4f1e3ca09c8 Mon Sep 17 00:00:00 2001 From: Didier Lafforgue Date: Fri, 20 Sep 2024 23:23:19 +0200 Subject: [PATCH 4/4] set a minimal Ruby version (3.2) --- .github/workflows/continuous_integration.yml | 2 +- Gemfile | 4 +--- Gemfile.lock | 4 +--- locomotivecms_steam.gemspec | 3 ++- 4 files changed, 5 insertions(+), 8 deletions(-) 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 5fff49ef..9b5f48f5 100644 --- a/Gemfile +++ b/Gemfile @@ -2,8 +2,6 @@ source 'https://rubygems.org' gemspec -gem 'parser' - group :development do # gem 'locomotivecms_common', github: 'locomotivecms/common', ref: '4d1bd56' # gem 'locomotivecms_common', path: '../common' @@ -34,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 b5cd0b3a..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) @@ -121,8 +122,6 @@ 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) @@ -194,7 +193,6 @@ DEPENDENCIES memory_profiler mongo (~> 2.18.2) origin (~> 2.3.1) - parser puma (~> 6.4.0) rack (~> 3.0) rack-mini-profiler (~> 0.10.1) 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