Merge pull request #3288 from DMPRoadmap/rubocop-performance

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

@@ -51,6 +51,12 @@ For more detailed explanation, please refer to this video : https://www.youtube.
 - Cleaned up `spec/rails_helper.rb` and `spec/spec_helper.rb`
 - Simplified the `spec/support/capybara.rb` helper to work with the latest version of Capybara and use its built in headless Chrome driver
 
+### Added Rubocop performance gem
+- Installed rubocop-performance gem and made suggested changes
+
+### Bug Fixes
+
+
 ## v4.0.2
 
 ### Added

Gemfile

@@ -278,6 +278,9 @@ group :ci, :development do
   # RuboCop rules for detecting and autocorrecting undecorated strings for i18n
   # (gettext and rails-i18n)
   gem 'rubocop-i18n'
+
+  # Performance checks by Rubocop
+  gem 'rubocop-performance', require: false
 end
 
 group :development do

Gemfile.lock

@@ -1,63 +1,63 @@
 GEM
   remote: https://rubygems.org/
   specs:
-    actioncable (6.1.7.2)
-      actionpack (= 6.1.7.2)
-      activesupport (= 6.1.7.2)
+    actioncable (6.1.7.3)
+      actionpack (= 6.1.7.3)
+      activesupport (= 6.1.7.3)
       nio4r (~> 2.0)
       websocket-driver (>= 0.6.1)
-    actionmailbox (6.1.7.2)
-      actionpack (= 6.1.7.2)
-      activejob (= 6.1.7.2)
-      activerecord (= 6.1.7.2)
-      activestorage (= 6.1.7.2)
-      activesupport (= 6.1.7.2)
+    actionmailbox (6.1.7.3)
+      actionpack (= 6.1.7.3)
+      activejob (= 6.1.7.3)
+      activerecord (= 6.1.7.3)
+      activestorage (= 6.1.7.3)
+      activesupport (= 6.1.7.3)
       mail (>= 2.7.1)
-    actionmailer (6.1.7.2)
-      actionpack (= 6.1.7.2)
-      actionview (= 6.1.7.2)
-      activejob (= 6.1.7.2)
-      activesupport (= 6.1.7.2)
+    actionmailer (6.1.7.3)
+      actionpack (= 6.1.7.3)
+      actionview (= 6.1.7.3)
+      activejob (= 6.1.7.3)
+      activesupport (= 6.1.7.3)
       mail (~> 2.5, >= 2.5.4)
       rails-dom-testing (~> 2.0)
-    actionpack (6.1.7.2)
-      actionview (= 6.1.7.2)
-      activesupport (= 6.1.7.2)
+    actionpack (6.1.7.3)
+      actionview (= 6.1.7.3)
+      activesupport (= 6.1.7.3)
       rack (~> 2.0, >= 2.0.9)
       rack-test (>= 0.6.3)
       rails-dom-testing (~> 2.0)
       rails-html-sanitizer (~> 1.0, >= 1.2.0)
-    actiontext (6.1.7.2)
-      actionpack (= 6.1.7.2)
-      activerecord (= 6.1.7.2)
-      activestorage (= 6.1.7.2)
-      activesupport (= 6.1.7.2)
+    actiontext (6.1.7.3)
+      actionpack (= 6.1.7.3)
+      activerecord (= 6.1.7.3)
+      activestorage (= 6.1.7.3)
+      activesupport (= 6.1.7.3)
       nokogiri (>= 1.8.5)
-    actionview (6.1.7.2)
-      activesupport (= 6.1.7.2)
+    actionview (6.1.7.3)
+      activesupport (= 6.1.7.3)
       builder (~> 3.1)
       erubi (~> 1.4)
       rails-dom-testing (~> 2.0)
       rails-html-sanitizer (~> 1.1, >= 1.2.0)
-    activejob (6.1.7.2)
-      activesupport (= 6.1.7.2)
+    activejob (6.1.7.3)
+      activesupport (= 6.1.7.3)
       globalid (>= 0.3.6)
-    activemodel (6.1.7.2)
-      activesupport (= 6.1.7.2)
-    activerecord (6.1.7.2)
-      activemodel (= 6.1.7.2)
-      activesupport (= 6.1.7.2)
+    activemodel (6.1.7.3)
+      activesupport (= 6.1.7.3)
+    activerecord (6.1.7.3)
+      activemodel (= 6.1.7.3)
+      activesupport (= 6.1.7.3)
     activerecord_json_validator (2.1.3)
       activerecord (>= 4.2.0, < 8)
       json_schemer (~> 0.2.18)
-    activestorage (6.1.7.2)
-      actionpack (= 6.1.7.2)
-      activejob (= 6.1.7.2)
-      activerecord (= 6.1.7.2)
-      activesupport (= 6.1.7.2)
+    activestorage (6.1.7.3)
+      actionpack (= 6.1.7.3)
+      activejob (= 6.1.7.3)
+      activerecord (= 6.1.7.3)
+      activesupport (= 6.1.7.3)
       marcel (~> 1.0)
       mini_mime (>= 1.1.0)
-    activesupport (6.1.7.2)
+    activesupport (6.1.7.3)
       concurrent-ruby (~> 1.0, >= 1.0.2)
       i18n (>= 1.6, < 2)
       minitest (>= 5.1)
