Merge pull request #1893 from coreinfrastructure/upgrade-rubocop

upgrade rubocop to 1.41.1

.rubocop.yml

@@ -87,8 +87,7 @@ Metrics/ClassLength:
     - 'db/migrate/*'
 # Weird failures on this, disabled for now.
 Layout/LineLength:
-  Enabled: false
-  # Max: 79
+   Max: 132
   # Exclude:
   #   - 'db/migrate/*'
 Metrics/MethodLength:
@@ -233,3 +232,29 @@ Rails/WhereNot:
   Enabled: false
 Rails/NegateInclude:
   Enabled: false
+Layout/RedundantLineBreak:
+  Enabled: false
+Naming/VariableNumber:
+  Enabled: false
+Layout/LineContinuationLeadingSpace:
+  Enabled: false
+Style/TopLevelMethodDefinition:
+  Enabled: false
+Naming/InclusiveLanguage:
+  Enabled: false
+Bundler/GemVersion:
+  Enabled: false
+Lint/EmptyBlock:
+  Enabled: false
+Lint/OrAssignmentToConstant:
+  Enabled: false
+Rails/EnvironmentVariableAccess:
+  Enabled: false
+Rails/DuplicateScope:
+  Enabled: false
+Rails/I18nLocaleTexts:
+  Enabled: false
+Rails/RedundantPresenceValidationOnBelongsTo:
+  Enabled: false
+Style/RequireOrder:
+  Enabled: false

Gemfile

@@ -110,9 +110,9 @@ group :development, :test do
   gem 'pronto-rails_best_practices', '0.11.0'
   gem 'pronto-rubocop', '0.11.3'
   # gem 'railroader', '4.3.8' # Security static analyzer. OSS fork of Brakeman
-  gem 'rubocop', '1.0.0', require: false # Style checker
-  gem 'rubocop-performance', '1.10.2', require: false # Performance cops
-  gem 'rubocop-rails', '2.8.0', require: false # Rails-specific cops
+  gem 'rubocop', '1.41.1', require: false # Style checker
+  gem 'rubocop-performance', '1.15.1', require: false # Performance cops
+  gem 'rubocop-rails', '2.17.3', require: false # Rails-specific cops
   gem 'ruby-graphviz', '1.2.5' # This is used for bundle viz
   gem 'spring', '4.0.0' # Preloads app so console, rake, and tests run faster
   # Do NOT upgrade to vcr 6.*, as that is not OSS:

Gemfile.lock

@@ -363,24 +363,25 @@ GEM
       rack (>= 1.4)
     require_all (3.0.0)
     rexml (3.2.5)
-    rubocop (1.0.0)
+    rubocop (1.41.1)
+      json (~> 2.3)
       parallel (~> 1.10)
-      parser (>= 2.7.1.5)
+      parser (>= 3.1.2.1)
       rainbow (>= 2.2.2, < 4.0)
-      regexp_parser (>= 1.8)
-      rexml
-      rubocop-ast (>= 0.6.0)
+      regexp_parser (>= 1.8, < 3.0)
+      rexml (>= 3.2.5, < 4.0)
+      rubocop-ast (>= 1.23.0, < 2.0)
       ruby-progressbar (~> 1.7)
-      unicode-display_width (>= 1.4.0, < 2.0)
-    rubocop-ast (1.23.0)
+      unicode-display_width (>= 1.4.0, < 3.0)
+    rubocop-ast (1.24.1)
       parser (>= 3.1.1.0)
-    rubocop-performance (1.10.2)
-      rubocop (>= 0.90.0, < 2.0)
+    rubocop-performance (1.15.1)
+      rubocop (>= 1.7.0, < 2.0)
       rubocop-ast (>= 0.4.0)
-    rubocop-rails (2.8.0)
+    rubocop-rails (2.17.3)
       activesupport (>= 4.2.0)
       rack (>= 1.1)
-      rubocop (>= 0.87.0)
+      rubocop (>= 1.33.0, < 2.0)
     ruby-graphviz (1.2.5)
       rexml
     ruby-progressbar (1.11.0)
@@ -532,9 +533,9 @@ DEPENDENCIES
   rails_12factor (= 0.0.3)
   railties (= 6.1.7)
   redcarpet (= 3.5.1)
