alphagov / rubocop-govuk

RuboCop style rules for GOV.UK's Ruby projects

RuboCop GOV.UK

Defines the linting rules for GDS Ruby applications, primarily those associated with GOV.UK.

GOV.UK has used a styleguide for many years, starting with rules in written form, which we then automated with RuboCop and later moved into this repo. A styleguide is a valuable asset: it keeps our code consistent and prevents stylistic squabbles. Everyone on GOV.UK is encouraged to use it in their Ruby projects and contribute to dependency upgrades and new releases, keeping pace with the rest of the Ruby community.

Installation

Add rubocop-govuk to your Gemfile and then run bundle install:

# Gemfile
gem 'rubocop-govuk', require: false

Then inherit the default rules by adding the following in your project:

# .rubocop.yml
inherit_gem:
  rubocop-govuk:
    - config/default.yml

inherit_mode:
  merge:
    - Exclude

# **************************************************************
# TRY NOT TO ADD OVERRIDES IN THIS FILE
#
# This repo is configured to follow the RuboCop GOV.UK styleguide.
# Any rules you override here will cause this repo to diverge from
# the way we write code in all other GOV.UK repos.
#
# See https://github.com/alphagov/rubocop-govuk/blob/main/CONTRIBUTING.md
# **************************************************************

You can also configure additional rules for Rails and RSpec:

# .rubocop.yml
inherit_gem:
  rubocop-govuk:
    ...
    - config/rails.yml

# .rubocop.yml
inherit_gem:
  rubocop-govuk:
    ...
    - config/rspec.yml

Contributing

Rules in this repo are defined based on their compatibility with GOV.UK apps and their code conventions. Everyone else is welcome to use it and suggest changes - see CONTRIBUTING.md for more details.

Licence

MIT License

rubocop-govuk's Issues

Lots of repos are out-of-sync with this styleguide

There's not much point having a styleguide if we're going to ignore it. For new code, it's seldom absolutely necessary to disable a Cop. It's rarely necessary to disable a Cop for an entire repo.

We should try to address this issue by:

• Removing redundant config e.g. some Metrics/BlockLength overrides
• Removing overrides that can be auto-corrected
• Removing overrides that can be manually corrected fairly easily
• Commenting why any remaining overrides are objectively necessary

Repos this issue applies to:

• smart-answers [4 custom config items]
• specialist-publisher [1 custom config item]
• travel-advice-publisher [5 custom config items]
• special-route-publisher [1 custom config item]
• router-api [3 custom config items]
• maslow [2 custom config items]
• release [1 custom config item]
• support [2 custom config items]
• short-url-manager [2 custom config items]
• search-admin [2 custom config items]
• government-frontend [3 custom config items]
• datagovuk_publish [3 custom config items]
• collections [3 custom config items]
• email-alert-frontend [2 custom config items]
• contacts-admin [1 custom config item]
• frontend [3 custom config items]
• transition [4 custom config items]
• service-manual-publisher [1 custom config item, not using Rails Cops]
• publishing-api [5 custom config items]
• support-api [2 custom config items, not using Rails Cops]
• manuals-frontend [4 custom config items]
• service-manual-frontend [3 custom config items]
• asset-manager [19 custom config items]
• content-data-api [7 custom config items]
• content-store [4 custom config items]
• publisher [6 custom config items]
• search-api [6 custom config items]
• local-links-manager [1 custom config item]
• signon [7 custom config items]
• feedback [3 custom config items]
• content-tagger [10 custom config items]
• email-alert-api [6 custom config items]
• whitehall [18 custom config items]
• content-publisher [4 custom config items]
• licence-finder [1 custom config item]
• calculators [3 custom config items]
• govuk-developer-docs [1 custom config item]
• finder-frontend [1 custom config item]
• collections-publisher [2 custom config items]
• link-checker-api [1 custom config item, not using Rails Cops]
• manuals-publisher [1 custom config items]

