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

cpplint.py

 #!/usr/bin/env python
 #
 # Copyright (c) 2009 Google Inc. All rights reserved.
 #
 # Redistribution and use in source and binary forms, with or without
 # modification, are permitted provided that the following conditions are
 # met:
 #
 #    * Redistributions of source code must retain the above copyright
 # notice, this list of conditions and the following disclaimer.
@@ skipping 664 matching lines @@
 
 
 class _CppLintState(object):
   """Maintains module-wide state.."""
 
   def __init__(self):
     self.verbose_level = 1  # global setting.
     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
 
     # output format:
     # "emacs" - format that emacs can parse (default)
     # "vs7" - format that Microsoft Visual Studio 7 can parse
     self.output_format = 'emacs'
 
   def SetOutputFormat(self, output_format):
     """Sets the output format for errors."""
@@ skipping 18 matching lines @@
     Args:
       filters: A string of comma-separated filters (eg "+whitespace/indent").
                Each filter should start with + or -; else we die.
 
     Raises:
       ValueError: The comma-separated filters did not all start with '+' or '-'.
                   E.g. "-,+whitespace,-whitespace/indent,whitespace/badfilter"
     """
     # 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:
         self.filters.append(clean_filt)
     for filt in self.filters:
       if not (filt.startswith('+') or filt.startswith('-')):
         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
     self.errors_by_category = {}
 
   def IncrementErrorCount(self, category):
     """Bumps the module's error statistic."""
     self.error_count += 1
     if self.counting in ('toplevel', 'detailed'):
       if self.counting != 'detailed':
@@ skipping 47 matching lines @@
 
   These filters are applied when deciding whether to emit a given
   error message.
 
   Args:
     filters: A string of comma-separated filters (eg "whitespace/indent").
              Each filter should start with + or -; else we die.
   """
   _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."""
 
   _NORMAL_TRIGGER = 250  # for --v=0, 500 for --v=1, etc.
   _TEST_TRIGGER = 400    # about 50% more than _NORMAL_TRIGGER.
 
   def __init__(self):
     self.in_a_function = False
     self.lines_in_function = 0
@@ skipping 4608 matching lines @@
   nesting_state.CheckCompletedBlocks(filename, error)
 
   CheckForIncludeWhatYouUse(filename, clean_lines, include_state, error)
 
   # We check here rather than inside ProcessLine so that we see raw
   # lines rather than "cleaned" lines.
   CheckForBadCharacters(filename, 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.
 
   Args:
     filename: The name of the file to parse.
 
     vlevel: The level of errors to report.  Every error of confidence
     >= verbose_level will be reported.  0 is a good default.
 
     extra_check_functions: An array of additional check functions that will be
                            run on each source line. Each function takes 4
                            arguments: filename, clean_lines, line, error
   """
 
   _SetVerboseLevel(vlevel)
+  _BackupFilters()
+
+  cfg_override_files = GetConfigOverrideFiles(filename)
+  if not ProcessConfigOverrides(cfg_override_files, filename):
+    _RestoreFilters()
+    return
 
   lf_lines = []
   crlf_lines = []
   try:
     # Support the UNIX convention of using "-" for stdin.  Note that
     # we are not opening the file with universal newline support
     # (which codecs doesn't support anyway), so the resulting lines do
     # contain trailing '\r' characters if we are reading a file that
     # has CRLF endings.
     # If after the split a trailing '\r' is present, it is removed
@@ skipping 11 matching lines @@
     for linenum in range(len(lines) - 1):
       if lines[linenum].endswith('\r'):
         lines[linenum] = lines[linenum].rstrip('\r')
         crlf_lines.append(linenum + 1)
       else:
         lf_lines.append(linenum + 1)
 
   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.
   file_extension = filename[filename.rfind('.') + 1:]
 
   # When reading from stdin, the extension is unknown, so no cpplint tests
   # should rely on the extension.
   if filename != '-' and file_extension not in _valid_extensions:
     sys.stderr.write('Ignoring %s; not a valid file name '
                      '(%s)\n' % (filename, ', '.join(_valid_extensions)))
@@ skipping 13 matching lines @@
     # server-side end-of-line sequence.
     if lf_lines and crlf_lines:
       # Warn on every line with CR.  An alternative approach might be to
       # check whether the file is mostly CRLF or just LF, and warn on the
       # minority, we bias toward LF here since most tools prefer LF.
       for linenum in crlf_lines:
         Error(filename, linenum, 'whitespace/newline', 1,
               'Unexpected \\r (^M) found; better to use only \\n')
 
   sys.stderr.write('Done processing %s\n' % filename)
+  _RestoreFilters()
 
 
 def PrintUsage(message):
   """Prints a brief usage string and exits, optionally with an error message.
 
   Args:
     message: The optional error message.
   """
   sys.stderr.write(_USAGE)
   if message:
@@ skipping 94 matching lines @@
   _cpplint_state.ResetErrorCounts()
   for filename in filenames:
     ProcessFile(filename, _cpplint_state.verbose_level)
   _cpplint_state.PrintErrorCounts()
 
   sys.exit(_cpplint_state.error_count > 0)
 
 
 if __name__ == '__main__':
   main()