Fix Performance/MapCompact cop (#24797)

Co-authored-by: Claire <claire.github-309c@sitedethib.com>

.rubocop_todo.yml

@@ -236,25 +236,6 @@ Naming/VariableNumber:
     - 'spec/models/user_spec.rb'
     - 'spec/services/activitypub/fetch_featured_collection_service_spec.rb'
 
-# This cop supports unsafe autocorrection (--autocorrect-all).
-Performance/MapCompact:
-  Exclude:
-    - 'app/lib/admin/metrics/dimension.rb'
-    - 'app/lib/admin/metrics/measure.rb'
-    - 'app/lib/feed_manager.rb'
-    - 'app/models/account.rb'
-    - 'app/models/account_statuses_cleanup_policy.rb'
-    - 'app/models/account_suggestions/setting_source.rb'
-    - 'app/models/account_suggestions/source.rb'
-    - 'app/models/follow_recommendation_filter.rb'
-    - 'app/models/notification.rb'
-    - 'app/models/user_role.rb'
-    - 'app/models/webhook.rb'
-    - 'app/services/process_mentions_service.rb'
-    - 'app/validators/existing_username_validator.rb'
-    - 'db/migrate/20200407202420_migrate_unavailable_inboxes.rb'
-    - 'spec/presenters/status_relationships_presenter_spec.rb'
-
 # This cop supports unsafe autocorrection (--autocorrect-all).
 Performance/UnfreezeString:
   Exclude:

app/lib/admin/metrics/dimension.rb

@@ -14,9 +14,9 @@ class Admin::Metrics::Dimension
   }.freeze
 
   def self.retrieve(dimension_keys, start_at, end_at, limit, params)
-    Array(dimension_keys).map do |key|
+    Array(dimension_keys).filter_map do |key|
       klass = DIMENSIONS[key.to_sym]
       klass&.new(start_at, end_at, limit, klass.with_params? ? params.require(key.to_sym) : nil)
-    end.compact
+    end
   end
 end

app/lib/admin/metrics/measure.rb

@@ -19,9 +19,9 @@ class Admin::Metrics::Measure
   }.freeze
 
   def self.retrieve(measure_keys, start_at, end_at, params)
-    Array(measure_keys).map do |key|
+    Array(measure_keys).filter_map do |key|
       klass = MEASURES[key.to_sym]
       klass&.new(start_at, end_at, klass.with_params? ? params.require(key.to_sym) : nil)
-    end.compact
+    end
   end
 end

app/lib/feed_manager.rb

@@ -188,7 +188,7 @@ class FeedManager
     timeline_key        = key(:home, account.id)
     timeline_status_ids = redis.zrange(timeline_key, 0, -1)
     statuses            = Status.where(id: timeline_status_ids).select(:id, :reblog_of_id, :account_id).to_a
-    reblogged_ids       = Status.where(id: statuses.map(&:reblog_of_id).compact, account: target_account).pluck(:id)
+    reblogged_ids       = Status.where(id: statuses.filter_map(&:reblog_of_id), account: target_account).pluck(:id)
     with_mentions_ids   = Mention.active.where(status_id: statuses.flat_map { |s| [s.id, s.reblog_of_id] }.compact, account: target_account).pluck(:status_id)
 
     target_statuses = statuses.select do |status|
@@ -208,7 +208,7 @@ class FeedManager
     timeline_key        = key(:list, list.id)
     timeline_status_ids = redis.zrange(timeline_key, 0, -1)
     statuses            = Status.where(id: timeline_status_ids).select(:id, :reblog_of_id, :account_id).to_a
-    reblogged_ids       = Status.where(id: statuses.map(&:reblog_of_id).compact, account: target_account).pluck(:id)
+    reblogged_ids       = Status.where(id: statuses.filter_map(&:reblog_of_id), account: target_account).pluck(:id)
     with_mentions_ids   = Mention.active.where(status_id: statuses.flat_map { |s| [s.id, s.reblog_of_id] }.compact, account: target_account).pluck(:status_id)
 
     target_statuses = statuses.select do |status|
@@ -543,9 +543,9 @@ class FeedManager
       arr
     end
 
