diff --git a/.rubocop.yml b/.rubocop.yml index 2a7de6ec9..4be43860a 100644 --- a/.rubocop.yml +++ b/.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 diff --git a/Gemfile b/Gemfile index e6bb86546..37dd73cb9 100644 --- a/Gemfile +++ b/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: diff --git a/Gemfile.lock b/Gemfile.lock index 99955923d..c58ace564 100644 --- a/Gemfile.lock +++ b/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) diff --git a/app/controllers/password_resets_controller.rb b/app/controllers/password_resets_controller.rb index 58415004d..fcab10e96 100644 --- a/app/controllers/password_resets_controller.rb +++ b/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 == '' diff --git a/app/controllers/project_stats_controller.rb b/app/controllers/project_stats_controller.rb index c289a8e40..27cafea0b 100644 --- a/app/controllers/project_stats_controller.rb +++ b/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 diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 9fd3e8d21..1b8022528 100644 --- a/app/controllers/projects_controller.rb +++ b/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 diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 71f4f4121..24a6ab348 100644 --- a/app/controllers/users_controller.rb +++ b/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] diff --git a/app/helpers/sessions_helper.rb b/app/helpers/sessions_helper.rb index aea914f13..e18cd21ae 100644 --- a/app/helpers/sessions_helper.rb +++ b/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 diff --git a/app/lib/chief.rb b/app/lib/chief.rb index ecea5f0f5..d30bee49e 100644 --- a/app/lib/chief.rb +++ b/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? && diff --git a/app/lib/hardened_sites_detective.rb b/app/lib/hardened_sites_detective.rb index f6b7ce0d2..d12e7d7f9 100644 --- a/app/lib/hardened_sites_detective.rb +++ b/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 = diff --git a/app/mailers/report_mailer.rb b/app/mailers/report_mailer.rb index 70fbd4964..bb4605ec3 100644 --- a/app/mailers/report_mailer.rb +++ b/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 diff --git a/app/models/badge.rb b/app/models/badge.rb index 58222a1a4..850beacdf 100644 --- a/app/models/badge.rb +++ b/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 diff --git a/app/models/translations.rb b/app/models/translations.rb index 759ef1c01..8ebf8ead7 100644 --- a/app/models/translations.rb +++ b/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 = diff --git a/config.ru b/config.ru index 61c04e13f..9db84a817 100644 --- a/config.ru +++ b/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 diff --git a/config/environments/production.rb b/config/environments/production.rb index 693dcdfd7..fc36decde 100644 --- a/config/environments/production.rb +++ b/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 diff --git a/config/initializers/octokit.rb b/config/initializers/octokit.rb index 6e516090f..2900c7301 100644 --- a/config/initializers/octokit.rb +++ b/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 diff --git a/config/initializers/omniauth.rb b/config/initializers/omniauth.rb index ae11df7c7..fe8e89ad6 100644 --- a/config/initializers/omniauth.rb +++ b/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 diff --git a/db/schema.rb b/db/schema.rb index 9d019c420..15544be64 100644 --- a/db/schema.rb +++ b/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" diff --git a/doc/send-translations.rb b/doc/send-translations.rb index a412ebeda..1633cc220 100755 --- a/doc/send-translations.rb +++ b/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! diff --git a/gen_markdown.rb b/gen_markdown.rb index d3cd6ff3f..e488263cb 100755 --- a/gen_markdown.rb +++ b/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 " [#{key}]" + print " [#{key}]" if CriteriaText[key].key?('details') || criterion.key?('rationale') print '