Use vanilla rubocop tools for per-pack rubocop config

ADVANCED_USAGE.md

@@ -1,32 +1,39 @@
 # Advanced Usage
 
-## Pack-Level `package_rubocop.yml` and `package_rubocop_todo.yml` files
+## Pack-Level `.rubocop.yml` and `.rubocop_todo.yml` files
 `rubocop-packs` also has some API that help you use rubocop in a pack-based context.
 
-### Basic Configuration
-In your top-level `.rubocop.yml` file, you'll want to include configuration from `rubocop-packs`:
+### Per-pack `.rubocop.yml`
+While `rubocop-packs` can be used like any other `rubocop` by configuring in your top-level `.rubocop.yml` file, we also have a number of tools to support per-pack configuration.
+
+To add a per-pack `.rubocop.yml`, you just need to create a `packs/your_pack/.rubocop.yml`. With this, each pack can specify an allow-listed set of cops (see below) that can be configured on a per-package level, while extending the root level config.
+
+Example:
 ```yml
-inherit_gem:
-  rubocop-packs:
-    - config/default.yml
-```
+inherit_from: '../../.rubocop.yml'
 
-This is the mechanism by which pack level rubocop files are incorporated into the top-level config.
+Packs/TypedPublicApis:
+  Enabled: true
 
-### Per-pack `package_rubocop.yml`
-While `rubocop-packs` can be used like any other `rubocop` by configuring in your top-level `.rubocop.yml` file, we also have a number of tools to support per-pack configuration.
+Packs/RootNamespaceIsPackName:
+  Enabled: true
+
+Packs/DocumentedPublicApis:
+  Enabled: false
 
-To add a per-pack `package_rubocop.yml`, you just need to create a `packs/your_pack/package_rubocop.yml`. With this, each pack can specify an allow-listed set of cops (see below) that can be configured on a per-package level.
+Sorbet/StrictSigil:
+  Enabled: false
+```
 
-### Per-pack `package_rubocop_todo.yml`
-To create a per-pack `package_rubocop_todo.yml`, you can use the following API from `rubocop-packs`:
+### Per-pack `.rubocop_todo.yml`
+To create a per-pack `.rubocop_todo.yml`, you can use the following API from `rubocop-packs`:
 ```ruby
 RuboCop::Packs.regenerate_todo(packs: Packs.all)
 ```
-This API will auto-generate a `packs/some_pack/package_rubocop_todo.yml`. This allows a pack to own its own exception list.
+This API will auto-generate a `packs/some_pack/.rubocop_todo.yml`. This allows a pack to own its own exception list.
 
 ### Configuration and Validation
-To use per-pack `package_rubocop.yml` and `package_rubocop_todo.yml` files, you need to configure `rubocop-packs`:
+To use per-pack `.rubocop.yml` and `.rubocop_todo.yml` files, you need to configure `rubocop-packs`:
 ```ruby
 # config/rubocop_packs.rb
 RuboCop::Packs.configure do |config|
@@ -43,5 +50,5 @@ end
 ```
 
 Validations include:
-- Ensuring that `packs/*/package_rubocop_todo.yml` files only contain exceptions for the allow-list of `permitted_pack_level_cops`
-- Ensuring that `packs/*/package_rubocop.yml` files contain all of the cops listed in `required_pack_level_cops` and no other cops. This is to ensure that these files are only used to turn on and off an allow-list of cops, as most users would not want packs to configure most `rubocop` rules in a way that is different from the rest of the application.
+- Ensuring that `packs/*/.rubocop_todo.yml` files only contain exceptions for the allow-list of `permitted_pack_level_cops`
+- Ensuring that `packs/*/.rubocop.yml` files contain all of the cops listed in `required_pack_level_cops` and no other cops. This is to ensure that these files are only used to turn on and off an allow-list of cops, as most users would not want packs to configure most `rubocop` rules in a way that is different from the rest of the application.

Gemfile.lock

@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    rubocop-packs (0.0.35)
+    rubocop-packs (0.0.36)
       activesupport
       packs
       parse_packwerk
@@ -30,7 +30,7 @@ GEM
     packs (0.0.6)
       sorbet-runtime
     parallel (1.22.1)
-    parse_packwerk (0.18.0)
+    parse_packwerk (0.18.2)
       sorbet-runtime
     parser (3.1.2.1)
       ast (~> 2.4.1)
@@ -75,7 +75,7 @@ GEM
       activesupport
       bundler
       rubocop (>= 1.22.0)
-    rubocop-sorbet (0.6.11)
+    rubocop-sorbet (0.7.0)
       rubocop (>= 0.90.0)
     ruby-progressbar (1.11.0)
     sorbet (0.5.10479)

README.md

@@ -41,7 +41,7 @@ Packs/RootNamespaceIsPackName:
     - lib/example.rb
 ```
 
