Skip to content

Commit e1e01e5

Browse files
escape brackets for files owned with a file annotation (#108)
* escape brackets for files owned with an annotation * bump version
1 parent 7195411 commit e1e01e5

File tree

4 files changed

+49
-3
lines changed

4 files changed

+49
-3
lines changed

code_ownership.gemspec

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
Gem::Specification.new do |spec|
22
spec.name = 'code_ownership'
3-
spec.version = '1.37.0'
3+
spec.version = '1.38.0'
44
spec.authors = ['Gusto Engineers']
55
spec.email = ['[email protected]']
66
spec.summary = 'A gem to help engineering teams declare ownership of code'

lib/code_ownership/private.rb

+1
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ def self.file_tracked?(file)
9999
in_unowned_globs = configuration.unowned_globs.any? do |unowned_glob|
100100
File.fnmatch?(unowned_glob, file, File::FNM_PATHNAME | File::FNM_EXTGLOB)
101101
end
102+
102103
in_owned_globs && !in_unowned_globs && File.exist?(file)
103104
end
104105

lib/code_ownership/private/ownership_mappers/file_annotations.rb

+30-2
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,8 @@ def globs_to_owner(files)
3939
owner = file_annotation_based_owner(filename_relative_to_root)
4040
next unless owner
4141

42-
mapping[filename_relative_to_root] = owner
42+
escaped_filename = escaped_path_for_codeowners_file(filename_relative_to_root)
43+
mapping[escaped_filename] = owner
4344
end
4445
end
4546

@@ -55,7 +56,8 @@ def update_cache(cache, files)
5556

5657
invalid_files = cache.keys.select do |file|
5758
# If a file is not tracked, it should be removed from the cache
58-
!Private.file_tracked?(file) ||
59+
unescaped_file = unescaped_path_for_codeowners_file(file)
60+
!Private.file_tracked?(unescaped_file) ||
5961
# If a file no longer has a file annotation (i.e. `globs_to_owner` doesn't map it)
6062
# it should be removed from the cache
6163
# We make sure to only apply this to the input files since otherwise `updated_cache_for_files.key?(file)` would always return `false` when files == []
@@ -126,6 +128,32 @@ def description
126128

127129
sig { override.void }
128130
def bust_caches!; end
131+
132+
sig { params(filename: String).returns(String) }
133+
def escaped_path_for_codeowners_file(filename)
134+
# Globs can contain certain regex characters, like "[" and "]".
135+
# However, when we are generating a glob from a file annotation, we
136+
# need to escape bracket characters and interpret them literally.
137+
# Otherwise the resulting glob will not actually match the directory
138+
# containing the file.
139+
#
140+
# Example
141+
# filename: "/some/[xId]/myfile.tsx"
142+
# matches: "/some/1/file"
143+
# matches: "/some/2/file"
144+
# matches: "/some/3/file"
145+
# does not match!: "/some/[xId]/myfile.tsx"
146+
filename.gsub(/[\[\]]/) { |x| "\\#{x}" }
147+
end
148+
149+
sig { params(filename: String).returns(String) }
150+
def unescaped_path_for_codeowners_file(filename)
151+
# Globs can contain certain regex characters, like "[" and "]".
152+
# We escape bracket characters and interpret them literally for
153+
# the CODEOWNERS file. However, we want to compare the unescaped
154+
# glob to the actual file path when we check if the file was deleted.
155+
filename.gsub(/\\([\[\]])/, '\1')
156+
end
129157
end
130158
end
131159
end

spec/lib/code_ownership/private/ownership_mappers/file_annotations_spec.rb

+17
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,23 @@ module CodeOwnership
7070
expect(CodeOwnership.for_file('frontend/javascripts/packages/my_package/owned_file.jsx').name).to eq 'Bar'
7171
end
7272
end
73+
74+
context 'javascript owned file with brackets' do
75+
before do
76+
write_configuration
77+
write_file('config/teams/bar.yml', <<~CONTENTS)
78+
name: Bar
79+
CONTENTS
80+
81+
write_file('frontend/javascripts/packages/my_package/[formID]/owned_file.jsx', <<~CONTENTS)
82+
// @team Bar
83+
CONTENTS
84+
end
85+
86+
it 'can find the owner of a javascript file with file annotations' do
87+
expect(CodeOwnership.for_file('frontend/javascripts/packages/my_package/[formID]/owned_file.jsx').name).to eq 'Bar'
88+
end
89+
end
7390
end
7491

7592
describe '.remove_file_annotation!' do

0 commit comments

Comments
 (0)