Skip to content

Commit e65492e

Browse files
dafyddcrosbydafyddcrosby
authored
Simplify setting default cops (#157)
* Remove Lint/Syntax (cannot disable) See https://docs.rubocop.org/rubocop/configuration.html#generic-configuration-parameters ``` $ rubocop -c config/disable_all.yml --show-cops Error: configuration for Syntax cop found in config/disable_all.yml It's not possible to disable this cop. ``` It's re-enabled in chefstyle.yml anyhow Confirmed no change in cop configuration ``` $ ./bin/chefstyle --show-cops | sha256sum faa8fd05273862f47447a5357b8001976e8badb8d53c4e761f046383396930b5 - ``` Signed-off-by: David Crosby <[email protected]> * Comment out obsolete/renamed lints In the course of updating the pinned RuboCop versions, there hadn't been changes to the enabled upstream defaults in chefstyle.yml, and so they've been silently broken. This becomes noticeable when running `rubocop -c config/chefstyle.yml`. The decisions on how I did this: - Enabled cops renamed? Comment out for now to keep parity, the re-enabling of those cops should be done in a separate PR to this one (which is designed to simplify the chefstyle internals). - Disabled cops renamed? Update name, keep parity. - Cop removed entirely? Remove cop from config, keep parity. This is all to say that this commit changes nothing: ``` $ ./bin/chefstyle --show-cops | sha256sum faa8fd05273862f47447a5357b8001976e8badb8d53c4e761f046383396930b5 - ``` Signed-off-by: David Crosby <[email protected]> * Use RuboCop::ConfigLoader.configuration_from_file to load configs, stop vendoring upstream.yml Here's the interesting part of the PR stack. Instead of clobbering the RuboCop default constants, load config/default.yml into the default configuration with RuboCop::ConfigLoader#default_configuration= We don't need to keep vendoring upstream.yml, since we're pinning the RuboCop version anyhow. As seen with the earlier chefstyle.yml issues, the more effective way of handling new lints in RuboCop upgrades is using `diff` and `sha256sum` against `--show-cops`. As with before, this maintains `--show-cops` parity ``` $ ./bin/chefstyle --show-cops | sha256sum faa8fd05273862f47447a5357b8001976e8badb8d53c4e761f046383396930b5 - ``` Signed-off-by: David Crosby <[email protected]> * Use DisableByDefault instead of disable_all.yml This changes the sha256 of `--show-cops` from faa8fd05273862f47447a5357b8001976e8badb8d53c4e761f046383396930b5 to bb9e6dd780a44bbe3f193644eb3db7389fae2a1c2b80cf3653dfd332d8842511 However, running `diff` against the output shows that it's cosmetic YAML changes - the `Enabled:` line has moved: ``` --- before 2023-09-20 10:45:00.948536892 -0700 +++ after 2023-09-20 10:46:44.887777282 -0700 @@ -77,8 +77,8 @@ # Department 'Chef/Ruby' (6): Chef/Ruby/GemspecLicense: - Description: All gemspec files should define their license. Enabled: true + Description: All gemspec files should define their license. VersionAdded: 1.7.0 Include: - "**/*.gemspec" @@ -88,37 +88,37 @@ # Supports --auto-correct Chef/Ruby/GemspecRequireRubygems: - Description: Rubygems does not need to be required in a Gemspec Enabled: true + Description: Rubygems does not need to be required in a Gemspec VersionAdded: 1.3.0 Include: - "**/*.gemspec" Chef/Ruby/LegacyPowershellOutMethods: + Enabled: true Description: Use powershell_exec!/powershell_exec instead of the slower legacy powershell_out!/powershell_out methods. - Enabled: true VersionAdded: 1.6.0 # Supports --auto-correct Chef/Ruby/RequireNetHttps: + Enabled: true Description: net/https is deprecated and just includes net/http and openssl. We should include those directly instead - Enabled: true VersionAdded: 1.3.0 # Supports --auto-correct Chef/Ruby/Ruby27KeywordArgumentWarnings: + Enabled: true Description: Pass options to shell_out helpers without the brackets to avoid Ruby 2.7 deprecation warnings. - Enabled: true VersionAdded: 1.3.0 # Supports --auto-correct Chef/Ruby/UnlessDefinedRequire: + Enabled: true Description: Workaround RubyGems' slow requires by only running require if the constant isn't already defined - Enabled: true VersionAdded: 1.3.0 Include: - lib/**/* ``` Signed-off-by: David Crosby <[email protected]> * Remove obsolete vendor rake task Signed-off-by: David Crosby <[email protected]> * [CI] Fix issue with BUNDLE_WITHOUT Signed-off-by: David Crosby <[email protected]> --------- Signed-off-by: David Crosby <[email protected]>
1 parent 71ae977 commit e65492e

File tree

8 files changed

+44
-6194
lines changed

8 files changed

+44
-6194
lines changed

‎.github/workflows/unit.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ jobs:
1616
ruby: ['2.5', '2.6', '2.7', '3.0']
1717
runs-on: ${{ matrix.os }}
1818
env:
19-
BUNDLE_WITHOUT: profiling,debug,docs
19+
BUNDLE_WITHOUT: profiling debug docs
2020
name: Unit test on ${{ matrix.os }} with Ruby ${{ matrix.ruby }}
2121
steps:
2222
- uses: actions/checkout@v2

‎README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ file.
4545

4646
## How It Works
4747

48-
This library has a direct dependency on one specific version of RuboCop (at a time), and [patches it][patch] to load the [upstream configuration][upstream] and [custom set][config] of rule updates. When a new RuboCop release comes out, this library can rev its pinned version dependency and [re-vendor][rakefile] the upstream configuration to determine if any breaking style or lint rules were added/dropped/reversed/etc.
48+
This library has a direct dependency on one specific version of RuboCop (at a time), and [patches it][patch] to load the [upstream configuration][upstream] and [custom set][config] of rule updates. When a new RuboCop release comes out, this library can rev its pinned version dependency to determine if any breaking style or lint rules were added/dropped/reversed/etc.
4949

5050
## Installation
5151

‎Rakefile

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,6 @@
11
# frozen_string_literal: true
22
require "bundler/gem_tasks"
33

4-
upstream = Gem::Specification.find_by_name("rubocop")
5-
6-
desc "Vendor rubocop-#{upstream.version} configuration into gem"
7-
task :vendor do
8-
src = Pathname.new(upstream.gem_dir).join("config")
9-
dst = Pathname.new(__FILE__).dirname.join("config")
10-
11-
mkdir_p dst
12-
cp(src.join("default.yml"), dst.join("upstream.yml"))
13-
14-
require "rubocop"
15-
require "yaml" unless defined?(YAML)
16-
cfg = RuboCop::Cop::Cop.all.each_with_object({}) { |cop, acc| acc[cop.cop_name] = { "Enabled" => false } unless cop.cop_name.start_with?("Chef") }
17-
File.open(dst.join("disable_all.yml"), "w") { |fh| fh.write YAML.dump(cfg) }
18-
19-
sh %{git add #{dst}/{upstream,disable_all}.yml}
20-
sh %{git commit -m "Vendor rubocop-#{upstream.version} upstream configuration." -m "Obvious fix; these changes are the result of automation not creative thinking."}
21-
end
22-
234
require "chefstyle"
245
require "rubocop/rake_task"
256
RuboCop::RakeTask.new(:style) do |task|

‎config/chefstyle.yml

Lines changed: 39 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
AllCops:
22
TargetRubyVersion: 2.5
3+
NewCops: disable
4+
DisabledByDefault: true
35
SuggestExtensions: false
46

57
#
@@ -43,8 +45,8 @@ Lint/DuplicateCaseCondition:
4345
Enabled: true
4446
Lint/DuplicateMethods:
4547
Enabled: true
46-
Lint/DuplicatedKey:
47-
Enabled: true
48+
# Lint/DuplicatedKey: TODO broken with rubocop 1.25.1 move
49+
# Enabled: true
4850
Lint/EachWithObjectArgument:
4951
Enabled: true
5052
Lint/ElseLayout:
@@ -60,8 +62,8 @@ Lint/EmptyWhen:
6062
Layout/EndAlignment:
6163
Enabled: true
6264
AutoCorrect: true
63-
Lint/EndInMethod:
64-
Enabled: true
65+
# Lint/EndInMethod: TODO broken with rubocop 1.25.1 move
66+
# Enabled: true
6567
Lint/EnsureReturn:
6668
Enabled: true
6769
Lint/FloatOutOfRange:
@@ -80,8 +82,8 @@ Lint/LiteralInInterpolation:
8082
Enabled: true
8183
Lint/Loop:
8284
Enabled: true
83-
Lint/MultipleCompare:
84-
Enabled: true
85+
# Lint/MultipleCompare: TODO broken with rubocop 1.25.1 move
86+
# Enabled: true
8587
Lint/NestedMethodDefinition:
8688
Enabled: true
8789
Lint/NextWithoutAccumulator:
@@ -104,14 +106,14 @@ Lint/ShadowedException:
104106
Enabled: true
105107
Lint/ShadowingOuterLocalVariable:
106108
Enabled: true
107-
Lint/StringConversionInInterpolation:
108-
Enabled: true
109+
# Lint/StringConversionInInterpolation: TODO broken with rubocop 1.25.1 move
110+
# Enabled: true
109111
Lint/UnderscorePrefixedVariableName:
110112
Enabled: true
111113
Lint/UnifiedInteger:
112114
Enabled: true
113-
Lint/UnneededSplatExpansion:
114-
Enabled: true
115+
# Lint/UnneededSplatExpansion: TODO broken with rubocop 1.25.1 move
116+
# Enabled: true
115117
Lint/UnreachableCode:
116118
Enabled: true
117119
Lint/UriEscapeUnescape:
@@ -120,8 +122,8 @@ Lint/UselessAccessModifier:
120122
Enabled: true
121123
Lint/UselessAssignment:
122124
Enabled: true
123-
Lint/UselessComparison:
124-
Enabled: true
125+
# Lint/UselessComparison: TODO broken with rubocop 1.25.1 move
126+
# Enabled: true
125127
Lint/UselessElseWithoutRescue:
126128
Enabled: true
127129
Lint/UselessSetterCall:
@@ -142,14 +144,14 @@ Lint/UnusedMethodArgument:
142144
Enabled: false
143145

144146
# We ignore Exceptions a lot
145-
Lint/HandleExceptions:
147+
Lint/SuppressedException:
146148
Enabled: false
147149
# We also decorate Exceptions a lot
148150
Lint/RescueException:
149151
Enabled: false
150152

151153
# disabling this will make it easier to stage chefstyle rollouts
152-
Lint/UnneededCopDisableDirective:
154+
Lint/RedundantCopDisableDirective:
153155
Enabled: false
154156

155157
#
@@ -193,17 +195,17 @@ Metrics/PerceivedComplexity:
193195

194196
Layout/AccessModifierIndentation:
195197
Enabled: true
196-
Layout/AlignArguments:
197-
Enabled: true
198-
EnforcedStyle: with_fixed_indentation
199-
Layout/AlignParameters:
200-
Enabled: true
201-
EnforcedStyle: with_fixed_indentation
198+
# Layout/AlignArguments: TODO broken with rubocop 1.25.1 move
199+
# Enabled: true
200+
# EnforcedStyle: with_fixed_indentation
201+
# Layout/AlignParameters: TODO broken with rubocop 1.25.1 move
202+
# Enabled: true
203+
# EnforcedStyle: with_fixed_indentation
202204
# the "ignore_implict" is here for keyword args in method calls which are
203205
# "implicit" hashes, and those should not be treated like normal hashes
204-
Layout/AlignHash:
205-
Enabled: true
206-
EnforcedLastArgumentHashStyle: ignore_implicit
206+
# Layout/AlignHash: TODO broken with rubocop 1.25.1 move
207+
# Enabled: true
208+
# EnforcedLastArgumentHashStyle: ignore_implicit
207209
Style/AndOr:
208210
Enabled: true
209211
Style/ArrayJoin:
@@ -280,13 +282,13 @@ Style/IfUnlessModifierOfIfUnless:
280282
Enabled: true
281283
Style/IfWithSemicolon:
282284
Enabled: true
283-
Layout/IndentAssignment:
284-
Enabled: true
285-
Layout/IndentFirstArgument:
286-
EnforcedStyle: consistent
287-
Enabled: true
288-
Layout/IndentHeredoc:
289-
Enabled: true
285+
# Layout/IndentAssignment: TODO broken with rubocop 1.25.1 move
286+
# Enabled: true
287+
# Layout/IndentFirstArgument: TODO broken with rubocop 1.25.1 move
288+
# EnforcedStyle: consistent
289+
# Enabled: true
290+
# Layout/IndentHeredoc: TODO broken with rubocop 1.25.1 move
291+
# Enabled: true
290292
Layout/IndentationConsistency:
291293
Enabled: true
292294
Layout/IndentationWidth:
@@ -454,8 +456,8 @@ Style/SymbolProc:
454456
Enabled: true
455457
Layout/IndentationStyle:
456458
Enabled: true
457-
Layout/TrailingBlankLines:
458-
Enabled: true
459+
# Layout/TrailingBlankLines: TODO broken with rubocop 1.25.1 move
460+
# Enabled: true
459461
Style/TrailingCommaInArguments:
460462
Enabled: true
461463
# rubocop's default gets this completely backwards
@@ -469,9 +471,9 @@ Style/TrailingUnderscoreVariable:
469471
Enabled: true
470472
Layout/TrailingWhitespace:
471473
Enabled: true
472-
Style/UnneededCapitalW:
473-
Enabled: true
474-
Style/UnneededInterpolation:
474+
# Style/UnneededCapitalW: TODO broken with rubocop 1.25.1 move
475+
# Enabled: true
476+
Style/RedundantInterpolation:
475477
Enabled: false # buggy: https://github.com/rubocop-hq/rubocop/issues/6099
476478
#Style/UnneededPercentQ: # would like to enable this one but its buggy as of 0.35.1
477479
# Enabled: true
@@ -540,7 +542,7 @@ Style/IfUnlessModifier:
540542
Enabled: false
541543

542544
# Dan is -1 on this one: https://github.com/chef/chef/pull/4526#issuecomment-179950045
543-
Layout/IndentFirstHashElement:
545+
Layout/FirstHashElementIndentation:
544546
Enabled: false
545547

546548
# This is overly aggressive and autofix broke stuff, would need to exclude classes
@@ -610,17 +612,13 @@ Naming/HeredocDelimiterNaming:
610612
Enabled: false
611613
Naming/PredicateName:
612614
Enabled: false
613-
Naming/UncommunicativeMethodParamName:
615+
Naming/MethodParameterName:
614616
Enabled: false
615617
Naming/MemoizedInstanceVariableName:
616618
Enabled: false
617619
Naming/VariableNumber:
618620
Enabled: false
619621

620-
# This breaks all kinds of specs in chef (i don't think it will ever quite work correctly)
621-
Style/BracesAroundHashParameters:
622-
Enabled: false
623-
624622
# We almost never use format strings but this cop triggers on any string with "%{whatever}" in it and is 99% false positives
625623
Style/FormatStringToken:
626624
Enabled: false

‎config/default.yml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,2 @@
11
inherit_from:
2-
- upstream.yml
3-
- disable_all.yml
42
- chefstyle.yml

0 commit comments

Comments
 (0)