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/OptionalBooleanParameter offenses are listed as auto-correctable #8755

Closed
jafuentest opened this issue Sep 21, 2020 · 9 comments
Closed
Labels

Comments

@jafuentest
Copy link

Here's the output I got running rubocop after updatingg

Offenses:

app/helpers/cards_helper.rb:3:21: C: Style/OptionalBooleanParameter: Use keyword arguments when defining method with boolean argument.
  def preview(item, full_width = false)
                    ^^^^^^^^^^^^^^^^^^
app/helpers/input_helper.rb:11:69: C: Style/OptionalBooleanParameter: Use keyword arguments when defining method with boolean argument.
  def material_switch(form, field_name, value, checked_value = nil, checked = false)
                                                                    ^^^^^^^^^^^^^^^
app/helpers/material_icons_helper.rb:19:36: C: Style/OptionalBooleanParameter: Use keyword arguments when defining method with boolean argument.
    def self.icon_join(icon, text, reverse_order = false)
                                   ^^^^^^^^^^^^^^^^^^^^^

57 files inspected, 3 offenses detected, 3 offenses auto-correctable

Expected behavior

n offenses auto-correctable should display the correct amount of offenses that can be fixed with rubocop -a

Actual behavior

It's listing all offenses as auto-correctable

Steps to reproduce the problem

I'm not sure, I think it's specific to the Style/OptionalBooleanParameter cop, so it should be enough to define a method like def do_something(bad = true)

RuboCop version

0.91.0 (using Parser 2.7.1.4, rubocop-ast 0.4.2, running on ruby 2.6.6 x86_64-linux)

@marcandre marcandre added the bug label Sep 21, 2020
@marcandre
Copy link
Contributor

Odd. Style/OptionalBooleanParameter does not auto-correct.

See also #8757 ?

@dvandersluis
Copy link
Member

@jafuentest what's in your .rubocop.yml? I could not duplicate this, whether I run rubocop -a or rubocop -A it doesn't tell me anything is auto-correctable:

$ bundle exec rubocop test.rb -A
Inspecting 1 file
C

Offenses:

test.rb:3:18: C: Style/OptionalBooleanParameter: Use keyword arguments when defining method with boolean argument.
def do_something(bad = true)
                 ^^^^^^^^^^

1 file inspected, 1 offense detected

@jafuentest
Copy link
Author

Sorry, I forgot about it, here's my config:

require:
  - rubocop-performance
  - rubocop-rails

AllCops:
  NewCops: enable

  Exclude:
    # This module is soon to be removed
    - 'app/models/user_mixins/statistics.rb'

    # Old rake tasks that exists only for future reference
    - 'doc/**/*'

    # Auto-generated binstubs
    - 'bin/*'

    # Temp files
    - 'tmp/**/*'

    # vendor dir is used by most CI services
    - 'vendor/**/*'

  TargetRubyVersion: 2.6.6

Layout/LineLength:
  Max: 100

Layout/MultilineMethodCallIndentation:
  EnforcedStyle: indented

Layout/MultilineOperationIndentation:
  EnforcedStyle: indented

Metrics/AbcSize:
  Exclude:
    - config/routes/*.rb # Routes

Metrics/MethodLength:
  Exclude:
    - config/routes/*.rb # Routes

Metrics/BlockLength:
  Exclude:
    - config/routes/*.rb # Routes
    - config/routes.rb   # Routes
    - lib/tasks/*/*
  ExcludedMethods:
    - configure # Configurations
    - context   # Specs
    - describe  # Specs
    - namespace # Tasks
    - task      # Tasks
    - included  # Model and controller concerns
    - setup     # Devise

Metrics/ModuleLength:
  Exclude:
    - config/routes/*.rb # Routes

Rails/FindEach:
  Enabled: false

Rails/HasAndBelongsToMany:
  Enabled: false

Rails/SkipsModelValidations:
  Enabled: false

Style/ClassAndModuleChildren:
  EnforcedStyle: compact

Style/Documentation:
    Enabled: false

Style/FrozenStringLiteralComment:
  Enabled: false

Style/RegexpLiteral:
  EnforcedStyle: mixed

@dvandersluis
Copy link
Member

@jafuentest I tried again using your config and still cannot replicate what you're seeing. Maybe can you make a repo with a minimum replication case?

@hatkyinc2
Copy link
Contributor

@jafuentest Does running -C false for no cache and/or deleting the cache in ~/.cache/rubocop change the behaviour for you?..

@jafuentest
Copy link
Author

Ok so here's what I found. I tried again and couldn't replicate the error, but also noticed I had an older version (I guess I discarded the changes to Gemfile.lock because of this). So went and updated again to 0.91, and then it started happening again

91 files inspected, 8 offenses detected, 8 offenses auto-correctable Only 6 of those 8 are actually auto-correctable.

Tried with rubocop -C false and I got the expected message:
91 files inspected, 8 offenses detected, 6 offenses auto-correctable

@hatkyinc2
Copy link
Contributor

hatkyinc2 commented Oct 3, 2020

👍
So this is related to the cache issue I found
I have this patch #8841
If you would like to test it, you can checkout that repo&branch, remove your local rubocop versions with gem uninstall rubocop, in the checked out version run rake install, clean your cache with like rm -R ~/.cache/rubocop, than run rubocop as normal and it should just work 🤞
If you do get to test it, be nice if you put like a 👍 and comment on it ;)

This issue, due to cache, happens on the 2nd and further run. It applies to any rubocop from 0.90.0 and up, cause that's when the auto-correctable text was added

I'm not sure what does cache invalidation and such, but 2 consecutive runs with nothing else for sure uses it..

Also noted this as a feature to let us know about cache usage #8842, feel free to support that as well.
The thesis being that you should have had some text that shows the difference between the runs that relate to the cache being used.

@hatkyinc2
Copy link
Contributor

hatkyinc2 commented Oct 3, 2020

This is a copy paste version for testing the patch

gem uninstall rubocop
git clone git@github.com:hatkyinc2/rubocop.git
cd rubocop
git checkout hatkyinc2:Fix-8804-cache-save-status
rake install
rm -Rf ~/.cache/rubocop

Use rubocop as before

cd -
rubocop
rubocop

To return later to mainstream rubocop

gem uninstall rubocop
bundle
# or
gem install rubocop

@koic
Copy link
Member

koic commented Oct 6, 2020

This issue will be closed due to #8841 has been merged.

@koic koic closed this as completed Oct 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants