depot_tools: modify cpplint.py to allow CPPLINT.cfg overrides

cpplint.py

Index: cpplint.py
diff --git a/cpplint.py b/cpplint.py
index 92384dc37181f6e982d0e6f74afe93d393c5de19..640ae08bfbac87a781a3a0e3e1a57600b601acc1 100755
--- a/cpplint.py
+++ b/cpplint.py
@@ -682,6 +682,8 @@ class _CppLintState(object):
     self.error_count = 0    # global count of reported errors
     # filters to apply when emitting error messages
     self.filters = _DEFAULT_FILTERS[:]
+      # backup of filter list. Used to restore the state after each file.

[sosa, 2014/07/23 17:35:00]

indentation also this seems super private to the class so _filters_backup would
be more appropriate.

[Alex Vakulenko (Google), 2014/07/23 19:04:50]

Done.

+    self.filters_backup = self.filters[:]
     self.counting = 'total'  # In what way are we counting errors?
     self.errors_by_category = {}  # string to int dict storing error counts
 
@@ -720,6 +722,10 @@ class _CppLintState(object):
     """
     # Default filters always have less priority than the flag ones.
     self.filters = _DEFAULT_FILTERS[:]
+    self.AddFilters(filters)
+
+  def AddFilters(self, filters):
+    """ Adds more filters to the existing list of error-message filters. """
     for filt in filters.split(','):
       clean_filt = filt.strip()
       if clean_filt:
@@ -729,6 +735,14 @@ class _CppLintState(object):
         raise ValueError('Every filter in --filters must start with + or -'
                          ' (%s does not)' % filt)
 
+  def BackupFilters(self):
+    """ Saves the current filter list to backup storage."""
+    self.filters_backup = self.filters[:]
+
+  def RestoreFilters(self):
+    """ Restores filters previously backed up."""
+    self.filters = self.filters_backup[:]
+
   def ResetErrorCounts(self):
     """Sets the module's error statistic back to zero."""
     self.error_count = 0
@@ -796,6 +810,22 @@ def _SetFilters(filters):
   """
   _cpplint_state.SetFilters(filters)
 
+def _AddFilters(filters):
+  """Adds more filter overrides. Unlike _SetFilters, this function does not

[sosa, 2014/07/23 17:35:00]

Unlike should be on a newline. We usually keep the first line of a docstring in
one line. Usual format:

Short description in one line.
<BREAK>
Longer description in many lines.

[Alex Vakulenko (Google), 2014/07/23 19:04:50]

Done.

+  reset the current list of filters available.
+  Args:
+    filters: A string of comma-separated filters (eg "whitespace/indent").
+             Each filter should start with + or -; else we die.
+  """
+  _cpplint_state.AddFilters(filters)
+
+def _BackupFilters():
+  """ Saves the current filter list to backup storage."""
+  _cpplint_state.BackupFilters()
+
+def _RestoreFilters():
+  """ Restores filters previously backed up."""
+  _cpplint_state.RestoreFilters()
 
 class _FunctionState(object):
   """Tracks current function name and the number of lines in its body."""
@@ -5424,6 +5454,84 @@ def ProcessFileData(filename, file_extension, lines, error,
 
   CheckForNewlineAtEOF(filename, lines, error)
 
+def GetConfigOverrideFiles(filename):
+  """ Looks for CPPLINT.cfg files in the directory containing |filename| and
+  its parents and returns a list of configuration files found.
+
+  Args:
+    filename: The name of the file being processed by the linter.
+  """
+
+  cfg_override_files = []
+  abs_path = os.path.abspath(os.path.dirname(filename))
+
+  while True:
+    file = os.path.join(abs_path, "CPPLINT.cfg")
+    if os.path.isfile(file):
+      cfg_override_files.append(file)
+    parent = os.path.dirname(abs_path)
+    if parent == abs_path:
+      break
+    abs_path = parent
+
+  return reversed(cfg_override_files)
+
+def ProcessConfigFile(cfg_file, filename):
+  """ Loads the configuration file and processes the config overrides

[sosa, 2014/07/23 17:35:00]

nit about docstring

[Alex Vakulenko (Google), 2014/07/23 19:04:51]

Done.

+  specified in it.
+
+  Config file contains a bunch of key=value pairs. Currently

