Merge pull request #338 from grzuy/rubocop

Enable more codestyle checks

.rubocop.yml

@@ -1,5 +1,28 @@
 AllCops:
+  TargetRubyVersion: 2.2
   DisabledByDefault: true
+  Exclude:
+    - "examples/instrumentation.rb"
+
+Bundler:
+  Enabled: true
+
+Gemspec:
+  Enabled: true
 
 Layout:
   Enabled: true
+
+Performance:
+  Enabled: true
+
+Security:
+  Enabled: true
+
+Lint:
+  Enabled: true
+
+# TODO
+# Remove cop disabling and fix offenses
+Lint/HandleExceptions:
+  Enabled: false

Appraisals

@@ -7,18 +7,18 @@ appraise "rack_1_6" do
 end
 
 appraise 'rails_5-2' do
-  gem 'activesupport', '~> 5.2.0'
   gem 'actionpack', '~> 5.2.0'
+  gem 'activesupport', '~> 5.2.0'
 end
 
 appraise 'rails_5-1' do
-  gem 'activesupport', '~> 5.1.0'
   gem 'actionpack', '~> 5.1.0'
+  gem 'activesupport', '~> 5.1.0'
 end
 
 appraise 'rails_4-2' do
-  gem 'activesupport', '~> 4.2.0'
   gem 'actionpack', '~> 4.2.0'
+  gem 'activesupport', '~> 4.2.0'
 end
 
 appraise 'dalli2' do

CONTRIBUTING.md

