From 9f5deb310b9e5c26eb3c0af018e92a4e6072661b Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Tue, 23 May 2023 04:49:12 -0400 Subject: [PATCH] Fix Performance/MapCompact cop (#24797) Co-authored-by: Claire --- .rubocop_todo.yml | 19 ------------------- app/lib/admin/metrics/dimension.rb | 4 ++-- app/lib/admin/metrics/measure.rb | 4 ++-- app/lib/feed_manager.rb | 8 ++++---- app/models/account.rb | 4 ++-- app/models/account_statuses_cleanup_policy.rb | 4 ++-- .../account_suggestions/setting_source.rb | 4 ++-- app/models/account_suggestions/source.rb | 2 +- app/models/follow_recommendation_filter.rb | 2 +- app/models/notification.rb | 2 +- app/models/user_role.rb | 2 +- app/models/webhook.rb | 2 +- app/services/process_mentions_service.rb | 2 +- app/validators/existing_username_validator.rb | 4 ++-- ...00407202420_migrate_unavailable_inboxes.rb | 4 ++-- .../status_relationships_presenter_spec.rb | 2 +- 16 files changed, 25 insertions(+), 44 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index e2441c9344..6fb910005a 100644 --- a/.rubocop_todo.yml +++ b/.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: diff --git a/app/lib/admin/metrics/dimension.rb b/app/lib/admin/metrics/dimension.rb index 81b89d9b32..e0122a65b5 100644 --- a/app/lib/admin/metrics/dimension.rb +++ b/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 diff --git a/app/lib/admin/metrics/measure.rb b/app/lib/admin/metrics/measure.rb index 0b510eb256..fe7e049290 100644 --- a/app/lib/admin/metrics/measure.rb +++ b/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 diff --git a/app/lib/feed_manager.rb b/app/lib/feed_manager.rb index 8e619f9cc9..643e6828d2 100644 --- a/app/lib/feed_manager.rb +++ b/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) diff --git a/app/models/account.rb b/app/models/account.rb index 32e9898160..f17d06be58 100644 --- a/app/models/account.rb +++ b/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) diff --git a/app/models/account_statuses_cleanup_policy.rb b/app/models/account_statuses_cleanup_policy.rb index 63582b9f9e..a102795446 100644 --- a/app/models/account_statuses_cleanup_policy.rb +++ b/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 diff --git a/app/models/account_suggestions/setting_source.rb b/app/models/account_suggestions/setting_source.rb index 7b8873e0c5..6185732b4b 100644 --- a/app/models/account_suggestions/setting_source.rb +++ b/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 diff --git a/app/models/account_suggestions/source.rb b/app/models/account_suggestions/source.rb index be462cd0f5..504d26a8bd 100644 --- a/app/models/account_suggestions/source.rb +++ b/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 diff --git a/app/models/follow_recommendation_filter.rb b/app/models/follow_recommendation_filter.rb index 5313326143..2fab975698 100644 --- a/app/models/follow_recommendation_filter.rb +++ b/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 diff --git a/app/models/notification.rb b/app/models/notification.rb index 8ba506fa1b..5527953afc 100644 --- a/app/models/notification.rb +++ b/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) diff --git a/app/models/user_role.rb b/app/models/user_role.rb index a1b91dc0f5..5472646c60 100644 --- a/app/models/user_role.rb +++ b/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) diff --git a/app/models/webhook.rb b/app/models/webhook.rb index 9a056a3862..c556bcc2bb 100644 --- a/app/models/webhook.rb +++ b/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 diff --git a/app/services/process_mentions_service.rb b/app/services/process_mentions_service.rb index b3b279147d..f3fbb80210 100644 --- a/app/services/process_mentions_service.rb +++ b/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)) diff --git a/app/validators/existing_username_validator.rb b/app/validators/existing_username_validator.rb index 45de4f4a44..037d92f39b 100644 --- a/app/validators/existing_username_validator.rb +++ b/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) diff --git a/db/migrate/20200407202420_migrate_unavailable_inboxes.rb b/db/migrate/20200407202420_migrate_unavailable_inboxes.rb index 8f9c687942..05a01be284 100644 --- a/db/migrate/20200407202420_migrate_unavailable_inboxes.rb +++ b/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 diff --git a/spec/presenters/status_relationships_presenter_spec.rb b/spec/presenters/status_relationships_presenter_spec.rb index 11116cabd2..a62fa004a1 100644 --- a/spec/presenters/status_relationships_presenter_spec.rb +++ b/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