Add Java support to checkdeps.py

tools/checkdeps/checkdeps.py

 #!/usr/bin/env python
 # Copyright (c) 2012 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.
 
 """Makes sure that files include headers from allowed directories.
 
 Checks DEPS files in the source tree for rules, and applies those rules to
 "#include" commands in source files. Any source file including something not
 permitted by the DEPS files will fail.
@@ skipping 41 matching lines @@
 only lowercase.
 """
 
 import os
 import optparse
 import pipes
 import re
 import sys
 import copy
 
+import checker

[M-A Ruel, 2012/07/17 14:23:20]

No need to import 'checker'. Furthermore doing that would cause aliasing at line
288.

My take:

def checker_factory(filename):
  """Returns an implementation to 'check' the source files."""
  ext = os.path.splitext(filename)[1]
  if ext in java_checker.Checker.EXTENSIONS:
    return java_checker.Checker()
  elif ext in cpp_checker.Checker.EXTENSIONS:
    return cpp_checker.Checker()

Your factory is now 7 lines. :)

[Iain Merrick, 2012/07/17 15:55:08]

That's sorta what I had originally, then I was convinced to make it more OO-y.
:)

The only twist is that we want to construct the java checker once, as it
pre-scans the files during __init__.

+import cpp_checker
+import java_checker
+
+
 # Variable name used in the DEPS file to add or subtract include files from
 # the module-level deps.
 INCLUDE_RULES_VAR_NAME = "include_rules"
 
 # Optionally present in the DEPS file to list subdirectories which should not
 # be checked. This allows us to skip third party code, for example.
 SKIP_SUBDIRS_VAR_NAME = "skip_child_includes"
 
-# The maximum number of non-include lines we can see before giving up.
-MAX_UNINTERESTING_LINES = 50
-
-# The maximum line length, this is to be efficient in the case of very long
-# lines (which can't be #includes).
-MAX_LINE_LENGTH = 128
-
 # Set to true for more output. This is set by the command line options.
 VERBOSE = False
 
-# This regular expression will be used to extract filenames from include
-# statements.
-EXTRACT_INCLUDE_PATH = re.compile('[ \t]*#[ \t]*(?:include|import)[ \t]+"(.*)"')
-
 # In lowercase, using forward slashes as directory separators, ending in a
 # forward slash. Set by the command line options.
 BASE_DIRECTORY = ""
 
 # The directories which contain the sources managed by git.
 GIT_SOURCE_DIRECTORY = set()
 
 
 # Specifies a single rule for an include, which can be either allow or disallow.
 class Rule(object):
@@ skipping 169 matching lines @@
     print "  No deps file found in", dir_name
 
   # Even if a DEPS file does not exist we still invoke ApplyRules
   # to apply the implicit "allow" rule for the current directory
   include_rules = local_scope.get(INCLUDE_RULES_VAR_NAME, [])
   skip_subdirs = local_scope.get(SKIP_SUBDIRS_VAR_NAME, [])
 
   return (ApplyRules(existing_rules, include_rules, dir_name), skip_subdirs)
 
 
