Auto-register new validators and mappers (#37)

* Auto-register mappers and validators

* combine team glob validation into mapper

Author: Alex Evanczuk

lib/code_ownership.rb

@@ -27,7 +27,7 @@ def for_file(file)
 
     owner = T.let(nil, T.nilable(CodeTeams::Team))
 
-    Private.mappers.each do |mapper|
+    Private::OwnershipMappers::Interface.all.each do |mapper|
       owner = mapper.map_file_to_owner(file)
       break if owner
     end
@@ -41,7 +41,7 @@ def for_team(team)
     ownership_information = T.let([], T::Array[String])
 
     ownership_information << "# Code Ownership Report for `#{team.name}` Team"
-    Private.mappers.each do |mapper|
+    Private::OwnershipMappers::Interface.all.each do |mapper|
       ownership_information << "## #{mapper.description}"
       codeowners_lines = mapper.codeowners_lines_to_owners
       ownership_for_mapper = []
@@ -69,7 +69,7 @@ class InvalidCodeOwnershipConfigurationError < StandardError
 
   sig { params(filename: String).void }
   def self.remove_file_annotation!(filename)
-    Private.file_annotations_mapper.remove_file_annotation!(filename)
+    Private::OwnershipMappers::FileAnnotations.new.remove_file_annotation!(filename)
   end
 
   sig do
@@ -172,6 +172,6 @@ def self.bust_caches!
     @for_file = nil
     @memoized_values = nil
     Private.bust_caches!
-    Private.mappers.each(&:bust_caches!)
+    Private::OwnershipMappers::Interface.all.each(&:bust_caches!)
   end
 end

lib/code_ownership/private.rb

@@ -10,7 +10,6 @@
 require 'code_ownership/private/validations/files_have_owners'
 require 'code_ownership/private/validations/github_codeowners_up_to_date'
 require 'code_ownership/private/validations/files_have_unique_owners'
-require 'code_ownership/private/validations/no_overlapping_globs'
 require 'code_ownership/private/ownership_mappers/interface'
 require 'code_ownership/private/ownership_mappers/file_annotations'
 require 'code_ownership/private/ownership_mappers/team_globs'
@@ -37,14 +36,7 @@ def self.bust_caches!
 
     sig { params(files: T::Array[String], autocorrect: T::Boolean, stage_changes: T::Boolean).void }
     def self.validate!(files:, autocorrect: true, stage_changes: true)
-      validators = [
-        Validations::FilesHaveOwners.new,
-        Validations::FilesHaveUniqueOwners.new,
-        Validations::GithubCodeownersUpToDate.new,
-        Validations::NoOverlappingGlobs.new,
-      ]
-
-      errors = validators.flat_map do |validator|
+      errors = Validations::Interface.all.flat_map do |validator|
         validator.validation_errors(
           files: files,
           autocorrect: autocorrect,
@@ -58,23 +50,6 @@ def self.validate!(files:, autocorrect: true, stage_changes: true)
       end
     end
 
-    sig { returns(T::Array[Private::OwnershipMappers::Interface]) }
-    def self.mappers
-      [
-        file_annotations_mapper,
-        Private::OwnershipMappers::TeamGlobs.new,
-        Private::OwnershipMappers::PackageOwnership.new,
-        Private::OwnershipMappers::JsPackageOwnership.new,
-        Private::OwnershipMappers::TeamYmlOwnership.new,
-      ]
-    end
-
-    sig { returns(Private::OwnershipMappers::FileAnnotations) }
-    def self.file_annotations_mapper
-      @file_annotations_mapper = T.let(@file_annotations_mapper, T.nilable(Private::OwnershipMappers::FileAnnotations))
-      @file_annotations_mapper ||= Private::OwnershipMappers::FileAnnotations.new
-    end
-
     # Returns a string version of the relative path to a Rails constant,
     # or nil if it can't find something
     sig { params(klass: T.nilable(T.any(Class, Module))).returns(T.nilable(String)) }
@@ -112,7 +87,7 @@ def self.files_by_mapper(files)
       @files_by_mapper ||= begin
         files_by_mapper = files.map { |file| [file, []] }.to_h
 
-        Private.mappers.each do |mapper|
+        Private::OwnershipMappers::Interface.all.each do |mapper|
           mapper.map_files_to_owners(files).each do |file, _team|
             files_by_mapper[file] ||= []
             T.must(files_by_mapper[file]) << mapper.description

lib/code_ownership/private/ownership_mappers/interface.rb

@@ -11,6 +11,22 @@ module Interface
 
         interface!
 
+        class << self
+          extend T::Sig
+
+          sig { params(base: Class).void }
+          def included(base)
+            @mappers ||= T.let(@mappers, T.nilable(T::Array[Class]))
+            @mappers ||= []
+            @mappers << base
+          end
+
+          sig { returns(T::Array[Interface]) }
+          def all
+            T.unsafe(@mappers).map(&:new)
+          end
+        end
+
         #
         # This should be fast when run with ONE file
         #

lib/code_ownership/private/ownership_mappers/team_globs.rb

@@ -8,6 +8,7 @@ module OwnershipMappers
       class TeamGlobs
         extend T::Sig
         include Interface
+        include Validations::Interface
 
         @@map_files_to_owners = T.let(@map_files_to_owners, T.nilable(T::Hash[String, T.nilable(::CodeTeams::Team)])) # rubocop:disable Style/ClassVars
         @@map_files_to_owners = {} # rubocop:disable Style/ClassVars
@@ -114,6 +115,23 @@ def bust_caches!
         def description
           'Team-specific owned globs'
         end
+
+        sig { override.params(files: T::Array[String], autocorrect: T::Boolean, stage_changes: T::Boolean).returns(T::Array[String]) }
+        def validation_errors(files:, autocorrect: true, stage_changes: true)
+          overlapping_globs = OwnershipMappers::TeamGlobs.new.find_overlapping_globs
+
+          errors = T.let([], T::Array[String])
+
+          if overlapping_globs.any?
+            errors << <<~MSG
+              `owned_globs` cannot overlap between teams. The following globs overlap:
+
+              #{overlapping_globs.map { |overlap| "- #{overlap.description}"}.join("\n")}
+            MSG
+          end
+
+          errors
+        end
       end
     end
   end

lib/code_ownership/private/validations/github_codeowners_up_to_date.rb

@@ -105,7 +105,7 @@ def codeowners_file_lines
             map[team.name] = team_github.team
           end
 
-          Private.mappers.flat_map do |mapper|
+          Private::OwnershipMappers::Interface.all.flat_map do |mapper|
             codeowners_lines = mapper.codeowners_lines_to_owners.filter_map do |line, team|
               team_mapping = github_team_map[team&.name]
               next unless team_mapping

lib/code_ownership/private/validations/interface.rb

@@ -12,6 +12,22 @@ module Interface
         sig { abstract.params(files: T::Array[String], autocorrect: T::Boolean, stage_changes: T::Boolean).returns(T::Array[String]) }
         def validation_errors(files:, autocorrect: true, stage_changes: true)
         end
+
+        class << self
+          extend T::Sig
+
+          sig { params(base: Class).void }
+          def included(base)
+            @validators ||= T.let(@validators, T.nilable(T::Array[Class]))
+            @validators ||= []
+            @validators << base
+          end
+
+          sig { returns(T::Array[Interface]) }
+          def all
+            T.unsafe(@validators).map(&:new)
+          end
+        end
       end
     end
   end

lib/code_ownership/private/validations/no_overlapping_globs.rb

@@ -1,24 +1,71 @@
 module CodeOwnership
   RSpec.describe Private::OwnershipMappers::TeamGlobs do
-    before do
-      create_configuration
-      write_file('config/teams/bar.yml', <<~CONTENTS)
-        name: Bar
-        owned_globs:
-          - app/services/bar_stuff/**/**
-          - frontend/javascripts/bar_stuff/**/**
-      CONTENTS
-
-      write_file('app/services/bar_stuff/thing.rb')
-      write_file('frontend/javascripts/bar_stuff/thing.jsx')
-    end
+    before { create_configuration }
+
+    describe 'CodeOwnership.for_file' do
+      before do
+        write_file('config/teams/bar.yml', <<~CONTENTS)
+          name: Bar
+          owned_globs:
+            - app/services/bar_stuff/**/**
+            - frontend/javascripts/bar_stuff/**/**
+        CONTENTS
+
+        write_file('app/services/bar_stuff/thing.rb')
+        write_file('frontend/javascripts/bar_stuff/thing.jsx')
+      end
 
-    it 'can find the owner of ruby files in owned_globs' do
-      expect(CodeOwnership.for_file('app/services/bar_stuff/thing.rb').name).to eq 'Bar'
+      it 'can find the owner of ruby files in owned_globs' do
+        expect(CodeOwnership.for_file('app/services/bar_stuff/thing.rb').name).to eq 'Bar'
+      end
+
+      it 'can find the owner of javascript files in owned_globs' do
+        expect(CodeOwnership.for_file('frontend/javascripts/bar_stuff/thing.jsx').name).to eq 'Bar'
+      end
     end
 
-    it 'can find the owner of javascript files in owned_globs' do
-      expect(CodeOwnership.for_file('frontend/javascripts/bar_stuff/thing.jsx').name).to eq 'Bar'
+    describe 'CodeOwnership.validate!' do
+      context 'two teams own the same exact glob' do
+        before do
+          write_file('config/code_ownership.yml', <<~YML)
+            owned_globs:
+              - '{app,components,config,frontend,lib,packs,spec}/**/*.{rb,rake,js,jsx,ts,tsx}'
+          YML
+
+          write_file('packs/my_pack/owned_file.rb')
+          write_file('frontend/javascripts/blah/my_file.rb')
+          write_file('frontend/javascripts/blah/subdir/my_file.rb')
+
+          write_file('config/teams/bar.yml', <<~CONTENTS)
+            name: Bar
+            owned_globs:
+              - packs/**/**
+              - frontend/javascripts/blah/subdir/my_file.rb
+          CONTENTS
+
+          write_file('config/teams/foo.yml', <<~CONTENTS)
+            name: Foo
+            owned_globs:
+              - packs/**/**
+              - frontend/javascripts/blah/**/**
+          CONTENTS
+        end
+
+        it 'lets the user know that `owned_globs` can not overlap' do
+          expect { CodeOwnership.validate! }.to raise_error do |e|
+            expect(e).to be_a CodeOwnership::InvalidCodeOwnershipConfigurationError
+            puts e.message
+            expect(e.message).to eq <<~EXPECTED.chomp
+              `owned_globs` cannot overlap between teams. The following globs overlap:
+
+              - `packs/**/**` (from `config/teams/bar.yml`), `packs/**/**` (from `config/teams/foo.yml`)
+              - `frontend/javascripts/blah/subdir/my_file.rb` (from `config/teams/bar.yml`), `frontend/javascripts/blah/**/**` (from `config/teams/foo.yml`)
+
+              See https://github.com/rubyatscale/code_ownership#README.md for more details
+            EXPECTED
+          end
+        end
+      end
     end
   end
 end