diff --git a/History.md b/History.md index 94feaabfd..8fd6ef372 100644 --- a/History.md +++ b/History.md @@ -1,5 +1,38 @@ # Liquid Change Log +## 6.0.0 + +### Architectural changes + +### Features +* (TODO) Add support for boolean expressions everywhere + * As variable output `{{ a or b }}` + * As filter argument `{{ collection | where: 'prop', a or b }}` + * As tag argument `{% render 'snip', enabled: a or b %}` + * As conditional tag argument `{% if cond %}` (extending previous behaviour) +* (TODO) Add support for subexpression prioritization and associativity + * In ascending order of priority: + * Logical: `and`, `or` (right to left) + * Equality: `==`, `!=`, `<>` (left to right) + * Comparison: `>`, `>=`, `<`, `<=`, `contains` (left to right) + - For example, this is now supported + * `{{ a > b == c < d or e == f }}` which is equivalent to + * `{{ ((a > b) == (c < d)) or (e == f) }}` +- (TODO) Add support for parenthesized expressions + * e.g. `(a or b) and c` + +### Breaking changes +* The Environment's `error_mode` option has been removed. + * `:warn` is no longer supported + * `:lax` and `lax_parse` is no longer supported + * `:strict` and `strict_parse` is no longer supported + * `strict2_parse` is renamed to `parse_markup` +* The `warnings` system has been removed. + +### Migrating from `^5.11.0` +- In custom tags that include `ParserSwitching`, rename `strict2_parse` to `parse_markup` +- Remove code depending on `:error_mode` + ## 5.11.0 * Revert the Inline Snippets tag (#2001), treat its inclusion in the latest Liquid release as a bug, and allow for feedback on RFC#1916 to better support Liquid developers [Guilherme Carreiro] * Rename the `:rigid` error mode to `:strict2` and display a warning when users attempt to use the `:rigid` mode [Guilherme Carreiro] diff --git a/README.md b/README.md index 5066dc659..b3745e024 100644 --- a/README.md +++ b/README.md @@ -93,31 +93,6 @@ LIQUID By using Environments, you ensure that custom tags and filters are only available in the contexts where they are needed, making your Liquid templates more robust and easier to manage. For smaller projects, a global environment is available via `Liquid::Environment.default`. -### 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 different parsers 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::Environment.default.error_mode = :strict2 # Raises a SyntaxError when invalid syntax is used in all tags -Liquid::Environment.default.error_mode = :strict # Raises a SyntaxError when invalid syntax is used in some tags -Liquid::Environment.default.error_mode = :warn # Adds strict errors to template.errors but continues as normal -Liquid::Environment.default.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. - ### Undefined variables and filters By default, the renderer doesn't raise or in any other way notify you if some variables or filters are missing, i.e. not passed to the `render` method. diff --git a/Rakefile b/Rakefile index 83afcbfa0..086a5a81e 100755 --- a/Rakefile +++ b/Rakefile @@ -33,29 +33,12 @@ task :rubocop do end end -desc('runs test suite with lax, strict, and strict2 parsers') +desc('runs test suite') 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 - - ENV['LIQUID_PARSER_MODE'] = 'strict2' Rake::Task['base_test'].reenable Rake::Task['base_test'].invoke if RUBY_ENGINE == 'ruby' || RUBY_ENGINE == 'truffleruby' - ENV['LIQUID_PARSER_MODE'] = 'lax' - Rake::Task['integration_test'].reenable - Rake::Task['integration_test'].invoke - - ENV['LIQUID_PARSER_MODE'] = 'strict' - Rake::Task['integration_test'].reenable - Rake::Task['integration_test'].invoke - - ENV['LIQUID_PARSER_MODE'] = 'strict2' Rake::Task['integration_test'].reenable Rake::Task['integration_test'].invoke end @@ -78,24 +61,11 @@ task release: :build do end namespace :benchmark do - desc "Run the liquid benchmark with lax parsing" - task :lax do - ruby "./performance/benchmark.rb lax" - end - - desc "Run the liquid benchmark with strict parsing" - task :strict do - ruby "./performance/benchmark.rb strict" - end - - desc "Run the liquid benchmark with strict2 parsing" - task :strict2 do - ruby "./performance/benchmark.rb strict2" + desc "Run the liquid benchmark" + task :run do + ruby "./performance/benchmark.rb" end - desc "Run the liquid benchmark with lax, strict, and strict2 parsing" - task run: [:lax, :strict, :strict2] - desc "Run unit benchmarks" namespace :unit do task :all do @@ -126,11 +96,6 @@ namespace :profile do task :run do ruby "./performance/profile.rb" end - - desc "Run the liquid profile/performance coverage with strict parsing" - task :strict do - ruby "./performance/profile.rb strict" - end end namespace :memory_profile do diff --git a/lib/liquid/context.rb b/lib/liquid/context.rb index 433b6d003..1db6bb663 100644 --- a/lib/liquid/context.rb +++ b/lib/liquid/context.rb @@ -60,10 +60,6 @@ def initialize(environments = {}, outer_scope = {}, registers = {}, rethrow_erro end # rubocop:enable Metrics/ParameterLists - def warnings - @warnings ||= [] - end - def strainer @strainer ||= @environment.create_strainer(self, @filters) end @@ -157,7 +153,6 @@ def new_isolated_subcontext subcontext.filters = @filters subcontext.strainer = nil subcontext.errors = errors - subcontext.warnings = warnings subcontext.disabled_tags = @disabled_tags end end @@ -244,7 +239,7 @@ def tag_disabled?(tag_name) protected - attr_writer :base_scope_depth, :warnings, :errors, :strainer, :filters, :disabled_tags + attr_writer :base_scope_depth, :errors, :strainer, :filters, :disabled_tags private diff --git a/lib/liquid/environment.rb b/lib/liquid/environment.rb index 100bd0bbd..7834f49c3 100644 --- a/lib/liquid/environment.rb +++ b/lib/liquid/environment.rb @@ -4,10 +4,6 @@ module Liquid # The Environment is the container for all configuration options of Liquid, such as # the registered tags, filters, and the default error mode. class Environment - # The default error mode for all templates. This can be overridden on a - # per-template basis. - attr_accessor :error_mode - # The tags that are available to use in the template. attr_accessor :tags @@ -33,17 +29,14 @@ class << self # the template. # @param file_system The default file system that is used # to load templates from. - # @param error_mode [Symbol] The default error mode for all templates - # (either :strict2, :strict, :warn, or :lax). # @param exception_renderer [Proc] The exception renderer that is used to # render exceptions. # @yieldparam environment [Environment] The environment instance that is being built. # @return [Environment] The new environment instance. - def build(tags: nil, file_system: nil, error_mode: nil, exception_renderer: nil) + def build(tags: nil, file_system: nil, exception_renderer: nil) ret = new ret.tags = tags if tags ret.file_system = file_system if file_system - ret.error_mode = error_mode if error_mode ret.exception_renderer = exception_renderer if exception_renderer yield ret if block_given? ret.freeze @@ -75,7 +68,6 @@ def dangerously_override(environment) # @api private def initialize @tags = Tags::STANDARD_TAGS.dup - @error_mode = :lax @strainer_template = Class.new(StrainerTemplate).tap do |klass| klass.add_filter(StandardFilters) end diff --git a/lib/liquid/expression.rb b/lib/liquid/expression.rb index 2605c5576..3f9f08837 100644 --- a/lib/liquid/expression.rb +++ b/lib/liquid/expression.rb @@ -11,9 +11,6 @@ class Expression 'false' => false, 'blank' => '', 'empty' => '', - # in lax mode, minus sign can be a VariableLookup - # For simplicity and performace, we treat it like a literal - '-' => VariableLookup.parse("-", nil).freeze, }.freeze DOT = ".".ord diff --git a/lib/liquid/parse_context.rb b/lib/liquid/parse_context.rb index 855acc64e..96413eb61 100644 --- a/lib/liquid/parse_context.rb +++ b/lib/liquid/parse_context.rb @@ -3,14 +3,13 @@ module Liquid class ParseContext attr_accessor :locale, :line_number, :trim_whitespace, :depth - attr_reader :partial, :warnings, :error_mode, :environment + attr_reader :partial, :environment def initialize(options = Const::EMPTY_HASH) @environment = options.fetch(:environment, Environment.default) @template_options = options ? options.dup : {} - @locale = @template_options[:locale] ||= I18n.new - @warnings = [] + @locale = @template_options[:locale] ||= I18n.new # constructing new StringScanner in Lexer, Tokenizer, etc is expensive # This StringScanner will be shared by all of them @@ -55,16 +54,12 @@ def safe_parse_expression(parser) end def parse_expression(markup, safe: false) - if !safe && @error_mode == :strict2 - # parse_expression is a widely used API. To maintain backward - # compatibility while raising awareness about strict2 parser standards, - # the safe flag supports API users make a deliberate decision. - # - # In strict2 mode, markup MUST come from a string returned by the parser - # (e.g., parser.expression). We're not calling the parser here to - # prevent redundant parser overhead. - raise Liquid::InternalError, "unsafe parse_expression cannot be used in strict2 mode" - end + # markup MUST come from a string returned by the parser + # (e.g., parser.expression). We're not calling the parser here to + # prevent redundant parser overhead. The `safe` opt-in + # exists to ensure it is not accidentally still called with + # the result of a regex. + raise Liquid::InternalError, "unsafe parse_expression cannot be used" unless safe Expression.parse(markup, @string_scanner, @expression_cache) end @@ -72,8 +67,6 @@ def parse_expression(markup, safe: false) def partial=(value) @partial = value @options = value ? partial_options : @template_options - - @error_mode = @options[:error_mode] || @environment.error_mode end def partial_options diff --git a/lib/liquid/parser_switching.rb b/lib/liquid/parser_switching.rb index e419dc999..ca4267668 100644 --- a/lib/liquid/parser_switching.rb +++ b/lib/liquid/parser_switching.rb @@ -2,74 +2,15 @@ module Liquid module ParserSwitching - # Do not use this. - # - # It's basically doing the same thing the {#parse_with_selected_parser}, - # except this will try the strict parser regardless of the error mode, - # and fall back to the lax parser if the error mode is lax or warn, - # except when in strict2 mode where it uses the strict2 parser. - # - # @deprecated Use {#parse_with_selected_parser} instead. - def strict_parse_with_error_mode_fallback(markup) - return strict2_parse_with_error_context(markup) if strict2_mode? - - strict_parse_with_error_context(markup) - rescue SyntaxError => e - case parse_context.error_mode - when :rigid - rigid_warn - raise - when :strict2 - raise - when :strict - raise - when :warn - parse_context.warnings << e - end - lax_parse(markup) - end - def parse_with_selected_parser(markup) - case parse_context.error_mode - when :rigid then rigid_warn && strict2_parse_with_error_context(markup) - when :strict2 then strict2_parse_with_error_context(markup) - when :strict then strict_parse_with_error_context(markup) - when :lax then lax_parse(markup) - when :warn - begin - strict2_parse_with_error_context(markup) - rescue SyntaxError => e - parse_context.warnings << e - lax_parse(markup) - end - end - end - - def strict2_mode? - parse_context.error_mode == :strict2 || parse_context.error_mode == :rigid - end - - private - - def rigid_warn - Deprecations.warn(':rigid', ':strict2') - end - - def strict2_parse_with_error_context(markup) - strict2_parse(markup) + parse_markup(markup) rescue SyntaxError => e e.line_number = line_number e.markup_context = markup_context(markup) raise e end - def strict_parse_with_error_context(markup) - strict_parse(markup) - rescue SyntaxError => e - e.line_number = line_number - e.markup_context = markup_context(markup) - raise e - end + private def markup_context(markup) "in \"#{markup.strip}\"" diff --git a/lib/liquid/partial_cache.rb b/lib/liquid/partial_cache.rb index f49d14d90..5cca4e04b 100644 --- a/lib/liquid/partial_cache.rb +++ b/lib/liquid/partial_cache.rb @@ -4,7 +4,7 @@ module Liquid class PartialCache def self.load(template_name, context:, parse_context:) cached_partials = context.registers[:cached_partials] - cache_key = "#{template_name}:#{parse_context.error_mode}" + cache_key = template_name.to_s cached = cached_partials[cache_key] return cached if cached diff --git a/lib/liquid/tags/case.rb b/lib/liquid/tags/case.rb index b32ea1ae7..0a466c44c 100644 --- a/lib/liquid/tags/case.rb +++ b/lib/liquid/tags/case.rb @@ -23,9 +23,6 @@ module Liquid # @liquid_syntax_keyword second_expression An expression to be rendered when the variable's value matches `second_value`. # @liquid_syntax_keyword third_expression An expression to be rendered when the variable's value has no match. class Case < Block - Syntax = /(#{QuotedFragment})/o - WhenSyntax = /(#{QuotedFragment})(?:(?:\s+or\s+|\s*\,\s*)(#{QuotedFragment}.*))?/om - attr_reader :blocks, :left def initialize(tag_name, markup, options) @@ -86,35 +83,19 @@ def render_to_output_buffer(context, output) private - def strict2_parse(markup) + def parse_markup(markup) parser = @parse_context.new_parser(markup) @left = safe_parse_expression(parser) parser.consume(:end_of_string) end - def strict_parse(markup) - lax_parse(markup) - end - - def lax_parse(markup) - if markup =~ Syntax - @left = parse_expression(Regexp.last_match(1)) - else - raise SyntaxError, options[:locale].t("errors.syntax.case") - end - end - def record_when_condition(markup) body = new_body - if strict2_mode? - parse_strict2_when(markup, body) - else - parse_lax_when(markup, body) - end + parse_when(markup, body) end - def parse_strict2_when(markup, body) + def parse_when(markup, body) parser = @parse_context.new_parser(markup) loop do @@ -129,20 +110,6 @@ def parse_strict2_when(markup, body) parser.consume(:end_of_string) end - def parse_lax_when(markup, body) - while markup - unless markup =~ WhenSyntax - raise SyntaxError, options[:locale].t("errors.syntax.case_invalid_when") - end - - markup = Regexp.last_match(2) - - block = Condition.new(@left, '==', Condition.parse_expression(parse_context, Regexp.last_match(1))) - block.attach(body) - @blocks << block - end - end - def record_else_condition(markup) unless markup.strip.empty? raise SyntaxError, options[:locale].t("errors.syntax.case_invalid_else") diff --git a/lib/liquid/tags/cycle.rb b/lib/liquid/tags/cycle.rb index b7d3069c8..a98d25392 100644 --- a/lib/liquid/tags/cycle.rb +++ b/lib/liquid/tags/cycle.rb @@ -15,8 +15,6 @@ module Liquid # @liquid_syntax # {% cycle string, string, ... %} class Cycle < Tag - SimpleSyntax = /\A#{QuotedFragment}+/o - NamedSyntax = /\A(#{QuotedFragment})\s*\:\s*(.*)/om UNNAMED_CYCLE_PATTERN = /\w+:0x\h{8}/ attr_reader :variables @@ -56,7 +54,7 @@ def render_to_output_buffer(context, output) private # cycle [name:] expression(, expression)* - def strict2_parse(markup) + def parse_markup(markup) p = @parse_context.new_parser(markup) @variables = [] @@ -91,35 +89,6 @@ def strict2_parse(markup) end end - def strict_parse(markup) - lax_parse(markup) - end - - def lax_parse(markup) - case markup - when NamedSyntax - @variables = variables_from_string(Regexp.last_match(2)) - @name = parse_expression(Regexp.last_match(1)) - @is_named = true - when SimpleSyntax - @variables = variables_from_string(markup) - @name = @variables.to_s - @is_named = !@name.match?(UNNAMED_CYCLE_PATTERN) - else - raise SyntaxError, options[:locale].t("errors.syntax.cycle") - end - end - - def variables_from_string(markup) - markup.split(',').collect do |var| - var =~ /\s*(#{QuotedFragment})\s*/o - next unless Regexp.last_match(1) - - var = parse_expression(Regexp.last_match(1)) - maybe_dup_lookup(var) - end.compact - end - # For backwards compatibility, whenever a lookup is used in an unnamed cycle, # we make it so that the @variables.to_s produces different strings for cycles # called with the same arguments (since @variables.to_s is used as the cycle counter key) diff --git a/lib/liquid/tags/for.rb b/lib/liquid/tags/for.rb index cbea85bcb..cac9d2393 100644 --- a/lib/liquid/tags/for.rb +++ b/lib/liquid/tags/for.rb @@ -25,8 +25,6 @@ module Liquid # @liquid_optional_param range [untyped] A custom numeric range to iterate over. # @liquid_optional_param reversed [untyped] Iterate in reverse order. class For < Block - Syntax = /\A(#{VariableSegment}+)\s+in\s+(#{QuotedFragment}+)\s*(reversed)?/o - attr_reader :collection_name, :variable_name, :limit, :from def initialize(tag_name, markup, options) @@ -72,22 +70,7 @@ def render_to_output_buffer(context, output) protected - def lax_parse(markup) - if markup =~ Syntax - @variable_name = Regexp.last_match(1) - collection_name = Regexp.last_match(2) - @reversed = !!Regexp.last_match(3) - @name = "#{@variable_name}-#{collection_name}" - @collection_name = parse_expression(collection_name) - markup.scan(TagAttributes) do |key, value| - set_attribute(key, value) - end - else - raise SyntaxError, options[:locale].t("errors.syntax.for") - end - end - - def strict_parse(markup) + def parse_markup(markup) p = @parse_context.new_parser(markup) @variable_name = p.consume(:id) raise SyntaxError, options[:locale].t("errors.syntax.for_invalid_in") unless p.id?('in') @@ -111,10 +94,6 @@ def strict_parse(markup) private - def strict2_parse(markup) - strict_parse(markup) - end - def collection_segment(context) offsets = context.registers[:for] ||= {} diff --git a/lib/liquid/tags/if.rb b/lib/liquid/tags/if.rb index c423c1e84..06c752da7 100644 --- a/lib/liquid/tags/if.rb +++ b/lib/liquid/tags/if.rb @@ -14,10 +14,6 @@ module Liquid # @liquid_syntax_keyword condition The condition to evaluate. # @liquid_syntax_keyword expression The expression to render if the condition is met. class If < Block - Syntax = /(#{QuotedFragment})\s*([=!<>a-z_]+)?\s*(#{QuotedFragment})?/o - ExpressionsAndOperators = /(?:\b(?:\s?and\s?|\s?or\s?)\b|(?:\s*(?!\b(?:\s?and\s?|\s?or\s?)\b)(?:#{QuotedFragment}|\S+)\s*)+)/o - BOOLEAN_OPERATORS = %w(and or).freeze - attr_reader :blocks def initialize(tag_name, markup, options) @@ -66,10 +62,6 @@ def render_to_output_buffer(context, output) private - def strict2_parse(markup) - strict_parse(markup) - end - def push_block(tag, markup) block = if tag == 'else' ElseCondition.new @@ -85,27 +77,7 @@ def parse_expression(markup, safe: false) Condition.parse_expression(parse_context, markup, safe: safe) end - def lax_parse(markup) - expressions = markup.scan(ExpressionsAndOperators) - raise SyntaxError, options[:locale].t("errors.syntax.if") unless expressions.pop =~ Syntax - - condition = Condition.new(parse_expression(Regexp.last_match(1)), Regexp.last_match(2), parse_expression(Regexp.last_match(3))) - - until expressions.empty? - operator = expressions.pop.to_s.strip - - raise SyntaxError, options[:locale].t("errors.syntax.if") unless expressions.pop.to_s =~ Syntax - - new_condition = Condition.new(parse_expression(Regexp.last_match(1)), Regexp.last_match(2), parse_expression(Regexp.last_match(3))) - raise SyntaxError, options[:locale].t("errors.syntax.if") unless BOOLEAN_OPERATORS.include?(operator) - new_condition.send(operator, condition) - condition = new_condition - end - - condition - end - - def strict_parse(markup) + def parse_markup(markup) p = @parse_context.new_parser(markup) condition = parse_binary_comparisons(p) p.consume(:end_of_string) diff --git a/lib/liquid/tags/include.rb b/lib/liquid/tags/include.rb index 969482d49..b50c68a66 100644 --- a/lib/liquid/tags/include.rb +++ b/lib/liquid/tags/include.rb @@ -20,9 +20,6 @@ module Liquid class Include < Tag prepend Tag::Disableable - SYNTAX = /(#{QuotedFragment}+)(\s+(?:with|for)\s+(#{QuotedFragment}+))?(\s+(?:as)\s+(#{VariableSegment}+))?/o - Syntax = SYNTAX - attr_reader :template_name_expr, :variable_name_expr, :attributes def initialize(tag_name, markup, options) @@ -84,7 +81,7 @@ def render_to_output_buffer(context, output) alias_method :parse_context, :options private :parse_context - def strict2_parse(markup) + def parse_markup(markup) p = @parse_context.new_parser(markup) @template_name_expr = safe_parse_expression(p) @@ -104,29 +101,6 @@ def strict2_parse(markup) p.consume(:end_of_string) end - def strict_parse(markup) - lax_parse(markup) - end - - def lax_parse(markup) - if markup =~ SYNTAX - template_name = Regexp.last_match(1) - variable_name = Regexp.last_match(3) - - @alias_name = Regexp.last_match(5) - @variable_name_expr = variable_name ? parse_expression(variable_name) : nil - @template_name_expr = parse_expression(template_name) - @attributes = {} - - markup.scan(TagAttributes) do |key, value| - @attributes[key] = parse_expression(value) - end - - else - raise SyntaxError, options[:locale].t("errors.syntax.include") - end - end - class ParseTreeVisitor < Liquid::ParseTreeVisitor def children [ diff --git a/lib/liquid/tags/render.rb b/lib/liquid/tags/render.rb index 6e1559cc2..5c071c803 100644 --- a/lib/liquid/tags/render.rb +++ b/lib/liquid/tags/render.rb @@ -27,7 +27,6 @@ module Liquid # @liquid_syntax_keyword filename The name of the snippet to render, without the `.liquid` extension. class Render < Tag FOR = 'for' - SYNTAX = /(#{QuotedString}+)(\s+(with|#{FOR})\s+(#{QuotedFragment}+))?(\s+(?:as)\s+(#{VariableSegment}+))?/o disable_tags "include" @@ -85,10 +84,10 @@ def render_tag(context, output) end # render (string) (with|for expression)? (as id)? (key: value)* - def strict2_parse(markup) + def parse_markup(markup) p = @parse_context.new_parser(markup) - @template_name_expr = parse_expression(strict2_template_name(p), safe: true) + @template_name_expr = parse_expression(template_name(p), safe: true) with_or_for = p.id?("for") || p.id?("with") @variable_name_expr = safe_parse_expression(p) if with_or_for @alias_name = p.consume(:id) if p.id?("as") @@ -107,32 +106,10 @@ def strict2_parse(markup) p.consume(:end_of_string) end - def strict2_template_name(p) + def template_name(p) p.consume(:string) end - def strict_parse(markup) - lax_parse(markup) - end - - def lax_parse(markup) - raise SyntaxError, options[:locale].t("errors.syntax.render") unless markup =~ SYNTAX - - template_name = Regexp.last_match(1) - with_or_for = Regexp.last_match(3) - variable_name = Regexp.last_match(4) - - @alias_name = Regexp.last_match(6) - @variable_name_expr = variable_name ? parse_expression(variable_name) : nil - @template_name_expr = parse_expression(template_name) - @is_for_loop = (with_or_for == FOR) - - @attributes = {} - markup.scan(TagAttributes) do |key, value| - @attributes[key] = parse_expression(value) - end - end - class ParseTreeVisitor < Liquid::ParseTreeVisitor def children [ diff --git a/lib/liquid/tags/table_row.rb b/lib/liquid/tags/table_row.rb index b69f91486..c91147541 100644 --- a/lib/liquid/tags/table_row.rb +++ b/lib/liquid/tags/table_row.rb @@ -24,7 +24,6 @@ module Liquid # @liquid_optional_param offset: [number] The 1-based index to start iterating at. # @liquid_optional_param range [untyped] A custom numeric range to iterate over. class TableRow < Block - Syntax = /(\w+)\s+in\s+(#{QuotedFragment}+)/o ALLOWED_ATTRIBUTES = ['cols', 'limit', 'offset', 'range'].freeze attr_reader :variable_name, :collection_name, :attributes @@ -34,7 +33,7 @@ def initialize(tag_name, markup, options) parse_with_selected_parser(markup) end - def strict2_parse(markup) + def parse_markup(markup) p = @parse_context.new_parser(markup) @variable_name = p.consume(:id) @@ -62,23 +61,6 @@ def strict2_parse(markup) p.consume(:end_of_string) end - def strict_parse(markup) - lax_parse(markup) - end - - def lax_parse(markup) - if markup =~ Syntax - @variable_name = Regexp.last_match(1) - @collection_name = parse_expression(Regexp.last_match(2)) - @attributes = {} - markup.scan(TagAttributes) do |key, value| - @attributes[key] = parse_expression(value) - end - else - raise SyntaxError, options[:locale].t("errors.syntax.table_row") - end - end - def render_to_output_buffer(context, output) (collection = context.evaluate(@collection_name)) || (return '') diff --git a/lib/liquid/template.rb b/lib/liquid/template.rb index b007765ce..a81fec423 100644 --- a/lib/liquid/template.rb +++ b/lib/liquid/template.rb @@ -16,25 +16,11 @@ module Liquid # class Template attr_accessor :root, :name - attr_reader :resource_limits, :warnings + attr_reader :resource_limits attr_reader :profiler class << self - # 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 enforces correct syntax for most tags - # :strict2 enforces correct syntax for all tags - def error_mode=(mode) - Deprecations.warn("Template.error_mode=", "Environment#error_mode=") - Environment.default.error_mode = mode - end - - def error_mode - Environment.default.error_mode - end - def default_exception_renderer=(renderer) Deprecations.warn("Template.default_exception_renderer=", "Environment#exception_renderer=") Environment.default.exception_renderer = renderer @@ -223,7 +209,6 @@ def configure_options(options) ParseContext.new(opts) end - @warnings = parse_context.warnings parse_context end diff --git a/lib/liquid/variable.rb b/lib/liquid/variable.rb index 6b5fb412b..fcc723515 100644 --- a/lib/liquid/variable.rb +++ b/lib/liquid/variable.rb @@ -30,7 +30,7 @@ def initialize(markup, parse_context) @parse_context = parse_context @line_number = parse_context.line_number - strict_parse_with_error_mode_fallback(markup) + parse_with_selected_parser(markup) end def raw @@ -41,58 +41,17 @@ def markup_context(markup) "in \"{{#{markup}}}\"" end - def lax_parse(markup) - @filters = [] - return unless markup =~ MarkupWithQuotedFragment - - name_markup = Regexp.last_match(1) - filter_markup = Regexp.last_match(2) - @name = parse_context.parse_expression(name_markup) - if filter_markup =~ FilterMarkupRegex - filters = Regexp.last_match(1).scan(FilterParser) - filters.each do |f| - next unless f =~ /\w+/ - filtername = Regexp.last_match(0) - filterargs = f.scan(FilterArgsRegex).flatten - @filters << lax_parse_filter_expressions(filtername, filterargs) - end - end - end - - def strict_parse(markup) - @filters = [] - p = @parse_context.new_parser(markup) - - return if p.look(:end_of_string) - - @name = parse_context.safe_parse_expression(p) - while p.consume?(:pipe) - filtername = p.consume(:id) - filterargs = p.consume?(:colon) ? parse_filterargs(p) : Const::EMPTY_ARRAY - @filters << lax_parse_filter_expressions(filtername, filterargs) - end - p.consume(:end_of_string) - end - - def strict2_parse(markup) + def parse_markup(markup) @filters = [] p = @parse_context.new_parser(markup) return if p.look(:end_of_string) @name = parse_context.safe_parse_expression(p) - @filters << strict2_parse_filter_expressions(p) while p.consume?(:pipe) + @filters << parse_filter_expressions(p) while p.consume?(:pipe) 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) @@ -133,22 +92,6 @@ def disabled_tags private - def lax_parse_filter_expressions(filter_name, unparsed_args) - filter_args = [] - keyword_args = nil - unparsed_args.each do |a| - if (matches = a.match(JustTagAttributes)) - keyword_args ||= {} - keyword_args[matches[1]] = parse_context.parse_expression(matches[2]) - else - filter_args << parse_context.parse_expression(a) - end - end - result = [filter_name, filter_args] - result << keyword_args if keyword_args - result - end - # Surprisingly, positional and keyword arguments can be mixed. # # filter = filtername [":" filterargs?] @@ -156,7 +99,7 @@ def lax_parse_filter_expressions(filter_name, unparsed_args) # argument = (positional_argument | keyword_argument) # positional_argument = expression # keyword_argument = id ":" expression - def strict2_parse_filter_expressions(p) + def parse_filter_expressions(p) filtername = p.consume(:id) filter_args = [] keyword_args = {} diff --git a/performance/benchmark.rb b/performance/benchmark.rb index b61e9057c..c8aa6769d 100644 --- a/performance/benchmark.rb +++ b/performance/benchmark.rb @@ -4,7 +4,6 @@ require_relative 'theme_runner' RubyVM::YJIT.enable if defined?(RubyVM::YJIT) -Liquid::Environment.default.error_mode = ARGV.first.to_sym if ARGV.first profiler = ThemeRunner.new diff --git a/performance/memory_profile.rb b/performance/memory_profile.rb index e2934297a..fb7312d55 100644 --- a/performance/memory_profile.rb +++ b/performance/memory_profile.rb @@ -53,8 +53,6 @@ def sanitize(string) end end -Liquid::Template.error_mode = ARGV.first.to_sym if ARGV.first - runner = ThemeRunner.new Profiler.run do |x| x.profile('parse') { runner.compile } diff --git a/performance/profile.rb b/performance/profile.rb index 70740778d..f756fb202 100644 --- a/performance/profile.rb +++ b/performance/profile.rb @@ -3,7 +3,6 @@ require 'stackprof' require_relative 'theme_runner' -Liquid::Template.error_mode = ARGV.first.to_sym if ARGV.first profiler = ThemeRunner.new profiler.run diff --git a/test/integration/assign_test.rb b/test/integration/assign_test.rb index fdb6c99ca..69163ab96 100644 --- a/test/integration/assign_test.rb +++ b/test/integration/assign_test.rb @@ -39,13 +39,11 @@ def test_assign_syntax_error assert_match_syntax_error(/assign/, '{% assign foo not values %}.') end - def test_assign_uses_error_mode + def test_assign_throws_on_unsupported_syntax assert_match_syntax_error( - "Expected dotdot but found pipe in ", + "Expected dotdot but found pipe", "{% assign foo = ('X' | downcase) %}", - error_mode: :strict, ) - assert_template_result("", "{% assign foo = ('X' | downcase) %}", error_mode: :lax) end def test_expression_with_whitespace_in_square_brackets diff --git a/test/integration/context_test.rb b/test/integration/context_test.rb index d230734f6..e1952e634 100644 --- a/test/integration/context_test.rb +++ b/test/integration/context_test.rb @@ -632,11 +632,9 @@ def notice(output) end def test_has_key_will_not_add_an_error_for_missing_keys - with_error_modes(:strict) do - context = Context.new - context.key?('unknown') - assert_empty(context.errors) - end + context = Context.new + context.key?('unknown') + assert_empty(context.errors) end def test_key_lookup_will_raise_for_missing_keys_when_strict_variables_is_enabled diff --git a/test/integration/error_handling_test.rb b/test/integration/error_handling_test.rb index 0fda83ca5..9de179f2d 100644 --- a/test/integration/error_handling_test.rb +++ b/test/integration/error_handling_test.rb @@ -67,20 +67,11 @@ def test_missing_endtag_parse_time_error end def test_unrecognized_operator - with_error_modes(:strict) do - assert_raises(SyntaxError) do - Liquid::Template.parse(' {% if 1 =! 2 %}ok{% endif %} ') - end + assert_raises(SyntaxError) do + Liquid::Template.parse(' {% if 1 =! 2 %}ok{% endif %} ') end end - def test_lax_unrecognized_operator - template = Liquid::Template.parse(' {% if 1 =! 2 %}ok{% endif %} ', error_mode: :lax) - assert_equal(' Liquid error: Unknown operator =! ', template.render) - assert_equal(1, template.errors.size) - assert_equal(Liquid::ArgumentError, template.errors.first.class) - end - def test_with_line_numbers_adds_numbers_to_parser_errors source = <<~LIQUID foobar @@ -104,25 +95,6 @@ def test_with_line_numbers_adds_numbers_to_parser_errors_with_whitespace_trim assert_match_syntax_error(/Liquid syntax error \(line 3\)/, source) end - def test_parsing_warn_with_line_numbers_adds_numbers_to_lexer_errors - template = Liquid::Template.parse( - ' - foobar - - {% if 1 =! 2 %}ok{% endif %} - - bla - ', - error_mode: :warn, - line_numbers: true, - ) - - assert_equal( - ['Liquid syntax error (line 4): Unexpected character = in "1 =! 2"'], - template.warnings.map(&:message), - ) - end - def test_parsing_strict_with_line_numbers_adds_numbers_to_lexer_errors err = assert_raises(SyntaxError) do Liquid::Template.parse( @@ -133,7 +105,6 @@ def test_parsing_strict_with_line_numbers_adds_numbers_to_lexer_errors bla ', - error_mode: :strict, line_numbers: true, ) end @@ -157,34 +128,16 @@ def test_syntax_errors_in_nested_blocks_have_correct_line_number def test_strict_error_messages err = assert_raises(SyntaxError) do - Liquid::Template.parse(' {% if 1 =! 2 %}ok{% endif %} ', error_mode: :strict) + Liquid::Template.parse(' {% if 1 =! 2 %}ok{% endif %} ') end assert_equal('Liquid syntax error: Unexpected character = in "1 =! 2"', err.message) err = assert_raises(SyntaxError) do - Liquid::Template.parse('{{%%%}}', error_mode: :strict) + Liquid::Template.parse('{{%%%}}') end assert_equal('Liquid syntax error: Unexpected character % in "{{%%%}}"', err.message) end - def test_warnings - template = Liquid::Template.parse('{% if ~~~ %}{{%%%}}{% else %}{{ hello. }}{% endif %}', error_mode: :warn) - assert_equal(3, template.warnings.size) - assert_equal('Unexpected character ~ in "~~~"', template.warnings[0].to_s(false)) - assert_equal('Unexpected character % in "{{%%%}}"', template.warnings[1].to_s(false)) - assert_equal('Expected id but found end_of_string in "{{ hello. }}"', template.warnings[2].to_s(false)) - assert_equal('', template.render) - end - - def test_warning_line_numbers - template = Liquid::Template.parse("{% if ~~~ %}\n{{%%%}}{% else %}\n{{ hello. }}{% endif %}", error_mode: :warn, line_numbers: true) - assert_equal('Liquid syntax error (line 1): Unexpected character ~ in "~~~"', template.warnings[0].message) - assert_equal('Liquid syntax error (line 2): Unexpected character % in "{{%%%}}"', template.warnings[1].message) - assert_equal('Liquid syntax error (line 3): Expected id but found end_of_string in "{{ hello. }}"', template.warnings[2].message) - assert_equal(3, template.warnings.size) - assert_equal([1, 2, 3], template.warnings.map(&:line_number)) - end - # Liquid should not catch Exceptions that are not subclasses of StandardError, like Interrupt and NoMemoryError def test_exceptions_propagate assert_raises(Exception) do diff --git a/test/integration/expression_test.rb b/test/integration/expression_test.rb index ae84fa36d..988f1d122 100644 --- a/test/integration/expression_test.rb +++ b/test/integration/expression_test.rb @@ -27,11 +27,6 @@ def test_float assert_template_result("-17.42", "{{ -17.42 }}") assert_template_result("2.5", "{{ 2.5 }}") - with_error_modes(:lax) do - assert_expression_result(0.0, "0.....5") - assert_expression_result(0.0, "-0..1") - end - assert_expression_result(1.5, "1.5") # this is a unfortunate quirky behavior of Liquid @@ -56,19 +51,6 @@ def test_range ) end - def test_quirky_negative_sign_expression_markup - result = Expression.parse("-", nil) - assert(result.is_a?(VariableLookup)) - assert_equal("-", result.name) - - # for this template, the expression markup is "-" - assert_template_result( - "", - "{{ - 'theme.css' - }}", - error_mode: :lax, - ) - end - def test_expression_cache skip("Liquid-C does not support Expression caching") if defined?(Liquid::C) && Liquid::C.enabled diff --git a/test/integration/parsing_quirks_test.rb b/test/integration/parsing_quirks_test.rb index b82ce86c5..75954f93f 100644 --- a/test/integration/parsing_quirks_test.rb +++ b/test/integration/parsing_quirks_test.rb @@ -31,58 +31,25 @@ def test_raise_on_label_and_no_close_bracets_percent def test_error_on_empty_filter assert(Template.parse("{{test}}")) - with_error_modes(:lax) do - assert(Template.parse("{{|test}}")) - end - - with_error_modes(:strict) do - assert_raises(SyntaxError) { Template.parse("{{|test}}") } - assert_raises(SyntaxError) { Template.parse("{{test |a|b|}}") } - end + assert_raises(Liquid::SyntaxError) { Template.parse("{{|test}}") } + assert_raises(Liquid::SyntaxError) { Template.parse("{{test |a|b|}}") } end def test_meaningless_parens_error - with_error_modes(:strict) do - assert_raises(SyntaxError) do - markup = "a == 'foo' or (b == 'bar' and c == 'baz') or false" - Template.parse("{% if #{markup} %} YES {% endif %}") - end + assert_raises(SyntaxError) do + markup = "a == 'foo' or (b == 'bar' and c == 'baz') or false" + Template.parse("{% if #{markup} %} YES {% endif %}") end end def test_unexpected_characters_syntax_error - with_error_modes(:strict) do - assert_raises(SyntaxError) do - markup = "true && false" - Template.parse("{% if #{markup} %} YES {% endif %}") - end - assert_raises(SyntaxError) do - markup = "false || true" - Template.parse("{% if #{markup} %} YES {% endif %}") - end - end - end - - def test_no_error_on_lax_empty_filter - assert(Template.parse("{{test |a|b|}}", error_mode: :lax)) - assert(Template.parse("{{test}}", error_mode: :lax)) - assert(Template.parse("{{|test|}}", error_mode: :lax)) - end - - def test_meaningless_parens_lax - with_error_modes(:lax) do - assigns = { 'b' => 'bar', 'c' => 'baz' } - markup = "a == 'foo' or (b == 'bar' and c == 'baz') or false" - assert_template_result(' YES ', "{% if #{markup} %} YES {% endif %}", assigns) - end - end - - def test_unexpected_characters_silently_eat_logic_lax - with_error_modes(:lax) do + assert_raises(SyntaxError) do markup = "true && false" - assert_template_result(' YES ', "{% if #{markup} %} YES {% endif %}") + Template.parse("{% if #{markup} %} YES {% endif %}") + end + assert_raises(SyntaxError) do markup = "false || true" - assert_template_result('', "{% if #{markup} %} YES {% endif %}") + Template.parse("{% if #{markup} %} YES {% endif %}") end end @@ -92,32 +59,6 @@ def test_raise_on_invalid_tag_delimiter end end - def test_unanchored_filter_arguments - with_error_modes(:lax) do - assert_template_result('hi', "{{ 'hi there' | split$$$:' ' | first }}") - - assert_template_result('x', "{{ 'X' | downcase) }}") - - # After the messed up quotes a filter without parameters (reverse) should work - # but one with parameters (remove) shouldn't be detected. - assert_template_result('here', "{{ 'hi there' | split:\"t\"\" | reverse | first}}") - assert_template_result('hi ', "{{ 'hi there' | split:\"t\"\" | remove:\"i\" | first}}") - end - end - - def test_invalid_variables_work - with_error_modes(:lax) do - assert_template_result('bar', "{% assign 123foo = 'bar' %}{{ 123foo }}") - assert_template_result('123', "{% assign 123 = 'bar' %}{{ 123 }}") - end - end - - def test_extra_dots_in_ranges - with_error_modes(:lax) do - assert_template_result('12345', "{% for i in (1...5) %}{{ i }}{% endfor %}") - end - end - def test_blank_variable_markup assert_template_result('', "{{}}") end @@ -131,24 +72,4 @@ def test_lookup_on_var_with_literal_name def test_contains_in_id assert_template_result(' YES ', '{% if containsallshipments == true %} YES {% endif %}', { 'containsallshipments' => true }) end - - def test_incomplete_expression - with_error_modes(:lax) do - assert_template_result("false", "{{ false - }}") - assert_template_result("false", "{{ false > }}") - assert_template_result("false", "{{ false < }}") - assert_template_result("false", "{{ false = }}") - assert_template_result("false", "{{ false ! }}") - assert_template_result("false", "{{ false 1 }}") - assert_template_result("false", "{{ false a }}") - - assert_template_result("false", "{% liquid assign foo = false -\n%}{{ foo }}") - assert_template_result("false", "{% liquid assign foo = false >\n%}{{ foo }}") - assert_template_result("false", "{% liquid assign foo = false <\n%}{{ foo }}") - assert_template_result("false", "{% liquid assign foo = false =\n%}{{ foo }}") - assert_template_result("false", "{% liquid assign foo = false !\n%}{{ foo }}") - assert_template_result("false", "{% liquid assign foo = false 1\n%}{{ foo }}") - assert_template_result("false", "{% liquid assign foo = false a\n%}{{ foo }}") - end - end end # ParsingQuirksTest diff --git a/test/integration/tags/cycle_tag_test.rb b/test/integration/tags/cycle_tag_test.rb index dfb5984d8..9edd19958 100644 --- a/test/integration/tags/cycle_tag_test.rb +++ b/test/integration/tags/cycle_tag_test.rb @@ -91,28 +91,21 @@ def test_cycle_tag_without_arguments assert_match(/Syntax Error in 'cycle' - Valid syntax: cycle \[name :\] var/, error.message) end - def test_cycle_tag_with_error_mode + def test_cycle_tag_unsupported_legacy_quirk # QuotedFragment is more permissive than what Parser#expression allows. template1 = "{% assign 5 = 'b' %}{% cycle .5, .4 %}" template2 = "{% cycle .5: 'a', 'b' %}" - with_error_modes(:lax, :strict) do - assert_template_result("b", template1) - assert_template_result("a", template2) - end - - with_error_modes(:strict2) do - error1 = assert_raises(Liquid::SyntaxError) { Template.parse(template1) } - error2 = assert_raises(Liquid::SyntaxError) { Template.parse(template2) } + error1 = assert_raises(Liquid::SyntaxError) { Template.parse(template1) } + error2 = assert_raises(Liquid::SyntaxError) { Template.parse(template2) } - expected_error = /Liquid syntax error: \[:dot, "."\] is not a valid expression/ + expected_error = /Liquid syntax error: \[:dot, "."\] is not a valid expression/ - assert_match(expected_error, error1.message) - assert_match(expected_error, error2.message) - end + assert_match(expected_error, error1.message) + assert_match(expected_error, error2.message) end - def test_cycle_with_trailing_elements + def test_cycle_with_trailing_elements_legacy_syntax assignments = "{% assign a = 'A' %}{% assign n = 'N' %}" template1 = "#{assignments}{% cycle 'a' 'b', 'c' %}" @@ -121,29 +114,19 @@ def test_cycle_with_trailing_elements template4 = "#{assignments}{% cycle n e: 'a', 'b', 'c' %}" template5 = "#{assignments}{% cycle n e 'a', 'b', 'c' %}" - with_error_modes(:lax, :strict) do - assert_template_result("a", template1) - assert_template_result("a", template2) - assert_template_result("a", template3) - assert_template_result("N", template4) - assert_template_result("N", template5) - end - - with_error_modes(:strict2) do - error1 = assert_raises(Liquid::SyntaxError) { Template.parse(template1) } - error2 = assert_raises(Liquid::SyntaxError) { Template.parse(template2) } - error3 = assert_raises(Liquid::SyntaxError) { Template.parse(template3) } - error4 = assert_raises(Liquid::SyntaxError) { Template.parse(template4) } - error5 = assert_raises(Liquid::SyntaxError) { Template.parse(template5) } + error1 = assert_raises(Liquid::SyntaxError) { Template.parse(template1) } + error2 = assert_raises(Liquid::SyntaxError) { Template.parse(template2) } + error3 = assert_raises(Liquid::SyntaxError) { Template.parse(template3) } + error4 = assert_raises(Liquid::SyntaxError) { Template.parse(template4) } + error5 = assert_raises(Liquid::SyntaxError) { Template.parse(template5) } - expected_error = /Expected end_of_string but found/ + expected_error = /Expected end_of_string but found/ - assert_match(expected_error, error1.message) - assert_match(expected_error, error2.message) - assert_match(expected_error, error3.message) - assert_match(expected_error, error4.message) - assert_match(expected_error, error5.message) - end + assert_match(expected_error, error1.message) + assert_match(expected_error, error2.message) + assert_match(expected_error, error3.message) + assert_match(expected_error, error4.message) + assert_match(expected_error, error5.message) end def test_cycle_name_with_invalid_expression @@ -153,14 +136,8 @@ def test_cycle_name_with_invalid_expression {% endfor %} LIQUID - with_error_modes(:lax, :strict) do - refute_nil(Template.parse(template)) - end - - with_error_modes(:strict2) do - error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } - assert_match(/Unexpected character =/, error.message) - end + error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } + assert_match(/Unexpected character =/, error.message) end def test_cycle_variable_with_invalid_expression @@ -170,13 +147,7 @@ def test_cycle_variable_with_invalid_expression {% endfor %} LIQUID - with_error_modes(:lax, :strict) do - refute_nil(Template.parse(template)) - end - - with_error_modes(:strict2) do - error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } - assert_match(/Unexpected character =/, error.message) - end + error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } + assert_match(/Unexpected character =/, error.message) end end diff --git a/test/integration/tags/include_tag_test.rb b/test/integration/tags/include_tag_test.rb index 44c0dcda7..b911ace16 100644 --- a/test/integration/tags/include_tag_test.rb +++ b/test/integration/tags/include_tag_test.rb @@ -204,23 +204,13 @@ def test_dynamically_choosen_template ) end - def test_strict2_parsing_errors - with_error_modes(:lax, :strict) do - assert_template_result( - 'hello value1 value2', - '{% include "snippet" !!! arg1: "value1" ~~~ arg2: "value2" %}', - partials: { 'snippet' => 'hello {{ arg1 }} {{ arg2 }}' }, - ) - end - - with_error_modes(:strict2) do - assert_syntax_error( - '{% include "snippet" !!! arg1: "value1" ~~~ arg2: "value2" %}', - ) - assert_syntax_error( - '{% include "snippet" | filter %}', - ) - end + def test_parsing_errors_for_legacy_quirk + assert_syntax_error( + '{% include "snippet" !!! arg1: "value1" ~~~ arg2: "value2" %}', + ) + assert_syntax_error( + '{% include "snippet" | filter %}', + ) end def test_optional_commas @@ -301,16 +291,10 @@ def test_passing_options_to_included_templates env = Liquid::Environment.build(file_system: TestFileSystem.new) assert_raises(Liquid::SyntaxError) do - Template.parse("{% include template %}", error_mode: :strict, environment: env).render!("template" => '{{ "X" || downcase }}') - end - with_error_modes(:lax) do - assert_equal('x', Template.parse("{% include template %}", error_mode: :strict, include_options_blacklist: true, environment: env).render!("template" => '{{ "X" || downcase }}')) + Template.parse("{% include template %}", environment: env).render!("template" => '{{ "X" || downcase }}') end assert_raises(Liquid::SyntaxError) do - Template.parse("{% include template %}", error_mode: :strict, include_options_blacklist: [:locale], environment: env).render!("template" => '{{ "X" || downcase }}') - end - with_error_modes(:lax) do - assert_equal('x', Template.parse("{% include template %}", error_mode: :strict, include_options_blacklist: [:error_mode], environment: env).render!("template" => '{{ "X" || downcase }}')) + Template.parse("{% include template %}", include_options_blacklist: [:locale], environment: env).render!("template" => '{{ "X" || downcase }}') end end @@ -365,7 +349,7 @@ def test_including_with_strict_variables file_system: StubFileSystem.new('simple' => 'simple'), ) - template = Liquid::Template.parse("{% include 'simple' %}", error_mode: :warn, environment: env) + template = Liquid::Template.parse("{% include 'simple' %}", environment: env) template.render(nil, strict_variables: true) assert_equal([], template.errors) @@ -404,39 +388,21 @@ def test_render_tag_renders_error_with_template_name_from_template_factory def test_include_template_with_invalid_expression template = "{% include foo=>bar %}" - with_error_modes(:lax, :strict) do - refute_nil(Template.parse(template)) - end - - with_error_modes(:strict2) do - error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } - assert_match(/Unexpected character =/, error.message) - end + error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } + assert_match(/Unexpected character =/, error.message) end def test_include_with_invalid_expression template = '{% include "snippet" with foo=>bar %}' - with_error_modes(:lax, :strict) do - refute_nil(Template.parse(template)) - end - - with_error_modes(:strict2) do - error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } - assert_match(/Unexpected character =/, error.message) - end + error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } + assert_match(/Unexpected character =/, error.message) end def test_include_attribute_with_invalid_expression template = '{% include "snippet", key: foo=>bar %}' - with_error_modes(:lax, :strict) do - refute_nil(Template.parse(template)) - end - - with_error_modes(:strict2) do - error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } - assert_match(/Unexpected character =/, error.message) - end + error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } + assert_match(/Unexpected character =/, error.message) end end # IncludeTagTest diff --git a/test/integration/tags/render_tag_test.rb b/test/integration/tags/render_tag_test.rb index 0bd09d088..2c69ba547 100644 --- a/test/integration/tags/render_tag_test.rb +++ b/test/integration/tags/render_tag_test.rb @@ -105,23 +105,13 @@ def test_dynamically_choosen_templates_are_not_allowed assert_syntax_error("{% assign name = 'snippet' %}{% render name %}") end - def test_strict2_parsing_errors - with_error_modes(:lax, :strict) do - assert_template_result( - 'hello value1 value2', - '{% render "snippet" !!! arg1: "value1" ~~~ arg2: "value2" %}', - partials: { 'snippet' => 'hello {{ arg1 }} {{ arg2 }}' }, - ) - end - - with_error_modes(:strict2) do - assert_syntax_error( - '{% render "snippet" !!! arg1: "value1" ~~~ arg2: "value2" %}', - ) - assert_syntax_error( - '{% render "snippet" | filter %}', - ) - end + def test_parsing_errors_legacy_syntax + assert_syntax_error( + '{% render "snippet" !!! arg1: "value1" ~~~ arg2: "value2" %}', + ) + assert_syntax_error( + '{% render "snippet" | filter %}', + ) end def test_optional_commas @@ -317,27 +307,13 @@ def test_render_tag_renders_error_with_template_name_from_template_factory def test_render_with_invalid_expression template = '{% render "snippet" with foo=>bar %}' - - with_error_modes(:lax, :strict) do - refute_nil(Template.parse(template)) - end - - with_error_modes(:strict2) do - error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } - assert_match(/Unexpected character =/, error.message) - end + error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } + assert_match(/Unexpected character =/, error.message) end def test_render_attribute_with_invalid_expression template = '{% render "snippet", key: foo=>bar %}' - - with_error_modes(:lax, :strict) do - refute_nil(Template.parse(template)) - end - - with_error_modes(:strict2) do - error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } - assert_match(/Unexpected character =/, error.message) - end + error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } + assert_match(/Unexpected character =/, error.message) end end diff --git a/test/integration/tags/table_row_test.rb b/test/integration/tags/table_row_test.rb index 45628ce9d..59b4cc25e 100644 --- a/test/integration/tags/table_row_test.rb +++ b/test/integration/tags/table_row_test.rb @@ -188,29 +188,6 @@ def test_tablerow_loop_drop_attributes assert_template_result(expected_output, template) end - def test_table_row_renders_correct_error_message_for_invalid_parameters - assert_template_result( - "Liquid error (line 1): invalid integer", - '{% tablerow n in (1...10) limit:true %} {{n}} {% endtablerow %}', - error_mode: :warn, - render_errors: true, - ) - - assert_template_result( - "Liquid error (line 1): invalid integer", - '{% tablerow n in (1...10) offset:true %} {{n}} {% endtablerow %}', - error_mode: :warn, - render_errors: true, - ) - - assert_template_result( - "Liquid error (line 1): invalid integer", - '{% tablerow n in (1...10) cols:true %} {{n}} {% endtablerow %}', - render_errors: true, - error_mode: :warn, - ) - end - def test_table_row_handles_interrupts assert_template_result( "\n 1 \n", @@ -259,7 +236,7 @@ def test_table_row_does_not_leak_interrupts ) end - def test_tablerow_with_cols_attribute_in_strict2_mode + def test_tablerow_with_cols_attribute template = <<~LIQUID.chomp {% tablerow i in (1..6) cols: 3 %}{{ i }}{% endtablerow %} LIQUID @@ -270,12 +247,10 @@ def test_tablerow_with_cols_attribute_in_strict2_mode 456 OUTPUT - with_error_modes(:strict2) do - assert_template_result(expected, template) - end + assert_template_result(expected, template) end - def test_tablerow_with_limit_attribute_in_strict2_mode + def test_tablerow_with_limit_attribute template = <<~LIQUID.chomp {% tablerow i in (1..10) limit: 3 %}{{ i }}{% endtablerow %} LIQUID @@ -285,12 +260,10 @@ def test_tablerow_with_limit_attribute_in_strict2_mode 123 OUTPUT - with_error_modes(:strict2) do - assert_template_result(expected, template) - end + assert_template_result(expected, template) end - def test_tablerow_with_offset_attribute_in_strict2_mode + def test_tablerow_with_offset_attribute template = <<~LIQUID.chomp {% tablerow i in (1..5) offset: 2 %}{{ i }}{% endtablerow %} LIQUID @@ -300,12 +273,10 @@ def test_tablerow_with_offset_attribute_in_strict2_mode 345 OUTPUT - with_error_modes(:strict2) do - assert_template_result(expected, template) - end + assert_template_result(expected, template) end - def test_tablerow_with_range_attribute_in_strict2_mode + def test_tablerow_with_range_attribute template = <<~LIQUID.chomp {% tablerow i in (1..3) range: (1..10) %}{{ i }}{% endtablerow %} LIQUID @@ -315,12 +286,10 @@ def test_tablerow_with_range_attribute_in_strict2_mode 123 OUTPUT - with_error_modes(:strict2) do - assert_template_result(expected, template) - end + assert_template_result(expected, template) end - def test_tablerow_with_multiple_attributes_in_strict2_mode + def test_tablerow_with_multiple_attributes template = <<~LIQUID.chomp {% tablerow i in (1..10) cols: 2, limit: 4, offset: 1 %}{{ i }}{% endtablerow %} LIQUID @@ -331,12 +300,10 @@ def test_tablerow_with_multiple_attributes_in_strict2_mode 45 OUTPUT - with_error_modes(:strict2) do - assert_template_result(expected, template) - end + assert_template_result(expected, template) end - def test_tablerow_with_variable_collection_in_strict2_mode + def test_tablerow_with_variable_collection template = <<~LIQUID.chomp {% tablerow n in numbers cols: 2 %}{{ n }}{% endtablerow %} LIQUID @@ -347,12 +314,10 @@ def test_tablerow_with_variable_collection_in_strict2_mode 34 OUTPUT - with_error_modes(:strict2) do - assert_template_result(expected, template, { 'numbers' => [1, 2, 3, 4] }) - end + assert_template_result(expected, template, { 'numbers' => [1, 2, 3, 4] }) end - def test_tablerow_with_dotted_access_in_strict2_mode + def test_tablerow_with_dotted_access template = <<~LIQUID.chomp {% tablerow n in obj.numbers cols: 2 %}{{ n }}{% endtablerow %} LIQUID @@ -363,12 +328,10 @@ def test_tablerow_with_dotted_access_in_strict2_mode 34 OUTPUT - with_error_modes(:strict2) do - assert_template_result(expected, template, { 'obj' => { 'numbers' => [1, 2, 3, 4] } }) - end + assert_template_result(expected, template, { 'obj' => { 'numbers' => [1, 2, 3, 4] } }) end - def test_tablerow_with_bracketed_access_in_strict2_mode + def test_tablerow_with_bracketed_access template = <<~LIQUID.chomp {% tablerow n in obj["numbers"] cols: 2 %}{{ n }}{% endtablerow %} LIQUID @@ -378,12 +341,10 @@ def test_tablerow_with_bracketed_access_in_strict2_mode 1020 OUTPUT - with_error_modes(:strict2) do - assert_template_result(expected, template, { 'obj' => { 'numbers' => [10, 20] } }) - end + assert_template_result(expected, template, { 'obj' => { 'numbers' => [10, 20] } }) end - def test_tablerow_without_attributes_in_strict2_mode + def test_tablerow_without_attributes template = <<~LIQUID.chomp {% tablerow i in (1..3) %}{{ i }}{% endtablerow %} LIQUID @@ -393,30 +354,24 @@ def test_tablerow_without_attributes_in_strict2_mode 123 OUTPUT - with_error_modes(:strict2) do - assert_template_result(expected, template) - end + assert_template_result(expected, template) end - def test_tablerow_without_in_keyword_in_strict2_mode + def test_tablerow_without_in_keyword template = '{% tablerow i (1..10) %}{{ i }}{% endtablerow %}' - with_error_modes(:strict2) do - error = assert_raises(SyntaxError) { Template.parse(template) } - assert_equal("Liquid syntax error: For loops require an 'in' clause in \"i (1..10)\"", error.message) - end + error = assert_raises(SyntaxError) { Template.parse(template) } + assert_equal("Liquid syntax error: For loops require an 'in' clause in \"i (1..10)\"", error.message) end - def test_tablerow_with_multiple_invalid_attributes_reports_first_in_strict2_mode + def test_tablerow_with_multiple_invalid_attributes_reports_first template = '{% tablerow i in (1..10) invalid1: 5, invalid2: 10 %}{{ i }}{% endtablerow %}' - with_error_modes(:strict2) do - error = assert_raises(SyntaxError) { Template.parse(template) } - assert_equal("Liquid syntax error: Invalid attribute 'invalid1' in tablerow loop. Valid attributes are cols, limit, offset, and range in \"i in (1..10) invalid1: 5, invalid2: 10\"", error.message) - end + error = assert_raises(SyntaxError) { Template.parse(template) } + assert_equal("Liquid syntax error: Invalid attribute 'invalid1' in tablerow loop. Valid attributes are cols, limit, offset, and range in \"i in (1..10) invalid1: 5, invalid2: 10\"", error.message) end - def test_tablerow_with_empty_collection_in_strict2_mode + def test_tablerow_with_empty_collection template = <<~LIQUID.chomp {% tablerow i in empty_array cols: 2 %}{{ i }}{% endtablerow %} LIQUID @@ -426,43 +381,18 @@ def test_tablerow_with_empty_collection_in_strict2_mode OUTPUT - with_error_modes(:strict2) do - assert_template_result(expected, template, { 'empty_array' => [] }) - end + assert_template_result(expected, template, { 'empty_array' => [] }) end - def test_tablerow_with_invalid_attribute_strict_vs_strict2 + def test_tablerow_with_invalid_attribute template = '{% tablerow i in (1..5) invalid_attr: 10 %}{{ i }}{% endtablerow %}' - - expected = <<~OUTPUT - - 12345 - OUTPUT - - with_error_modes(:lax, :strict) do - assert_template_result(expected, template) - end - - with_error_modes(:strict2) do - error = assert_raises(SyntaxError) { Template.parse(template) } - assert_match(/Invalid attribute 'invalid_attr'/, error.message) - end + error = assert_raises(SyntaxError) { Template.parse(template) } + assert_match(/Invalid attribute 'invalid_attr'/, error.message) end - def test_tablerow_with_invalid_expression_strict_vs_strict2 + def test_tablerow_with_invalid_expression template = '{% tablerow i in (1..5) limit: foo=>bar %}{{ i }}{% endtablerow %}' - - with_error_modes(:lax, :strict) do - expected = <<~OUTPUT - - - OUTPUT - assert_template_result(expected, template) - end - - with_error_modes(:strict2) do - error = assert_raises(SyntaxError) { Template.parse(template) } - assert_match(/Unexpected character =/, error.message) - end + error = assert_raises(SyntaxError) { Template.parse(template) } + assert_match(/Unexpected character =/, error.message) end end diff --git a/test/integration/template_test.rb b/test/integration/template_test.rb index 92ece017c..e29ca3b6b 100644 --- a/test/integration/template_test.rb +++ b/test/integration/template_test.rb @@ -44,16 +44,6 @@ def test_instance_assigns_persist_on_same_template_object_between_parses assert_equal('from instance assigns', t.parse("{{ foo }}").render!) end - def test_warnings_is_not_exponential_time - str = "false" - 100.times do - str = "{% if true %}true{% else %}#{str}{% endif %}" - end - - t = Template.parse(str) - assert_equal([], Timeout.timeout(1) { t.warnings }) - end - def test_instance_assigns_persist_on_same_template_parsing_between_renders t = Template.new.parse("{{ foo }}{% assign foo = 'foo' %}{{ foo }}") assert_equal('foo', t.render!) @@ -259,7 +249,7 @@ def test_undefined_variables end def test_nil_value_does_not_raise - t = Template.parse("some{{x}}thing", error_mode: :strict) + t = Template.parse("some{{x}}thing") result = t.render!({ 'x' => nil }, strict_variables: true) assert_equal(0, t.errors.count) diff --git a/test/integration/variable_test.rb b/test/integration/variable_test.rb index e272b96b9..38096e41d 100644 --- a/test/integration/variable_test.rb +++ b/test/integration/variable_test.rb @@ -176,102 +176,33 @@ def test_double_nested_variable_lookup ) end - def test_variable_lookup_should_not_hang_with_invalid_syntax - Timeout.timeout(1) do - assert_template_result( - 'bar', - "{{['foo'}}", - { - 'foo' => 'bar', - }, - error_mode: :lax, - ) - end - - very_long_key = "1234567890" * 100 - - template_list = [ - "{{['#{very_long_key}']}}", # valid - "{{['#{very_long_key}'}}", # missing closing bracket - "{{[['#{very_long_key}']}}", # extra open bracket - ] - - template_list.each do |template| - Timeout.timeout(1) do - assert_template_result( - 'bar', - template, - { - very_long_key => 'bar', - }, - error_mode: :lax, - ) - end - end - end - def test_filter_with_single_trailing_comma template = '{{ "hello" | append: "world", }}' - with_error_modes(:strict) do - error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } - assert_match(/is not a valid expression/, error.message) - end - - with_error_modes(:strict2) do - assert_template_result('helloworld', template) - end + assert_template_result('helloworld', template) end def test_multiple_filters_with_trailing_commas template = '{{ "hello" | append: "1", | append: "2", }}' - with_error_modes(:strict) do - error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } - assert_match(/is not a valid expression/, error.message) - end - - with_error_modes(:strict2) do - assert_template_result('hello12', template) - end + assert_template_result('hello12', template) end def test_filter_with_colon_but_no_arguments template = '{{ "test" | upcase: }}' - with_error_modes(:strict) do - error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } - assert_match(/is not a valid expression/, error.message) - end - - with_error_modes(:strict2) do - assert_template_result('TEST', template) - end + assert_template_result('TEST', template) end def test_filter_chain_with_colon_no_args template = '{{ "test" | append: "x" | upcase: }}' - with_error_modes(:strict) do - error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } - assert_match(/is not a valid expression/, error.message) - end - - with_error_modes(:strict2) do - assert_template_result('TESTX', template) - end + assert_template_result('TESTX', template) end def test_combining_trailing_comma_and_empty_args template = '{{ "test" | append: "x", | upcase: }}' - with_error_modes(:strict) do - error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } - assert_match(/is not a valid expression/, error.message) - end - - with_error_modes(:strict2) do - assert_template_result('TESTX', template) - end + assert_template_result('TESTX', template) end end diff --git a/test/test_helper.rb b/test/test_helper.rb index 293ea4766..0c9596f4e 100755 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -8,13 +8,6 @@ require 'liquid.rb' require 'liquid/profiler' -mode = :strict -if (env_mode = ENV['LIQUID_PARSER_MODE']) - puts "-- #{env_mode.upcase} ERROR MODE" - mode = env_mode.to_sym -end -Liquid::Environment.default.error_mode = mode - if Minitest.const_defined?('Test') # We're on Minitest 5+. Nothing to do here. else @@ -34,27 +27,27 @@ module Assertions def assert_template_result( expected, template, assigns = {}, - message: nil, partials: nil, error_mode: Liquid::Environment.default.error_mode, render_errors: false, + message: nil, partials: nil, render_errors: false, template_factory: nil ) file_system = StubFileSystem.new(partials || {}) environment = Liquid::Environment.build(file_system: file_system) - template = Liquid::Template.parse(template, line_numbers: true, error_mode: error_mode&.to_sym, environment: environment) + template = Liquid::Template.parse(template, line_numbers: true, environment: environment) registers = Liquid::Registers.new(file_system: file_system, template_factory: template_factory) context = Liquid::Context.build(static_environments: assigns, rethrow_errors: !render_errors, registers: registers, environment: environment) output = template.render(context) assert_equal(expected, output, message) end - def assert_match_syntax_error(match, template, error_mode: nil) + def assert_match_syntax_error(match, template) exception = assert_raises(Liquid::SyntaxError) do - Template.parse(template, line_numbers: true, error_mode: error_mode&.to_sym).render + Template.parse(template, line_numbers: true).render end assert_match(match, exception.message) end - def assert_syntax_error(template, error_mode: nil) - assert_match_syntax_error("", template, error_mode: error_mode) + def assert_syntax_error(template) + assert_match_syntax_error("", template) end def assert_usage_increment(name, times: 1) @@ -82,16 +75,6 @@ def with_global_filter(*globals, &blk) Environment.dangerously_override(environment, &blk) end - def with_error_modes(*modes) - old_mode = Liquid::Environment.default.error_mode - modes.each do |mode| - Liquid::Environment.default.error_mode = mode - yield - end - ensure - Liquid::Environment.default.error_mode = old_mode - end - def with_custom_tag(tag_name, tag_class, &block) environment = Liquid::Environment.default.dup environment.register_tag(tag_name, tag_class) diff --git a/test/unit/condition_unit_test.rb b/test/unit/condition_unit_test.rb index eb466f7a9..584951e21 100644 --- a/test/unit/condition_unit_test.rb +++ b/test/unit/condition_unit_test.rb @@ -166,35 +166,25 @@ def test_default_context_is_deprecated assert_includes(err.lines.map(&:strip), expected) end - def test_parse_expression_in_strict_mode - environment = Environment.build(error_mode: :strict) + def test_parse_expression_with_safe_true + environment = Environment.build parse_context = ParseContext.new(environment: environment) - result = Condition.parse_expression(parse_context, 'product.title') + result = Condition.parse_expression(parse_context, 'product.title', safe: true) assert_instance_of(VariableLookup, result) assert_equal('product', result.name) assert_equal(['title'], result.lookups) end - def test_parse_expression_in_strict2_mode_raises_internal_error - environment = Environment.build(error_mode: :strict2) + def test_parse_expression_raises_internal_error_if_not_safe + environment = Environment.build parse_context = ParseContext.new(environment: environment) error = assert_raises(Liquid::InternalError) do Condition.parse_expression(parse_context, 'product.title') end - assert_match(/unsafe parse_expression cannot be used in strict2 mode/, error.message) - end - - def test_parse_expression_with_safe_true_in_strict2_mode - environment = Environment.build(error_mode: :strict2) - parse_context = ParseContext.new(environment: environment) - result = Condition.parse_expression(parse_context, 'product.title', safe: true) - - assert_instance_of(VariableLookup, result) - assert_equal('product', result.name) - assert_equal(['title'], result.lookups) + assert_match(/unsafe parse_expression cannot be used/, error.message) end private diff --git a/test/unit/parse_context_unit_test.rb b/test/unit/parse_context_unit_test.rb index f53cd1148..f75a48045 100644 --- a/test/unit/parse_context_unit_test.rb +++ b/test/unit/parse_context_unit_test.rb @@ -6,118 +6,81 @@ class ParseContextUnitTest < Minitest::Test include Liquid def test_safe_parse_expression_with_variable_lookup - parser_strict = strict_parse_context.new_parser('product.title') - result_strict = strict_parse_context.safe_parse_expression(parser_strict) + parser = parse_context.new_parser('product.title') + result = parse_context.safe_parse_expression(parser) - parser_strict2 = strict2_parse_context.new_parser('product.title') - result_strict2 = strict2_parse_context.safe_parse_expression(parser_strict2) - - assert_instance_of(VariableLookup, result_strict) - assert_equal('product', result_strict.name) - assert_equal(['title'], result_strict.lookups) - - assert_instance_of(VariableLookup, result_strict2) - assert_equal('product', result_strict2.name) - assert_equal(['title'], result_strict2.lookups) + assert_instance_of(VariableLookup, result) + assert_equal('product', result.name) + assert_equal(['title'], result.lookups) end def test_safe_parse_expression_raises_syntax_error_for_invalid_expression - parser_strict = strict_parse_context.new_parser('') - parser_strict2 = strict2_parse_context.new_parser('') - - error_strict = assert_raises(Liquid::SyntaxError) do - strict_parse_context.safe_parse_expression(parser_strict) - end - assert_match(/is not a valid expression/, error_strict.message) + parser = parse_context.new_parser('') - error_strict2 = assert_raises(Liquid::SyntaxError) do - strict2_parse_context.safe_parse_expression(parser_strict2) + error = assert_raises(Liquid::SyntaxError) do + parse_context.safe_parse_expression(parser) end - assert_match(/is not a valid expression/, error_strict2.message) + assert_match(/is not a valid expression/, error.message) end def test_parse_expression_with_variable_lookup - result_strict = strict_parse_context.parse_expression('product.title') - - assert_instance_of(VariableLookup, result_strict) - assert_equal('product', result_strict.name) - assert_equal(['title'], result_strict.lookups) - error = assert_raises(Liquid::InternalError) do - strict2_parse_context.parse_expression('product.title') + parse_context.parse_expression('product.title') end - assert_match(/unsafe parse_expression cannot be used in strict2 mode/, error.message) + assert_match(/unsafe parse_expression cannot be used/, error.message) end def test_parse_expression_with_safe_true - result_strict = strict_parse_context.parse_expression('product.title', safe: true) - - assert_instance_of(VariableLookup, result_strict) - assert_equal('product', result_strict.name) - assert_equal(['title'], result_strict.lookups) + result = parse_context.parse_expression('product.title', safe: true) - result_strict2 = strict2_parse_context.parse_expression('product.title', safe: true) - - assert_instance_of(VariableLookup, result_strict2) - assert_equal('product', result_strict2.name) - assert_equal(['title'], result_strict2.lookups) + assert_instance_of(VariableLookup, result) + assert_equal('product', result.name) + assert_equal(['title'], result.lookups) end def test_parse_expression_with_empty_string - result_strict = strict_parse_context.parse_expression('') - assert_nil(result_strict) - error = assert_raises(Liquid::InternalError) do - strict2_parse_context.parse_expression('') + parse_context.parse_expression('') end - assert_match(/unsafe parse_expression cannot be used in strict2 mode/, error.message) + assert_match(/unsafe parse_expression cannot be used/, error.message) end def test_parse_expression_with_empty_string_and_safe_true - result_strict = strict_parse_context.parse_expression('', safe: true) - assert_nil(result_strict) - - result_strict2 = strict2_parse_context.parse_expression('', safe: true) - assert_nil(result_strict2) + result = parse_context.parse_expression('', safe: true) + assert_nil(result) end def test_safe_parse_expression_advances_parser_pointer - parser = strict2_parse_context.new_parser('foo, bar') + parser = parse_context.new_parser('foo, bar') # safe_parse_expression consumes "foo" - first_result = strict2_parse_context.safe_parse_expression(parser) + first_result = parse_context.safe_parse_expression(parser) assert_instance_of(VariableLookup, first_result) assert_equal('foo', first_result.name) parser.consume(:comma) # safe_parse_expression consumes "bar" - second_result = strict2_parse_context.safe_parse_expression(parser) + second_result = parse_context.safe_parse_expression(parser) assert_instance_of(VariableLookup, second_result) assert_equal('bar', second_result.name) parser.consume(:end_of_string) end - def test_parse_expression_with_whitespace_in_strict2_mode - result = strict2_parse_context.parse_expression(' ', safe: true) + def test_parse_expression_with_whitespace + result = parse_context.parse_expression(' ', safe: true) assert_nil(result) end private - def strict_parse_context - @strict_parse_context ||= ParseContext.new( - environment: Environment.build(error_mode: :strict), - ) - end - - def strict2_parse_context - @strict2_parse_context ||= ParseContext.new( - environment: Environment.build(error_mode: :strict2), + def parse_context + @parse_context ||= ParseContext.new( + environment: Environment.build, ) end end diff --git a/test/unit/partial_cache_unit_test.rb b/test/unit/partial_cache_unit_test.rb index 623241b40..cb8e2d354 100644 --- a/test/unit/partial_cache_unit_test.rb +++ b/test/unit/partial_cache_unit_test.rb @@ -175,7 +175,7 @@ def test_uses_template_name_from_template_factory assert_equal('some/path/my_partial', partial.name) end - def test_includes_error_mode_into_template_cache + def test_cache_key template_factory = StubTemplateFactory.new context = Liquid::Context.build( registers: { @@ -184,16 +184,14 @@ def test_includes_error_mode_into_template_cache }, ) - [:lax, :warn, :strict, :strict2].each do |error_mode| - Liquid::PartialCache.load( - 'my_partial', - context: context, - parse_context: Liquid::ParseContext.new(error_mode: error_mode), - ) - end + Liquid::PartialCache.load( + 'my_partial', + context: context, + parse_context: Liquid::ParseContext.new, + ) assert_equal( - ["my_partial:lax", "my_partial:warn", "my_partial:strict", "my_partial:strict2"], + ["my_partial"], context.registers[:cached_partials].keys, ) end diff --git a/test/unit/tags/case_tag_unit_test.rb b/test/unit/tags/case_tag_unit_test.rb index 9d7d34a39..0d70c6c0a 100644 --- a/test/unit/tags/case_tag_unit_test.rb +++ b/test/unit/tags/case_tag_unit_test.rb @@ -20,15 +20,9 @@ def test_case_with_trailing_element {%- endcase -%} LIQUID - with_error_modes(:lax, :strict) do - assert_template_result("one", template) - end + error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } - with_error_modes(:strict2) do - error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } - - assert_match(/Expected end_of_string but found/, error.message) - end + assert_match(/Expected end_of_string but found/, error.message) end def test_case_when_with_trailing_element @@ -41,15 +35,9 @@ def test_case_when_with_trailing_element {%- endcase -%} LIQUID - with_error_modes(:lax, :strict) do - assert_template_result("one", template) - end - - with_error_modes(:strict2) do - error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } + error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } - assert_match(/Expected end_of_string but found/, error.message) - end + assert_match(/Expected end_of_string but found/, error.message) end def test_case_when_with_comma @@ -62,9 +50,7 @@ def test_case_when_with_comma {%- endcase -%} LIQUID - with_error_modes(:lax, :strict, :strict2) do - assert_template_result("one", template) - end + assert_template_result("one", template) end def test_case_when_with_or @@ -77,9 +63,7 @@ def test_case_when_with_or {%- endcase -%} LIQUID - with_error_modes(:lax, :strict, :strict2) do - assert_template_result("one", template) - end + assert_template_result("one", template) end def test_case_with_invalid_expression @@ -93,15 +77,9 @@ def test_case_with_invalid_expression LIQUID assigns = { 'foo' => { 'bar' => 'baz' } } - with_error_modes(:lax, :strict) do - assert_template_result("one", template, assigns) - end + error = assert_raises(Liquid::SyntaxError) { Template.parse(template, assigns) } - with_error_modes(:strict2) do - error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } - - assert_match(/Unexpected character =/, error.message) - end + assert_match(/Unexpected character =/, error.message) end def test_case_when_with_invalid_expression @@ -115,14 +93,8 @@ def test_case_when_with_invalid_expression LIQUID assigns = { 'foo' => { 'bar' => 'baz' } } - with_error_modes(:lax, :strict) do - assert_template_result("one", template, assigns) - end - - with_error_modes(:strict2) do - error = assert_raises(Liquid::SyntaxError) { Template.parse(template) } + error = assert_raises(Liquid::SyntaxError) { Template.parse(template, assigns) } - assert_match(/Unexpected character =/, error.message) - end + assert_match(/Unexpected character =/, error.message) end end diff --git a/test/unit/variable_unit_test.rb b/test/unit/variable_unit_test.rb index 03f5862aa..6379c6ece 100644 --- a/test/unit/variable_unit_test.rb +++ b/test/unit/variable_unit_test.rb @@ -72,12 +72,6 @@ def test_filters_without_whitespace 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) - end - def test_string_to_filter var = create_variable("'http://disney.com/logo.gif' | image: 'med' ") assert_equal('http://disney.com/logo.gif', var.name) @@ -108,11 +102,9 @@ 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) - with_error_modes(:strict) do - assert_raises(Liquid::SyntaxError) { create_variable('foo - bar') } - assert_raises(Liquid::SyntaxError) { create_variable('-foo') } - assert_raises(Liquid::SyntaxError) { create_variable('2foo') } - end + assert_raises(Liquid::SyntaxError) { create_variable('foo - bar') } + assert_raises(Liquid::SyntaxError) { create_variable('-foo') } + assert_raises(Liquid::SyntaxError) { create_variable('2foo') } end def test_string_with_special_chars @@ -131,70 +123,38 @@ def test_filter_with_keyword_arguments 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([['pluralize', ['comment', 'comments']]], var.filters) + def test_filter_argument_parsing + # optional colon + var = create_variable(%(n | f1 | f2:)) + assert_equal([['f1', []], ['f2', []]], var.filters) + + # missing argument throws error + assert_raises(SyntaxError) { create_variable(%(n | f1: ,)) } + assert_raises(SyntaxError) { create_variable(%(n | f1: ,| f2)) } + + # arg requires colon + assert_raises(SyntaxError) { create_variable(%(n | f1 1)) } - # missing does not throws error - create_variable(%(n | f1: ,), error_mode: :lax) - create_variable(%(n | f1: ,| f2), error_mode: :lax) + # trailing comma doesn't throw + create_variable(%(n | f1: 1, 2, 3, | f2:)) - # arg does not require colon, but ignores args :O, also ignores first kwarg since it splits on ':' - var = create_variable(%(n | f1 1 | f2 k1: v1), error_mode: :lax) - assert_equal([['f1', []], ['f2', [VariableLookup.new('v1')]]], var.filters) + # missing comma throws error + assert_raises(SyntaxError) { create_variable(%(n | filter: 1 2, 3)) } # positional and kwargs parsing - var = create_variable(%(n | filter: 1, 2, 3 | filter2: k1: 1, k2: 2), error_mode: :lax) + var = create_variable(%(n | filter: 1, 2, 3 | filter2: k1: 1, k2: 2)) assert_equal([['filter', [1, 2, 3]], ['filter2', [], { "k1" => 1, "k2" => 2 }]], var.filters) + # positional and kwargs mixed + var = create_variable(%(n | filter: 'a', 'b', key1: 1, key2: 2, 'c')) + assert_equal([["filter", ["a", "b", "c"], { "key1" => 1, "key2" => 2 }]], var.filters) + # positional and kwargs intermixed (pos1, key1: val1, pos2) - var = create_variable(%(n | link_to: class: "black", "https://example.com", title: "title"), error_mode: :lax) + var = create_variable(%(n | link_to: class: "black", "https://example.com", title: "title")) assert_equal([['link_to', ["https://example.com"], { "class" => "black", "title" => "title" }]], var.filters) - end - - def test_strict_filter_argument_parsing - with_error_modes(:strict) do - assert_raises(SyntaxError) do - create_variable(%( number_of_comments | pluralize: 'comment': 'comments' )) - end - end - end - - def test_strict2_filter_argument_parsing - with_error_modes(:strict2) do - # optional colon - var = create_variable(%(n | f1 | f2:)) - assert_equal([['f1', []], ['f2', []]], var.filters) - - # missing argument throws error - assert_raises(SyntaxError) { create_variable(%(n | f1: ,)) } - assert_raises(SyntaxError) { create_variable(%(n | f1: ,| f2)) } - - # arg requires colon - assert_raises(SyntaxError) { create_variable(%(n | f1 1)) } - - # trailing comma doesn't throw - create_variable(%(n | f1: 1, 2, 3, | f2:)) - - # missing comma throws error - assert_raises(SyntaxError) { create_variable(%(n | filter: 1 2, 3)) } - - # positional and kwargs parsing - var = create_variable(%(n | filter: 1, 2, 3 | filter2: k1: 1, k2: 2)) - assert_equal([['filter', [1, 2, 3]], ['filter2', [], { "k1" => 1, "k2" => 2 }]], var.filters) - - # positional and kwargs mixed - var = create_variable(%(n | filter: 'a', 'b', key1: 1, key2: 2, 'c')) - assert_equal([["filter", ["a", "b", "c"], { "key1" => 1, "key2" => 2 }]], var.filters) - - # positional and kwargs intermixed (pos1, key1: val1, pos2) - var = create_variable(%(n | link_to: class: "black", "https://example.com", title: "title")) - assert_equal([['link_to', ["https://example.com"], { "class" => "black", "title" => "title" }]], var.filters) - # string key throws - assert_raises(SyntaxError) { create_variable(%(n | pluralize: 'comment': 'comments')) } - end + # string key throws + assert_raises(SyntaxError) { create_variable(%(n | pluralize: 'comment': 'comments')) } end def test_output_raw_source_of_variable