Add lint step to CI

.github/workflows/build.yml

@@ -8,23 +8,45 @@ on:
 
 jobs:
   test:
+    defaults:
+      run:
+        working-directory: /app
     runs-on: ubuntu-latest
     name: Run specs
     strategy:
       matrix:
         ruby-version: ['2.7']
+    steps:
+      - uses: actions/checkout@v2
+      - name: Set up Ruby
+        uses: ruby/setup-ruby@v1
+        with:
+          ruby-version: ${{ matrix.ruby-version }}
+          bundler-cache: true
+      - name: Run rspec
+        run: |
+          cd spec/dummy
+          bundle
+          bundle exec rails db:test:prepare
+          cd ../..
+          bundle exec rspec
 
+  lint:
+    defaults:
+      run:
+        working-directory: /app
+    runs-on: ubuntu-latest
+    name: Run rubocop
+    strategy:
+      matrix:
+        ruby-version: ['2.7']
     steps:
-    - uses: actions/checkout@v2
-    - name: Set up Ruby
-      uses: ruby/setup-ruby@v1
-      with:
-        ruby-version: ${{ matrix.ruby-version }}
-        bundler-cache: true # runs 'bundle install' and caches installed gems automatically
-    - name: Run rspec
-      run: |
-        cd spec/dummy
-        bundle
-        bundle exec rails db:test:prepare
-        cd ../..
-        bundle exec rspec
+      - uses: actions/checkout@v2
+      - name: Set up Ruby
+        uses: ruby/setup-ruby@v1
+        with:
+          ruby-version: ${{ matrix.ruby-version }}
+          bundler-cache: true
+      - name: Run rubocop
+        run: |
+          bundle exec rubocop

.rubocop.yml

@@ -25,6 +25,9 @@ Layout/FirstHashElementIndentation:
 Layout/MultilineMethodCallIndentation:
   Enabled: false
 
+Style/FrozenStringLiteralComment:
+  Enabled: false
+
 # Offense count: 2
 # This cop supports safe auto-correction (--auto-correct).
 # Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns.
@@ -40,3 +43,5 @@ Style/EachWithObject:
 
 AllCops:
   NewCops: enable
+  Exclude:
+    - 'spec/dummy/db/schema.rb'

.rubocop_todo.yml

@@ -1,3 +1,21 @@
+# This configuration was generated by
+# `rubocop --auto-gen-config`
+# on 2022-05-20 13:06:05 UTC using RuboCop version 1.26.1.
+# 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
+# Configuration parameters: ExpectMatchingDefinition, CheckDefinitionPathHierarchy, CheckDefinitionPathHierarchyRoots, Regex, IgnoreExecutableScripts, AllowedAcronyms.
+# CheckDefinitionPathHierarchyRoots: lib, spec, test, src
+# AllowedAcronyms: CLI, DSL, ACL, API, ASCII, CPU, CSS, DNS, EOF, GUID, HTML, HTTP, HTTPS, ID, IP, JSON, LHS, QPS, RAM, RHS, RPC, SLA, SMTP, SQL, SSH, TCP, TLS, TTL, UDP, UI, UID, UUID, URI, URL, UTF8, VM, XML, XMPP, XSRF, XSS
+Naming/FileName:
+  Exclude:
+    - 'spec/dummy/config/initializers/dfe-analytics.rb'
+
+# Offense count: 12
+# Configuration parameters: AllowedConstants.
 Style/Documentation:
   Exclude:
     - 'spec/**/*'
@@ -10,3 +28,4 @@ Style/Documentation:
     - 'lib/dfe/analytics/send_events.rb'
     - 'lib/dfe/analytics/testing.rb'
     - 'lib/dfe/analytics/testing/helpers.rb'
+    - 'lib/generators/dfe/analytics/install_generator.rb'

Gemfile

@@ -9,4 +9,4 @@ gemspec
 gem 'rake', '~> 12.0'
 
 # Start debugger with binding.b [https://github.com/ruby/debug]
