diff --git a/History.md b/History.md index 95af4940e..237ef2c9a 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.14.0 * Avoid materializing integer ranges in `for` and `tablerow` loops, and account for each visited range item [Ian Ker-Seymer] diff --git a/README.md b/README.md index 2c4362e00..cc047321f 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 e14d49aa6..d9b5b4303 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/block_body.rb b/lib/liquid/block_body.rb index 90e7c8237..bab80cbae 100644 --- a/lib/liquid/block_body.rb +++ b/lib/liquid/block_body.rb @@ -86,24 +86,19 @@ def self.raise_missing_variable_terminator(token, parse_context) def self.render_node(context, output, node) node.render_to_output_buffer(context, output) rescue => exc - blank_tag = !node.instance_of?(Variable) && node.blank? - rescue_render_node(context, output, node.line_number, exc, blank_tag) + rescue_render_node(context, output, node.line_number, exc) end # @api private - def self.rescue_render_node(context, output, line_number, exc, blank_tag) + # The unused blank_tag argument keeps the liquid-c call signature compatible. + def self.rescue_render_node(context, output, line_number, exc, _blank_tag = false) case exc when MemoryError raise when UndefinedVariable, UndefinedDropMethod, UndefinedFilter context.handle_error(exc, line_number) else - error_message = context.handle_error(exc, line_number) - error_mode = context.registers.static[:template_error_mode] - suppress_error_text = blank_tag && error_mode != :strict2 && error_mode != :rigid - unless suppress_error_text # blank-tag suppression is kept for backwards compatibility outside strict2 - output << error_message - end + output << context.handle_error(exc, line_number) end end diff --git a/lib/liquid/context.rb b/lib/liquid/context.rb index 30492578e..5b8310282 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 @@ -262,7 +257,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 7e15f85b0..f76f6a8b4 100644 --- a/lib/liquid/expression.rb +++ b/lib/liquid/expression.rb @@ -13,9 +13,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 d956d1c61..21f6a963f 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 @@ -38,7 +37,7 @@ def new_block_body def new_parser(input) @string_scanner.string = input - Parser.new(@string_scanner, reject_bare_brackets: @error_mode == :strict2 || @error_mode == :rigid) + Parser.new(@string_scanner, reject_bare_brackets: true) end def new_tokenizer(source, start_line_number: nil, for_liquid_tag: false) @@ -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/assign.rb b/lib/liquid/tags/assign.rb index 901db40f9..74f065f7b 100644 --- a/lib/liquid/tags/assign.rb +++ b/lib/liquid/tags/assign.rb @@ -34,20 +34,7 @@ def initialize(tag_name, markup, parse_context) parse_with_selected_parser(markup) end - def lax_parse(markup) - if markup =~ Syntax - @to = Regexp.last_match(1) - @from = Variable.new(Regexp.last_match(2), parse_context) - else - self.class.raise_syntax_error(parse_context) - end - end - - def strict_parse(markup) - lax_parse(markup) - end - - def strict2_parse(markup) + def parse_markup(markup) unless markup =~ Syntax self.class.raise_syntax_error(parse_context) end diff --git a/lib/liquid/tags/capture.rb b/lib/liquid/tags/capture.rb index 5bebe20e4..9cbd0b088 100644 --- a/lib/liquid/tags/capture.rb +++ b/lib/liquid/tags/capture.rb @@ -22,8 +22,6 @@ module Liquid class Capture < Block include ParserSwitching - Syntax = /(#{VariableSignature}+)/o - attr_reader :to def initialize(tag_name, markup, options) @@ -31,19 +29,7 @@ def initialize(tag_name, markup, options) parse_with_selected_parser(markup) end - def lax_parse(markup) - if markup =~ Syntax - @to = Regexp.last_match(1) - else - raise SyntaxError, options[:locale].t("errors.syntax.capture") - end - end - - def strict_parse(markup) - lax_parse(markup) - end - - def strict2_parse(markup) + def parse_markup(markup) p = @parse_context.new_parser(markup.strip) @to = p.consume(:id) p.consume(:end_of_string) diff --git a/lib/liquid/tags/case.rb b/lib/liquid/tags/case.rb index e87e402e5..ca1d329c9 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/decrement.rb b/lib/liquid/tags/decrement.rb index 70661716d..4e9ebd12e 100644 --- a/lib/liquid/tags/decrement.rb +++ b/lib/liquid/tags/decrement.rb @@ -32,15 +32,7 @@ def initialize(tag_name, markup, options) parse_with_selected_parser(markup) end - def lax_parse(markup) - @variable_name = markup.strip - end - - def strict_parse(markup) - lax_parse(markup) - end - - def strict2_parse(markup) + def parse_markup(markup) p = @parse_context.new_parser(markup.strip) @variable_name = p.consume(:id) p.consume(:end_of_string) diff --git a/lib/liquid/tags/for.rb b/lib/liquid/tags/for.rb index 598d7bee5..34b8b0f84 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 f0161c6e9..110f472f8 100644 --- a/lib/liquid/tags/include.rb +++ b/lib/liquid/tags/include.rb @@ -21,8 +21,6 @@ class Include < Tag prepend Tag::Disableable FOR = 'for' - SYNTAX = /(#{QuotedFragment}+)(\s+(with|#{FOR})\s+(#{QuotedFragment}+))?(\s+(?:as)\s+(#{VariableSegment}+))?/o - Syntax = SYNTAX attr_reader :template_name_expr, :variable_name_expr, :attributes @@ -89,7 +87,7 @@ def for_loop? @is_for_loop end - def strict2_parse(markup) + def parse_markup(markup) p = @parse_context.new_parser(markup) @template_name_expr = safe_parse_expression(p) @@ -111,31 +109,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) - 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 - - else - raise SyntaxError, options[:locale].t("errors.syntax.include") - end - end - class ParseTreeVisitor < Liquid::ParseTreeVisitor def children [ diff --git a/lib/liquid/tags/increment.rb b/lib/liquid/tags/increment.rb index ba4a54d2c..94db10bcb 100644 --- a/lib/liquid/tags/increment.rb +++ b/lib/liquid/tags/increment.rb @@ -32,15 +32,7 @@ def initialize(tag_name, markup, options) parse_with_selected_parser(markup) end - def lax_parse(markup) - @variable_name = markup.strip - end - - def strict_parse(markup) - lax_parse(markup) - end - - def strict2_parse(markup) + def parse_markup(markup) p = @parse_context.new_parser(markup.strip) @variable_name = p.consume(:id) p.consume(:end_of_string) 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 7fd8530dd..083362f27 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 2190e928d..74b3872e1 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 @@ -189,20 +175,12 @@ def render(*args) context.template_name ||= name - previous_error_mode = context.registers.static[:template_error_mode] - context.registers.static[:template_error_mode] = @error_mode - begin # render the nodelist. @root.render_to_output_buffer(context, output || +'') rescue Liquid::MemoryError => e context.handle_error(e) ensure - if previous_error_mode - context.registers.static[:template_error_mode] = previous_error_mode - else - context.registers.static.delete(:template_error_mode) - end @errors = context.errors end end @@ -233,8 +211,6 @@ def configure_options(options) ParseContext.new(opts) end - @warnings = parse_context.warnings - @error_mode = parse_context.error_mode parse_context end diff --git a/lib/liquid/variable.rb b/lib/liquid/variable.rb index 12b8d9f28..4cf11b8d9 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 @@ -45,58 +45,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) @@ -137,22 +96,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?] @@ -160,7 +103,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/spec/ruby_liquid.rb b/spec/ruby_liquid.rb index c78049776..b3e69cd64 100644 --- a/spec/ruby_liquid.rb +++ b/spec/ruby_liquid.rb @@ -23,7 +23,6 @@ # Compile a template string into a Liquid::Template LiquidSpec.compile do |ctx, source, options| - options[:error_mode] ||= :strict ctx[:template] = Liquid::Template.parse(source, **options) end diff --git a/spec/ruby_liquid_lax.rb b/spec/ruby_liquid_lax.rb deleted file mode 100644 index 42cf9a9a3..000000000 --- a/spec/ruby_liquid_lax.rb +++ /dev/null @@ -1,46 +0,0 @@ -# frozen_string_literal: true - -# Liquid Spec Adapter for Shopify/liquid with lax parsing mode -# -# Run with: bundle exec liquid-spec run spec/ruby_liquid_lax.rb - -$LOAD_PATH.unshift(File.expand_path('../lib', __dir__)) -require 'liquid' -require_relative 'support/liquid_spec_adapter_helper' - -LiquidSpec.configure do |config| - config.missing_features = [ - :activesupport, - :shopify_filters, - :shopify_includes, - :shopify_blank, - :shopify_error_handling, - :shopify_error_format, - :shopify_string_access, - ] -end - -# Compile a template string into a Liquid::Template -LiquidSpec.compile do |ctx, source, options| - # Default to lax mode while still honoring specs that explicitly set error_mode. - options = { error_mode: :lax }.merge(options) - ctx[:template] = Liquid::Template.parse(source, **options) -end - -# Render a compiled template with the given context -LiquidSpec.render do |ctx, assigns, options| - registers = Liquid::Registers.new(options[:registers] || {}) - - context = Liquid::Context.build( - static_environments: assigns, - registers: registers, - rethrow_errors: options[:strict_errors], - resource_limits: LiquidSpecAdapterHelper.resource_limits(options), - ) - - context.exception_renderer = options[:exception_renderer] if options[:exception_renderer] - - LiquidSpecAdapterHelper.with_frozen_time do - ctx[:template].render(context) - end -end diff --git a/spec/ruby_liquid_with_active_support.rb b/spec/ruby_liquid_with_active_support.rb index 25e02b3f3..9ffbc6a2c 100644 --- a/spec/ruby_liquid_with_active_support.rb +++ b/spec/ruby_liquid_with_active_support.rb @@ -23,7 +23,6 @@ # Compile a template string into a Liquid::Template LiquidSpec.compile do |ctx, source, options| - options[:error_mode] ||= :strict ctx[:template] = Liquid::Template.parse(source, **options) end diff --git a/spec/ruby_liquid_yjit.rb b/spec/ruby_liquid_yjit.rb index f43f5ac70..6ea103f6f 100644 --- a/spec/ruby_liquid_yjit.rb +++ b/spec/ruby_liquid_yjit.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Liquid Spec Adapter for Shopify/liquid with YJIT + strict mode + ActiveSupport +# Liquid Spec Adapter for Shopify/liquid with YJIT + ActiveSupport # # Run with: bundle exec liquid-spec run spec/ruby_liquid_yjit.rb @@ -29,8 +29,6 @@ # Compile a template string into a Liquid::Template LiquidSpec.compile do |ctx, source, options| - # Force strict mode - options = { error_mode: :strict }.merge(options) ctx[:template] = Liquid::Template.parse(source, **options) end diff --git a/test/integration/assign_test.rb b/test/integration/assign_test.rb index 2388b357d..5164ceacf 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 @@ -97,44 +95,40 @@ def test_assign_score_of_hash assert_equal(12, assign_score_of('int' => 123, 'str' => 'abcd')) end - def test_assign_with_valid_identifier_in_strict2 - assert_template_result("hello", "{% assign my_var = 'hello' %}{{ my_var }}", error_mode: :strict2) + def test_assign_with_valid_identifier + assert_template_result("hello", "{% assign my_var = 'hello' %}{{ my_var }}") end - def test_assign_with_hyphen_in_strict2 - assert_template_result("hello", "{% assign my-var = 'hello' %}{{ my-var }}", error_mode: :strict2) + def test_assign_with_hyphen + assert_template_result("hello", "{% assign my-var = 'hello' %}{{ my-var }}") end - def test_assign_rejects_parentheses_in_variable_name_in_strict2 + def test_assign_rejects_parentheses_in_variable_name assert_raises(Liquid::SyntaxError) do - Liquid::Template.parse("{% assign (a(b(c) = 1234 %}", error_mode: :strict2) + Liquid::Template.parse("{% assign (a(b(c) = 1234 %}") end end - def test_assign_rejects_brackets_in_variable_name_in_strict2 + def test_assign_rejects_brackets_in_variable_name assert_raises(Liquid::SyntaxError) do - Liquid::Template.parse("{% assign [x.y] = 'hello' %}", error_mode: :strict2) + Liquid::Template.parse("{% assign [x.y] = 'hello' %}") end end - def test_assign_rejects_dot_in_variable_name_in_strict2 + def test_assign_rejects_dot_in_variable_name assert_raises(Liquid::SyntaxError) do - Liquid::Template.parse("{% assign a.b = 'hello' %}", error_mode: :strict2) + Liquid::Template.parse("{% assign a.b = 'hello' %}") end end - def test_assign_rejects_numeric_variable_name_in_strict2 + def test_assign_rejects_numeric_variable_name assert_raises(Liquid::SyntaxError) do - Liquid::Template.parse("{% assign 1abc = 'hello' %}", error_mode: :strict2) + Liquid::Template.parse("{% assign 1abc = 'hello' %}") end end - def test_assign_allows_invalid_names_in_lax - assert_template_result("1234", "{% assign (a(b(c) = 1234 %}{{ self['(a(b(c)'] }}", error_mode: :lax) - end - - def test_assign_with_filter_in_strict2 - assert_template_result("HELLO", "{% assign my_var = 'hello' | upcase %}{{ my_var }}", error_mode: :strict2) + def test_assign_with_filter_to_valid_identifier + assert_template_result("HELLO", "{% assign my_var = 'hello' | upcase %}{{ my_var }}") end private diff --git a/test/integration/blank_body_error_handling_test.rb b/test/integration/blank_body_error_handling_test.rb index c395f867e..2c3ded75b 100644 --- a/test/integration/blank_body_error_handling_test.rb +++ b/test/integration/blank_body_error_handling_test.rb @@ -6,80 +6,56 @@ class BlankBodyErrorHandlingTest < Minitest::Test COMPARISON_ERROR = 'Liquid error (line 1): comparison of Integer with String failed' INVALID_INTEGER_ERROR = 'Liquid error (line 1): invalid integer' - def render_inline(source, error_mode:, assigns: {}) - Liquid::Template.parse(source, line_numbers: true, error_mode: error_mode).render(assigns, render_errors: true) + def render_inline(source, assigns: {}) + Liquid::Template.parse(source, line_numbers: true).render(assigns, render_errors: true) end - def assert_render_raises(source, error_mode:, assigns: {}, message: nil) + def assert_render_raises(source, assigns: {}, message: nil) error = assert_raises(Liquid::ArgumentError) do - Liquid::Template.parse(source, line_numbers: true, error_mode: error_mode).render!(assigns) + Liquid::Template.parse(source, line_numbers: true).render!(assigns) end assert_includes(error.message, message) if message end - def test_blank_if_body_suppresses_inline_error_text_in_lax_and_strict - [:lax, :strict].each do |mode| - assert_equal('', render_inline('{% if 5 > "x" %}{% endif %}', error_mode: mode)) - end - end - - def test_blank_unless_body_suppresses_inline_error_text_in_lax_and_strict - [:lax, :strict].each do |mode| - assert_equal('', render_inline('{% unless 5 > "x" %} {% endunless %}', error_mode: mode)) - end - end - - def test_blank_for_body_suppresses_inline_error_text_in_lax_and_strict - [:lax, :strict].each do |mode| - assert_equal('', render_inline('{% for i in (1..3) offset: xs %}{% endfor %}', error_mode: mode, assigns: { 'xs' => 'bad' })) - end - end - - def test_strict2_blank_if_body_shows_inline_error_text - assert_equal(COMPARISON_ERROR, render_inline('{% if 5 > "x" %}{% endif %}', error_mode: :strict2)) + def test_blank_if_body_shows_inline_error_text + assert_equal(COMPARISON_ERROR, render_inline('{% if 5 > "x" %}{% endif %}')) end - def test_strict2_whitespace_if_body_shows_inline_error_text - assert_equal(COMPARISON_ERROR, render_inline('{% if 5 > "x" %} {% endif %}', error_mode: :strict2)) + def test_whitespace_if_body_shows_inline_error_text + assert_equal(COMPARISON_ERROR, render_inline('{% if 5 > "x" %} {% endif %}')) end - def test_strict2_assign_if_body_shows_inline_error_text - assert_equal(COMPARISON_ERROR, render_inline('{% if 5 > "x" %}{% assign a = 1 %}{% endif %}', error_mode: :strict2)) + def test_assign_if_body_shows_inline_error_text + assert_equal(COMPARISON_ERROR, render_inline('{% if 5 > "x" %}{% assign a = 1 %}{% endif %}')) end - def test_strict2_comment_if_body_shows_inline_error_text - assert_equal(COMPARISON_ERROR, render_inline('{% if 5 > "x" %}{% comment %}c{% endcomment %}{% endif %}', error_mode: :strict2)) + def test_comment_if_body_shows_inline_error_text + assert_equal(COMPARISON_ERROR, render_inline('{% if 5 > "x" %}{% comment %}c{% endcomment %}{% endif %}')) end - def test_strict2_capture_if_body_shows_inline_error_text - assert_equal(COMPARISON_ERROR, render_inline('{% if 5 > "x" %}{% capture c %}text{% endcapture %}{% endif %}', error_mode: :strict2)) + def test_capture_if_body_shows_inline_error_text + assert_equal(COMPARISON_ERROR, render_inline('{% if 5 > "x" %}{% capture c %}text{% endcapture %}{% endif %}')) end - def test_strict2_blank_unless_body_shows_inline_error_text - assert_equal(COMPARISON_ERROR, render_inline('{% unless 5 > "x" %} {% endunless %}', error_mode: :strict2)) + def test_blank_unless_body_shows_inline_error_text + assert_equal(COMPARISON_ERROR, render_inline('{% unless 5 > "x" %} {% endunless %}')) end - def test_strict2_blank_for_body_shows_inline_error_text - assert_equal(INVALID_INTEGER_ERROR, render_inline('{% for i in (1..3) offset: xs %}{% endfor %}', error_mode: :strict2, assigns: { 'xs' => 'bad' })) + def test_blank_for_body_shows_inline_error_text + assert_equal(INVALID_INTEGER_ERROR, render_inline('{% for i in (1..3) offset: xs %}{% endfor %}', assigns: { 'xs' => 'bad' })) end - def test_nonblank_bodies_show_inline_error_text_in_all_modes - [:lax, :strict, :strict2].each do |mode| - assert_equal(COMPARISON_ERROR, render_inline('{% if 5 > "x" %}{% echo 1 %}{% endif %}', error_mode: mode)) - assert_equal(COMPARISON_ERROR, render_inline('{% if 5 > "x" %}{{ "" }}{% endif %}', error_mode: mode)) - assert_equal(COMPARISON_ERROR, render_inline('{% if 5 > "x" %}{% else %}E{% endif %}', error_mode: mode)) - end + def test_nonblank_bodies_show_inline_error_text + assert_equal(COMPARISON_ERROR, render_inline('{% if 5 > "x" %}{% echo 1 %}{% endif %}')) + assert_equal(COMPARISON_ERROR, render_inline('{% if 5 > "x" %}{{ "" }}{% endif %}')) + assert_equal(COMPARISON_ERROR, render_inline('{% if 5 > "x" %}{% else %}E{% endif %}')) end def test_raised_errors_are_not_swallowed_by_blank_if_body - [:lax, :strict, :strict2].each do |mode| - assert_render_raises('{% if 5 > "x" %}{% endif %}', error_mode: mode, message: 'comparison of Integer with String failed') - end + assert_render_raises('{% if 5 > "x" %}{% endif %}', message: 'comparison of Integer with String failed') end def test_raised_errors_are_not_swallowed_by_blank_for_body - [:lax, :strict, :strict2].each do |mode| - assert_render_raises('{% for i in (1..3) offset: xs %}{% endfor %}', error_mode: mode, assigns: { 'xs' => 'bad' }, message: 'invalid integer') - end + assert_render_raises('{% for i in (1..3) offset: xs %}{% endfor %}', assigns: { 'xs' => 'bad' }, message: 'invalid integer') end end diff --git a/test/integration/capture_test.rb b/test/integration/capture_test.rb index 2f844a7c0..25828e2a3 100644 --- a/test/integration/capture_test.rb +++ b/test/integration/capture_test.rb @@ -9,10 +9,6 @@ def test_captures_block_content_in_variable assert_template_result("test string", "{% capture var %}test string{% endcapture %}{{var}}", {}) end - def test_captures_block_content_in_quoted_variable_in_lax - assert_template_result("test string", "{% capture 'var' %}test string{% endcapture %}{{var}}", {}, error_mode: :lax) - end - def test_capture_with_hyphen_in_variable_name template_source = <<~END_TEMPLATE {% capture this-thing %}Print this-thing{% endcapture -%} @@ -54,34 +50,29 @@ def test_increment_assign_score_by_bytes_not_characters assert_equal(9, t.resource_limits.assign_score) end - def test_capture_with_valid_identifier_in_strict2 - assert_template_result("hello", "{% capture my_var %}hello{% endcapture %}{{ my_var }}", error_mode: :strict2) + def test_capture_with_valid_identifier + assert_template_result("hello", "{% capture my_var %}hello{% endcapture %}{{ my_var }}") end - def test_capture_with_hyphen_in_strict2 - assert_template_result("hello", "{% capture my-var %}hello{% endcapture %}{{ my-var }}", error_mode: :strict2) + def test_capture_with_hyphen + assert_template_result("hello", "{% capture my-var %}hello{% endcapture %}{{ my-var }}") end - def test_capture_rejects_parentheses_in_variable_name_in_strict2 + def test_capture_rejects_parentheses_in_variable_name assert_raises(Liquid::SyntaxError) do - Liquid::Template.parse("{% capture (x[y %}hello{% endcapture %}", error_mode: :strict2) + Liquid::Template.parse("{% capture (x[y %}hello{% endcapture %}") end end - def test_capture_rejects_dot_in_variable_name_in_strict2 + def test_capture_rejects_dot_in_variable_name assert_raises(Liquid::SyntaxError) do - Liquid::Template.parse("{% capture a.b %}hello{% endcapture %}", error_mode: :strict2) + Liquid::Template.parse("{% capture a.b %}hello{% endcapture %}") end end - def test_capture_rejects_numeric_variable_name_in_strict2 + def test_capture_rejects_numeric_variable_name assert_raises(Liquid::SyntaxError) do - Liquid::Template.parse("{% capture 1abc %}hello{% endcapture %}", error_mode: :strict2) + Liquid::Template.parse("{% capture 1abc %}hello{% endcapture %}") end end - - def test_capture_allows_invalid_names_in_lax - t = Liquid::Template.parse("{% capture (x[y %}hello{% endcapture %}", error_mode: :lax) - assert_equal("(x[y", t.root.nodelist.first.to) - end end diff --git a/test/integration/context_test.rb b/test/integration/context_test.rb index 8fa83ddb5..88712e07a 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 e2e3ed111..248b09d97 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 @@ -264,16 +217,6 @@ def test_included_template_name_with_line_numbers assert_equal("product", template.errors.first.template_name) end - def test_bug_compatible_silencing_of_errors_in_blank_nodes - with_error_modes(:lax, :strict) do - output = Liquid::Template.parse("{% assign x = 0 %}{% if 1 < '2' %}not blank{% assign x = 3 %}{% endif %}{{ x }}").render - assert_equal("Liquid error: comparison of Integer with String failed0", output) - - output = Liquid::Template.parse("{% assign x = 0 %}{% if 1 < '2' %}{% assign x = 3 %}{% endif %}{{ x }}").render - assert_equal("0", output) - end - end - def test_syntax_error_is_raised_with_template_name file_system = StubFileSystem.new("snippet" => "1\n2\n{{ 1") 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 b0cdf606f..3e5c30a34 100644 --- a/test/integration/tags/cycle_tag_test.rb +++ b/test/integration/tags/cycle_tag_test.rb @@ -91,26 +91,19 @@ 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) } - assert_match(/Liquid syntax error:/, error1.message) - assert_match(/Liquid syntax error: \[:dot, "."\] is not a valid expression/, error2.message) - end + assert_match(/Liquid syntax error:/, error1.message) + assert_match(/Liquid syntax error: \[:dot, "."\] is not a valid expression/, 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' %}" @@ -119,29 +112,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 @@ -151,14 +134,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 @@ -168,13 +145,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 bd4085f21..ee07178c7 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,84 +388,56 @@ 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 def test_include_for_loop_true_with_for_keyword - with_error_modes(:lax, :strict, :strict2) do - template = Template.parse("{% include 'product' for products %}") - include_node = template.root.nodelist.first + template = Template.parse("{% include 'product' for products %}") + include_node = template.root.nodelist.first - assert(include_node.for_loop?, "Expected for_loop? to be true for 'for' keyword") - end + assert(include_node.for_loop?, "Expected for_loop? to be true for 'for' keyword") end def test_include_for_loop_false_with_with_keyword - with_error_modes(:lax, :strict, :strict2) do - template = Template.parse("{% include 'product' with product %}") - include_node = template.root.nodelist.first + template = Template.parse("{% include 'product' with product %}") + include_node = template.root.nodelist.first - refute(include_node.for_loop?, "Expected for_loop? to be false for 'with' keyword") - end + refute(include_node.for_loop?, "Expected for_loop? to be false for 'with' keyword") end def test_include_for_loop_false_without_keyword - with_error_modes(:lax, :strict, :strict2) do - template = Template.parse("{% include 'header' %}") - include_node = template.root.nodelist.first + template = Template.parse("{% include 'header' %}") + include_node = template.root.nodelist.first - refute(include_node.for_loop?, "Expected for_loop? to be false when no keyword") - end + refute(include_node.for_loop?, "Expected for_loop? to be false when no keyword") end def test_include_for_loop_with_alias - with_error_modes(:lax, :strict, :strict2) do - template = Template.parse("{% include 'product' for products as item %}") - include_node = template.root.nodelist.first + template = Template.parse("{% include 'product' for products as item %}") + include_node = template.root.nodelist.first - assert(include_node.for_loop?, "Expected for_loop? to be true for 'for' with alias") - end + assert(include_node.for_loop?, "Expected for_loop? to be true for 'for' with alias") end def test_include_with_keyword_and_alias - with_error_modes(:lax, :strict, :strict2) do - template = Template.parse("{% include 'product' with products[0] as item %}") - include_node = template.root.nodelist.first + template = Template.parse("{% include 'product' with products[0] as item %}") + include_node = template.root.nodelist.first - refute(include_node.for_loop?, "Expected for_loop? to be false for 'with' with alias") - end + refute(include_node.for_loop?, "Expected for_loop? to be false for 'with' with alias") end end # IncludeTagTest diff --git a/test/integration/tags/increment_tag_test.rb b/test/integration/tags/increment_tag_test.rb index bb7245ed5..10b342a9a 100644 --- a/test/integration/tags/increment_tag_test.rb +++ b/test/integration/tags/increment_tag_test.rb @@ -28,49 +28,49 @@ def test_dec ) end - def test_increment_strict2_rejects_invalid_variable_name + def test_increment_rejects_invalid_variable_name assert_raises(Liquid::SyntaxError) do - Template.parse('{% increment foo bar %}', error_mode: :strict2) + Template.parse('{% increment foo bar %}') end end - def test_increment_strict2_rejects_variable_starting_with_number + def test_increment_rejects_variable_starting_with_number assert_raises(Liquid::SyntaxError) do - Template.parse('{% increment 11aa %}', error_mode: :strict2) + Template.parse('{% increment 11aa %}') end end - def test_increment_strict2_accepts_valid_variable_name - template = Template.parse('{% increment my-var %}', error_mode: :strict2) + def test_increment_accepts_valid_variable_name + template = Template.parse('{% increment my-var %}') assert_equal('0', template.render) end - def test_decrement_strict2_rejects_invalid_variable_name + def test_decrement_rejects_invalid_variable_name assert_raises(Liquid::SyntaxError) do - Template.parse('{% decrement foo bar %}', error_mode: :strict2) + Template.parse('{% decrement foo bar %}') end end - def test_decrement_strict2_rejects_variable_starting_with_number + def test_decrement_rejects_variable_starting_with_number assert_raises(Liquid::SyntaxError) do - Template.parse('{% decrement 11aa %}', error_mode: :strict2) + Template.parse('{% decrement 11aa %}') end end - def test_decrement_strict2_accepts_valid_variable_name - template = Template.parse('{% decrement my-var %}', error_mode: :strict2) + def test_decrement_accepts_valid_variable_name + template = Template.parse('{% decrement my-var %}') assert_equal('-1', template.render) end - def test_increment_strict2_rejects_empty_variable_name + def test_increment_rejects_empty_variable_name assert_raises(Liquid::SyntaxError) do - Template.parse('{% increment %}', error_mode: :strict2) + Template.parse('{% increment %}') end end - def test_decrement_strict2_rejects_empty_variable_name + def test_decrement_rejects_empty_variable_name assert_raises(Liquid::SyntaxError) do - Template.parse('{% decrement %}', error_mode: :strict2) + Template.parse('{% decrement %}') end end end diff --git a/test/integration/tags/render_tag_test.rb b/test/integration/tags/render_tag_test.rb index 6773ad251..3fa67f5df 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 2e831f416..9e4469046 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,44 +381,19 @@ 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 def test_integer_range_is_not_materialized_and_charges_empty_iterations diff --git a/test/integration/template_test.rb b/test/integration/template_test.rb index 1e41be06e..7d42af75c 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!) @@ -339,7 +329,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 58f73a315..4accf5176 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 326142b7f..ee1799d98 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 @@ -43,27 +36,27 @@ def bounded_integer_range_with_tripwires 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) @@ -91,16 +84,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 d7f35f609..3f35db59f 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 # Tests for blank? comparison without ActiveSupport 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 11458db80..79212dd69 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_when_empty @@ -92,14 +76,12 @@ def test_case_when_empty {%- endcase -%} LIQUID - with_error_modes(:lax, :strict, :strict2) do - assert_template_result("2 or empty", template, { 'x' => 2 }) - assert_template_result("2 or empty", template, { 'x' => {} }) - assert_template_result("2 or empty", template, { 'x' => [] }) - assert_template_result("not 2 or empty", template, { 'x' => { 'a' => 'b' } }) - assert_template_result("not 2 or empty", template, { 'x' => ['a'] }) - assert_template_result("not 2 or empty", template, { 'x' => 4 }) - end + assert_template_result("2 or empty", template, { 'x' => 2 }) + assert_template_result("2 or empty", template, { 'x' => {} }) + assert_template_result("2 or empty", template, { 'x' => [] }) + assert_template_result("not 2 or empty", template, { 'x' => { 'a' => 'b' } }) + assert_template_result("not 2 or empty", template, { 'x' => ['a'] }) + assert_template_result("not 2 or empty", template, { 'x' => 4 }) end def test_case_with_invalid_expression @@ -113,15 +95,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 @@ -135,14 +111,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