Rubocop govuk (#7199)

.github/workflows/lint.yml

@@ -24,8 +24,8 @@ jobs:
       - name: Run Rubocop
         run: bundle exec rubocop
 
-      - name: Run Slim-Lint
-        run: bundle exec slim-lint app/views app/components
+      - name: Run Slim-Lint with PR annotations
+        run: bundle exec slim-lint -r github app/views app/components
 
       - name: Run Brakeman
         run: bundle exec brakeman

.rubocop.yml

@@ -1,14 +1,51 @@
+inherit_mode:
+  merge:
+    - Exclude
+
+inherit_from: .rubocop_todo.yml
+
+inherit_gem:
+  rubocop-govuk:
+#  Avoid the 'default' config as it disables lots of useful metrics
+#    - config/default.yml
+    - config/layout.yml
+    - config/naming.yml
+    - config/style.yml
+    - config/rake.yml
+    - config/rails.yml
+    - config/rspec.yml
+
 require:
+  - rubocop-capybara
+  - rubocop-factory_bot
   - rubocop-performance
+  - rubocop-rails
+  - rubocop-rspec
+  - rubocop-rspec_rails
 
 AllCops:
   NewCops: enable
+  UseCache: true
   Exclude:
     - "bin/**"
     - "config.ru"
     - "tmp/**/*"
     - "db/schema.rb"
-    - "db/migrate/20*"
+    - "db/migrate/2017*"
+    - "db/migrate/2018*"
+    - "db/migrate/2019*"
+    - "db/migrate/2020*"
+    - "db/migrate/2021*"
+    - "db/migrate/2022*"
+    - "db/migrate/2023*"
+    - "db/migrate/202401*"
+    - "db/migrate/202402*"
+    - "db/migrate/202403*"
+    - "db/migrate/202404*"
+    - "db/migrate/202405*"
+    - "db/migrate/202406*"
+    - "db/migrate/202407*"
+    - "db/migrate/202408*"
     - "vendor/**/*"
     - "node_modules/**/*"
 
@@ -99,25 +136,16 @@ Style/TrailingCommaInArrayLiteral:
 Style/TrailingCommaInHashLiteral:
   EnforcedStyleForMultiline: comma
 
-##
-# Temporary rules
-# TODO: These have a lot of warnings, or relate to unfinished stylistic discussions within the team
-
-Lint/MissingSuper:
-  Enabled: false
-
-Naming/AccessorMethodName:
-  Enabled: false
-
-Naming/BlockForwarding:
-  # We do want to use this, but are blocked by Brakeman's parser not supporting 3.1 syntax
-  Enabled: false
+Layout/AccessModifierIndentation:
+  EnforcedStyle: indent
 
 Style/HashSyntax:
-  # This causes some issues in Slim templates that require a wider style discussion, and is
-  # currently unsupported by Brakeman's parser
-  Enabled: false
+  EnforcedStyle: ruby19
+  EnforcedShorthandSyntax: either
 
+# This is incompatible with our other migration checks
+Rails/BulkChangeTable:
+  Enabled: false
 ##
 # Tech Debt
 # TODO: `rubocop-govuk` previously had all these metrics disabled as they are "just heuristics".
@@ -131,14 +159,9 @@ Metrics/AbcSize:
     - "spec/**/*"
 
 Metrics/BlockLength:
-  Max: 25 # Default 25
   Exclude:
     - "config/**/*"
     - "spec/**/*"
-    - "app/models/concerns/indexable.rb"
-
-Metrics/ClassLength:
-  Max: 200 # Default 100
 
 Metrics/CyclomaticComplexity:
   Max: 18 # Default 7

.slim-lint.yml

@@ -6,3 +6,30 @@ linters:
     enabled: false
   LineLength:
     enabled: false
+  RuboCop:
+    ignored_cops:
+      - Style/IdenticalConditionalBranches
+      - Rails/Blank
+      - Rails/Date
+      - Rails/FindEach
+      - Rails/LinkToBlank
+      - Rails/PluralizationGrammar
+      - Rails/Presence
+      - Rails/Present
+      - Rails/TimeZone
+      - Rails/ToFormattedS
+      - # defaults from slim-lint:
+      - Layout/ArgumentAlignment
+      - Layout/ArrayAlignment
+      - Layout/EmptyLineAfterGuardClause
+      - Layout/FirstHashElementIndentation
+      - Layout/HashAlignment
+      - Layout/IndentationWidth
+      - Layout/TrailingEmptyLines
+      - Lint/Void
+      - Metrics/BlockLength
+      - Metrics/BlockNesting
+      - Naming/FileName
+      - Style/HashSyntax
+      - Style/Next
+      - Style/WordArray

Gemfile

@@ -88,6 +88,10 @@ group :development do
   gem "better_errors"
   gem "binding_of_caller"
   gem "listen"
+  gem "rubocop-factory_bot", require: false
+  gem "rubocop-govuk", require: false
+  gem "rubocop-performance", require: false
+  gem "rubocop-rspec_rails", require: false
   gem "solargraph"
   gem "spring"
   gem "spring-commands-rspec"
@@ -109,9 +113,6 @@ group :development, :test do
   gem "rack-mini-profiler", require: false
   gem "rails-controller-testing"
   gem "rspec-rails"
-  gem "rubocop-performance"
-  gem "rubocop-rails"
-  gem "rubocop-rspec"
   gem "slim_lint", require: false
 end
 

Gemfile.lock

@@ -569,37 +569,45 @@ GEM
       rspec-mocks (~> 3.13)
       rspec-support (~> 3.13)
     rspec-support (3.13.1)
-    rubocop (1.66.1)
+    rubocop (1.64.1)
       json (~> 2.3)
       language_server-protocol (>= 3.17.0)
       parallel (~> 1.10)
       parser (>= 3.3.0.2)
       rainbow (>= 2.2.2, < 4.0)
-      regexp_parser (>= 2.4, < 3.0)
-      rubocop-ast (>= 1.32.2, < 2.0)
+      regexp_parser (>= 1.8, < 3.0)
+      rexml (>= 3.2.5, < 4.0)
+      rubocop-ast (>= 1.31.1, < 2.0)
       ruby-progressbar (~> 1.7)
       unicode-display_width (>= 2.4.0, < 3.0)
-    rubocop-ast (1.32.3)
+    rubocop-ast (1.31.3)
       parser (>= 3.3.1.0)
     rubocop-capybara (2.21.0)
       rubocop (~> 1.41)
     rubocop-factory_bot (2.26.0)
       rubocop (~> 1.41)
+    rubocop-govuk (5.0.2)
+      rubocop (= 1.64.1)
+      rubocop-ast (= 1.31.3)
+      rubocop-capybara (= 2.21.0)
+      rubocop-rails (= 2.25.1)
+      rubocop-rake (= 0.6.0)
+      rubocop-rspec (= 3.0.1)
     rubocop-performance (1.22.1)
       rubocop (>= 1.48.1, < 2.0)
       rubocop-ast (>= 1.31.1, < 2.0)
-    rubocop-rails (2.26.2)
+    rubocop-rails (2.25.1)
       activesupport (>= 4.2.0)
       rack (>= 1.1)
-      rubocop (>= 1.52.0, < 2.0)
+      rubocop (>= 1.33.0, < 2.0)
       rubocop-ast (>= 1.31.1, < 2.0)
-    rubocop-rspec (2.31.0)
-      rubocop (~> 1.40)
-      rubocop-capybara (~> 2.17)
-      rubocop-factory_bot (~> 2.22)
-      rubocop-rspec_rails (~> 2.28)
-    rubocop-rspec_rails (2.29.0)
-      rubocop (~> 1.40)
+    rubocop-rake (0.6.0)
+      rubocop (~> 1.0)
+    rubocop-rspec (3.0.1)
+      rubocop (~> 1.61)
+    rubocop-rspec_rails (2.30.0)
+      rubocop (~> 1.61)
+      rubocop-rspec (~> 3, >= 3.0.1)
     ruby-progressbar (1.13.0)
     ruby-rc4 (0.1.5)
     rubyzip (2.3.2)
@@ -829,9 +837,10 @@ DEPENDENCIES
   redis
   rgeo-geojson
   rspec-rails
+  rubocop-factory_bot
+  rubocop-govuk
   rubocop-performance
-  rubocop-rails
-  rubocop-rspec
+  rubocop-rspec_rails
   sanitize
   selenium-webdriver
   sentry-rails

Rakefile

@@ -5,19 +5,19 @@
 require_relative "config/application"
 Rails.application.load_tasks
 
-if Rails.env.development? || Rails.env.test?
+if Rails.env.local?
   desc "Run Rubocop"
-  task :rubocop do
+  task rubocop: :environment do
     sh "bundle exec rubocop"
   end
 
   desc "Run Slim Lint"
-  task :slim_lint do
+  task slim_lint: :environment do
     sh "bundle exec slim-lint app/views app/components"
   end
 
   desc "Run Brakeman"
-  task :brakeman do
+  task brakeman: :environment do
     # CI env var makes Brakeman not put its output in a pager
     sh "CI=true bundle exec brakeman"
   end

app/controllers/jobseekers/job_applications_controller.rb

@@ -24,13 +24,11 @@ def review
     session[:back_to_review] = (session[:back_to_review] || []).push(job_application.id).uniq
   end
 
-  # rubocop:disable Style/GuardClause
   def about_your_application
     if profile.nil? || profile&.personal_details&.right_to_work_in_uk? || vacancy.visa_sponsorship_available?
       redirect_to new_quick_apply_jobseekers_job_job_application_path(vacancy.id)
     end
   end
-  # rubocop:enable Style/GuardClause
 
   def new_quick_apply
     @has_previous_application = previous_application?

app/helpers/pdf_helper.rb

@@ -1,10 +1,11 @@
-# rubocop:disable Metrics/AbcSize
 module PdfHelper
   include JobApplicationsHelper
   include QualificationsHelper
 
-  # Arial Unicode is a font that supports all characters.
+  # false rubocop assertion
+  # rubocop:disable Rails/SaveBang
   def update_font_family(pdf)
+    # Arial Unicode is a font that supports all characters.
     pdf.font_families.update(
       "Arial Unicode" =>
         {
@@ -15,6 +16,7 @@ def update_font_family(pdf)
     )
     pdf.font("Arial Unicode", encoding: "UTF-8")
   end
+  # rubocop:enable Rails/SaveBang
 
   def add_section_title(pdf, title)
     pdf.move_down 20
@@ -41,7 +43,7 @@ def add_headers(pdf)
     pdf.move_down 20
   end
 
-  def add_personal_details(pdf)
+  def add_personal_details(pdf) # rubocop:disable Metrics/AbcSize
     personal_details = [
       [I18n.t("helpers.label.jobseekers_job_application_personal_details_form.first_name"), job_application.first_name],
       [I18n.t("helpers.label.jobseekers_job_application_personal_details_form.last_name"), job_application.last_name],
@@ -330,4 +332,3 @@ def pdf_job_application_qualified_teacher_status_info(job_application)
     end
   end
 end
-# rubocop:enable Metrics/AbcSize

app/models/concerns/profile_section.rb

@@ -1,7 +1,7 @@
 module ProfileSection
   extend ActiveSupport::Concern
 
-  class_methods do # rubocop:disable Metrics/BlockLength
+  class_methods do
     def prepare(**, &block)
       find_or_initialize_by(**).tap do |record|
         if record.new_record?

app/models/vacancy.rb

@@ -101,7 +101,7 @@ class Vacancy < ApplicationRecord
   validates :slug, presence: true
   validate :enable_job_applications_cannot_be_changed_once_listed
   validates_with ExternalVacancyValidator, if: :external?
-  validates :organisations, :presence => true
+  validates :organisations, presence: true
 
   validates :application_email, email_address: true
   validates :contact_email, email_address: true

config/routes.rb

@@ -341,7 +341,7 @@
     resources :jobs, only: %i[create destroy delete show], controller: "publishers/vacancies" do
       resources :build, only: %i[show update], controller: "publishers/vacancies/build"
       resources :documents, only: %i[index new create destroy], controller: "publishers/vacancies/documents" do
-        post :confirm, :on => :collection
+        post :confirm, on: :collection
       end
       resource :application_forms, only: %i[create destroy], controller: "publishers/vacancies/application_forms"
 

spec/controllers/jobseekers/sessions_controller_spec.rb

@@ -1,5 +1,6 @@
 require "rails_helper"
 
+# rubocop:disable RSpec/InstanceVariable
 RSpec.describe Jobseekers::SessionsController do
   context "unrelated session contents" do
     let(:unrelated_session_contents) { { keep_me: { foo: "bar" } }.stringify_keys }
@@ -37,3 +38,4 @@
     end
   end
 end
+# rubocop:enable RSpec/InstanceVariable

spec/factories/locations.rb

@@ -1,5 +1,5 @@
 FactoryBot.define do
-  factory :job_preferences_location, class: JobPreferences::Location do
+  factory :job_preferences_location, class: "JobPreferences::Location" do
     name { Faker::Address.city }
     radius { [0, 1, 5, 10, 15, 20, 25, 50, 100, 200].sample }
 

spec/helpers/qualifications_helper_spec.rb

@@ -3,7 +3,7 @@
 RSpec.describe QualificationsHelper do
   describe "#qualifications_sort_and_group" do
     it "groups qualifications of same type and sorts into desired order" do
-      expect(helper.qualifications_sort_and_group([{ category: "gcse" }, { category: "a_level" }, { category: "undergraduate" }, { category: "gcse" }])).to eq({ "a_level" => [{ :category => "a_level" }], "gcse" => [{ :category => "gcse" }, { :category => "gcse" }], "undergraduate" => [{ :category => "undergraduate" }] })
+      expect(helper.qualifications_sort_and_group([{ category: "gcse" }, { category: "a_level" }, { category: "undergraduate" }, { category: "gcse" }])).to eq({ "a_level" => [{ category: "a_level" }], "gcse" => [{ category: "gcse" }, { category: "gcse" }], "undergraduate" => [{ category: "undergraduate" }] })
     end
   end
 end