-## Pack-Level `package_rubocop.yml` and `package_rubocop_todo.yml` files
+## Pack-Level `.rubocop.yml` and `.rubocop_todo.yml` files
 See [ADVANCED_USAGE.md](ADVANCED_USAGE.md)
 
 ## Contributing

config/default.yml

@@ -27,11 +27,3 @@ PackwerkLite/Privacy:
 PackwerkLite/Dependency:
   # It is recommended to use packwerk
   Enabled: false
-
-# We do this inherit *after* setting the defaults so that pack-specific rubocops can override the defaults
-# Relevant documentation:
-#  - Inheriting config from a gem:
-#    - https://docs.rubocop.org/rubocop/configuration.html#inheriting-configuration-from-a-dependency-gem
-#  - ERB in a .rubocop.yml file
-#    - https://docs.rubocop.org/rubocop/configuration.html#pre-processing
-<%= RuboCop::Packs.pack_based_rubocop_config %>

docs/packwerk_lite.md

@@ -16,7 +16,7 @@ So why would we want to do this...?
 - Experimentally and hypothetically, as a way to provide early feedback loop for engineers who are more familiar with rubocop
 
 What is missing?
-- In order to run this, we need to read from AND write to `package_todo.yml` file so that they pick up and ignore the same things (right now we only read from). We'd likely also need to expose a validation to allow the client to confirm that the `**/package_rubocop_todo.yml` entries for `PackwerkLite/Privacy` and `PackwerkLite/Dependency` are empty.
+- In order to run this, we need to read from AND write to `package_todo.yml` file so that they pick up and ignore the same things (right now we only read from). We'd likely also need to expose a validation to allow the client to confirm that the `**/.rubocop_todo.yml` entries for `PackwerkLite/Privacy` and `PackwerkLite/Dependency` are empty.
 - There were very few false positives (things this picked up that `packwerk` did not) in Gusto's codebase, but we'd want to address those too.
 
 # Appendix

lib/rubocop/packs.rb

@@ -7,13 +7,8 @@ module RuboCop
   module Packs
     extend T::Sig
 
-    # Pack-level rubocop and rubocop_todo YML files are named differently because they are not integrated
-    # into rubocop in the standard way. For example, we could call these the standard `.rubocop.yml` and
-    # `.rubocop_todo.yml`. However, this introduces a number of path relativity issues (https://docs.rubocop.org/rubocop/configuration.html#path-relativity)
-    # that make this approach not possible. Therefore, for pack level rubocops, we name them in a way that mirrors packwerk `package_todo.yml` files
-    # for consistency and to ensure that thes are not read by rubocop except via the ERB templating mechanism.
-    PACK_LEVEL_RUBOCOP_YML = 'package_rubocop.yml'
-    PACK_LEVEL_RUBOCOP_TODO_YML = 'package_rubocop_todo.yml'
+    PACK_LEVEL_RUBOCOP_YML = '.rubocop.yml'
+    PACK_LEVEL_RUBOCOP_TODO_YML = '.rubocop_todo.yml'
 
     PROJECT_ROOT   = T.let(Pathname.new(__dir__).parent.parent.expand_path.freeze, Pathname)
     CONFIG_DEFAULT = T.let(PROJECT_ROOT.join('config', 'default.yml').freeze, Pathname)