-gem "debug", ">= 1.0.0"
+gem 'debug', '>= 1.0.0'

Gemfile.lock

@@ -244,6 +244,8 @@ GEM
       unicode-display_width (>= 1.4.0, < 3.0)
     rubocop-ast (1.16.0)
       parser (>= 3.1.1.0)
+    rubocop-rspec (2.11.1)
+      rubocop (~> 1.19)
     ruby-progressbar (1.11.0)
     ruby2_keywords (0.0.5)
     signet (0.16.1)
@@ -281,8 +283,9 @@ DEPENDENCIES
   rake (~> 12.0)
   rspec-rails (~> 5.0)
   rubocop (~> 1.26)
+  rubocop-rspec (~> 2)
   sqlite3
   webmock (~> 3.14)
 
 BUNDLED WITH
-   2.3.10
+   2.3.13

dfe-analytics.gemspec

@@ -31,6 +31,8 @@ Gem::Specification.new do |spec|
   spec.add_development_dependency 'json-schema', '~> 2.8'
   spec.add_development_dependency 'rspec-rails', '~> 5.0'
   spec.add_development_dependency 'rubocop', '~> 1.26'
-  spec.add_development_dependency 'webmock', '~> 3.14'
+  spec.add_development_dependency 'rubocop-rspec', '~> 2'
   spec.add_development_dependency 'sqlite3'
+  spec.add_development_dependency 'webmock', '~> 3.14'
+  spec.metadata['rubygems_mfa_required'] = 'true'
 end

lib/dfe/analytics/fields.rb

@@ -1,5 +1,7 @@
 module DfE
   module Analytics
+    # Tools to check and update configuration for model fields sent via
+    # DfE::Analytics
     module Fields
       def self.blocklist
         DfE::Analytics.blocklist
@@ -29,11 +31,11 @@ def self.diff_model_attributes_against(*lists)
             table_name = next_model.table_name&.to_sym
 
             if table_name.present?
-              attributes_considered = lists.map do |list|
-                # for each list of model attrs, look up the attrs for this model
-                list.fetch(table_name, [])
-              end.reduce(:concat) # then combine to get all the attrs we deal with
-
+              attributes_considered = # then combine to get all the attrs we deal with
+                lists.map do |list|
+                  # for each list of model attrs, look up the attrs for this model
+                  list.fetch(table_name, [])
+                end.reduce(:concat)
               missing_attributes = next_model.attribute_names - attributes_considered
               surplus_attributes = attributes_considered - next_model.attribute_names
 

lib/dfe/analytics/middleware/request_identity.rb

@@ -1,6 +1,10 @@
 module DfE
   module Analytics
     module Middleware
+      # Middleware to persist the Rails request UUID in memory where it can be
+      # retrieved by all code running in the context of this request,
+      # irrespective of whether that code has access to the original
+      # ActionDispatch::Request object
       class RequestIdentity
         def initialize(app)
           @app = app

lib/dfe/analytics/railtie.rb

@@ -2,13 +2,14 @@
 
 module DfE
   module Analytics
+    # Railtie
     class Railtie < Rails::Railtie
       config.before_initialize do
         i18n_files = File.expand_path("#{File.dirname(__FILE__)}/../../../config/locales/en.yml")
         I18n.load_path << i18n_files
       end
 
-      initializer "dfe.analytics.insert_middleware" do |app|
+      initializer 'dfe.analytics.insert_middleware' do |app|
         app.config.middleware.use DfE::Analytics::Middleware::RequestIdentity
       end
 

lib/dfe/analytics/send_events.rb

@@ -15,7 +15,7 @@ def self.do(events)
 
       def perform(events)
         if DfE::Analytics.log_only?
-          Rails.logger.info('DfE::Analytics: ' + events.inspect)
+          Rails.logger.info("DfE::Analytics: #{events.inspect}")
         else
           DfE::Analytics.events_client.insert(events, ignore_unknown: true)
         end

lib/generators/dfe/analytics/install_generator.rb

