Revert "Use vanilla rubocop tools for per-pack rubocop config (#59)"

This reverts commit 8c4738f.

ADVANCED_USAGE.md

@@ -1,39 +1,32 @@
 # Advanced Usage
 
-## Pack-Level `.rubocop.yml` and `.rubocop_todo.yml` files
+## Pack-Level `package_rubocop.yml` and `package_rubocop_todo.yml` files
 `rubocop-packs` also has some API that help you use rubocop in a pack-based context.
 
-### 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:
+### Basic Configuration
+In your top-level `.rubocop.yml` file, you'll want to include configuration from `rubocop-packs`:
 ```yml
-inherit_from: '../../.rubocop.yml'
-
-Packs/TypedPublicApis:
-  Enabled: true
+inherit_gem:
+  rubocop-packs:
+    - config/default.yml
+```
 
-Packs/RootNamespaceIsPackName:
-  Enabled: true
+This is the mechanism by which pack level rubocop files are incorporated into the top-level config.
 
-Packs/DocumentedPublicApis:
-  Enabled: false
+### 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.
 
-Sorbet/StrictSigil:
-  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.
 
-### Per-pack `.rubocop_todo.yml`
-To create a per-pack `.rubocop_todo.yml`, you can use the following API from `rubocop-packs`:
+### Per-pack `package_rubocop_todo.yml`
+To create a per-pack `package_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/.rubocop_todo.yml`. This allows a pack to own its own exception list.
+This API will auto-generate a `packs/some_pack/package_rubocop_todo.yml`. This allows a pack to own its own exception list.
 
 ### Configuration and Validation
-To use per-pack `.rubocop.yml` and `.rubocop_todo.yml` files, you need to configure `rubocop-packs`:
+To use per-pack `package_rubocop.yml` and `package_rubocop_todo.yml` files, you need to configure `rubocop-packs`:
 ```ruby
 # config/rubocop_packs.rb
 RuboCop::Packs.configure do |config|
@@ -50,5 +43,5 @@ end
 ```
 
 Validations include:
-- 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.
+- 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.

Gemfile.lock

@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    rubocop-packs (0.0.36)
+    rubocop-packs (0.0.35)
       activesupport
       packs
       parse_packwerk
@@ -30,7 +30,7 @@ GEM
     packs (0.0.6)
       sorbet-runtime
     parallel (1.22.1)
-    parse_packwerk (0.18.2)
+    parse_packwerk (0.18.0)
       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.7.0)
+    rubocop-sorbet (0.6.11)
       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 `.rubocop.yml` and `.rubocop_todo.yml` files
+## Pack-Level `package_rubocop.yml` and `package_rubocop_todo.yml` files
 See [ADVANCED_USAGE.md](ADVANCED_USAGE.md)
 
 ## Contributing

config/default.yml

@@ -27,3 +27,11 @@ 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 `**/.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 `**/package_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,8 +7,13 @@ module RuboCop
   module Packs
     extend T::Sig
 
-    PACK_LEVEL_RUBOCOP_YML = '.rubocop.yml'
-    PACK_LEVEL_RUBOCOP_TODO_YML = '.rubocop_todo.yml'
+    # 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'
 
     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)
@@ -53,7 +58,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'] << Pathname.new(filepath).relative_path_from(pack.relative_path).to_s
+            rubocop_todo[cop_name]['Exclude'] << filepath
           end
         end
 
@@ -87,6 +92,46 @@ 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,8 +87,6 @@ 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.36'
+  spec.version       = '0.0.35'
   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']
+  spec.files = Dir['README.md', 'lib/**/*', 'config/default.yml', 'config/pack_config.yml']
   spec.require_paths = ['lib']
   spec.required_ruby_version = '>= 2.7'