Skip to content

Commit 2504cf3

Browse files
martinemdehelloswetaashleywillarddkisselevivy
committed
Restore support for CodeOwnership.remove_file_annotations!
This method is called by rubyatscale/packs gem when moving packs. Co-authored-by: Sweta Sanghavi <sweta.sanghavi@gusto.com> Co-authored-by: Ashley Willard <ashley.willard@gusto.com> Co-authored-by: Denis Kisselev <denis.kisselev@gusto.com> Co-authored-by: Ivy Evans <ivy.evans@gusto.com>
1 parent 94f4400 commit 2504cf3

2 files changed

Lines changed: 221 additions & 0 deletions

File tree

lib/code_ownership.rb

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,57 @@ def validate!(
239239
end
240240
end
241241

242+
# Removes the file annotation (e.g., "# @team TeamName") from a file.
243+
#
244+
# This method removes the ownership annotation from the first line of a file,
245+
# which is typically used to declare team ownership at the file level.
246+
# The annotation can be in the form of:
247+
# - Ruby comments: # @team TeamName
248+
# - JavaScript/TypeScript comments: // @team TeamName
249+
# - YAML comments: -# @team TeamName
250+
#
251+
# If the file does not have an annotation or the annotation doesn't match a valid team,
252+
# this method does nothing.
253+
#
254+
# @param filename [String] The path to the file from which to remove the annotation.
255+
# Can be relative or absolute.
256+
#
257+
# @return [void]
258+
#
259+
# @example Remove annotation from a Ruby file
260+
# # Before: File contains "# @team Platform\nclass User; end"
261+
# CodeOwnership.remove_file_annotation!('app/models/user.rb')
262+
# # After: File contains "class User; end"
263+
#
264+
# @example Remove annotation from a JavaScript file
265+
# # Before: File contains "// @team Frontend\nexport default function() {}"
266+
# CodeOwnership.remove_file_annotation!('app/javascript/component.js')
267+
# # After: File contains "export default function() {}"
268+
#
269+
# @note This method modifies the file in place.
270+
# @note Leading newlines after the annotation are also removed to maintain clean formatting.
271+
#
272+
sig { params(filename: String).void }
273+
def remove_file_annotation!(filename)
274+
filepath = Pathname.new(filename)
275+
276+
begin
277+
content = filepath.read
278+
rescue Errno::EISDIR, Errno::ENOENT
279+
# Ignore files that fail to read (directories, missing files, etc.)
280+
return
281+
end
282+
283+
# Remove the team annotation and any trailing newlines after it
284+
team_pattern = %r{\A(?:#|//|-#) @team .*\n+}
285+
new_content = content.sub(team_pattern, '')
286+
287+
filepath.write(new_content) if new_content != content
288+
rescue ArgumentError => e
289+
# Handle invalid byte sequences gracefully
290+
raise unless e.message.include?('invalid byte sequence')
291+
end
292+
242293
# Given a backtrace from either `Exception#backtrace` or `caller`, find the
243294
# first line that corresponds to a file with assigned ownership
244295
sig { params(backtrace: T.nilable(T::Array[String]), excluded_teams: T::Array[::CodeTeams::Team]).returns(T.nilable(::CodeTeams::Team)) }

spec/lib/code_ownership_spec.rb

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -385,4 +385,174 @@
385385
expect(described_class.version).to eq ["code_ownership version: #{CodeOwnership::VERSION}", "codeowners-rs version: #{RustCodeOwners.version}"]
386386
end
387387
end
388+
389+
describe '.remove_file_annotation!' do
390+
subject(:remove_file_annotation) do
391+
CodeOwnership.remove_file_annotation!(filename)
392+
# Getting the owner gets stored in the cache, so after we remove the file annotation we want to bust the cache
393+
CodeOwnership.bust_caches!
394+
end
395+
396+
before do
397+
write_file('config/teams/foo.yml', <<~CONTENTS)
398+
name: Foo
399+
github:
400+
team: '@MyOrg/foo-team'
401+
CONTENTS
402+
write_configuration
403+
end
404+
405+
context 'ruby file has no annotation' do
406+
let(:filename) { 'app/my_file.rb' }
407+
408+
before do
409+
write_file(filename, <<~CONTENTS)
410+
# Empty file
411+
CONTENTS
412+
end
413+
414+
it 'has no effect' do
415+
expect(File.read(filename)).to eq "# Empty file\n"
416+
417+
remove_file_annotation
418+
419+
expect(File.read(filename)).to eq "# Empty file\n"
420+
end
421+
end
422+
423+
context 'ruby file has annotation' do
424+
let(:filename) { 'app/my_file.rb' }
425+
426+
before do
427+
write_file(filename, <<~CONTENTS)
428+
# @team Foo
429+
430+
# Some content
431+
CONTENTS
432+
433+
RustCodeOwners.generate_and_validate(nil, false)
434+
end
435+
436+
it 'removes the annotation' do
437+
current_ownership = CodeOwnership.for_file(filename, from_codeowners: false)
438+
expect(current_ownership&.name).to eq 'Foo'
439+
expect(File.read(filename)).to eq <<~RUBY
440+
# @team Foo
441+
442+
# Some content
443+
RUBY
444+
445+
remove_file_annotation
446+
447+
new_ownership = CodeOwnership.for_file(filename, from_codeowners: false)
448+
expect(new_ownership).to eq nil
449+
expected_output = <<~RUBY
450+
# Some content
451+
RUBY
452+
453+
expect(File.read(filename)).to eq expected_output
454+
end
455+
end
456+
457+
context 'javascript file has annotation' do
458+
let(:filename) { 'app/my_file.jsx' }
459+
460+
before do
461+
write_file(filename, <<~CONTENTS)
462+
// @team Foo
463+
464+
// Some content
465+
CONTENTS
466+
467+
RustCodeOwners.generate_and_validate(nil, false)
468+
end
469+
470+
it 'removes the annotation' do
471+
current_ownership = CodeOwnership.for_file(filename, from_codeowners: false)
472+
expect(current_ownership&.name).to eq 'Foo'
473+
expect(File.read(filename)).to eq <<~JAVASCRIPT
474+
// @team Foo
475+
476+
// Some content
477+
JAVASCRIPT
478+
479+
remove_file_annotation
480+
481+
new_ownership = CodeOwnership.for_file(filename, from_codeowners: false)
482+
expect(new_ownership).to eq nil
483+
expected_output = <<~JAVASCRIPT
484+
// Some content
485+
JAVASCRIPT
486+
487+
expect(File.read(filename)).to eq expected_output
488+
end
489+
end
490+
491+
context "haml has annotation (only verifies file is changed, the curren implementation doesn't verify haml files)" do
492+
let(:filename) { 'app/views/my_file.html.haml' }
493+
494+
before do
495+
write_file(filename, <<~CONTENTS)
496+
-# @team Foo
497+
498+
-# Some content
499+
CONTENTS
500+
end
501+
502+
it 'removes the annotation' do
503+
expect(File.read(filename)).to eq <<~HAML
504+
-# @team Foo
505+
506+
-# Some content
507+
HAML
508+
509+
remove_file_annotation
510+
511+
expected_output = <<~HAML
512+
-# Some content
513+
HAML
514+
515+
expect(File.read(filename)).to eq expected_output
516+
end
517+
end
518+
519+
context 'file has new lines after the annotation' do
520+
let(:filename) { 'app/my_file.rb' }
521+
522+
before do
523+
write_file(filename, <<~CONTENTS)
524+
# @team Foo
525+
526+
527+
# Some content
528+
529+
530+
# Some other content
531+
CONTENTS
532+
end
533+
534+
it 'removes the annotation and the leading new lines' do
535+
expect(File.read(filename)).to eq <<~RUBY
536+
# @team Foo
537+
538+
539+
# Some content
540+
541+
542+
# Some other content
543+
RUBY
544+
545+
remove_file_annotation
546+
547+
expected_output = <<~RUBY
548+
# Some content
549+
550+
551+
# Some other content
552+
RUBY
553+
554+
expect(File.read(filename)).to eq expected_output
555+
end
556+
end
557+
end
388558
end

0 commit comments

Comments
 (0)