Fix the owners implementation to validate method inputs.

owners.py

 # Copyright (c) 2010 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 """A database of OWNERS files."""
 
+import collections
 import re
 
 
 # If this is present by itself on a line, this means that everyone can review.
 EVERYONE = '*'
 
 
 # Recognizes 'X@Y' email addresses. Very simplistic.
 BASIC_EMAIL_REGEXP = r'^[\w\-\+\%\.]+\@[\w\-\+\%\.]+$'
 
@@ skipping 34 matching lines @@
     self.owned_by = {EVERYONE: set()}
 
     # Mapping of paths to authorized owners.
     self.owners_for = {}
 
     # Set of paths that stop us from looking above them for owners.
     # (This is implicitly true for the root directory).
     self.stop_looking = set([''])
 
   def reviewers_for(self, files):
-    """Returns a suggested set of reviewers that will cover the set of files.
+    """Returns a suggested set of reviewers that will cover the files.
 
-    files is a set of paths relative to (and under) self.root."""
+    files is a sequence of paths relative to (and under) self.root."""
     self._check_paths(files)
     self._load_data_needed_for(files)
     return self._covering_set_of_owners_for(files)
 
   def files_are_covered_by(self, files, reviewers):
     """Returns whether every file is owned by at least one reviewer."""
     return not self.files_not_covered_by(files, reviewers)
 
   def files_not_covered_by(self, files, reviewers):
-    """Returns the set of files that are not owned by at least one reviewer."""
+    """Returns the set of files that are not owned by at least one reviewer.
+
+    Args:
+        files is a sequence of paths relative to (and under) self.root.
+        reviewers is a sequence of strings matching self.email_regexp."""
     self._check_paths(files)
     self._check_reviewers(reviewers)
     if not reviewers:
       return files
 
     self._load_data_needed_for(files)
     files_by_dir = self._files_by_dir(files)
     covered_dirs = self._dirs_covered_by(reviewers)
     uncovered_files = []
     for d, files_in_d in files_by_dir.iteritems():
       if not self._is_dir_covered_by(d, covered_dirs):
         uncovered_files.extend(files_in_d)
     return set(uncovered_files)
 
+  def _check_collection(self, obj):

[M-A Ruel, 2011/03/17 01:27:47]

I prefer this function to not be a member function. It has no relation to the
object.

+    assert (isinstance(obj, collections.Iterable) and

[M-A Ruel, 2011/03/17 01:27:47]

assert (isinstance(obj, (collections.Iterable, collections.Sized) and
        not isinstance(obj, basestring))

+            isinstance(obj, collections.Sized) and
+            not isinstance(obj, basestring))
+
   def _check_paths(self, files):
     def _is_under(f, pfx):
       return self.os_path.abspath(self.os_path.join(pfx, f)).startswith(pfx)
+    self._check_collection(files)
     assert all(_is_under(f, self.os_path.abspath(self.root)) for f in files)
 
   def _check_reviewers(self, reviewers):
-    """Verifies each reviewer is a valid email address."""
+    self._check_collection(reviewers)
     assert all(self.email_regexp.match(r) for r in reviewers)
 
   def _files_by_dir(self, files):
     dirs = {}
     for f in files:
       dirs.setdefault(self.os_path.dirname(f), []).append(f)
     return dirs
 
   def _dirs_covered_by(self, reviewers):
     dirs = self.owned_by[EVERYONE]
@@ skipping 49 matching lines @@
     # short combinations of owners.
     every_owner = set()
     for f in files:
       dirname = self.os_path.dirname(f)
       while dirname in self.owners_for:
         every_owner |= self.owners_for[dirname]
         if self._stop_looking(dirname):
           break
         dirname = self.os_path.dirname(dirname)
     return every_owner