From 5e4dbc79f83876116aa6fe0d521d18777834494f Mon Sep 17 00:00:00 2001 From: "miguelg@chromium.org" Date: Wed, 25 Jun 2014 14:10:21 +0000 Subject: Revert of Add PRESUBMIT.py warning for contradictory NOTREACHED() use. (https://codereview.chromium.org/344563003/) Reason for revert: It seems to be checking code that is not part of the diff. I also think there are some legitimate cases for not reached in the middle of a block but that is something that can be discussed on the bug. Filed https://code.google.com/p/chromium/issues/detail?id=388731 to track. Original issue's description: > Add PRESUBMIT.py warning for contradictory NOTREACHED() use. > > As recently discussed on chromium-dev [1] and subsequently incorporated into the > Chromium Coding Style [2], NOTREACHED() followed by code is an anti-pattern. > This CL lets PRESUBMIT.py detect most of these cases and print a warning to the > developer. > [1] https://groups.google.com/a/chromium.org/d/msg/chromium-dev/c2X0D1Z6X2o/ddkFLqQP0P0J > [2] http://www.chromium.org/developers/coding-style#TOC-CHECK-DCHECK-and-NOTREACHED- > > BUG=none > > Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=279681 TBR=maruel@chromium.org,tnagel@chromium.org NOTREECHECKS=true NOTRY=true BUG=none Review URL: https://codereview.chromium.org/357693004 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@279710 0039d316-1c4b-4281-b951-d872f2087c98 --- PRESUBMIT.py | 89 ------------------------------------------------------- PRESUBMIT_test.py | 46 ---------------------------- 2 files changed, 135 deletions(-) diff --git a/PRESUBMIT.py b/PRESUBMIT.py index 0fe2e17..c2a4c5c 100644 --- a/PRESUBMIT.py +++ b/PRESUBMIT.py @@ -1229,94 +1229,6 @@ def _CheckNoDeprecatedCSS(input_api, output_api): (fpath.LocalPath(), line_num, deprecated_value, value))) return results - -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.""" - - 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'".*?(?.*?)((\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] + ' \u2026' # ellipsis - 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 = [] @@ -1354,7 +1266,6 @@ def _CommonChecks(input_api, output_api): results.extend(_CheckUserActionUpdate(input_api, output_api)) results.extend(_CheckNoDeprecatedCSS(input_api, output_api)) results.extend(_CheckParseErrors(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( diff --git a/PRESUBMIT_test.py b/PRESUBMIT_test.py index 760d70b..f81b316 100755 --- a/PRESUBMIT_test.py +++ b/PRESUBMIT_test.py @@ -413,52 +413,6 @@ class InvalidOSMacroNamesTest(unittest.TestCase): self.assertEqual(0, len(errors)) -class CheckContradictoryNotreachedUseTest(unittest.TestCase): - def testValid(self): - lines = ['{', - ' // NOTREACHED();', - ' /* NOTREACHED(); */', - " char a = '\\\\', b = '\\0', c = '\\'';", - ' char d[] = "NOTREACHED();";', - ' NOTREACHED(); // blah', - ' /* comment */', - ' // line continuation \\', - ' still inside comment', - ' // comment followed by empty line', - '', - ' ;; ; ;;;', - '}', - 'switch (i) {', - ' case 7: NOTREACHED(); break;', - '}'] - output = PRESUBMIT._CheckContradictoryNotreachedUseInFile( - MockInputApi(), MockFile('some/path/foo_platform.cc', lines)) - self.assertEqual(0, len(output)) - - def testInvalid(self): - lines = ['{', - ' NOTREACHED();', - ' return;', - '}', - '{', - ' NOTREACHED();', - ' /* */', - ' return;', - ' /* */', - '}', - '{', - ' NOTREACHED();', - ' // trailing space, not a line continuation \\ ', - ' return;', - '}', - 'switch (i) {', - ' case 7: NOTREACHED(); some_thing(); break;', - '}'] - output = PRESUBMIT._CheckContradictoryNotreachedUseInFile( - MockInputApi(), MockFile('some/path/foo_platform.cc', lines)) - self.assertEqual(4, len(output)) - - class CheckAddedDepsHaveTetsApprovalsTest(unittest.TestCase): def testFilesToCheckForIncomingDeps(self): changed_lines = [ -- cgit v1.1