Merge pull request #1046 from Shopify/make-builds-green

Make builds green

.rubocop.yml

@@ -71,10 +71,7 @@ Style/Documentation:
 Style/ClassAndModuleChildren:
   Enabled: false
 
-Style/TrailingCommaInArrayLiteral:
+Style/TrailingCommaInLiteral:
-  Enabled: false
-
-Style/TrailingCommaInHashLiteral:
   Enabled: false
 
 Layout/IndentHash:
@@ -125,6 +122,6 @@ Style/TrivialAccessors:
 Style/WordArray:
   Enabled: false
 
-Naming/MethodName:
+Style/MethodName:
   Exclude:
     - 'example/server/liquid_servlet.rb'

Rakefile

@@ -3,7 +3,7 @@ require 'rake/testtask'
 $LOAD_PATH.unshift File.expand_path("../lib", __FILE__)
 require "liquid/version"
 
-task default: [:rubocop, :test]
+task default: [:test, :rubocop]
 
 desc 'run test suite with default parser'
 Rake::TestTask.new(:base_test) do |t|

lib/liquid/standardfilters.rb

@@ -120,25 +120,16 @@ module Liquid
     # provide optional property with which to sort an array of hashes or drops
     def sort(input, property = nil)
       ary = InputIterator.new(input)
+
+      return [] if ary.empty?
+
       if property.nil?
         ary.sort do |a, b|
-          if !a.nil? && !b.nil?
+          nil_safe_compare(a, b)
-            a <=> b
-          else
-            a.nil? ? 1 : -1
-          end
         end
-      elsif ary.empty? # The next two cases assume a non-empty array.
-        []
       elsif ary.all? { |el| el.respond_to?(:[]) }
         ary.sort do |a, b|
-          a = a[property]
+          nil_safe_compare(a[property], b[property])
-          b = b[property]
-          if !a.nil? && !b.nil?
-            a <=> b
-          else
-            a.nil? ? 1 : -1
-          end
         end
       end
     end
@@ -148,25 +139,15 @@ module Liquid
     def sort_natural(input, property = nil)
       ary = InputIterator.new(input)
 
+      return [] if ary.empty?
+
       if property.nil?
         ary.sort do |a, b|
-          if !a.nil? && !b.nil?
+          nil_safe_casecmp(a, b)
-            a.to_s.casecmp(b.to_s)
-          else
-            a.nil? ? 1 : -1
-          end
         end
-      elsif ary.empty? # The next two cases assume a non-empty array.
-        []
       elsif ary.all? { |el| el.respond_to?(:[]) }
         ary.sort do |a, b|
-          a = a[property]
+          nil_safe_casecmp(a[property], b[property])
-          b = b[property]
-          if !a.nil? && !b.nil?
-            a.to_s.casecmp(b.to_s)
-          else
-            a.nil? ? 1 : -1
-          end
         end
       end
     end
@@ -418,6 +399,22 @@ module Liquid
       result.is_a?(BigDecimal) ? result.to_f : result
     end
 
+    def nil_safe_compare(a, b)
+      if !a.nil? && !b.nil?
+        a <=> b
+      else
+        a.nil? ? 1 : -1
+      end
+    end
+
+    def nil_safe_casecmp(a, b)
+      if !a.nil? && !b.nil?
+        a.to_s.casecmp(b.to_s)
+      else
+        a.nil? ? 1 : -1
+      end
+    end
+
     class InputIterator
       include Enumerable