Update Rubocop checking (#106)

* Fix problem with RuboCop checks

We were using Hound for our RuboCop checks, but it wasn't working
properly for some reason.  Switching to [Reviewdog's GitHub Action][1].

[1]: https://github.com/reviewdog/action-rubocop

* Rename Reviewdog GitHub Actions

The previous naming format led to our Reviewdog actions appearing with
an incorrect name in the GitHub Actions UI.

* Use the RuboCop default line length of 120 chars

[RuboCop changed the default from 80 chars to 120][1] in May 2020.

[1]: rubocop/rubocop#7952

* Add an empty line after guard clauses

As prompted by RuboCop.

* Rename variable as prompted by RuboCop

* Rename variable as prompted by RuboCop

* Remove unused block argument as per RuboCop

* Move private declaration to outside method

As per RuboCop

* Remove obsolete RuboCop exclusion

* Enable all new RuboCop cops

The more RuboCop checks, the better :-)

* Fix new RuboCop errors

.github/workflows/reviewdog.yml

@@ -6,7 +6,7 @@ on:  # yamllint disable-line rule:truthy
 jobs:
 
   hadolint:
-    name: runner / hadolint
+    name: hadolint
     runs-on: ubuntu-20.04
     steps:
       - name: Check out code
@@ -18,7 +18,7 @@ jobs:
           reporter: github-check
 
   yamllint:
-    name: runner / yamllint
+    name: yamllint
     runs-on: ubuntu-20.04
     steps:
       - uses: actions/checkout@v2.3.2
@@ -27,3 +27,14 @@ jobs:
         with:
           github_token: ${{ secrets.github_token }}
           reporter: github-check
+
+  rubocop:
+    name: rubocop
+    runs-on: ubuntu-20.04
+    steps:
+      - uses: actions/checkout@v2.3.2
+      - name: rubocop
+        uses: reviewdog/action-rubocop@v1.5.0
+        with:
+          github_token: ${{ secrets.github_token }}
+          reporter: github-check

.rubocop.yml

@@ -1,14 +1,11 @@
 ---
-Layout/LineLength:
-  Max: 100
+
+AllCops:
+  NewCops: enable
+
 Style/FrozenStringLiteralComment:
   Enabled: false
 
-Style/FormatStringToken:
-  # Thefalse positives in the full journey spec should be fixed in a new version of RuboCop soon - see #
-  Exclude:
-    - spec/full_journey_spec.rb
-
 Metrics/BlockLength:
   Exclude:
     - 'Rakefile'

config.ru

@@ -14,5 +14,5 @@ OmniAuth.config.on_failure = proc do |env|
   OmniAuth::FailureEndpoint.new(env).redirect_to_failure
 end
 
-require './hot_custard_payments.rb'
+require './hot_custard_payments'
 run HotCustardApp

google_sheet.rb

@@ -20,31 +20,23 @@ def range(spreadsheet_key, range)
       end
     end
 
-    private def boundaries(a1notation_range)
-      Hash.new({}).tap do |the_boundaries|
-        a1_notation = a1notation_range.split('!').last.split(':')
-        the_boundaries[:start_column], the_boundaries[:start_row] = a1_notation.first.scan(/\D+|\d+/)
-        the_boundaries[:end_column], the_boundaries[:end_row] = a1_notation.last.split(':').last.scan(/\D+|\d+/)
-      end
-    end
-
     def spreadsheet(spreadsheet_key)
       # We use a backoff here to avoid hitting the Google Sheets API rate limit per 100 seconds
       exponential_backoff do
         google_sheets.get_spreadsheet(spreadsheet_key)
       end
     end
 
-    def exponential_wait_time(n)
-      2**n.tap { |wait_time| puts "wait time: #{wait_time}s" }
+    def exponential_wait_time(counter)
+      2**counter.tap { |wait_time| puts "wait time: #{wait_time}s" }
     end
 
     # rubocop:disable Style/RescueStandardError
     def exponential_backoff
       (0..10).each do |n|
         return yield
-      rescue => error
-        puts error.inspect
+      rescue => e
+        puts e.inspect
         sleep(exponential_wait_time(n))
         next
       end
@@ -76,5 +68,15 @@ def to_hash_array(cells_with_header_row)
     def title(spreadsheet_key)
       spreadsheet(spreadsheet_key).properties.title
     end
+
+    private
+
+    def boundaries(a1notation_range)
+      Hash.new({}).tap do |the_boundaries|
+        a1_notation = a1notation_range.split('!').last.split(':')
+        the_boundaries[:start_column], the_boundaries[:start_row] = a1_notation.first.scan(/\D+|\d+/)
+        the_boundaries[:end_column], the_boundaries[:end_row] = a1_notation.last.split(':').last.scan(/\D+|\d+/)
+      end
+    end
   end
 end

hcmoney.rb

@@ -22,6 +22,7 @@ class HCMoney
 
   def self.amount_that_can_be_credited(creditor_balance:, hot_custard_balance:)
     return [hot_custard_balance, -creditor_balance].max if hot_custard_balance.negative?
+
     case hot_custard_balance <=> creditor_balance
     when -1 then hot_custard_balance
     when 0 then hot_custard_balance
@@ -59,6 +60,7 @@ def markup_to_cover_transfer_fees_and_rate_fluctuation(markup_percentage)
 
   def worth_showing?
     return false if blank? @monetary_string
+
     (to_i >= 1) || (to_i <= -1)
   end
 
@@ -89,6 +91,7 @@ def /(other)
 
   def <=>(other)
     return nil unless other.is_a?(HCMoney)
+
     @money <=> other.money
   end
 

hot_custard_payments.rb

@@ -24,13 +24,14 @@ class HotCustardApp < Sinatra::Base
   set(:role) { |role| condition { halt 403 if (role == :financial_admin) && !financial_admin? } }
 
   before do
-    pass if request.path_info.match? %r{^\/auth\/|^\/privacy$}
+    pass if request.path_info.match? %r{^/auth/|^/privacy$}
     redirect to('/auth/facebook') unless current_user
   end
 
   def individual_balances_for(username)
     datastore_balances = user_datastore["balance:#{username}"]
     return {} unless datastore_balances
+
     JSON.parse(datastore_balances).select { |_key, value| HCMoney.new(value).worth_showing? }
   end
 
@@ -92,7 +93,7 @@ def creditor_item_amounts(creditor_balance, hot_custard_balance)
 
   def balances_for(person)
     string_balances = JSON.parse user_datastore["balance:#{person}"]
-    string_balances.map { |item, amount| [item, HCMoney.new(amount)] }.to_h
+    string_balances.transform_values { |amount| HCMoney.new(amount) }
   end
 
   def credits_for(creditor)

populate_datastore.rb

@@ -12,14 +12,15 @@
 DATASTORE = Redis.new(url: ENV['REDIS_URL'])
 
 def check_worksheets
-  spreadsheet_keys.reverse.take(20).each_with_index { |key, i| check_worksheet(key) }
+  spreadsheet_keys.reverse.take(20).each { |key| check_worksheet(key) }
 end
 
 def check_worksheet(spreadsheet_key)
   people_with_costs = GoogleSheet.range(spreadsheet_key, 'PeopleWithCosts')
   start_row = people_with_costs[:boundaries][:start_row]
   full_people_row = GoogleSheet.range(spreadsheet_key, "#{start_row}:#{start_row}")[:values].first
   raise invalid_spreadsheet_message(spreadsheet_key) unless full_people_row_valid? full_people_row
+
   puts "Successfully validated spreadsheet #{spreadsheet_key}"
 end
 
@@ -31,9 +32,11 @@ def full_people_row_valid?(full_people_row)
   sliced = full_people_row.slice_after('').to_a
   return false unless sliced.size == 5
   return false unless sliced[1..3].all? { |a| a == [''] }
+
   sliced[0].pop
   return false unless sliced[0] == sliced[4]
   return false unless sliced[0] == sliced[0].uniq
+
   true
 end