Merge pull request #50597 from Shopify/cleanup-defined-usage

Cleanup `defined?` usage

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

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

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

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

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

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)

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

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)

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

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!

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)

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

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

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
 

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

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

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

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

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

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

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