Add a tool to look for source files that have unneeded grit header includes.

tools/unused-grit-header.py

+#!/usr/bin/env python
+# Copyright 2014 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.
+
+"""A tool to scan source files for unneeded grit includes."""

[Nico, 2014/08/24 23:58:25]

Include a usage line

[Lei Zhang, 2014/08/26 02:35:24]

Usage() is literally 6 lines below...

+
+import os
+import sys
+import xml.etree.ElementTree
+

[Nico, 2014/08/24 23:58:26]

2 empty lines between toplevels

[Lei Zhang, 2014/08/26 02:35:25]

Done.

+def Usage(prog_name):
+  print prog_name, 'GRD_FILE DIRS_TO_SCAN'

[Nico, 2014/08/24 23:58:25]

Consider using optparse or argparse (see e.g. tools/sort-headers.py)

[Lei Zhang, 2014/08/26 02:35:24]

There's no options. If we do add options later, I'll switch over.

[Nico, 2014/08/26 02:43:37]

It also offers usage printing (you can hand it the module docstring as help,
etc)

+
+
+def GetResourcesForNode(node, parent_file, resource_tag):
+  """Recursively iterate through a node and extract resource names.
+
+  Args:
+    node: The node to iterate through.
+    parent_file: The file that contains node.
+    resource_tag: The resource tag to extract names from.
+
+  Returns:
+    A list of resource names.
+  """
+  resources = []
+  for child in node.getchildren():
+    if child.tag == resource_tag:
+      resources.append(child.attrib['name'])
+    elif child.tag == 'if':
+      resources.extend(GetResourcesForNode(child, parent_file, resource_tag))
+    elif child.tag == 'part':
+      parent_dir = os.path.dirname(parent_file)
+      part_file = os.path.join(parent_dir, child.attrib['file'])
+      part_tree = xml.etree.ElementTree.parse(part_file)
+      part_root = part_tree.getroot()
+      assert part_root.tag == 'grit-part'
+      resources.extend(GetResourcesForNode(part_root, part_file, resource_tag))
+    else:
+      raise Exception('unknown tag:', child.tag)
+  return resources
+
+
+def GetResourcesForGrdFile(tree, grd_file):
+  """Find all the message and include resources from a given grit file.
+
+  Args:
+    tree: The XML tree.
+    grd_file: The file that contains the XML tree.
+
+  Returns:
+    A list of resource names.
+  """
+  root = tree.getroot()
+  assert root.tag == 'grit'
+  for node in root.getchildren():
+    if node.tag == 'release':
+      release_node = node
+      break
+  assert release_node != None
+
+  for node in release_node.getchildren():
+    if node.tag == 'messages':
+      messages_node = node
+      break
+  messages = set()
+  if messages_node != None:
+    messages = set(GetResourcesForNode(messages_node, grd_file, 'message'))
+
+  for node in release_node.getchildren():
+    if node.tag == 'includes':
+      includes_node = node
+      break

[Nico, 2014/08/24 23:58:25]

Since you have this for loop 3 times, consider having a
FindNodeWithTag(release_node, 'includes') function

[Lei Zhang, 2014/08/26 02:35:24]

Done.

+  includes = set()
+  if includes_node != None:
+    includes = set(GetResourcesForNode(includes_node, grd_file, 'include'))
+  return messages.union(includes)
+
+
+def GetOutputFileForNode(node):
+  """Find the output file starting from a given node.
+
+  Args:
+    node: The root node to scan from.
+
+  Returns:
+    A grit header file name.
+  """
+  output_file = None
+  for child in node.getchildren():
+    if child.tag == 'output':
+      if child.attrib['type'] == 'rc_header':
+        assert output_file == None
+        output_file = child.attrib['filename']
+    elif child.tag == 'if':

[Nico, 2014/08/24 23:58:26]

do you need to look at 'else' children of if nodes? I think someone added else
nodes to grit somewhat recently. (Is there a way to ask grit for this stuff
instead of doing the parsing yourself?) 

[Lei Zhang, 2014/08/26 02:35:25]

