default codeowners (#17)

- added support for section default codeowners notation - bumped version to 1.1.2 closes #15
wickedOne · Feb 18, 2024 · 6aed489 · 6aed489
1 parent 79ebb8b
commit 6aed489
Show file tree

Hide file tree

Showing 4 changed files with 120 additions and 49 deletions.
diff --git a/lib/GPH.pm b/lib/GPH.pm
@@ -3,6 +3,6 @@ package GPH;
 use strict;
 use warnings FATAL => 'all';
 
-our $VERSION = '1.1.1';
+our $VERSION = '1.1.2';
 
 1;
diff --git a/lib/GPH/Gitlab.pm b/lib/GPH/Gitlab.pm
@@ -15,6 +15,8 @@ package GPH::Gitlab;
 use strict;
 use warnings FATAL => 'all';
 
+use Data::Dumper;
+
 #------------------------------------------------------------------------------
 # Construct new class
 #
@@ -49,7 +51,7 @@ sub new {
 # Returns: reference to Gitlab object
 sub parseCodeowners {
     my ($self, %args) = @_;
-    my ($fh, %codeowners, %excludeHash, %blacklist);
+    my ($fh, %excludes, $default_owners);
 
     open $fh, '<', $args{codeowners} or die "unable to open codeowners file: $!";
     my @lines = <$fh>;
@@ -58,49 +60,77 @@ sub parseCodeowners {
     # build excludes hash for quick lookup
     if (exists($args{excludes})) {
         foreach my $item (@{$args{excludes}}) {
-            $excludeHash{$item} = 1;
+            $excludes{$item} = 1;
         }
     }
 
-    for my $line (@lines) {
-        # skip section line. default codeowners not yet supported
-        next if $line =~ /[\[\]]/;
-        # skip if line does not contain @
-        next unless $line =~ /^.*\s\@[\w]+\/.*$/x;
-        # replace /**/* with a trailing forward slash
-        my $pat = quotemeta('/**/* ');
-        $line =~ s|$pat|/ |;
+    foreach (@lines) {
+        next if $_ =~ /^#.*/ or $_ =~ /^[\s]?$/;
+        my $line = $self->sanitise($_);
+
+        if ($line =~ /\]/) {
+            $default_owners = ($line =~ /^[\^]?\[[^\]]+\](?:[\[0-9\]]{0,}) (.*)$/) ? $1 : undef;
+
+            next;
+        }
+
+        my ($class_path, $owners) = split(/\s/, $line, 2);
 
-        my ($class_path, $owners) = split(' ', $line, 2);
+        next if exists $excludes{$class_path};
 
-        # skip if path is excluded
-        next if exists $excludeHash{$class_path};
+        $owners = $owners || $default_owners;
 
-        foreach (split(' ', $owners)) {
-            next unless /(\@[\w\-\/]{0,})$/x;
+        next unless defined $owners;
 
-            if (not exists $codeowners{$1}) {
-                $codeowners{$1} = [];
-                $blacklist{$1} = [];
+        foreach my $owner (split(/\s/, $owners)) {
+            next unless $owner =~ /@/;
+            if (not exists $self->{codeowners}{$owner}) {
+                $self->{codeowners}{$owner} = [];
+                $self->{blacklist}{$owner} = [];
             }
 
-            push(@{$codeowners{$1}}, $class_path);
+            push(@{$self->{codeowners}{$owner}}, $class_path);
 
-            # check whether less specific path is already defined and add it to the blacklist
-            foreach my $key (keys %codeowners) {
-                foreach my $defined (@{$codeowners{$key}}) {
-                    if ($class_path =~ $defined and $class_path ne $defined) {
-                        push(@{$blacklist{$key}}, $class_path);
-                    }
-                }
+            $self->blacklist($class_path);
+        }
+    }
+
+    return ($self);
+}
+
+#------------------------------------------------------------------------------
+# Check whether less specific path is already defined and add it to the blacklist
+#
+# Inputs:  class_path => (string) path to check and blacklist
+#
+# Returns: reference to Gitlab object
+sub blacklist {
+    my ($self, $class_path) = @_;
+
+    foreach my $owner (keys %{$self->{codeowners}}) {
+        foreach my $path (@{$self->{codeowners}{$owner}}) {
+            if ($class_path =~ $path and $class_path ne $path) {
+                push(@{$self->{blacklist}{$owner}}, $class_path);
             }
         }
     }
 
-    $self->{codeowners} = \%codeowners;
-    $self->{blacklist} = \%blacklist;
+    return ($self);
+}
+
+#------------------------------------------------------------------------------
+# Replace /**/* with a trailing forward slash
+#
+# Inputs:  line => (string) line to sanitise
+#
+# Returns: string
+sub sanitise {
+    my ($self, $line) = @_;
+
+    my $pat = quotemeta('/**/* ');
+    $line =~ s|$pat|/ |;
 
-    return $self;
+    return ($line);
 }
 
 #------------------------------------------------------------------------------

diff --git a/t/share/Gitlab/CODEOWNERS b/t/share/Gitlab/CODEOWNERS
@@ -1,9 +1,13 @@
+# Lines that start with `#` are ignored.
 [alpha]
 /src/Command/**/* @teams/alpha
-/src/Service/ @teams/alpha @#!$%^
-.gitlab-ci.yml @teams/alpha
+/src/Service/ @teams/alpha john@doe.com
+.gitlab-ci.yml @teams/alpha henkie
 
-[beta]
-/src/Command/Config/ConfigPhraseKeyCommand.php @teams/beta
-/src/DependencyInjection/ @teams/beta
-.gitlab-ci.yml @teams/beta
+[beta] @teams/beta
+/src/Command/Config/ConfigPhraseKeyCommand.php
+/src/DependencyInjection/
+.gitlab-ci.yml
+
+^[gamma][3] @teams/gamma
+/tests/Unit/Service/
diff --git a/t/unit/GPH/Gitlab.t b/t/unit/GPH/Gitlab.t
@@ -16,26 +16,26 @@ local $SIG{__WARN__} = sub {};
 describe "class `$CLASS`" => sub {
     my %config = (
         codeowners => CODEOWNERS_FILE,
-        owner =>'@teams/alpha',
-        excludes => ['.gitlab-ci.yml']
+        owner      => '@teams/alpha',
+        excludes   => [ '.gitlab-ci.yml' ]
     );
 
     tests 'it can be instantiated' => sub {
         can_ok($CLASS, 'new');
     };
 
     tests "codeowners file not found" => sub {
-        ok(dies{$CLASS->new((codeowners => 'foo.php', owner =>'@teams/alpha'))}, 'died with codeowners not found') or note ($@);
+        ok(dies {$CLASS->new((codeowners => 'foo.php', owner => '@teams/alpha'))}, 'died with codeowners not found') or note($@);
     };
 
     tests "mandatory config options" => sub {
-        ok(dies{$CLASS->new((owner =>'@teams/alpha'))}, 'died with missing codeowners option') or note ($@);
-        ok(dies{$CLASS->new((codeowners => CODEOWNERS_FILE))}, 'died with missing owner option') or note ($@);
-        ok(lives{$CLASS->new((owner =>'@teams/alpha', codeowners => CODEOWNERS_FILE))}, 'lived with mandatory options') or note ($@);
+        ok(dies {$CLASS->new((owner => '@teams/alpha'))}, 'died with missing codeowners option') or note($@);
+        ok(dies {$CLASS->new((codeowners => CODEOWNERS_FILE))}, 'died with missing owner option') or note($@);
+        ok(lives {$CLASS->new((owner => '@teams/alpha', codeowners => CODEOWNERS_FILE))}, 'lived with mandatory options') or note($@);
     };
 
     tests 'owner with blacklist and exclude' => sub {
-        my ($object, $exception, $warnings, @excludes);
+        my ($object, $exception, $warnings);
 
         $exception = dies {
             $warnings = warns {
@@ -46,12 +46,12 @@ describe "class `$CLASS`" => sub {
         is($exception, undef, 'no exception thrown');
         is($warnings, 0, 'no warnings generated');
 
-        is($object->{blacklist}{'@teams/alpha'}, ['/src/Command/Config/ConfigPhraseKeyCommand.php'], 'blacklist correct');
+        is($object->{blacklist}{'@teams/alpha'}, [ '/src/Command/Config/ConfigPhraseKeyCommand.php' ], 'blacklist correct') or diag Dumper($object);
         is('.gitlab-ci.yml', not_in_set(@{$object->{codeowners}{'@teams/alpha'}}), 'excluded file not defined');
     };
 
     tests 'module methods' => sub {
-        my ($object, $exception, $warnings, @excludes);
+        my ($object, $exception, $warnings);
 
         $exception = dies {
             $warnings = warns {
@@ -68,15 +68,15 @@ describe "class `$CLASS`" => sub {
                 item '/src/Service/';
                 end;
             },
-            'GetPaths call correct'
+                'GetPaths call correct'
         );
 
         is($object->getBlacklistPaths(),
             array {
                 item '/src/Command/Config/ConfigPhraseKeyCommand.php';
                 end;
             },
-            'GetBlacklistPaths call correct'
+                'GetBlacklistPaths call correct'
         );
 
         is($object->getCommaSeparatedPathList(), '/src/Command/,/src/Service/', 'GetCommaSeparatedPathList call correct');
@@ -85,12 +85,12 @@ describe "class `$CLASS`" => sub {
 
         is($object->intersectCommaSeparatedPathList(@arr), '/src/Command/', 'IntersectToCommaSeparatedPathList call correct');
 
-        is([$object->intersect(@arr)],
+        is([ $object->intersect(@arr) ],
             array {
                 item '/src/Command/';
                 end;
             },
-            'Intersect call correct'
+                'Intersect call correct'
         );
 
         is($object->match('/src/Service/'), 1, 'Match call match correct');
@@ -101,5 +101,42 @@ describe "class `$CLASS`" => sub {
     };
 };
 
+describe 'test codeowners syntax' => sub {
+    my ($object, %config, $owner, $expected_paths, $exception, $warnings);
+
+    case 'section with default owner' => sub {
+        $owner = '@teams/beta';
+        $expected_paths = [ '/src/Command/Config/ConfigPhraseKeyCommand.php', '/src/DependencyInjection/' ];
+    };
+
+    case 'optional section with default owner and required approvals' => sub {
+        $owner = '@teams/gamma';
+        $expected_paths = [ '/tests/Unit/Service/' ];
+    };
+
+    case 'owner with email' => sub {
+        $owner = 'john@doe.com';
+        $expected_paths = [ '/src/Service/' ];
+    };
+
+    tests 'module get paths' => sub {
+        %config = (
+            codeowners => CODEOWNERS_FILE,
+            owner      => $owner,
+            excludes   => [ '.gitlab-ci.yml' ]
+        );
+
+        $exception = dies {
+            $warnings = warns {
+                $object = $CLASS->new(%config);
+            };
+        };
+
+        is($exception, undef, 'no exception thrown');
+        is($warnings, 0, 'no warnings generated');
+        is($object->getPaths(), $expected_paths, 'paths correct') or diag Dumper($object);
+    };
+};
+
 done_testing();