diff --git a/code_ownership.gemspec b/code_ownership.gemspec index cdf01f5..5cd0cb0 100644 --- a/code_ownership.gemspec +++ b/code_ownership.gemspec @@ -1,6 +1,6 @@ Gem::Specification.new do |spec| spec.name = 'code_ownership' - spec.version = '1.36.3' + spec.version = '1.37.0' spec.authors = ['Gusto Engineers'] spec.email = ['dev@gusto.com'] spec.summary = 'A gem to help engineering teams declare ownership of code' diff --git a/lib/code_ownership/private/ownership_mappers/directory_ownership.rb b/lib/code_ownership/private/ownership_mappers/directory_ownership.rb index 2175046..b4a00b6 100644 --- a/lib/code_ownership/private/ownership_mappers/directory_ownership.rb +++ b/lib/code_ownership/private/ownership_mappers/directory_ownership.rb @@ -48,7 +48,8 @@ def globs_to_owner(files) .map(&:cleanpath) .each_with_object({}) do |pathname, res| owner = owner_for_codeowners_file(pathname) - res[pathname.dirname.cleanpath.join('**/**').to_s] = owner + glob = glob_for_codeowners_file(pathname) + res[glob] = owner end end @@ -77,7 +78,7 @@ def owner_for_codeowners_file(codeowners_file) # Takes a file and finds the relevant `.codeowner` file by walking up the directory # structure. Example, given `a/b/c.rb`, this looks for `a/b/.codeowner`, `a/.codeowner`, # and `.codeowner` in that order, stopping at the first file to actually exist. - # If the parovided file is a directory, it will look for `.codeowner` in that directory and then upwards. + # If the provided file is a directory, it will look for `.codeowner` in that directory and then upwards. # We do additional caching so that we don't have to check for file existence every time. sig { params(file: String).returns(T.nilable(CodeTeams::Team)) } def map_file_to_relevant_owner(file) @@ -123,6 +124,26 @@ def get_team_from_codeowners_file_within_directory(directory) team end + + sig { params(codeowners_file: Pathname).returns(String) } + def glob_for_codeowners_file(codeowners_file) + unescaped = codeowners_file.dirname.cleanpath.join('**/**').to_s + + # Globs can contain certain regex characters, like "[" and "]". + # However, when we are generating a glob from a .codeowner file, we + # need to escape bracket characters and interpret them literally. + # Otherwise the resulting glob will not actually match the directory + # containing the .codeowner file. + # + # Example + # file: "/some/[dir]/.codeowner" + # unescaped: "/some/[dir]/**/**" + # matches: "/some/d/file" + # matches: "/some/i/file" + # matches: "/some/r/file" + # does not match!: "/some/[dir]/file" + unescaped.gsub(/[\[\]]/) { |x| "\\#{x}" } + end end end end diff --git a/spec/lib/code_ownership_spec.rb b/spec/lib/code_ownership_spec.rb index 1edbaba..14227a3 100644 --- a/spec/lib/code_ownership_spec.rb +++ b/spec/lib/code_ownership_spec.rb @@ -25,6 +25,19 @@ end end + context 'directory with [] characters containing a .codeowner file' do + before do + write_file('app/services/[test]/.codeowner', <<~CONTENTS) + Bar + CONTENTS + write_file('app/services/[test]/some_file.rb', '') + end + + it 'has no validation errors' do + expect { CodeOwnership.validate!(files: ['app/services/[test]/some_file.rb']) }.to_not raise_error + end + end + context 'file ownership with [] characters' do before do write_file('app/services/[test]/some_file.ts', <<~TYPESCRIPT) @@ -161,6 +174,19 @@ end end + context '.codeowner in a directory with [] characters' do + before do + write_file('app/javascript/[test]/.codeowner', <<~CONTENTS) + Bar + CONTENTS + write_file('app/javascript/[test]/test.js', '') + end + + it 'properly assigns ownership' do + expect(CodeOwnership.for_file('app/javascript/[test]/test.js')).to eq CodeTeams.find('Bar') + end + end + before { create_non_empty_application } end