Add PRESUBMIT check for #ifdefs on values that are always defined.

PRESUBMIT.py

Index: PRESUBMIT.py
diff --git a/PRESUBMIT.py b/PRESUBMIT.py
index 46dabb98c4d0cbf2b0e8164520303bc42a62931a..2d4f43fa1050d9c2454d494003ab95e9c3606159 100644
--- a/PRESUBMIT.py
+++ b/PRESUBMIT.py
@@ -1283,6 +1283,7 @@ def _CommonChecks(input_api, output_api):
   results.extend(_CheckHardcodedGoogleHostsInLowerLayers(input_api, output_api))
   results.extend(_CheckNoAbbreviationInPngFileName(input_api, output_api))
   results.extend(_CheckForInvalidOSMacros(input_api, output_api))
+  results.extend(_CheckForInvalidIfDefinedMacros(input_api, output_api))
   results.extend(_CheckAddedDepsHaveTargetApprovals(input_api, output_api))
   results.extend(
       input_api.canned_checks.CheckChangeHasNoTabs(
@@ -1393,6 +1394,43 @@ def _CheckForInvalidOSMacros(input_api, output_api):
       'Possibly invalid OS macro[s] found. Please fix your code\n'
       'or add your macro to src/PRESUBMIT.py.', bad_macros)]
 
+
+def _CheckForInvalidIfDefinedMacrosInFile(input_api, f):
+  """Check all affected files for invalid "if defined" macros."""
+  ALWAYS_DEFINED_MACROS = (
+      "TARGET_IPHONE_SIMULATOR",

[stuartmorgan, 2014/09/25 23:42:32]

From TargetConditionals.h, I'd suggest we also add TARGET_OS_MAC,
TARGET_OS_IPHONE, and TARGET_CPU_*. At least the OS one has bitten me before.

[lliabraa, 2014/09/26 18:22:43]

Done. I added all the CPU and OS defines

+  )
+  preprocessor_statement = input_api.re.compile(r'^\s*#')
+  ifdef_macro = input_api.re.compile(r'(ifdef\s|defined\()([^\s\)]+)')

[stuartmorgan, 2014/09/25 23:42:32]

Why separate regexes, vs ^\s*#.*(ifdef|defined\() ... ? Is it to catch multiple
violations on the same line in one pass instead of two? (I'm not sure it's worth
it since this won't work with sufficiently long ifs anyway, since after one or
two you run out of room and continue with \)

Also, if you aren't using match group one you can replace () with (?:) to make
it a non-capturing group.

[lliabraa, 2014/09/26 18:22:43]

I copied that bit from _CheckForInvalidOSMacrosInFile, but your single regex
works just as well. Thx.
Done.

+  results = []
+  for lnum, line in f.ChangedContents():
+    if preprocessor_statement.search(line):
+      for match in ifdef_macro.finditer(line):
+        if match.group(2) in ALWAYS_DEFINED_MACROS:
+          always_defined = ' %s is always defined. ' % match.group(2)
+          did_you_mean = 'Did you mean \'#if %s\'?' % match.group(2)
+          results.append('    %s:%d %s\n\t%s' % (f.LocalPath(),
+                                             lnum,
+                                             always_defined,
+                                             did_you_mean))
+  return results
+
+
+def _CheckForInvalidIfDefinedMacros(input_api, output_api):
+  """Check all affected files for invalid "if defined" macros."""
+  bad_macros = []
+  for f in input_api.AffectedFiles():
+    if f.LocalPath().endswith(('.h', '.c', '.cc', '.m', '.mm')):
+      bad_macros.extend(_CheckForInvalidIfDefinedMacrosInFile(input_api, f))
+
+  if not bad_macros:
+    return []
+
+  return [output_api.PresubmitError(
+      'Possibly invalid if defined macro[s] found. Please fix your code\n'
+      'or add your macro to src/PRESUBMIT.py.', bad_macros)]

[stuartmorgan, 2014/09/25 23:42:32]

Where would they add it to fix the error?

[lliabraa, 2014/09/26 18:22:43]

Done. I've changed this text a bit. If the ifdef is intentional then they
probably need to remove the macro from the of always defined macros.

+
+
 def _CheckForIPCRules(input_api, output_api):
   """Check for same IPC rules described in
   http://www.chromium.org/Home/chromium-security/education/security-tips-for-ipc