@@ -134,9 +134,9 @@ GEM
       no_proxy_fix
       octokit (~> 5.0)
       terminal-table (>= 1, < 4)
-    database_cleaner (2.0.1)
-      database_cleaner-active_record (~> 2.0.0)
-    database_cleaner-active_record (2.0.1)
+    database_cleaner (2.0.2)
+      database_cleaner-active_record (>= 2, < 3)
+    database_cleaner-active_record (2.1.0)
       activerecord (>= 5.a)
       database_cleaner-core (~> 2.0.0)
     database_cleaner-core (2.0.1)
@@ -207,7 +207,7 @@ GEM
       locale (>= 2.0.5)
       prime
       text (>= 1.3.0)
-    git (1.17.2)
+    git (1.18.0)
       addressable (~> 2.8)
       rchardet (~> 1.8)
     globalid (1.1.0)
@@ -351,27 +351,27 @@ GEM
     pundit-matchers (1.8.4)
       rspec-rails (>= 3.0.0)
     racc (1.6.2)
-    rack (2.2.6.3)
+    rack (2.2.6.4)
     rack-mini-profiler (3.0.0)
       rack (>= 1.2.0)
     rack-protection (3.0.5)
       rack
-    rack-test (2.0.2)
+    rack-test (2.1.0)
       rack (>= 1.3)
-    rails (6.1.7.2)
-      actioncable (= 6.1.7.2)
-      actionmailbox (= 6.1.7.2)
-      actionmailer (= 6.1.7.2)
-      actionpack (= 6.1.7.2)
-      actiontext (= 6.1.7.2)
-      actionview (= 6.1.7.2)
-      activejob (= 6.1.7.2)
-      activemodel (= 6.1.7.2)
-      activerecord (= 6.1.7.2)
-      activestorage (= 6.1.7.2)
-      activesupport (= 6.1.7.2)
+    rails (6.1.7.3)
+      actioncable (= 6.1.7.3)
+      actionmailbox (= 6.1.7.3)
+      actionmailer (= 6.1.7.3)
+      actionpack (= 6.1.7.3)
+      actiontext (= 6.1.7.3)
+      actionview (= 6.1.7.3)
+      activejob (= 6.1.7.3)
+      activemodel (= 6.1.7.3)
+      activerecord (= 6.1.7.3)
+      activestorage (= 6.1.7.3)
+      activesupport (= 6.1.7.3)
       bundler (>= 1.15.0)
-      railties (= 6.1.7.2)
+      railties (= 6.1.7.3)
       sprockets-rails (>= 2.0.0)
     rails-controller-testing (1.0.5)
       actionpack (>= 5.0.1.rc1)
@@ -382,9 +382,9 @@ GEM
       nokogiri (>= 1.6)
     rails-html-sanitizer (1.5.0)
       loofah (~> 2.19, >= 2.19.1)
-    railties (6.1.7.2)
-      actionpack (= 6.1.7.2)
-      activesupport (= 6.1.7.2)
+    railties (6.1.7.3)
+      actionpack (= 6.1.7.3)
+      activesupport (= 6.1.7.3)
       method_source
       rake (>= 12.2)
       thor (~> 1.0)
@@ -409,7 +409,7 @@ GEM
     rspec-expectations (3.12.2)
       diff-lcs (>= 1.2.0, < 2.0)
       rspec-support (~> 3.12.0)
-    rspec-mocks (3.12.3)
+    rspec-mocks (3.12.4)
       diff-lcs (>= 1.2.0, < 2.0)
       rspec-support (~> 3.12.0)
     rspec-rails (6.0.1)
@@ -421,7 +421,7 @@ GEM
       rspec-mocks (~> 3.11)
       rspec-support (~> 3.11)
     rspec-support (3.12.0)
-    rubocop (1.48.0)
+    rubocop (1.48.1)
       json (~> 2.3)
       parallel (~> 1.10)
       parser (>= 3.2.0.0)
@@ -435,6 +435,9 @@ GEM
       parser (>= 3.2.1.0)
     rubocop-i18n (3.0.0)
       rubocop (~> 1.0)
+    rubocop-performance (1.16.0)
+      rubocop (>= 1.7.0, < 2.0)
+      rubocop-ast (>= 0.4.0)
     ruby-progressbar (1.13.0)
     ruby2_keywords (0.0.5)
     ruby_dig (0.0.2)
@@ -486,7 +489,7 @@ GEM
     unicode-display_width (2.4.2)
     uniform_notifier (1.16.0)
     uri_template (0.7.0)
-    version_gem (1.1.1)
+    version_gem (1.1.2)
     warden (1.2.9)
       rack (>= 2.0.9)
     web-console (4.2.0)
@@ -582,6 +585,7 @@ DEPENDENCIES
   rspec-rails
   rubocop
   rubocop-i18n
+  rubocop-performance
   shoulda
   spring
   spring-commands-rspec
@@ -598,7 +602,7 @@ DEPENDENCIES
   yard-tomdoc
 
 RUBY VERSION
-   ruby 3.0.5p211
+   ruby 3.0.4p208
 
 BUNDLED WITH
-   2.4.6
+   2.4.8

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/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