Only attribute comments to owners that are close to the comment

A comment that is preceded with an empty line (or starts at the
beginning of the file) will be attributed to owners listed directly
below the comment. Otherwise, if the comment is in the middle of
a list of owners, it will only be attributed to the next owner.

BUG=712589
R=dpranke@chromium.org

Change-Id: I19bd7809836b6ee65ef56e2ec399e5cd09eaa132
Reviewed-on: https://chromium-review.googlesource.com/481303
Commit-Queue: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>

owners.py

@@ -244,6 +244,9 @@ class Database(object):
     comment = []
     dirpath = self.os_path.dirname(path)
     in_comment = False
+    # We treat the beginning of the file as an blank line.
+    previous_line_was_blank = True
+    reset_comment_after_use = False
     lineno = 0
 
     if path in self.override_files:
@@ -262,13 +265,18 @@ class Database(object):
             continue
         if not in_comment:
           comment = []
+          reset_comment_after_use = not previous_line_was_blank
         comment.append(line[1:].strip())
         in_comment = True
         continue
+      in_comment = False
+
       if line == '':
+        comment = []
+        previous_line_was_blank = True
         continue
-      in_comment = False
 
+      previous_line_was_blank = False
       if line == 'set noparent':
         self._stop_looking.add(dirpath)
         continue
@@ -285,6 +293,8 @@ class Database(object):
         relative_glob_string = self.os_path.relpath(full_glob_string, self.root)
         self._add_entry(relative_glob_string, directive, owners_path,
                         lineno, '\n'.join(comment))
+        if reset_comment_after_use:
+          comment = []
         continue
 
       if line.startswith('set '):
@@ -293,6 +303,8 @@ class Database(object):
 
       self._add_entry(dirpath, line, owners_path, lineno,
                       ' '.join(comment))
+      if reset_comment_after_use:
+        comment = []
 
   def _read_global_comments(self):
     if not self._status_file:

tests/owners_finder_test.py

@@ -34,7 +34,6 @@ def owners_file(*email_addresses, **kwargs):
     s += '# %s\n' % kwargs.get('comment')
   if kwargs.get('noparent'):
     s += 'set noparent\n'
-  s += '\n'.join(kwargs.get('lines', [])) + '\n'
   return s + '\n'.join(email_addresses) + '\n'
 
 

tests/owners_unittest.py

@@ -33,7 +33,8 @@ def owners_file(*email_addresses, **kwargs):
     s += 'set noparent\n'
   if kwargs.get('file'):
     s += 'file:%s\n' % kwargs.get('file')
-  s += '\n'.join(kwargs.get('lines', [])) + '\n'
+  if kwargs.get('lines'):
+    s += '\n'.join(kwargs.get('lines', [])) + '\n'
   return s + '\n'.join(email_addresses) + '\n'
 
 
@@ -343,6 +344,34 @@ class OwnersDatabaseTest(_BaseTestCase):
     self.files['/foo/DEPS'] = ''
     self.assertRaises(IOError, db.reviewers_for, ['foo/DEPS'], None)
 
+  def test_comment_to_owners_mapping(self):
+    db = self.db()
+    self.files['/OWNERS'] = '\n'.join([
+        '# first comment',
+        ben,
+        brett,
+        '',
+        darin,
+        '',
+        '# comment preceeded by empty line',
+        'per-file bar.*=%s' % jochen,
+        john,
+        '',
+        ken,
+        '# comment in the middle',
+        peter,
+        tom])
+    # Force loading of the OWNERS file.
+    self.files['/bar.cc'] = ''
+    db.reviewers_for(['bar.cc'], None)
+
+    self.assertEqual(db.comments, {
+        ben: {'': 'first comment'},
+        brett: {'': 'first comment'},
+        jochen: {'bar.*': 'comment preceeded by empty line'},
+        john: {'': 'comment preceeded by empty line'},
+        peter: {'': 'comment in the middle'}})
+
 
 class ReviewersForTest(_BaseTestCase):
   def assert_reviewers_for(self, files, potential_suggested_reviewers,