Use rubocop-modularization (#31)

* Use rubocop-modularization

* Only run tests on push

* rubocop

.github/workflows/ci.yml

@@ -1,6 +1,6 @@
 name: CI
 
-on: [push, pull_request]
+on: [push]
 
 jobs:
   build:

Gemfile.lock

@@ -1,10 +1,11 @@
 PATH
   remote: .
   specs:
-    package_protections (2.2.1)
+    package_protections (2.3.0)
       activesupport
       parse_packwerk
       rubocop
+      rubocop-modularization
       rubocop-sorbet
       sorbet-runtime
 
@@ -22,12 +23,13 @@ GEM
     diff-lcs (1.5.0)
     i18n (1.12.0)
       concurrent-ruby (~> 1.0)
+    json (2.6.2)
     method_source (1.0.0)
     minitest (5.16.3)
-    parallel (1.21.0)
+    parallel (1.22.1)
     parse_packwerk (0.12.1)
       sorbet-runtime
-    parser (3.1.0.0)
+    parser (3.1.2.1)
       ast (~> 2.4.1)
     pry (0.14.1)
       coderay (~> 1.1)
@@ -39,7 +41,7 @@ GEM
       parser
       sorbet-runtime (>= 0.5.9204)
       unparser
-    regexp_parser (2.2.0)
+    regexp_parser (2.6.0)
     rexml (3.2.5)
     rspec (3.10.0)
       rspec-core (~> 3.10.0)
@@ -54,25 +56,32 @@ GEM
       diff-lcs (>= 1.2.0, < 2.0)
       rspec-support (~> 3.10.0)
     rspec-support (3.10.3)
-    rubocop (1.25.0)
+    rubocop (1.36.0)
+      json (~> 2.3)
       parallel (~> 1.10)
-      parser (>= 3.1.0.0)
+      parser (>= 3.1.2.1)
       rainbow (>= 2.2.2, < 4.0)
       regexp_parser (>= 1.8, < 3.0)
-      rexml
-      rubocop-ast (>= 1.15.1, < 2.0)
+      rexml (>= 3.2.5, < 4.0)
+      rubocop-ast (>= 1.20.1, < 2.0)
       ruby-progressbar (~> 1.7)
       unicode-display_width (>= 1.4.0, < 3.0)
-    rubocop-ast (1.15.1)
-      parser (>= 3.0.1.1)
+    rubocop-ast (1.21.0)
+      parser (>= 3.1.1.0)
+    rubocop-modularization (0.0.3)
+      activesupport
+      parse_packwerk
+      rubocop
+      rubocop-sorbet
+      sorbet-runtime
     rubocop-rspec (2.8.0)
       rubocop (~> 1.19)
     rubocop-sorbet (0.6.11)
       rubocop (>= 0.90.0)
     ruby-progressbar (1.11.0)
     sorbet (0.5.9588)
       sorbet-static (= 0.5.9588)
-    sorbet-runtime (0.5.9619)
+    sorbet-runtime (0.5.10481)
     sorbet-static (0.5.9588-universal-darwin-14)
     sorbet-static (0.5.9588-universal-darwin-15)
     sorbet-static (0.5.9588-universal-darwin-16)
@@ -98,7 +107,7 @@ GEM
     thor (1.2.1)
     tzinfo (2.0.5)
       concurrent-ruby (~> 1.0)
-    unicode-display_width (2.1.0)
+    unicode-display_width (2.3.0)
     unparser (0.6.3)
       diff-lcs (~> 1.3)
       parser (>= 3.1.0)

lib/package_protections.rb

@@ -1,12 +1,14 @@
 # frozen_string_literal: true
 
 # typed: strict
+
 require 'sorbet-runtime'
 require 'open3'
 require 'set'
 require 'parse_packwerk'
 require 'rubocop'
 require 'rubocop-sorbet'
+require 'rubocop-modularization'
 
 #
 # Welcome to PackageProtections!
@@ -66,7 +68,7 @@ def self.config
   sig { params(identifier: Identifier).returns(ProtectionInterface) }
   def self.with_identifier(identifier)
     @map ||= T.let(@map, T.nilable(T::Hash[Identifier, ProtectionInterface]))
-    @map ||= all.map { |protection| [protection.identifier, protection] }.to_h
+    @map ||= all.to_h { |protection| [protection.identifier, protection] }
     @map.fetch(identifier)
   end
 

lib/package_protections/per_file_violation.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 # typed: strict
+
 module PackageProtections
   # Perhaps this should be in ParsePackwerk. For now, this is here to help us break down violations per file.
   # This is analogous to `Packwerk::ReferenceOffense`

lib/package_protections/private.rb

@@ -128,7 +128,7 @@ def self.private_cop_config(identifier)
       @private_cop_config ||= begin
         protected_packages = all_protected_packages
         protection = T.cast(PackageProtections.with_identifier(identifier), PackageProtections::RubocopProtectionInterface)
-        protected_packages.map { |p| [p.name, protection.custom_cop_config(p)] }.to_h
+        protected_packages.to_h { |p| [p.name, protection.custom_cop_config(p)] }
       end
     end
 

lib/package_protections/private/metadata_modifiers.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 # typed: strict
+
 module PackageProtections
   module Private
     class MetadataModifiers

lib/package_protections/private/output.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 # typed: strict
+
 module PackageProtections
   module Private
     class Output

lib/package_protections/protected_package.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 # typed: strict
+
 module PackageProtections
   class ProtectedPackage < T::Struct
     extend T::Sig

lib/package_protections/protection_interface.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 # typed: strict
+
 module PackageProtections
   module ProtectionInterface
     extend T::Sig

lib/package_protections/rspec/application_fixture_helper.rb

@@ -17,7 +17,6 @@ def write_package_yml(
     enforce_dependencies: true,
     enforce_privacy: true,
     protections: {},
-    global_namespaces: [],
     visible_to: []
   )
     defaults = {
@@ -33,10 +32,6 @@ def write_package_yml(
       metadata.merge!('visible_to' => visible_to)
     end
 
-    if global_namespaces.any?
-      metadata.merge!('global_namespaces' => global_namespaces)
-    end
-
     package = ParsePackwerk::Package.new(
       name: pack_name,
       dependencies: dependencies,

lib/package_protections/rubocop_protection_interface.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 # typed: strict
+
 module PackageProtections
   module RubocopProtectionInterface
     class CopConfig < T::Struct
@@ -9,6 +10,7 @@ class CopConfig < T::Struct
       const :enabled, T::Boolean, default: true
       const :include_paths, T::Array[String], default: []
       const :exclude_paths, T::Array[String], default: []
+      const :metadata, T.untyped, default: {}
 
       sig { returns(String) }
       def to_rubocop_yml_compatible_format
@@ -22,6 +24,10 @@ def to_rubocop_yml_compatible_format
           cop_config['Exclude'] = exclude_paths
         end
 
+        if metadata.any?
+          cop_config.merge!(metadata)
+        end
+
         { name => cop_config }.to_yaml.gsub("---\n", '')
       end
     end

lib/package_protections/violation_behavior.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 # typed: strict
+
 module PackageProtections
   class IncorrectPublicApiUsageError < StandardError; end
 

lib/rubocop/cop/package_protections/namespaced_under_package_name.rb

@@ -2,81 +2,14 @@
 
 # For String#camelize
 require 'active_support/core_ext/string/inflections'
-require 'rubocop/cop/package_protections/namespaced_under_package_name/desired_zeitwerk_api'
 
 module RuboCop
   module Cop
     module PackageProtections
-      #
-      # TODO:
-      # This class is in serious need of being split up into helpful abstractions.
-      # A really helpful abstraction would be one that takes a file path and can spit out information about
-      # namespacing, such as the exposed namespace, the file path for a different namespace, and more.
-      #
       class NamespacedUnderPackageName < Base
         extend T::Sig
-
-        include RangeHelp
         include ::PackageProtections::RubocopProtectionInterface
 
-        sig { void }
-        def on_new_investigation
-          absolute_filepath = Pathname.new(processed_source.file_path)
-          relative_filepath = absolute_filepath.relative_path_from(Pathname.pwd)
-          relative_filename = relative_filepath.to_s
-
-          # This cop only works for files ruby files in `app`
-          return if !relative_filename.include?('app/') || relative_filepath.extname != '.rb'
-
-          relative_filename = relative_filepath.to_s
-          package_for_path = ParsePackwerk.package_from_path(relative_filename)
-          return if package_for_path.nil?
-
-          namespace_context = self.class.desired_zeitwerk_api.for_file(relative_filename, package_for_path)
-          return if namespace_context.nil?
-
-          allowed_global_namespaces = Set.new([
-                                                namespace_context.expected_namespace,
-                                                *::PackageProtections.config.globally_permitted_namespaces
-                                              ])
-
-          package_name = package_for_path.name
-          actual_namespace = namespace_context.current_namespace
-
-          if allowed_global_namespaces.include?(actual_namespace)
-            # No problem!
-          else
-            package_enforces_namespaces = !::PackageProtections::ProtectedPackage.from(package_for_path).violation_behavior_for(NamespacedUnderPackageName::IDENTIFIER).fail_never?
-            expected_namespace = namespace_context.expected_namespace
-            relative_desired_path = namespace_context.expected_filepath
-            pack_owning_this_namespace = self.class.namespaces_to_packs[actual_namespace]
-
-            if package_enforces_namespaces
-              add_offense(
-                source_range(processed_source.buffer, 1, 0),
-                message: format(
-                  '`%<package_name>s` prevents modules/classes that are not submodules of the package namespace. Should be namespaced under `%<expected_namespace>s` with path `%<expected_path>s`. See https://go/packwerk_cheatsheet_namespaces for more info.',
-                  package_name: package_name,
-                  expected_namespace: expected_namespace,
-                  expected_path: relative_desired_path
-                )
-              )
-            elsif pack_owning_this_namespace
-              add_offense(
-                source_range(processed_source.buffer, 1, 0),
-                message: format(
-                  '`%<pack_owning_this_namespace>s` prevents other packs from sitting in the `%<actual_namespace>s` namespace. This should be namespaced under `%<expected_namespace>s` with path `%<expected_path>s`. See https://go/packwerk_cheatsheet_namespaces for more info.',
-                  package_name: package_name,
-                  pack_owning_this_namespace: pack_owning_this_namespace,
-                  expected_namespace: expected_namespace,
-                  actual_namespace: actual_namespace,
-                  expected_path: relative_desired_path
-                )
-              )
-            end
-          end
-        end
-
         # We override `cop_configs` for this protection.
         # The default behavior disables cops when a package has turned off a protection.
         # However: namespace violations can occur even when one package has TURNED OFF their namespace protection
@@ -87,22 +20,34 @@ def on_new_investigation
           .returns(T::Array[::PackageProtections::RubocopProtectionInterface::CopConfig])
         end
         def cop_configs(packages)
-          include_paths = T.let([], T::Array[String])
+          include_packs = T.let([], T::Array[String])
           packages.each do |p|
-            included_globs_for_pack.each do |glob|
-              include_paths << p.original_package.directory.join(glob).to_s
+            enabled_for_pack = !p.violation_behavior_for(NamespacedUnderPackageName::IDENTIFIER).fail_never?
+            if enabled_for_pack
+              include_packs << p.name
             end
           end
 
           [
             ::PackageProtections::RubocopProtectionInterface::CopConfig.new(
               name: cop_name,
-              enabled: include_paths.any?,
-              include_paths: include_paths
+              enabled: include_packs.any?,
+              metadata: {
+                'IncludePacks' => include_packs,
+                'GloballyPermittedNamespaces' => ::PackageProtections.config.globally_permitted_namespaces
+              }
             )
           ]
         end
 
+        sig { override.returns(T::Array[String]) }
+        def included_globs_for_pack
+          [
+            'app/**/*',
+            'lib/**/*'
+          ]
+        end
+
         IDENTIFIER = T.let('prevent_this_package_from_creating_other_namespaces'.freeze, String)
 
         sig { override.returns(String) }
@@ -132,14 +77,6 @@ def unmet_preconditions_for_behavior(behavior, package)
           end
         end
 
-        sig { override.returns(T::Array[String]) }
-        def included_globs_for_pack
-          [
-            'app/**/*',
-            'lib/**/*'
-          ]
-        end
-
         sig do
           override.params(file: String).returns(String)
         end
@@ -168,31 +105,6 @@ def humanized_protection_description
             See https://go/packwerk_cheatsheet_namespaces for more info.
           MESSAGE
         end
-
-        sig { returns(DesiredZeitwerkApi) }
-        def self.desired_zeitwerk_api
-          # This is cached at the class level so we will cache more expensive operations
-          # across rubocop requests.
-          @desired_zeitwerk_api ||= T.let(nil, T.nilable(DesiredZeitwerkApi))
-          @desired_zeitwerk_api ||= DesiredZeitwerkApi.new
-        end
-
-        sig { returns(T::Hash[String, String]) }
-        def self.namespaces_to_packs
-          @namespaces_to_packs = T.let(nil, T.nilable(T::Hash[String, String]))
-          @namespaces_to_packs ||= begin
-            all_packs_enforcing_namespaces = ParsePackwerk.all.reject do |p|
-              ::PackageProtections::ProtectedPackage.from(p).violation_behavior_for(NamespacedUnderPackageName::IDENTIFIER).fail_never?
-            end
-
-            namespaces_to_packs = {}
-            all_packs_enforcing_namespaces.each do |package|
-              namespaces_to_packs[desired_zeitwerk_api.get_pack_based_namespace(package)] = package.name
-            end
-
-            namespaces_to_packs
-          end
-        end
       end
     end
   end