Add Rubocop config and resolve most style issues

.rubocop.yml

@@ -0,0 +1,94 @@
+AllCops:
+  TargetRubyVersion: 2.1
+
+# Sometimes we want to capture all exceptions. We need to make sure to re-raise
+# these again in any case.
+Lint/RescueException:
+  Enabled: false
+
+# As long as methods and classes are readable, this should be fine.
+# In any case, this shouldn't be a hard error. We try to improve where sensible
+# using tools like RubyCritic instead
+Metrics/AbcSize:
+  Enabled: false
+Metrics/CyclomaticComplexity:
+  Enabled: false
+Metrics/PerceivedComplexity:
+  Enabled: false
+
+# Long classes and methods are not a problem per se, as long as it is kept kind
+# of sane. In any case, this shouldn't be a hard error..
+Metrics/ClassLength:
+  Enabled: false
+Metrics/MethodLength:
+  Enabled: false
+
+# As long as methods are readable, this should be fine.
+# In any case, this shouldn't be a hard error. We try to improve where sensible
+# using tools like RubyCritic instead
+
+# rspec uses long blocks by definition
+Metrics/BlockLength:
+  Exclude:
+    - 'spec/**/*'
+
+# Try to keep lines below 80 chars wherever possible though
+Metrics/LineLength:
+  AllowURI: true
+  Max: 100
+
+Style/BlockDelimiters:
+  EnforcedStyle: semantic
+  Exclude:
+    - 'spec/**/*'
+
+Style/Copyright:
+  Enabled: true
+  Notice: '^# Copyright (\(c\) )?2[0-9]{3} .+'
+
+# Use === when it makes sense to do so
+Style/CaseEquality:
+  Enabled: false
+
+# Double negation is fine to reliably produce a Boolean from any value.
+Style/DoubleNegation:
+  Enabled: false
+
+# Multiline indentation is always tricky. Format it as deemed useful in the
+# local method, but try to avoid it at all wherever possible
+Style/MultilineOperationIndentation:
+  Enabled: false
+
+# We generally use `obj == nil` instead of `obj.nil?` because the former is
+# faster.
+Style/NilComparison:
+  Enabled: false
+
+Style/PercentLiteralDelimiters:
+  PreferredDelimiters:
+    '%w': '[]'
+    '%i': '[]'
+
+# Sometimes, an explicit self really adds some clarity to the code
+# We make sure to not use it when not needed though
+Style/RedundantSelf:
+  Enabled: false
+
+# Doesn't work with Refinements and is sometimes clearer with the expanded
+# version
+Style/SymbolProc:
+  Enabled: false
+
+Style/RegexpLiteral:
+  Exclude:
+    - 'spec/**/*'
+
+# We know what we are doing and only use the rescue modifier when we really
+# don't care
+Style/RescueModifier:
+  Enabled: false
+
+# Use %w when it makes sense. Use literal arrays where it is more clear.
+Style/WordArray:
+  Enabled: false
+

Rakefile

@@ -12,4 +12,4 @@ require 'yard'
 YARD::Rake::YardocTask.new
 task doc: :yard
 
-task :default => :spec
+task default: :spec

lib/rackstash/buffer.rb

@@ -144,10 +144,10 @@ module Rackstash
     # @return [self,nil] returns `self` if the buffer was flushed, `nil`
     #   otherwise
     def flush
-      if pending?
-        @sink.flush(self)
-        self
-      end
+      return unless pending?
+
+      @sink.flush(self)
+      self
     end
 
     # Return all logged messages on the current buffer.

lib/rackstash/fields/abstract_collection.rb

@@ -12,15 +12,15 @@ require 'concurrent'
 module Rackstash
   module Fields
     class AbstractCollection
-      # Equality — Two collections are equal if they are of exactly the same class
-      # and contain the same raw data according to `Object#==`.
+      # Equality -- Two collections are equal if they are of exactly the same
+      # class and contain the same raw data according to `Object#==`.
       #
       # @return [Boolean] `true` if `other` is an object of the same class and
       #   contains the same raw data as this object.
       def ==(other)
         self.class == other.class && raw == other.raw
       end