-    crutches[:following]       = Follow.where(account_id: receiver_id, target_account_id: statuses.map(&:in_reply_to_account_id).compact).pluck(:target_account_id).index_with(true)
+    crutches[:following]       = Follow.where(account_id: receiver_id, target_account_id: statuses.filter_map(&:in_reply_to_account_id)).pluck(:target_account_id).index_with(true)
     crutches[:languages]       = Follow.where(account_id: receiver_id, target_account_id: statuses.map(&:account_id)).pluck(:target_account_id, :languages).to_h
-    crutches[:hiding_reblogs]  = Follow.where(account_id: receiver_id, target_account_id: statuses.map { |s| s.account_id if s.reblog? }.compact, show_reblogs: false).pluck(:target_account_id).index_with(true)
+    crutches[:hiding_reblogs]  = Follow.where(account_id: receiver_id, target_account_id: statuses.filter_map { |s| s.account_id if s.reblog? }, show_reblogs: false).pluck(:target_account_id).index_with(true)
     crutches[:blocking]        = Block.where(account_id: receiver_id, target_account_id: check_for_blocks).pluck(:target_account_id).index_with(true)
     crutches[:muting]          = Mute.where(account_id: receiver_id, target_account_id: check_for_blocks).pluck(:target_account_id).index_with(true)
     crutches[:domain_blocking] = AccountDomainBlock.where(account_id: receiver_id, domain: statuses.flat_map { |s| [s.account.domain, s.reblog&.account&.domain] }.compact).pluck(:domain).index_with(true)

app/models/account.rb

@@ -299,11 +299,11 @@ class Account < ApplicationRecord
   end
 
   def fields
-    (self[:fields] || []).map do |f|
+    (self[:fields] || []).filter_map do |f|
       Account::Field.new(self, f)
     rescue
       nil
-    end.compact
+    end
   end
 
   def fields_attributes=(attributes)

app/models/account_statuses_cleanup_policy.rb

@@ -117,12 +117,12 @@ class AccountStatusesCleanupPolicy < ApplicationRecord
   private
 
   def update_last_inspected
-    if EXCEPTION_BOOLS.map { |name| attribute_change_to_be_saved(name) }.compact.include?([true, false])
+    if EXCEPTION_BOOLS.filter_map { |name| attribute_change_to_be_saved(name) }.include?([true, false])
       # Policy has been widened in such a way that any previously-inspected status
       # may need to be deleted, so we'll have to start again.
       redis.del("account_cleanup:#{account_id}")
     end
-    redis.del("account_cleanup:#{account_id}") if EXCEPTION_THRESHOLDS.map { |name| attribute_change_to_be_saved(name) }.compact.any? { |old, new| old.present? && (new.nil? || new > old) }
+    redis.del("account_cleanup:#{account_id}") if EXCEPTION_THRESHOLDS.filter_map { |name| attribute_change_to_be_saved(name) }.any? { |old, new| old.present? && (new.nil? || new > old) }
   end
 
   def validate_local_account

app/models/account_suggestions/setting_source.rb

@@ -48,14 +48,14 @@ class AccountSuggestions::SettingSource < AccountSuggestions::Source
   end
 
   def setting_to_usernames_and_domains
-    setting.split(',').map do |str|
+    setting.split(',').filter_map do |str|
       username, domain = str.strip.gsub(/\A@/, '').split('@', 2)
       domain           = nil if TagManager.instance.local_domain?(domain)
 
       next if username.blank?
 
       [username.downcase, domain&.downcase]
-    end.compact
+    end
   end
 
   def setting

app/models/account_suggestions/source.rb

@@ -20,7 +20,7 @@ class AccountSuggestions::Source
 
     map = scope.index_by { |account| to_ordered_list_key(account) }
 