I don't think grit has <else> nodes? It may be possible to use grit rather than
parsing myself, but I'm not sure if that's more painful or not.

[Nico, 2014/08/26 02:43:37]

see https://codereview.chromium.org/11155024

+      child_output_file = GetOutputFileForNode(child)
+      if not child_output_file:
+        continue
+      assert output_file == None

[Nico, 2014/08/24 23:58:25]

nit: it's more common to see "output_file is None" for comparisons with None
(also in line 95)

[Lei Zhang, 2014/08/26 02:35:25]

Done.

+      output_file = child_output_file
+    else:
+      raise Exception('unknown tag:', child.tag)
+  return output_file
+
+
+def GetOutputHeaderFile(tree):
+  """Find the output file for a given tree.
+
+  Args:
+    tree: The tree to scan.
+
+  Returns:
+    A grit header file name.
+  """
+  root = tree.getroot()
+  assert root.tag == 'grit'
+  for node in root.getchildren():
+    if node.tag == 'outputs':
+      output_node = node
+      break

[Nico, 2014/08/24 23:58:26]

Maybe `assert not output_node` before the assignment instead of breaking, to
make sure there's only one outputs?

[Lei Zhang, 2014/08/26 02:35:24]

Done. And elsewhere above.

+  assert output_node != None
+  return GetOutputFileForNode(output_node)
+
+
+def ShouldScanFile(filename):
+  """Return if the filename has one of the extensions below."""
+  extensions = ['.cc', '.cpp', '.h', '.mm']
+  file_extension = os.path.splitext(filename)[1]
+  return file_extension in extensions
+

[Nico, 2014/08/24 23:58:26]

add a newline

[Lei Zhang, 2014/08/26 02:35:24]

Done.

+def NeedsGritInclude(grit_header, resources, filename):
+  """Return whether a file needs a given grit header or not.
+
+  Args:
+    grit_header: The grit header file name.
+    resources: The list of resource names in grit_header.
+    filename: The file to scan.
+
+  Returns:
+    True if the file should include the grit header.
+  """
+  with open(filename, 'rb') as f:
+    grit_header_line = grit_header + '"\n'
+    has_grit_header = False
+    while True:
+      line = f.readline()
+      if not line:
+        break
+      if line.endswith(grit_header_line):
+        has_grit_header = True
+        break
+
+    if not has_grit_header:
+      return True
+    rest_of_the_file = f.read()
+    return any(resource in rest_of_the_file for resource in resources)
+
+
+def main(argv):
+  if len(argv) < 3:
+    Usage(argv[0])
+    sys.exit(1)
+  grd_file = argv[1]
+  dirs_to_scan = argv[2:]
+  for f in dirs_to_scan:
+    if not os.path.exists(f):
+      print 'Error: %s does not exist' % f
+      sys.exit(1)
+
+  tree = xml.etree.ElementTree.parse(grd_file)
+  grit_header = GetOutputHeaderFile(tree)
+  resources = GetResourcesForGrdFile(tree, grd_file)
+
+  files_with_unneeded_grit_includes = []
+  for dir_to_scan in dirs_to_scan:
+    for root, dirs, files in os.walk(dir_to_scan):
+      if '.git' in dirs:
+        dirs.remove('.git')
+      full_paths = [os.path.join(root, f) for f in files if ShouldScanFile(f)]
+      files_with_unneeded_grit_includes.extend(
+          [f for f in full_paths
+           if not NeedsGritInclude(grit_header, resources, f)])
+  if files_with_unneeded_grit_includes:
+    print '\n'.join(files_with_unneeded_grit_includes)
+    sys.exit(2)
+  sys.exit(0)
+

[Nico, 2014/08/24 23:58:25]

add newline

[Lei Zhang, 2014/08/26 02:35:24]

Done.

+if __name__ == '__main__':
+  main(sys.argv)

[Nico, 2014/08/24 23:58:25]

nit:  the usual thing is `sys.exit(main(sys.argv))`, then main() can return
error codes

[Lei Zhang, 2014/08/26 02:35:24]

Done.