@@ -58,7 +53,7 @@ def self.regenerate_todo(packs: [], files: [])
         offenses_for_pack.group_by(&:filepath).each do |filepath, offenses_by_filepath|
           offenses_by_filepath.map(&:cop_name).uniq.each do |cop_name|
             rubocop_todo[cop_name] ||= { 'Exclude' => [] }
-            rubocop_todo[cop_name]['Exclude'] << filepath
+            rubocop_todo[cop_name]['Exclude'] << Pathname.new(filepath).relative_path_from(pack.relative_path).to_s
           end
         end
 
@@ -92,46 +87,6 @@ def self.set_default_rubocop_yml(packs:)
       end
     end
 
-    sig { params(root_pathname: String).returns(String) }
-    # It would be great if rubocop (upstream) could take in a glob for `inherit_from`, which
-    # would allow us to delete this method and this additional complexity.
-    def self.pack_based_rubocop_config(root_pathname: Bundler.root)
-      rubocop_config = {}
-      # We do this because when the ERB is evaluated Dir.pwd is at the directory containing the YML.
-      # Ideally rubocop wouldn't change the PWD before invoking this method.
-      Dir.chdir(root_pathname) do
-        ::Packs.all.each do |package|
-          rubocop_todo = package.relative_path.join(PACK_LEVEL_RUBOCOP_TODO_YML)
-          if rubocop_todo.exist?
-            loaded_rubocop_todo = YAML.load_file(rubocop_todo)
-            loaded_rubocop_todo.each do |cop_name, key_config|
-              rubocop_config[cop_name] ||= {}
-              rubocop_config[cop_name]['Exclude'] ||= []
-              rubocop_config[cop_name]['Exclude'] += key_config['Exclude']
-            end
-          end
-
-          pack_rubocop = package.relative_path.join(PACK_LEVEL_RUBOCOP_YML)
-          next unless pack_rubocop.exist?
-
-          loaded_pack_rubocop = YAML.load_file(pack_rubocop)
-          loaded_pack_rubocop.each do |cop_name, key_config|
-            rubocop_config[cop_name] ||= {}
-
-            if key_config['Enabled']
-              rubocop_config[cop_name]['Include'] ||= []
-              rubocop_config[cop_name]['Include'] << package.relative_path.join('**/*').to_s
-            else
-              rubocop_config[cop_name]['Exclude'] ||= []
-              rubocop_config[cop_name]['Exclude'] << package.relative_path.join('**/*').to_s
-            end
-          end
-        end
-      end
-
-      YAML.dump(rubocop_config)
-    end
-
     sig { void }
     def self.bust_cache!
       config.bust_cache!

lib/rubocop/packs/private.rb

@@ -87,6 +87,8 @@ def self.validate_rubocop_yml(package)
         end
 
         loaded_rubocop_yml.each_key do |key|
+          next if key == 'inherit_from'
+
           if !Packs.config.permitted_pack_level_cops.include?(key)
             errors << <<~ERROR_MESSAGE
               #{rubocop_yml} contains invalid configuration for #{key}.

rubocop-packs.gemspec

@@ -1,6 +1,6 @@
 Gem::Specification.new do |spec|
   spec.name          = 'rubocop-packs'
-  spec.version       = '0.0.35'
+  spec.version       = '0.0.36'
   spec.authors       = ['Gusto Engineers']
   spec.email         = ['dev@gusto.com']
   spec.summary       = 'A collection of Rubocop rules for gradually modularizing a ruby codebase'
@@ -19,7 +19,7 @@ Gem::Specification.new do |spec|
   end
 
   # Specify which files should be added to the gem when it is released.
-  spec.files = Dir['README.md', 'lib/**/*', 'config/default.yml', 'config/pack_config.yml']
+  spec.files = Dir['README.md', 'lib/**/*', 'config/default.yml']
   spec.require_paths = ['lib']
   spec.required_ruby_version = '>= 2.7'