[sosa, 2014/07/23 17:35:00]

This should probably go in the module docstring / help. Also maybe a commented
out example file would be nice.

[Alex Vakulenko (Google), 2014/07/23 19:04:50]

Done.

+  we are looking only for:
+    filter=+filter1,-filter2,...
+    exclude_files=regex
+
+  Args:
+    cfg_file: config file to process
+    filename: The name of the file being processed by the linter.
+
+  Returns:
+    false if the current |filename| should not be processed further.
+  """
+
+  try:
+    #lines = codecs.open(cfg_file, 'r', 'utf8', 'replace').read().split('\n')

[sosa, 2014/07/23 17:35:00]

What's the line commented out for?

[Alex Vakulenko (Google), 2014/07/23 19:04:50]

Just forgot to remove. Done.

+    with open(cfg_file) as lines:

[sosa, 2014/07/23 17:35:00]

Not completely correct naming. This is the file_handle that you can iterate with
as for line in file_handle

[Alex Vakulenko (Google), 2014/07/23 19:04:50]

Done.

+      for line in lines:
+        name, val = line.partition('=')[::2]

[sosa, 2014/07/23 17:35:00]

mmm, this works but not really the correct usage of a step. I usually see this
done as:

name, _, val = line.partition('=')

[Alex Vakulenko (Google), 2014/07/23 19:04:50]

I copied this from somewhere.
Fixed

+        name = name.strip()

[sosa, 2014/07/23 17:35:00]

val = val.strip() here 

[Alex Vakulenko (Google), 2014/07/23 19:04:50]

Done.

+        if name == 'filter':
+          _AddFilters(val.strip())
+        elif name == 'exclude_files':

[sosa, 2014/07/23 17:35:00]

Should support comments but also error out for names that don't match so we
avoid people making bad files that are silently ignored.

[Alex Vakulenko (Google), 2014/07/23 19:04:50]

Done.

+          pattern = re.compile(val.strip())
+          if pattern.match(filename):
+            sys.stderr.write('Ignoring %s; file excluded by (%s)\n' %
+                             (filename, cfg_file))
+            return False
+
+  except IOError:
+    sys.stderr.write(
+        "Skipping config file '%s': Can't open for reading\n" % cfg_file)
+    _RestoreFilters()
+    return True
+
+  return True
+

[sosa, 2014/07/23 17:35:00]

2 lines should separate top-level methods

[Alex Vakulenko (Google), 2014/07/23 19:04:50]

Done.

+def ProcessConfigOverrides(cfg_override_files, filename):
+  """ Loads the configuration files and processes the config overrides

[sosa, 2014/07/23 17:35:00]

same nit about docstrings.

[Alex Vakulenko (Google), 2014/07/23 19:04:50]

Done.

+  specified in those files..
+
+  Args:
+    cfg_override_files: list of cfg files to process
+    filename: The name of the file being processed by the linter.
+
+  Returns:
+    false if the current |filename| should not be processed further.
+  """
+  for cfg_file in cfg_override_files:
+    if not ProcessConfigFile(cfg_file, filename):
+      return False
+
+  return True
 
 def ProcessFile(filename, vlevel, extra_check_functions=[]):
   """Does google-lint on a single file.
@@ -5440,6 +5548,12 @@ def ProcessFile(filename, vlevel, extra_check_functions=[]):
   """
 
   _SetVerboseLevel(vlevel)
+  _BackupFilters()
+
+  cfg_override_files = GetConfigOverrideFiles(filename)
+  if not ProcessConfigOverrides(cfg_override_files, filename):
+    _RestoreFilters()
+    return
 
   lf_lines = []
   crlf_lines = []
@@ -5471,6 +5585,7 @@ def ProcessFile(filename, vlevel, extra_check_functions=[]):
   except IOError:
     sys.stderr.write(
         "Skipping input '%s': Can't open for reading\n" % filename)
+    _RestoreFilters()
     return
 
   # Note, if no dot is found, this will give the entire filename as the ext.
@@ -5504,6 +5619,7 @@ def ProcessFile(filename, vlevel, extra_check_functions=[]):
               'Unexpected \\r (^M) found; better to use only \\n')
 
   sys.stderr.write('Done processing %s\n' % filename)
+  _RestoreFilters()
 
 
 def PrintUsage(message):