Bump rubocop and address complaints

Also: use cops from mas-standards
This requires `rubocop` to be bumped to `0.54.0`

.gitignore

@@ -1,5 +1,6 @@
 .bundle
 .DS_Store
+.rubocop-*
 /log/*.log
 /tmp
 /vendor/assets/bower_components

.rubocop.yml

@@ -1,17 +1,27 @@
+inherit_from:
+  - .rubocop_todo.yml
+  - https://raw.githubusercontent.com/moneyadviceservice/mas-standards/master/.rubocop.yml
+
 Documentation:
   Enabled: false
 Style/FrozenStringLiteralComment:
   Enabled: false
 Metrics/BlockLength:
   Exclude:
-  - 'spec/**/*'
+    - 'spec/**/*'
+Metrics/ClassLength:
+  Exclude:
+    - 'app/models/firm.rb'
 Metrics/LineLength:
   Max: 80
   IgnoredPatterns: ['^\s*class\s']
   Exclude:
-  - 'spec/**/*'
+    - 'spec/**/*'
+    - 'app/models/snapshot/*.rb'
 Metrics/MethodLength:
   Max: 15
+  Exclude:
+    - 'app/models/snapshot/*.rb'
 Style/ClassAndModuleChildren:
   Enabled: false
 Style/CollectionMethods:
@@ -42,24 +52,24 @@ AllCops:
   TargetRailsVersion: 4.2
   DisplayCopNames: true
   Exclude:
-  - 'bin/**/*'
-  - 'config/**/*'
-  - 'db/**/*'
-  - 'Guardfile'
-  - 'lib/tasks/**/*'
-  - 'node_modules/**/*'
-  - 'script/**/*'
-  - 'spec/spec_helper.rb'
-  - 'spec/cassettes/**/*'
-  - 'features/**/*'
-  - 'spec/features/**/*'
-  - 'tmp/**/*'
-  - 'vendor/**/*'
-  - 'Rakefile'
-  - 'spec/dummy/config/**/*'
-  - 'spec/dummy/db/**/*'
-  - 'spec/dummy/script/**/*'
-  # Project specific exclusions
-  - 'lib/fca/**/*'
+    - 'bin/**/*'
+    - 'config/**/*'
+    - 'db/**/*'
+    - 'Guardfile'
+    - 'lib/tasks/**/*'
+    - 'node_modules/**/*'
+    - 'script/**/*'
+    - 'spec/spec_helper.rb'
+    - 'spec/cassettes/**/*'
+    - 'features/**/*'
+    - 'spec/features/**/*'
+    - 'tmp/**/*'
+    - 'vendor/**/*'
+    - 'Rakefile'
+    - 'spec/dummy/config/**/*'
+    - 'spec/dummy/db/**/*'
+    - 'spec/dummy/script/**/*'
+    # Project specific exclusions
+    - 'lib/fca/**/*'
 
 # Project specific settings

.rubocop_todo.yml