-    ordered_list.map { |ordered_list_key| map[ordered_list_key] }.compact.map do |account|
+    ordered_list.filter_map { |ordered_list_key| map[ordered_list_key] }.map do |account|
       AccountSuggestions::Suggestion.new(
         account: account,
         source: key

app/models/follow_recommendation_filter.rb

@@ -22,7 +22,7 @@ class FollowRecommendationFilter
       account_ids = redis.zrevrange("follow_recommendations:#{@language}", 0, -1).map(&:to_i)
       accounts    = Account.where(id: account_ids).index_by(&:id)
 
-      account_ids.map { |id| accounts[id] }.compact
+      account_ids.filter_map { |id| accounts[id] }
     end
   end
 end

app/models/notification.rb

@@ -114,7 +114,7 @@ class Notification < ApplicationRecord
         ActiveRecord::Associations::Preloader.new.preload(grouped_notifications, associations)
       end
 
-      unique_target_statuses = notifications.map(&:target_status).compact.uniq
+      unique_target_statuses = notifications.filter_map(&:target_status).uniq
       # Call cache_collection in block
       cached_statuses_by_id = yield(unique_target_statuses).index_by(&:id)
 

app/models/user_role.rb

@@ -125,7 +125,7 @@ class UserRole < ApplicationRecord
   end
 
   def permissions_as_keys=(value)
-    self.permissions = value.map(&:presence).compact.reduce(Flags::NONE) { |bitmask, privilege| FLAGS.key?(privilege.to_sym) ? (bitmask | FLAGS[privilege.to_sym]) : bitmask }
+    self.permissions = value.filter_map(&:presence).reduce(Flags::NONE) { |bitmask, privilege| FLAGS.key?(privilege.to_sym) ? (bitmask | FLAGS[privilege.to_sym]) : bitmask }
   end
 
   def can?(*any_of_privileges)

app/models/webhook.rb

@@ -53,7 +53,7 @@ class Webhook < ApplicationRecord
   end
 
   def strip_events
-    self.events = events.map { |str| str.strip.presence }.compact if events.present?
+    self.events = events.filter_map { |str| str.strip.presence } if events.present?
   end
 
   def generate_secret

app/services/process_mentions_service.rb

@@ -68,7 +68,7 @@ class ProcessMentionsService < BaseService
   def assign_mentions!
     # Make sure we never mention blocked accounts
     unless @current_mentions.empty?
-      mentioned_domains = @current_mentions.map { |m| m.account.domain }.compact.uniq
+      mentioned_domains = @current_mentions.filter_map { |m| m.account.domain }.uniq
       blocked_domains   = Set.new(mentioned_domains.empty? ? [] : AccountDomainBlock.where(account_id: @status.account_id, domain: mentioned_domains))
       mentioned_account_ids = @current_mentions.map(&:account_id)
       blocked_account_ids = Set.new(@status.account.block_relationships.where(target_account_id: mentioned_account_ids).pluck(:target_account_id))

app/validators/existing_username_validator.rb

@@ -4,14 +4,14 @@ class ExistingUsernameValidator < ActiveModel::EachValidator
   def validate_each(record, attribute, value)
     return if value.blank?
 
-    usernames_and_domains = value.split(',').map do |str|
+    usernames_and_domains = value.split(',').filter_map do |str|
       username, domain = str.strip.gsub(/\A@/, '').split('@', 2)
       domain = nil if TagManager.instance.local_domain?(domain)
 
       next if username.blank?
 
       [str, username, domain]
-    end.compact
+    end
 
     usernames_with_no_accounts = usernames_and_domains.filter_map do |(str, username, domain)|
       str unless Account.find_remote(username, domain)

db/migrate/20200407202420_migrate_unavailable_inboxes.rb

@@ -5,9 +5,9 @@ class MigrateUnavailableInboxes < ActiveRecord::Migration[5.2]
     redis = RedisConfiguration.pool.checkout
     urls = redis.smembers('unavailable_inboxes')
 
-    hosts = urls.map do |url|
+    hosts = urls.filter_map do |url|
       Addressable::URI.parse(url).normalized_host
-    end.compact.uniq
+    end.uniq
 
     UnavailableDomain.delete_all
 

spec/presenters/status_relationships_presenter_spec.rb

@@ -15,7 +15,7 @@ RSpec.describe StatusRelationshipsPresenter do
     let(:presenter)          { StatusRelationshipsPresenter.new(statuses, current_account_id, **options) }
     let(:current_account_id) { Fabricate(:account).id }
     let(:statuses)           { [Fabricate(:status)] }
-    let(:status_ids)         { statuses.map(&:id) + statuses.map(&:reblog_of_id).compact }
+    let(:status_ids)         { statuses.map(&:id) + statuses.filter_map(&:reblog_of_id) }
     let(:default_map)        { { 1 => true } }
 
     context 'when options are not set' do