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

Bump RuboCop to 0.58.2 #33278

Merged
merged 1 commit into from
Jul 26, 2018
Merged

Conversation

koic
Copy link
Contributor

@koic koic commented Jul 3, 2018

Summary

RuboCop 0.57.2 was released.
https://github.com/rubocop-hq/rubocop/releases/tag/v0.57.2

And rubocop-0-57 channel is available in Code Climate.
https://github.com/codeclimate/codeclimate/releases/tag/v0.75.0

In addition, the following changes are made in this PR.

  • Replace Custom cops with Rails cops
  • Add jaro_winkler gem to Gemfile.lock
  • Suppress Lint/Syntax errors

Replace Custom cops with Rails cops

These are compatible replacements.

  • Replace CustomCops/AssertNot cop with Rails/AssertNot cop.
  • Replace CustomCops/RefuteNot cop with Rails/RefuteMethods cop.

With this replacement, it was decided to use cop of RuboCop itself. It removes the code related to CustomCops accordingly.

cc @composerinteralia

Add jaro_winkler gem to Gemfile.lock

Since RuboCop 0.57.0 depends on jaro_winkler gem, it has been added to Gemfile.lock.

Suppress Lint/Syntax errors

There are several Lint/Syntax errors in the following code.
Parentheses are added to prevent this error.

-check_open = -> *args do
+check_open = ->(*args) do

Below is a partial error log.

% bundle exec rubocop railties/test/generators/plugin_generator_test.rb
Inspecting 1 file
E

Offenses:

railties/test/generators/plugin_generator_test.rb:743:27: E:
Lint/Syntax: unexpected token kDO
(Using Ruby 2.4 parser; configure using TargetRubyVersion parameter,
under AllCops)
    check_open = -> *args do
                          ^^
railties/test/generators/plugin_generator_test.rb:756:85: E:
Lint/Syntax: unexpected token kDO_LAMBDA
(Using Ruby 2.4 parser; configure using TargetRubyVersion parameter,
under AllCops)
    generator([destination_root], template: path).stub(:open,
    check_open, template) do
                                                                                    ^^
railties/test/generators/plugin_generator_test.rb:768:3: E: Lint/Syntax:
unexpected token kEND
(Using Ruby 2.4 parser; configure using TargetRubyVersion parameter,
under AllCops)
  end
  ^^^
railties/test/generators/plugin_generator_test.rb:804:1: E: Lint/Syntax:
unexpected token kEND
(Using Ruby 2.4 parser; configure using TargetRubyVersion parameter,
under AllCops)
end
^^^

1 file inspected, 4 offenses detected

@rails-bot
Copy link

r? @schneems

(@rails-bot has picked a reviewer for you, use r? to override)

@kamipo
Copy link
Member

kamipo commented Jul 3, 2018

Suppress Lint/Syntax errors

There are several Lint/Syntax errors in the following code.
Parentheses are added to prevent this error.

-check_open = -> *args do
+check_open = ->(*args) do

Lint/Syntax errors are not Ruby's error but parser gem's error, -> *args do is still valid Ruby code which raise no warnings.
We don't want to change our code base to suppress RuboCop's issues.

@koic
Copy link
Contributor Author

koic commented Jul 3, 2018

Thank you for the review.
I get the point. I reverted it to the original code base about parser gem's error,

Copy link
Member

@sikachu sikachu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code change looks good after you revert the *args change.

However, looks like we now have 12 Code Climate issues. Do we need to disable a cop, or is there a way we can mitigate those?

# Prefer assert_not_x over refute_x
CustomCops/RefuteNot:
Rails:
Enabled: true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't want to enable a whole category of checks here -- we should just enable the individual ones we want.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK, the behavior of Rails department is special.

Rails department is disabled by default. And enabling Rails department does not enable all Rails cops. So only specified Rails cops will be enabled. Here Rails/AssertNot cop and Rails/RefuteMethods cop are enabled.