@@ -0,0 +1,157 @@
+# This configuration was generated by
+# `rubocop --auto-gen-config`
+# on 2019-01-08 10:21:45 +0000 using RuboCop version 0.54.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
+# Cop supports --auto-correct.
+# Configuration parameters: Categories, ExpectedOrder.
+# ExpectedOrder: module_inclusion, constants, public_class_methods, initializer, public_methods, protected_methods, private_methods
+Layout/ClassStructure:
+  Exclude:
+    - 'app/controllers/self_service/abstract_firms_controller.rb'
+
+# Offense count: 2
+Lint/AmbiguousOperator:
+  Exclude:
+    - 'app/models/firm.rb'
+
+# Offense count: 2
+Lint/DuplicateMethods:
+  Exclude:
+    - 'app/forms/admin/move_advisers_form.rb'
+    - 'app/models/firm.rb'
+
+# Offense count: 1
+Lint/NestedMethodDefinition:
+  Exclude:
+    - 'app/models/friendly_namable.rb'
+
+# Offense count: 2
+Lint/ReturnInVoidContext:
+  Exclude:
+    - 'app/models/office.rb'
+
+# Offense count: 3
+Metrics/AbcSize:
+  Max: 17
+
+# Offense count: 3
+Naming/MemoizedInstanceVariableName:
+  Exclude:
+    - 'app/jobs/fca_import_job.rb'
+    - 'app/models/snapshot.rb'
+
+# Offense count: 2
+# Configuration parameters: NamePrefix, NamePrefixBlacklist, NameWhitelist, MethodDefinitionMacros.
+# NamePrefix: is_, has_, have_
+# NamePrefixBlacklist: is_, has_, have_
+# NameWhitelist: is_a?
+# MethodDefinitionMacros: define_method, define_singleton_method
+Naming/PredicateName:
+  Exclude:
+    - 'spec/**/*'
+    - 'app/models/adviser.rb'
+    - 'app/models/office.rb'
+
+# Offense count: 6
+# Configuration parameters: MinNameLength, AllowNamesEndingInNumbers, AllowedNames, ForbiddenNames.
+# AllowedNames: io, id, to
+Naming/UncommunicativeMethodParamName:
+  Exclude:
+    - 'app/controllers/admin/lookup/fca_import_controller.rb'
+    - 'app/helpers/admin/lookup/fca_import_helper.rb'
+    - 'lib/adviser_list_csv.rb'
+    - 'lib/river/core.rb'
+    - 'lib/river/runners.rb'
+
+# Offense count: 9
+# Cop supports --auto-correct.
+Rails/ActiveRecordAliases:
+  Exclude:
+    - 'app/controllers/admin/principals_controller.rb'
+    - 'app/controllers/admin/users_controller.rb'
+    - 'spec/controllers/admin/firms_controller_spec.rb'
+    - 'spec/controllers/selfservice/firms_controller_spec.rb'
+    - 'spec/controllers/selfservice/trading_names_controller_spec.rb'
+    - 'spec/models/firm_spec.rb'
+    - 'spec/models/office_spec.rb'
+    - 'spec/support/contexts/advisers_controller.rb'
+    - 'spec/support/contexts/offices_controller.rb'
+
+# Offense count: 16
+# Configuration parameters: Include.
+# Include: app/models/**/*.rb
+Rails/HasAndBelongsToMany:
+  Exclude:
+    - 'app/models/adviser.rb'
+    - 'app/models/allowed_payment_method.rb'
+    - 'app/models/firm.rb'
+    - 'app/models/in_person_advice_method.rb'
+    - 'app/models/initial_advice_fee_structure.rb'
+    - 'app/models/investment_size.rb'
+    - 'app/models/ongoing_advice_fee_structure.rb'
+    - 'app/models/other_advice_method.rb'
+
+# Offense count: 2
+# Configuration parameters: Include.
+# Include: app/models/**/*.rb
+Rails/HasManyOrHasOneDependent:
+  Exclude:
+    - 'app/models/initial_meeting_duration.rb'
+    - 'app/models/lookup/firm.rb'
+
+# Offense count: 8
+# Configuration parameters: Include.
+# Include: app/models/**/*.rb
+Rails/InverseOf:
+  Exclude:
+    - 'app/models/firm.rb'
+    - 'app/models/lookup/firm.rb'
+    - 'app/models/principal.rb'
+    - 'app/models/user.rb'
+
+# Offense count: 3
+# Configuration parameters: Blacklist.
+# Blacklist: decrement!, decrement_counter, increment!, increment_counter, toggle!, touch, update_all, update_attribute, update_column, update_columns, update_counters
+Rails/SkipsModelValidations:
+  Exclude:
+    - 'spec/models/office_spec.rb'
+
+# Offense count: 8
+# Cop supports --auto-correct.
+Style/EmptyLineAfterGuardClause:
+  Exclude:
+    - 'app/controllers/self_service/principals_controller.rb'
+    - 'app/forms/new_principal_form.rb'
+    - 'app/helpers/self_service/self_service_helper.rb'
+    - 'app/models/firm.rb'
+    - 'lib/firm_indexer.rb'
+    - 'spec/support/self_service/firm_edit_page.rb'
+    - 'spec/support/shared_examples/system_named.rb'
+
+# Offense count: 1
+# Configuration parameters: AllowedVariables.
+Style/GlobalVars:
+  Exclude:
+    - 'lib/stats.rb'
+
+# Offense count: 3
+# Configuration parameters: MinBodyLength.
+Style/GuardClause:
+  Exclude:
+    - 'app/models/adviser.rb'
+    - 'app/models/office.rb'
+    - 'app/models/principal.rb'
+
+# Offense count: 5
+# Configuration parameters: SuspiciousParamNames.
+# SuspiciousParamNames: options, opts, args, params, parameters
+Style/OptionHash:
+  Exclude:
+    - 'app/presenters/self_service/status_presenter.rb'
+    - 'spec/controllers/selfservice/firms_controller_spec.rb'
+    - 'spec/controllers/selfservice/trading_names_controller_spec.rb'

Gemfile

@@ -25,16 +25,13 @@ gem 'dough-ruby',
     github: 'moneyadviceservice/dough',
     require: 'dough',
     tag: 'v5.12.0.267'
+gem 'geocoder', '~> 1.4.7'
+gem 'httpclient', '~> 2.8.3'
 gem 'jquery-rails'
 gem 'kaminari'
+gem 'language_list', '~> 1.2.1'
 gem 'letter_opener', group: :development
 gem 'mailjet'
-gem 'uk_postcode', '~> 2.1.2'
-gem 'uk_phone_numbers', '~> 0.1.1'
-gem 'language_list', '~> 1.2.1'
-gem 'httpclient', '~> 2.8.3'
-gem 'geocoder', '~> 1.4.7'
-gem 'statsd-ruby', '~> 1.4.0'
 gem 'oga'
 gem 'pg', '0.21.0'
 gem 'rails_email_validator'
