Make presubmit check that #if or #ifdef does not come before includes

PRESUBMIT.py

Index: PRESUBMIT.py
diff --git a/PRESUBMIT.py b/PRESUBMIT.py
index a4d90c8a8e88d1fec2dad3af6fe2e7472e3a396b..089670fe80154699ad99f26aa3aa32b7f8616583 100644
--- a/PRESUBMIT.py
+++ b/PRESUBMIT.py
@@ -77,6 +77,46 @@ def _PythonChecks(input_api, output_api):
       white_list=affected_python_files)
 
 
+def _IfDefChecks(input_api, output_api):
+  """Ensures if/ifdef are not before includes. See skbug/3362 for details."""
+  copyright_start_pattern = re.compile('\/\*\w*\n')
+  copyright_middle_pattern = re.compile(' \*.*\n')
+  copyright_end_pattern = re.compile(' \*\/.*\n')

[borenet, 2015/03/12 12:47:48]

Suggest using '^\/\*w*$' etc

[rmistry, 2015/03/12 13:13:59]

Yes, I forgot to do this. Done.

+  def is_in_copyright_block(line):
+    return (copyright_start_pattern.match(line) or
+            copyright_middle_pattern.match(line) or
+            copyright_end_pattern.match(line))
+
+  empty_line_pattern = re.compile('\w*\n\w*')

[borenet, 2015/03/12 12:47:48]

Shouldn't this be '^$' ?  Why match characters in a line that's supposed to be
empty?

[rmistry, 2015/03/12 13:13:59]

This is to treat lines that have only whitespaces in it as empty lines. If there
is a '  ' line and empty_line_pattern does not catch it then it will be treated
as the first real line and will be tested for '#if' and '#ifdef'.

+  def is_empty_line(line):
+    return empty_line_pattern.match(line)
+
+  failing_files = []
+  for affected_file in input_api.AffectedSourceFiles(None):
+    affected_file_path = affected_file.LocalPath()
+    if affected_file_path.endswith('.cpp') or affected_file_path.endswith('.h'):
+      f = open(affected_file_path)
+      for line in f.xreadlines():
+        if is_in_copyright_block(line) or is_empty_line(line):
+          continue
+        # The below will be the first real line after the copyright block and
+        # newlines.
+        if line.startswith('#if 0 '):
+          pass

[borenet, 2015/03/12 12:47:48]

Why not break here?

[rmistry, 2015/03/12 13:13:59]

Either way works. I prefer having less places to exit whenever possible.

+        elif line.startswith('#if ') or line.startswith('#ifdef '):
+          failing_files.append(affected_file_path)
+        break
+
+  results = []
+  if failing_files:
+    results.append(
+        output_api.PresubmitError(
+            'The following files have #if or #ifdef before includes:\n%s\n\n'
+            'See skbug.com/3362 for why this should be fixed.' %
+                '\n'.join(failing_files)))
+  return results
+
+
 def _CommonChecks(input_api, output_api):
   """Presubmit checks common to upload and commit."""
   results = []
@@ -90,6 +130,7 @@ def _CommonChecks(input_api, output_api):
       _CheckChangeHasEol(
           input_api, output_api, source_file_filter=sources))
   results.extend(_PythonChecks(input_api, output_api))
+  results.extend(_IfDefChecks(input_api, output_api))
   return results