Skip to content

Commit 9f6d311

Browse files
martinemdehelloswetaashleywillarddkisselevivy
authored
Restore support for CodeOwnership.remove_file_annotations! (#146)
This method is called by rubyatscale/packs gem when moving packs. Co-authored-by: Sweta Sanghavi <[email protected]> Co-authored-by: Ashley Willard <[email protected]> Co-authored-by: Denis Kisselev <[email protected]> Co-authored-by: Ivy Evans <[email protected]>
1 parent bb759f6 commit 9f6d311

File tree

2 files changed

+221
-0
lines changed

2 files changed

+221
-0
lines changed

lib/code_ownership.rb

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

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