Fix last_files_match_only to evaluate per file instead of globally (#135)

Previously, when last_files_match_only was enabled, every time a pattern
matched any changed file, all previously accumulated reviewers were
cleared. This meant the last pattern that matched ANY file in the PR
determined all reviewers, which was wrong when different files had
different last-matching patterns.

Now, for each changed file independently, we find its last matching
pattern's reviewers, then union the results across all files. This
correctly handles the case where e.g. a generated file overrides
reviewers to [] while a non-generated file in the same directory still
matches the wildcard pattern's reviewers.

Author: k0kubun

dist/index.js

@@ -48,16 +48,28 @@ function identify_reviewers_by_changed_files({ config, changed_files, excludes =
     return [];
   }
 
-  const matching_reviewers = [];
-
-  Object.entries(config.files).forEach(([ glob_pattern, reviewers ]) => {
-    if (changed_files.some((changed_file) => minimatch(changed_file, glob_pattern))) {
-      if (last_files_match_only) {
-        matching_reviewers.length = 0; // clear previous matches
+  let matching_reviewers;
+
+  if (last_files_match_only) {
+    const file_patterns = Object.entries(config.files);
+    const per_file_reviewers = changed_files.flatMap((changed_file) => {
+      let last_matching_reviewers = null;
+      file_patterns.forEach(([ glob_pattern, reviewers ]) => {
+        if (minimatch(changed_file, glob_pattern)) {
+          last_matching_reviewers = reviewers;
+        }
+      });
+      return last_matching_reviewers || [];
+    });
+    matching_reviewers = per_file_reviewers;
+  } else {
+    matching_reviewers = [];
+    Object.entries(config.files).forEach(([ glob_pattern, reviewers ]) => {
+      if (changed_files.some((changed_file) => minimatch(changed_file, glob_pattern))) {
+        matching_reviewers.push(...reviewers);
       }
-      matching_reviewers.push(...reviewers);
-    }
-  });
+    });
+  }
 
   const individuals = replace_groups_with_individuals({ reviewers: matching_reviewers, config });
 

src/reviewer.js

@@ -126,6 +126,43 @@ describe('reviewer', function() {
       };
       expect(identify_reviewers_by_changed_files({ config: config_with_last_files_match_only, changed_files })).to.have.members([ 'mario', 'someone-specific' ]);
     });
+
+    context('with `last_files_match_only` and a specific file override with empty reviewers', function() {
+      const override_config = {
+        reviewers: {
+          groups: {
+            'team-a': [ 'alice', 'bob' ],
+          },
+        },
+        files: {
+          'src/**/*': [ 'team-a' ],
+          'src/generated-file.dat': [],
+        },
+        options: {
+          last_files_match_only: true,
+        },
+      };
+
+      it('returns reviewers when a file only matches the wildcard pattern', function() {
+        const changed_files = [ 'src/main-code.js' ];
+        expect(identify_reviewers_by_changed_files({ config: override_config, changed_files })).to.have.members([ 'alice', 'bob' ]);
+      });
+
+      it('returns no reviewers when a file only matches the overriding empty pattern', function() {
+        const changed_files = [ 'src/generated-file.dat' ];
+        expect(identify_reviewers_by_changed_files({ config: override_config, changed_files })).to.deep.equal([]);
+      });
+
+      it('returns no reviewers when all files are unmatched or overridden with empty reviewers', function() {
+        const changed_files = [ 'unrelated-file.txt', 'src/generated-file.dat' ];
+        expect(identify_reviewers_by_changed_files({ config: override_config, changed_files })).to.deep.equal([]);
+      });
+
+      it('returns reviewers when one file matches the wildcard and another is overridden with empty reviewers', function() {
+        const changed_files = [ 'src/main-code.js', 'src/generated-file.dat' ];
+        expect(identify_reviewers_by_changed_files({ config: override_config, changed_files })).to.have.members([ 'alice', 'bob' ]);
+      });
+    });
   });
 
   describe('identify_reviewers_by_author()', function() {