Merge pull request #83 from pxlpnk/make_rubocop_pass

make rake ci with rspec and rubocop pass

.rubocop.yml

@@ -0,0 +1,14 @@
+inherit_from: .rubocop_todo.yml
+
+LineLength:
+  Max: 120
+
+Documentation:
+  Enabled: false
+
+AllCops:
+  Exclude:
+    - 'Guardfile'
+
+Style/RegexpLiteral:
+  MaxSlashes: 2

.rubocop_todo.yml

@@ -0,0 +1,15 @@
+# This configuration was generated by `rubocop --auto-gen-config`
+# on 2014-09-18 22:28:11 +0200 using RuboCop version 0.26.0.
+# The point is for the user to remove these configuration records
+# one by one as the offenses are removed from the code base.
+# Note that changes in the inspected code, or installation of new
+# versions of RuboCop, may require this file to be generated again.
+
+# Offense count: 1
+# Configuration parameters: CountComments.
+Metrics/MethodLength:
+  Max: 16
+
+# Offense count: 2
+Style/EachWithObject:
+  Enabled: false

.travis.yml

@@ -14,4 +14,4 @@ gemfile:
   - Gemfile
   - gemfiles/Gemfile.actionpack3.2
   - gemfiles/Gemfile.actionpack4.0
-script: bundle exec rspec
+script: bundle exec rake

CHANGELOG.md

@@ -1,5 +1,8 @@
 # Change Log
 
+## Fixed
+* Make rubocop pass
+
 ## Added
 * Add formatter for lines (<https://github.com/zimbatm/lines>) #35
 * Rubocop and rake ci task

Rakefile

@@ -9,4 +9,4 @@ RuboCop::RakeTask.new
 desc 'Run specs, rubocop and reek'
 task ci: %w(spec rubocop)
 
-task default: :spec
+task default: :ci

gemfiles/Gemfile.actionpack3.2

@@ -7,4 +7,5 @@ group :test do
   gem 'actionpack', '~> 3.2.0'
   gem 'logstash-event', '1.1.5'
   gem 'lines'
+  gem 'rubocop'
 end

gemfiles/Gemfile.actionpack4.0

@@ -7,4 +7,5 @@ group :test do
   gem 'actionpack', '~> 4.0.0'
   gem 'logstash-event', '1.1.5'
   gem 'lines'
+  gem 'rubocop'
 end

lib/lograge.rb

@@ -13,7 +13,9 @@
 require 'active_support/ordered_options'
 
 module Lograge
-  mattr_accessor :logger
+  module_function
+
+  mattr_accessor :logger, :application, :ignore_tests
 
   # Custom options that will be appended to log line
   #
@@ -58,21 +60,21 @@ def self.before_format(data, payload)
 
   def self.ignore_actions(actions)
     ignore(lambda do |event|
-      params = event.payload[:params]
-      Array(actions).include?("#{params['controller']}##{params['action']}")
-    end)
+             params = event.payload[:params]
+             Array(actions).include?("#{params['controller']}##{params['action']}")
+           end)
   end
 
-  def self.ignore_tests
-    @@ignore_tests ||= []
+  def ignore_tests
+    @ignore_tests ||= []
   end
 
   def self.ignore(test)
     ignore_tests.push(test) if test
   end
 
-  def self.ignore_nothing
-    @@ignore_tests = []
+  def ignore_nothing
+    @ignore_tests = []
   end
 
   def self.ignore?(event)
@@ -113,26 +115,34 @@ def self.unsubscribe(component, subscriber)
   end
 
   def self.setup(app)
+    self.application = app
     app.config.action_dispatch.rack_cache[:verbose] = false if app.config.action_dispatch.rack_cache
     require 'lograge/rails_ext/rack/logger'
     Lograge.remove_existing_log_subscriptions
     Lograge::RequestLogSubscriber.attach_to :action_controller
-    Lograge.custom_options = app.config.lograge.custom_options
-    Lograge.before_format = app.config.lograge.before_format
-    Lograge.log_level = app.config.lograge.log_level || :info
-    support_deprecated_config(app) # TODO: Remove with version 1.0
-    Lograge.formatter = app.config.lograge.formatter || Lograge::Formatters::KeyValue.new
-    Lograge.ignore_actions(app.config.lograge.ignore_actions)
-    Lograge.ignore(app.config.lograge.ignore_custom)
+    Lograge.custom_options = lograge_config.custom_options
+    Lograge.before_format = lograge_config.before_format
+    Lograge.log_level = lograge_config.log_level || :info
+    support_deprecated_config # TODO: Remove with version 1.0
+    Lograge.formatter = lograge_config.formatter || Lograge::Formatters::KeyValue.new
+    Lograge.ignore_actions(lograge_config.ignore_actions)
+    Lograge.ignore(lograge_config.ignore_custom)
   end
 
   # TODO: Remove with version 1.0