@@ -6,7 +6,7 @@ class InstallGenerator < ::Rails::Generators::Base
       def install
         create_file 'config/initializers/dfe_analytics.rb', <<~FILE
           DfE::Analytics.configure do |config|
-          #{indent(config_options.map(&:strip).join("\n\n").gsub(/# $/, "#").chomp.chomp, 2)}
+          #{indent(config_options.map(&:strip).join("\n\n").gsub(/# $/, '#').chomp.chomp, 2)}
           end
         FILE
 
@@ -20,7 +20,7 @@ def install
       def config_options
         DfE::Analytics.config.members.map do |option|
           <<~DESC
-            # #{I18n.t("dfe.analytics.config.#{option}.description").lines.join("# ").chomp}
+            # #{I18n.t("dfe.analytics.config.#{option}.description").lines.join('# ').chomp}
             #
             # config.#{option} = #{I18n.t("dfe.analytics.config.#{option}.default")}\n
           DESC

spec/dfe/analytics/entities_spec.rb

@@ -1,13 +1,11 @@
 # frozen_string_literal: true
 
-require 'active_record'
-
 RSpec.describe DfE::Analytics::Entities do
   let(:interesting_fields) { [] }
   let(:pii_fields) { [] }
 
   let(:model) do
-    klass = Class.new(Candidate) do
+    Class.new(Candidate) do
       include DfE::Analytics::Entities
 
       # yes, ugh, but another part of the code is going to enumerate

spec/dfe/analytics_spec.rb

@@ -18,12 +18,12 @@
     before do
       allow(DfE::Analytics).to receive(:allowlist).and_return({
         candidates: %i[id],
-        institutions: %i[id], # table name for the School model, which doesn’t follow convention
+        institutions: %i[id] # table name for the School model, which doesn’t follow convention
       })
     end
 
     it 'returns the Rails models in the allowlist' do
-      expect(DfE::Analytics.models_for_analytics).to eq ['Candidate', 'School']
+      expect(DfE::Analytics.models_for_analytics).to eq %w[Candidate School]
     end
   end
 end

spec/dummy/config/application.rb

@@ -3,15 +3,15 @@
 require_relative 'boot'
 
 # Pick the frameworks you want:
-require "active_model/railtie"
-require "active_job/railtie"
-require "active_record/railtie"
+require 'active_model/railtie'
+require 'active_job/railtie'
+require 'active_record/railtie'
 # require "active_storage/engine"
-require "action_controller/railtie"
+require 'action_controller/railtie'
 # require "action_mailer/railtie"
 # require "action_mailbox/engine"
 # require "action_text/engine"
-require "action_view/railtie"
+require 'action_view/railtie'
 # require "action_cable/engine"
 # require "sprockets/railtie"
 

spec/dummy/config/routes.rb

@@ -5,5 +5,5 @@
 
   # Defines the root path route ("/")
   # root "articles#index"
-  get "/manual" => "manual#index"
+  get '/manual' => 'manual#index'
 end

spec/requests/analytics_spec.rb

@@ -43,7 +43,6 @@ def index
   end
 
   it 'works end-to-end' do
-
     request_event = { environment: 'test',
                       event_type: 'web_request',
                       request_method: 'GET',
@@ -78,21 +77,21 @@ def index
     end).to have_been_made
   end
 
-  context "when a queue is specified" do
+  context 'when a queue is specified' do
     it 'uses the specified queue' do
       with_analytics_config(queue: :my_custom_queue) do
-        expect {
+        expect do
           get '/example/path'
-        }.to have_enqueued_job.twice.on_queue(:my_custom_queue)
+        end.to have_enqueued_job.twice.on_queue(:my_custom_queue)
       end
     end
   end
 
-  context "when no queue is specified" do
+  context 'when no queue is specified' do
     it 'uses the default queue' do
-      expect {
+      expect do
         get '/example/path'
-      }.to have_enqueued_job.twice.on_queue(:default)
+      end.to have_enqueued_job.twice.on_queue(:default)
     end
   end
 end