Skip to content

Use up-to-date Rubocop#83

Merged
joshpencheon merged 2 commits intomasterfrom
rubocop-for-ruby-3-0
Jan 6, 2021
Merged

Use up-to-date Rubocop#83
joshpencheon merged 2 commits intomasterfrom
rubocop-for-ruby-3-0

Conversation

@joshpencheon
Copy link
Contributor

@joshpencheon joshpencheon commented Jan 4, 2021

Summary

This PR relaxes the dependency on rubocop / rubocop-rails, in line with our policy of sticking with the wider Ruby community's shared consensus by default.

Additionally, we add rubocop-rake, which runs some basic sanity checks that are helpful given Rake's DSL doesn't always behave as might be expected. (These checks would've saved my bacon in the past!)

The driver here is that the version of Rubocop we're currently locked on blocks Ruby upgrade paths.

Other notes

It may also be worth investigating the rubocop-minitest extension as a separate PR.

Which includes better support for newer Ruby versions
@joshpencheon
Copy link
Contributor Author

Comparing before-vs-after outputs against a large project:

- 1029 files inspected, 4300 offenses detected
+ 1029 files inspected, 5491 offenses detected, 3058 offenses auto-correctable

Top 20

Before:

$ ag -o ': \w:( \[Correctable\])? \w+/\w+:' old.output | sed 's/\[Correctable\] //' | sort | uniq -c | sort -nr | head -n 20
    905 : C: Style/StringLiterals:
    450 : C: Style/Documentation:
    286 : C: Layout/LineLength:
    165 : C: Layout/EmptyLineAfterGuardClause:
    162 : C: Metrics/MethodLength:
    142 : C: Layout/TrailingWhitespace:
    123 : C: Style/PercentLiteralDelimiters:
    123 : C: Metrics/AbcSize:
    113 : C: Rails/Output:
     73 : C: Rails/BulkChangeTable:
     69 : C: Layout/HashAlignment:
     67 : C: Layout/ExtraSpacing:
     58 : C: Rails/ReversibleMigration:
     53 : C: Layout/SpaceAfterComma:
     52 : C: Rails/FilePath:
     49 : C: Layout/SpaceAfterColon:
     48 : C: Layout/ArrayAlignment:
     43 : E: Lint/Syntax:
     43 : C: Layout/IndentationConsistency:
     41 : C: Rails/HasManyOrHasOneDependent:

After:

$ ag -o ': \w:( \[Correctable\])? \w+/\w+:' new.output | sed 's/\[Correctable\] //' | sort | uniq -c | sort -nr | head -n 20
    905 : C: Style/StringLiterals:
    497 : C: Naming/VariableNumber:
    409 : C: Style/Documentation:
    286 : C: Layout/LineLength:
    165 : C: Layout/EmptyLineAfterGuardClause:
    162 : C: Metrics/MethodLength:
    150 : C: Layout/SpaceAroundMethodCallOperator:
    142 : C: Layout/TrailingWhitespace:
    139 : C: Metrics/AbcSize:
    123 : C: Style/PercentLiteralDelimiters:
    113 : C: Rails/Output:
    106 : W: Lint/DuplicateBranch:
     73 : C: Rails/BulkChangeTable:
     72 : C: Style/StringConcatenation:
     70 : C: Metrics/CyclomaticComplexity:
     69 : C: Layout/HashAlignment:
     61 : C: Style/RedundantRegexpCharacterClass:
     59 : C: Layout/ExtraSpacing:
     58 : C: Rails/ReversibleMigration:
     53 : C: Layout/SpaceAfterComma:

Looking at the significant new entrants:

TOP_K=20
comm -13 \
  <(ag -o ': \w:( \[Correctable\])? \w+/\w+:' old.output | sed 's/\[Correctable\] //' | sort | uniq -c | sort -nr | head -n $TOP_K | cut -d: -f 3 | sort) \
  <(ag -o ': \w:( \[Correctable\])? \w+/\w+:' new.output | sed 's/\[Correctable\] //' | sort | uniq -c | sort -nr | head -n $TOP_K | cut -d: -f 3 | sort)
 Layout/SpaceAroundMethodCallOperator
 Lint/DuplicateBranch
 Metrics/CyclomaticComplexity
 Naming/VariableNumber
 Style/RedundantRegexpCharacterClass
 Style/StringConcatenation

Of these, Naming/VariableNumber is the biggest swing (being a new introduction).

I tried also with the configuration:

Naming/VariableNumber:
  EnforcedStyle: snake_case

but got remarkably similar results (both styles are used regularly):

- 1029 files inspected, 5491 offenses detected, 3058 offenses auto-correctable
+ 1029 files inspected, 5484 offenses detected, 3058 offenses auto-correctable

So on balance it's probably better to stick with the Ruby community's chosen default.

Copy link
Contributor

@ollietulloch ollietulloch left a comment

Choose a reason for hiding this comment

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

Thanks @joshpencheon - good to keep up to date with rubocop, NewCops is neat!

Nice addition of rubocop-rake

@joshpencheon joshpencheon merged commit 7d6c52a into master Jan 6, 2021
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.

2 participants