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

Several values are always defined and the developer actually wants to
do an #if check, not #ifdef.

BUG=417804

Review URL: https://codereview.chromium.org/595213004

Cr-Commit-Position: refs/heads/master@{#297627}

2 files changed, 72 insertions, 0 deletions

diff --git a/PRESUBMIT.py b/PRESUBMIT.py
index 46dabb9..65a6cba 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,55 @@ 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_CPU_PPC",
+      "TARGET_CPU_PPC64",
+      "TARGET_CPU_68K",
+      "TARGET_CPU_X86",
+      "TARGET_CPU_ARM",
+      "TARGET_CPU_MIPS",
+      "TARGET_CPU_SPARC",
+      "TARGET_CPU_ALPHA",
+      "TARGET_IPHONE_SIMULATOR",
+      "TARGET_OS_EMBEDDED",
+      "TARGET_OS_IPHONE",
+      "TARGET_OS_MAC",
+      "TARGET_OS_UNIX",
+      "TARGET_OS_WIN32",
+  )
+  ifdef_macro = input_api.re.compile(r'^\s*#.*(?:ifdef\s|defined\()([^\s\)]+)')
+  results = []
+  for lnum, line in f.ChangedContents():
+    for match in ifdef_macro.finditer(line):
+      if match.group(1) in ALWAYS_DEFINED_MACROS:
+        always_defined = ' %s is always defined. ' % match.group(1)
+        did_you_mean = 'Did you mean \'#if %s\'?' % match.group(1)
+        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(
+      'Found ifdef check on always-defined macro[s]. Please fix your code\n'
+      'or check the list of ALWAYS_DEFINED_MACROS in src/PRESUBMIT.py.',
+      bad_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
diff --git a/PRESUBMIT_test.py b/PRESUBMIT_test.py
index dd67eb1..2aed5df 100755
--- a/PRESUBMIT_test.py
+++ b/PRESUBMIT_test.py
@@ -413,6 +413,28 @@ class InvalidOSMacroNamesTest(unittest.TestCase):
     self.assertEqual(0, len(errors))
 
 
+class InvalidIfDefinedMacroNamesTest(unittest.TestCase):
+  def testInvalidIfDefinedMacroNames(self):
+    lines = ['#if defined(TARGET_IPHONE_SIMULATOR)',
+             '#if !defined(TARGET_IPHONE_SIMULATOR)',
+             '#elif defined(TARGET_IPHONE_SIMULATOR)',
+             '#ifdef TARGET_IPHONE_SIMULATOR',
+             ' # ifdef TARGET_IPHONE_SIMULATOR',
+             '# if defined(VALID) || defined(TARGET_IPHONE_SIMULATOR)',
+             '# else  // defined(TARGET_IPHONE_SIMULATOR)',
+             '#endif  // defined(TARGET_IPHONE_SIMULATOR)',]
+    errors = PRESUBMIT._CheckForInvalidIfDefinedMacrosInFile(
+        MockInputApi(), MockFile('some/path/source.mm', lines))
+    self.assertEqual(len(lines), len(errors))
+
+  def testValidIfDefinedMacroNames(self):
+    lines = ['#if defined(FOO)',
+             '#ifdef BAR',]
+    errors = PRESUBMIT._CheckForInvalidIfDefinedMacrosInFile(
+        MockInputApi(), MockFile('some/path/source.cc', lines))
+    self.assertEqual(0, len(errors))
+
+
 class CheckAddedDepsHaveTetsApprovalsTest(unittest.TestCase):
   def testFilesToCheckForIncomingDeps(self):
     changed_lines = [