Detect usages of methods.include? and suggest respond_to? instead

[jacobobq]

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

We've found some instances of bad performance related to calling include? directly over the result of Kernel#methods. This can be answered more efficiently by calling Object#respond_to?.

Here's a benchmark for the difference in my system. dummies is an array of 1000 empty structs, while methods is an array of 1000 randomly generated symbols 5 lowercase letter characters in length.

Benchmark.bm do |x|
  x.report { dummies.map { |dummy| dummy.methods.include?(methods.sample) } } 
  x.report { dummies.map { |dummy| dummy.respond_to?(methods.sample, false) } }  
  x.report { set = Set.new(dummies.first.methods); dummies.map { |dummy| set.include?(methods.sample) } }  
end

       user     system      total        real
   0.009405   0.001026   0.010431 (  0.011214)
   0.000393   0.000872   0.001265 (  0.001265)
   0.000134   0.000169   0.000303 (  0.000303)

The difference between calling methods.include? and using respond_to? accounts for a full order of magnitude in real time. Similarly, it's shown that if you are repeatedly checking for different methods over the same class, creating and querying a set with the result of methods will provide an additional performance boost.

For the reasons stated above, I think it'd be useful to detect cases where this happens and provide a warning to programmers.

Describe the solution you'd like

A rubocop cop that detects methods.include?(:x) and public_methods.include?(:x) and suggests respond_to?(:x, true) and respond_to?(:x, false) respectively.

Describe alternatives you've considered

I cannot think of alternative solutions besides not doing this.

Additional context

The system where I run the benchmark was a MacBook Pro with the Apple M2 chip and 16 GB of RAM memory. The Ruby version was Ruby 3.3.3.

[Earlopain]

This makes sense to me. I recently saw a PR in rails (rails/rails#52449 (comment)) about the same thing with instance methods.

The same thing can also apply for constants => const_defined?, class_variables => class_variable_defined?, *class_methods => *class_method_defined?, though I have neither tested performance nor checked if there are subtle differences between these versions.

It would be nice if this can also regognize [foo.methods + foo.private_methods].include? => foo.method_defined? || foo.private_method_defined? but for a first implementation not necessary.