diff --git a/.rubocop.yml b/.rubocop.yml index 6b9aa9f..1c0f832 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,5 +1,5 @@ inherit_from: - - https://shopify.github.io/ruby-style-guide/rubocop.yml + - 'https://shopify.github.io/ruby-style-guide/rubocop.yml' - .rubocop_todo.yml require: rubocop-performance @@ -8,9 +8,10 @@ Performance: Enabled: true AllCops: + TargetRubyVersion: 2.4 Exclude: - 'vendor/bundle/**/*' - + Naming/MethodName: Exclude: - - 'example/server/liquid_servlet.rb' \ No newline at end of file + - 'example/server/liquid_servlet.rb' diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index c1e39a4..70fce22 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -6,26 +6,6 @@ # Note that changes in the inspected code, or installation of new # versions of RuboCop, may require this file to be generated again. -# Offense count: 2 -Lint/AmbiguousOperator: - Exclude: - - 'test/unit/condition_unit_test.rb' - -# Offense count: 21 -# Configuration parameters: AllowSafeAssignment. -Lint/AssignmentInCondition: - Exclude: - - 'lib/liquid/block_body.rb' - - 'lib/liquid/lexer.rb' - - 'lib/liquid/standardfilters.rb' - - 'lib/liquid/tags/for.rb' - - 'lib/liquid/tags/if.rb' - - 'lib/liquid/tags/raw.rb' - - 'lib/liquid/variable.rb' - - 'performance/profile.rb' - - 'test/test_helper.rb' - - 'test/unit/tokenizer_unit_test.rb' - # Offense count: 2 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle. @@ -34,17 +14,6 @@ Lint/InheritException: Exclude: - 'lib/liquid/interrupts.rb' -# Offense count: 2 -Lint/UselessAssignment: - Exclude: - - 'performance/shopify/database.rb' - -# Offense count: 1 -# Configuration parameters: CheckForMethodsWithNoSideEffects. -Lint/Void: - Exclude: - - 'lib/liquid/parse_context.rb' - # Offense count: 98 # Cop supports --auto-correct. # Configuration parameters: AutoCorrect, AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns. @@ -57,19 +26,4 @@ Style/ClassVars: Exclude: - 'lib/liquid/condition.rb' - 'lib/liquid/strainer.rb' - - 'lib/liquid/template.rb' - -# Offense count: 1 -# Configuration parameters: AllowCoercion. -Style/DateTime: - Exclude: - - 'test/unit/context_unit_test.rb' - -# Offense count: 9 -# Cop supports --auto-correct. -# Configuration parameters: AllowAsExpressionSeparator. -Style/Semicolon: - Exclude: - - 'test/integration/error_handling_test.rb' - - 'test/integration/template_test.rb' - - 'test/unit/context_unit_test.rb' + - 'lib/liquid/template.rb' \ No newline at end of file diff --git a/lib/liquid/block_body.rb b/lib/liquid/block_body.rb index 4faba0b..2f3433e 100644 --- a/lib/liquid/block_body.rb +++ b/lib/liquid/block_body.rb @@ -27,7 +27,7 @@ module Liquid end private def parse_for_liquid_tag(tokenizer, parse_context) - while token = tokenizer.shift + while (token = tokenizer.shift) unless token.empty? || token =~ WHITESPACE_OR_NOTHING unless token =~ LIQUID_TAG_TOKEN # line isn't empty but didn't match tag syntax, yield and let the @@ -36,7 +36,7 @@ module Liquid end tag_name = Regexp.last_match(1) markup = Regexp.last_match(2) - unless tag = registered_tags[tag_name] + unless (tag = registered_tags[tag_name]) # end parsing if we reach an unknown tag and let the caller decide # determine how to proceed return yield tag_name, markup @@ -52,7 +52,7 @@ module Liquid end private def parse_for_document(tokenizer, parse_context, &block) - while token = tokenizer.shift + while (token = tokenizer.shift) next if token.empty? case when token.start_with?(TAG_START_STRING) @@ -74,7 +74,7 @@ module Liquid next parse_for_liquid_tag(liquid_tag_tokenizer, parse_context, &block) end - unless tag = registered_tags[tag_name] + unless (tag = registered_tags[tag_name]) # end parsing if we reach an unknown tag and let the caller decide # determine how to proceed return yield tag_name, markup @@ -122,7 +122,7 @@ module Liquid context.resource_limits.render_score += @nodelist.length idx = 0 - while node = @nodelist[idx] + while (node = @nodelist[idx]) previous_output_size = output.bytesize case node diff --git a/lib/liquid/file_system.rb b/lib/liquid/file_system.rb index b2093ae..27ab632 100644 --- a/lib/liquid/file_system.rb +++ b/lib/liquid/file_system.rb @@ -59,7 +59,7 @@ module Liquid end def full_path(template_path) - raise FileSystemError, "Illegal template name '#{template_path}'" unless template_path =~ %r{\A[^./][a-zA-Z0-9_/]+\z} + raise FileSystemError, "Illegal template name '#{template_path}'" unless %r{\A[^./][a-zA-Z0-9_/]+\z}.match?(template_path) full_path = if template_path.include?('/') File.join(root, File.dirname(template_path), @pattern % File.basename(template_path)) diff --git a/lib/liquid/lexer.rb b/lib/liquid/lexer.rb index 04e0c11..a251c3e 100644 --- a/lib/liquid/lexer.rb +++ b/lib/liquid/lexer.rb @@ -33,15 +33,21 @@ module Liquid until @ss.eos? @ss.skip(WHITESPACE_OR_NOTHING) break if @ss.eos? - tok = if t = @ss.scan(COMPARISON_OPERATOR) then [:comparison, t] - elsif t = @ss.scan(SINGLE_STRING_LITERAL) then [:string, t] - elsif t = @ss.scan(DOUBLE_STRING_LITERAL) then [:string, t] - elsif t = @ss.scan(NUMBER_LITERAL) then [:number, t] - elsif t = @ss.scan(IDENTIFIER) then [:id, t] - elsif t = @ss.scan(DOTDOT) then [:dotdot, t] + tok = if (t = @ss.scan(COMPARISON_OPERATOR)) + [:comparison, t] + elsif (t = @ss.scan(SINGLE_STRING_LITERAL)) + [:string, t] + elsif (t = @ss.scan(DOUBLE_STRING_LITERAL)) + [:string, t] + elsif (t = @ss.scan(NUMBER_LITERAL)) + [:number, t] + elsif (t = @ss.scan(IDENTIFIER)) + [:id, t] + elsif (t = @ss.scan(DOTDOT)) + [:dotdot, t] else c = @ss.getch - if s = SPECIALS[c] + if (s = SPECIALS[c]) [s, c] else raise SyntaxError, "Unexpected character #{c}" diff --git a/lib/liquid/parse_context.rb b/lib/liquid/parse_context.rb index 2da3ad7..4afdbe5 100644 --- a/lib/liquid/parse_context.rb +++ b/lib/liquid/parse_context.rb @@ -21,7 +21,6 @@ module Liquid @partial = value @options = value ? partial_options : @template_options @error_mode = @options[:error_mode] || Template.error_mode - value end def partial_options diff --git a/lib/liquid/standardfilters.rb b/lib/liquid/standardfilters.rb index 517857a..6855cd2 100644 --- a/lib/liquid/standardfilters.rb +++ b/lib/liquid/standardfilters.rb @@ -327,7 +327,7 @@ module Liquid def date(input, format) return input if format.to_s.empty? - return input unless date = Utils.to_date(input) + return input unless (date = Utils.to_date(input)) date.strftime(format.to_s) end diff --git a/lib/liquid/static_registers.rb b/lib/liquid/static_registers.rb index 6f22f70..06c52ab 100644 --- a/lib/liquid/static_registers.rb +++ b/lib/liquid/static_registers.rb @@ -2,10 +2,10 @@ module Liquid class StaticRegisters - attr_reader :static_registers, :registers + attr_reader :static, :registers def initialize(registers = {}) - @static_registers = registers.is_a?(StaticRegisters) ? registers.static_registers : registers.freeze + @static = registers.is_a?(StaticRegisters) ? registers.static : registers @registers = {} end @@ -17,7 +17,7 @@ module Liquid if @registers.key?(key) @registers[key] else - @static_registers[key] + @static[key] end end @@ -30,7 +30,7 @@ module Liquid end def key?(key) - @registers.key?(key) || @static_registers.key?(key) + @registers.key?(key) || @static.key?(key) end end end diff --git a/lib/liquid/tags/for.rb b/lib/liquid/tags/for.rb index 2819337..b63b9e1 100644 --- a/lib/liquid/tags/for.rb +++ b/lib/liquid/tags/for.rb @@ -111,7 +111,7 @@ module Liquid @reversed = p.id?('reversed') while p.look(:id) && p.look(:colon, 1) - unless attribute = p.id?('limit') || p.id?('offset') + unless (attribute = p.id?('limit') || p.id?('offset')) raise SyntaxError, options[:locale].t("errors.syntax.for_invalid_attribute") end p.consume diff --git a/lib/liquid/tags/if.rb b/lib/liquid/tags/if.rb index db1a283..50cb334 100644 --- a/lib/liquid/tags/if.rb +++ b/lib/liquid/tags/if.rb @@ -94,7 +94,7 @@ module Liquid def parse_binary_comparisons(p) condition = parse_comparison(p) first_condition = condition - while op = (p.id?('and') || p.id?('or')) + while (op = (p.id?('and') || p.id?('or'))) child_condition = parse_comparison(p) condition.send(op, child_condition) condition = child_condition @@ -104,7 +104,7 @@ module Liquid def parse_comparison(p) a = Expression.parse(p.expression) - if op = p.consume?(:comparison) + if (op = p.consume?(:comparison)) b = Expression.parse(p.expression) Condition.new(a, op, b) else diff --git a/lib/liquid/tags/raw.rb b/lib/liquid/tags/raw.rb index 996a34c..e4b2a6a 100644 --- a/lib/liquid/tags/raw.rb +++ b/lib/liquid/tags/raw.rb @@ -13,7 +13,7 @@ module Liquid def parse(tokens) @body = +'' - while token = tokens.shift + while (token = tokens.shift) if token =~ FULL_TOKEN_POSSIBLY_INVALID @body << Regexp.last_match(1) if Regexp.last_match(1) != "" return if block_delimiter == Regexp.last_match(2) @@ -40,7 +40,7 @@ module Liquid protected def ensure_valid_markup(tag_name, markup, parse_context) - unless markup =~ SYNTAX + unless SYNTAX.match?(markup) raise SyntaxError, parse_context.locale.t("errors.syntax.tag_unexpected_args", tag: tag_name) end end diff --git a/lib/liquid/utils.rb b/lib/liquid/utils.rb index 406d667..709fb00 100644 --- a/lib/liquid/utils.rb +++ b/lib/liquid/utils.rb @@ -52,7 +52,7 @@ module Liquid when Numeric obj when String - obj.strip =~ /\A-?\d+\.\d+\z/ ? BigDecimal(obj) : obj.to_i + /\A-?\d+\.\d+\z/.match?(obj.strip) ? BigDecimal(obj) : obj.to_i else if obj.respond_to?(:to_number) obj.to_number diff --git a/lib/liquid/variable.rb b/lib/liquid/variable.rb index d18c9d0..2ede9e8 100644 --- a/lib/liquid/variable.rb +++ b/lib/liquid/variable.rb @@ -110,7 +110,7 @@ module Liquid filter_args = [] keyword_args = nil unparsed_args.each do |a| - if matches = a.match(JUST_TAG_ATTRIBUTES) + if (matches = a.match(JUST_TAG_ATTRIBUTES)) keyword_args ||= {} keyword_args[matches[1]] = Expression.parse(matches[2]) else diff --git a/performance/profile.rb b/performance/profile.rb index 101f6e5..7074077 100644 --- a/performance/profile.rb +++ b/performance/profile.rb @@ -15,7 +15,7 @@ profiler.run end end - if profile_type == :cpu && graph_filename = ENV['GRAPH_FILENAME'] + if profile_type == :cpu && (graph_filename = ENV['GRAPH_FILENAME']) File.open(graph_filename, 'w') do |f| StackProf::Report.new(results).print_graphviz(nil, f) end diff --git a/performance/shopify/database.rb b/performance/shopify/database.rb index 9836cd4..2db6d30 100644 --- a/performance/shopify/database.rb +++ b/performance/shopify/database.rb @@ -32,8 +32,8 @@ module Database db['article'] = db['blog']['articles'].first db['cart'] = { - 'total_price' => db['line_items'].values.inject(0) { |sum, item| sum += item['line_price'] * item['quantity'] }, - 'item_count' => db['line_items'].values.inject(0) { |sum, item| sum += item['quantity'] }, + 'total_price' => db['line_items'].values.inject(0) { |sum, item| sum + item['line_price'] * item['quantity'] }, + 'item_count' => db['line_items'].values.inject(0) { |sum, item| sum + item['quantity'] }, 'items' => db['line_items'].values, } diff --git a/test/integration/error_handling_test.rb b/test/integration/error_handling_test.rb index 265632c..7abaec0 100644 --- a/test/integration/error_handling_test.rb +++ b/test/integration/error_handling_test.rb @@ -211,7 +211,10 @@ class ErrorHandlingTest < Minitest::Test def test_setting_default_exception_renderer old_exception_renderer = Liquid::Template.default_exception_renderer exceptions = [] - Liquid::Template.default_exception_renderer = ->(e) { exceptions << e; '' } + Liquid::Template.default_exception_renderer = ->(e) { + exceptions << e + '' + } template = Liquid::Template.parse('This is a runtime error: {{ errors.argument_error }}') output = template.render('errors' => ErrorDrop.new) @@ -225,7 +228,10 @@ class ErrorHandlingTest < Minitest::Test def test_exception_renderer_exposing_non_liquid_error template = Liquid::Template.parse('This is a runtime error: {{ errors.runtime_error }}', line_numbers: true) exceptions = [] - handler = ->(e) { exceptions << e; e.cause } + handler = ->(e) { + exceptions << e + e.cause + } output = template.render({ 'errors' => ErrorDrop.new }, exception_renderer: handler) diff --git a/test/integration/template_test.rb b/test/integration/template_test.rb index 75dd95b..48549f5 100644 --- a/test/integration/template_test.rb +++ b/test/integration/template_test.rb @@ -81,7 +81,10 @@ class TemplateTest < Minitest::Test def test_lambda_is_called_once_from_persistent_assigns_over_multiple_parses_and_renders t = Template.new - t.assigns['number'] = -> { @global ||= 0; @global += 1 } + t.assigns['number'] = -> { + @global ||= 0 + @global += 1 + } assert_equal '1', t.parse("{{number}}").render! assert_equal '1', t.parse("{{number}}").render! assert_equal '1', t.render! @@ -90,7 +93,10 @@ class TemplateTest < Minitest::Test def test_lambda_is_called_once_from_custom_assigns_over_multiple_parses_and_renders t = Template.new - assigns = { 'number' => -> { @global ||= 0; @global += 1 } } + assigns = { 'number' => -> { + @global ||= 0 + @global += 1 + } } assert_equal '1', t.parse("{{number}}").render!(assigns) assert_equal '1', t.parse("{{number}}").render!(assigns) assert_equal '1', t.render!(assigns) @@ -237,7 +243,10 @@ class TemplateTest < Minitest::Test def test_exception_renderer_that_returns_string exception = nil - handler = ->(e) { exception = e; '' } + handler = ->(e) { + exception = e + '' + } output = Template.parse("{{ 1 | divided_by: 0 }}").render({}, exception_renderer: handler) @@ -248,7 +257,10 @@ class TemplateTest < Minitest::Test def test_exception_renderer_that_raises exception = nil assert_raises(Liquid::ZeroDivisionError) do - Template.parse("{{ 1 | divided_by: 0 }}").render({}, exception_renderer: ->(e) { exception = e; raise }) + Template.parse("{{ 1 | divided_by: 0 }}").render({}, exception_renderer: ->(e) { + exception = e + raise + }) end assert exception.is_a?(Liquid::ZeroDivisionError) end diff --git a/test/test_helper.rb b/test/test_helper.rb index d7a6641..9606ef8 100755 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -9,7 +9,7 @@ require 'liquid.rb' require 'liquid/profiler' mode = :strict -if env_mode = ENV['LIQUID_PARSER_MODE'] +if (env_mode = ENV['LIQUID_PARSER_MODE']) puts "-- #{env_mode.upcase} ERROR MODE" mode = env_mode.to_sym end diff --git a/test/unit/condition_unit_test.rb b/test/unit/condition_unit_test.rb index 69f6b90..8d4e02f 100644 --- a/test/unit/condition_unit_test.rb +++ b/test/unit/condition_unit_test.rb @@ -26,9 +26,9 @@ class ConditionUnitTest < Minitest::Test assert_evaluates_true 1, '<=', 1 # negative numbers assert_evaluates_true 1, '>', -1 - assert_evaluates_true -1, '<', 1 + assert_evaluates_true(-1, '<', 1) assert_evaluates_true 1.0, '>', -1.0 - assert_evaluates_true -1.0, '<', 1.0 + assert_evaluates_true(-1.0, '<', 1.0) end def test_default_operators_evalute_false diff --git a/test/unit/context_unit_test.rb b/test/unit/context_unit_test.rb index fe790cf..3b460d7 100644 --- a/test/unit/context_unit_test.rb +++ b/test/unit/context_unit_test.rb @@ -85,7 +85,7 @@ class ContextUnitTest < Minitest::Test @context['date'] = Date.today assert_equal Date.today, @context['date'] - now = DateTime.now + now = Time.now @context['datetime'] = now assert_equal now, @context['datetime'] @@ -405,7 +405,11 @@ class ContextUnitTest < Minitest::Test end def test_lambda_is_called_once - @context['callcount'] = proc { @global ||= 0; @global += 1; @global.to_s } + @context['callcount'] = proc { + @global ||= 0 + @global += 1 + @global.to_s + } assert_equal '1', @context['callcount'] assert_equal '1', @context['callcount'] @@ -415,7 +419,11 @@ class ContextUnitTest < Minitest::Test end def test_nested_lambda_is_called_once - @context['callcount'] = { "lambda" => proc { @global ||= 0; @global += 1; @global.to_s } } + @context['callcount'] = { "lambda" => proc { + @global ||= 0 + @global += 1 + @global.to_s + } } assert_equal '1', @context['callcount.lambda'] assert_equal '1', @context['callcount.lambda'] @@ -425,7 +433,11 @@ class ContextUnitTest < Minitest::Test end def test_lambda_in_array_is_called_once - @context['callcount'] = [1, 2, proc { @global ||= 0; @global += 1; @global.to_s }, 4, 5] + @context['callcount'] = [1, 2, proc { + @global ||= 0 + @global += 1 + @global.to_s + }, 4, 5] assert_equal '1', @context['callcount[2]'] assert_equal '1', @context['callcount[2]'] diff --git a/test/unit/static_registers_unit_test.rb b/test/unit/static_registers_unit_test.rb index 0db80cd..125440f 100644 --- a/test/unit/static_registers_unit_test.rb +++ b/test/unit/static_registers_unit_test.rb @@ -82,7 +82,7 @@ class StaticRegistersUnitTest < Minitest::Test static_register["two"] = 4 static_register[true] = "foo" - assert_equal({ nil => true, 1 => :one, :one => "one", "two" => 3, false => nil }, static_register.static_registers) + assert_equal({ nil => true, 1 => :one, :one => "one", "two" => 3, false => nil }, static_register.static) assert_equal({ nil => false, "two" => 4, true => "foo" }, static_register.registers) static_register @@ -109,7 +109,7 @@ class StaticRegistersUnitTest < Minitest::Test assert_nil static_register.delete(:one) assert_equal({}, static_register.registers) - assert_equal({ nil => true, 1 => :one, :one => "one", "two" => 3, false => nil }, static_register.static_registers) + assert_equal({ nil => true, 1 => :one, :one => "one", "two" => 3, false => nil }, static_register.static) end def test_fetch_with_static @@ -135,10 +135,10 @@ class StaticRegistersUnitTest < Minitest::Test assert_equal true, static_register.key?(true) end - def test_static_register_frozen + def test_static_register_can_be_frozen static_register = set_with_static - static = static_register.static_registers + static = static_register.static.freeze assert_raises(RuntimeError) do static["two"] = "foo" @@ -176,9 +176,9 @@ class StaticRegistersUnitTest < Minitest::Test assert_equal({ "one" => 1, "two" => 2, "three" => 3 }, static_register.registers) assert_equal({ "one" => 4, "two" => 5, "three" => 6 }, new_register.registers) assert_equal({ "one" => 7, "two" => 8, "three" => 9 }, newest_register.registers) - assert_equal({ nil => true, 1 => :one, :one => "one", "two" => 3, false => nil }, static_register.static_registers) - assert_equal({ nil => true, 1 => :one, :one => "one", "two" => 3, false => nil }, new_register.static_registers) - assert_equal({ nil => true, 1 => :one, :one => "one", "two" => 3, false => nil }, newest_register.static_registers) + assert_equal({ nil => true, 1 => :one, :one => "one", "two" => 3, false => nil }, static_register.static) + assert_equal({ nil => true, 1 => :one, :one => "one", "two" => 3, false => nil }, new_register.static) + assert_equal({ nil => true, 1 => :one, :one => "one", "two" => 3, false => nil }, newest_register.static) end def test_multiple_instances_are_unique @@ -204,8 +204,45 @@ class StaticRegistersUnitTest < Minitest::Test assert_equal({ "one" => 1, "two" => 2, "three" => 3 }, static_register.registers) assert_equal({ "one" => 4, "two" => 5, "three" => 6 }, new_register.registers) assert_equal({ "one" => 7, "two" => 8, "three" => 9 }, newest_register.registers) - assert_equal({ nil => true, 1 => :one, :one => "one", "two" => 3, false => nil }, static_register.static_registers) - assert_equal({ foo: :bar }, new_register.static_registers) - assert_equal({ bar: :foo }, newest_register.static_registers) + assert_equal({ nil => true, 1 => :one, :one => "one", "two" => 3, false => nil }, static_register.static) + assert_equal({ foo: :bar }, new_register.static) + assert_equal({ bar: :foo }, newest_register.static) + end + + def test_can_update_static_directly_and_updates_all_instances + static_register = StaticRegisters.new(nil => true, 1 => :one, :one => "one", "two" => 3, false => nil) + static_register["one"] = 1 + static_register["two"] = 2 + static_register["three"] = 3 + + new_register = StaticRegisters.new(static_register) + assert_equal({}, new_register.registers) + + assert_equal({ nil => true, 1 => :one, :one => "one", "two" => 3, false => nil }, static_register.static) + + new_register["one"] = 4 + new_register["two"] = 5 + new_register["three"] = 6 + new_register.static["four"] = 10 + + newest_register = StaticRegisters.new(new_register) + assert_equal({}, newest_register.registers) + + assert_equal({ nil => true, 1 => :one, :one => "one", "two" => 3, false => nil, "four" => 10 }, new_register.static) + + newest_register["one"] = 7 + newest_register["two"] = 8 + newest_register["three"] = 9 + new_register.static["four"] = 5 + new_register.static["five"] = 15 + + assert_equal({ "one" => 1, "two" => 2, "three" => 3 }, static_register.registers) + assert_equal({ "one" => 4, "two" => 5, "three" => 6 }, new_register.registers) + assert_equal({ "one" => 7, "two" => 8, "three" => 9 }, newest_register.registers) + + assert_equal({ nil => true, 1 => :one, :one => "one", "two" => 3, false => nil, "four" => 5, "five" => 15 }, newest_register.static) + + assert_equal({ nil => true, 1 => :one, :one => "one", "two" => 3, false => nil, "four" => 5, "five" => 15 }, static_register.static) + assert_equal({ nil => true, 1 => :one, :one => "one", "two" => 3, false => nil, "four" => 5, "five" => 15 }, new_register.static) end end diff --git a/test/unit/tokenizer_unit_test.rb b/test/unit/tokenizer_unit_test.rb index d094aa1..44342d6 100644 --- a/test/unit/tokenizer_unit_test.rb +++ b/test/unit/tokenizer_unit_test.rb @@ -35,7 +35,7 @@ class TokenizerTest < Minitest::Test def tokenize(source) tokenizer = Liquid::Tokenizer.new(source) tokens = [] - while t = tokenizer.shift + while (t = tokenizer.shift) tokens << t end tokens