-  def self.support_deprecated_config(app)
-    if legacy_log_format = app.config.lograge.log_format
-      ActiveSupport::Deprecation.warn 'config.lograge.log_format is deprecated. Use config.lograge.formatter instead.', caller
-      legacy_log_format = :key_value if legacy_log_format == :lograge
-      app.config.lograge.formatter = "Lograge::Formatters::#{legacy_log_format.to_s.classify}".constantize.new
-    end
+
+  def support_deprecated_config
+    return unless lograge_config.log_format
+
+    legacy_log_format = lograge_config.log_format
+    warning = 'config.lograge.log_format is deprecated. Use config.lograge.formatter instead.'
+    ActiveSupport::Deprecation.warn(warning, caller)
+    legacy_log_format = :key_value if legacy_log_format == :lograge
+    lograge_config.formatter = "Lograge::Formatters::#{legacy_log_format.to_s.classify}".constantize.new
+  end
+
+  def lograge_config
+    application.config.lograge
   end
 end
 

lib/lograge/formatters/graylog2.rb

@@ -3,16 +3,27 @@ module Formatters
     class Graylog2
       def call(data)
         # Cloning because we don't want to mess with the original when mutating keys.
-        my = data.clone
+        data_clone = data.clone
 
         base = {
-          short_message: "[#{my[:status]}] #{my[:method]} #{my[:path]} (#{my[:controller]}##{my[:action]})"
+          short_message: short_message(data_clone)
         }
 
         # Add underscore to every key to follow GELF additional field syntax.
-        my.keys.each { |k| my["_#{k}".to_sym] = my[k]; my.delete(k) }
+        data_clone.keys.each do |key|
+          data_clone[underscore_prefix(key)] = data_clone[key]
+          data_clone.delete(key)
+        end
 
-        my.merge(base)
+        data_clone.merge(base)
+      end
+
+      def underscore_prefix(key)
+        "_#{key}".to_sym
+      end
+
+      def short_message(data)
+        "[#{data[:status]}] #{data[:method]} #{data[:path]} (#{data[:controller]}##{data[:action]})"
       end
     end
   end

lib/lograge/formatters/key_value.rb

@@ -29,7 +29,7 @@ def format(key, value)
         value = "'#{value}'" if key == :error
 
         # Ensure that we always have exactly two decimals
-        value = '%.2f' % value if value.is_a? Float
+        value = Kernel.format('%.2f', value) if value.is_a? Float
 
         "#{key}=#{value}"
       end

lib/lograge/formatters/l2met.rb

@@ -23,9 +23,7 @@ def fields_to_display(data)
       end
 
       def modify_payload(data)
-        if data[:controller] && data[:action]
-          data[:source] = source_field(data)
-        end
+        data[:source] = source_field(data) if data[:controller] && data[:action]
 
         data
       end

lib/lograge/formatters/lines.rb

@@ -8,9 +8,9 @@ def call(data)
       end
 
       def load_dependencies
-        require "lines"
+        require 'lines'
       rescue LoadError
-        puts "You need to install the lines gem to use this output."
+        puts 'You need to install the lines gem to use this output.'
         raise
       end
     end

lib/lograge/log_subscriber.rb

@@ -84,7 +84,9 @@ def runtimes(event)
     end
 
     def location(_event)
-      if location = Thread.current[:lograge_location]
+      location = Thread.current[:lograge_location]
+
+      if location
         Thread.current[:lograge_location] = nil
         { location: location }
       else

spec/formatters/cee_spec.rb

@@ -2,11 +2,11 @@
 require 'lograge'
 
 describe Lograge::Formatters::Cee do
-  it "prepends the output with @cee" do
+  it 'prepends the output with @cee' do
     expect(subject.call({})).to match(/^@cee/)
   end
 
-  it "serializes custom attributes" do
+  it 'serializes custom attributes' do
     expect(subject.call(custom: 'data')).to match('{"custom":"data"}')
   end
 end

spec/formatters/json_spec.rb

@@ -4,7 +4,7 @@
 describe Lograge::Formatters::Json do
   let(:deserialized_output) { JSON.parse(subject.call(custom: 'data')) }
 
-  it "serializes custom attributes" do
+  it 'serializes custom attributes' do
     expect(deserialized_output).to eq('custom' => 'data')
   end
 end

spec/formatters/lines_spec.rb

@@ -2,7 +2,7 @@
 require 'lograge'
 
 describe Lograge::Formatters::Lines do
