[Android] Add lint as a gyp action.

build/android/gyp/lint.py

Index: build/android/gyp/lint.py
diff --git a/build/android/gyp/lint.py b/build/android/gyp/lint.py
new file mode 100755
index 0000000000000000000000000000000000000000..9be634607202a35d47fb1f19b1f7f9b30cb25e58
--- /dev/null
+++ b/build/android/gyp/lint.py
@@ -0,0 +1,102 @@
+#!/usr/bin/env python
+#
+# Copyright (c) 2013 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.
+
+"""Runs Android's lint tool."""
+
+
+import optparse
+import os
+import sys
+import time
+from xml.dom import minidom
+
+from util import build_utils
+
+
+def _ParseAndShowResultFile(result_path):
+  dom = minidom.parse(result_path)
+  for issue in dom.getElementsByTagName('issue'):
+    issue_id = issue.attributes['id'].value
+    severity = issue.attributes['severity'].value
+    message = issue.attributes['message'].value
+    location_elem = issue.getElementsByTagName('location')[0]
+    path = location_elem.attributes['file'].value
+    line = location_elem.getAttribute('line')
+    if line:
+      error = '%s:%s %s: %s [%s]' % (path, line, severity, message,
+                                     issue_id)
+    else:
+      # Issues in class files don't have a line number.
+      error = '%s %s: %s [%s]' % (path, severity, message, issue_id)
+    print >> sys.stderr, error
+
+
+def _RunLint(lint_path, config_path, result_path, src_root, src_dirs,
+             classes_dir):
+
+  def _RelativizePath(path):
+    """Returns relative path to top-level src dir.
+
+    Args:
+      path: A path relative to cwd.
+    """
+    return os.path.relpath(os.path.abspath(path), os.path.abspath(src_root))
+
+  cmd = [
+      lint_path, '-Werror', '--exitcode', '--showall',
+      '--config', _RelativizePath(config_path),
+      '--classpath', _RelativizePath(classes_dir),
+      '--xml', _RelativizePath(result_path),
+  ]
+
+  for src in src_dirs:
+    cmd.extend(['--sources', _RelativizePath(src)])
+
+  cmd.append(_RelativizePath(os.path.join(config_path, os.pardir)))
+
+  rc, _ = build_utils.CheckCall(cmd, suppress_output=True,
+                                cwd=os.path.abspath(src_root))
+  if rc:
+    _ParseAndShowResultFile(result_path)
+    print >> sys.stderr, ('\nWanna suppress lint issues? Refer to %s' %
+                          _RelativizePath(config_path))
+    sys.exit(rc)

[newt (away), 2013/12/03 00:54:16]

I'd probably return "rc" instead of calling sys.exit() here. BTW, is lint's
return code non-zero whenever it detects a new lint error, or only when lint
fails to run (e.g. because of invalid command line arguments)?

[frankf, 2013/12/04 23:29:31]

Done. You only return rc of zero if there are no lint issues (or lint is
disabled) and no exceptions are raised.

+
+
+def main(argv):
+  start = time.time()
+
+  parser = optparse.OptionParser()
+  parser.add_option('--lint-path', help='Path to lint executable.')
+  parser.add_option('--config-path', help='Path to lint suppressions file.')
+  parser.add_option('--result-path', help='Path to xml file')

[newt (away), 2013/12/03 00:54:16]

"Path to xml file"?

[frankf, 2013/12/04 23:29:31]

Rephrased

+  parser.add_option('--src-root', help='Path to top-level src dir.')
+  parser.add_option('--src-dirs', help='Directories containing java files.')
+  parser.add_option('--classes-dir', help='Directory containing class files.')
+  parser.add_option('--stamp', help='Path to touch on success.')
+  parser.add_option('--enable', action='store_true',

[newt (away), 2013/12/03 00:54:16]

I would use gyp to conditionally enable or disable running lint, not an
"--enable" command line flag

[frankf, 2013/12/04 23:29:31]

We want to trivially touch the stamp if lint is not enabled. We could do this
directly in gyp, but this is a pattern used elsewhere in build/android/gyp as
well. I guess the motivation is that it keeps logic in gyp simpler.

+                    help='Run lint instead of just touching stamp.')
+
+  options, _ = parser.parse_args()
+
+  assert (options.lint_path and options.config_path and options.result_path and

[newt (away), 2013/12/03 00:54:16]

you can use build_utils.CheckOptions() to make this a little more user friendly:

    build_utils.CheckOptions(options, parser, required=['lint_path', ...])

[frankf, 2013/12/04 23:29:31]

Done. Although having required be an optional parameter kind of beats the
purpose.

On 2013/12/03 00:54:16, newt wrote:

[newt (away), 2013/12/05 23:39:33]

The irony ;)

+          options.src_root and options.src_dirs and options.classes_dir)
+
+  src_dirs = build_utils.ParseGypList(options.src_dirs)
+
+  if options.enable:
+    _RunLint(options.lint_path, options.config_path, options.result_path,
+             options.src_root, src_dirs, options.classes_dir)
+
+  if options.stamp:

[newt (away), 2013/12/03 00:54:16]

should the stamp only be touched if lint detects no new errors?

[frankf, 2013/12/04 23:29:31]

Nope, previously we exited in line 66, with new patch we explicitly check the
rc.

+    build_utils.Touch(options.stamp)
+
+  # TODO(frankf): Remove this before committing.
+  print 'Lint took %s for %s' % (int(time.time() - start), options.classes_dir)
+
+
+if __name__ == '__main__':
+  sys.exit(main(sys.argv))

[newt (away), 2013/12/03 00:54:16]

main() doesn't return anything, so this line isn't useful. Perhaps main *should*
return a value?