From eba646225db35071634dbe7ea14e312e8ef0e478 Mon Sep 17 00:00:00 2001 From: "maruel@chromium.org" Date: Mon, 20 Jun 2011 18:26:48 +0000 Subject: [PATCH] Improve CheckLongLines() to increase the hard limit at 50% when an exception is found. Improve the list of exception to include very_long_symbol_names. R=dpranke@chromium.org BUG= TEST= Review URL: http://codereview.chromium.org/7097012 git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@89696 0039d316-1c4b-4281-b951-d872f2087c98 --- presubmit_canned_checks.py | 24 +++++++++++++++------ tests/presubmit_unittest.py | 42 ++++++++++++++++++++++++++++++++----- 2 files changed, 55 insertions(+), 11 deletions(-) diff --git a/presubmit_canned_checks.py b/presubmit_canned_checks.py index 469578e88..4f267ca66 100644 --- a/presubmit_canned_checks.py +++ b/presubmit_canned_checks.py @@ -302,13 +302,25 @@ def CheckLongLines(input_api, output_api, maxlen=80, source_file_filter=None): """Checks that there aren't any lines longer than maxlen characters in any of the text files to be submitted. """ + # Stupidly long symbols that needs to be worked around if takes 66% of line. + long_symbol = maxlen * 2 / 3 + # Hard line length limit at 50% more. + extra_maxlen = maxlen * 3 / 2 + # Note: these are C++ specific but processed on all languages. :( + MACROS = ('#define', '#include', '#import', '#pragma', '#if', '#endif') + def no_long_lines(line): - # Allow lines with http://, https:// and #define/#pragma/#include/#if/#endif - # to exceed the maxlen rule. - return (len(line) <= maxlen or - any((url in line) for url in ('http://', 'https://')) or - line.startswith(('#define', '#include', '#import', '#pragma', - '#if', '#endif'))) + if len(line) <= maxlen: + return True + + if len(line) > extra_maxlen: + return False + + return ( + line.startswith(MACROS) or + any((url in line) for url in ('http://', 'https://')) or + input_api.re.match( + r'.*[A-Za-z][A-Za-z_0-9]{%d,}.*' % long_symbol, line)) def format_error(filename, line_num, line): return '%s, line %s, %s chars' % (filename, line_num, len(line)) diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py index 5f87a742e..9ef2675b0 100755 --- a/tests/presubmit_unittest.py +++ b/tests/presubmit_unittest.py @@ -1427,7 +1427,7 @@ class CannedChecksUnittest(PresubmitTestsBase): file_filter=mox.IgnoreArg()).AndReturn([affected_file]) affected_file.NewContents().AndReturn([ 'ahoy', - 'yo' + content1, + content1, 'hay', 'yer', 'ya']) @@ -1441,12 +1441,15 @@ class CannedChecksUnittest(PresubmitTestsBase): file_filter=mox.IgnoreArg()).AndReturn([affected_file]) affected_file.NewContents().AndReturn([ 'ahoy', - 'yo' + content2, + content2, 'hay', 'yer', 'ya']) + # It falls back to ChangedContents when there is a failure. This is an + # optimization since NewContents() is much faster to execute than + # ChangedContents(). affected_file.ChangedContents().AndReturn([ - (42, 'yo, ' + content2), + (42, content2), (43, 'yer'), (23, 'ya')]) affected_file.LocalPath().AndReturn('foo.cc') @@ -1646,12 +1649,41 @@ class CannedChecksUnittest(PresubmitTestsBase): self.assertEquals(results1[0]._long_text, 'makefile.foo, line 46') - def testCannedCheckLongLines(self): check = lambda x, y, z: presubmit_canned_checks.CheckLongLines(x, y, 10, z) - self.ContentTest(check, '', 'blah blah blah', + self.ContentTest(check, '0123456789', '01234567890', + presubmit.OutputApi.PresubmitPromptWarning) + + def testCannedCheckLongLinesLF(self): + check = lambda x, y, z: presubmit_canned_checks.CheckLongLines(x, y, 10, z) + self.ContentTest(check, '012345678\n', '0123456789\n', presubmit.OutputApi.PresubmitPromptWarning) + def testCannedCheckLongLinesMacro(self): + check = lambda x, y, z: presubmit_canned_checks.CheckLongLines(x, y, 10, z) + self.ContentTest( + check, + # Put a space in so it doesn't trigger long symbols. Allow 1/3 more. + '#if 56 89 12 45', + '#if 56 89 12 456', + presubmit.OutputApi.PresubmitPromptWarning) + + def testCannedCheckLongLinesHttp(self): + check = lambda x, y, z: presubmit_canned_checks.CheckLongLines(x, y, 10, z) + self.ContentTest( + check, + ' http:// 0 23 5', + ' http:// 0 23 56', + presubmit.OutputApi.PresubmitPromptWarning) + + def testCannedCheckLongLinesLongSymbol(self): + check = lambda x, y, z: presubmit_canned_checks.CheckLongLines(x, y, 10, z) + self.ContentTest( + check, + ' TUP5D_LoNG_SY ', + ' TUP5D_LoNG_SY5 ', + presubmit.OutputApi.PresubmitPromptWarning) + def testCheckChangeSvnEolStyleCommit(self): # Test CheckSvnProperty at the same time. self.SvnPropertyTest(presubmit_canned_checks.CheckChangeSvnEolStyle,