Add MaxLength parameter to IfUnlessModifier and WhileUntilModifier

[agrimm]

Currently, if you have a high value for Max in LineLength, then the IfUnlessModifier and WhileUntilModifier cops will suggest a modifier style, even if it results in a very long line.

This change means you can specify a maximum length for these specific cops.

[yujinakayama]

👍 Actually I've been thinking same thing. :)

[yujinakayama]

I think the parameter name MaxLength is a bit ambiguous.

IfUnlessModifier:
  MaxLength: 80 # Length of what?

[bbatsov]

Yep, MaxConditionLineLength (or something similar) makes more sense.

[bbatsov]

I also wonder if it makes sense to add some config regarding the complexity of the condition - this could make the cop accept conditions with && and || regardless of other factors.

# this would normally be an offence, but it has an `&&` in it
if something && something_else
  do_something
end

This is based on an observation of mine that some people like to use the modifier forms only for conditions that consist of a single expression.

[jonas054]

MaxLineLength would be enough in my opinion. The cop wants the conditional statement to be rewritten as one line, so it's pretty clear what length the parameter refers to. Especially if we write a comment about it. Which leads me to my additional concern.

This is the first parameter we introduce that doesn't have a default value. This means that

• it's not visible in default.yml
• it's not included in the output from rubocop --show-cops

I'm thinking about where to describe it. The parameter is a bit hidden. One solution would be to set a default value of 79 in config/default.yml. Or a special value such as "LineLength/Max" or null to indicate that it gets its value from another cop.

[bbatsov]

I think that a default value of 79 is a good enough (and pretty simple) solution.

[agrimm]

I'm happy with a more verbose parameter name, such as MaxLineLength.

I agree that 79 as a default value is a good enough solution.

I won't be able to implement && or || - I'll leave that to someone more familiar with the code.

[bbatsov]

@agrimm OK. Let me you know when you've updated the PR.

[yujinakayama]

👍 for MaxLineLength.

[bbatsov]

@agrimm Ping :-)

[agrimm]

Been a bit busy, but my Ukrainian revolution is finished now. :)

An updated PR should be ready by the end of this weekend.

[mockdeep]

Ha, I was just going to open an issue for this. I usually do single line, but wrap it into a block if it's over 80 characters.

[agrimm]

@bbatsov Pong :-)

[bbatsov]

Thanks!