Add PRESUBMIT.py warning for contradictory NOTREACHED() use.

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 1044 matching lines @@
         input_api.PresubmitLocalPath(), 'tools', 'android', 'checkstyle')]
     import checkstyle
   finally:
     # Restore sys.path to what it was before.
     sys.path = original_sys_path
 
   return checkstyle.RunCheckstyle(
       input_api, output_api, 'tools/android/checkstyle/chromium-style-5.0.xml')
 
 
+def _StripCommentsAndStrings(input_api, s):
+  """Remove comments, replace string literals by a single token.  Requires that
+  input data is formatted with unix-style line ends."""

[M-A Ruel, 2014/06/24 17:45:22]

The CRLF -> LF reduction is unnecessary as per 
https://chromium.googlesource.com/chromium/tools/depot_tools.git/+/master/pre...
'rU' means python does the conversion for you.

[Thiemo Nagel, 2014/06/25 06:45:54]

Thanks.  I was foiled into believing \r wasn't stripped because MockFile doesn't
do it.

+
+  s = input_api.re.sub(r'\\\n', r'', s)    # Continue lines ending in backslash.
+
+  out = ''
+  i = 0
+  while i < len(s):
+    c = s[i]
+
+    if c == '/':
+      mo = input_api.re.match(r'//.*', s[i:])
+      if mo:
+        i += len(mo.group(0))
+        continue
+      mo = input_api.re.match(r'/\*.*?\*/', s[i:], input_api.re.DOTALL)
+      if mo:
+        i += len(mo.group(0))
+        continue
+
+    if c == "'":
+      mo = input_api.re.match(r"'((\\\\)|(\\')|[^']+?)'", s[i:])
+      if not mo:
+        raise Exception('bad char: ' + s[i:])
+      i += len(mo.group(0))
+      out += ' CHAR_LITERAL '
+      continue
+
+    if c == '"':
+      mo = input_api.re.match(r'".*?(?<!\\)(\\\\)*"', s[i:])
+      if not mo:
+        raise Exception('bad string: ' + s[i:])
+      i += len(mo.group(0))
+      out += ' STRING_LITERAL '
+      continue
+
+    out += c
+    i += 1
+
+  return out
+
+
+def _CheckContradictoryNotreachedUse(input_api, output_api):
+  file_inclusion_pattern = (
+      r".+\.c$", r".+\.cc$", r".+\.cpp$", r".+\.h$", r".+\.hpp$", r".+\.inl$",
+      r".+\.m$", r".+\.mm$" )
+  black_list = (_EXCLUDED_PATHS + input_api.DEFAULT_BLACK_LIST)
+  file_filter = lambda f: input_api.FilterSourceFile(
+      f, white_list=file_inclusion_pattern, black_list=black_list)
+  results = []
+  for fpath in input_api.AffectedFiles(file_filter=file_filter):
+    results.extend(_CheckContradictoryNotreachedUseInFile(input_api, fpath))
+  return [output_api.PresubmitPromptWarning(r) for r in results]
+
+
+def _CheckContradictoryNotreachedUseInFile(input_api, f):
+  style_url = (
+      'http://www.chromium.org/developers/coding-style'

[M-A Ruel, 2014/06/24 17:45:22]

optional: you can skip 'www.' to save a few chars if you want.

[Thiemo Nagel, 2014/06/25 06:45:54]

Done.

+      '#TOC-CHECK-DCHECK-and-NOTREACHED-')
+  contents = f.NewContents()
+  text = ''.join(line.rstrip('\r') + '\n' for line in f.NewContents())
+  text = _StripCommentsAndStrings(input_api, text)
+
+  results = []
+  while True:
+    # Capture text between NOTREACHED(); and the next closing brace or "break".
+    mo = input_api.re.search(
+        r'[ \t]*NOTREACHED\(\s*\).*?;(?P<between>.*?)((\bbreak\b)|})',
+        text, input_api.re.DOTALL)
+    # TODO(tnagel): Catch loops inside which NOTREACHED() is followed by break.
+    if not mo:
+      break
+    text = text[mo.end():]
+    if input_api.re.match(r'[\s;]*$', mo.group('between'), input_api.re.DOTALL):
+      continue
+    excerpt = mo.group(0).rstrip()
+    if len(excerpt) > 100:
+      excerpt = excerpt[:100] + ' ...'

[M-A Ruel, 2014/06/24 17:45:22]

nit: Replace ' ...' with '\u2026'. It should work fine even on Windows.

[Thiemo Nagel, 2014/06/25 06:45:54]

Done.

+    results.append(
+        '%s:  NOTREACHED() may only be used at end-of-block '
+        'but is followed by code.\n%s\n'
+        'Offending section (comments/strings possibly stripped):\n%s'
+        % (f, style_url, excerpt))
+  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 + _TESTRUNNER_PATHS))
   results.extend(_CheckAuthorizedAuthor(input_api, output_api))
   results.extend(
       _CheckNoProductionCodeUsingTestOnlyFunctions(input_api, output_api))
   results.extend(_CheckNoIOStreamInHeaders(input_api, output_api))
@@ skipping 15 matching lines @@
   results.extend(_CheckAddedDepsHaveTargetApprovals(input_api, output_api))
   results.extend(
       input_api.canned_checks.CheckChangeHasNoTabs(
           input_api,
           output_api,
           source_file_filter=lambda x: x.LocalPath().endswith('.grd')))
   results.extend(_CheckSpamLogging(input_api, output_api))
   results.extend(_CheckForAnonymousVariables(input_api, output_api))
   results.extend(_CheckCygwinShell(input_api, output_api))
   results.extend(_CheckUserActionUpdate(input_api, output_api))
+  results.extend(_CheckContradictoryNotreachedUse(input_api, output_api))
 
   if any('PRESUBMIT.py' == f.LocalPath() for f in input_api.AffectedFiles()):
     results.extend(input_api.canned_checks.RunUnitTestsInDirectory(
         input_api, output_api,
         input_api.PresubmitLocalPath(),
         whitelist=[r'^PRESUBMIT_test\.py$']))
   return results
 
 
 def _CheckSubversionConfig(input_api, output_api):
@@ skipping 352 matching lines @@
     builders.extend(['cros_x86'])
 
   # The AOSP bot doesn't build the chrome/ layer, so ignore any changes to it
   # unless they're .gyp(i) files as changes to those files can break the gyp
   # step on that bot.
   if (not all(re.search('^chrome', f) for f in files) or
       any(re.search('\.gypi?$', f) for f in files)):
     builders.extend(['android_aosp'])
 
   return GetDefaultTryConfigs(builders)