-      alias :eql? :==
+      alias eql? ==
 
       # Show a human-readable representation of `self`. To get a machine-
       # readable "export" of the contained data, use {#as_json} or one of its
@@ -28,7 +28,7 @@ module Rackstash
       #
       # @return [String] human-redable details about the object.
       def inspect
-        "#<#{self.class}:#{format '0x%014x', object_id << 1} #{to_s}>"
+        "#<#{self.class}:#{format '0x%014x', object_id << 1} #{self}>"
       end
 
       # Provide a copy of the wrapped {#raw} data in a format allowing direct
@@ -106,17 +106,21 @@ module Rackstash
           hash = value.each_with_object(Concurrent::Hash.new) do |(k, v), memo|
             memo[utf8_encode(k)] = normalize(v, scope: scope)
           end
-          hash = Rackstash::Fields::Hash.new.tap do |hash_field|
-            hash_field.raw = hash
-          end if wrap
+          if wrap
+            hash = Rackstash::Fields::Hash.new.tap do |hash_field|
+              hash_field.raw = hash
+            end
+          end
           return hash
         when ::Array, ::Set, ::Enumerator
           array = value.each_with_object(Concurrent::Array.new) do |e, memo|
             memo << normalize(e, scope: scope)
           end
-          array = Rackstash::Fields::Array.new.tap do |array_field|
-            array_field.raw = array
-          end if wrap
+          if wrap
+            array = Rackstash::Fields::Array.new.tap do |array_field|
+              array_field.raw = array
+            end
+          end
           return array
         when ::Time
           return value.utc.iso8601(ISO8601_PRECISION).freeze

lib/rackstash/fields/array.rb

@@ -13,7 +13,7 @@ module Rackstash
       end
 
       # @!method +(array)
-      #   Concatenation — Returns a new {Rackstash::Fields::Array} built by
+      #   Concatenation -- Returns a new {Rackstash::Fields::Array} built by
       #   concatenating `self` and  the given `array` together to produce a
       #   third array.
       #
@@ -21,7 +21,7 @@ module Rackstash
       #   @return [Rackstash::Fields::Array]
 
       # @!method -(array)
-      #   Array Difference — Returns a new {Rackstash::Fields::Array} that is a
+      #   Array Difference -- Returns a new {Rackstash::Fields::Array} that is a
       #   copy of `self`, removing any items that also appear in the given
       #   `array`. The order is preserved from `self`.
       #
@@ -29,7 +29,7 @@ module Rackstash
       #   @return [Rackstash::Fields::Array]
 
       # @!method |(array)
-      #   Set Union — Returns a new {Rackstash::Fields::Array} by joining `self`
+      #   Set Union -- Returns a new {Rackstash::Fields::Array} by joining `self`
       #   with the given `array`, excluding any duplicates and preserving the
       #   order from `self`.
       #
@@ -37,7 +37,7 @@ module Rackstash
       #   @return [Rackstash::Fields::Array]
 
       # @!method &(array)
-      #   Set Intersection — Returns a new {Rackstash::Fields::Array} containing
+      #   Set Intersection -- Returns a new {Rackstash::Fields::Array} containing
       #   elements common to `self` and the given `array`, excluding any
       #   duplicates. The order is preserved from `self`.
       #
@@ -45,7 +45,7 @@ module Rackstash
       #   @return [Rackstash::Fields::Array]
 
       %i[+ - | &].each do |op|
-        class_eval <<-RUBY, __FILE__ , __LINE__ + 1
+        class_eval <<-RUBY, __FILE__, __LINE__ + 1
           def #{op}(array)
             new(@raw #{op} normalize(array, wrap: false))
           end
@@ -92,8 +92,8 @@ module Rackstash
           value.is_a?(AbstractCollection) ? value.as_json : value
         }
       end
-      alias :to_ary :as_json
-      alias :to_a :as_json
+      alias to_ary as_json
+      alias to_a as_json
 
       # Removes all elements from `self`.
       #
@@ -127,7 +127,7 @@ module Rackstash
         @raw.length
       end
 
