From bbbc861f717689a0a28f031fe2176d0f8a6b07c7 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Fri, 23 Apr 2021 16:31:19 +0900 Subject: [PATCH] Enable `Performance/MapCompact` cop Follow up to #42053. --- .rubocop.yml | 3 +++ Gemfile.lock | 16 +++++++++++++--- .../journey/gtg/transition_table.rb | 4 ++-- actiontext/lib/action_text/encryption.rb | 2 +- .../action_view/helpers/form_options_helper.rb | 4 ++-- .../lib/active_model/attribute_methods.rb | 2 +- .../lib/active_model/validations/confirmation.rb | 8 ++++---- .../associations/preloader/branch.rb | 4 ++-- .../abstract/database_statements.rb | 4 ++-- .../abstract_mysql_adapter.rb | 5 ++--- .../connection_adapters/postgresql_adapter.rb | 4 +--- .../sqlite3/schema_statements.rb | 4 ++-- activerecord/lib/active_record/core.rb | 4 ++-- activerecord/lib/active_record/insert_all.rb | 4 ++-- activerecord/lib/active_record/migration.rb | 4 ++-- .../active_record/tasks/mysql_database_tasks.rb | 2 +- activerecord/lib/active_record/timestamp.rb | 3 +-- .../active_support/core_ext/object/to_query.rb | 4 ++-- .../test_unit/scaffold/scaffold_generator.rb | 4 ++-- railties/lib/rails/tasks/zeitwerk.rake | 4 ++-- 20 files changed, 49 insertions(+), 40 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index d4d986d716..c4eef5af25 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -253,6 +253,9 @@ Performance/BindCall: Performance/FlatMap: Enabled: true +Performance/MapCompact: + Enabled: true + Performance/RedundantMerge: Enabled: true diff --git a/Gemfile.lock b/Gemfile.lock index 45748c6ba3..94797b3313 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -221,6 +221,9 @@ GEM tzinfo event_emitter (0.2.6) eventmachine (1.2.7) + eventmachine (1.2.7-java) + eventmachine (1.2.7-x64-mingw32) + eventmachine (1.2.7-x86-mingw32) execjs (2.7.0) faraday (1.3.0) faraday-net_http (~> 1.0) @@ -288,12 +291,14 @@ GEM hiredis (0.6.3) hiredis (0.6.3-java) http_parser.rb (0.6.0) + http_parser.rb (0.6.0-java) httpclient (2.8.3) i18n (1.8.10) concurrent-ruby (~> 1.0) image_processing (1.12.1) mini_magick (>= 4.9.5, < 5) ruby-vips (>= 2.0.17, < 3) + jar-dependencies (0.4.1) jdbc-mysql (5.1.47) jdbc-postgres (42.2.14) jdbc-sqlite3 (3.28.0) @@ -305,6 +310,7 @@ GEM mustache nokogiri libxml-ruby (3.2.1) + libxml-ruby (3.2.1-x64-mingw32) listen (3.5.1) rb-fsevent (~> 0.10, >= 0.10.3) rb-inotify (~> 0.9, >= 0.9.10) @@ -361,7 +367,11 @@ GEM ast (~> 2.4.1) path_expander (1.1.0) pg (1.2.3) + pg (1.2.3-x64-mingw32) + pg (1.2.3-x86-mingw32) psych (3.3.1) + psych (3.3.1-java) + jar-dependencies (>= 0.1.7) public_suffix (4.0.6) puma (5.2.2) nio4r (~> 2.0) @@ -417,7 +427,7 @@ GEM retriable (3.1.2) rexml (3.2.5) rouge (3.26.0) - rubocop (1.12.1) + rubocop (1.13.0) parallel (~> 1.10) parser (>= 3.0.0.0) rainbow (>= 2.2.2, < 4.0) @@ -430,8 +440,8 @@ GEM parser (>= 2.7.1.5) rubocop-packaging (0.5.1) rubocop (>= 0.89, < 2.0) - rubocop-performance (1.10.2) - rubocop (>= 0.90.0, < 2.0) + rubocop-performance (1.11.0) + rubocop (>= 1.7.0, < 2.0) rubocop-ast (>= 0.4.0) rubocop-rails (2.9.1) activesupport (>= 4.2.0) diff --git a/actionpack/lib/action_dispatch/journey/gtg/transition_table.rb b/actionpack/lib/action_dispatch/journey/gtg/transition_table.rb index f3c93d5340..ff98a2cb06 100644 --- a/actionpack/lib/action_dispatch/journey/gtg/transition_table.rb +++ b/actionpack/lib/action_dispatch/journey/gtg/transition_table.rb @@ -130,7 +130,7 @@ module ActionDispatch states = "function tt() { return #{to_json}; }" fun_routes = paths.sample(3).map do |ast| - ast.map { |n| + ast.filter_map { |n| case n when Nodes::Symbol case n.left @@ -143,7 +143,7 @@ module ActionDispatch else nil end - }.compact.join + }.join end stylesheets = [fsm_css] diff --git a/actiontext/lib/action_text/encryption.rb b/actiontext/lib/action_text/encryption.rb index af680a2ab1..d79afd302b 100644 --- a/actiontext/lib/action_text/encryption.rb +++ b/actiontext/lib/action_text/encryption.rb @@ -31,7 +31,7 @@ module ActionText def encryptable_rich_texts @encryptable_rich_texts ||= self.class.rich_text_association_names - .collect { |attribute_name| send(attribute_name) }.compact + .filter_map { |attribute_name| send(attribute_name) } .find_all { |record| record.is_a?(ActionText::EncryptedRichText) } end end diff --git a/actionview/lib/action_view/helpers/form_options_helper.rb b/actionview/lib/action_view/helpers/form_options_helper.rb index 0b9d5dcc67..719d2849b8 100644 --- a/actionview/lib/action_view/helpers/form_options_helper.rb +++ b/actionview/lib/action_view/helpers/form_options_helper.rb @@ -793,9 +793,9 @@ module ActionView def extract_values_from_collection(collection, value_method, selected) if selected.is_a?(Proc) - collection.map do |element| + collection.filter_map do |element| element.public_send(value_method) if selected.call(element) - end.compact + end else selected end diff --git a/activemodel/lib/active_model/attribute_methods.rb b/activemodel/lib/active_model/attribute_methods.rb index 1b2e7c1653..c7b43c5c71 100644 --- a/activemodel/lib/active_model/attribute_methods.rb +++ b/activemodel/lib/active_model/attribute_methods.rb @@ -399,7 +399,7 @@ module ActiveModel def attribute_method_matchers_matching(method_name) attribute_method_matchers_cache.compute_if_absent(method_name) do - attribute_method_matchers.map { |matcher| matcher.match(method_name) }.compact + attribute_method_matchers.filter_map { |matcher| matcher.match(method_name) } end end diff --git a/activemodel/lib/active_model/validations/confirmation.rb b/activemodel/lib/active_model/validations/confirmation.rb index 9ef2031be5..d9c560acf4 100644 --- a/activemodel/lib/active_model/validations/confirmation.rb +++ b/activemodel/lib/active_model/validations/confirmation.rb @@ -19,13 +19,13 @@ module ActiveModel private def setup!(klass) - klass.attr_reader(*attributes.map do |attribute| + klass.attr_reader(*attributes.filter_map do |attribute| :"#{attribute}_confirmation" unless klass.method_defined?(:"#{attribute}_confirmation") - end.compact) + end) - klass.attr_writer(*attributes.map do |attribute| + klass.attr_writer(*attributes.filter_map do |attribute| :"#{attribute}_confirmation" unless klass.method_defined?(:"#{attribute}_confirmation=") - end.compact) + end) end def confirmation_value_equal?(record, attribute, value, confirmed) diff --git a/activerecord/lib/active_record/associations/preloader/branch.rb b/activerecord/lib/active_record/associations/preloader/branch.rb index c7d43ff295..1bea9884d4 100644 --- a/activerecord/lib/active_record/associations/preloader/branch.rb +++ b/activerecord/lib/active_record/associations/preloader/branch.rb @@ -46,9 +46,9 @@ module ActiveRecord def likely_reflections parent_classes = parent.target_classes - parent_classes.map do |parent_klass| + parent_classes.filter_map do |parent_klass| parent_klass._reflect_on_association(@association) - end.compact + end end def root? diff --git a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb index 24974dbb7f..4a1541a91b 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb @@ -498,10 +498,10 @@ module ActiveRecord end def build_fixture_statements(fixture_set) - fixture_set.map do |table_name, fixtures| + fixture_set.filter_map do |table_name, fixtures| next if fixtures.empty? build_fixture_sql(fixtures, table_name) - end.compact + end end def build_truncate_statement(table_name) diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb index 1f9d796fc2..645482a2b1 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -787,14 +787,13 @@ module ActiveRecord end # Gather up all of the SET variables... - variable_assignments = variables.map do |k, v| + variable_assignments = variables.filter_map do |k, v| if defaults.include?(v) "@@SESSION.#{k} = DEFAULT" # Sets the value to the global or compile default elsif !v.nil? "@@SESSION.#{k} = #{quote(v)}" end - # or else nil; compact to clear nils out - end.compact.join(", ") + end.join(", ") # ...and send them all in one query execute("SET #{encoding} #{sql_mode_assignment} #{variable_assignments}", "SCHEMA") diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index d8ac678578..ef60c2cfcb 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -917,9 +917,7 @@ module ActiveRecord WHERE t.typname IN (%s) SQL coders = execute_and_clear(query, "SCHEMA", []) do |result| - result - .map { |row| construct_coder(row, coders_by_name[row["typname"]]) } - .compact + result.filter_map { |row| construct_coder(row, coders_by_name[row["typname"]]) } end map = PG::TypeMapByOid.new diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/sqlite3/schema_statements.rb index d9698b01ca..1a6e0dbe53 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3/schema_statements.rb @@ -6,7 +6,7 @@ module ActiveRecord module SchemaStatements # :nodoc: # Returns an array of indexes for the given table. def indexes(table_name) - exec_query("PRAGMA index_list(#{quote_table_name(table_name)})", "SCHEMA").map do |row| + exec_query("PRAGMA index_list(#{quote_table_name(table_name)})", "SCHEMA").filter_map do |row| # Indexes SQLite creates implicitly for internal use start with "sqlite_". # See https://www.sqlite.org/fileformat2.html#intschema next if row["name"].start_with?("sqlite_") @@ -49,7 +49,7 @@ module ActiveRecord where: where, orders: orders ) - end.compact + end end def add_foreign_key(from_table, to_table, **options) diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index d54f6ad217..c08688e4ea 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -796,11 +796,11 @@ module ActiveRecord # We check defined?(@attributes) not to issue warnings if the object is # allocated but not initialized. inspection = if defined?(@attributes) && @attributes - self.class.attribute_names.collect do |name| + self.class.attribute_names.filter_map do |name| if _has_attribute?(name) "#{name}: #{attribute_for_inspect(name)}" end - end.compact.join(", ") + end.join(", ") else "not initialized" end diff --git a/activerecord/lib/active_record/insert_all.rb b/activerecord/lib/active_record/insert_all.rb index 8f6231a5fa..fefd8bd0a5 100644 --- a/activerecord/lib/active_record/insert_all.rb +++ b/activerecord/lib/active_record/insert_all.rb @@ -196,11 +196,11 @@ module ActiveRecord end def touch_model_timestamps_unless(&block) - model.timestamp_attributes_for_update_in_model.map do |column_name| + model.timestamp_attributes_for_update_in_model.filter_map do |column_name| if touch_timestamp_attribute?(column_name) "#{column_name}=(CASE WHEN (#{updatable_columns.map(&block).join(" AND ")}) THEN #{model.quoted_table_name}.#{column_name} ELSE CURRENT_TIMESTAMP END)," end - end.compact.join + end.join end def raw_update_sql diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index 54830e5c4b..312b6b3c1d 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -1159,13 +1159,13 @@ module ActiveRecord def migrations_status # :nodoc: db_list = schema_migration.normalized_versions - file_list = migration_files.map do |file| + file_list = migration_files.filter_map do |file| version, name, scope = parse_migration_filename(file) raise IllegalMigrationNameError.new(file) unless version version = schema_migration.normalize_migration_number(version) status = db_list.delete(version) ? "up" : "down" [status, version, (name + scope).humanize] - end.compact + end db_list.map! do |version| ["up", version, "********** NO FILE **********"] diff --git a/activerecord/lib/active_record/tasks/mysql_database_tasks.rb b/activerecord/lib/active_record/tasks/mysql_database_tasks.rb index fdba782ca3..300753be4d 100644 --- a/activerecord/lib/active_record/tasks/mysql_database_tasks.rb +++ b/activerecord/lib/active_record/tasks/mysql_database_tasks.rb @@ -94,7 +94,7 @@ module ActiveRecord sslcapath: "--ssl-capath", sslcipher: "--ssl-cipher", sslkey: "--ssl-key" - }.map { |opt, arg| "#{arg}=#{configuration_hash[opt]}" if configuration_hash[opt] }.compact + }.filter_map { |opt, arg| "#{arg}=#{configuration_hash[opt]}" if configuration_hash[opt] } args end diff --git a/activerecord/lib/active_record/timestamp.rb b/activerecord/lib/active_record/timestamp.rb index 9b8eeb8c09..592b05f28d 100644 --- a/activerecord/lib/active_record/timestamp.rb +++ b/activerecord/lib/active_record/timestamp.rb @@ -148,8 +148,7 @@ module ActiveRecord def max_updated_column_timestamp timestamp_attributes_for_update_in_model - .map { |attr| self[attr]&.to_time } - .compact + .filter_map { |attr| self[attr]&.to_time } .max end diff --git a/activesupport/lib/active_support/core_ext/object/to_query.rb b/activesupport/lib/active_support/core_ext/object/to_query.rb index bac6ff9c97..736643b384 100644 --- a/activesupport/lib/active_support/core_ext/object/to_query.rb +++ b/activesupport/lib/active_support/core_ext/object/to_query.rb @@ -75,11 +75,11 @@ class Hash # # This method is also aliased as +to_param+. def to_query(namespace = nil) - query = collect do |key, value| + query = filter_map do |key, value| unless (value.is_a?(Hash) || value.is_a?(Array)) && value.empty? value.to_query(namespace ? "#{namespace}[#{key}]" : key) end - end.compact + end query.sort! unless namespace.to_s.include?("[]") query.join("&") diff --git a/railties/lib/rails/generators/test_unit/scaffold/scaffold_generator.rb b/railties/lib/rails/generators/test_unit/scaffold/scaffold_generator.rb index a4bc81cad6..3a82081d6a 100644 --- a/railties/lib/rails/generators/test_unit/scaffold/scaffold_generator.rb +++ b/railties/lib/rails/generators/test_unit/scaffold/scaffold_generator.rb @@ -45,13 +45,13 @@ module TestUnit # :nodoc: def attributes_hash return {} if attributes_names.empty? - attributes_names.map do |name| + attributes_names.filter_map do |name| if %w(password password_confirmation).include?(name) && attributes.any?(&:password_digest?) ["#{name}", "'secret'"] elsif !virtual?(name) ["#{name}", "@#{singular_table_name}.#{name}"] end - end.compact.sort.to_h + end.sort.to_h end def boolean?(name) diff --git a/railties/lib/rails/tasks/zeitwerk.rake b/railties/lib/rails/tasks/zeitwerk.rake index 6e5c75fe8d..4b3e0ef95f 100644 --- a/railties/lib/rails/tasks/zeitwerk.rake +++ b/railties/lib/rails/tasks/zeitwerk.rake @@ -54,11 +54,11 @@ namespace :zeitwerk do end require "active_support/core_ext/object/try" - eager_load_paths = Rails.configuration.eager_load_namespaces.map do |eln| + eager_load_paths = Rails.configuration.eager_load_namespaces.filter_map do |eln| # Quick regression fix for 6.0.3 to support namespaces that do not have # eager load paths, like the recently added i18n. I'll rewrite this task. eln.try(:config).try(:eager_load_paths) - end.compact.flatten + end.flatten not_checked = ActiveSupport::Dependencies.autoload_paths - eager_load_paths not_checked.select! { |dir| Dir.exist?(dir) }