installed rubocop-performance gem and made necessary adjustments

.rubocop.yml

@@ -23,6 +23,11 @@
 #
 # Try to place any new Cops under their relevant section and in alphabetical order
 
+require:
+#   - rubocop-rails
+#   - rubocop-rspec
+  - rubocop-performance
+
 AllCops:
   # Show the name of the cops being voilated in the feedback
   DisplayCopNames: true

CHANGELOG.md

@@ -1,5 +1,10 @@
 # Changelog
 
+## v4.0.3
+
+### Maintenance
+- Installed rubocop-performance gem and made suggested changes
+
 ### Added
 
 - Added CHANGELOG.md and Danger Github Action [#3257](https://github.com/DMPRoadmap/roadmap/issues/3257)

Gemfile

@@ -336,6 +336,9 @@ group :ci, :development do
   # Thread-safety checks via static analysis. A plugin for the RuboCop code style
   # enforcing & linting tool.
   # gem 'rubocop-thread_safety'
+
+  # Performance checks by Rubocop
+  gem 'rubocop-performance', require: false
 end
 
 group :development do

Gemfile.lock

@@ -437,6 +437,9 @@ GEM
       parser (>= 3.1.1.0)
     rubocop-i18n (3.0.0)
       rubocop (~> 1.0)
+    rubocop-performance (1.15.2)
+      rubocop (>= 1.7.0, < 2.0)
+      rubocop-ast (>= 0.4.0)
     ruby-progressbar (1.11.0)
     ruby2_keywords (0.0.5)
     ruby_dig (0.0.2)
@@ -540,6 +543,7 @@ GEM
     zeitwerk (2.6.6)
 
 PLATFORMS
+  arm64-darwin-21
   arm64-darwin-22
   x86_64-linux
 
@@ -602,6 +606,7 @@ DEPENDENCIES
   rspec-rails
   rubocop
   rubocop-i18n
+  rubocop-performance
   sass-rails
   sassc-rails
   selenium-webdriver
@@ -625,4 +630,4 @@ RUBY VERSION
    ruby 2.7.6p219
 
 BUNDLED WITH
-   2.3.26
+   2.4.2

app/controllers/answers_controller.rb

@@ -185,8 +185,8 @@ def permitted_params
   # rubocop:enable Metrics/AbcSize
 
   def check_answered(section, q_array, all_answers)
-    n_qs = section.questions.select { |question| q_array.include?(question.id) }.length
-    n_ans = all_answers.select { |ans| q_array.include?(ans.question.id) and ans.answered? }.length
+    n_qs = section.questions.count { |question| q_array.include?(question.id) }
+    n_ans = all_answers.count { |ans| q_array.include?(ans.question.id) and ans.answered? }
     [n_qs, n_ans]
   end
 end

app/controllers/api/v0/plans_controller.rb

@@ -88,7 +88,7 @@ def index
         if params['updated_after'].present? || params['updated_before'].present?
           @plans = @plans.where(updated_at: dates_to_range(params, 'updated_after', 'updated_before'))
         end
-        if params['remove_tests'].present? && params['remove_tests'].downcase == 'true'
+        if params['remove_tests'].present? && params['remove_tests'].casecmp('true').zero?
           @plans = @plans.where.not(visibility: Plan.visibilities[:is_test])
         end
         # filter on funder (dmptemplate_id)

app/controllers/api/v0/statistics_controller.rb

@@ -219,7 +219,7 @@ def plans
         raise Pundit::NotAuthorizedError unless Api::V0::StatisticsPolicy.new(@user, :statistics).plans?
 
         @org_plans = @user.org.plans
-        if params['remove_tests'].present? && params['remove_tests'].downcase == 'true'
+        if params['remove_tests'].present? && params['remove_tests'].casecmp('true').zero?
           @org_plans = @org_plans.where.not(visibility: Plan.visibilities[:is_test])
         end
         if params['start_date'].present? || params['end_date'].present?

app/controllers/api/v1/plans_controller.rb

@@ -108,7 +108,7 @@ def plan_exists?(json:)
 
       # Get the Plan's owner
       def determine_owner(client:, plan:)
-        contact = plan.contributors.select(&:data_curation?).first
+        contact = plan.contributors.find(&:data_curation?)
         # Use the contact if it was sent in and has an affiliation defined
         return contact if contact.present? && contact.org.present?
 

app/controllers/concerns/conditional_user_mailer.rb

@@ -12,14 +12,14 @@ module ConditionalUserMailer
   #
   # Returns Boolean
   def deliver_if(key:, recipients: [], &block)
-    return false unless block_given?
+    return false unless block
 
     Array(recipients).each do |recipient|
       email_hash = recipient.get_preferences('email').with_indifferent_access
       # Violation of rubocop's DoubleNegation check
       # preference_value = !!email_hash.dig(*key.to_s.split("."))
       preference_value = email_hash.dig(*key.to_s.split('.'))
-      block.call(recipient) if preference_value
+      yield(recipient) if preference_value
     end
 
     true

app/controllers/concerns/paginable.rb

@@ -8,7 +8,7 @@ module Paginable
 
   ##
   # Regex to validate sort_field param is safe
-  SORT_COLUMN_FORMAT = /[\w_]+\.[\w_]+$/.freeze
+  SORT_COLUMN_FORMAT = /[\w_]+\.[\w_]+$/
 
   PAGINATION_QUERY_PARAMS = %i[page sort_field sort_direction
                                search controller action].freeze

app/controllers/org_admin/plans_controller.rb

@@ -50,7 +50,7 @@ def download_plans
       raise Pundit::NotAuthorizedError unless current_user.present? && current_user.can_org_admin?
 
       org = current_user.org
-      file_name = org.name.gsub(/ /, '_')
+      file_name = org.name.tr(' ', '_')
                      .gsub(/[.;,]/, '')
       header_cols = [
         _('Project title').to_s,

app/controllers/org_admin/templates_controller.rb

@@ -15,7 +15,7 @@ class TemplatesController < ApplicationController
     def index
       authorize Template
       templates = Template.latest_version.where(customization_of: nil)
-      published = templates.select { |t| t.published? || t.draft? }.length
+      published = templates.count { |t| t.published? || t.draft? }
 
       @orgs              = Org.includes(:identifiers).managed
       @title             = _('All Templates')
@@ -40,7 +40,7 @@ def organisational
       authorize Template
       templates = Template.latest_version_per_org(current_user.org.id)
                           .where(customization_of: nil, org_id: current_user.org.id)
-      published = templates.select { |t| t.published? || t.draft? }.length
+      published = templates.count { |t| t.published? || t.draft? }
 
       @orgs  = current_user.can_super_admin? ? Org.includes(:identifiers).all : nil
       @title = if current_user.can_super_admin?
@@ -78,7 +78,7 @@ def customisable
       customizations = customizations.select do |t|
         funder_template_families.include?(t.customization_of)
       end
-      published = customizations.select { |t| t.published? || t.draft? }.length
+      published = customizations.count { |t| t.published? || t.draft? }
 
       @orgs = current_user.can_super_admin? ? Org.includes(:identifiers).all : []
       @title = _('Customizable Templates')
@@ -328,7 +328,7 @@ def template_export
       @formatting = Settings::Template::DEFAULT_SETTINGS[:formatting]
 
       begin
-        safe_title = @template.title.gsub(/[^a-zA-Z\d\s]/, '').gsub(/ /, '_')
+        safe_title = @template.title.gsub(/[^a-zA-Z\d\s]/, '').tr(' ', '_')
         file_name = "#{safe_title}_v#{@template.version}"
         respond_to do |format|
           format.docx do

app/controllers/plan_exports_controller.rb

@@ -116,7 +116,7 @@ def file_name
     # Sanitize bad characters and replace spaces with underscores
     ret = @plan.title
     ret = ret.strip.gsub(/\s+/, '_')
-    ret = ret.gsub(/"/, '')
+    ret = ret.delete('"')
     ret = ActiveStorage::Filename.new(ret).sanitized
     # limit the filename length to 100 chars. Windows systems have a MAX_PATH allowance
     # of 255 characters, so this should provide enough of the title to allow the user

app/controllers/plans_controller.rb

@@ -237,7 +237,7 @@ def edit
                .find(params[:id])
     authorize plan
     phase_id = params[:phase_id].to_i
-    phase = plan.template.phases.select { |p| p.id == phase_id }.first
+    phase = plan.template.phases.find { |p| p.id == phase_id }
     raise ActiveRecord::RecordNotFound if phase.nil?
 
     guidance_groups = GuidanceGroup.where(published: true, id: plan.guidance_group_ids)

app/controllers/public_pages_controller.rb

@@ -52,7 +52,7 @@ def template_export
     @formatting = Settings::Template::DEFAULT_SETTINGS[:formatting]
 
     begin
-      file_name = @template.title.gsub(/[^a-zA-Z\d\s]/, '').gsub(/ /, '_')
+      file_name = @template.title.gsub(/[^a-zA-Z\d\s]/, '').tr(' ', '_')
       file_name = "#{file_name}_v#{@template.version}"
       respond_to do |format|
         format.docx do

app/helpers/conditions_helper.rb

@@ -89,11 +89,9 @@ def num_section_questions(plan, section, phase = nil)
 
       phase = plan.template
                   .phases
-                  .select { |ph| ph.number == phase[:number] }
-                  .first
+                  .find { |ph| ph.number == phase[:number] }
       section = phase.sections
-                     .select { |s| s.phase_id == phase.id && s.title == section[:title] }
-                     .first
+                     .find { |s| s.phase_id == phase.id && s.title == section[:title] }
     end
     count = 0
     plan_remove_list = remove_list(plan)

app/models/concerns/exportable_plan.rb

@@ -220,7 +220,7 @@ def prepare_research_outputs_for_csv(csv, _headings, hash)
 
     csv << [_('Research Outputs: ')]
     # Convert the hash keys to column headers
-    csv << hash[:research_outputs].first.keys.map { |key| key.to_s.capitalize.gsub('_', ' ') }
+    csv << hash[:research_outputs].first.keys.map { |key| key.to_s.capitalize.tr('_', ' ') }
     hash[:research_outputs].each do |research_output|
       csv << research_output.values
     end

app/models/concerns/identifiable.rb

@@ -54,7 +54,7 @@ def self.from_identifiers(array:)
     # gets the identifier for the scheme
     def identifier_for_scheme(scheme:)
       scheme = IdentifierScheme.by_name(scheme.downcase).first if scheme.is_a?(String)
-      identifiers.select { |id| id.identifier_scheme == scheme }.last
+      identifiers.reverse.find { |id| id.identifier_scheme == scheme }
     end
 
     # Combines the existing identifiers with the new ones

app/models/exported_plan.rb

@@ -67,7 +67,7 @@ def project_description
   end
 
   def owner
-    plan.roles.to_a.select(&:creator?).first.user
+    plan.roles.to_a.find(&:creator?)&.user
   end
 
   def funder

app/models/identifier.rb

@@ -96,7 +96,7 @@ def identifier_format
     return 'ark' if value.include?('ark:')
 
     doi_regex = %r{(doi:)?[0-9]{2}\.[0-9]+/.}
-    return 'doi' if value =~ doi_regex
+    return 'doi' if value.match?(doi_regex)
 
     return 'url' if value.starts_with?('http')
 

app/models/language.rb

@@ -19,7 +19,7 @@ class Language < ApplicationRecord
 
   ABBREVIATION_MAXIMUM_LENGTH = 5
 
-  ABBREVIATION_FORMAT = /\A[a-z]{2}(-[A-Z]{2})?\Z/.freeze
+  ABBREVIATION_FORMAT = /\A[a-z]{2}(-[A-Z]{2})?\Z/
 
   NAME_MAXIMUM_LENGTH = 20
 

app/models/plan.rb

@@ -54,6 +54,8 @@ class Plan < ApplicationRecord
   # = Constants =
   # =============
 
+  DMP_ID_TYPES = %w[ark doi].freeze
+
   # Returns visibility message given a Symbol type visibility passed, otherwise
   # nil
   VISIBILITY_MESSAGE = {
@@ -290,7 +292,7 @@ def settings(key)
   # rubocop:disable Metrics/AbcSize, Style/OptionalBooleanParameter
   def answer(qid, create_if_missing = true)
     answer = answers.select { |a| a.question_id == qid }
-                    .max { |a, b| a.created_at <=> b.created_at }
+                    .max_by(&:created_at)
     if answer.nil? && create_if_missing
       question = Question.find(qid)
       answer = Answer.new
@@ -440,7 +442,7 @@ def latest_update
   # Returns nil
   def owner
     r = roles.select { |rr| rr.active && rr.administrator }
-             .min { |a, b| a.created_at <=> b.created_at }
+             .min_by(&:created_at)
     r.nil? ? nil : r.user
   end
 
@@ -510,7 +512,7 @@ def authors
   #
   # Returns Integer
   def num_answered_questions(phase = nil)
-    return answers.select(&:answered?).length unless phase.present?
+    return answers.count(&:answered?) unless phase.present?
 
     answered = answers.select do |answer|
       answer.answered? && phase.questions.include?(answer.question)
@@ -577,7 +579,7 @@ def deactivate!
 
   # Returns the plan's identifier (either a DOI/ARK)
   def landing_page
-    identifiers.select { |i| %w[doi ark].include?(i.identifier_format) }.first
+    identifiers.find { |i| DMP_ID_TYPES.include?(i.identifier_format) }
   end
 
   # Since the Grant is not a normal AR association, override the getter and setter

app/models/question.rb

@@ -167,7 +167,7 @@ def example_answers(org_ids)
     org_ids = Array(org_ids)
     annotations.select { |a| org_ids.include?(a.org_id) }
                .select(&:example_answer?)
-               .sort { |a, b| a.created_at <=> b.created_at }
+               .sort_by(&:created_at)
   end
 
   alias get_example_answers example_answers
@@ -183,8 +183,7 @@ def example_answers(org_ids)
   # Returns Annotation
   def guidance_annotation(org_id)
     annotations.select { |a| a.org_id == org_id }
-               .select(&:guidance?)
-               .first
+               .find(&:guidance?)
   end
 
   alias get_guidance_annotation guidance_annotation

app/models/settings/template.rb

@@ -21,8 +21,8 @@ class Template < RailsSettings::SettingObject
       'Roboto, Arial, Sans-Serif'
     ].freeze
 
-    VALID_FONT_SIZE_RANGE = (8..14).freeze
-    VALID_MARGIN_RANGE = (5..25).freeze
+    VALID_FONT_SIZE_RANGE = (8..14)
+    VALID_MARGIN_RANGE = (5..25)
 
     VALID_ADMIN_FIELDS = %w[project_name project_identifier grant_title
                             principal_investigator project_data_contact

app/presenters/api/v1/contributor_presenter.rb

@@ -8,13 +8,13 @@ class << self
         # Convert the specified role into a CRediT Taxonomy URL
         def role_as_uri(role:)
           return nil unless role.present?
-          return 'other' if role.to_s.downcase == 'other'
+          return 'other' if role.to_s.casecmp('other').zero?
 
-          "#{Contributor::ONTOLOGY_BASE_URL}/#{role.to_s.downcase.gsub('_', '-')}"
+          "#{Contributor::ONTOLOGY_BASE_URL}/#{role.to_s.downcase.tr('_', '-')}"
         end
 
         def contributor_id(identifiers:)
-          identifiers.select { |id| id.identifier_scheme.name == 'orcid' }.first
+          identifiers.find { |id| id.identifier_scheme.name == 'orcid' }
         end
       end
     end

app/presenters/api/v1/org_presenter.rb

@@ -6,10 +6,10 @@ module V1
     class OrgPresenter
       class << self
         def affiliation_id(identifiers:)
-          ident = identifiers.select { |id| id.identifier_scheme&.name == 'ror' }.first
+          ident = identifiers.find { |id| id.identifier_scheme&.name == 'ror' }
           return ident if ident.present?
 
-          identifiers.select { |id| id.identifier_scheme&.name == 'fundref' }.first
+          identifiers.find { |id| id.identifier_scheme&.name == 'fundref' }
         end
       end
     end

app/presenters/api/v1/plan_presenter.rb

@@ -27,7 +27,7 @@ def initialize(plan:)
       # Extract the ARK or DOI for the DMP OR use its URL if none exists
       def identifier
         doi = @plan.identifiers.select do |id|
-          %w[ark doi].include?(id.identifier_format)
+          ::Plan::DMP_ID_TYPES.include?(id.identifier_format)
         end
         return doi.first if doi.first.present?
 

app/presenters/api/v1/research_output_presenter.rb

@@ -62,7 +62,7 @@ def fetch_q_and_a(themes:)
         ret = themes.map do |theme|
           qs = @plan.questions.select { |q| q.themes.collect(&:title).include?(theme) }
           descr = qs.map do |q|
-            a = @plan.answers.select { |ans| ans.question_id = q.id }.first
+            a = @plan.answers.find { |ans| ans.question_id = q.id }
             next unless a.present? && !a.blank?
 
             "<strong>Question:</strong> #{q.text}<br><strong>Answer:</strong> #{a.text}"