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

PRESUBMIT.py

Index: PRESUBMIT.py
diff --git a/PRESUBMIT.py b/PRESUBMIT.py
index 3e18813a589db12b25e0da7f0e93b7217e83e83b..43adb248f5db7670d3f0c36d7345244748a33959 100644
--- a/PRESUBMIT.py
+++ b/PRESUBMIT.py
@@ -1062,6 +1062,93 @@ def _CheckJavaStyle(input_api, output_api):
       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 = []
@@ -1097,6 +1184,7 @@ def _CommonChecks(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(