Rubocop: Allow specific MaxLineLength for individual cops

Created on 5 Jul 2018  Â·  10Comments  Â·  Source: rubocop-hq/rubocop

The option for individual MaxLineLengths for specific cops used to be an option and was removed here: https://github.com/rubocop-hq/rubocop/commit/9e76387952725f1a7fa54989024114549c0b7d80#diff-262d848b3b481343a51bf597d6f4d574L44. I agree that removing it from default.yml is critical because that causes unexpected behavior, but removing the option entirely reduces RuboCop's flexibility.

In our company we set LineLength's Max setting to 89. The company's official guide states 80 characters, but passing RuboCop is a requirement for merging code, so we allow up to 89 to provide a little leeway and reduce resistance to following the style requirements.

However, because the official guide for the company is 80, we don't want other cops requiring changes that would force the line length over 80. Previously setting the MaxLineLength option for the IfUnlessModifier cop to 80 would prevent that cop flagging an offense on code that would be over 80 characters when converted to a single line modifier. Now I have no way to handle this situation.

Therefore I propose re-adding the following to StatmentModifier:

def max_line_length
  cop_config['MaxLineLength'] ||
    config.for_cop('Metrics/LineLength')['Max']
end

While keeping those options out of default.yml, so the default behaivor is to follow Max from LineLength.

If this is agreeable, I can provide a quick PR.


Expected behavior

No offense

Actual behavior

test/test_helper.rb:363:7: C: Style/IfUnlessModifier: Favor modifier if usage when having a single-line body. Another good alternative is the usage of control flow &&/||. (https://github.com/rubocop-hq/ruby-style-guide#if-as-a-modifier)
      if Time.now - wait_start > 2
      ^^

Steps to reproduce the problem

Have the following config:

LineLength: { Max: 89 }
Style/IfUnlessModifier: { MaxLineLength: 80 }

and the following code:

      if Time.now - wait_start > 2
        raise "Expected email to have been sent to #{address}"
      end

RuboCop version

➜  rubocop -V
0.57.2 (using Parser 2.5.1.0, running on ruby 2.3.1 x86_64-darwin16)
question

Most helpful comment

Since the default Max: for Layout/LineLength has been increased to 120, this would be very useful to avoid extra long lines with If/Unless modifiers.

All 10 comments

Updated to include the output of rubocop -V instead of rubocop -v

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!

Homebrew would still find this useful so just commenting so stale bot knows someone cares (although this could be combined with https://github.com/rubocop-hq/rubocop/issues/3534)

I would still love this as well

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!

I would still like to see this implemented

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!

A workaround is possible, as IgnoredPatterns defined in Layout/LineLength cop do not affect the other cops. In this configuration, Layout/LineLength will allow lines up to 89 characters long, but Style/IfUnlessModifier and other cops will treat the limit as 80 characters.

Layout/LineLength:
  Max: 80
  IgnoredPatterns:
    - '^.{,89}$'

Since the default Max: for Layout/LineLength has been increased to 120, this would be very useful to avoid extra long lines with If/Unless modifiers.

I agree with @mvz . Guard clauses (If/Unless modifiers) should be kept short, even if other lines are allowed to be long. They're inherently difficult to read, because the logic is out of order. If they get too long, they're like those trick quizzes substitute teachers give you, where the last step is to ignore all the previous steps.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

AndreiMotinga picture AndreiMotinga  Â·  3Comments

mlammers picture mlammers  Â·  3Comments

NobodysNightmare picture NobodysNightmare  Â·  3Comments

deivid-rodriguez picture deivid-rodriguez  Â·  3Comments

herwinw picture herwinw  Â·  3Comments