Lint/NestedMethodDefinition does not know about ActiveRecord association blocks

[philayres]

A standard ActiveRecord association can accept a block, such as this example. If the association is applied to a class dynamically, within a method (rather than at the top level on class creation), Lint/NestedMethodDefinition is reported.

def somemethod
  has_many :employees do
    def find_or_create_by_name(name)
      first_name, last_name = name.split(" ", 2)
      find_or_create_by(first_name: first_name, last_name: last_name)
    end
  end
end

( adapted from https://apidock.com/rails/ActiveRecord/Associations/ClassMethods/has_many )

I would expect this not to fail. Although I have not tested, I expect all ActiveRecord associations fail in the same way.

---

Expected behavior

The above should pass.

This appears to be similar to #4185 and possibly others. Is it in fact correct that anything with the following structure should pass?:

def somemethod
  blah do
    def good_or_bad?
      puts 'something'
    end
  end

Actual behavior

The above fails

RuboCop version

0.92.0 (using Parser 2.7.1.5, rubocop-ast 0.7.1, running on ruby 2.6.6 x86_64-linux)

[bbatsov]

I get the problem, but it's still a problem as you're nesting a definition results in some misleading assumption about it's scope. The fact that you can do something doesn't mean it's a good idea. :-)

If you example the "nested" method definition will actually be pulled to the containing class definition.

[philayres]

Will it really be pulled to the containing class definition? I'm not sure I understand.

If that is the case, ActiveSupport Concerns would have the same issue, right? This would mean that foo is really defined as an instance method in the module in the following example?

module Foo
  extend ActiveSupport::Concern

  class_methods do
    def foo

    end
  end
end

Or is this just an issue with defs in blocks in a class?

I'll note that I don't get a cop complaining about Concerns. But equally, my original example with the Rails association does not appear to result in the method being defined at level of the outer class, but correctly as a method that can be called from the association.

[marcandre]

I think we should have a config for methods that are known to execute the block within a class_eval, and that list could have the rails has_many/one, class_methods, scope.

Right now, that list exists but is hardcoded to {:instance_eval :class_eval :module_eval}. Clearly, we're not removing these! Rails is so prevalent it should be considered as core.

[fatkodima]

@koic This can be closed after the merge of #11019.