-  rubocop (= 1.0.0)
-  rubocop-performance (= 1.10.2)
-  rubocop-rails (= 2.8.0)
+  rubocop (= 1.41.1)
+  rubocop-performance (= 1.15.1)
+  rubocop-rails (= 2.17.3)
   ruby-graphviz (= 1.2.5)
   sass-rails (= 5.1.0)
   scout_apm (= 4.1.2)

app/controllers/password_resets_controller.rb

@@ -12,6 +12,8 @@ class PasswordResetsController < ApplicationController
 
   def new; end
 
+  def edit; end
+
   # NOTE: password resets *always* reply with the same message, in all cases.
   # At one time we replied with error reports if there was no account or if
   # there was a GitHub account (not a local account) with the email address.
@@ -40,8 +42,6 @@ def create
     redirect_to root_url
   end
 
-  def edit; end
-
   def update
     new_password = nested_params(:user, :password)
     if new_password.nil? || new_password == ''

app/controllers/project_stats_controller.rb

@@ -288,7 +288,7 @@ def daily_activity
         data: series_dataset
       }
       # Calculate moving average over ndays
-      series_counts = stat_data.map { |e| e[desired_field] }
+      series_counts = stat_data.pluck(desired_field)
       series_moving_average =
         series_counts.each_cons(ndays).map do |e|
           e.sum.to_f / ndays

app/controllers/projects_controller.rb

@@ -215,7 +215,7 @@ def new
   def edit
     return unless @project.notify_for_static_analysis?('0')
 
-    message = t('projects.edit.static_analysis_updated_html')
+    message = t('.static_analysis_updated_html')
     flash.now[:danger] = message
   end
 
@@ -327,7 +327,7 @@ def destroy
       @project.homepage_url ||= project_find_default_url
       format.html do
         redirect_to projects_path
-        flash[:success] = t('projects.delete.done')
+        flash.now[:success] = t('projects.delete.done')
       end
       format.json { head :no_content }
     end
@@ -678,9 +678,9 @@ def retrieve_projects
     # This will NOT match full URLs, but will match partial URLs.
     @projects = @projects.search_for(params[:q]) if params[:q].present?
     if params[:ids].present?
-      @projects = @projects.where(
-        'id in (?)', params[:ids].split(',').map { |x| Integer(x) }
-      )
+      @projects = @projects.where(id: params[:ids].split(',').map do |x|
+                                        Integer(x)
+                                      end)
     end
     @projects
   end

app/controllers/users_controller.rb

@@ -16,10 +16,6 @@ class UsersController < ApplicationController
   before_action :enable_maximum_privacy_headers
   include SessionsHelper
 
-  def new
-    @user = User.new
-  end
-
   def index
     @pagy, @users = pagy(User.all)
     @pagy_locale = I18n.locale.to_s # Pagy requires a string version
@@ -38,7 +34,7 @@ def show
     # we practically never have that many and the interface would be confusing.
     @projects_additional_rights =
       select_needed(Project.includes(:user).joins(:additional_rights)
-        .where('additional_rights.user_id = ?', @user.id))
+        .where(additional_rights: { user_id: @user.id }))
     # *Separately* list edit_projects from projects_additional_rights.
     # Jason Dossett thinks they should be combined, but David A. Wheeler
     # thinks these are important to keep separate because how to *change*
@@ -50,8 +46,21 @@ def show
         Project.includes(:user).where(repo_url: github_user_projects)
       ) - @projects
   end
+
+  def new
+    @user = User.new
+  end
+
   # rubocop: enable Metrics/MethodLength, Metrics/AbcSize
 
+  def edit
+    @user = User.find(params[:id])
+    # Force redirect if current_user cannot edit.  Otherwise, the process
+    # of displaying the edit fields (with their defaults) could cause an
+    # unauthorized exposure of an email address.
+    redirect_to @user unless current_user_can_edit(@user)
+  end
+
   # rubocop: disable Metrics/MethodLength, Metrics/AbcSize
   def create
     if Rails.application.config.deny_login
@@ -79,15 +88,6 @@ def create
     flash[:info] = t('users.new_activation_link_created')
     redirect_to root_path, status: :found
   end