@@ -54,7 +51,10 @@ gem 'sidekiq-unique-jobs'
 gem 'sidetiq'
 gem 'sinatra', require: false
 gem 'slack-ruby-client'
+gem 'statsd-ruby', '~> 1.4.0'
 gem 'uglifier', '>= 1.3.0'
+gem 'uk_phone_numbers', '~> 0.1.1'
+gem 'uk_postcode', '~> 2.1.2'
 gem 'unicorn'
 
 group :test, :development do
@@ -66,7 +66,7 @@ group :test, :development do
   gem 'pry-byebug'
   gem 'pry-rails'
   gem 'rspec-rails'
-  gem 'rubocop', '0.49.0'
+  gem 'rubocop', '0.54.0'
   gem 'timecop'
 end
 

Gemfile.lock

@@ -244,7 +244,7 @@ GEM
       ruby-ll (~> 2.1)
     orm_adapter (0.5.0)
     parallel (1.12.1)
-    parser (2.5.0.5)
+    parser (2.5.3.0)
       ast (~> 2.4.0)
     pg (0.21.0)
     poltergeist (1.17.0)
@@ -253,7 +253,7 @@ GEM
       websocket-driver (>= 0.2.0)
     polyamorous (1.3.3)
       activerecord (>= 3.0)
-    powerpack (0.1.1)
+    powerpack (0.1.2)
     pry (0.11.3)
       coderay (~> 1.1.0)
       method_source (~> 0.9.0)
@@ -294,8 +294,7 @@ GEM
       activesupport (= 4.2.10)
       rake (>= 0.8.7)
       thor (>= 0.18.1, < 2.0)
-    rainbow (2.2.2)
-      rake
+    rainbow (3.0.0)
     raindrops (0.19.0)
     rake (11.3.0)
     ransack (1.8.8)
@@ -335,17 +334,17 @@ GEM
       rspec-mocks (~> 3.7.0)
       rspec-support (~> 3.7.0)
     rspec-support (3.7.1)
-    rubocop (0.49.0)
+    rubocop (0.54.0)
       parallel (~> 1.10)
-      parser (>= 2.3.3.1, < 3.0)
+      parser (>= 2.5)
       powerpack (~> 0.1)
-      rainbow (>= 1.99.1, < 3.0)
+      rainbow (>= 2.2.2, < 4.0)
       ruby-progressbar (~> 1.7)
       unicode-display_width (~> 1.0, >= 1.0.1)
     ruby-ll (2.1.2)
       ansi
       ast
-    ruby-progressbar (1.9.0)
+    ruby-progressbar (1.10.0)
     rubyzip (1.2.1)
     safe_yaml (1.0.4)
     sass (3.2.19)
@@ -408,7 +407,7 @@ GEM
     unf (0.1.4)
       unf_ext
     unf_ext (0.0.7.5)
-    unicode-display_width (1.3.0)
+    unicode-display_width (1.4.1)
     unicorn (5.4.0)
       kgio (~> 2.6)
       raindrops (~> 0.7)
@@ -468,7 +467,7 @@ DEPENDENCIES
   rollbar
   rspec-collection_matchers
   rspec-rails
-  rubocop (= 0.49.0)
+  rubocop (= 0.54.0)
   rubyzip
   sass-rails
   sidekiq (~> 3.3.4)

app/helpers/admin/firms_helper.rb

@@ -1,17 +1,15 @@
-# coding: utf-8
-
 module Admin::FirmsHelper
   def render_questionnaire_response(firm, attribute_name)
     value = firm.send(attribute_name)
 
     if value.present?
-      if value.respond_to?(:collect)
-        output = value.collect(&:name).to_sentence
-      elsif value.respond_to?(:name)
-        output = value.name
-      else
-        output = render_literal_or_fee_or_percentage(value, attribute_name)
-      end
+      output = if value.respond_to?(:collect)
+                 value.collect(&:name).to_sentence
+               elsif value.respond_to?(:name)
+                 value.name
+               else
+                 render_literal_or_fee_or_percentage(value, attribute_name)
+               end
       output
     else
       'not set'

app/jobs/fca_import_job.rb

@@ -60,7 +60,7 @@ def notify_slack(text)
       as_user: true,
       text:    "<!here> #{text}"
     )
-  rescue
+  rescue StandardError
     logger.error 'An error occured while trying to post msg'
     retry if (tries += 1) <= 3
   end

app/models/accreditation.rb

@@ -1,7 +1,7 @@
 class Accreditation < ActiveRecord::Base
   include FriendlyNamable
 
-  validates_presence_of :name
+  validates :name, presence: true
 
   default_scope { order(:order) }
 end