New cop `Rails/ClassDescendants` that warns developers when they use `Class.descendants`

[alexanderadam]

Is your feature request related to a problem? Please describe.

A wise man, let's call him Ben, once said:

There are two reasons why this is an unreliable solution:

• it doesn't know about things that have yet to be autoloaded

• it's non-deterministic with regards to Garbage Collection of classes. If you use Class.descendants in Test, where there is a pattern to dynamically define classes, GC is unpredictable for when those classes are cleaned up and removed by the GC.

And I totally trust him on that because he's making a… GoodJob 🥁

Describe the solution you'd like

A new cop Rails/ClassDescendants that warns developers when they use Class#descendants, explaining the potential issues.

Describe alternatives you've considered

Not being warned by a Cop I guess? 😆

Additional context

• New Cop: Avoid usage of Class.descendents github/rubocop-github#110 (comment)

[pirj]

it doesn't know about things that have yet to be autoloaded

What is an alternative? The self.inherited(subclass) hook? How do you know if the inheritor class is later GC'd or not?

it's non-deterministic with regards to Garbage Collection of classes. If you use Class.descendants in Test

What are the alternatives? What would we suggest using in production code for something like a RuboCop --auto-gen-config config generator given an open principle? Should cops register themselves?
Should RuboCop also provide a way to clean its conf when in a test a temporary cop wants to unlist itself?

Practical discussion rspec/rspec-mocks#1568 rspec/rspec-mocks#1570

cc @bensheldon

[bensheldon]

descendants is not unsafe, but it is unreliable when using autoloading or without intentional/explicit loading/lifecycle management.

My ideal would be that rubocop warns when descendants (or Object#subclasses) is used, and the developer would then be responsible for determining whether it is used correctly, and if so, would and a rubocop:disable to the usage along with a directive comment why it's okay.

...though I can understand if that's not a design-philosophy Rubocop subscribes to because there's not an alternative way of doing it that's better, it's just simply a problem I've seen repeatedly because people are not aware of it.

[Earlopain]

it's just simply a problem I've seen repeatedly because people are not aware of it.

Probably because it is not really documented. Rails for example doesn't mention any trouble with this method. As I wrote in rubocop/rails-style-guide#373 (comment), I would much rather see that improved.

In Ruby you can just modify stuff at runtime to your hearts content. That makes things like descendants naturally brittle, you have to make sure to call it at the right time.

[koic]

I'll close this proposal. For details, please see rubocop/rails-style-guide#373 and rubocop/rails-style-guide#373.