-  # rubocop: enable Metrics/MethodLength, Metrics/AbcSize
-
-  def edit
-    @user = User.find(params[:id])
-    # Force redirect if current_user cannot edit.  Otherwise, the process
-    # of displaying the edit fields (with their defaults) could cause an
-    # unauthorized exposure of an email address.
-    redirect_to @user unless current_user_can_edit(@user)
-  end
 
   # Produce a cleaned-up hash of changes.
   # The key is the field that was changed, the value is [old, new]

app/helpers/sessions_helper.rb

@@ -142,7 +142,7 @@ def current_user_is_github_owner?(url)
   def github_user_projects
     github = Octokit::Client.new access_token: session[:user_token]
     github.auto_paginate = true
-    github.repos.map(&:html_url).reject(&:blank?)
+    github.repos.map(&:html_url).compact_blank
   end
 
   # Logs out the current user.
@@ -190,7 +190,7 @@ def can_current_user_edit_on_github?(url)
   # Returns true iff this is not the REAL final production system,
   # including the master/main and staging systems.
   # It only returns false if we are "truly in production"
-  def in_development?(hostname = ENV['PUBLIC_HOSTNAME'])
+  def in_development?(hostname = ENV.fetch('PUBLIC_HOSTNAME', nil))
     return true if hostname.nil?
 
     hostname != PRODUCTION_HOSTNAME

app/lib/chief.rb

@@ -69,9 +69,7 @@ def merge_changeset(c1, c2)
   def update_value?(project, key, changeset_data)
     if changeset_data.blank?
       false
-    elsif !project.attribute_present?(key) || project[key].blank?
-      true
-    elsif project[key] == '?'
+    elsif !project.attribute_present?(key) || project[key].blank? || project[key] == '?'
       true
     else
       changeset_data[:confidence].present? &&

app/lib/hardened_sites_detective.rb

@@ -29,7 +29,7 @@ class HardenedSitesDetective < Detective
   UNMET_MISSING =
     {
       value: 'Unmet', confidence: 5,
-      explanation: '// One or more of the required security hardening headers '\
+      explanation: '// One or more of the required security hardening headers ' \
                    'is missing.'
     }.freeze
   UNMET_NOSNIFF =

app/mailers/report_mailer.rb

@@ -131,10 +131,12 @@ def report_reminder_summary(projects)
   # We currently only send these out in English, so it's not internationalized
   # (no point in asking the translators to do unnecessary work).
   def report_monthly_announcement(
-    projects, month_display,
-    last_stat_in_prev_month, last_stat_in_prev_prev_month
+    projects,
+    month_display,
+    last_stat_in_prev_month,
+    last_stat_in_prev_prev_month
   )
-    @report_destination = ENV['REPORT_MONTHLY_EMAIL']
+    @report_destination = ENV.fetch('REPORT_MONTHLY_EMAIL', nil)
     return if @report_destination.blank?
 
     @projects = projects

app/models/badge.rb

@@ -21,6 +21,7 @@ class Badge
   # We very rarely change the static images in a way that affects widths,
   # so it's simpler to just copy the information into the source code here.
   # As a style recommendation remove the comma from the last entry.
+  # rubocop:disable Lint/SymbolConversion
   BADGE_WIDTHS = {
     'passing': 184,
     'silver': 172,
@@ -126,6 +127,7 @@ class Badge
     '98': 234,
     '99': 234
   }.freeze
+  # rubocop:enable Lint/SymbolConversion
 
   attr_accessor :svg
 

app/models/translations.rb

@@ -4,12 +4,14 @@
 # OpenSSF Best Practices badge contributors
 # SPDX-License-Identifier: MIT
 
-class Translations
+module Translations
+  module_function
+
   # This class is a used to gather sets of translations which will commonly
   # be called in bulk.  For example, there are translations we would like
   # to export to JavaScript.  We can use a method here to do that quickly.
 
-  def self.for_js
+  def for_js
     js_translations =
       I18n.available_locales.map do |locale|
         locale_t =

config.ru

@@ -2,5 +2,5 @@
 
 # This file is used by Rack-based servers to start the application.
 
-require ::File.expand_path('../config/environment', __FILE__)
+require File.expand_path('../config/environment', __FILE__)
 run Rails.application

config/environments/production.rb

@@ -108,8 +108,8 @@
     address: 'smtp.sendgrid.net',
     port: '587',
     authentication: :plain,