@@ -20,7 +20,7 @@ Any of the following is greatly appreciated:
 As an effort to keep the codebase consistent, we encourage the use of [Rubocop](https://github.com/bbatsov/rubocop).
 This tool helps us abstract most of the decisions we have to make when coding.
 
-To check your code, simply type `rubocop` in the shell. The resulting output are all the offenses currently present in the code.
+To check your code, simply type `bundle exec rubocop` in the shell. The resulting output are all the offenses currently present in the code.
 
 It is highly recommended that you integrate a linter with your editor.
 This way you receive real time feedback about your code. Most editors have some kind of plugin for that.

gemfiles/rails_4_2.gemfile

@@ -2,7 +2,7 @@
 
 source "https://rubygems.org"
 
-gem "activesupport", "~> 4.2.0"
 gem "actionpack", "~> 4.2.0"
+gem "activesupport", "~> 4.2.0"
 
 gemspec path: "../"

gemfiles/rails_5_1.gemfile

@@ -2,7 +2,7 @@
 
 source "https://rubygems.org"
 
-gem "activesupport", "~> 5.1.0"
 gem "actionpack", "~> 5.1.0"
+gem "activesupport", "~> 5.1.0"
 
 gemspec path: "../"

gemfiles/rails_5_2.gemfile

@@ -2,7 +2,7 @@
 
 source "https://rubygems.org"
 
-gem "activesupport", "~> 5.2.0"
 gem "actionpack", "~> 5.2.0"
+gem "activesupport", "~> 5.2.0"
 
 gemspec path: "../"

lib/rack/attack.rb

@@ -101,7 +101,7 @@ class Rack::Attack
     end
 
     def throttled?(req)
-      throttles.any? do |name, throttle|
+      throttles.any? do |_name, throttle|
         throttle[req]
       end
     end
@@ -149,7 +149,7 @@ class Rack::Attack
 
   # Set defaults
   @notifier             = ActiveSupport::Notifications if defined?(ActiveSupport::Notifications)
-  @blocklisted_response = lambda { |env| [403, { 'Content-Type' => 'text/plain' }, ["Forbidden\n"]] }
+  @blocklisted_response = lambda { |_env| [403, { 'Content-Type' => 'text/plain' }, ["Forbidden\n"]] }
   @throttled_response   = lambda { |env|
     retry_after = (env['rack.attack.match_data'] || {})[:period]
     [429, { 'Content-Type' => 'text/plain', 'Retry-After' => retry_after.to_s }, ["Retry later\n"]]

lib/rack/attack/store_proxy.rb

@@ -12,8 +12,6 @@ module Rack
         klass ? klass.new(client) : client
       end
 
-      private
-
       def self.unwrap_active_support_stores(store)
         # ActiveSupport::Cache::RedisStore doesn't expose any way to set an expiry,
         # so use the raw Redis::Store instead.

lib/rack/attack/store_proxy/mem_cache_proxy.rb

@@ -23,12 +23,12 @@ module Rack
         rescue MemCache::MemCacheError
         end
 
-        def increment(key, amount, options = {})
+        def increment(key, amount, _options = {})
           incr(key, amount)
         rescue MemCache::MemCacheError
         end
 
-        def delete(key, options = {})
+        def delete(key, _options = {})
           with do |client|
             client.delete(key)
           end

lib/rack/attack/store_proxy/redis_store_proxy.rb

@@ -45,7 +45,7 @@ module Rack
         rescue Redis::BaseError
         end
 
-        def delete(key, options = {})
+        def delete(key, _options = {})
           del(key)
         rescue Redis::BaseError
         end

rack-attack.gemspec

@@ -21,24 +21,26 @@ Gem::Specification.new do |s|
   s.summary = %q{Block & throttle abusive requests}
   s.test_files = Dir.glob("spec/**/*")
 
-  s.required_ruby_version = '>= 2.0.0'
+  s.required_ruby_version = '>= 2.2'
 
   s.add_dependency 'rack'
 
+  s.add_development_dependency 'actionpack', '>= 3.0.0'
+  s.add_development_dependency 'activesupport', '>= 3.0.0'
+  s.add_development_dependency 'appraisal'
+  s.add_development_dependency 'connection_pool'
+  s.add_development_dependency 'dalli'
+  s.add_development_dependency 'guard-minitest'
+  s.add_development_dependency 'memcache-client'
   s.add_development_dependency 'minitest'
   s.add_development_dependency "minitest-stub-const"
+  s.add_development_dependency 'pry'
   s.add_development_dependency 'rack-test'
   s.add_development_dependency 'rake'
-  s.add_development_dependency 'appraisal'
-  s.add_development_dependency 'activesupport', '>= 3.0.0'
-  s.add_development_dependency 'actionpack', '>= 3.0.0'
   s.add_development_dependency 'redis-activesupport'
-  s.add_development_dependency 'dalli'
-  s.add_development_dependency 'connection_pool'
-  s.add_development_dependency 'memcache-client'
+  s.add_development_dependency "rubocop", "0.55.0"
   s.add_development_dependency "timecop"
-  s.add_development_dependency 'pry'
-  s.add_development_dependency 'guard-minitest'
+
   # Need to explicitly depend on guard because guard-minitest doesn't declare
   # the dependency intentionally
   #

spec/acceptance/cache_store_config_for_allow2ban_spec.rb

@@ -81,11 +81,11 @@ describe "Cache store config when using allow2ban" do
         @backend[key]
       end
 
-      def write(key, value, options = {})
+      def write(key, value, _options = {})
         @backend[key] = value
       end
 
-      def increment(key, count, options = {})
+      def increment(key, _count, _options = {})
         @backend[key] ||= 0
         @backend[key] += 1
       end

spec/acceptance/cache_store_config_for_fail2ban_spec.rb

@@ -81,11 +81,11 @@ describe "Cache store config when using fail2ban" do
         @backend[key]
       end
 
-      def write(key, value, options = {})
+      def write(key, value, _options = {})
         @backend[key] = value
       end
 
-      def increment(key, count, options = {})
+      def increment(key, _count, _options = {})
         @backend[key] ||= 0
         @backend[key] += 1
       end

spec/acceptance/cache_store_config_for_throttle_spec.rb

@@ -29,7 +29,7 @@ describe "Cache store config when throttling without Rails" do
         @counts = {}
       end
 
-      def increment(key, count, options)
+      def increment(key, _count, _options)
         @counts[key] ||= 0
         @counts[key] += 1
       end

spec/acceptance/customizing_blocked_response_spec.rb

@@ -12,7 +12,7 @@ describe "Customizing block responses" do
 
     assert_equal 403, last_response.status
 
-    Rack::Attack.blocklisted_response = lambda do |env|
+    Rack::Attack.blocklisted_response = lambda do |_env|
       [503, {}, ["Blocked"]]
     end
 

spec/acceptance/customizing_throttled_response_spec.rb

@@ -18,7 +18,7 @@ describe "Customizing throttled response" do
 
     assert_equal 429, last_response.status
 
-    Rack::Attack.throttled_response = lambda do |env|
+    Rack::Attack.throttled_response = lambda do |_env|
       [503, {}, ["Throttled"]]
     end
 

spec/rack_attack_throttle_spec.rb

@@ -51,7 +51,7 @@ describe 'Rack::Attack.throttle with limit as proc' do
   before do
     @period = 60 # Use a long period; failures due to cache key rotation less likely
     Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new
-    Rack::Attack.throttle('ip/sec', :limit => lambda { |req| 1 }, :period => @period) { |req| req.ip }
+    Rack::Attack.throttle('ip/sec', :limit => lambda { |_req| 1 }, :period => @period) { |req| req.ip }
   end
 
   it_allows_ok_requests
@@ -75,7 +75,7 @@ describe 'Rack::Attack.throttle with period as proc' do
   before do
     @period = 60 # Use a long period; failures due to cache key rotation less likely
     Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new
-    Rack::Attack.throttle('ip/sec', :limit => lambda { |req| 1 }, :period => lambda { |req| @period }) { |req| req.ip }
+    Rack::Attack.throttle('ip/sec', :limit => lambda { |_req| 1 }, :period => lambda { |_req| @period }) { |req| req.ip }
   end
 
   it_allows_ok_requests

spec/rack_attack_track_spec.rb

@@ -16,7 +16,7 @@ describe 'Rack::Attack.track' do
   end
 
   before do
-    Rack::Attack.track("everything") { |req| true }
+    Rack::Attack.track("everything") { |_req| true }
   end
 
   it_allows_ok_requests
@@ -33,7 +33,7 @@ describe 'Rack::Attack.track' do
       # A second track
       Rack::Attack.track("homepage") { |req| req.path == "/" }
 
-      ActiveSupport::Notifications.subscribe("rack.attack") do |*args|
+      ActiveSupport::Notifications.subscribe("rack.attack") do |*_args|
         Counter.incr
       end
 

spec/spec_helper.rb

@@ -42,7 +42,7 @@ class MiniTest::Spec
       use Rack::Attack
       use Rack::Lint
 
-      run lambda { |env| [200, {}, ['Hello World']] }
+      run lambda { |_env| [200, {}, ['Hello World']] }
     }.to_app
   end