Skip to content

Commit

Permalink
Refactor/initial rubocop rework (#1733)
Browse files Browse the repository at this point in the history
* Remove poor cop config for this setting. It should be fixed up

* Complexity metrics should definitely be fixed up in the codebase

* Remove redundant enabling of cops now we use them by default in latest rubocop

* These two cops are purely style related and relatively simple to tag and fix up

* Reduce some metric offenses by 10%

* Run rubocop on ruby 2.7 as that's our lowest supported version and target for cop offenses

* Remove a bunch of hidden rubocop metric overrides

* Re-generate the auto-gen-config

* Add changelog

* Re-generate config with new rule settings

* Add new manual hacked settings

* Make run green again

* Add todo note

* Fix changelog PR number
  • Loading branch information
luke-hill authored Sep 2, 2023
1 parent 6e2551b commit 67d4d5a
Show file tree
Hide file tree
Showing 9 changed files with 111 additions and 76 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,6 @@ jobs:
- uses: actions/checkout@v3
- uses: ruby/setup-ruby@v1
with:
ruby-version: 3.2
ruby-version: 2.7
bundler-cache: true
- run: bundle exec rake rubocop
- run: bundle exec rubocop
59 changes: 20 additions & 39 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,6 @@ AllCops:
Layout/EndOfLine:
EnforcedStyle: lf

# TODO: [LH] - This needs removing - legacy code. Initially move to auto-gen-config and then fix
# Disabling this cop until the minimum Ruby version is >= 2.3 as squiggly
# heredocs '<<~' were introduced then. The files below are the current ones
# with offenses
Layout/HeredocIndentation:
Exclude:
- 'features/lib/support/feature_factory.rb'
- 'lib/cucumber/cli/options.rb'
- 'lib/cucumber/cli/profile_loader.rb'
- 'spec/cucumber/cli/configuration_spec.rb'
- 'spec/cucumber/cli/profile_loader_spec.rb'
- 'spec/cucumber/formatter/pretty_spec.rb'

# Reviewed: Formatters put trailing spaces after things like 'Feature: '
# In pretty_spec.rb & progress_spec.rb offences look false as the trailing spaces are in fact multiline string literals
Layout/TrailingWhitespace:
Expand All @@ -59,10 +46,14 @@ Lint/UselessMethodDefinition:
Exclude:
- 'lib/cucumber/glue/proto_world.rb'

# TODO: [LH] - This definitely needs a partial fix / reduction. Even if only an interim one that pushes some stuff
# TODO: [LH] - Initial 10% reduction done Aug '23 - Further reductions should happen as/when we fix a bunch of the todo
# to the auto-gen-config
Metrics/AbcSize:
Max: 45
Max: 41
# TODO: Manually added! - be careful with regenning the file - this needs a fix
Exclude:
- lib/cucumber/formatter/junit.rb
- spec/cucumber/formatter/http_io_spec.rb

# TODO: [LH] - This needs a re-review. I think we can pretty much delete / phase this out with incremental updates
Metrics/BlockLength:
Expand All @@ -77,29 +68,31 @@ Metrics/ClassLength:
Max: 375
Exclude:
- 'lib/cucumber/cli/options.rb'

# TODO: [LH] - This definitely needs a partial fix / reduction. Even if only an interim one that pushes some stuff
Metrics/CyclomaticComplexity:
Max: 12
# TODO: Manually added! - be careful with regenning the file - this needs a fix
- lib/cucumber/formatter/pretty.rb

# A line length of 200 covers most violations in the repo while still being a more up to date length given today's screen sizes
Layout/LineLength:
Max: 200

# TODO: [LH] - This definitely needs a partial fix / reduction. Even if only an interim one that pushes some stuff
# TODO: [LH] - Initial 10% reduction done Aug '23 - Further reductions should happen as/when we fix a bunch of the todo
Metrics/ModuleLength:
Max: 150
Max: 135
Exclude:
- './spec/**/*'
- 'cck/spec/**/*'
# TODO: Manually added! - be careful with regenning the file - this needs a fix
- lib/cucumber/formatter/console.rb

# TODO: [LH] - This definitely needs a partial fix / reduction. Even if only an interim one that pushes some stuff
# TODO: [LH] - Initial 10% reduction done Aug '23 - Further reductions should happen as/when we fix a bunch of the todo
Metrics/MethodLength:
Max: 30

# TODO: [LH] - This definitely needs a partial fix / reduction. Even if only an interim one that pushes some stuff
Metrics/PerceivedComplexity:
Max: 13
Max: 27
# TODO: Manually added! - be careful with regenning the file - this needs a fix
Exclude:
- lib/cucumber/cli/main.rb
- lib/cucumber/cli/options.rb
- lib/cucumber/formatter/publish_banner_printer.rb
- spec/cucumber/formatter/http_io_spec.rb

# Rubocop doesn't like method names in other languages but as Cucumber supports
# languages, this cop needs to be disabled.
Expand Down Expand Up @@ -139,29 +132,17 @@ Naming/VariableNumber:
# This cop throws errors when parsing a lot of the repo's code, possibly due
# to splat issues?
Style/AccessModifierDeclarations:
Enabled: false

Style/ClassAndModuleChildren:
Enabled: false

Style/ClassEqualityComparison:
Enabled: true

Style/Documentation:
Enabled: false

Style/FormatStringToken:
EnforcedStyle: annotated

Style/FloatDivision:
Enabled: false

Style/StderrPuts:
Enabled: false

Style/RegexpLiteral:
EnforcedStyle: slashes
AllowInnerSlashes: true

Style/YodaCondition:
Enabled: true
107 changes: 85 additions & 22 deletions .rubocop_todo.yml
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
# This configuration was generated by
# `rubocop --auto-gen-config`
# on 2023-09-01 11:47:49 UTC using RuboCop version 1.43.0.
# on 2023-09-02 09:39:29 UTC using RuboCop version 1.56.2.
# The point is for the user to remove these configuration records
# one by one as the offenses are removed from the code base.
# Note that changes in the inspected code, or installation of new
# versions of RuboCop, may require this file to be generated again.

# TODO - [LH] -> Aug '23 - 370 files inspected, 1110 offenses detected, 482 offenses autocorrectable

# Offense count: 38
# This cop supports safe autocorrection (--autocorrect).
# Configuration parameters: AllowMultipleStyles, EnforcedHashRocketStyle, EnforcedColonStyle, EnforcedLastArgumentHashStyle.
Expand All @@ -19,12 +21,46 @@ Layout/HashAlignment:
- 'lib/cucumber/formatter/ansicolor.rb'
- 'lib/cucumber/gherkin/formatter/ansi_escapes.rb'

# Offense count: 13
# This cop supports safe autocorrection (--autocorrect).
Layout/HeredocIndentation:
Exclude:
- 'features/lib/support/feature_factory.rb'
- 'lib/cucumber/cli/options.rb'
- 'lib/cucumber/cli/profile_loader.rb'
- 'spec/cucumber/cli/configuration_spec.rb'
- 'spec/cucumber/cli/profile_loader_spec.rb'
- 'spec/cucumber/formatter/pretty_spec.rb'

# Offense count: 1
# This cop supports safe autocorrection (--autocorrect).
# Configuration parameters: AllowedMethods, AllowedPatterns.
Lint/AmbiguousBlockAssociation:
Exclude:
- 'spec/cucumber/glue/step_definition_spec.rb'

# Offense count: 1
# This cop supports safe autocorrection (--autocorrect).
Lint/RedundantCopDisableDirective:
Exclude:
- 'lib/cucumber/cli/options.rb'

# Offense count: 1
# Configuration parameters: AllowComments, AllowNil.
Lint/SuppressedException:
Exclude:
- 'lib/cucumber/rake/task.rb'

# Offense count: 9
# Configuration parameters: AllowedMethods, AllowedPatterns.
Metrics/CyclomaticComplexity:
Max: 12

# Offense count: 9
# Configuration parameters: AllowedMethods, AllowedPatterns.
Metrics/PerceivedComplexity:
Max: 13

# Offense count: 1
# Configuration parameters: ExpectMatchingDefinition, CheckDefinitionPathHierarchy, CheckDefinitionPathHierarchyRoots, Regex, IgnoreExecutableScripts, AllowedAcronyms.
# CheckDefinitionPathHierarchyRoots: lib, spec, test, src
Expand All @@ -39,7 +75,7 @@ RSpec/AnyInstance:
- 'spec/cucumber/cli/main_spec.rb'

# Offense count: 5
# This cop supports safe autocorrection (--autocorrect).
# This cop supports unsafe autocorrection (--autocorrect-all).
RSpec/BeEql:
Exclude:
- 'spec/cucumber/cli/configuration_spec.rb'
Expand Down Expand Up @@ -69,7 +105,7 @@ RSpec/ContextMethod:
- 'spec/cucumber/term/banner_spec.rb'

# Offense count: 98
# Configuration parameters: Prefixes.
# Configuration parameters: Prefixes, AllowedPatterns.
# Prefixes: when, with, without
RSpec/ContextWording:
Enabled: false
Expand All @@ -78,6 +114,11 @@ RSpec/ContextWording:
# Configuration parameters: IgnoredMetadata.
RSpec/DescribeClass:
Exclude:
- '**/spec/features/**/*'
- '**/spec/requests/**/*'
- '**/spec/routing/**/*'
- '**/spec/system/**/*'
- '**/spec/views/**/*'
- 'spec/cck/cck_spec.rb'

# Offense count: 167
Expand All @@ -88,6 +129,7 @@ RSpec/DescribedClass:
Enabled: false

# Offense count: 2
# This cop supports unsafe autocorrection (--autocorrect-all).
RSpec/EmptyExampleGroup:
Exclude:
- 'spec/cucumber/filters/activate_steps_spec.rb'
Expand Down Expand Up @@ -122,28 +164,27 @@ RSpec/EmptyLineAfterFinalLet:

# Offense count: 3
# This cop supports safe autocorrection (--autocorrect).
# Configuration parameters: AllowConsecutiveOneLiners.
RSpec/EmptyLineAfterHook:
Exclude:
- 'spec/cucumber/formatter/junit_spec.rb'

# Offense count: 6
# Offense count: 2
# This cop supports safe autocorrection (--autocorrect).
RSpec/EmptyLineAfterSubject:
Exclude:
- 'spec/cucumber/filters/tag_limits/verifier_spec.rb'
- 'spec/cucumber/hooks_spec.rb'
- 'spec/cucumber/runtime/meta_message_builder_spec.rb'
- 'spec/cucumber/runtime/support_code_spec.rb'
- 'spec/cucumber/runtime_spec.rb'

# Offense count: 150
# Configuration parameters: Max.
# Configuration parameters: CountAsOne.
RSpec/ExampleLength:
Enabled: false
Max: 58

# Offense count: 51
# This cop supports safe autocorrection (--autocorrect).
# Configuration parameters: CustomTransform, IgnoredWords.
# Configuration parameters: CustomTransform, IgnoredWords, DisallowedExamples.
# DisallowedExamples: works
RSpec/ExampleWording:
Exclude:
- 'spec/cucumber/cli/configuration_spec.rb'
Expand All @@ -155,7 +196,7 @@ RSpec/ExampleWording:
# This cop supports safe autocorrection (--autocorrect).
RSpec/ExpectActual:
Exclude:
- 'spec/routing/**/*'
- '**/spec/routing/**/*'
- 'spec/cucumber/cli/configuration_spec.rb'
- 'spec/cucumber/step_match_spec.rb'

Expand Down Expand Up @@ -195,16 +236,6 @@ RSpec/IteratedExpectation:
Exclude:
- 'spec/cucumber/filters/gated_receiver_spec.rb'

# Offense count: 6
# This cop supports safe autocorrection (--autocorrect).
RSpec/LeadingSubject:
Exclude:
- 'spec/cucumber/cli/main_spec.rb'
- 'spec/cucumber/configuration_spec.rb'
- 'spec/cucumber/formatter/url_reporter_spec.rb'
- 'spec/cucumber/rake/forked_spec.rb'
- 'spec/cucumber/runtime/support_code_spec.rb'

# Offense count: 59
# Configuration parameters: EnforcedStyle.
# SupportedStyles: have_received, receive
Expand Down Expand Up @@ -242,7 +273,8 @@ RSpec/MultipleMemoizedHelpers:
Max: 15

# Offense count: 46
# Configuration parameters: IgnoreSharedExamples.
# Configuration parameters: EnforcedStyle, IgnoreSharedExamples.
# SupportedStyles: always, named_only
RSpec/NamedSubject:
Exclude:
- 'spec/cucumber/cli/main_spec.rb'
Expand All @@ -255,6 +287,7 @@ RSpec/NamedSubject:
- 'spec/cucumber/runtime_spec.rb'

# Offense count: 6
# Configuration parameters: AllowedGroups.
RSpec/NestedGroups:
Max: 6

Expand Down Expand Up @@ -354,6 +387,36 @@ Rake/Desc:
- 'gem_tasks/contributors.rake'
- 'gem_tasks/environment.rake'

# Offense count: 11
# This cop supports unsafe autocorrection (--autocorrect-all).
# Configuration parameters: EnforcedStyle.
# SupportedStyles: nested, compact
Style/ClassAndModuleChildren:
Exclude:
- 'lib/autotest/cucumber.rb'
- 'lib/autotest/cucumber_mixin.rb'
- 'lib/autotest/cucumber_rails.rb'
- 'lib/autotest/cucumber_rails_rspec.rb'
- 'lib/autotest/cucumber_rails_rspec2.rb'
- 'lib/autotest/cucumber_rspec.rb'
- 'lib/autotest/cucumber_rspec2.rb'
- 'spec/cucumber/deprecate_spec.rb'
- 'spec/cucumber/formatter/fail_fast_spec.rb'
- 'spec/cucumber/formatter/interceptor_spec.rb'
- 'spec/cucumber/hooks_spec.rb'

# Offense count: 3
Style/ClassVars:
Exclude:
- 'spec/cucumber/glue/step_definition_spec.rb'

# Offense count: 22
# This cop supports unsafe autocorrection (--autocorrect-all).
# Configuration parameters: EnforcedStyle.
# SupportedStyles: left_coerce, right_coerce, single_coerce, fdiv
Style/FloatDivision:
Enabled: false

# Offense count: 5
# This cop supports unsafe autocorrection (--autocorrect-all).
# Configuration parameters: EnforcedStyle.
Expand Down
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ Please visit [cucumber/CONTRIBUTING.md](https://github.com/cucumber/cucumber/blo

- Gem update. Update rubocop gems to latest and increase minimum version of some cucumber sub-gems ([#1732](https://github.com/cucumber/cucumber-ruby/pull/1732) [luke-hill](https://github.com/luke-hill))

- Rubocop update. Massively overhauled the cucumber style / rubocop expectations and began to tackle some long-standing tech-debt ([#1733](https://github.com/cucumber/cucumber-ruby/pull/1733) [luke-hill](https://github.com/luke-hill))

## [9.0.1] - 2023-09-01
### Changed
- Update dependency of a few associated cucumber sub-gems ([#1715](https://github.com/cucumber/cucumber-ruby/pull/1715) [luke-hill](https://github.com/luke-hill))
Expand Down
3 changes: 0 additions & 3 deletions lib/cucumber/formatter/console.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
# frozen_string_literal: true

# rubocop:disable Metrics/ModuleLength

require 'cucumber/formatter/ansicolor'
require 'cucumber/formatter/duration'
require 'cucumber/gherkin/i18n'
Expand Down Expand Up @@ -262,4 +260,3 @@ def initialize(actual_keyword, step)
end
end
end
# rubocop:enable Metrics/ModuleLength
2 changes: 1 addition & 1 deletion lib/cucumber/formatter/pretty.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ module Formatter
#
# If the output is STDOUT (and not a file), there are bright colours to watch too.
#
class Pretty # rubocop:disable Metrics/ClassLength
class Pretty
include FileUtils
include Console
include Io
Expand Down
2 changes: 0 additions & 2 deletions lib/cucumber/formatter/publish_banner_printer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ def initialize(configuration)
end
end

# rubocop:disable Metrics/MethodLength
def display_publish_ad(io)
display_banner(
[
Expand Down Expand Up @@ -59,7 +58,6 @@ def display_publish_ad(io)
io
)
end
# rubocop:enable Metrics/MethodLength

def highlight(text)
[text, :cyan]
Expand Down
Loading

0 comments on commit 67d4d5a

Please sign in to comment.