Enable Lint/NestedMethodDefinition cop

This would have prevented the nested test mistake in rails#51826.

Unfortunately there is a few false positive that need to be
opted out, but overall I think it's a win.

Author: byroot

.rubocop.yml

@@ -215,6 +215,9 @@ Layout/TrailingWhitespace:
 Style/RedundantPercentQ:
   Enabled: true
 
+Lint/NestedMethodDefinition:
+  Enabled: true
+
 Lint/AmbiguousOperator:
   Enabled: true
 

actionpack/test/controller/params_wrapper_test.rb

@@ -257,7 +257,8 @@ def test_handles_empty_content_type
   end
 
   def test_derived_wrapped_keys_from_nested_attributes
-    def User.nested_attributes_options
+    assert_not_respond_to User, :nested_attributes_options
+    User.define_singleton_method(:nested_attributes_options) do
       { person: {} }
     end
 
@@ -268,6 +269,8 @@ def User.nested_attributes_options
         assert_parameters("username" => "sikachu", "person_attributes" => { "title" => "Developer" }, "user" => { "username" => "sikachu", "person_attributes" => { "title" => "Developer" } })
       end
     end
+  ensure
+    User.singleton_class.undef_method(:nested_attributes_options)
   end
 end
 

actionview/test/template/log_subscriber_test.rb

@@ -22,7 +22,7 @@ def setup
 
     unless Rails.respond_to?(:root)
       @defined_root = true
-      def Rails.root; :defined_root; end # Minitest `stub` expects the method to be defined.
+      Rails.define_singleton_method(:root) { :defined_root } # Minitest `stub` expects the method to be defined.
     end
   end
 

activemodel/lib/active_model/lint.rb

@@ -30,7 +30,7 @@ module Tests
       # of the model, and is used to a generate unique DOM id for the object.
       def test_to_key
         assert_respond_to model, :to_key
-        def model.persisted?() false end
+        def_method(model, :persisted?) { false }
         assert model.to_key.nil?, "to_key should return nil when `persisted?` returns false"
       end
 
@@ -45,8 +45,8 @@ def model.persisted?() false end
       # any of the possible implementation strategies on the implementer.
       def test_to_param
         assert_respond_to model, :to_param
-        def model.to_key() [1] end
-        def model.persisted?() false end
+        def_method(model, :to_key) { [1] }
+        def_method(model, :persisted?) { false }
         assert model.to_param.nil?, "to_param should return nil when `persisted?` returns false"
       end
 
@@ -105,6 +105,10 @@ def test_errors_aref
       end
 
       private
+        def def_method(receiver, name, &block)
+          ::Object.instance_method(:define_singleton_method).bind_call(receiver, name, &block)
+        end
+
         def model
           assert_respond_to @model, :to_model
           @model.to_model

activerecord/test/cases/associations/eager_test.rb

@@ -1026,11 +1026,6 @@ def test_polymorphic_type_condition
   end
 
   def test_eager_with_multiple_associations_with_same_table_has_many_and_habtm
-    # Eager includes of has many and habtm associations aren't necessarily sorted in the same way
-    def assert_equal_after_sort(item1, item2, item3 = nil)
-      assert_equal(item1.sort { |a, b| a.id <=> b.id }, item2.sort { |a, b| a.id <=> b.id })
-      assert_equal(item3.sort { |a, b| a.id <=> b.id }, item2.sort { |a, b| a.id <=> b.id }) if item3
-    end
     # Test regular association, association with conditions, association with
     # STI, and association with conditions assured not to be true
     post_types = [:posts, :other_posts, :special_posts]
@@ -1754,4 +1749,10 @@ def test_preloading_has_many_through_with_custom_scope
     def find_all_ordered(klass, include = nil)
       klass.order("#{klass.table_name}.#{klass.primary_key}").includes(include).to_a
     end
+
+    # Eager includes of has many and habtm associations aren't necessarily sorted in the same way
+    def assert_equal_after_sort(item1, item2, item3 = nil)
+      assert_equal(item1.sort { |a, b| a.id <=> b.id }, item2.sort { |a, b| a.id <=> b.id })
+      assert_equal(item3.sort { |a, b| a.id <=> b.id }, item2.sort { |a, b| a.id <=> b.id }) if item3
+    end
 end

activerecord/test/cases/relations_test.rb

@@ -1730,7 +1730,7 @@ def test_only
 
   def test_anonymous_extension
     relation = Post.where(author_id: 1).order("id ASC").extending do
-      def author
+      def author # rubocop:disable Lint/NestedMethodDefinition
         "lifo"
       end
     end

activesupport/test/concern_test.rb

@@ -201,9 +201,9 @@ def test_prepended_and_included_class_methods
     prepended = Module.new.extend(ActiveSupport::Concern)
 
     @klass.class_eval { @foo = [] }
-    included.class_methods { def foo; @foo << :included; end }
+    included.class_methods { def foo; @foo << :included; end } # rubocop:disable Lint/NestedMethodDefinition
     @klass.class_eval { def self.foo; super; @foo << :class; end }
-    prepended.class_methods { def foo; super; @foo << :prepended; end }
+    prepended.class_methods { def foo; super; @foo << :prepended; end } # rubocop:disable Lint/NestedMethodDefinition
 
     @klass.include included
     @klass.prepend prepended

activesupport/test/lazy_load_hooks_test.rb

@@ -128,7 +128,7 @@ def test_hook_with_yield_true_afterward
 
   def test_hook_uses_class_eval_when_base_is_a_class
     ActiveSupport.on_load(:uses_class_eval) do
-      def first_wrestler
+      def first_wrestler # rubocop:disable Lint/NestedMethodDefinition
         "John Cena"
       end
     end
@@ -142,7 +142,7 @@ def first_wrestler
   def test_hook_uses_class_eval_when_base_is_a_module
     mod = Module.new
     ActiveSupport.on_load(:uses_class_eval2) do
-      def last_wrestler
+      def last_wrestler # rubocop:disable Lint/NestedMethodDefinition
         "Dwayne Johnson"
       end
     end
@@ -157,7 +157,7 @@ def last_wrestler
 
   def test_hook_uses_instance_eval_when_base_is_an_instance
     ActiveSupport.on_load(:uses_instance_eval) do
-      def second_wrestler
+      def second_wrestler # rubocop:disable Lint/NestedMethodDefinition
         "Hulk Hogan"
       end
     end