The following is quoted from the official document.
https://github.com/rubocop-hq/rubocop/blob/v0.57.2/manual/configuration.md#enabled

The exception to the rule is the Rails department, which can not be enabled
in its entirety this way. Setting Rails: Enabled: true will have the same
effect as running with the --rails command line option, which in the context
of DisabledByDefault: true means to make it possible to enable Rails cops
individually.

AllCops:
  EnabledByDefault: true

@koic
Copy link
Contributor Author

koic commented Jul 3, 2018

Do we need to disable a cop, or is there a way we can mitigate those?

This problem seems to have already been solved in parser gem.

However, it seems that this problem is occurring, as it has not been released yet.

I'm not sure why this problem started again in Code Climate. This is speculation, perhaps it may be occurring just after updating the rubocop-channel.

What we can do is to ask for the release of parser gem, but it may take time to deploy to Code Climate.

@koic
Copy link
Contributor Author

koic commented Jul 11, 2018

RuboCop 0.58 has been released. And Lint/Syntax error is fixed with latest Parser gem 2.5.1.2.

I will update this PR when Code Climate supports RuboCop 0.58.

@aried3r
Copy link
Contributor

aried3r commented Jul 12, 2018

@koic
Copy link
Contributor Author

koic commented Jul 25, 2018

Upgrading was suspended due to the following regression up to RuboCop 0.58.1.

% bundle exec rubocop actionview/test/template/form_helper/form_with_test.rb
Inspecting 1 file
C

Offenses:

actionview/test/template/form_helper/form_with_test.rb:453:9: C: Layout/IndentationWidth: Use 2 (not -2) spaces for indentation.
        "That would be great."
        ^^
actionview/test/template/form_helper/form_with_test.rb:457:9: C: Layout/IndentationWidth: Use 2 (not -2) spaces for indentation.
        "I believe you have my stapler."
        ^^

1 file inspected, 2 offenses detected

RuboCop 0.58.2 containing rubocop/rubocop#6103 that fixes this has been released, so I will try the upgrade. Thank you.

@koic koic changed the title Bump RuboCop to 0.57.2 Bump RuboCop to 0.58.2 Jul 25, 2018
@koic
Copy link
Contributor Author

koic commented Jul 25, 2018

Unfortunately, it seems that the behavior of RuboCop in Code Climate is old. I think I will ask a question later to Code Climate.

## Summary

RuboCop 0.58.2 was released.
https://github.com/rubocop-hq/rubocop/releases/tag/v0.58.2

And rubocop-0-58 channel is available in Code Climate.
https://github.com/codeclimate/codeclimate/releases/tag/v0.76.0
codeclimate/codeclimate@38f21f0

In addition, the following changes are made in this PR.

- Replace Custom cops with Rails cops
- Add jaro_winkler gem to Gemfile.lock

### Replace Custom cops with Rails cops

These are compatible replacements.

- Replace `CustomCops/AssertNot` cop with `Rails/AssertNot` cop.
- Replace `CustomCops/RefuteNot` cop with `Rails/RefuteMethods` cop.

With this replacement, it was decided to use cop of RuboCop itself.
It removes the code related to CustomCops accordingly.

### Add jaro_winkler gem to Gemfile.lock

Since RuboCop 0.57.0 depends on jaro_winkler gem,
it has been added to Gemfile.lock.
@koic
Copy link
Contributor Author

koic commented Jul 26, 2018

RuboCop 0.58.2 is enabled with Code Climate and warning is zero 💚

@georgeclaghorn georgeclaghorn merged commit 5f92e43 into rails:master Jul 26, 2018
@koic koic deleted the bump_rubocop_to_0_57_2 branch July 26, 2018 19:06
toshimaru added a commit to toshimaru/rubocop-rails_config that referenced this pull request Jul 26, 2018
koic added a commit to koic/oracle-enhanced that referenced this pull request Jul 27, 2018
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 this pull request may close these issues.

8 participants