Add Java support to checkdeps.py

tools/checkdeps/checkdeps.py

Index: tools/checkdeps/checkdeps.py
diff --git a/tools/checkdeps/checkdeps.py b/tools/checkdeps/checkdeps.py
index dae44c336da50728f03e8f3a6c5973cf773fc5b6..6214599fcf0f02e20a14035539b5684dcff354b7 100755
--- a/tools/checkdeps/checkdeps.py
+++ b/tools/checkdeps/checkdeps.py
@@ -59,6 +59,11 @@ 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"
@@ -67,20 +72,9 @@ INCLUDE_RULES_VAR_NAME = "include_rules"
 # 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 = ""
@@ -270,111 +264,7 @@ def ApplyDirectoryRules(existing_rules, dir_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
@@ -390,19 +280,20 @@ def CheckDirectory(parent_rules, dir_name):
     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
@@ -488,7 +379,8 @@ def checkdeps(options, args):
     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