Merge pull request #965 from dafyddcrosby/main

Simplify setting default cops

DEVELOPER_GUIDE.md

@@ -1,31 +1,21 @@
 ## How It Works
 
-The project itself is a derivative of [finstyle](https://github.com/fnichol/finstyle), but starts with all cops (rules) disabled. Cookstyle has a direct dependency on one specific version of RuboCop (at a time), and patches it to load the upstream configuration and custom set of cop updates. When a new RuboCop release comes out, this library can bump the pinned RuboCop version dependency and [re-vendor](https://github.com/chef/cookstyle/blob/main/Rakefile) the upstream configuration to determine if any breaking style or lint cops were added/dropped/reversed/etc.
+The project itself is a derivative of [finstyle](https://github.com/fnichol/finstyle), but starts with all cops (rules) disabled. Cookstyle has a direct dependency on one specific version of RuboCop (at a time), and patches it to load the upstream configuration and custom set of cop updates. When a new RuboCop release comes out, this library can bump the pinned RuboCop version dependency.
 
 ## NOTE CAREFULLY ABOUT UPDATING COOKSTYLE
 
 Cookstyle is designed to allow bumping the RuboCop engine while keeping backwards compatibility with the config and not requiring all the cookbooks to update. This isn't *always* possible since many bugfixes catch new issues in cookbooks, but we do our best to prevent constantly linting of large codebases.
 
 Enabling or disabling any new cops in this file must be accompanied by comments demonstrating why the change should be made. Keep in mind anything you add here causes Chef users significant work, so make sure it's worth it.
 
-## Cookstyle Configuration Files
-
-Cookstyle works by overwriting RuboCop's standard config with our own default.yml file. This file sources three other files to provide the final layer-cake that is Cookstyle:
-
-[upstream.yml](https://github.com/chef/cookstyle/blob/main/config/upstream.yml)
-
-The upstream.yml config file is a vendored copy of the out-of-the-box RuboCop config from the version of the engine we're using. It's necessary to start with this file as it includes cop metadata like descriptions and also includes the actual configuration details for each cop.
-
-[disable_all.yml](https://github.com/chef/cookstyle/blob/main/config/disable_all.yml)
-
-The disable_all.yml is loaded after the upstream.yml config in order to disable all of the cops that ship with RuboCop. This ensures that we only turn on the cops we want. Since updating the RuboCop engine also updates this file, we'll never introduce a new cop just by updating the engine.
+## Cookstyle Configuration File
 
 [cookstyle.yml](https://github.com/chef/cookstyle/blob/main/config/cookstyle.yml)
 
-The cookstyle.yml config file is the enabled.yml file from RuboCop 0.37.2 combined with a sprinkling of cops released in the dozens of RuboCop releases since then as well as configs for all our own Chef cops. All cops enabled since 0.37.2 include a comment or link to a PR so you can see the justification for adding the cop.
+The cookstyle.yml config file is the enabled.yml file from RuboCop 0.37.2 combined with a sprinkling of cops released in the dozens of RuboCop releases since then as well as configs for all our own Chef cops. All cops enabled since 0.37.2 include a comment or link to a PR so you can see the justification for adding the cop. The file takes an allowlist approach to cops, with AllCops having DisabledByDefault as true. If a rule is not explicitly enabled, it will not run.
 
 `NOTE`: The `cookstyle.yml` file was generated by hand and it may be necessary to edit that file (again by hand) to resolve issues with later RuboCop engines changing cop names or behavior.
 
 ## Updating the RuboCop Engine
 
-Cookstyle ships with a rake task to aid in updating RuboCop releases. Start by updating the RuboCop version in the [lib/cookstyle/version.rb](https://github.com/chef/cookstyle/blob/main/lib/cookstyle/version.rb) file. Then run `bundle update` and `bundle exec rake vendor` to update the `upstream.yml` and `disable_all.yml` files. You'll want to check the changes in the `upstream.yml` file. Updating this file often requires updating the configs in the cookstyle.yml to prevent warnings when running Cookstyle.
+Before updating the engine, save a copy of the current engine's rule output with `cookstyle --show-cops`. Start by updating the RuboCop version in the [lib/cookstyle/version.rb](https://github.com/chef/cookstyle/blob/main/lib/cookstyle/version.rb) file. Then run `bundle update` to update the RuboCop gem. Confirm the engine is updated with `cookstyle --version`. Then save the output from another run of `cookstyle --show-cops` and compare the older output to the new output using `diff` and `shasum` to confirm that there are no new/unexplained rules being added or removed between the engine releases.

Rakefile

@@ -3,25 +3,6 @@ require 'bundler/gem_tasks'
 
 Dir['tasks/**/*.rake'].each { |t| load t }
 
-upstream = Gem::Specification.find_by_name('rubocop')
-
-desc "Vendor rubocop-#{upstream.version} configuration into gem"
-task :vendor do
-  src = Pathname.new(upstream.gem_dir).join('config')
-  dst = Pathname.new(__FILE__).dirname.join('config')
-
-  mkdir_p dst
-  cp(src.join('default.yml'), dst.join('upstream.yml'))
-
-  require 'rubocop'
-  require 'yaml' unless defined?(YAML)
-  cfg = RuboCop::Cop::Cop.all.each_with_object({}) { |cop, acc| acc[cop.cop_name] = { 'Enabled' => false } unless cop.cop_name.start_with?('Chef'); }
-  File.write(dst.join('disable_all.yml'), YAML.dump(cfg))
-
-  sh %(git add #{dst}/{upstream,disable_all}.yml)
-  sh %(git commit -m "Vendor rubocop-#{upstream.version} upstream configuration." -m "Obvious fix; these changes are the result of automation not creative thinking.")
-end
-
 require 'cookstyle'
 desc 'Run cookstyle against cookstyle'
 task :style do

config/cookstyle.yml

@@ -1,5 +1,7 @@
 AllCops:
   SuggestExtensions: false
+  NewCops: disable
+  DisabledByDefault: true
   TargetRubyVersion: 2.5
   TargetChefVersion: ~
   inherit_mode:
@@ -2371,10 +2373,10 @@ Chef/Security/SshPrivateKey:
 
 Layout/AccessModifierIndentation:
   Enabled: true
-Layout/AlignArray:
-  Enabled: true
-Layout/AlignHash:
-  Enabled: true
+# Layout/AlignArray: TODO broken with rubocop 1.25.1 move
+#   Enabled: true
+# Layout/AlignHash: TODO broken with rubocop 1.25.1 move
+#   Enabled: true
 Style/AndOr:
   Enabled: true
 Style/ArrayJoin:
@@ -2393,8 +2395,6 @@ Layout/BlockEndNewline:
   Enabled: true
 Style/BlockDelimiters:
   Enabled: true
-Style/BracesAroundHashParameters:
-  Enabled: true
 Style/CaseEquality:
   Enabled: true
 Layout/CaseIndentation:
@@ -2425,8 +2425,8 @@ Naming/ConstantName:
   Enabled: true
 Style/DefWithParentheses:
   Enabled: true
-Style/DeprecatedHashMethods:
-  Enabled: true
+# Style/DeprecatedHashMethods: TODO broken with rubocop 1.25.1 move
+#   Enabled: true
 Layout/DotPosition:
   Enabled: true
 Style/EachWithObject:
@@ -2482,12 +2482,12 @@ Layout/IndentationWidth:
   Enabled: true
 Style/IdenticalConditionalBranches:
   Enabled: true
-Layout/IndentArray:
-  Enabled: true
-Layout/IndentAssignment:
-  Enabled: true
-Layout/IndentHash:
-  Enabled: true
+# Layout/IndentArray: TODO broken with rubocop 1.25.1 move
+#   Enabled: true
+# Layout/IndentAssignment: TODO broken with rubocop 1.25.1 move
+#   Enabled: true
+# Layout/IndentHash: TODO broken with rubocop 1.25.1 move
+#   Enabled: true
 Style/InfiniteLoop:
   Enabled: true
 Style/Lambda:
@@ -2498,8 +2498,8 @@ Layout/LeadingCommentSpace:
   Enabled: true
 Style/LineEndConcatenation:
   Enabled: true
-Style/MethodCallParentheses:
-  Enabled: true
+# Style/MethodCallParentheses: TODO broken with rubocop 1.25.1 move
+#   Enabled: true
 Style/MethodDefParentheses:
   Enabled: true
 Style/MultilineBlockChain:
@@ -2536,8 +2536,8 @@ Style/Not:
   Enabled: true
 Style/OneLineConditional:
   Enabled: true
-Naming/OpMethod:
-  Enabled: true
+# Naming/OpMethod: TODO broken with rubocop 1.25.1 move
+#   Enabled: true
 Style/OptionalArguments:
   Enabled: true
 Style/ParallelAssignment:
@@ -2641,12 +2641,12 @@ Style/TrivialAccessors:
   Enabled: true
 Style/UnlessElse:
   Enabled: true
-Style/UnneededCapitalW:
-  Enabled: true
-Style/UnneededInterpolation:
-  Enabled: true
-Style/UnneededPercentQ:
-  Enabled: true
+# Style/UnneededCapitalW: TODO broken with rubocop 1.25.1 move
+#   Enabled: true
+# Style/UnneededInterpolation: TODO broken with rubocop 1.25.1 move
+#   Enabled: true
+# Style/UnneededPercentQ: TODO broken with rubocop 1.25.1 move
+#   Enabled: true
 Style/TrailingUnderscoreVariable:
   Enabled: true
 Style/VariableInterpolation:
@@ -2680,8 +2680,8 @@ Lint/DeprecatedClassMethods:
   Enabled: true
 Lint/DuplicateMethods:
   Enabled: true
-Lint/DuplicatedKey:
-  Enabled: true
+# Lint/DuplicatedKey: TODO broken with rubocop 1.25.1 move
+#   Enabled: true
 Lint/EachWithObjectArgument:
   Enabled: true
 Lint/ElseLayout:
@@ -2693,8 +2693,8 @@ Lint/EmptyInterpolation:
 Layout/EndAlignment:
   Enabled: true
   AutoCorrect: true
-Lint/EndInMethod:
-  Enabled: true
+# Lint/EndInMethod: TODO broken with rubocop 1.25.1 move
+#   Enabled: true
 Lint/EnsureReturn:
   Enabled: true
 Security/Eval:
@@ -2703,8 +2703,8 @@ Lint/FloatOutOfRange:
   Enabled: true
 Lint/FormatParameterMismatch:
   Enabled: true
-Lint/HandleExceptions:
-  Enabled: true
+# Lint/HandleExceptions: TODO broken with rubocop 1.25.1 move
+#   Enabled: true
 Lint/ImplicitStringConcatenation:
   Enabled: true
   Exclude:
@@ -2733,12 +2733,12 @@ Lint/RescueException:
   Enabled: true
 Lint/ShadowingOuterLocalVariable:
   Enabled: true
-Lint/StringConversionInInterpolation:
-  Enabled: true
+# Lint/StringConversionInInterpolation: TODO broken with rubocop 1.25.1 move
+#   Enabled: true
 Lint/UnderscorePrefixedVariableName:
   Enabled: true
-Lint/UnneededCopDisableDirective:
-  Enabled: true
+# Lint/UnneededCopDisableDirective: TODO broken with rubocop 1.25.1 move
+#   Enabled: true
 Lint/UnusedBlockArgument:
   Enabled: true
 Lint/UnusedMethodArgument:
@@ -2749,8 +2749,8 @@ Lint/UselessAccessModifier:
   Enabled: true
 Lint/UselessAssignment:
   Enabled: true
-Lint/UselessComparison:
-  Enabled: true
+# Lint/UselessComparison: TODO broken with rubocop 1.25.1 move
+#   Enabled: true
 Lint/UselessElseWithoutRescue:
   Enabled: true
 Lint/UselessSetterCall:
@@ -2867,8 +2867,8 @@ Bundler/DuplicatedGem:
   Enabled: true
 
 # This results in very confusing code to read with little perf benefit
-Performance/Casecmp:
-  Enabled: false
+# Performance/Casecmp: # TODO: move to cookstyle project .rubocop.yml?
+#   Enabled: false
 
 # This comes with changing the ruby target to 2.3+
 Style/FrozenStringLiteralComment:
@@ -2926,8 +2926,8 @@ Style/MinMax:
 Lint/UriRegexp:
   Enabled: true
 
-Performance/UriDefaultParser:
-  Enabled: true
+# Performance/UriDefaultParser: # TODO: move to cookstyle project .rubocop.yml?
+#   Enabled: true
 
 # :true or :false seems like a horrible idea
 Lint/BooleanSymbol:
@@ -2938,16 +2938,16 @@ Style/RedundantConditional:
   Enabled: true
 
 # catches people writing a regex check wrong
-Lint/RegexpInCondition:
-  Enabled: true
+# Lint/RegexpInCondition: TODO broken with rubocop 1.25.1 move
+#   Enabled: true
 
 # Avoids pointless / complex code
 Lint/RedundantWithObject:
   Enabled: true
 
 # avoid requiring things that come for free
-Lint/UnneededRequireStatement:
-  Enabled: true
+# Lint/UnneededRequireStatement: TODO broken with rubocop 1.25.1 move
+#   Enabled: true
 
 # Avoid poorly formatted methods
 Style/TrailingBodyOnMethodDefinition:
@@ -2970,8 +2970,8 @@ Lint/BigDecimalNew:
   Enabled: true
 
 # remove bogus rubocop comments. We already enabled Disable directives
-Lint/UnneededCopEnableDirective:
-  Enabled: true
+# Lint/UnneededCopEnableDirective: TODO broken with rubocop 1.25.1 move
+#   Enabled: true
 
 # get people on a much simpler ruby 2.4+ way of doing things
 Style/UnpackFirst:

config/default.yml

@@ -1,4 +1,2 @@
 inherit_from:
-  - upstream.yml
-  - disable_all.yml
   - cookstyle.yml