From 27140247c21c3925ee971ca4437e9a7f6fe198a3 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Fri, 5 Jan 2024 14:37:50 +0100 Subject: [PATCH] Cleanup `defined?` usage Now that we dropped support for Ruby 2.7, we no longer need to check if variables are defined before accessing them to avoid the undefined variable warning. --- .../testing/assertions/routing.rb | 6 ++-- actionview/lib/action_view/test_case.rb | 2 +- activejob/lib/active_job/core.rb | 2 +- activemodel/lib/active_model/attribute.rb | 2 +- .../preloader/through_association.rb | 4 +-- .../lib/active_record/attribute_methods.rb | 9 ++--- .../lib/active_record/connection_handling.rb | 2 +- activerecord/lib/active_record/core.rb | 6 ++-- activerecord/lib/active_record/inheritance.rb | 2 +- .../lib/active_record/model_schema.rb | 9 +++-- .../active_record/relation/query_methods.rb | 3 +- activerecord/lib/active_record/touch_later.rb | 2 +- .../adapters/postgresql/geometric_test.rb | 2 +- .../previewer/mupdf_previewer.rb | 2 +- .../previewer/poppler_pdf_previewer.rb | 2 +- .../previewer/video_previewer.rb | 2 +- activesupport/lib/active_support/cache.rb | 2 +- .../cache/behaviors/cache_logging_behavior.rb | 2 +- .../lib/rails/application/configuration.rb | 4 +-- .../commands/dbconsole/dbconsole_command.rb | 35 +++++++++---------- .../generators/rails/app/app_generator.rb | 2 +- 21 files changed, 45 insertions(+), 57 deletions(-) diff --git a/actionpack/lib/action_dispatch/testing/assertions/routing.rb b/actionpack/lib/action_dispatch/testing/assertions/routing.rb index e6a9c56263628..25fb9810a578f 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/routing.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/routing.rb @@ -248,7 +248,7 @@ def assert_routing(path, options, defaults = {}, extras = {}, message = nil) # ROUTES TODO: These assertions should really work in an integration context def method_missing(selector, ...) - if defined?(@controller) && @controller && defined?(@routes) && @routes && @routes.named_routes.route_defined?(selector) + if @controller && @routes&.named_routes.route_defined?(selector) @controller.public_send(selector, ...) else super @@ -258,7 +258,7 @@ def method_missing(selector, ...) private def create_routes @routes = ActionDispatch::Routing::RouteSet.new - if defined?(@controller) && @controller + if @controller @controller = @controller.clone _routes = @routes @@ -282,7 +282,7 @@ def create_routes def reset_routes(old_routes, old_controller) @routes = old_routes - if defined?(@controller) && @controller + if @controller @controller = old_controller end end diff --git a/actionview/lib/action_view/test_case.rb b/actionview/lib/action_view/test_case.rb index 073c5fad0234a..c12feeac954a9 100644 --- a/actionview/lib/action_view/test_case.rb +++ b/actionview/lib/action_view/test_case.rb @@ -433,7 +433,7 @@ def method_missing(selector, ...) def respond_to_missing?(name, include_private = false) begin - routes = defined?(@controller) && @controller.respond_to?(:_routes) && @controller._routes + routes = @controller.respond_to?(:_routes) && @controller._routes rescue # Don't call routes, if there is an error on _routes call end diff --git a/activejob/lib/active_job/core.rb b/activejob/lib/active_job/core.rb index b54361a4ed0c0..879505e48d4bb 100644 --- a/activejob/lib/active_job/core.rb +++ b/activejob/lib/active_job/core.rb @@ -196,7 +196,7 @@ def deserialize_arguments(serialized_args) end def arguments_serialized? - defined?(@serialized_arguments) && @serialized_arguments + @serialized_arguments end end end diff --git a/activemodel/lib/active_model/attribute.rb b/activemodel/lib/active_model/attribute.rb index f5965ba5773f8..40554eb0b9fa0 100644 --- a/activemodel/lib/active_model/attribute.rb +++ b/activemodel/lib/active_model/attribute.rb @@ -153,7 +153,7 @@ def original_value_for_database alias :assigned? :original_attribute def initialize_dup(other) - if defined?(@value) && @value.duplicable? + if @value&.duplicable? @value = @value.dup end end diff --git a/activerecord/lib/active_record/associations/preloader/through_association.rb b/activerecord/lib/active_record/associations/preloader/through_association.rb index 3ee1b4bb21622..50d9031542420 100644 --- a/activerecord/lib/active_record/associations/preloader/through_association.rb +++ b/activerecord/lib/active_record/associations/preloader/through_association.rb @@ -9,9 +9,7 @@ def preloaded_records end def records_by_owner - return @records_by_owner if defined?(@records_by_owner) - - @records_by_owner = owners.each_with_object({}) do |owner, result| + @records_by_owner ||= owners.each_with_object({}) do |owner, result| if loaded?(owner) result[owner] = target_for(owner) next diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb index bf2970e75145b..e0fdbec957b8a 100644 --- a/activerecord/lib/active_record/attribute_methods.rb +++ b/activerecord/lib/active_record/attribute_methods.rb @@ -137,7 +137,7 @@ def define_attribute_methods # :nodoc: def undefine_attribute_methods # :nodoc: GeneratedAttributeMethods::LOCK.synchronize do - super if defined?(@attribute_methods_generated) && @attribute_methods_generated + super if @attribute_methods_generated @attribute_methods_generated = false @alias_attributes_mass_generated = false end @@ -288,9 +288,7 @@ def respond_to?(name, include_private = false) # If the result is true then check for the select case. # For queries selecting a subset of columns, return false for unselected columns. - # We check defined?(@attributes) not to issue warnings if called on objects that - # have been allocated but not yet initialized. - if defined?(@attributes) + if @attributes if name = self.class.symbol_column_to_string(name.to_sym) return _has_attribute?(name) end @@ -460,8 +458,7 @@ def accessed_fields private def attribute_method?(attr_name) - # We check defined? because Syck calls respond_to? before actually calling initialize. - defined?(@attributes) && @attributes.key?(attr_name) + @attributes&.key?(attr_name) end def attributes_with_values(attribute_names) diff --git a/activerecord/lib/active_record/connection_handling.rb b/activerecord/lib/active_record/connection_handling.rb index e179d69b4d6a8..c495adef800b2 100644 --- a/activerecord/lib/active_record/connection_handling.rb +++ b/activerecord/lib/active_record/connection_handling.rb @@ -258,7 +258,7 @@ def connection # Return the connection specification name from the current class or its parent. def connection_specification_name - if !defined?(@connection_specification_name) || @connection_specification_name.nil? + if @connection_specification_name.nil? return self == Base ? Base.name : superclass.connection_specification_name end @connection_specification_name diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index 625dd365513fb..44a3a37cad233 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -720,7 +720,7 @@ def full_inspect def pretty_print(pp) return super if custom_inspect_method_defined? pp.object_address_group(self) do - if defined?(@attributes) && @attributes + if @attributes attr_names = attributes_for_inspect.select { |name| _has_attribute?(name.to_s) } pp.seplist(attr_names, proc { pp.text "," }) do |attr_name| attr_name = attr_name.to_s @@ -791,9 +791,7 @@ def inspection_filter end def inspect_with_attributes(attributes_to_list) - # We check defined?(@attributes) not to issue warnings if the object is - # allocated but not initialized. - inspection = if defined?(@attributes) && @attributes + inspection = if @attributes attributes_to_list.filter_map do |name| name = name.to_s if _has_attribute?(name) diff --git a/activerecord/lib/active_record/inheritance.rb b/activerecord/lib/active_record/inheritance.rb index edb66ff6aea0f..f560f7b096d53 100644 --- a/activerecord/lib/active_record/inheritance.rb +++ b/activerecord/lib/active_record/inheritance.rb @@ -165,7 +165,7 @@ def base_class? # Returns whether this class is an abstract class or not. def abstract_class? - defined?(@abstract_class) && @abstract_class == true + @abstract_class == true end # Sets the application record class for Active Record diff --git a/activerecord/lib/active_record/model_schema.rb b/activerecord/lib/active_record/model_schema.rb index 3d4be984c2515..1d0c1ecdff5bb 100644 --- a/activerecord/lib/active_record/model_schema.rb +++ b/activerecord/lib/active_record/model_schema.rb @@ -273,7 +273,7 @@ def table_name=(value) @table_name = value @quoted_table_name = nil @arel_table = nil - @sequence_name = nil unless defined?(@explicit_sequence_name) && @explicit_sequence_name + @sequence_name = nil unless @explicit_sequence_name @predicate_builder = nil end @@ -414,11 +414,10 @@ def table_exists? end def attributes_builder # :nodoc: - unless defined?(@attributes_builder) && @attributes_builder + @attributes_builder ||= begin defaults = _default_attributes.except(*(column_names - [primary_key])) - @attributes_builder = ActiveModel::AttributeSet::Builder.new(attribute_types, defaults) + ActiveModel::AttributeSet::Builder.new(attribute_types, defaults) end - @attributes_builder end def columns_hash # :nodoc: @@ -573,7 +572,7 @@ def inherited(child_class) end def schema_loaded? - defined?(@schema_loaded) && @schema_loaded + @schema_loaded end def load_schema! diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 4ccd5d78aa9dc..b9dc7392323c1 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -1575,8 +1575,7 @@ def build_join_dependencies end def assert_mutability! - raise ImmutableRelation if @loaded - raise ImmutableRelation if defined?(@arel) && @arel + raise ImmutableRelation if @loaded || @arel end def build_arel(aliases = nil) diff --git a/activerecord/lib/active_record/touch_later.rb b/activerecord/lib/active_record/touch_later.rb index 6458414fd5cba..f84c543f8c0aa 100644 --- a/activerecord/lib/active_record/touch_later.rb +++ b/activerecord/lib/active_record/touch_later.rb @@ -64,7 +64,7 @@ def touch_deferred_attributes end def has_defer_touch_attrs? - defined?(@_defer_touch_attrs) && @_defer_touch_attrs.present? + @_defer_touch_attrs.present? end end end diff --git a/activerecord/test/cases/adapters/postgresql/geometric_test.rb b/activerecord/test/cases/adapters/postgresql/geometric_test.rb index f312b6e23d7a0..b3406b66a6db8 100644 --- a/activerecord/test/cases/adapters/postgresql/geometric_test.rb +++ b/activerecord/test/cases/adapters/postgresql/geometric_test.rb @@ -257,7 +257,7 @@ class PostgresqlLine < ActiveRecord::Base; end end teardown do - if defined?(@connection) + if @connection @connection.drop_table "postgresql_lines", if_exists: true end end diff --git a/activestorage/lib/active_storage/previewer/mupdf_previewer.rb b/activestorage/lib/active_storage/previewer/mupdf_previewer.rb index 3899a2065aa38..f1f9aa85be200 100644 --- a/activestorage/lib/active_storage/previewer/mupdf_previewer.rb +++ b/activestorage/lib/active_storage/previewer/mupdf_previewer.rb @@ -12,7 +12,7 @@ def mutool_path end def mutool_exists? - return @mutool_exists if defined?(@mutool_exists) && !@mutool_exists.nil? + return @mutool_exists unless @mutool_exists.nil? system mutool_path, out: File::NULL, err: File::NULL diff --git a/activestorage/lib/active_storage/previewer/poppler_pdf_previewer.rb b/activestorage/lib/active_storage/previewer/poppler_pdf_previewer.rb index 8eaffc64914b5..e20c2b7f20720 100644 --- a/activestorage/lib/active_storage/previewer/poppler_pdf_previewer.rb +++ b/activestorage/lib/active_storage/previewer/poppler_pdf_previewer.rb @@ -12,7 +12,7 @@ def pdftoppm_path end def pdftoppm_exists? - return @pdftoppm_exists if defined?(@pdftoppm_exists) + return @pdftoppm_exists unless @pdftoppm_exists.nil? @pdftoppm_exists = system(pdftoppm_path, "-v", out: File::NULL, err: File::NULL) end diff --git a/activestorage/lib/active_storage/previewer/video_previewer.rb b/activestorage/lib/active_storage/previewer/video_previewer.rb index ad9054412c17c..ed8982d6c9676 100644 --- a/activestorage/lib/active_storage/previewer/video_previewer.rb +++ b/activestorage/lib/active_storage/previewer/video_previewer.rb @@ -10,7 +10,7 @@ def accept?(blob) end def ffmpeg_exists? - return @ffmpeg_exists if defined?(@ffmpeg_exists) + return @ffmpeg_exists unless @ffmpeg_exists.nil? @ffmpeg_exists = system(ffmpeg_path, "-version", out: File::NULL, err: File::NULL) end diff --git a/activesupport/lib/active_support/cache.rb b/activesupport/lib/active_support/cache.rb index 2d4d10664957a..f0a8ea198a986 100644 --- a/activesupport/lib/active_support/cache.rb +++ b/activesupport/lib/active_support/cache.rb @@ -344,7 +344,7 @@ def silence! # Silences the logger within a block. def mute - previous_silence, @silence = defined?(@silence) && @silence, true + previous_silence, @silence = @silence, true yield ensure @silence = previous_silence diff --git a/activesupport/test/cache/behaviors/cache_logging_behavior.rb b/activesupport/test/cache/behaviors/cache_logging_behavior.rb index e685c75aeb282..3333bbdbe41fc 100644 --- a/activesupport/test/cache/behaviors/cache_logging_behavior.rb +++ b/activesupport/test/cache/behaviors/cache_logging_behavior.rb @@ -77,7 +77,7 @@ def assert_logs(pattern, &block) assert_match pattern, io.string end - def key_pattern(key, namespace: defined?(@namespace) && @namespace) + def key_pattern(key, namespace: @namespace) /#{Regexp.escape namespace.to_s}#{":" if namespace}#{Regexp.escape key}/ end end diff --git a/railties/lib/rails/application/configuration.rb b/railties/lib/rails/application/configuration.rb index 2a9df93cd8586..7e67ffea9f9ae 100644 --- a/railties/lib/rails/application/configuration.rb +++ b/railties/lib/rails/application/configuration.rb @@ -378,9 +378,7 @@ def log_level=(level) @broadcast_log_level = level end - def broadcast_log_level # :nodoc: - defined?(@broadcast_log_level) ? @broadcast_log_level : nil - end + attr_reader :broadcast_log_level # :nodoc: def debug_exception_response_format @debug_exception_response_format || :default diff --git a/railties/lib/rails/commands/dbconsole/dbconsole_command.rb b/railties/lib/rails/commands/dbconsole/dbconsole_command.rb index 0d7ea0f0a28ba..e3e7a3245580c 100644 --- a/railties/lib/rails/commands/dbconsole/dbconsole_command.rb +++ b/railties/lib/rails/commands/dbconsole/dbconsole_command.rb @@ -21,25 +21,24 @@ def start end def db_config - return @db_config if defined?(@db_config) - - # If the user provided a database, use that. Otherwise find - # the first config in the database.yml - if database - @db_config = configurations.configs_for(env_name: environment, name: database, include_hidden: true) - else - @db_config = configurations.find_db_config(environment) + @db_config ||= begin + # If the user provided a database, use that. Otherwise find + # the first config in the database.yml + config = if database + @db_config = configurations.configs_for(env_name: environment, name: database, include_hidden: true) + else + @db_config = configurations.find_db_config(environment) + end + + unless config + missing_db = database ? "'#{database}' database is not" : "No databases are" + raise ActiveRecord::AdapterNotSpecified, + "#{missing_db} configured for '#{environment}'. Available configuration: #{configurations.inspect}" + end + + config.validate! + config end - - unless @db_config - missing_db = database ? "'#{database}' database is not" : "No databases are" - raise ActiveRecord::AdapterNotSpecified, - "#{missing_db} configured for '#{environment}'. Available configuration: #{configurations.inspect}" - end - - @db_config.validate! - - @db_config end def database diff --git a/railties/lib/rails/generators/rails/app/app_generator.rb b/railties/lib/rails/generators/rails/app/app_generator.rb index c28dd5844f0ac..50e006df28aa9 100644 --- a/railties/lib/rails/generators/rails/app/app_generator.rb +++ b/railties/lib/rails/generators/rails/app/app_generator.rb @@ -266,7 +266,7 @@ def vendor end def config_target_version - defined?(@config_target_version) ? @config_target_version : Rails::VERSION::STRING.to_f + @config_target_version || Rails::VERSION::STRING.to_f end end