-      # Set Union — Add value from `array` to `self` excluding any duplicates
+      # Set Union -- Add value from `array` to `self` excluding any duplicates
       # and preserving the order from `self`.
       #
       # @param array [Array, ::Array, Proc] an array of values. Each value is
@@ -137,12 +137,13 @@ module Rackstash
       #   object (when given).
       # @return [self]
       #
-      # @see #merge
+      # @see #|
+      # @see #merge!
       def merge(array, scope: nil)
         new(@raw | normalize(array, wrap: false, scope: scope))
       end
 
-      # Set Union — Add value from `array` to `self` excluding any duplicates
+      # Set Union -- Add value from `array` to `self` excluding any duplicates
       # and preserving the order from `self`.
       #
       # @param array [Array, ::Array, Proc] an array of values. Each value is
@@ -170,7 +171,6 @@ module Rackstash
           array.raw = raw
         end
       end
-
     end
 
     def self.Array(array)

lib/rackstash/fields/hash.rb

@@ -50,7 +50,7 @@ module Rackstash
 
         @raw[key] = normalize(value)
       end
-      alias :store :[]=
+      alias store []=
 
       # @return [::Hash] deep-transforms the hash into a plain Ruby Hash
       def as_json(*)
@@ -59,8 +59,8 @@ module Rackstash
           memo[key] = value
         end
       end
-      alias :to_hash :as_json
-      alias :to_h :as_json
+      alias to_hash as_json
+      alias to_h as_json
 
       # Removes all key-value pairs from `self`.
       #
@@ -143,6 +143,7 @@ module Rackstash
       # @param scope [Object, nil] if `hash` or any of its (deeply-nested)
       #   values is a proc, it will be called in the instance scope of this
       #   object (when given).
+      #
       # @yield [key, old_val, new-val] if a block is given and there is a
       #   duplicate key, we call the block and use its return value as the value
       #   to insert
@@ -179,7 +180,7 @@ module Rackstash
         end
         self
       end
-      alias :update :merge!
+      alias update merge!
 
       # @param key [String] The name of a key to check. This MUST be a correctly
       #   encoded String in order to return valid results

lib/rackstash/fields/tags.rb

@@ -24,8 +24,8 @@ module Rackstash
       def as_json(*)
         @raw.to_a
       end
-      alias :to_ary :as_json
-      alias :to_a :as_json
+      alias to_ary as_json
+      alias to_a as_json
 
       def clear
         @raw.clear
@@ -83,4 +83,3 @@ module Rackstash
     end
   end
 end
-

lib/rackstash/logger.rb

@@ -65,12 +65,11 @@ module Rackstash
     def <<(msg)
       buffer.add_message Message.new(
         msg,
-        time: Time.now.utc.freeze,
+        time: Time.now.utc.freeze
       )
       msg
     end
 
-
     # Set the base log level as either one of the {SEVERITIES} or a
     # String/Symbol describing the level. When logging a message, it will only
     # be added if its log level is at or above the base level defined here
@@ -262,7 +261,7 @@ module Rackstash
 
       msg
     end
-    alias :log :add
+    alias log add
 
     # Create a new buffering {Buffer} and puts in on the {BufferStack} for the
     # current Thread. For the duration of the block, all new logged messages

lib/rackstash/message.rb

@@ -52,9 +52,9 @@ module Rackstash
       cleanup @formatter.call(severity_label, @time, @progname, @message)
     end
 
-    alias :as_json :to_s
+    alias as_json to_s
     # Messages are implicitly conversible to Strings
-    alias :to_str :to_s
+    alias to_str to_s
 
     def to_json
       as_json.to_json

spec/rackstash/buffer_spec.rb

@@ -21,7 +21,7 @@ describe Rackstash::Buffer do
   describe '#add_message' do
     it 'adds a message to the buffer' do
       msg = double(message: 'Hello World', time: Time.now)
-      expect(buffer.add_message msg).to equal msg
+      expect(buffer.add_message(msg)).to equal msg
 
       expect(buffer.messages).to eql [msg]
     end

