PRESUBMIT.py: Check #include file order.

PRESUBMIT.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.
 
 """Top-level presubmit script for Chromium.
 
 See http://dev.chromium.org/developers/how-tos/depottools/presubmit-scripts
 for more details about the presubmit API built into gcl.
 """
 
@@ skipping 19 matching lines @@
 
 
 _TEST_ONLY_WARNING = (
     'You might be calling functions intended only for testing from\n'
     'production code.  It is OK to ignore this warning if you know what\n'
     'you are doing, as the heuristics used to detect the situation are\n'
     'not perfect.  The commit queue will not block on this warning.\n'
     'Email [email protected] if you have questions.')
 
 
+_INCLUDE_ORDER_WARNING = (
+    'Your #include order seems to be broken. Send mail to\n'
+    '[email protected] if this is not the case.')
+
+
 _BANNED_OBJC_FUNCTIONS = (
     (
       'addTrackingRect:',
       (
        'The use of -[NSView addTrackingRect:owner:userData:assumeInside:] is'
        'prohibited. Please use CrTrackingArea instead.',
        'http://dev.chromium.org/developers/coding-style/cocoa-dos-and-donts',
       ),
       False,
     ),
@@ skipping 443 matching lines @@
       if pattern.match(line):
         errors.append('    %s:%d' % (f.LocalPath(), line_num))
 
   results = []
   if errors:
     results.append(output_api.PresubmitError(
       'Header files should not include ui/aura/window_property.h', errors))
   return results
 
 
+def _CheckIncludeOrderForScope(scope, input_api, file_path, changed_linenums):
+  """Checks that the lines in scope occur in the right order.
+
+  1. C system files in alphabetical order
+  2. C++ system files in alphabetical order
+  3. Project's .h files
+  """
+
+  c_system_include_pattern = input_api.re.compile(r'\s*#include <.*\.h>')
+  cpp_system_include_pattern = input_api.re.compile(r'\s*#include <.*>')
+  custom_include_pattern = input_api.re.compile(r'\s*#include ".*')
+
+  cpp_system_include_seen = False

[M-A Ruel, 2012/11/13 01:55:59]

You should use a one-variable state machine, it'd be simpler IMHO but I don't
mind.

[marja, 2012/11/13 17:30:20]

Done. This was ugly.

+  custom_include_seen = False
+
+  previous_line = ''
+  problem_linenums = []
+  for line_num, line in scope:
+    if c_system_include_pattern.match(line):
+      if cpp_system_include_seen or custom_include_seen:
+        problem_linenums.append(line_num)
+      elif previous_line and previous_line > line:
+        problem_linenums.append(line_num)
+    elif cpp_system_include_pattern.match(line):
+      if custom_include_seen:
+        problem_linenums.append(line_num)
+      if not cpp_system_include_seen:
+        cpp_system_include_seen = True
+      elif previous_line and previous_line > line:
+        problem_linenums.append(line_num)
+    elif custom_include_pattern.match(line):
+      if not custom_include_seen:
+        custom_include_seen = True
+      elif previous_line and previous_line > line:
+        problem_linenums.append(line_num)
+    else:
+      problem_linenums.append(line_num)
+    previous_line = line
+
+  warnings = []
+  for line_num in problem_linenums:
+    if line_num in changed_linenums or line_num - 1 in changed_linenums:
+      warnings.append('    %s:%d' % (file_path, line_num))
+  return warnings
+
+
+def _CheckIncludeOrderInFile(input_api, output_api, f, is_source,
+                             changed_linenums):
+  """Checks the #include order for the given file f."""
+
+  include_pattern = input_api.re.compile(r'\s*#include.*')
+  if_pattern = input_api.re.compile(r'\s*#if.*')
+  endif_pattern = input_api.re.compile(r'\s*#endif.*')
+
+  contents = input_api.ReadFile(f).splitlines()

[M-A Ruel, 2012/11/13 01:55:59]

That's slow. There are other functions that cache the file content, you should
use these.

[marja, 2012/11/13 17:30:20]

Done (f.NewContents()).

+  warnings = []
+  line_num = 0
+
+  # Handle the special first include for source files.
+  if is_source:
+    for line in contents:
+      line_num += 1
+      if include_pattern.match(line):
+        wanted_line = ('#include "%s"' % f.LocalPath().replace('.cc', '.h'))
+        if line != wanted_line:
+          warnings.append('    %s:%d' % (f.LocalPath(), line_num))
+        break
+
+  # Split into scopes: Each region between #if and #endif is its own scope.
+  scopes = []
+  current_scope = []
+  for line in contents[line_num:]:
+    line_num += 1
+    if if_pattern.match(line) or endif_pattern.match(line):
+      scopes.append(current_scope)
+      current_scope = []
+    elif include_pattern.match(line):
+      current_scope.append((line_num, line))
+
+  for scope in scopes:
+    warnings.extend(_CheckIncludeOrderForScope(scope, input_api, f.LocalPath(),
+                                               changed_linenums))
+  return warnings
+

[M-A Ruel, 2012/11/08 18:46:20]

2 lines

[marja, 2012/11/13 17:30:20]

Done.

+def _CheckIncludeOrder(input_api, output_api):
+  """Checks that the #include order is correct.
+
+  1. The corresponding header for source files.
+  2. C system files in alphabetical order
+  3. C++ system files in alphabetical order
+  4. Project's .h files in alphabetical order
+
+  Each region between #if and #endif follows these rules separately.
+  """
+
+  warnings = []
+  for f in input_api.AffectedFiles():
+    changed_linenums = set([line_num for line_num, _ in f.ChangedContents()])
+    if f.LocalPath().endswith('.cc'):
+      warnings = _CheckIncludeOrderInFile(input_api, output_api, f, True,
+                                          changed_linenums)
+    elif f.LocalPath().endswith('.h'):
+      warnings = _CheckIncludeOrderInFile(input_api, output_api, f, False,
+                                          changed_linenums)
+
+  results = []
+  if warnings:
+    results.append(output_api.PresubmitPromptWarning(_INCLUDE_ORDER_WARNING,
+                                                     warnings))
+  return results
+
+
 def _CommonChecks(input_api, output_api):
   """Checks common to both upload and commit."""
   results = []
   results.extend(input_api.canned_checks.PanProjectChecks(
       input_api, output_api, excluded_paths=_EXCLUDED_PATHS))
   results.extend(_CheckAuthorizedAuthor(input_api, output_api))
   results.extend(
     _CheckNoProductionCodeUsingTestOnlyFunctions(input_api, output_api))
   results.extend(_CheckNoIOStreamInHeaders(input_api, output_api))
   results.extend(_CheckNoUNIT_TESTInSourceFiles(input_api, output_api))
   results.extend(_CheckNoNewWStrings(input_api, output_api))
   results.extend(_CheckNoDEPSGIT(input_api, output_api))
   results.extend(_CheckNoBannedFunctions(input_api, output_api))
   results.extend(_CheckNoPragmaOnce(input_api, output_api))
   results.extend(_CheckNoTrinaryTrueFalse(input_api, output_api))
   results.extend(_CheckUnwantedDependencies(input_api, output_api))
   results.extend(_CheckFilePermissions(input_api, output_api))
   results.extend(_CheckNoAuraWindowPropertyHInHeaders(input_api, output_api))
+  results.extend(_CheckIncludeOrder(input_api, output_api))
   return results
 
 
 def _CheckSubversionConfig(input_api, output_api):
   """Verifies the subversion config file is correctly setup.
 
   Checks that autoprops are enabled, returns an error otherwise.
   """
   join = input_api.os_path.join
   if input_api.platform == 'win32':
@@ skipping 127 matching lines @@
 
   # Match things like path/aura/file.cc and path/file_aura.cc.
   # Same for ash and chromeos.
   if any(re.search('[/_](ash|aura)', f) for f in files):
     trybots += ['linux_chromeos_clang:compile', 'win_aura',
                 'linux_chromeos_asan']
   elif any(re.search('[/_]chromeos', f) for f in files):
     trybots += ['linux_chromeos_clang:compile', 'linux_chromeos_asan']
 
   return trybots