4.0.0 pre-release issues

With the 4.0.0 changes to rubocop-govuk there are quite a lot of new rules (1, 2) which make us unsure how big the impact will be for GOV.UK. Therefore we are releasing this version as a pre-release to help identify whether it presents any particular challenges for GOV.UK codebases before a wider release.

The approach to adding new rules is to favour consistency with the default rules of the upstream libraries (rubocop, rubcop-rails, etc) over individual opinions on approaches. We've aimed to disable only rules that would produce significant problems in applying to the GOV.UK codebase.

In this pre-release we should ensure that we test

• Frequently changed big GOV.UK apps (for example: Whitehall, Publishing API, Smart Answers
• Some older and less loved GOV.UK apps (for example: licence-finder, bouncer)
• Newer GOV.UK apps that represent current patterns (for example: Content Publisher, Email Alert API)
• Apps/utilities that don't use Rails (for example Search API, Cache Clearing Service, GOV.UK Developer Docs, Content Schemas)
• GOV.UK Gems (for example govuk_publishing_components, govuk_app_config)

To install this pre-release in app you need to update the Gemfile accordingly:

gem "rubocop-govuk, "4.0.0.pre-1"

This thread can be used to raise any concerns regarding these new rules, particularly where they conflict with GOV.UK conventions or require so many changes adoption will be very difficult.

Initially there have been 2 rules that have proved divisive, where wider opinions are welcomed, these are: Lint/MissingSuper and Lint/UselessMethodDefinition. There are also some concerns that these rules could be difficult to apply are Gemspec/RequiredRubyVersion, Style/CombinableLoops, Style/HashLikeCase. So it'd be good to learn if these are too painful to apply consistently.

RuboCop 0.80 surfaces which unconfigured new cops we have

I noticed this on an app's Dependabot gem bump PR CI failure for rubocop-govuk.

The following cops were added to RuboCop, but are not configured. Please set Enabled to either `true` or `false` in your `.rubocop.yml` file:
 - Style/HashEachMethods (0.80)
 - Style/HashTransformKeys (0.80)
 - Style/HashTransformValues (0.80)

This new informational messaging is a feature which wasn't immediately obvious when dealing with #13 (I was looking for added and removed cops, but not much else).

We should make decisions about whether to include them and bump the gem version again.

Blacklist has been renamed to "ForbiddenMethods"

Hey geting a lot of errors when running rubocop-govuk on finder-frontend right now.
Should be a simple find and replce job. Will document here then get a PR up to fix.

Example output

Warning: Rails/SkipsModelValidations does not support Blacklist parameter.                                                                                                                                         
                                                                                                                                                                                                                   
Supported parameters are:                                                                                                                                                                                          
                                                                                                                                                                                                                   
  - Enabled                                                                                                                                                                                                        
  - ForbiddenMethods                                                                                                                                                                                               
  - AllowedMethods                                                                                                                                                                                                 
                                                                                                                                                                                                                   
Warning: Rails/SkipsModelValidations does not support Blacklist parameter.                                                                                                                                         
                                                                                                                                                                                                                   
Supported parameters are:                                                                                                                                                                                          
                                                                                                                                                                                                                   
  - Enabled                                                                                                                                                                                                        
  - ForbiddenMethods                                                                                                                                                                                               
  - AllowedMethods                                                                                                                                                                                                 
                                                                                                                                                                                                                   
Inspecting 279 files                                                                                                                                                                                               
`Blacklist` has been renamed to `ForbiddenMethods`.                                                                                                                                                                
.`Blacklist` has been renamed to `ForbiddenMethods`.                                                                                                                                                               
.`Blacklist` has been renamed to `ForbiddenMethods`.                                                                                                                                                               
.`Blacklist` has been renamed to `ForbiddenMethods`.                                                                                                                                                               
.`Blacklist` has been renamed to `ForbiddenMethods`. 
[.. others omitted]

New Release

Could you create a new release now that the Rubocop version being depended on has been bumped?

gem `rubocop-rails` support

Can you provide support rubocop-railsgem ?

Lots of Cops are switched off with no explanation

In doing #38 we discovered a large number of Cops that have been disabled without any explanation. This could be for a valid reason, but 3cc7240 suggests it's arbitrary.

We should investigate the Cops that have been disabled in this way, and either enable them, or add a comment to explain why they should continue to be disabled.

• Layout/MultilineMethodCallIndentation
• Layout/ClosingParenthesisIndentation
• Rails/SkipsModelValidations
• Rails/Validation
• Rails/ScopeArgs
• Rails/ReadWriteAttribute
• Rails/Output
• Rails/HasAndBelongsToMany
• Rails/Delegate
• Rails/ActionFilter
• Style/WhileUntilModifier
• Style/WhileUntilDo
• Style/WhenThen
• Style/VariableInterpolation
• Style/CommandLiteral
• Style/RedundantCapitalW
• Style/SpecialGlobalVars
• Layout/SpaceBeforeComment
• Layout/SpaceAfterNot
• Layout/SpaceAroundKeyword
• Style/SingleLineMethods
• Style/SingleLineBlockParams
• Style/SignalException
• Style/Semicolon
• Style/SelfAssignment
• Style/SafeNavigation
• Style/RescueModifier
• Style/RedundantSelf
• Style/RedundantException
• Style/RedundantBegin
• Style/RaiseArgs
• Style/Proc
• Naming/PredicateName
• Style/PerlBackrefs
• Style/PercentLiteralDelimiters
• Style/NumericLiterals
• Style/Not
• Style/NonNilCheck
• Style/NilComparison
• Style/Next
• Style/NegatedWhile
• Style/NegatedIf
• Style/ModuleFunction
• Style/MethodDefParentheses
• Style/LineEndConcatenation
• Layout/LeadingCommentSpace
• Style/LambdaCall
• Style/Lambda
• Layout/FirstHashElementIndentation
• Layout/FirstArrayElementIndentation
• Style/IfWithSemicolon
• Style/GlobalVars
• Lint/FlipFlop
• Naming/FileName
• Style/EvenOdd
• Style/EndBlock
• Style/Encoding
• Style/EmptyLiteral
• Style/EachWithObject
• Style/DoubleNegation
• Layout/DotPosition
• Style/PreferredHashMethods
• Layout/CommentIndentation
• Style/CommentAnnotation
• Style/ColonMethodCall
• Style/CollectionMethods
• Style/ClassAndModuleChildren
• Style/CharacterLiteral
• Style/CaseEquality
• Style/BlockDelimiters
• Metrics/BlockNesting
• Style/BlockComments
• Style/BeginBlock
• Style/Attr
• Naming/AsciiIdentifiers
• Style/AsciiComments
• Style/ArrayJoin
• Layout/ParameterAlignment
• Layout/HashAlignment
• Style/Alias
• Style/FormatString

Other config we may want to look at:

• Style/MutableConstant (enabled without explanation)
• Layout/MultilineOperationIndentation (EnforcedStyle set without explanation)

Latest release surfaces unconfigured new cops and wrong namespace

Hi team,

Dependabot kindly dropped a PR into one of my teams repos to bump this gem to v3.4.0. One inspected the terminal output I spotted the following warning messages are now being outputted.

/../ruby/2.6.5/lib/ruby/gems/2.6.0/gems/rubocop-govuk-3.4.0/config/other-lint.yml: Lint/EndBlock has the wrong namespace - should be Style
The following cops were added to RuboCop, but are not configured. Please set Enabled to either `true` or `false` in your `.rubocop.yml` file:
 - Lint/RaiseException (0.81)
 - Lint/StructNewOverride (0.81)
For more information: https://docs.rubocop.org/en/latest/versioning/