TooManyMethods false positive with nested classes

[eloyesp]

I'm having false positive with TooManyMethods, with a code that looks like this:

class WeeklyReminder
  attr_reader :user, :week, :result_set, :dates

  WeekPerformance = Struct.new :week, :submitted, :missing do
    def multiple_methods_here
    end
  end

  def other_methods_here
  end
end

# => TooManyMethods: WeeklyReminder has at least (multiple_methods + other_methods_here) methods.

Seems that reek does not catch the context switch when defining methods on the Struct.

Is it possible for it to detect it or should I add a magic comment (or may be just define the struct in other way).

[mvz]

Hi @eloyesp, thanks for reporting this. We do intend to support such code in Reek, so this is a bug.

[eloyesp]

I were checking a little bit, but I lack experience with the parser library.

It seems that the AST that should be detected is something like:

s(:casgn, nil, :WeekPerformance,
  s(:block,
    s(:send,
      s(:const, nil, :Struct), :new,
      # struct args to ignore
    s(:args),
    s(:begin,
      # everithing inside is another class (so defs should count towards other class)

But I'm not sure how to implement that.

[mvz]

I'll take a look in the coming week. I think we already handle this construction elsewhere, so it shouldn't be too hard to fix this.

[JuanVqz]

Hi! I tried to reproduce it locally but seems to not raise anything.

Am I missing something? or can we close the issue?

[troessner]

@JuanVqz check out the config for TooManyMethods. I think you just need to put more dummy methods in your example and then this should indeed be falsely reported (since we never fixed that bug)