From 7bab83d165325a37f2d41732383a766d8e973576 Mon Sep 17 00:00:00 2001 From: Martin Emde Date: Tue, 5 Aug 2025 11:17:55 -0700 Subject: [PATCH] add zeitwerk --- .github/workflows/ci.yml | 1 - .rubocop.yml | 2 +- Gemfile | 9 +++ code_ownership.gemspec | 10 +-- lib/code_ownership.rb | 44 +++++------ lib/code_ownership/configuration.rb | 3 +- .../private/extension_loader.rb | 1 + .../ownership_mappers/file_annotations.rb | 2 +- .../private/ownership_mappers/team_globs.rb | 2 +- .../ownership_mappers/team_yml_ownership.rb | 2 +- ...vel_key.rb => pack_ownership_validator.rb} | 0 .../private/validations/files_have_owners.rb | 2 +- .../file_annotations_spec.rb | 2 - spec/zeitwerk_spec.rb | 76 +++++++++++++++++++ 14 files changed, 115 insertions(+), 41 deletions(-) rename lib/code_ownership/private/{permit_pack_owner_top_level_key.rb => pack_ownership_validator.rb} (100%) create mode 100644 spec/zeitwerk_spec.rb diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e4a52bf8..9c7a124f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -12,7 +12,6 @@ jobs: strategy: matrix: ruby: - - 3.1 - 3.2 - 3.3 - 3.4 diff --git a/.rubocop.yml b/.rubocop.yml index 64b735de..94c24cba 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -120,4 +120,4 @@ Style/HashSyntax: Gemspec/DevelopmentDependencies: Enabled: true - EnforcedStyle: gemspec \ No newline at end of file + EnforcedStyle: Gemfile diff --git a/Gemfile b/Gemfile index fa75df15..118353bb 100644 --- a/Gemfile +++ b/Gemfile @@ -1,3 +1,12 @@ source 'https://rubygems.org' gemspec + +gem 'debug' +gem 'packwerk' +gem 'railties' +gem 'rake' +gem 'rspec' +gem 'rubocop' +gem 'sorbet' +gem 'tapioca' diff --git a/code_ownership.gemspec b/code_ownership.gemspec index 2f391226..c2b4837b 100644 --- a/code_ownership.gemspec +++ b/code_ownership.gemspec @@ -30,13 +30,5 @@ Gem::Specification.new do |spec| spec.add_dependency 'code_teams', '~> 1.0' spec.add_dependency 'packs-specification' spec.add_dependency 'sorbet-runtime', '>= 0.5.11249' - - spec.add_development_dependency 'debug' - spec.add_development_dependency 'packwerk' - spec.add_development_dependency 'railties' - spec.add_development_dependency 'rake' - spec.add_development_dependency 'rspec', '~> 3.0' - spec.add_development_dependency 'rubocop' - spec.add_development_dependency 'sorbet' - spec.add_development_dependency 'tapioca' + spec.add_dependency 'zeitwerk' end diff --git a/lib/code_ownership.rb b/lib/code_ownership.rb index ba4b3484..11f7e834 100644 --- a/lib/code_ownership.rb +++ b/lib/code_ownership.rb @@ -2,19 +2,17 @@ # typed: strict -require 'set' require 'code_teams' require 'sorbet-runtime' require 'json' require 'packs-specification' -require 'code_ownership/mapper' -require 'code_ownership/validator' -require 'code_ownership/private' -require 'code_ownership/cli' -require 'code_ownership/configuration' +require 'zeitwerk' + +loader = Zeitwerk::Loader.for_gem +loader.setup if defined?(Packwerk) - require 'code_ownership/private/permit_pack_owner_top_level_key' + require 'code_ownership/private/pack_ownership_validator' end module CodeOwnership @@ -137,22 +135,22 @@ def backtrace_with_ownership(backtrace) # ./app/controllers/some_controller.rb:43:in `block (3 levels) in create' # backtrace_line = if RUBY_VERSION >= '3.4.0' - %r{\A(#{Pathname.pwd}/|\./)? - (?.+) # Matches 'app/controllers/some_controller.rb' - : - (?\d+) # Matches '43' - :in\s - '(?.*)' # Matches "`block (3 levels) in create'" - \z}x - else - %r{\A(#{Pathname.pwd}/|\./)? - (?.+) # Matches 'app/controllers/some_controller.rb' - : - (?\d+) # Matches '43' - :in\s - `(?.*)' # Matches "`block (3 levels) in create'" - \z}x - end + %r{\A(#{Pathname.pwd}/|\./)? + (?.+) # Matches 'app/controllers/some_controller.rb' + : + (?\d+) # Matches '43' + :in\s + '(?.*)' # Matches "`block (3 levels) in create'" + \z}x + else + %r{\A(#{Pathname.pwd}/|\./)? + (?.+) # Matches 'app/controllers/some_controller.rb' + : + (?\d+) # Matches '43' + :in\s + `(?.*)' # Matches "`block (3 levels) in create'" + \z}x + end backtrace.lazy.filter_map do |line| match = line.match(backtrace_line) diff --git a/lib/code_ownership/configuration.rb b/lib/code_ownership/configuration.rb index a93ff985..4fe4d35d 100644 --- a/lib/code_ownership/configuration.rb +++ b/lib/code_ownership/configuration.rb @@ -3,6 +3,7 @@ module CodeOwnership class Configuration < T::Struct extend T::Sig + DEFAULT_JS_PACKAGE_PATHS = T.let(['**/'], T::Array[String]) const :owned_globs, T::Array[String] @@ -31,7 +32,7 @@ def self.fetch skip_codeowners_validation: config_hash.fetch('skip_codeowners_validation', false), raw_hash: config_hash, require_github_teams: config_hash.fetch('require_github_teams', false), - codeowners_path: config_hash.fetch('codeowners_path', '.github'), + codeowners_path: config_hash.fetch('codeowners_path', '.github') ) end diff --git a/lib/code_ownership/private/extension_loader.rb b/lib/code_ownership/private/extension_loader.rb index 2819e735..24db1e8d 100644 --- a/lib/code_ownership/private/extension_loader.rb +++ b/lib/code_ownership/private/extension_loader.rb @@ -8,6 +8,7 @@ module Private module ExtensionLoader class << self extend T::Sig + sig { params(require_directive: String).void } def load(require_directive) # We want to transform the require directive to behave differently diff --git a/lib/code_ownership/private/ownership_mappers/file_annotations.rb b/lib/code_ownership/private/ownership_mappers/file_annotations.rb index 657f4559..6ce6d91f 100644 --- a/lib/code_ownership/private/ownership_mappers/file_annotations.rb +++ b/lib/code_ownership/private/ownership_mappers/file_annotations.rb @@ -18,7 +18,7 @@ class FileAnnotations extend T::Sig include Mapper - TEAM_PATTERN = T.let(%r{\A(?:#|//|-#) @team (?.*)\Z}.freeze, Regexp) + TEAM_PATTERN = T.let(%r{\A(?:#|//|-#) @team (?.*)\Z}, Regexp) DESCRIPTION = 'Annotations at the top of file' sig do diff --git a/lib/code_ownership/private/ownership_mappers/team_globs.rb b/lib/code_ownership/private/ownership_mappers/team_globs.rb index 9caa5f2c..0d6709af 100644 --- a/lib/code_ownership/private/ownership_mappers/team_globs.rb +++ b/lib/code_ownership/private/ownership_mappers/team_globs.rb @@ -17,7 +17,7 @@ class TeamGlobs returns(T::Hash[String, ::CodeTeams::Team]) end def map_files_to_owners - return @@map_files_to_owners if @@map_files_to_owners&.keys && @@map_files_to_owners.keys.count.positive? + return @@map_files_to_owners if @@map_files_to_owners&.keys&.any? @@map_files_to_owners = CodeTeams.all.each_with_object({}) do |team, map| # rubocop:disable Style/ClassVars code_team = TeamPlugins::Ownership.for(team) diff --git a/lib/code_ownership/private/ownership_mappers/team_yml_ownership.rb b/lib/code_ownership/private/ownership_mappers/team_yml_ownership.rb index 21071507..f68112a9 100644 --- a/lib/code_ownership/private/ownership_mappers/team_yml_ownership.rb +++ b/lib/code_ownership/private/ownership_mappers/team_yml_ownership.rb @@ -17,7 +17,7 @@ class TeamYmlOwnership .returns(T::Hash[String, ::CodeTeams::Team]) end def map_files_to_owners(files) - return @@map_files_to_owners if @@map_files_to_owners&.keys && @@map_files_to_owners.keys.count.positive? + return @@map_files_to_owners if @@map_files_to_owners&.keys&.any? @@map_files_to_owners = CodeTeams.all.each_with_object({}) do |team, map| # rubocop:disable Style/ClassVars map[team.config_yml] = team diff --git a/lib/code_ownership/private/permit_pack_owner_top_level_key.rb b/lib/code_ownership/private/pack_ownership_validator.rb similarity index 100% rename from lib/code_ownership/private/permit_pack_owner_top_level_key.rb rename to lib/code_ownership/private/pack_ownership_validator.rb diff --git a/lib/code_ownership/private/validations/files_have_owners.rb b/lib/code_ownership/private/validations/files_have_owners.rb index 221dc96f..676ffca6 100644 --- a/lib/code_ownership/private/validations/files_have_owners.rb +++ b/lib/code_ownership/private/validations/files_have_owners.rb @@ -13,7 +13,7 @@ def validation_errors(files:, autocorrect: true, stage_changes: true) cache = Private.glob_cache file_mappings = cache.mapper_descriptions_that_map_files(files) files_not_mapped_at_all = file_mappings.select do |_file, mapper_descriptions| - mapper_descriptions.count.zero? + mapper_descriptions.none? end errors = T.let([], T::Array[String]) diff --git a/spec/lib/code_ownership/private/ownership_mappers/file_annotations_spec.rb b/spec/lib/code_ownership/private/ownership_mappers/file_annotations_spec.rb index 0fc17c9d..981fc087 100644 --- a/spec/lib/code_ownership/private/ownership_mappers/file_annotations_spec.rb +++ b/spec/lib/code_ownership/private/ownership_mappers/file_annotations_spec.rb @@ -128,8 +128,6 @@ module CodeOwnership end end - - describe '.remove_file_annotation!' do subject(:remove_file_annotation) do CodeOwnership.remove_file_annotation!(filename) diff --git a/spec/zeitwerk_spec.rb b/spec/zeitwerk_spec.rb new file mode 100644 index 00000000..ae0cbebb --- /dev/null +++ b/spec/zeitwerk_spec.rb @@ -0,0 +1,76 @@ +# typed: false +# frozen_string_literal: true + +# Zeitwerk Compliance Smoke Test +# +# This test serves as a Zeitwerk compliance smoke test that validates the gem's +# file structure and naming conventions. It ensures that all files in the gem +# follow Zeitwerk's strict naming conventions by forcing the autoloader to +# eagerly load every constant and file in the gem. +# +# How it works: +# 1. Eager Loading: Forces Zeitwerk to immediately load all files and constants +# in the gem, rather than loading them on-demand +# 2. Error Detection: If there are any naming convention violations, Zeitwerk +# will raise an error during this process +# 3. Validation: The test passes only if no errors are raised +# +# What it catches: +# - Misnamed files (e.g., my_class.rb should define MyClass) +# - Incorrect directory structure relative to module nesting +# - Missing constants (files that exist but don't define expected constants) +# - Extra or orphaned files that don't follow naming patterns +# - Namespace violations (constants defined in wrong namespace for their location) + +RSpec.describe 'zeitwerk autoloader' do + it 'werks (successfully loads the gem)' do + Zeitwerk::Loader.eager_load_namespace(CodeOwnership) + rescue StandardError => e + # Enhance the error message with more specific information + enhanced_message = build_enhanced_error_message(e) + raise enhanced_message + end + + private + + def build_enhanced_error_message(error) + message_parts = [ + 'Zeitwerk eager loading failed with the following error:', + '', + "Original Error: #{error.class}: #{error.message}", + '' + ] + + # Add backtrace information to help identify the problematic file + if error.backtrace + gem_related_trace = error.backtrace.select do |line| + line.include?('lib/') || line.include?('zeitwerk') + end + + if gem_related_trace.any? + message_parts << 'Relevant backtrace:' + gem_related_trace.first(5).each do |line| + message_parts << " #{line}" + end + message_parts << '' + end + end + + # Try to identify which file might be causing the issue + if /(?:wrong constant name|uninitialized constant|expected.*to define)/i.match?(error.message) + message_parts << 'This error typically indicates:' + message_parts << "- A file name doesn't match its constant name" + message_parts << '- A constant is defined in the wrong namespace' + message_parts << "- A file exists but doesn't define the expected constant" + message_parts << '' + end + + # Add helpful debugging information + message_parts << 'To debug this issue:' + message_parts << '1. Check that all files in lib/ follow zeitwerk naming conventions' + message_parts << '2. Ensure each file defines a constant matching its file path' + message_parts << '3. Verify modules/classes are in the correct namespace' + + message_parts.join("\n") + end +end