Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(45)

Side by Side Diff: presubmit_canned_checks.py

Issue 6883050: presubmit checks: skip computing the diff if possible (Closed) Base URL: http://src.chromium.org/svn/trunk/tools/depot_tools/
Patch Set: Fixed O(n^2) issue. Created 9 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch | Annotate | Revision Log
« no previous file with comments | « no previous file | tests/presubmit_unittest.py » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 # Copyright (c) 2010 The Chromium Authors. All rights reserved. 1 # Copyright (c) 2010 The Chromium Authors. All rights reserved.
2 # Use of this source code is governed by a BSD-style license that can be 2 # Use of this source code is governed by a BSD-style license that can be
3 # found in the LICENSE file. 3 # found in the LICENSE file.
4 4
5 """Generic presubmit checks that can be reused by other presubmit checks.""" 5 """Generic presubmit checks that can be reused by other presubmit checks."""
6 6
7 7
8 ### Description checks 8 ### Description checks
9 9
10 def CheckChangeHasTestField(input_api, output_api): 10 def CheckChangeHasTestField(input_api, output_api):
(...skipping 49 matching lines...) Expand 10 before | Expand all | Expand 10 after
60 if input_api.is_committing: 60 if input_api.is_committing:
61 return [output_api.PresubmitError('Add a description.')] 61 return [output_api.PresubmitError('Add a description.')]
62 else: 62 else:
63 return [output_api.PresubmitNotifyResult('Add a description.')] 63 return [output_api.PresubmitNotifyResult('Add a description.')]
64 return [] 64 return []
65 65
66 ### Content checks 66 ### Content checks
67 67
68 def CheckDoNotSubmitInFiles(input_api, output_api): 68 def CheckDoNotSubmitInFiles(input_api, output_api):
69 """Checks that the user didn't add 'DO NOT ' + 'SUBMIT' to any files.""" 69 """Checks that the user didn't add 'DO NOT ' + 'SUBMIT' to any files."""
70 # We want to check every text file, not just source files.
71 file_filter = lambda x : x
70 keyword = 'DO NOT ' + 'SUBMIT' 72 keyword = 'DO NOT ' + 'SUBMIT'
71 # We want to check every text files, not just source files. 73 errors = _FindNewViolationsOfRule(lambda line : keyword not in line,
72 for f, line_num, line in input_api.RightHandSideLines(lambda x: x): 74 input_api, file_filter)
73 if keyword in line: 75 text = '\n'.join('Found %s in %s' % (keyword, loc) for loc in errors)
74 text = 'Found ' + keyword + ' in %s, line %s' % (f.LocalPath(), line_num) 76 if text:
75 return [output_api.PresubmitError(text)] 77 return [output_api.PresubmitError(text)]
76 return [] 78 return []
77 79
78 80
79 def CheckChangeLintsClean(input_api, output_api, source_file_filter=None): 81 def CheckChangeLintsClean(input_api, output_api, source_file_filter=None):
80 """Checks that all '.cc' and '.h' files pass cpplint.py.""" 82 """Checks that all '.cc' and '.h' files pass cpplint.py."""
81 _RE_IS_TEST = input_api.re.compile(r'.*tests?.(cc|h)$') 83 _RE_IS_TEST = input_api.re.compile(r'.*tests?.(cc|h)$')
82 result = [] 84 result = []
83 85
84 # Initialize cpplint. 86 # Initialize cpplint.
85 import cpplint 87 import cpplint
(...skipping 120 matching lines...) Expand 10 before | Expand all | Expand 10 after
206 if cr_files: 208 if cr_files:
207 outputs.append(output_api.PresubmitPromptWarning( 209 outputs.append(output_api.PresubmitPromptWarning(
208 'Found a CR character in these files:', items=cr_files)) 210 'Found a CR character in these files:', items=cr_files))
209 if eof_files: 211 if eof_files:
210 outputs.append(output_api.PresubmitPromptWarning( 212 outputs.append(output_api.PresubmitPromptWarning(
211 'These files should end in one (and only one) newline character:', 213 'These files should end in one (and only one) newline character:',
212 items=eof_files)) 214 items=eof_files))
213 return outputs 215 return outputs
214 216
215 217
218 def _ReportErrorFileAndLine(filename, line_num, line):
219 """Default error formatter for _FindNewViolationsOfRule."""
220 return '%s, line %s' % (filename, line_num)
221
222
223 def _FindNewViolationsOfRule(callable_rule, input_api, source_file_filter=None,
224 error_formatter=_ReportErrorFileAndLine):
225 """Find all newly introduced violations of a per-line rule (a callable).
226
227 Arguments:
228 callable_rule: a callable taking a line of input and returning True
229 if the rule is satisfied and False if there was a problem.
230 input_api: object to enumerate the affected files.
231 source_file_filter: a filter to be passed to the input api.
232 error_formatter: a callable taking (filename, line_number, line) and
233 returning a formatted error string.
234
235 Returns:
236 A list of the newly-introduced violations reported by the rule.
237 """
238 errors = []
239 for f in input_api.AffectedFiles(source_file_filter, include_deletes=False):
240 # For speed, we do two passes, checking first the full file. Shelling out
241 # to the SCM to determine the changed region can be quite expensive on
242 # Win32. Assuming that most files will be kept problem-free, we can
243 # skip the SCM operations most of the time.
244 if all(callable_rule(line) for line in f.NewContents()):
245 continue # No violation found in full text: can skip considering diff.
246
247 for line_num, line in f.ChangedContents():
248 if not callable_rule(line):
249 errors.append(error_formatter(f.LocalPath(), line_num, line))
250
251 return errors
252
253
216 def CheckChangeHasNoTabs(input_api, output_api, source_file_filter=None): 254 def CheckChangeHasNoTabs(input_api, output_api, source_file_filter=None):
217 """Checks that there are no tab characters in any of the text files to be 255 """Checks that there are no tab characters in any of the text files to be
218 submitted. 256 submitted.
219 """ 257 """
220 # In addition to the filter, make sure that makefiles are blacklisted. 258 # In addition to the filter, make sure that makefiles are blacklisted.
221 if not source_file_filter: 259 if not source_file_filter:
222 # It's the default filter. 260 # It's the default filter.
223 source_file_filter = input_api.FilterSourceFile 261 source_file_filter = input_api.FilterSourceFile
224 def filter_more(affected_file): 262 def filter_more(affected_file):
225 return (not input_api.os_path.basename(affected_file.LocalPath()) in 263 return (not input_api.os_path.basename(affected_file.LocalPath()) in
226 ('Makefile', 'makefile') and 264 ('Makefile', 'makefile') and
227 source_file_filter(affected_file)) 265 source_file_filter(affected_file))
228 tabs = [] 266
229 for f, line_num, line in input_api.RightHandSideLines(filter_more): 267 tabs = _FindNewViolationsOfRule(lambda line : '\t' not in line,
230 if '\t' in line: 268 input_api, filter_more)
231 tabs.append('%s, line %s' % (f.LocalPath(), line_num)) 269
232 if tabs: 270 if tabs:
233 return [output_api.PresubmitPromptWarning('Found a tab character in:', 271 return [output_api.PresubmitPromptWarning('Found a tab character in:',
234 long_text='\n'.join(tabs))] 272 long_text='\n'.join(tabs))]
235 return [] 273 return []
236 274
237 275
238 def CheckChangeTodoHasOwner(input_api, output_api, source_file_filter=None): 276 def CheckChangeTodoHasOwner(input_api, output_api, source_file_filter=None):
239 """Checks that the user didn't add TODO(name) without an owner.""" 277 """Checks that the user didn't add TODO(name) without an owner."""
240 278
241 unowned_todo = input_api.re.compile('TO' + 'DO[^(]') 279 unowned_todo = input_api.re.compile('TO' + 'DO[^(]')
242 for f, line_num, line in input_api.RightHandSideLines(source_file_filter): 280 errors = _FindNewViolationsOfRule(lambda x : not unowned_todo.search(x),
243 if unowned_todo.search(line): 281 input_api, source_file_filter)
244 text = ('Found TO' + 'DO with no owner in %s, line %s' % 282 errors = ['Found TO' + 'DO with no owner in ' + x for x in errors]
245 (f.LocalPath(), line_num)) 283 if errors:
246 return [output_api.PresubmitPromptWarning(text)] 284 return [output_api.PresubmitPromptWarning('\n'.join(errors))]
247 return [] 285 return []
248 286
249 287
250 def CheckChangeHasNoStrayWhitespace(input_api, output_api, 288 def CheckChangeHasNoStrayWhitespace(input_api, output_api,
251 source_file_filter=None): 289 source_file_filter=None):
252 """Checks that there is no stray whitespace at source lines end.""" 290 """Checks that there is no stray whitespace at source lines end."""
253 errors = [] 291 errors = _FindNewViolationsOfRule(lambda line : line.rstrip() == line,
254 for f, line_num, line in input_api.RightHandSideLines(source_file_filter): 292 input_api, source_file_filter)
255 if line.rstrip() != line:
256 errors.append('%s, line %s' % (f.LocalPath(), line_num))
257 if errors: 293 if errors:
258 return [output_api.PresubmitPromptWarning( 294 return [output_api.PresubmitPromptWarning(
259 'Found line ending with white spaces in:', 295 'Found line ending with white spaces in:',
260 long_text='\n'.join(errors))] 296 long_text='\n'.join(errors))]
261 return [] 297 return []
262 298
263 299
264 def CheckLongLines(input_api, output_api, maxlen=80, source_file_filter=None): 300 def CheckLongLines(input_api, output_api, maxlen=80, source_file_filter=None):
265 """Checks that there aren't any lines longer than maxlen characters in any of 301 """Checks that there aren't any lines longer than maxlen characters in any of
266 the text files to be submitted. 302 the text files to be submitted.
267 """ 303 """
268 bad = [] 304 def no_long_lines(line):
269 for f, line_num, line in input_api.RightHandSideLines(source_file_filter):
270 # Allow lines with http://, https:// and #define/#pragma/#include/#if/#endif 305 # Allow lines with http://, https:// and #define/#pragma/#include/#if/#endif
271 # to exceed the maxlen rule. 306 # to exceed the maxlen rule.
272 if (len(line) > maxlen and 307 return (len(line) <= maxlen or
273 not 'http://' in line and 308 any((url in line) for url in ('http://', 'https://')) or
274 not 'https://' in line and 309 line.startswith(('#define', '#include', '#import', '#pragma',
275 not line.startswith('#define') and 310 '#if', '#endif')))
276 not line.startswith('#include') and
277 not line.startswith('#import') and
278 not line.startswith('#pragma') and
279 not line.startswith('#if') and
280 not line.startswith('#endif')):
281 bad.append(
282 '%s, line %s, %s chars' %
283 (f.LocalPath(), line_num, len(line)))
284 if len(bad) == 5: # Just show the first 5 errors.
285 break
286 311
287 if bad: 312 def format_error(filename, line_num, line):
313 return '%s, line %s, %s chars' % (filename, line_num, len(line))
314
315 errors = _FindNewViolationsOfRule(no_long_lines, input_api,
316 source_file_filter,
317 error_formatter=format_error)
318 if errors:
288 msg = 'Found lines longer than %s characters (first 5 shown).' % maxlen 319 msg = 'Found lines longer than %s characters (first 5 shown).' % maxlen
289 return [output_api.PresubmitPromptWarning(msg, items=bad)] 320 return [output_api.PresubmitPromptWarning(msg, items=errors[:5])]
290 else: 321 else:
291 return [] 322 return []
292 323
293 324
294 def CheckLicense(input_api, output_api, license_re, source_file_filter=None, 325 def CheckLicense(input_api, output_api, license_re, source_file_filter=None,
295 accept_empty_files=True): 326 accept_empty_files=True):
296 """Verifies the license header. 327 """Verifies the license header.
297 """ 328 """
298 license_re = input_api.re.compile(license_re, input_api.re.MULTILINE) 329 license_re = input_api.re.compile(license_re, input_api.re.MULTILINE)
299 bad_files = [] 330 bad_files = []
(...skipping 573 matching lines...) Expand 10 before | Expand all | Expand 10 after
873 # This code loads the default black list (e.g. third_party, experimental, etc) 904 # This code loads the default black list (e.g. third_party, experimental, etc)
874 # and add our black list (breakpad, skia and v8 are still not following 905 # and add our black list (breakpad, skia and v8 are still not following
875 # google style and are not really living this repository). 906 # google style and are not really living this repository).
876 # See presubmit_support.py InputApi.FilterSourceFile for the (simple) usage. 907 # See presubmit_support.py InputApi.FilterSourceFile for the (simple) usage.
877 black_list = input_api.DEFAULT_BLACK_LIST + excluded_paths 908 black_list = input_api.DEFAULT_BLACK_LIST + excluded_paths
878 white_list = input_api.DEFAULT_WHITE_LIST + text_files 909 white_list = input_api.DEFAULT_WHITE_LIST + text_files
879 sources = lambda x: input_api.FilterSourceFile(x, black_list=black_list) 910 sources = lambda x: input_api.FilterSourceFile(x, black_list=black_list)
880 text_files = lambda x: input_api.FilterSourceFile(x, black_list=black_list, 911 text_files = lambda x: input_api.FilterSourceFile(x, black_list=black_list,
881 white_list=white_list) 912 white_list=white_list)
882 913
914
915 snapshot_memory = []
916 def snapshot(msg):
917 """Measures & prints performance warning if a rule is running slow."""
918 dt2 = input_api.time.clock()
919 if snapshot_memory:
920 delta_ms = int(1000*(dt2 - snapshot_memory[0]))
921 if delta_ms > 500:
922 print " %s took a long time: %dms" % (snapshot_memory[1], delta_ms)
923 snapshot_memory[:] = (dt2, msg)
924
883 if owners_check: 925 if owners_check:
926 snapshot("checking owners")
884 results.extend(input_api.canned_checks.CheckOwners( 927 results.extend(input_api.canned_checks.CheckOwners(
885 input_api, output_api, source_file_filter=sources)) 928 input_api, output_api, source_file_filter=sources))
886 929
930 snapshot("checking long lines")
887 results.extend(input_api.canned_checks.CheckLongLines( 931 results.extend(input_api.canned_checks.CheckLongLines(
888 input_api, output_api, source_file_filter=sources)) 932 input_api, output_api, source_file_filter=sources))
933 snapshot( "checking tabs")
889 results.extend(input_api.canned_checks.CheckChangeHasNoTabs( 934 results.extend(input_api.canned_checks.CheckChangeHasNoTabs(
890 input_api, output_api, source_file_filter=sources)) 935 input_api, output_api, source_file_filter=sources))
936 snapshot( "checking stray whitespace")
891 results.extend(input_api.canned_checks.CheckChangeHasNoStrayWhitespace( 937 results.extend(input_api.canned_checks.CheckChangeHasNoStrayWhitespace(
892 input_api, output_api, source_file_filter=sources)) 938 input_api, output_api, source_file_filter=sources))
939 snapshot("checking eol style")
893 results.extend(input_api.canned_checks.CheckChangeSvnEolStyle( 940 results.extend(input_api.canned_checks.CheckChangeSvnEolStyle(
894 input_api, output_api, source_file_filter=text_files)) 941 input_api, output_api, source_file_filter=text_files))
942 snapshot("checking svn mime types")
895 results.extend(input_api.canned_checks.CheckSvnForCommonMimeTypes( 943 results.extend(input_api.canned_checks.CheckSvnForCommonMimeTypes(
896 input_api, output_api)) 944 input_api, output_api))
945 snapshot("checking license")
897 results.extend(input_api.canned_checks.CheckLicense( 946 results.extend(input_api.canned_checks.CheckLicense(
898 input_api, output_api, license_header, source_file_filter=sources)) 947 input_api, output_api, license_header, source_file_filter=sources))
948 snapshot("checking nsobjects")
899 results.extend(_CheckConstNSObject( 949 results.extend(_CheckConstNSObject(
900 input_api, output_api, source_file_filter=sources)) 950 input_api, output_api, source_file_filter=sources))
951 snapshot("checking singletons")
901 results.extend(_CheckSingletonInHeaders( 952 results.extend(_CheckSingletonInHeaders(
902 input_api, output_api, source_file_filter=sources)) 953 input_api, output_api, source_file_filter=sources))
954 snapshot("done")
903 return results 955 return results
OLDNEW
« no previous file with comments | « no previous file | tests/presubmit_unittest.py » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698