Rubocop: Don't penalize for multiline hashes in MethodLength Cop

[ABaldwinHunter]

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

I and some dev friends have been frustrated by having to disable the MethodLength check on methods that are too long because they include a multiline hash or array.

While we value the single responsibility enforcement of the MethodLength cop, we find using multiline hashes increases readability. For us, a method that has a multline hash might be v simple, only containing that object:

# in a creation model

def user_params
  {
    age: 28,
    name: "Avery",
    preference_foo: default,
    ...
  }
end

# or in controller

def creation_params
  params.require(:user).permit(
    email,
    name,
    phone_number,
    ...
  )
end

# or a presentation constant

PIZZA_DISPLAY_NAMES = {
  egg_n_bacon: "Egg n' Bacon",
  sopressata: "Sopressata picante",
  margherita: "Double margherita",
  truffle_prosciutto: 'Truffle prosciutto',
 ...
}

Describe the solution you'd like

I think it could be valuable to be able to use the MethodLength cop but not have it penalize where a method was too long _only_ because of the multiline hash.

Proposal: increment method length count one line for a multiline hash. That way, we could still apply the method length cop to methods containing a multiline hash (say that had +n lines outside of it) without having the hash alone trigger the cop.

If there are mixed feelings, maybe the preference to treat multiline hashes and arrays as one increment could be an option on the MethodLength cop.

(WRT incrementing class length, I've noticed this issue less and feel neutral. Could see an argument for having this configuration option there too though)

Describe alternatives you've considered

Also considered

• having method length cop ignore methods with multi line hashes entirely (this might miss too much though IMO)

Additional context

I looked a little in previous issues here and didn't think I saw this one. But apologies if I missed a discussion! I'd love to help build this feature if there's interest in adopting it from other folks too, but wanted to validate that first with an issue. Thanks!

[KNejad]

I agree completely. I would also say that the exact same logic should apply to heredoc strings. For example the below should only count as one line:

some_complex_query = <<~SQL
SELECT ...
...
SQL

In general I would say that rubocop should consider lines in the same way that simplecov does

[marcandre]

Without expressing an opinion on the request: have you tried using other complexity/size metrics instead (e.g. Metrics/AbcSize or CyclotomicComplexity)? See #7204 for a discussion about "PerceivedMethodLength" which this sounds similar.

[KNejad]

@marcandre I've always seen Rubocop as more of a set of default guidelines for all projects first and a tool for enforcing those guidelines second. By commenting and up-voting this issue, it's not so much me saying "This is annoying me in Rubocop, and I want feature X" but more, that "I think the default 'best practices' should be X". If it comes down to it I will disable the cop and only use AbcSize (maybe) but I would rather make my case for why I think the default behaviour should be that it acts a certain way (i.e. by default I shouldn't be given a warning for having a 20 line method if 15 of them are a single heredoc)

[marcandre]

I would agree that it's not clear what MethodLength really adds if one has the complexity metrics enabled.

[fatkodima]

I think, this should be closed due to this - https://github.com/rubocop-hq/rubocop/pull/8159

[ABaldwinHunter]

Hooray! thank you! this is quite exciting. @marcandre to answer your question that I missed from before

Without expressing an opinion on the request: have you tried using other complexity/size metrics instead (e.g. Metrics/AbcSize or CyclotomicComplexity)? See #7204 for a discussion about "PerceivedMethodLength" which this sounds similar.

I think the line length metrics is one of the strongest complexity metrics because of its simplicity to understand. It's also a rule of thumb that generally encourages compliance with the other complexity rules.

I've seen many complexity checks used in conjunction (not sure that's ideal). Not to flip the question but it seems akin to asking why ever use more than one complexity check? (which rubocop default suggests, and most of us do. This requires a longer reflection for me than why I like the method length check above :))

Anyway thank you for the work and the change! 🚀 🚀

P.S. Perceived _Complexity_, is also one of my favorites. (I've read sonar source's white paper on cognitive complexity, which is similar and +1)