Rubocop: auto-gen-config generated Style/GuardClause Exclude doesn't seem to work

[cthiel]

Running rubocop --auto-gen-config generated a config like:

Style/GuardClause:
  Exclude:
    - 'app/controllers/some_controller.rb'

but running rubocop again with that config would still report the same error in some_controller.rb

Only setting

Style/GuardClause:
  Enabled: false

turned it off effectively. Exclude lists seem to work correctly for other Style/* cops.

[jonas054]

Do you have an Exclude property for Style/GuardClause set somewhere else, for example in .rubocop.yml?

[cthiel]

No.

[jonas054]

I've been trying to reproduce the problem, but have not been able to. @cthiel Can you create a minimal set-up that causes the problem?

[TimMoore]

I also have a problem like this. When I run rubocop --auto-gen-config, it generates a .rubocop_todo.yml with 7 offenders excluded:

# Offense count: 7
# Configuration parameters: MinBodyLength.
Style/GuardClause:
  Exclude:
    - 'app/controllers/exit_responses_controller.rb'
    - 'app/controllers/participant_controller.rb'
    - 'app/helpers/excel_helper.rb'
    - 'app/helpers/response_helper.rb'
    - 'lib/comments/comment_query.rb'
    - 'vendor/appamp/lib/controlamp/component.rb'

Then, if I immediately run rubocop again, it finds many more violations. It isn't immediately obvious what's different between the cases it picks up in --auto-gen-config and those it does not.

[TimMoore]

and it looks like I'm also having a similar problem with Style/ConditionalAssignment finding more violations after I ran with --auto-gen-config.

[TimMoore]

More information:

I have this problem with Rubocop 0.36 through 0.37.1, but 0.35.1 does not. That version detects 55 violations and disables the cop entirely.

The problem actually seems to be worse with 0.37.1 than 0.37.0: after updating and then running rubocop --auto-gen-config again, it _drops_ two files from the exclusion list, and then one of the two files is reported as a violation on the next normal run. (The other dropped file must have been a false positive fixed in 0.37.1.)

We're using TargetRubyVersion: 2.1. I'll try to come up with a reproducible test case.

[TimMoore]

I worked out what's happening here, but it's not totally clear how best to fix it.

The GuardClause cop has a condition: if replacing the conditional block with a guard clause would make the line longer than the Metrics/LineLength Max configuration, then it does not consider it an offence. This behaviour was newly introduced in 0.36.0 (#2527).

Because we have some long lines in our source code, our .rubocop_todo.yml has a large max line length (961 at the moment).

When running with --auto-gen-config, of course the .rubocop_todo.yml is ignored, and the default max line length of 80 characters is used.

I can't think of a good solution to this without an extensive change to the way Rubocop config works, from what I understand of it. @alexdowad do you have an opinion on this?

I can work around it in my project by setting a longer max line length in .rubocop.yml (I set it to 120). Then, it catches all of the same offences during --auto-gen-config as it does without it. Spoke too soon... of course that causes it to override the max line length in the todo file and then those checks fail.

[jonas054]

Excellent work, @TimMoore!

I seem to recall now that we've had similar problems before with the cops Style/IfUnlessModifier and Style/WhileUntilModifier. They now have their own MaxLineLength parameter (see #795), and I think it would solve the problem if we added such a parameter for Style/GuardClause and Style/Next too.

[TimMoore]

That's a good idea @jonas054. Thanks for the pointer.

I'll try to get a pull request together this weekend for this (and Style/ConditionalAssignment as well).

[TimMoore]

I've proposed a fix for GuardClause and ConditionalAssignment at #2839.

I noticed that Style/Next does not do any line length checking at the moment, so I've left that for a separate PR.

[madwort]

Just started having issues like this when updating to rubocop 0.43.0. Here's my test case:

$ cat test.rb 
def do_something
  if myvar
    puts 'stuff'
  end
end
$ bundle exec rubocop test.rb --auto-gen-config
Inspecting 1 file
C

Offenses:

test.rb:2:3: C: Use a guard clause instead of wrapping the code inside a conditional expression.
  if myvar
  ^^
test.rb:2:3: C: Favor modifier if usage when having a single-line body. Another good alternative is the usage of control flow &&/||.
  if myvar
  ^^

1 file inspected, 2 offenses detected
Created .rubocop_todo.yml.
Run `rubocop --config .rubocop_todo.yml`, or add `inherit_from: .rubocop_todo.yml` in a .rubocop.yml file.
$ cat .rubocop_todo.yml 
# This configuration was generated by
# `rubocop --auto-gen-config`
# on 2016-10-03 22:04:20 +0100 using RuboCop version 0.43.0.
# The point is for the user to remove these configuration records
# one by one as the offenses are removed from the code base.
# Note that changes in the inspected code, or installation of new
# versions of RuboCop, may require this file to be generated again.

# Offense count: 1
# Configuration parameters: MinBodyLength.
Style/GuardClause:
  Exclude:
    - 'test.rb'

# Offense count: 1
# Cop supports --auto-correct.
# Configuration parameters: MaxLineLength.
Style/IfUnlessModifier:
  Exclude:
    - 'test.rb'
$ bundle exec rubocop test.rb
Inspecting 1 file
C

Offenses:

test.rb:2:3: C: Use a guard clause instead of wrapping the code inside a conditional expression.
  if myvar
  ^^

1 file inspected, 1 offense detected
$ bundle exec rubocop -V
0.43.0 (using Parser 2.3.1.4, running on ruby 2.2.2 x86_64-linux)

Expected behaviour is that the final bundle exec rubocop passes without offenses.

Doesn't seem like it's to do with max line length, as the lines in question are pretty short?

[jonas054]

Did you see:

Run rubocop --config .rubocop_todo.yml, or add inherit_from: .rubocop_todo.yml in a .rubocop.yml file.

I don't think you've done that.