Style/IfUnlessModifier not reporting lines longer than MaxLineLength

[reitermarkus]

RuboCop auto-corrects if/unless-statements to use the modifier form if the line length after the correction will be less than MaxLineLength, but it doesn't actually work in the opposite direction, i.e. if the line is already in modifier form, but is longer than MaxLineLength.

---

Expected behavior

Lines that are using the modifier form which are longer than MaxLineLength are reported as offenses, and auto-corrected if specified.

Actual behavior

Lines that are using the modifier form which are longer than MaxLineLength are not reported as offenses, and hence, not auto-corrected.

Steps to reproduce the problem

Create a file which has line longer than MaxLineLength using the modifier form.

RuboCop version

$ rubocop -V
0.43.0 (using Parser 2.3.1.4, running on ruby 2.3.1 x86_64-darwin15)

[Drenmi]

I would venture to guess the reason for this is that the offense would actually be picked up in Style/MaxLineLength, and adding auto-correct to that cop would be more or less impossible, because it needs to take into consideration the configuration of all other cops in order to produce the right autocorrect.

Auto-correcting a MaxLineLength violation in another cop seems a bit dubious to me, at least at first glance, although I admit I've never given that option much consideration. 😀

[jonas054]

I've tried to think of situations that could become messy if Style/IfUnlessModifier and Metrics/LineLength fight over the same offense, but I can't come up with anything solid. I think it would be OK for Style/IfUnlessModifier to auto-correct modifler forms into the normal form if the line is longer than MaxLineLength.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution and understanding!

[MikeMcQuaid]

Homebrew would still find this useful so just commenting so stale bot knows someone cares.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution and understanding!

[maxim]

We just updated to rubocop with this change, and looks like we found an edge case. When you have an IfUnlessModifier line that exceeds maximum line length, then having it wrapped in

# rubocop:disable Metrics/LineLength

does not disable this cop's line length check. As a result, we get line length complaints from this cop, even with Metrics/LineLength disabled.

Since the documentation for this cop says "line length is configured in Metrics/LineLength", could it make sense for this cop then to automatically ignore line length if Metrics/LineLength is disabled?

/cc @jonas054

Edit: Just browsed the source, and looks like the code does attempt to handle this case, which makes me think this has got to be a bug… Rubocop version is 0.75.1. Errors are

Style/IfUnlessModifier: Modifier form of if makes the line too long.

[jfelchner]

@jonas054 I believe this line length check needs to be an option unless you can literally ask the LineLength cop if the line is valid. The LineLength cop allows for things such as regex patterns to ignore, etc which means that just checking for the MaxLineLength is insufficient. The rubocop:disable case above is just one edge case of a possible multitude.

[jonas054]

You're right, @jfelchner. I'd be tempted to move the check on modifier lines into the LineLengthcop if it wasn't for the fact that the updated IfUnlessModifier has been released, and users may have started to disable this offense locally because of the false positives in edge cases.

I'll try to fix it so that IfUnlessModifier asks LineLength if the line is too long. Probably through some shared code.