-  it "can serialize custom data" do
+  it 'can serialize custom data' do
     expect(subject.call(custom: 'data')).to eq('custom=data')
   end
 end

spec/formatters/raw_spec.rb

@@ -2,7 +2,7 @@
 require 'lograge'
 
 describe Lograge::Formatters::Raw do
-  it "serializes custom attributes" do
+  it 'serializes custom attributes' do
     expect(subject.call(custom: 'data')).to eq(custom: 'data')
   end
 end

spec/lograge_logsubscriber_spec.rb

@@ -12,16 +12,31 @@
   end
 
   let(:subscriber) { Lograge::RequestLogSubscriber.new }
+  let(:event_params) { { 'controller' => 'home', 'action' => 'index', 'foo' => 'bar' } }
+
   let(:event) do
     ActiveSupport::Notifications::Event.new(
-      'process_action.action_controller', Time.now, Time.now, 2, status: 200, format: 'application/json', method: 'GET', path: '/home?foo=bar', params: {
-        'controller' => 'home', 'action' => 'index', 'foo' => 'bar'
-      }, db_runtime: 0.02, view_runtime: 0.01
+      'process_action.action_controller',
+      Time.now,
+      Time.now,
+      2,
+      status: 200,
+      format: 'application/json',
+      method: 'GET',
+      path: '/home?foo=bar',
+      params: event_params,
+      db_runtime: 0.02,
+      view_runtime: 0.01
     )
   end
   let(:redirect) do
     ActiveSupport::Notifications::Event.new(
-      'redirect_to.action_controller', Time.now, Time.now, 1, location: 'http://example.com', status: 302
+      'redirect_to.action_controller',
+      Time.now,
+      Time.now,
+      1,
+      location: 'http://example.com',
+      status: 302
     )
   end
 
@@ -39,13 +54,15 @@
     end
 
     it 'combines the output of a lambda properly' do
-      Lograge.custom_options = lambda { |_event| { data: 'value' } }
+      Lograge.custom_options = ->(_event) { { data: 'value' } }
+
       subscriber.process_action(event)
       expect(log_output.string).to match(/^My test: {.*:data=>"value"/)
     end
 
     it 'works when the method returns nil' do
-      Lograge.custom_options = lambda { |_event| nil }
+      Lograge.custom_options = ->(_event) { nil }
+
       subscriber.process_action(event)
       expect(log_output.string).to be_present
     end
@@ -155,13 +172,14 @@
     end
 
     it 'combines the output of a lambda properly' do
-      Lograge.custom_options = lambda { |_event| { data: 'value' } }
+      Lograge.custom_options = ->(_event) { { data: 'value' } }
+
       subscriber.process_action(event)
       expect(log_output.string).to match(/ data=value/)
     end
-
     it 'works when the method returns nil' do
-      Lograge.custom_options = lambda { |_event| nil }
+      Lograge.custom_options = ->(_event) { nil }
+
       subscriber.process_action(event)
       expect(log_output.string).to be_present
     end
@@ -174,17 +192,16 @@
     end
 
     it 'outputs correctly' do
-      Lograge.before_format = lambda { |data, payload|
-        Hash[*data.first].merge(Hash[*payload.first])
-      }
+      Lograge.before_format = ->(data, payload) { Hash[*data.first].merge(Hash[*payload.first]) }
+
       subscriber.process_action(event)
 
       expect(log_output.string).to include('method=GET')
       expect(log_output.string).to include('status=200')
     end
-
     it 'works if the method returns nil' do
-      Lograge.before_format = lambda { |_data, _payload| nil }
+      Lograge.before_format = ->(_data, _payload) { nil }
+
       subscriber.process_action(event)
       expect(log_output.string).to be_present
     end
@@ -202,7 +219,8 @@
     end
 
     it 'does not log ignored controller actions given a single ignored action after a custom ignore' do
-      Lograge.ignore(lambda { |_event| false })
+      Lograge.ignore(->(_event) { false })
+
       Lograge.ignore_actions 'home#index'
       subscriber.process_action(event)
       expect(log_output.string).to be_blank
@@ -227,17 +245,15 @@
     end
 
     it 'does not log ignored events' do
-      Lograge.ignore(lambda do |event|
-        'GET' == event.payload[:method]
-      end)
+      Lograge.ignore(->(event) { 'GET' == event.payload[:method] })
+
       subscriber.process_action(event)
       expect(log_output.string).to be_blank
     end
 
     it 'logs non-ignored events' do
-      Lograge.ignore(lambda do |event|
-        'foo' == event.payload[:method]
-      end)
+      Lograge.ignore(->(event) { 'foo' == event.payload[:method] })
+
       subscriber.process_action(event)
       expect(log_output.string).not_to be_blank
     end