spec/rackstash/fields/abstract_collection_spec.rb

@@ -191,7 +191,7 @@ describe Rackstash::Fields::AbstractCollection do
         hash = { 1 => 1, :two => 2, 'three' => 3, nil => 4 }
 
         expect(normalize(hash, wrap: false)).to eql(
-          { '1' => 1, 'two' => 2, 'three' => 3, '' => 4 }
+          '1' => 1, 'two' => 2, 'three' => 3, '' => 4
         )
         expect(normalize(hash, wrap: false).keys).to all be_frozen
       end
@@ -316,18 +316,17 @@ describe Rackstash::Fields::AbstractCollection do
       end
 
       it 'resolves nested procs' do
-        expect(normalize(-> { [-> { 'foo' } ] })).to be_instance_of Rackstash::Fields::Array
-        expect(normalize(-> { [-> { 'foo' } ] })).to contain_exactly 'foo'
+        expect(normalize(-> { [-> { 'foo' }] })).to be_instance_of Rackstash::Fields::Array
+        expect(normalize(-> { [-> { 'foo' }] })).to contain_exactly 'foo'
       end
 
-
       it 'returns a raw array returned from a proc with wrap: false' do
-        expect(normalize(-> { ['foo'] }, wrap: false )).to be_a ::Array
+        expect(normalize(-> { ['foo'] }, wrap: false)).to be_a ::Array
         expect(normalize(-> { ['foo'] }, wrap: false)).to eql ['foo']
       end
 
       it 'returns a raw array returned from a nested proc with wrap: false' do
-        expect(normalize(-> { [-> { 'foo' }] }, wrap: false )).to be_a ::Array
+        expect(normalize(-> { [-> { 'foo' }] }, wrap: false)).to be_a ::Array
         expect(normalize(-> { [-> { 'foo' }] }, wrap: false)).to eql ['foo']
       end
     end
@@ -458,7 +457,7 @@ describe Rackstash::Fields::AbstractCollection do
         inner = proc { :return }
         outer = proc { inner }
 