-def ShouldCheckFile(file_name):
-  """Returns True if the given file is a type we want to check."""
-  checked_extensions = [
-      '.h',
-      '.cc',
-      '.m',
-      '.mm',
-  ]
-  basename, extension = os.path.splitext(file_name)
-  return extension in checked_extensions
-
-
-def CheckLine(rules, line):
-  """Checks the given file with the given rule set.
-  Returns a tuple (is_include, illegal_description).
-  If the line is an #include directive the first value will be True.
-  If it is also an illegal include, the second value will be a string describing
-  the error.  Otherwise, it will be None."""
-  found_item = EXTRACT_INCLUDE_PATH.match(line)
-  if not found_item:
-    return False, None  # Not a match
-
-  include_path = found_item.group(1)
-
-  # Fix up backslashes in case somebody accidentally used them.
-  include_path.replace("\\", "/")
-
-  if include_path.find("/") < 0:
-    # Don't fail when no directory is specified. We may want to be more
-    # strict about this in the future.
-    if VERBOSE:
-      print " WARNING: directory specified with no path: " + include_path
-    return True, None
-
-  (allowed, why_failed) = rules.DirAllowed(include_path)
-  if not allowed:
-    if VERBOSE:
-      retval = "\nFor " + rules.__str__()
-    else:
-      retval = ""
-    return True, retval + ('Illegal include: "%s"\n    Because of %s' %
-        (include_path, why_failed))
-
-  return True, None
-
-
-def CheckFile(rules, file_name):
-  """Checks the given file with the given rule set.
-
-  Args:
-    rules: The set of rules that apply to files in this directory.
-    file_name: The source file to check.
-
-  Returns: Either a string describing the error if there was one, or None if
-           the file checked out OK.
-  """
-  if VERBOSE:
-    print "Checking: " + file_name
-
-  ret_val = ""  # We'll collect the error messages in here
-  last_include = 0
-  try:
-    cur_file = open(file_name, "r")
-    in_if0 = 0
-    for line_num in xrange(sys.maxint):
-      if line_num - last_include > MAX_UNINTERESTING_LINES:
-        break
-
-      cur_line = cur_file.readline(MAX_LINE_LENGTH)
-      if cur_line == "":
-        break
-      cur_line = cur_line.strip()
-
-      # Check to see if we're at / inside a #if 0 block
-      if cur_line == '#if 0':
-        in_if0 += 1
-        continue
-      if in_if0 > 0:
-        if cur_line.startswith('#if'):
-          in_if0 += 1
-        elif cur_line == '#endif':
-          in_if0 -= 1
-        continue
-
-      is_include, line_status = CheckLine(rules, cur_line)
-      if is_include:
-        last_include = line_num
-      if line_status is not None:
-        if len(line_status) > 0:  # Add newline to separate messages.
-          line_status += "\n"
-        ret_val += line_status
-    cur_file.close()
-
-  except IOError:
-    if VERBOSE:
-      print "Unable to open file: " + file_name
-    cur_file.close()
-
-  # Map empty string to None for easier checking.
-  if len(ret_val) == 0:
-    return None
-  return ret_val
-
-
-def CheckDirectory(parent_rules, dir_name):
+def CheckDirectory(parent_rules, checker_factory, dir_name):
   (rules, skip_subdirs) = ApplyDirectoryRules(parent_rules, dir_name)
   if rules == None:
     return True
 
   # Collect a list of all files and directories to check.
   files_to_check = []
   dirs_to_check = []
   success = True
   contents = os.listdir(dir_name)
   for cur in contents:
     if cur in skip_subdirs:
       continue  # Don't check children that DEPS has asked us to skip.
     full_name = os.path.join(dir_name, cur)
     if os.path.isdir(full_name):
       dirs_to_check.append(full_name)
-    elif ShouldCheckFile(full_name):
+    elif checker_factory.GetChecker(full_name):
       files_to_check.append(full_name)
 
   # First check all files in this directory.
   for cur in files_to_check:
-    file_status = CheckFile(rules, cur)
+    checker = checker_factory.GetChecker(cur)
+    file_status = checker.CheckFile(rules, cur)
     if file_status != None:

[M-A Ruel, 2012/07/17 14:23:20]

if file_status:

Then you can remove the weird checks for len(ret_val) != 0.

[Iain Merrick, 2012/07/17 15:55:08]

Done.

       print "ERROR in " + cur + "\n" + file_status
       success = False
 
   # Next recurse into the subdirectories.
   for cur in dirs_to_check:
-    if not CheckDirectory(rules, cur):
+    if not CheckDirectory(rules, checker_factory, cur):
       success = False
 
   return success
 
 
 def GetGitSourceDirectory(root):
   """Returns a set of the directories to be checked.
 
   Args:
     root: The repository root where .git directory exists.
@@ skipping 65 matching lines @@
   # systems. Plus, we always use slashes here since the include parsing code
   # will also normalize to slashes.
   BASE_DIRECTORY = BASE_DIRECTORY.lower()
   BASE_DIRECTORY = BASE_DIRECTORY.replace("\\", "/")
   start_dir = start_dir.replace("\\", "/")
 
   if os.path.exists(os.path.join(BASE_DIRECTORY, ".git")):
     global GIT_SOURCE_DIRECTORY
     GIT_SOURCE_DIRECTORY = GetGitSourceDirectory(BASE_DIRECTORY)
 
-  success = CheckDirectory(base_rules, start_dir)
+  checker_factory = checker.Factory(BASE_DIRECTORY, VERBOSE)
+  success = CheckDirectory(base_rules, checker_factory, start_dir)
   if not success:
     print "\nFAILED\n"
     return 1
   print "\nSUCCESS\n"
   return 0
 
 
 def main():
   option_parser = optparse.OptionParser()
   option_parser.add_option("", "--root", default="", dest="base_directory",
                            help='Specifies the repository root. This defaults '
                            'to "../../.." relative to the script file, which '
                            'will normally be the repository root.')
   option_parser.add_option("-v", "--verbose", action="store_true",
                            default=False, help="Print debug logging")
   options, args = option_parser.parse_args()
   return checkdeps(options, args)
 
 
 if '__main__' == __name__:
   sys.exit(main())