[Fixes #302] Initial style guide adoption (#330)

* Initial Rubocop configuration

* Fix Rubocop layout offenses for lib

* Fix some spec offenses

* Fix leftover layout offenses

.rubocop.yml

@@ -0,0 +1,5 @@
+AllCops:
+  DisabledByDefault: true
+
+Layout:
+  Enabled: true

Guardfile

@@ -7,4 +7,3 @@ guard :minitest do
   watch(%r{^lib/(.+)\.rb$})         { |m| "spec/#{m[1]}_spec.rb" }
   watch(%r{^spec/spec_helper\.rb$}) { 'spec' }
 end
-

lib/rack/attack.rb

@@ -22,7 +22,6 @@ class Rack::Attack
   autoload :Allow2Ban,       'rack/attack/allow2ban'
 
   class << self
-
     attr_accessor :notifier, :blocklisted_response, :throttled_response
 
     def safelist(name, &block)
@@ -64,8 +63,11 @@ class Rack::Attack
     end
 
     def safelists;  @safelists  ||= {}; end
+
     def blocklists; @blocklists ||= {}; end
+
     def throttles;  @throttles  ||= {}; end
+
     def tracks;     @tracks     ||= {}; end
 
     def whitelists
@@ -126,7 +128,7 @@ class Rack::Attack
 
     def blacklisted_response=(res)
       warn "[DEPRECATION] 'Rack::Attack.blacklisted_response=' is deprecated.  Please use 'blocklisted_response=' instead."
-      self.blocklisted_response=(res)
+      self.blocklisted_response = res
     end
 
     def blacklisted_response
@@ -147,10 +149,10 @@ class Rack::Attack
 
   # Set defaults
   @notifier             = ActiveSupport::Notifications if defined?(ActiveSupport::Notifications)
-  @blocklisted_response = lambda {|env| [403, {'Content-Type' => 'text/plain'}, ["Forbidden\n"]] }
-  @throttled_response   = lambda {|env|
+  @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"]]
+    [429, { 'Content-Type' => 'text/plain', 'Retry-After' => retry_after.to_s }, ["Retry later\n"]]
   }
 
   def initialize(app)
@@ -174,8 +176,5 @@ class Rack::Attack
   end
 
   extend Forwardable
-  def_delegators self, :safelisted?,
-                       :blocklisted?,
-                       :throttled?,
-                       :tracked?
+  def_delegators self, :safelisted?, :blocklisted?, :throttled?, :tracked?
 end

lib/rack/attack/allow2ban.rb

@@ -3,6 +3,7 @@ module Rack
     class Allow2Ban < Fail2Ban
       class << self
         protected
+
         def key_prefix
           'allow2ban'
         end

lib/rack/attack/blocklist.rb

@@ -5,7 +5,6 @@ module Rack
         super
         @type = :blocklist
       end
-
     end
   end
 end

lib/rack/attack/cache.rb

@@ -1,7 +1,6 @@
 module Rack
   class Attack
     class Cache
-
       attr_accessor :prefix
 
       def initialize

lib/rack/attack/check.rb

@@ -8,7 +8,7 @@ module Rack
       end
 
       def [](req)
-        block[req].tap {|match|
+        block[req].tap { |match|
           if match
             req.env["rack.attack.matched"] = name
             req.env["rack.attack.match_type"] = type
@@ -21,4 +21,3 @@ module Rack
     end
   end
 end
-

lib/rack/attack/fail2ban.rb

@@ -27,6 +27,7 @@ module Rack
         end
 
         protected
+
         def key_prefix
           'fail2ban'
         end
@@ -40,8 +41,8 @@ module Rack
           true
         end
 
-
         private
+
         def ban!(discriminator, bantime)
           cache.write("#{key_prefix}:ban:#{discriminator}", 1, bantime)
         end

lib/rack/attack/path_normalizer.rb

@@ -1,5 +1,4 @@
 class Rack::Attack
-
   # When using Rack::Attack with a Rails app, developers expect the request path
   # to be normalized. In particular, trailing slashes are stripped.
   # (See https://git.io/v0rrR for implementation.)
@@ -15,10 +14,9 @@ class Rack::Attack
   end
 
   PathNormalizer = if defined?(::ActionDispatch::Journey::Router::Utils)
-                 # For Rails apps
-                 ::ActionDispatch::Journey::Router::Utils
-               else
-                 FallbackPathNormalizer
-               end
-
+                     # For Rails apps
+                     ::ActionDispatch::Journey::Router::Utils
+                   else
+                     FallbackPathNormalizer
+                   end
 end

lib/rack/attack/safelist.rb

@@ -5,7 +5,6 @@ module Rack
         super
         @type = :safelist
       end
-
     end
   end
 end

lib/rack/attack/store_proxy.rb

@@ -12,8 +12,8 @@ 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/dalli_proxy.rb

@@ -28,14 +28,14 @@ module Rack
         rescue Dalli::DalliError
         end
 
-        def write(key, value, options={})
+        def write(key, value, options = {})
           with do |client|
             client.set(key, value, options.fetch(:expires_in, 0), raw: true)
           end
         rescue Dalli::DalliError
         end
 
-        def increment(key, amount, options={})
+        def increment(key, amount, options = {})
           with do |client|
             client.incr(key, amount, options.fetch(:expires_in, 0), amount)
           end
@@ -58,7 +58,6 @@ module Rack
             end
           end
         end
-
       end
     end
   end

lib/rack/attack/store_proxy/mem_cache_proxy.rb

@@ -14,21 +14,21 @@ module Rack
         def read(key)
           # Second argument: reading raw value
           get(key, true)
-          rescue MemCache::MemCacheError
+        rescue MemCache::MemCacheError
         end
 
-        def write(key, value, options={})
+        def write(key, value, options = {})
           # Third argument: writing raw value
           set(key, value, options.fetch(:expires_in, 0), true)
         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
@@ -44,7 +44,6 @@ module Rack
             end
           end
         end
-
       end
     end
   end

lib/rack/attack/store_proxy/redis_store_proxy.rb

@@ -24,7 +24,7 @@ module Rack
         rescue Redis::BaseError
         end
 
-        def write(key, value, options={})
+        def write(key, value, options = {})
           if (expires_in = options[:expires_in])
             setex(key, expires_in, value, raw: true)
           else
@@ -33,7 +33,7 @@ module Rack
         rescue Redis::BaseError
         end
 
-        def increment(key, amount, options={})
+        def increment(key, amount, options = {})
           count = nil
 
           pipelined do
@@ -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

@@ -1,4 +1,5 @@
 # -*- encoding: utf-8 -*-
+
 lib = File.expand_path('../lib/', __FILE__)
 $:.unshift lib unless $:.include?(lib)
 

spec/acceptance/safelisting_ip_spec.rb

@@ -46,4 +46,3 @@ describe "Safelist an IP" do
     assert_equal :safelist, notification_type
   end
 end
-

spec/allow2ban_spec.rb

@@ -7,10 +7,10 @@ describe 'Rack::Attack.Allow2Ban' do
     @findtime = 60
     @bantime  = 60
     Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new
-    @f2b_options = {:bantime => @bantime, :findtime => @findtime, :maxretry => 2}
+    @f2b_options = { :bantime => @bantime, :findtime => @findtime, :maxretry => 2 }
 
     Rack::Attack.blocklist('pentest') do |req|
-      Rack::Attack::Allow2Ban.filter(req.ip, @f2b_options){req.query_string =~ /OMGHAX/}
+      Rack::Attack::Allow2Ban.filter(req.ip, @f2b_options) { req.query_string =~ /OMGHAX/ }
     end
   end
 
@@ -31,7 +31,7 @@ describe 'Rack::Attack.Allow2Ban' do
         end
 
         it 'increases fail count' do
-          key = "rack::attack:#{Time.now.to_i/@findtime}:allow2ban:count:1.2.3.4"
+          key = "rack::attack:#{Time.now.to_i / @findtime}:allow2ban:count:1.2.3.4"
           @cache.store.read(key).must_equal 1
         end
 
@@ -53,7 +53,7 @@ describe 'Rack::Attack.Allow2Ban' do
         end
 
         it 'increases fail count' do
-          key = "rack::attack:#{Time.now.to_i/@findtime}:allow2ban:count:1.2.3.4"
+          key = "rack::attack:#{Time.now.to_i / @findtime}:allow2ban:count:1.2.3.4"
           @cache.store.read(key).must_equal 2
         end
 
@@ -89,7 +89,7 @@ describe 'Rack::Attack.Allow2Ban' do
       end
 
       it 'does not increase fail count' do
-        key = "rack::attack:#{Time.now.to_i/@findtime}:allow2ban:count:1.2.3.4"
+        key = "rack::attack:#{Time.now.to_i / @findtime}:allow2ban:count:1.2.3.4"
         @cache.store.read(key).must_equal 2
       end
 
@@ -109,7 +109,7 @@ describe 'Rack::Attack.Allow2Ban' do
       end
 
       it 'does not increase fail count' do
-        key = "rack::attack:#{Time.now.to_i/@findtime}:allow2ban:count:1.2.3.4"
+        key = "rack::attack:#{Time.now.to_i / @findtime}:allow2ban:count:1.2.3.4"
         @cache.store.read(key).must_equal 2
       end
 

spec/fail2ban_spec.rb

@@ -7,10 +7,10 @@ describe 'Rack::Attack.Fail2Ban' do
     @findtime = 60
     @bantime  = 60
     Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new
-    @f2b_options = {:bantime => @bantime, :findtime => @findtime, :maxretry => 2}
+    @f2b_options = { :bantime => @bantime, :findtime => @findtime, :maxretry => 2 }
 
     Rack::Attack.blocklist('pentest') do |req|
-      Rack::Attack::Fail2Ban.filter(req.ip, @f2b_options){req.query_string =~ /OMGHAX/}
+      Rack::Attack::Fail2Ban.filter(req.ip, @f2b_options) { req.query_string =~ /OMGHAX/ }
     end
   end
 
@@ -31,7 +31,7 @@ describe 'Rack::Attack.Fail2Ban' do
         end
 
         it 'increases fail count' do
-          key = "rack::attack:#{Time.now.to_i/@findtime}:fail2ban:count:1.2.3.4"
+          key = "rack::attack:#{Time.now.to_i / @findtime}:fail2ban:count:1.2.3.4"
           @cache.store.read(key).must_equal 1
         end
 
@@ -53,7 +53,7 @@ describe 'Rack::Attack.Fail2Ban' do
         end
 
         it 'increases fail count' do
-          key = "rack::attack:#{Time.now.to_i/@findtime}:fail2ban:count:1.2.3.4"
+          key = "rack::attack:#{Time.now.to_i / @findtime}:fail2ban:count:1.2.3.4"
           @cache.store.read(key).must_equal 2
         end
 
@@ -75,7 +75,7 @@ describe 'Rack::Attack.Fail2Ban' do
         end
 
         it 'resets fail count' do
-          key = "rack::attack:#{Time.now.to_i/@findtime}:fail2ban:count:1.2.3.4"
+          key = "rack::attack:#{Time.now.to_i / @findtime}:fail2ban:count:1.2.3.4"
           assert_nil @cache.store.read(key)
         end
 
@@ -110,7 +110,7 @@ describe 'Rack::Attack.Fail2Ban' do
       end
 
       it 'does not increase fail count' do
-        key = "rack::attack:#{Time.now.to_i/@findtime}:fail2ban:count:1.2.3.4"
+        key = "rack::attack:#{Time.now.to_i / @findtime}:fail2ban:count:1.2.3.4"
         @cache.store.read(key).must_equal 2
       end
 
@@ -130,7 +130,7 @@ describe 'Rack::Attack.Fail2Ban' do
       end
 
       it 'does not increase fail count' do
-        key = "rack::attack:#{Time.now.to_i/@findtime}:fail2ban:count:1.2.3.4"
+        key = "rack::attack:#{Time.now.to_i / @findtime}:fail2ban:count:1.2.3.4"
         @cache.store.read(key).must_equal 2
       end
 

spec/rack_attack_dalli_proxy_spec.rb

@@ -1,10 +1,8 @@
 require_relative 'spec_helper'
 
 describe Rack::Attack::StoreProxy::DalliProxy do
-
   it 'should stub Dalli::Client#with on older clients' do
     proxy = Rack::Attack::StoreProxy::DalliProxy.new(Class.new)
     proxy.with {} # will not raise an error
   end
-
 end

spec/rack_attack_spec.rb

@@ -5,7 +5,7 @@ describe 'Rack::Attack' do
 
   describe 'normalizing paths' do
     before do
-      Rack::Attack.blocklist("banned_path") {|req| req.path == '/foo' }
+      Rack::Attack.blocklist("banned_path") { |req| req.path == '/foo' }
     end
 
     it 'blocks requests with trailing slash' do
@@ -17,7 +17,7 @@ describe 'Rack::Attack' do
   describe 'blocklist' do
     before do
       @bad_ip = '1.2.3.4'
-      Rack::Attack.blocklist("ip #{@bad_ip}") {|req| req.ip == @bad_ip }
+      Rack::Attack.blocklist("ip #{@bad_ip}") { |req| req.ip == @bad_ip }
     end
 
     it('has a blocklist') {
@@ -25,7 +25,7 @@ describe 'Rack::Attack' do
     }
 
     it('has a blacklist with a deprication warning') {
-      _, stderror  = capture_io do
+      _, stderror = capture_io do
         Rack::Attack.blacklists.key?("ip #{@bad_ip}").must_equal true
       end
       assert_match "[DEPRECATION] 'Rack::Attack.blacklists' is deprecated.  Please use 'blocklists' instead.", stderror
@@ -50,13 +50,13 @@ describe 'Rack::Attack' do
     describe "and safelist" do
       before do
         @good_ua = 'GoodUA'
-        Rack::Attack.safelist("good ua") {|req| req.user_agent == @good_ua }
+        Rack::Attack.safelist("good ua") { |req| req.user_agent == @good_ua }
       end
 
       it('has a safelist') { Rack::Attack.safelists.key?("good ua") }
 
       it('has a whitelist with a deprication warning') {
-        _, stderror  = capture_io do
+        _, stderror = capture_io do
           Rack::Attack.whitelists.key?("good ua")
         end
         assert_match "[DEPRECATION] 'Rack::Attack.whitelists' is deprecated.  Please use 'safelists' instead.", stderror
@@ -82,7 +82,7 @@ describe 'Rack::Attack' do
       end
 
       it 'should give a deprication warning for blacklisted_response' do
-        _, stderror  = capture_io do
+        _, stderror = capture_io do
           Rack::Attack.blacklisted_response
         end
         assert_match "[DEPRECATION] 'Rack::Attack.blacklisted_response' is deprecated.  Please use 'blocklisted_response' instead.", stderror

spec/rack_attack_throttle_spec.rb

@@ -15,7 +15,7 @@ describe 'Rack::Attack.throttle' do
     before { get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' }
 
     it 'should set the counter for one request' do
-      key = "rack::attack:#{Time.now.to_i/@period}:ip/sec:1.2.3.4"
+      key = "rack::attack:#{Time.now.to_i / @period}:ip/sec:1.2.3.4"
       Rack::Attack.cache.store.read(key).must_equal 1
     end
 
@@ -37,7 +37,7 @@ describe 'Rack::Attack.throttle' do
     it 'should tag the env' do
       last_request.env['rack.attack.matched'].must_equal 'ip/sec'
       last_request.env['rack.attack.match_type'].must_equal :throttle
-      last_request.env['rack.attack.match_data'].must_equal({:count => 2, :limit => 1, :period => @period})
+      last_request.env['rack.attack.match_data'].must_equal({ :count => 2, :limit => 1, :period => @period })
       last_request.env['rack.attack.match_discriminator'].must_equal('1.2.3.4')
     end
 
@@ -60,7 +60,7 @@ describe 'Rack::Attack.throttle with limit as proc' do
     before { get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' }
 
     it 'should set the counter for one request' do
-      key = "rack::attack:#{Time.now.to_i/@period}:ip/sec:1.2.3.4"
+      key = "rack::attack:#{Time.now.to_i / @period}:ip/sec:1.2.3.4"
       Rack::Attack.cache.store.read(key).must_equal 1
     end
 
@@ -84,7 +84,7 @@ describe 'Rack::Attack.throttle with period as proc' do
     before { get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' }
 
     it 'should set the counter for one request' do
-      key = "rack::attack:#{Time.now.to_i/@period}:ip/sec:1.2.3.4"
+      key = "rack::attack:#{Time.now.to_i / @period}:ip/sec:1.2.3.4"
       Rack::Attack.cache.store.read(key).must_equal 1
     end
 
@@ -108,7 +108,7 @@ describe 'Rack::Attack.throttle with block retuning nil' do
     before { get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' }
 
     it 'should not set the counter' do
-      key = "rack::attack:#{Time.now.to_i/@period}:ip/sec:1.2.3.4"
+      key = "rack::attack:#{Time.now.to_i / @period}:ip/sec:1.2.3.4"
       assert_nil Rack::Attack.cache.store.read(key)
     end
 

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
@@ -31,7 +31,7 @@ describe 'Rack::Attack.track' do
     before do
       Counter.reset
       # A second track
-      Rack::Attack.track("homepage"){ |req| req.path == "/"}
+      Rack::Attack.track("homepage") { |req| req.path == "/" }
 
       ActiveSupport::Notifications.subscribe("rack.attack") do |*args|
         Counter.incr
@@ -47,14 +47,14 @@ describe 'Rack::Attack.track' do
 
   describe "without limit and period options" do
     it "should assign the track filter to a Check instance" do
-      tracker = Rack::Attack.track("homepage") { |req| req.path == "/"}
+      tracker = Rack::Attack.track("homepage") { |req| req.path == "/" }
       tracker.filter.class.must_equal Rack::Attack::Check
     end
   end
 
   describe "with limit and period options" do
     it "should assign the track filter to a Throttle instance" do
-      tracker = Rack::Attack.track("homepage", :limit => 10, :period => 10) { |req| req.path == "/"}
+      tracker = Rack::Attack.track("homepage", :limit => 10, :period => 10) { |req| req.path == "/" }
       tracker.filter.class.must_equal Rack::Attack::Throttle
     end
   end

spec/spec_helper.rb

@@ -12,7 +12,7 @@ require "rack/attack"
 begin
   require 'pry'
 rescue LoadError
-  #nothing to do here
+  # nothing to do here
 end
 
 if RUBY_ENGINE == "ruby"
@@ -20,7 +20,6 @@ if RUBY_ENGINE == "ruby"
 end
 
 class MiniTest::Spec
-
   include Rack::Test::Methods
 
   before do
@@ -43,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