Add Java support to checkdeps.py

tools/checkdeps/java_checker.py

+# 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.
+
+"""Checker implementation that checks Java files.
+
+If you import this module it will register itself with checker.Factory.
+"""
+
+import fileinput
+import os
+import re
+import sys
+
+import checker
+
+
+class JavaChecker(checker.Checker):
+  """Checker implementation for Java files.
+
+  The Checker interface uses real filesystem paths, but Java imports work in
+  terms of package names. To deal with this, we have an extra "prescan" pass
+  that reads all the .java files and builds a mapping of class name -> filename.
+  In CheckFile, we convert each import statement into a real filename, and check
+  that against the rules in the DEPS files.
+
+  Note that in Java you can always use classes in the same directory without an
+  explicit import statement, so these imports can't be blocked with DEPS files.
+  But that shouldn't be a problem, because same-package imports are pretty much
+  always correct by definition. (If we find a case where this is *not* correct,
+  it probably means the package is too big and needs to be split up.)
+
+  Properties:
+    _classmap: dict of fully-qualified Java class name -> filename
+  """
+

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

EXTENSIONS = [
  '.java',
]

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

Done.

+  def __init__(self, *args):
+    super(JavaChecker, self).__init__(*args)
+    self._PrescanFiles()
+
+  def _PrescanFiles(self):
+    self._classmap = {}

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

Define it in the __init__ function. Make sure your code is pylint error free.

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

Done.

+    for root, dirs, files in os.walk(self._base_directory):
+      # Skip unwanted subdirectories. TODO(husky): it would be better to do this
+      # via the skip_child_includes flag in DEPS files. Maybe hoist this prescan
+      # logic into checkdeps.py itself?
+      for d in dirs:
+        # Skip hidden directories.
+        if d.startswith('.'):
+          dirs.remove(d)
+        # Skip the "out" directory, as dealing with generated files is awkward.
+        # We don't want paths like "out/Release/lib.java" in our DEPS files.
+        # TODO(husky): We need some way of determining the "real" path to
+        # a generated file -- i.e., where it would be in source control if
+        # it weren't generated.
+        if d == 'out':
+          dirs.remove(d)
+        # Skip third-party directories.
+        if d == 'third_party':
+          dirs.remove(d)
+      for f in files:
+        if f.endswith('.java'):
+          self._PrescanFile(os.path.join(root, f))
+
+  def _PrescanFile(self, filename):

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

It's really more a filepath than a filename, but I don't mind.

[Iain Merrick, 2012/07/18 13:22:12]

Changed (and I just noticed that my search-and-replace also affected the
docstring, but it seems valid)

+    filename = os.path.relpath(filename, self._base_directory)
+    if self._verbose:
+      print 'Prescanning: ' + filename
+    f = fileinput.input(filename)

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

I don't see why you are using fileinput at all here.

with codecs.open(filename, encoding='utf-8') as f:
or something like that. Beware of default file encoding.

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

Just insufficient time spent searching the library docs -- I'm only an
occasional Pythoner. :)
Done

+    try:
+      short_class_name, _ = os.path.splitext(os.path.basename(filename))

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

short_class_name = os.path.splitext(os.path.basename(filename))[0]

+      for line in f:
+        for package in re.findall('^package ([\w\.]+);', line):
+          full_class_name = package + '.' + short_class_name
+          if full_class_name in self._classmap:
+            print 'WARNING: multiple definitions of %s:' % full_class_name
+            print '    ' + filename
+            print '    ' + self._classmap[full_class_name]
+            print
+          else:
+            self._classmap[full_class_name] = filename
+          return
+      print 'WARNING: no package definition found in %s' % filename
+    finally:
+      f.close()
+
+  def Extensions(self):

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

Remove. This doesn't need to be a member function.

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

Done.

+    return ['.java']
+
+  def CheckFile(self, rules, filename):
+    if self._verbose:
+      print 'Checking: ' + filename
+
+    result = ''
+    f = fileinput.input(filename)

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

with fileinput.input(filename) as f:

Remove the finally section. Note that python 2.6 is required for depot_tools so
using the 'with' statement is fine.

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

Done.

+    try:
+      for line in f:
+        for clazz in re.findall('^import\s+(?:static\s+)?([\w\.]+)\s*;', line):
+          if clazz not in self._classmap:
+            # Importing a class from outside the Chromium tree. That's fine --
+            # it's probably a Java or Android system class.
+            continue
+          include_path = self._classmap[clazz]
+          (allowed, why_failed) = rules.DirAllowed(include_path)
+          if not allowed:
+            if self._verbose:
+              result += '\nFor ' + rules.__str__()
+            result += 'Illegal include: "%s"\n    Because of %s\n' % (
+                include_path, why_failed)
+        if '{' in line:
+          # This is code, so we're finished reading imports for this file.
+          break
+
+    except IOError:

[Iain Merrick, 2012/07/18 13:22:12]

Note: I've removed all IOError handling, as this is an exceptional situation and
we shouldn't try to keep running.

Errors will dump a stack trace which seems okay. If we don't want a stack trace
we should handle the exception at the top level, in checkdeps.py

+      if self._verbose:
+        print 'Unable to open file: ' + filename
+
+    finally:
+      f.close()
+
+    if len(result) == 0:

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

if result:
  return result

None is implicitly returned otherwise. Not sure why you would do that.

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

Done.

+      return None
+    return result
+

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

Remove extra line.

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

Done.