alphagov / rubocop-govuk Goto Github PK

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

Can you provide support rubocop-railsgem ?

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.

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.

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/

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]

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)

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]