-        expect(normalize(outer)).to match %r{\A#<Proc:0x[0-9a-f]+@#{__FILE__}:#{__LINE__-3}>\z}
+        expect(normalize(outer)).to match %r{\A#<Proc:0x[0-9a-f]+@#{__FILE__}:#{__LINE__ - 3}>\z}
       end
     end
 
@@ -487,7 +486,7 @@ describe Rackstash::Fields::AbstractCollection do
           obj = double("#{method} - successful")
 
           methods[0..i].each_with_index do |check, j|
-            expect(obj).to receive(:respond_to?).with(check).and_return(i==j)
+            expect(obj).to receive(:respond_to?).with(check).and_return(i == j)
               .ordered.once
           end
 

spec/rackstash/fields/array_spec.rb

@@ -194,12 +194,12 @@ describe Rackstash::Fields::Array do
     end
 
     it 'resolves nested procs' do
-      expect(array.concat(-> { [-> { :foo } ] } )).to contain_exactly 'foo'
+      expect(array.concat(-> { [-> { :foo }] })).to contain_exactly 'foo'
     end
 
     it 'resolves nested procs with a custom scope' do
       expect(
-        array.concat(-> { [self, -> { self.to_s.upcase } ] }, scope: :stuff)
+        array.concat(-> { [self, -> { self.to_s.upcase }] }, scope: :stuff)
       ).to contain_exactly 'stuff', 'STUFF'
     end
   end
@@ -229,19 +229,19 @@ describe Rackstash::Fields::Array do
   describe '#merge' do
     it 'returns the union of elements' do
       array[0] = 'existing'
-      expect(array.merge ['new', :existing, -> { 123 }])
+      expect(array.merge(['new', :existing, -> { 123 }]))
         .to contain_exactly('existing', 'new', 123)
         .and be_a(Rackstash::Fields::Array)
     end
 
     it 'returns a new Array' do
-      expect(array.merge [:foo]).to be_a(Rackstash::Fields::Array)
-      expect(array.merge [:foo]).not_to equal array
+      expect(array.merge([:foo])).to be_a(Rackstash::Fields::Array)
+      expect(array.merge([:foo])).not_to equal array
     end
 
     it 'resolves nested procs with a custom scope' do
       expect(
-        array.merge(-> { [self, -> { self.to_s.upcase } ] }, scope: :stuff)
+        array.merge(-> { [self, -> { self.to_s.upcase }] }, scope: :stuff)
       ).to contain_exactly 'stuff', 'STUFF'
     end
   end
@@ -249,17 +249,17 @@ describe Rackstash::Fields::Array do
   describe '#merge!' do
     it 'sets the union of elements to self' do
       array[0] = 'existing'
-      expect(array.merge! ['new', :existing, -> { 123 }])
+      expect(array.merge!(['new', :existing, -> { 123 }]))
         .to contain_exactly 'existing', 'new', 123
     end
 
     it 'returns self' do
-      expect(array.merge! [:foo]).to equal array
+      expect(array.merge!([:foo])).to equal array
     end
 
     it 'resolves nested procs with a custom scope' do
       expect(
-        array.merge!(-> { [self, -> { self.to_s.upcase } ] }, scope: :stuff)
+        array.merge!(-> { [self, -> { self.to_s.upcase }] }, scope: :stuff)
       ).to contain_exactly 'stuff', 'STUFF'
     end
   end

spec/rackstash/fields/hash_spec.rb

@@ -177,7 +177,6 @@ describe Rackstash::Fields::Hash do
       expect(hash.forbidden_key?('forbidden')).to be true
       expect(hash.forbidden_key?('foo')).to be false
     end
-
   end
 
   describe '#keys' do
@@ -215,7 +214,7 @@ describe Rackstash::Fields::Hash do
     end
 
     it 'merges a normalized hash' do
-      to_merge = {foo: :bar}
+      to_merge = { foo: :bar }
       expect(hash).to receive(:normalize).with(to_merge, anything).ordered.and_call_original
       expect(hash).to receive(:normalize).with(:bar, anything).ordered.and_call_original
 
@@ -377,7 +376,8 @@ describe Rackstash::Fields::Hash do
 
     it 'can specify forbidden_keys' do
       raw = { foo: :bar, forbidden: 'ignored' }
-      expect { Rackstash::Fields::Hash(raw, forbidden_fields: ['forbidden']) }.to raise_error ArgumentError
+      expect { Rackstash::Fields::Hash(raw, forbidden_fields: ['forbidden']) }
+        .to raise_error ArgumentError
     end
   end
 end

spec/rackstash/formatter_spec.rb

@@ -19,8 +19,8 @@ describe Rackstash::Formatter do
     expect(formatter.call('ERROR', Time.now, 'progname', 123)).to eql "123\n"
   end
 
-  it 'formats Hashes' do
-    expect(formatter.call('ERROR', Time.now, 'progname', { k: 'v' })).to eql "{:k=>\"v\"}\n"
+  it 'formats Arrays' do
+    expect(formatter.call('ERROR', Time.now, 'progname', [1, 'y'])).to eql "[1, \"y\"]\n"
   end
 
   it 'formats exceptions' do
@@ -61,5 +61,4 @@ describe Rackstash::RawFormatter do
     expect(obj).to receive(:inspect).and_return('object')
     expect(formatter.call('ERROR', Time.now, 'progname', obj)).to eql 'object'
   end
-
 end

spec/rackstash/logger_spec.rb

@@ -378,7 +378,6 @@ describe Rackstash::Logger do
       expect(logger.fields['key']).to eql 'outer'
     end
 
-
     it 'buffers multiple messages' do
       expect(logger.sink).to receive(:flush).once
 

spec/rackstash_spec.rb

@@ -12,7 +12,7 @@ describe Rackstash do
   end
 
   it 'defines SEVERITIES constants' do
-    expect(Rackstash::SEVERITIES).to eql (0..5).to_a
+    expect(Rackstash::SEVERITIES).to eql((0..5).to_a)
 
     expect(Rackstash::DEBUG).to eql 0
     expect(Rackstash::INFO).to eql 1