From b624bfe0d0a5f759e7f1316918c37f0522426122 Mon Sep 17 00:00:00 2001
From: Jochen Eisinger <jochen@chromium.org>
Date: Wed, 19 Apr 2017 14:55:34 +0200
Subject: [PATCH] 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                   | 14 +++++++++++++-
 tests/owners_finder_test.py |  1 -
 tests/owners_unittest.py    | 31 ++++++++++++++++++++++++++++++-
 3 files changed, 43 insertions(+), 3 deletions(-)

diff --git a/owners.py b/owners.py
index 89ce219b3..407567b86 100644
--- a/owners.py
+++ b/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:
diff --git a/tests/owners_finder_test.py b/tests/owners_finder_test.py
index 2801aea05..ff7f31398 100755
--- a/tests/owners_finder_test.py
+++ b/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'
 
 
diff --git a/tests/owners_unittest.py b/tests/owners_unittest.py
index 698d8f060..0cb0a1ede 100755
--- a/tests/owners_unittest.py
+++ b/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,