Shopify stye guide fixes (Shopify#1160)

.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.
@@ -76,19 +45,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'

lib/liquid/block_body.rb

@@ -27,7 +27,7 @@ def parse(tokenizer, parse_context, &block)
     end
 
     private def parse_for_liquid_tag(tokenizer, parse_context)
-      while token = tokenizer.shift
+      while (token = tokenizer.shift)
         unless token.empty? || token =~ WhitespaceOrNothing
           unless token =~ LiquidTagToken
             # line isn't empty but didn't match tag syntax, yield and let the
@@ -36,7 +36,7 @@ def parse(tokenizer, parse_context, &block)
           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 @@ def parse(tokenizer, parse_context, &block)
     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?(TAGSTART)
@@ -74,7 +74,7 @@ def parse(tokenizer, parse_context, &block)
             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 @@ def render_to_output_buffer(context, output)
       context.resource_limits.render_score += @nodelist.length
 
       idx = 0
-      while node = @nodelist[idx]
+      while (node = @nodelist[idx])
         previous_output_size = output.bytesize
 
         case node

lib/liquid/lexer.rb

@@ -33,15 +33,21 @@ def tokenize
       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}"

lib/liquid/parse_context.rb

@@ -21,7 +21,6 @@ def partial=(value)
       @partial = value
       @options = value ? partial_options : @template_options
       @error_mode = @options[:error_mode] || Template.error_mode
-      value
     end
 
     def partial_options

lib/liquid/standardfilters.rb

@@ -327,7 +327,7 @@ def newline_to_br(input)
     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

lib/liquid/tags/for.rb

@@ -111,7 +111,7 @@ def strict_parse(markup)
       @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

lib/liquid/tags/if.rb

@@ -94,7 +94,7 @@ def strict_parse(markup)
     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 @@ def parse_binary_comparisons(p)
 
     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

lib/liquid/tags/raw.rb

@@ -13,7 +13,7 @@ def initialize(tag_name, markup, parse_context)
 
     def parse(tokens)
       @body = +''
-      while token = tokens.shift
+      while (token = tokens.shift)
         if token =~ FullTokenPossiblyInvalid
           @body << Regexp.last_match(1) if Regexp.last_match(1) != ""
           return if block_delimiter == Regexp.last_match(2)

lib/liquid/variable.rb

@@ -110,7 +110,7 @@ def parse_filter_expressions(filter_name, unparsed_args)
       filter_args = []
       keyword_args = nil
       unparsed_args.each do |a|
-        if matches = a.match(JustTagAttributes)
+        if (matches = a.match(JustTagAttributes))
           keyword_args ||= {}
           keyword_args[matches[1]] = Expression.parse(matches[2])
         else

performance/profile.rb

@@ -15,7 +15,7 @@
     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

performance/shopify/database.rb

@@ -32,8 +32,8 @@ def self.tables
       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,
       }
 

test/integration/error_handling_test.rb

@@ -211,7 +211,10 @@ def test_default_exception_renderer_with_internal_error
   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 @@ def test_setting_default_exception_renderer
   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)
 

test/integration/template_test.rb

@@ -81,7 +81,10 @@ def test_persistent_assigns_squash_instance_assigns
 
   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 @@ def test_lambda_is_called_once_from_persistent_assigns_over_multiple_parses_and_
 
   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 @@ def test_render_bang_force_rethrow_errors_on_passed_context
 
   def test_exception_renderer_that_returns_string
     exception = nil
-    handler = ->(e) { exception = e; '<!-- error -->' }
+    handler = ->(e) {
+      exception = e
+      '<!-- error -->'
+    }
 
     output = Template.parse("{{ 1 | divided_by: 0 }}").render({}, exception_renderer: handler)
 
@@ -248,7 +257,10 @@ def test_exception_renderer_that_returns_string
   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

test/test_helper.rb

@@ -9,7 +9,7 @@
 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

test/unit/condition_unit_test.rb

@@ -26,9 +26,9 @@ def test_default_operators_evalute_true
     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

test/unit/context_unit_test.rb

@@ -85,7 +85,7 @@ def test_variables
     @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 @@ def test_array_containing_lambda_as_variable
   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 @@ def test_lambda_is_called_once
   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 @@ def test_nested_lambda_is_called_once
   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]']

test/unit/tokenizer_unit_test.rb

@@ -35,7 +35,7 @@ def test_calculate_line_numbers_per_token_with_profiling
   def tokenize(source)
     tokenizer = Liquid::Tokenizer.new(source)
     tokens = []
-    while t = tokenizer.shift
+    while (t = tokenizer.shift)
       tokens << t
     end
     tokens