Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

'Rails/Blank' + 'Style/UnlessElse' cops result in wrong boolean expression #6785

Closed
themilkman opened this issue Feb 21, 2019 · 2 comments · Fixed by #6876
Closed

'Rails/Blank' + 'Style/UnlessElse' cops result in wrong boolean expression #6785

themilkman opened this issue Feb 21, 2019 · 2 comments · Fixed by #6876

Comments

@themilkman
Copy link
Contributor

Hi,

I used the rubocop autocorrect for the first time today. I am really impressed (by rubocop in general). But I managed to get in a situation where result is wrong (although using the safe-variant). My command was:

rubocop --safe-auto-correct -c .rubocop.yml app/ lib/

It seems that the 'Rails/Blank' and 'Style/UnlessElse' cops don't work together.
Are they meant to be? If not, at least a warning would be nice (and yes, reviewing automated changes is advisable in most cases, I know).

Thanks for any response!

# original
unless User.current.present?
  # do some stuff
  redirect_to :login
else
  redirect_to :internal_route
end

# rubocop result
if User.current.blank?
  redirect_to :internal_route
else
  # do some stuff
  redirect_to :login
end

# original
unless user.present?
  redirect_to :login
else
  redirect_to :internal_route
end

# rubocop result
if user.blank?
  redirect_to :internal_route
else
  redirect_to :login
end

My rubocop.yml:

AllCops:
  TargetRubyVersion: 2.6
  Exclude:
    - db/schema.rb

Rails:
  Enabled: true

Style/ClassAndModuleChildren:
  Enabled: false
Style/FrozenStringLiteralComment:
  Enabled: false
Style/RedundantReturn:
  Enabled: false
Style/RedundantSelf:
  Enabled: false
Style/ConditionalAssignment:
  Enabled: false

Metrics/LineLength:
  Max: 120
Metrics/MethodLength:
  Max: 25
Metrics/ClassLength:
  Max: 150
Metrics/AbcSize:
  Max: 20
Metrics/BlockLength:
  Max: 30

Layout/EmptyLinesAroundClassBody:
  Enabled: false
Layout/EmptyLinesAroundModuleBody:
  Enabled: false
Layout/AlignHash:
  EnforcedColonStyle: table
  EnforcedHashRocketStyle: table

Steps to reproduce the problem

CnP the code in a file and run 'rubocop --safe-auto-correct -c .rubocop.yml FILE.rb' with my rubocop.yml.

RuboCop version

Tested with ruby 2.6.1 and rubocop 0.63.1 and 0.65.0

0.65.0 (using Parser 2.6.0.0, running on ruby 2.6.1 x86_64-linux)
@rrosenblum
Copy link
Contributor

This issue will apply to both Rails/Blank and Rails/Present.

Thanks for the bug report. The auto-correction of cops is typically only flagged as unsafe once we have a reasonable reason to do so.

The issue here happens because both cops register independent offenses and auto-correct independently. I think we can make Rails/Blank and Rails/Present aware of if Style/UnlessElse is enabled and avoid registering an offense for this situation. Essentially, disable the option for UnlessPresent and UnlessBlank if Style/UnlessElse is activated.

@themilkman
Copy link
Contributor Author

Works like a charm, thank you very much @rrosenblum !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants