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

Style/UnlessLogicalOperators reporting false offenses #9551

Closed
Incanus3 opened this issue Mar 1, 2021 · 2 comments
Closed

Style/UnlessLogicalOperators reporting false offenses #9551

Incanus3 opened this issue Mar 1, 2021 · 2 comments
Labels

Comments

@Incanus3
Copy link

@Incanus3 Incanus3 commented Mar 1, 2021

After upgrading rubocop to 1.11.0 I started getting

C: Style/UnlessLogicalOperators: Do not use mixed logical operators in an unless.

on a line that reads

unless values[:client][:insurance_company] == 999

I don't think there's a logical operator in this condition (equality operator is a boolean operator, but not a logical one AFAIK) and they definitely aren't mixed.


Expected behavior

No offense reported.

Actual behavior

When running rubocop --debug I get the following output (filtered out lines starting with "Scanning" and "Loading cache"):

For /home/jakub/Projects/spital/covid-form-backend: configuration from /home/jakub/Projects/spital/covid-form-backend/.rubocop.yml
configuration from /home/jakub/.rbenv/versions/2.7.2/gemsets/covid/gems/rubocop-performance-1.10.0/config/default.yml
configuration from /home/jakub/.rbenv/versions/2.7.2/gemsets/covid/gems/rubocop-performance-1.10.0/config/default.yml
Default configuration from /home/jakub/.rbenv/versions/2.7.2/gemsets/covid/gems/rubocop-1.11.0/config/default.yml
configuration from /home/jakub/.rbenv/versions/2.7.2/gemsets/covid/gems/rubocop-rake-0.5.1/config/default.yml
configuration from /home/jakub/.rbenv/versions/2.7.2/gemsets/covid/gems/rubocop-rake-0.5.1/config/default.yml
configuration from /home/jakub/.rbenv/versions/2.7.2/gemsets/covid/gems/rubocop-rspec-2.2.0/config/default.yml
configuration from /home/jakub/.rbenv/versions/2.7.2/gemsets/covid/gems/rubocop-rspec-2.2.0/config/default.yml
Inspecting 89 files
For /home/jakub/Projects/spital/covid-form-backend/app/persistence/migrations: configuration from /home/jakub/Projects/spital/covid-form-backend/app/persistence/migrations/.rubocop.yml
Inheriting configuration from /home/jakub/Projects/spital/covid-form-backend/.rubocop.yml
AllCops/Exclude configuration from /home/jakub/Projects/spital/covid-form-backend/.rubocop.yml
For /home/jakub/Projects/spital/covid-form-backend/spec: configuration from /home/jakub/Projects/spital/covid-form-backend/spec/.rubocop.yml
Inheriting configuration from /home/jakub/Projects/spital/covid-form-backend/.rubocop.yml
AllCops/Exclude configuration from /home/jakub/Projects/spital/covid-form-backend/.rubocop.yml
For /home/jakub/Projects/spital/covid-form-backend/spec/helpers: configuration from /home/jakub/Projects/spital/covid-form-backend/spec/helpers/.rubocop.yml
Inheriting configuration from /home/jakub/Projects/spital/covid-form-backend/spec/.rubocop.yml
Inheriting configuration from /home/jakub/Projects/spital/covid-form-backend/.rubocop.yml
AllCops/Exclude configuration from /home/jakub/Projects/spital/covid-form-backend/.rubocop.yml
.

Offenses:

app/web/validation/contracts.rb:64:13: C: Style/UnlessLogicalOperators: Do not use mixed logical operators in an unless.
            unless values[:client][:insurance_company] == 999 ...
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

89 files inspected, 1 offense detected
Finished in 0.4577370000188239 seconds

Steps to reproduce the problem

Create a file with the following contents:

unless values[:client][:insurance_company] == 999
  key.failure(Messages.must_not_be_shorter(than: 9))

  if (match = value.match(INSURANCE_NUMBER_REGEX))
    month = match[:month]

    if month < 1 || month > 12
      key.failure(Messages.not_a_valid_month(month))
    else
      puts 'whatever'
    end
  else
    puts 'whatever'
  end
end

and run rubocop on it with the rule enabled. The code is actually pretty nonsensical as I tried to get a minimal failing example while also avoiding triggering other offenses. The original code is here. The interesting thing here is that if I remove any of the nested blocks, rubocop stops reporting this offense. On the other hand, if I just commend something out, the offense persists.

RuboCop version

❯ bundle exec rubocop -V
1.11.0 (using Parser 3.0.0.0, rubocop-ast 1.4.1, running on ruby 2.7.2 x86_64-linux)
  - rubocop-performance 1.10.0
  - rubocop-rake 0.5.1
  - rubocop-rspec 2.2.0
@bbatsov bbatsov added the bug label Mar 1, 2021
koic added a commit to koic/rubocop that referenced this issue Mar 1, 2021
…tors`

Fixes rubocop#9551.

This PR fixes a false positive for `Style/UnlessLogicalOperators`
when using `||` operator and invoked method name includes "or"
in the conditional branch.

This is an issue due to string matching of source code instead of AST.
@bbatsov bbatsov closed this in #9552 Mar 1, 2021
bbatsov added a commit that referenced this issue Mar 1, 2021
Fixes #9551.

This PR fixes a false positive for `Style/UnlessLogicalOperators`
when using `||` operator and invoked method name includes "or"
in the conditional branch.

This is an issue due to string matching of source code instead of AST.
@Incanus3
Copy link
Author

@Incanus3 Incanus3 commented Mar 1, 2021

can confirm that this fixes the issue. thanks for the hard work guys.

@bbatsov
Copy link
Collaborator

@bbatsov bbatsov commented Mar 1, 2021

You're welcome!

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

Successfully merging a pull request may close this issue.

2 participants