frontpage and it will show up here.- {{ "Learn more about handles" | link_to "http://wiki.shopify.com/Handle" }} + {{ "Learn more about handles" | link_to: "http://wiki.shopify.com/Handle" }}
diff --git a/README.md b/README.md index 80716815..481f18e6 100644 --- a/README.md +++ b/README.md @@ -48,4 +48,28 @@ For standard use you can just pass it the content of a file and call render with @template.render('name' => 'tobi') # => "hi tobi" ``` +### Error Modes + +Setting the error mode of Liquid lets you specify how strictly you want your templates to be interpreted. +Normally the parser is very lax and will accept almost anything without error. Unfortunately this can make +it very hard to debug and can lead to unexpected behaviour. + +Liquid also comes with a stricter parser that can be used when editing templates to give better error messages +when templates are invalid. You can enable this new parser like this: + +```ruby +Liquid::Template.error_mode = :strict # Raises a SyntaxError when invalid syntax is used +Liquid::Template.error_mode = :warn # Adds errors to template.errors but continues as normal +Liquid::Template.error_mode = :lax # The default mode, accepts almost anything. +``` + +If you want to set the error mode only on specific templates you can pass `:error_mode` as an option to `parse`: +```ruby +Liquid::Template.parse(source, :error_mode => :strict) +``` +This is useful for doing things like enabling strict mode only in the theme editor. + +It is recommended that you enable `:strict` or `:warn` mode on new apps to stop invalid templates from being created. +It is also recommended that you use it in the template editors of existing apps to give editors better error messages. + [](http://travis-ci.org/Shopify/liquid) diff --git a/Rakefile b/Rakefile index 862e80f5..137b8e3a 100755 --- a/Rakefile +++ b/Rakefile @@ -7,12 +7,28 @@ require 'rubygems/package_task' task :default => 'test' -Rake::TestTask.new(:test) do |t| +desc 'run test suite with default parser' +Rake::TestTask.new(:base_test) do |t| t.libs << '.' << 'lib' << 'test' t.test_files = FileList['test/liquid/**/*_test.rb'] t.verbose = false end +desc 'run test suite with warn error mode' +task :warn_test do + ENV['LIQUID_PARSER_MODE'] = 'warn' + Rake::Task['base_test'].invoke +end + +desc 'runs test suite with both strict and lax parsers' +task :test do + ENV['LIQUID_PARSER_MODE'] = 'lax' + Rake::Task['base_test'].invoke + ENV['LIQUID_PARSER_MODE'] = 'strict' + Rake::Task['base_test'].reenable + Rake::Task['base_test'].invoke +end + gemspec = eval(File.read('liquid.gemspec')) Gem::PackageTask.new(gemspec) do |pkg| pkg.gem_spec = gemspec @@ -25,11 +41,15 @@ end namespace :benchmark do - desc "Run the liquid benchmark" + desc "Run the liquid benchmark with lax parsing" task :run do - ruby "./performance/benchmark.rb" + ruby "./performance/benchmark.rb lax" end + desc "Run the liquid benchmark with strict parsing" + task :strict do + ruby "./performance/benchmark.rb strict" + end end diff --git a/lib/liquid.rb b/lib/liquid.rb index cb84a6ee..62b1c80d 100644 --- a/lib/liquid.rb +++ b/lib/liquid.rb @@ -46,6 +46,8 @@ module Liquid end require "liquid/version" +require 'liquid/lexer' +require 'liquid/parser' require 'liquid/drop' require 'liquid/extensions' require 'liquid/errors' diff --git a/lib/liquid/block.rb b/lib/liquid/block.rb index 642bfb3e..a5c3b1e4 100644 --- a/lib/liquid/block.rb +++ b/lib/liquid/block.rb @@ -14,6 +14,9 @@ module Liquid @nodelist ||= [] @nodelist.clear + # All child tags of the current block. + @children = [] + while token = tokens.shift case token when IsTag @@ -28,9 +31,10 @@ module Liquid # fetch the tag from registered blocks if tag = Template.tags[$1] - new_tag = tag.new($1, $2, tokens) + new_tag = tag.new_with_options($1, $2, tokens, @options || {}) @blank &&= new_tag.blank? @nodelist << new_tag + @children << new_tag else # this tag is not registered with the system # pass it to the current block for special handling or error reporting @@ -40,7 +44,9 @@ module Liquid raise SyntaxError, "Tag '#{token}' was not properly terminated with regexp: #{TagEnd.inspect} " end when IsVariable - @nodelist << create_variable(token) + new_var = create_variable(token) + @nodelist << new_var + @children << new_var @blank = false when '' # pass @@ -56,6 +62,18 @@ module Liquid assert_missing_delimitation! end + # warnings of this block and all sub-tags + def warnings + all_warnings = [] + all_warnings.concat(@warnings) if @warnings + + @children.each do |node| + all_warnings.concat(node.warnings || []) + end + + all_warnings + end + def end_tag end @@ -80,7 +98,7 @@ module Liquid def create_variable(token) token.scan(ContentOfVariable) do |content| - return Variable.new(content.first) + return Variable.new(content.first, @options) end raise SyntaxError.new("Variable '#{token}' was not properly terminated with regexp: #{VariableEnd.inspect} ") end diff --git a/lib/liquid/document.rb b/lib/liquid/document.rb index a1287629..b802c859 100644 --- a/lib/liquid/document.rb +++ b/lib/liquid/document.rb @@ -1,7 +1,8 @@ module Liquid class Document < Block # we don't need markup to open this block - def initialize(tokens) + def initialize(tokens, options = {}) + @options = options parse(tokens) end diff --git a/lib/liquid/lexer.rb b/lib/liquid/lexer.rb new file mode 100644 index 00000000..32991c9f --- /dev/null +++ b/lib/liquid/lexer.rb @@ -0,0 +1,49 @@ +require "strscan" +module Liquid + class Lexer + SPECIALS = { + '|' => :pipe, + '.' => :dot, + ':' => :colon, + ',' => :comma, + '[' => :open_square, + ']' => :close_square, + '(' => :open_round, + ')' => :close_round + } + IDENTIFIER = /[\w\-?!]+/ + SINGLE_STRING_LITERAL = /'[^\']*'/ + DOUBLE_STRING_LITERAL = /"[^\"]*"/ + NUMBER_LITERAL = /-?\d+(\.\d+)?/ + COMPARISON_OPERATOR = /==|!=|<>|<=?|>=?|contains/ + + def initialize(input) + @ss = StringScanner.new(input.rstrip) + end + + def tokenize + @output = [] + + while !@ss.eos? + @ss.skip(/\s*/) + tok = case + when t = @ss.scan(COMPARISON_OPERATOR) then [:comparison, t] + when t = @ss.scan(SINGLE_STRING_LITERAL) then [:string, t] + when t = @ss.scan(DOUBLE_STRING_LITERAL) then [:string, t] + when t = @ss.scan(NUMBER_LITERAL) then [:number, t] + when t = @ss.scan(IDENTIFIER) then [:id, t] + else + c = @ss.getch + if s = SPECIALS[c] + [s,c] + else + raise SyntaxError, "Unexpected character #{c}" + end + end + @output << tok + end + + @output << [:end_of_string] + end + end +end diff --git a/lib/liquid/parser.rb b/lib/liquid/parser.rb new file mode 100644 index 00000000..96260018 --- /dev/null +++ b/lib/liquid/parser.rb @@ -0,0 +1,90 @@ +module Liquid + class Parser + def initialize(input) + l = Lexer.new(input) + @tokens = l.tokenize + @p = 0 # pointer to current location + end + + def jump(point) + @p = point + end + + def consume(type = nil) + token = @tokens[@p] + if type && token[0] != type + raise SyntaxError, "Expected #{type} but found #{@tokens[@p].first}" + end + @p += 1 + token[1] + end + + # Only consumes the token if it matches the type + # Returns the token's contents if it was consumed + # or false otherwise. + def consume?(type) + token = @tokens[@p] + return false unless token && token[0] == type + @p += 1 + token[1] + end + + # Like consume? Except for an :id token of a certain name + def id?(str) + token = @tokens[@p] + return false unless token && token[0] == :id + return false unless token[1] == str + @p += 1 + token[1] + end + + def look(type, ahead = 0) + tok = @tokens[@p + ahead] + return false unless tok + tok[0] == type + end + + def expression + token = @tokens[@p] + if token[0] == :id + variable_signature + elsif [:string, :number].include? token[0] + consume + elsif token.first == :open_round + consume + first = expression + consume(:dot) + consume(:dot) + last = expression + consume(:close_round) + "(#{first}..#{last})" + 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 + + str << expression + end + + def variable_signature + str = consume(:id) + if look(:open_square) + str << consume + str << expression + str << consume(:close_square) + end + if look(:dot) + str << consume + str << variable_signature + end + str + end + end +end diff --git a/lib/liquid/tag.rb b/lib/liquid/tag.rb index ba04ba61..c1195622 100644 --- a/lib/liquid/tag.rb +++ b/lib/liquid/tag.rb @@ -1,10 +1,21 @@ module Liquid class Tag - attr_accessor :nodelist + attr_accessor :nodelist, :options + attr_reader :warnings + + def self.new_with_options(tag_name, markup, tokens, options) + # Forgive me Matz for I have sinned. I know this code is weird + # but it was necessary to maintain API compatibility. + new_tag = self.allocate + new_tag.options = options + new_tag.send(:initialize, tag_name, markup, tokens) + new_tag + end def initialize(tag_name, markup, tokens) @tag_name = tag_name @markup = markup + @options ||= {} # needs || because might be set before initialize parse(tokens) end @@ -22,5 +33,28 @@ module Liquid def blank? @blank || true end + + def parse_with_selected_parser(markup) + case @options[:error_mode] || Template.error_mode + when :strict then strict_parse_with_error_context(markup) + when :lax then lax_parse(markup) + when :warn + begin + return strict_parse_with_error_context(markup) + rescue SyntaxError => e + @warnings ||= [] + @warnings << e + return lax_parse(markup) + end + end + end + + private + def strict_parse_with_error_context(markup) + strict_parse(markup) + rescue SyntaxError => e + e.message << " in \"#{markup.strip}\"" + raise e + end end # Tag end # Liquid diff --git a/lib/liquid/tags/for.rb b/lib/liquid/tags/for.rb index b752b219..69fc9d33 100644 --- a/lib/liquid/tags/for.rb +++ b/lib/liquid/tags/for.rb @@ -47,19 +47,7 @@ module Liquid Syntax = /\A(#{VariableSegment}+)\s+in\s+(#{QuotedFragment}+)\s*(reversed)?/o def initialize(tag_name, markup, tokens) - if markup =~ Syntax - @variable_name = $1 - @collection_name = $2 - @name = "#{$1}-#{$2}" - @reversed = $3 - @attributes = {} - markup.scan(TagAttributes) do |key, value| - @attributes[key] = value - end - else - raise SyntaxError.new("Syntax Error in 'for loop' - Valid syntax: for [item] in [collection]") - end - + parse_with_selected_parser(markup) @nodelist = @for_block = [] super end @@ -127,6 +115,43 @@ module Liquid result end + protected + + def lax_parse(markup) + if markup =~ Syntax + @variable_name = $1 + @collection_name = $2 + @name = "#{$1}-#{$2}" + @reversed = $3 + @attributes = {} + markup.scan(TagAttributes) do |key, value| + @attributes[key] = value + end + else + raise SyntaxError.new("Syntax Error in 'for loop' - Valid syntax: for [item] in [collection]") + end + end + + def strict_parse(markup) + p = Parser.new(markup) + @variable_name = p.consume(:id) + raise SyntaxError, "For loops require an 'in' clause" unless p.id?('in') + @collection_name = p.expression + @name = "#{@variable_name}-#{@collection_name}" + @reversed = p.id?('reversed') + + @attributes = {} + while p.look(:id) && p.look(:colon, 1) + unless attribute = p.id?('limit') || p.id?('offset') + raise SyntaxError, "Invalid attribute in for loop. Valid attributes are limit and offset" + end + p.consume + val = p.expression + @attributes[attribute] = val + end + p.consume(:end_of_string) + end + private def render_else(context) diff --git a/lib/liquid/tags/if.rb b/lib/liquid/tags/if.rb index c7b55beb..ac1767d3 100644 --- a/lib/liquid/tags/if.rb +++ b/lib/liquid/tags/if.rb @@ -45,28 +45,56 @@ module Liquid block = if tag == 'else' ElseCondition.new else - - expressions = markup.scan(ExpressionsAndOperators).reverse - raise(SyntaxError, SyntaxHelp) unless expressions.shift =~ Syntax - - condition = Condition.new($1, $2, $3) - - while not expressions.empty? - operator = (expressions.shift).to_s.strip - - raise(SyntaxError, SyntaxHelp) unless expressions.shift.to_s =~ Syntax - - new_condition = Condition.new($1, $2, $3) - new_condition.send(operator.to_sym, condition) - condition = new_condition - end - - condition + parse_with_selected_parser(markup) end @blocks.push(block) @nodelist = block.attach(Array.new) end + + def lax_parse(markup) + expressions = markup.scan(ExpressionsAndOperators).reverse + raise(SyntaxError, SyntaxHelp) unless expressions.shift =~ Syntax + + condition = Condition.new($1, $2, $3) + + while not expressions.empty? + operator = (expressions.shift).to_s.strip + + raise(SyntaxError, SyntaxHelp) unless expressions.shift.to_s =~ Syntax + + new_condition = Condition.new($1, $2, $3) + new_condition.send(operator.to_sym, condition) + condition = new_condition + end + + condition + end + + def strict_parse(markup) + p = Parser.new(markup) + + condition = parse_comparison(p) + + while op = (p.id?('and') || p.id?('or')) + new_cond = parse_comparison(p) + new_cond.send(op, condition) + condition = new_cond + end + p.consume(:end_of_string) + + condition + end + + def parse_comparison(p) + a = p.expression + if op = p.consume?(:comparison) + b = p.expression + Condition.new(a, op, b) + else + Condition.new(a) + end + end end Template.register_tag('if', If) diff --git a/lib/liquid/template.rb b/lib/liquid/template.rb index 5e0675fe..87f1491c 100644 --- a/lib/liquid/template.rb +++ b/lib/liquid/template.rb @@ -34,6 +34,18 @@ module Liquid @tags ||= {} end + # Sets how strict the parser should be. + # :lax acts like liquid 2.5 and silently ignores malformed tags in most cases. + # :warn is the default and will give deprecation warnings when invalid syntax is used. + # :strict will enforce correct syntax. + def error_mode=(mode) + @error_mode = mode + end + + def error_mode + @error_mode || :lax + end + # Pass a module with filter methods which should be available # to all liquid views. Good for registering the standard library def register_filter(mod) @@ -41,9 +53,9 @@ module Liquid end # creates a new Template object from liquid source code - def parse(source) + def parse(source, options = {}) template = Template.new - template.parse(source) + template.parse(source, options) template end end @@ -55,11 +67,17 @@ module Liquid # Parse source code. # Returns self for easy chaining - def parse(source) - @root = Document.new(tokenize(source)) + def parse(source, options = {}) + @root = Document.new(tokenize(source), options) + @warnings = nil self end + def warnings + return [] unless @root + @warnings ||= @root.warnings + end + def registers @registers ||= {} end diff --git a/lib/liquid/variable.rb b/lib/liquid/variable.rb index 883e3eaa..7ad60feb 100644 --- a/lib/liquid/variable.rb +++ b/lib/liquid/variable.rb @@ -12,11 +12,30 @@ module Liquid # class Variable FilterParser = /(?:#{FilterSeparator}|(?:\s*(?:#{QuotedFragment}|#{ArgumentSeparator})\s*)+)/o - attr_accessor :filters, :name + EasyParse = /^ *(\w+(?:\.\w+)*) *$/ + attr_accessor :filters, :name, :warnings - def initialize(markup) + def initialize(markup, options = {}) @markup = markup @name = nil + @options = options || {} + + + case @options[:error_mode] || Template.error_mode + when :strict then strict_parse(markup) + when :lax then lax_parse(markup) + when :warn + begin + strict_parse(markup) + rescue SyntaxError => e + @warnings ||= [] + @warnings << e + lax_parse(markup) + end + end + end + + def lax_parse(markup) @filters = [] if match = markup.match(/\s*(#{QuotedFragment})(.*)/o) @name = match[1] @@ -33,6 +52,39 @@ module Liquid end end + def strict_parse(markup) + # Very simple valid cases + if markup =~ EasyParse + @name = $1 + @filters = [] + return + end + + @filters = [] + p = Parser.new(markup) + # Could be just filters with no input + @name = p.look(:pipe) ? '' : p.expression + while p.consume?(:pipe) + filtername = p.consume(:id) + filterargs = p.consume?(:colon) ? parse_filterargs(p) : [] + @filters << [filtername, filterargs] + end + p.consume(:end_of_string) + rescue SyntaxError => e + e.message << " in \"{{#{markup}}}\"" + raise e + end + + def parse_filterargs(p) + # first argument + filterargs = [p.argument] + # followed by comma separated others + while p.consume?(:comma) + filterargs << p.argument + end + filterargs + end + def render(context) return '' if @name.nil? @filters.inject(context[@name]) do |output, filter| diff --git a/performance/benchmark.rb b/performance/benchmark.rb index afb6ffab..d206c65e 100644 --- a/performance/benchmark.rb +++ b/performance/benchmark.rb @@ -2,6 +2,7 @@ require 'rubygems' require 'benchmark' require File.dirname(__FILE__) + '/theme_runner' +Liquid::Template.error_mode = ARGV.first.to_sym if ARGV.first profiler = ThemeRunner.new Benchmark.bmbm do |x| diff --git a/performance/profile.rb b/performance/profile.rb index c07be5c5..017435ff 100644 --- a/performance/profile.rb +++ b/performance/profile.rb @@ -6,14 +6,14 @@ profiler = ThemeRunner.new puts 'Running profiler...' -results = profiler.run +results = profiler.run_profile puts 'Success' puts -[RubyProf::FlatPrinter, RubyProf::GraphPrinter, RubyProf::GraphHtmlPrinter, RubyProf::CallTreePrinter].each do |klass| +[RubyProf::FlatPrinter, RubyProf::GraphHtmlPrinter, RubyProf::CallTreePrinter, RubyProf::DotPrinter].each do |klass| filename = (ENV['TMP'] || '/tmp') + (klass.name.include?('Html') ? "/liquid.#{klass.name.downcase}.html" : "/callgrind.liquid.#{klass.name.downcase}.txt") filename.gsub!(/:+/, '_') - File.open(filename, "w+") { |fp| klass.new(results).print(fp, :print_file => true) } + File.open(filename, "w+") { |fp| klass.new(results).print(fp, :print_file => true, :min_percent => 3) } $stderr.puts "wrote #{klass.name} output to #{filename}" end diff --git a/performance/tests/dropify/index.liquid b/performance/tests/dropify/index.liquid index cbc12bb2..f1ea1376 100644 --- a/performance/tests/dropify/index.liquid +++ b/performance/tests/dropify/index.liquid @@ -28,7 +28,7 @@ {% else %}
frontpage and it will show up here.frontpage and it will show up here.frontpage and it will show up here.