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

Update to rubocop 0.76 #163

Merged
merged 18 commits into from Nov 13, 2019
Merged

Conversation

zachsabin
Copy link
Collaborator

@zachsabin zachsabin commented Nov 9, 2019

This will drop support for ruby 2.2 and add it for ruby 2.6. It's probably best reviewed commit by commit as I tried to bundle changes logically -- many cops switched files and that will be more clear if you look solely at commits.

I looked through the changelogs for rubocop up to 0.76.0 and the changelogs for rubocop-performance and rubocop-rails.

@@ -474,11 +472,6 @@ Layout/SpaceInsideBlockBraces:
EnforcedStyleForEmptyBraces: no_space
SpaceBeforeBlockParameters: true

Layout/SpaceInsideParens:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was just duplicated elsewhere in the file! It still exists, but the newer rubocop doesn't allow duplicated styles it seems.

@@ -344,7 +343,6 @@ Layout/MultilineMethodCallIndentation:
SupportedStyles:
- aligned
- indented
IndentationWidth:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

IndentationWidth isn't supported anymore, although I can't find in the rubocop changelog where exactly that happened.

Copy link
Contributor

Choose a reason for hiding this comment

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

The name of the cop is Layout/IndentationWidth. This line with bare IndentationWidth should probably never have been here, so you were right to remove it.

SupportedStyles:
- when_needed
- always
EnforcedStyle: always
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

when_needed does not appear to be allowed anymore.

Error: invalid EnforcedStyle 'when_needed' for Style/FrozenStringLiteralComment found in config/rubocop-style.yml
Valid choices are: always, never

@@ -328,11 +328,6 @@ Style/ExpandPathArguments:
Description: "Use `expand_path(__dir__)` instead of `expand_path('..', __FILE__)`."
Enabled: false

Style/FlipFlop:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Moved to rubocop-airbnb/config/rubocop-lint.yml

@@ -7,3 +7,6 @@ Gemspec/RequiredRubyVersion:
Enabled: false
Include:
- '**/*.gemspec'

Gemspec/RubyVersionGlobalsUsage:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is enabled by default -- is it worth explicitly enabling here or should we just use the rubocop default settings?

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's explicitly enable it here for clarity. I don't know where the Rubocop defaults are set but I see tons of enabled cops in our configs that are probably also enabled by default.

@@ -344,7 +343,6 @@ Layout/MultilineMethodCallIndentation:
SupportedStyles:
- aligned
- indented
IndentationWidth:
Copy link
Contributor

Choose a reason for hiding this comment

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

The name of the cop is Layout/IndentationWidth. This line with bare IndentationWidth should probably never have been here, so you were right to remove it.

EnforcedStyle: when_needed
SupportedStyles:
- when_needed
- always
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this stay here, but with - never and - always as the choices?

@@ -2,11 +2,6 @@
# They are custom built for use inside Airbnb and address issues that we have experienced in
# testing and production.

Airbnb/ClassName:
Copy link
Contributor

Choose a reason for hiding this comment

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

We can probably delete class_name.rb and class_name_spec.rb now that we're using Rubocop's ReflectionClassName cop instead.

@@ -551,3 +551,6 @@ Layout/TrailingWhitespace:
Description: Avoid trailing whitespace.
StyleGuide: https://github.com/rubocop-hq/ruby-style-guide#no-trailing-whitespace
Enabled: true

Layout/IndentFirstParameter:
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like the documented name is Layout/FirstParameterIndentation. Let's use that name instead of the alias to make it easier to find documentation.

https://github.com/rubocop-hq/rubocop/blob/a29694f7821ef51ca58528ca703bce9ec713716b/lib/rubocop/config_obsoletion.rb#L17

@@ -7,3 +7,6 @@ Gemspec/RequiredRubyVersion:
Enabled: false
Include:
- '**/*.gemspec'

Gemspec/RubyVersionGlobalsUsage:
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's explicitly enable it here for clarity. I don't know where the Rubocop defaults are set but I see tons of enabled cops in our configs that are probably also enabled by default.

@@ -227,7 +227,7 @@ Layout/FirstMethodParameterLineBreak:
Enabled: false

# Supports --auto-correct
Layout/FirstParameterIndentation:
Layout/IndentFirstArgument:
Copy link
Contributor

Choose a reason for hiding this comment

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

The new name is Layout/FirstArgumentIndentation.

@@ -238,7 +238,7 @@ Layout/FirstParameterIndentation:
- special_for_inner_method_call_in_parentheses

# Supports --auto-correct
Layout/IndentArray:
Layout/IndentFirstArrayElement:
Copy link
Contributor

Choose a reason for hiding this comment

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

The new name is Layout/FirstArrayElementIndentation.


# Supports --auto-correct
Layout/IndentHash:
Layout/IndentFirstHashElement:
Copy link
Contributor

Choose a reason for hiding this comment

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

The new name is Layout/FirstHashElementIndentation

Copy link
Collaborator Author

@zachsabin zachsabin Nov 13, 2019

Choose a reason for hiding this comment

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

I think all of these Indent -> Indentation name changes are in rubocop 0.77, which isn't released yet.

rubocop/rubocop#7468

@BMorearty
Copy link
Contributor

This is great. I would also like to see this change:

  • Disable Layout/EmptyLineAfterGuardClause (new cop in 0.58.3).

Copy link
Contributor

@BMorearty BMorearty left a comment

Choose a reason for hiding this comment

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

Looks like Layout/EmptyLineAfterGuardClause was already disabled and the class_name.rb cop was already removed.

LGTM!

@zachsabin zachsabin merged commit dcbc6dc into airbnb:master Nov 13, 2019
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.

None yet

2 participants