From 8a4fb0ec4bcde959f5b3026a8f906a679c6df372 Mon Sep 17 00:00:00 2001 From: Splines Date: Thu, 16 Nov 2023 00:12:59 +0100 Subject: [PATCH 1/6] Shorten RuboCop config Many cops were explicitly enabled. We don't necessarily need to do that since the defaults of RuboCop are always included. So we just specify where we deviate from RuboCops default values. --- .rubocop.yml | 353 +++++++-------------------------------------------- 1 file changed, 46 insertions(+), 307 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index d7b548944..05ab8dc58 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,341 +1,80 @@ +# SEVERITY LEVELS +# In the GitHub CI/CD, we can only distinguish between "error" and "warning". +# In Rubocop this is much more granular: "info", "refactor", "convention", +# "warning", "error" and "fatal". +# From the docs: "The level is normally 'warning' for Lint and 'convention' for +# all the others, but this can be changed in user configuration." +# However, we don't want to set the severity for each cop individually, so instead +# we set the fatal level as "convention" in the CI/CD, +# i.e. severity: "convention" and up will be treated as an error on GitHub +# and below as warning such that the check will still pass (with a warning). +# +# Short: +# "convention" severity and up = Error on GitHub (CI/CD) -> check fails +# "refactor" severity and below = Warning on GitHub (CI/CD) -> check passes +# +# see the docs on severity +# https://docs.rubocop.org/rubocop/configuration.html#severity + +# Rubocop defaults are by default required/included: +# https://github.com/rubocop/rubocop/blob/master/config/default.yml require: - - rubocop-packaging - rubocop-performance - rubocop-rails -AllCops: - TargetRubyVersion: 3.0 - # RuboCop has a bunch of cops enabled by default. This setting tells RuboCop - # to ignore them, so only the ones explicitly set in this file are enabled. - DisabledByDefault: false - Exclude: - - '**/tmp/**/*' - - '**/templates/**/*' - - '**/vendor/**/*' - - 'actionpack/lib/action_dispatch/journey/parser.rb' - - 'actionmailbox/test/dummy/**/*' - - 'actiontext/test/dummy/**/*' - - '**/node_modules/**/*' +# Ruby version is determined from the Gemfile.lock -Performance: - Exclude: - - '**/test/**/*' - -# Prefer assert_not over assert ! -Rails/AssertNot: - Include: - - '**/test/**/*' - -# Prefer assert_not_x over refute_x -Rails/RefuteMethods: - Include: - - '**/test/**/*' - -Rails/IndexBy: - Enabled: true - -Rails/IndexWith: - Enabled: true - -# Prefer &&/|| over and/or. -Style/AndOr: - Enabled: true - -Layout/ArgumentAlignment: - Enabled: true - -Layout/ArrayAlignment: - Enabled: true - -Layout/BlockAlignment: - Enabled: true - -# Align `when` with `case`. -Layout/CaseIndentation: - Enabled: true - -Layout/ClosingHeredocIndentation: - Enabled: true - -# Align comments with method definitions. -Layout/CommentIndentation: - Enabled: true +############################################# +# Layout +############################################# -Layout/ElseAlignment: - Enabled: true - -# Align `end` with the matching keyword or starting expression except for -# assignments, where it should be aligned with the LHS. Layout/EndAlignment: - Enabled: true EnforcedStyleAlignWith: variable - AutoCorrect: true - -Layout/EmptyLines: - Enabled: true - -Layout/EmptyLineAfterMagicComment: - Enabled: true - -Layout/EmptyLinesAroundAccessModifier: - Enabled: true - -Layout/EmptyLinesAroundBlockBody: - Enabled: true - -# In a regular class definition, no empty lines around the body. -Layout/EmptyLinesAroundClassBody: - Enabled: true - -# In a regular method definition, no empty lines around the body. -Layout/EmptyLinesAroundMethodBody: - Enabled: true - -# In a regular module definition, no empty lines around the body. -Layout/EmptyLinesAroundModuleBody: - Enabled: true - -Layout/ExtraSpacing: - Enabled: true Layout/LineLength: - Max: 80 - -# Use Ruby >= 1.9 syntax for hashes. Prefer { a: :b } over { :a => :b }. -Style/HashSyntax: - Enabled: true - -Layout/FirstArgumentIndentation: - Enabled: true + Max: 100 -Layout/HashAlignment: - Enabled: true - -# Method definitions after `private` or `protected` isolated calls need one -# extra level of indentation. Layout/IndentationConsistency: - Enabled: true EnforcedStyle: indented_internal_methods -# Two spaces, no tabs (for indentation). -Layout/IndentationWidth: - Enabled: true - -Layout/LeadingCommentSpace: - Enabled: true - -Layout/MultilineOperationIndentation: - Enabled: true - -Layout/SpaceAfterColon: - Enabled: true - -Layout/SpaceAfterComma: - Enabled: true - -Layout/SpaceAfterSemicolon: - Enabled: true - -Layout/SpaceAroundEqualsInParameterDefault: - Enabled: true - -Layout/SpaceAroundKeyword: - Enabled: true +############################################# +# Metrics +############################################# -Layout/SpaceBeforeComma: - Enabled: true - -Layout/SpaceBeforeComment: - Enabled: true - -Layout/SpaceBeforeFirstArg: - Enabled: true - -Layout/SpaceInsideArrayLiteralBrackets: - Enabled: true - -Style/BlockDelimiters: - Enabled: true - -Style/BlockComments: - Enabled: true +Metrics: + Enabled: false -Style/ConditionalAssignment: +Metrics/ParameterLists: Enabled: true -Style/DefWithParentheses: - Enabled: true +############################################# +# Style +############################################# Style/Documentation: - Enabled: true - -# Defining a method with parameters needs parentheses. -Style/MethodDefParentheses: - Enabled: true - -Style/MethodCallWithoutArgsParentheses: - Enabled: true + Enabled: false Style/FrozenStringLiteralComment: - Enabled: true - EnforcedStyle: always - Exclude: - - 'actionview/test/**/*.builder' - - 'actionview/test/**/*.ruby' - - 'actionpack/test/**/*.builder' - - 'actionpack/test/**/*.ruby' - - 'activestorage/db/migrate/**/*.rb' - - 'activestorage/db/update_migrate/**/*.rb' - - 'actionmailbox/db/migrate/**/*.rb' - - 'actiontext/db/migrate/**/*.rb' - -Style/NumericLiterals: - Enabled: true - -Style/NumericPredicate: - Enabled: true - -Style/RedundantFreeze: - Enabled: true - -Style/SymbolProc: - Enabled: true + EnforcedStyle: never Style/SymbolArray: - Enabled: false - -# Use `foo {}` not `foo{}`. -Layout/SpaceBeforeBlockBraces: - Enabled: true + EnforcedStyle: brackets -# Use `foo { bar }` not `foo {bar}`. -Layout/SpaceInsideBlockBraces: - Enabled: true - EnforcedStyleForEmptyBraces: space - -# Use `{ a: 1 }` not `{a:1}`. -Layout/SpaceInsideHashLiteralBraces: - Enabled: true - -Layout/SpaceInsideParens: - Enabled: true - -# Check quotes usage according to lint rule below. Style/StringLiterals: - Enabled: true - EnforcedStyle: single_quotes - -# Detect hard tabs, no hard tabs. -Layout/IndentationStyle: - Enabled: true - -# Empty lines should not have any spaces. -Layout/TrailingEmptyLines: - Enabled: true - -# No trailing whitespace. -Layout/TrailingWhitespace: - Enabled: true - -# Use quotes for string literals when they are enough. -Style/RedundantPercentQ: - Enabled: true - -Lint/AmbiguousOperator: - Enabled: true - -Lint/AmbiguousRegexpLiteral: - Enabled: true - -Lint/ErbNewArguments: - Enabled: true - -# Use my_method(my_arg) not my_method( my_arg ) or my_method my_arg. -Lint/RequireParentheses: - Enabled: true - -Lint/ShadowingOuterLocalVariable: - Enabled: true - -Lint/RedundantStringCoercion: - Enabled: true - -Lint/UriEscapeUnescape: - Enabled: true - -Lint/UselessAssignment: - Enabled: true - -Lint/DeprecatedClassMethods: - Enabled: true + EnforcedStyle: double_quotes -Naming/VariableNumber: - Enabled: true - -Style/ParenthesesAroundCondition: - Enabled: true - -Style/HashTransformKeys: - Enabled: true - -Style/HashTransformValues: - Enabled: true - -Style/RedundantBegin: - Enabled: true +Style/StringLiteralsInInterpolation: + EnforcedStyle: double_quotes Style/RedundantReturn: - Enabled: true AllowMultipleReturnValues: true -Style/Semicolon: - Enabled: true - AllowAsExpressionSeparator: true - -# Prefer Foo.method over Foo::method -Style/ColonMethodCall: - Enabled: true +############################################# +# Performance +############################################# -Style/TrivialAccessors: - Enabled: true +Performance: + Severity: refactor # a warning in CI/CD Performance/FlatMap: - Enabled: true - -Performance/RedundantMerge: - Enabled: true - -Performance/StartWith: - Enabled: true - -Performance/EndWith: - Enabled: true - -Performance/RegexpMatch: - Enabled: true - -Performance/ReverseEach: - Enabled: true - -Performance/UnfreezeString: - Enabled: true - -Performance/DeletePrefix: - Enabled: true - -Performance/DeleteSuffix: - Enabled: true - -Metrics/ModuleLength: - Enabled: false - -Metrics/ClassLength: - Enabled: false - -Metrics/MethodLength: - Exclude: - - 'app/abilities/**/*' - -Metrics/AbcSize: - Exclude: - - 'app/abilities/**/*' \ No newline at end of file + Severity: warning # an error in CI/CD From 3cf8c37be4a4034fc64a4e7d3e397dbe2ada7e79 Mon Sep 17 00:00:00 2001 From: Splines Date: Thu, 16 Nov 2023 00:17:16 +0100 Subject: [PATCH 2/6] Add comment regarding "NewCops" --- .rubocop.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.rubocop.yml b/.rubocop.yml index 05ab8dc58..7819eed08 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -22,7 +22,10 @@ require: - rubocop-performance - rubocop-rails -# Ruby version is determined from the Gemfile.lock +# AllCops: +# Ruby version is determined automatically from the Gemfile.lock +# We don't use NewCops: enable|disable since we want new cops to show up +# as pending, so that we can decide whether to use them or not. ############################################# # Layout From e768bfcf3eee91b295b8b4c62935f38ad6bab4b8 Mon Sep 17 00:00:00 2001 From: Splines Date: Thu, 16 Nov 2023 00:33:58 +0100 Subject: [PATCH 3/6] Add "NewCops: enable" This has as consequence that whenever we upgrade RuboCop, we should run checks on the whole codebase to see if new cops somehow don't work with our code. We can then discuss if we maybe want to explicitly disable a new cop. --- .rubocop.yml | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 7819eed08..eb3c37ed6 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -16,16 +16,20 @@ # see the docs on severity # https://docs.rubocop.org/rubocop/configuration.html#severity -# Rubocop defaults are by default required/included: +# "Rubocop defaults" are by default required/included: # https://github.com/rubocop/rubocop/blob/master/config/default.yml require: - rubocop-performance - rubocop-rails -# AllCops: -# Ruby version is determined automatically from the Gemfile.lock -# We don't use NewCops: enable|disable since we want new cops to show up -# as pending, so that we can decide whether to use them or not. +AllCops: + # While default cops are automatically included, they are not "configured" + # by default (only on each next major version of RuboCop). Therefore, we + # have to explicitly configure them here. + # see https://docs.rubocop.org/rubocop/configuration.html#defaults + # and https://docs.rubocop.org/rubocop/1.2/versioning.html#pending-cops + NewCops: enable + # Ruby version is determined automatically from the Gemfile.lock ############################################# # Layout From 51b3098aa3f753a960da34bdbea099ea75a2d33a Mon Sep 17 00:00:00 2001 From: Splines Date: Thu, 16 Nov 2023 00:41:57 +0100 Subject: [PATCH 4/6] Update RuboCop related packages in Gemfile --- Gemfile | 8 +++----- Gemfile.lock | 50 +++++++++++++++++--------------------------------- 2 files changed, 20 insertions(+), 38 deletions(-) diff --git a/Gemfile b/Gemfile index 228b9e901..371aa4264 100644 --- a/Gemfile +++ b/Gemfile @@ -118,11 +118,9 @@ group :development, :docker_development do # Spring speeds up development by keeping your application running in the background. Read more: https://github.com/rails/spring gem "spring" gem "spring-watcher-listen", "~> 2.0.0" - gem "rubocop", "~> 1.50", require: false - gem "rubocop-packaging", require: false - gem "rubocop-performance", require: false - gem "rubocop-rails", require: false - gem "erb_lint", require: false + gem "rubocop", "~> 1.57", require: false + gem 'rubocop-performance', '~> 1.16', require: false + gem 'rubocop-rails', '~> 2.22', '>= 2.22.1', require: false gem "pgreset" gem "marcel" # gem 'bullet' diff --git a/Gemfile.lock b/Gemfile.lock index c1296235d..946c11b31 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -132,13 +132,6 @@ GEM execjs (~> 2.0) barby (0.6.8) bcrypt (3.1.18) - better_html (2.0.1) - actionview (>= 6.0) - activesupport (>= 6.0) - ast (~> 2.0) - erubi (~> 1.4) - parser (>= 2.4) - smart_properties bindex (0.8.1) bootsnap (1.16.0) msgpack (~> 1.2) @@ -205,13 +198,6 @@ GEM unf (>= 0.0.5, < 1.0.0) down (5.4.0) addressable (~> 2.8) - erb_lint (0.4.0) - activesupport - better_html (>= 2.0.1) - parser (>= 2.7.1.4) - rainbow - rubocop - smart_properties erubi (1.12.0) erubis (2.7.0) et-orbi (1.2.7) @@ -324,6 +310,7 @@ GEM kramdown (~> 2.0) kramdown-parser-gfm (1.1.0) kramdown (~> 2.0) + language_server-protocol (3.17.0.3) launchy (2.5.2) addressable (~> 2.8) listen (3.0.8) @@ -373,8 +360,9 @@ GEM orm_adapter (0.5.0) pairing_heap (3.0.0) parallel (1.23.0) - parser (3.2.2.0) + parser (3.2.2.4) ast (~> 2.4.1) + racc pdf-reader (2.11.0) Ascii85 (~> 1.0) afm (~> 0.2.1) @@ -455,7 +443,7 @@ GEM ffi (~> 1.0) redis-client (0.14.1) connection_pool - regexp_parser (2.8.0) + regexp_parser (2.8.2) request_store (1.5.1) rack (>= 1.4) responders (3.1.0) @@ -496,27 +484,26 @@ GEM rspec-mocks (~> 3.11) rspec-support (~> 3.11) rspec-support (3.12.0) - rubocop (1.50.2) + rubocop (1.57.2) json (~> 2.3) + language_server-protocol (>= 3.17.0) parallel (~> 1.10) - parser (>= 3.2.0.0) + parser (>= 3.2.2.4) rainbow (>= 2.2.2, < 4.0) regexp_parser (>= 1.8, < 3.0) rexml (>= 3.2.5, < 4.0) - rubocop-ast (>= 1.28.0, < 2.0) + rubocop-ast (>= 1.28.1, < 2.0) ruby-progressbar (~> 1.7) unicode-display_width (>= 2.4.0, < 3.0) - rubocop-ast (1.28.0) + rubocop-ast (1.30.0) parser (>= 3.2.1.0) - rubocop-packaging (0.5.1) - rubocop (>= 0.89, < 2.0) - rubocop-performance (1.10.2) - rubocop (>= 0.90.0, < 2.0) + rubocop-performance (1.19.1) + rubocop (>= 1.7.0, < 2.0) rubocop-ast (>= 0.4.0) - rubocop-rails (2.9.1) + rubocop-rails (2.22.1) activesupport (>= 4.2.0) rack (>= 1.1) - rubocop (>= 0.90.0, < 2.0) + rubocop (>= 1.33.0, < 2.0) ruby-graphviz (1.2.5) rexml ruby-progressbar (1.13.0) @@ -564,7 +551,6 @@ GEM simplecov (~> 0.19) simplecov-html (0.12.3) simplecov_json_formatter (0.1.4) - smart_properties (1.17.0) spring (2.1.1) spring-watcher-listen (2.0.1) listen (>= 2.7, < 4.0) @@ -627,7 +613,7 @@ GEM unf (0.1.4) unf_ext unf_ext (0.0.8.2) - unicode-display_width (2.4.2) + unicode-display_width (2.5.0) warden (1.2.9) rack (>= 2.0.9) web-console (4.2.0) @@ -677,7 +663,6 @@ DEPENDENCIES database_cleaner devise devise-bootstrap-views - erb_lint erubis exception_handler (~> 0.8.0.0) factory_bot_rails @@ -716,10 +701,9 @@ DEPENDENCIES rgl rqrcode rspec-rails - rubocop (~> 1.50) - rubocop-packaging - rubocop-performance - rubocop-rails + rubocop (~> 1.57) + rubocop-performance (~> 1.16) + rubocop-rails (~> 2.22, >= 2.22.1) rubyzip (~> 2.3.0) sass-rails (>= 6) selenium-webdriver From 8c63039acfbd08075694e0466a40b050ff16d047 Mon Sep 17 00:00:00 2001 From: Splines Date: Mon, 20 Nov 2023 00:37:51 +0100 Subject: [PATCH 5/6] Add WordArray & EmptyMethod enforced styles --- .rubocop.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.rubocop.yml b/.rubocop.yml index eb3c37ed6..abfd66700 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -76,6 +76,12 @@ Style/StringLiteralsInInterpolation: Style/RedundantReturn: AllowMultipleReturnValues: true +Style/WordArray: + EnforcedStyle: brackets + +Style/EmptyMethod: + EnforcedStyle: expanded + ############################################# # Performance ############################################# From 96c70051e263d8e5177ea83ca651273b55dac064 Mon Sep 17 00:00:00 2001 From: Splines Date: Mon, 20 Nov 2023 00:45:58 +0100 Subject: [PATCH 6/6] Sort RuboCop keys alphabetically & improve file comment --- .rubocop.yml | 52 ++++++++++++++++++++++++++++------------------------ 1 file changed, 28 insertions(+), 24 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index abfd66700..f260bf21d 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -9,11 +9,15 @@ # i.e. severity: "convention" and up will be treated as an error on GitHub # and below as warning such that the check will still pass (with a warning). # -# Short: -# "convention" severity and up = Error on GitHub (CI/CD) -> check fails -# "refactor" severity and below = Warning on GitHub (CI/CD) -> check passes +# Overview: +# | RuboCop | GitHub | +# error error -> check fails +# warning error -> check fails +# convention error -> check fails +# refactor warning -> check passes +# info warning -> check passes # -# see the docs on severity +# also see the RuboCop docs on severity # https://docs.rubocop.org/rubocop/configuration.html#severity # "Rubocop defaults" are by default required/included: @@ -38,12 +42,12 @@ AllCops: Layout/EndAlignment: EnforcedStyleAlignWith: variable -Layout/LineLength: - Max: 100 - Layout/IndentationConsistency: EnforcedStyle: indented_internal_methods +Layout/LineLength: + Max: 100 + ############################################# # Metrics ############################################# @@ -54,6 +58,16 @@ Metrics: Metrics/ParameterLists: Enabled: true +############################################# +# Performance +############################################# + +Performance: + Severity: refactor # a warning in CI/CD + +Performance/FlatMap: + Severity: warning # an error in CI/CD + ############################################# # Style ############################################# @@ -61,11 +75,14 @@ Metrics/ParameterLists: Style/Documentation: Enabled: false +Style/EmptyMethod: + EnforcedStyle: expanded + Style/FrozenStringLiteralComment: EnforcedStyle: never -Style/SymbolArray: - EnforcedStyle: brackets +Style/RedundantReturn: + AllowMultipleReturnValues: true Style/StringLiterals: EnforcedStyle: double_quotes @@ -73,21 +90,8 @@ Style/StringLiterals: Style/StringLiteralsInInterpolation: EnforcedStyle: double_quotes -Style/RedundantReturn: - AllowMultipleReturnValues: true +Style/SymbolArray: + EnforcedStyle: brackets Style/WordArray: EnforcedStyle: brackets - -Style/EmptyMethod: - EnforcedStyle: expanded - -############################################# -# Performance -############################################# - -Performance: - Severity: refactor # a warning in CI/CD - -Performance/FlatMap: - Severity: warning # an error in CI/CD