-    user_name: ENV['SENDGRID_USERNAME'],
-    password: ENV['SENDGRID_PASSWORD'],
+    user_name: ENV.fetch('SENDGRID_USERNAME', nil),
+    password: ENV.fetch('SENDGRID_PASSWORD', nil),
     domain: 'heroku.com',
     enable_starttls_auto: true
   }
@@ -123,7 +123,7 @@
   config.active_support.deprecation = :notify
 
   # Use default logging formatter so that PID and timestamp are not suppressed.
-  config.log_formatter = ::Logger::Formatter.new
+  config.log_formatter = Logger::Formatter.new
 
   # Do not dump schema after migrations.
   config.active_record.dump_schema_after_migration = false

config/initializers/octokit.rb

@@ -7,10 +7,10 @@
 Octokit.configure do |c|
   if Rails.env.test?
     # Test app OAuth returns to a different port
-    ENV['GITHUB_KEY'] = ENV['TEST_GITHUB_KEY']
-    ENV['GITHUB_SECRET'] = ENV['TEST_GITHUB_SECRET']
+    ENV['GITHUB_KEY'] = ENV.fetch('TEST_GITHUB_KEY', nil)
+    ENV['GITHUB_SECRET'] = ENV.fetch('TEST_GITHUB_SECRET', nil)
   end
-  c.client_id = ENV['GITHUB_KEY']
-  c.client_secret = ENV['GITHUB_SECRET']
+  c.client_id = ENV.fetch('GITHUB_KEY', nil)
+  c.client_secret = ENV.fetch('GITHUB_SECRET', nil)
   c.auto_paginate = true
 end

config/initializers/omniauth.rb

@@ -7,10 +7,10 @@
 Rails.application.config.middleware.use OmniAuth::Builder do
   if Rails.env.test?
     # Test app OAuth returns to a different port
-    ENV['GITHUB_KEY'] = ENV['TEST_GITHUB_KEY']
-    ENV['GITHUB_SECRET'] = ENV['TEST_GITHUB_SECRET']
+    ENV['GITHUB_KEY'] = ENV.fetch('TEST_GITHUB_KEY', nil)
+    ENV['GITHUB_SECRET'] = ENV.fetch('TEST_GITHUB_SECRET', nil)
   end
-  provider :github, ENV['GITHUB_KEY'], ENV['GITHUB_SECRET'],
+  provider :github, ENV.fetch('GITHUB_KEY', nil), ENV.fetch('GITHUB_SECRET', nil),
            scope: 'user:email, read:org'
   Hashie.logger = Rails.logger
 end

db/schema.rb

@@ -34,8 +34,8 @@
 
   create_table "pg_search_documents", id: :serial, force: :cascade do |t|
     t.text "content"
-    t.integer "searchable_id"
     t.string "searchable_type"
+    t.integer "searchable_id"
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
     t.index ["searchable_type", "searchable_id"], name: "index_pg_search_documents_on_searchable_type_and_searchable_id"

doc/send-translations.rb

@@ -54,7 +54,7 @@
 $SOURCE = {}
 
 # Get API key - warn very early if we can't get it!
-$API_KEY = ENV['API_KEY']
+$API_KEY = ENV.fetch('API_KEY', nil)
 if $API_KEY.nil? || $API_KEY == ''
   STDERR.puts 'Error: Need API_KEY environment variable'
   exit 1
@@ -189,8 +189,7 @@ def process_data(lang, key, data)
   elsif data.is_a?(String)
     if $CURRENT_TRANSLATIONS[lang].key?(key)
       # Potential change to an existing translation
-      this_current_translation = $CURRENT_TRANSLATIONS[lang][key]['target']
-                                 .rstrip
+      this_current_translation = $CURRENT_TRANSLATIONS[lang][key]['target'].rstrip
       new_translation = data.rstrip
       if this_current_translation != new_translation
         # Translation has changed!

gen_markdown.rb

@@ -49,7 +49,7 @@ def puts_criterion(key, criterion)
     print ' (Justification required for "N/A".)'
   end
   print ' (URL required for "met".)' if criterion.key?('met_url_required')
-  print " <sup>[<a href=\"\##{key}\">#{key}</a>]</sup>"
+  print " <sup>[<a href=\"##{key}\">#{key}</a>]</sup>"
   if CriteriaText[key].key?('details') || criterion.key?('rationale')
     print '<dl>' # Put details and rationale in a detail list
     show_details(key)