PRESUBMIT.py

   1 # Copyright (c) 2012 The Chromium Authors. All rights reserved.
   2 # Use of this source code is governed by a BSD-style license that can be
   3 # found in the LICENSE file.
   4
   5 """Top-level presubmit script for Chromium.
   6
   7 See http://dev.chromium.org/developers/how-tos/depottools/presubmit-scripts
   8 for more details about the presubmit API built into gcl.
   9 """
  10
  11
  12 import re
  13 import sys
  14
  15
  16 _EXCLUDED_PATHS = (
  17     r"^breakpad[\\\/].*",
  18     r"^native_client_sdk[\\\/].*",
  19     r"^net[\\\/]tools[\\\/]spdyshark[\\\/].*",
  20     r"^skia[\\\/].*",
  21     r"^v8[\\\/].*",
  22     r".*MakeFile$",
  23     r".+_autogen\.h$",
  24 )
  25
  26
  27 _TEST_ONLY_WARNING = (
  28     'You might be calling functions intended only for testing from\n'
  29     'production code.  It is OK to ignore this warning if you know what\n'
  30     'you are doing, as the heuristics used to detect the situation are\n'
  31     'not perfect.  The commit queue will not block on this warning.\n'
  32     'Email joi@chromium.org if you have questions.')
  33
  34
  35 _BANNED_OBJC_FUNCTIONS = (
  36     (
  37       'addTrackingRect:',
  38       (
  39        'The use of -[NSView addTrackingRect:owner:userData:assumeInside:] is'
  40        'prohibited. Please use CrTrackingArea instead.',
  41        'http://dev.chromium.org/developers/coding-style/cocoa-dos-and-donts',
  42       ),
  43       False,
  44     ),
  45     (
  46       'NSTrackingArea',
  47       (
  48        'The use of NSTrackingAreas is prohibited. Please use CrTrackingArea',
  49        'instead.',
  50        'http://dev.chromium.org/developers/coding-style/cocoa-dos-and-donts',
  51       ),
  52       False,
  53     ),
  54     (
  55       'convertPointFromBase:',
  56       (
  57        'The use of -[NSView convertPointFromBase:] is almost certainly wrong.',
  58        'Please use |convertPoint:(point) fromView:nil| instead.',
  59        'http://dev.chromium.org/developers/coding-style/cocoa-dos-and-donts',
  60       ),
  61       True,
  62     ),
  63     (
  64       'convertPointToBase:',
  65       (
  66        'The use of -[NSView convertPointToBase:] is almost certainly wrong.',
  67        'Please use |convertPoint:(point) toView:nil| instead.',
  68        'http://dev.chromium.org/developers/coding-style/cocoa-dos-and-donts',
  69       ),
  70       True,
  71     ),
  72     (
  73       'convertRectFromBase:',
  74       (
  75        'The use of -[NSView convertRectFromBase:] is almost certainly wrong.',
  76        'Please use |convertRect:(point) fromView:nil| instead.',
  77        'http://dev.chromium.org/developers/coding-style/cocoa-dos-and-donts',
  78       ),
  79       True,
  80     ),
  81     (
  82       'convertRectToBase:',
  83       (
  84        'The use of -[NSView convertRectToBase:] is almost certainly wrong.',
  85        'Please use |convertRect:(point) toView:nil| instead.',
  86        'http://dev.chromium.org/developers/coding-style/cocoa-dos-and-donts',
  87       ),
  88       True,
  89     ),
  90     (
  91       'convertSizeFromBase:',
  92       (
  93        'The use of -[NSView convertSizeFromBase:] is almost certainly wrong.',
  94        'Please use |convertSize:(point) fromView:nil| instead.',
  95        'http://dev.chromium.org/developers/coding-style/cocoa-dos-and-donts',
  96       ),
  97       True,
  98     ),
  99     (
 100       'convertSizeToBase:',
 101       (
 102        'The use of -[NSView convertSizeToBase:] is almost certainly wrong.',
 103        'Please use |convertSize:(point) toView:nil| instead.',
 104        'http://dev.chromium.org/developers/coding-style/cocoa-dos-and-donts',
 105       ),
 106       True,
 107     ),
 108 )
 109
 110
 111 _BANNED_CPP_FUNCTIONS = (
 112     # Make sure that gtest's FRIEND_TEST() macro is not used; the
 113     # FRIEND_TEST_ALL_PREFIXES() macro from base/gtest_prod_util.h should be
 114     # used instead since that allows for FLAKY_, FAILS_ and DISABLED_ prefixes.
 115     (
 116       'FRIEND_TEST(',
 117       (
 118        'Chromium code should not use gtest\'s FRIEND_TEST() macro. Include',
 119        'base/gtest_prod_util.h and use FRIEND_TEST_ALL_PREFIXES() instead.',
 120       ),
 121       False,
 122     ),
 123     (
 124       'ScopedAllowIO',
 125       (
 126        'New code should not use ScopedAllowIO. Post a task to the blocking',
 127        'pool or the FILE thread instead.',
 128       ),
 129       True,
 130     ),
 131     (
 132       'FilePathWatcher::Delegate',
 133       (
 134        'New code should not use FilePathWatcher::Delegate. Use the callback',
 135        'interface instead.',
 136       ),
 137       False,
 138     ),
 139     (
 140       'browser::FindLastActiveWithProfile',
 141       (
 142        'This function is deprecated and we\'re working on removing it. Pass',
 143        'more context to get a Browser*, like a WebContents, window, or session',
 144        'id. Talk to ben@ or jam@ for more information.',
 145       ),
 146       True,
 147     ),
 148     (
 149       'browser::FindBrowserWithProfile',
 150       (
 151        'This function is deprecated and we\'re working on removing it. Pass',
 152        'more context to get a Browser*, like a WebContents, window, or session',
 153        'id. Talk to ben@ or jam@ for more information.',
 154       ),
 155       True,
 156     ),
 157     (
 158       'browser::FindAnyBrowser',
 159       (
 160        'This function is deprecated and we\'re working on removing it. Pass',
 161        'more context to get a Browser*, like a WebContents, window, or session',
 162        'id. Talk to ben@ or jam@ for more information.',
 163       ),
 164       True,
 165     ),
 166     (
 167       'browser::FindOrCreateTabbedBrowser',
 168       (
 169        'This function is deprecated and we\'re working on removing it. Pass',
 170        'more context to get a Browser*, like a WebContents, window, or session',
 171        'id. Talk to ben@ or jam@ for more information.',
 172       ),
 173       True,
 174     ),
 175     (
 176       'browser::FindTabbedBrowser',
 177       (
 178        'This function is deprecated and we\'re working on removing it. Pass',
 179        'more context to get a Browser*, like a WebContents, window, or session',
 180        'id. Talk to ben@ or jam@ for more information.',
 181       ),
 182       True,
 183     ),
 184 )
 185
 186
 187
 188 def _CheckNoProductionCodeUsingTestOnlyFunctions(input_api, output_api):
 189   """Attempts to prevent use of functions intended only for testing in
 190   non-testing code. For now this is just a best-effort implementation
 191   that ignores header files and may have some false positives. A
 192   better implementation would probably need a proper C++ parser.
 193   """
 194   # We only scan .cc files and the like, as the declaration of
 195   # for-testing functions in header files are hard to distinguish from
 196   # calls to such functions without a proper C++ parser.
 197   platform_specifiers = r'(_(android|chromeos|gtk|mac|posix|win))?'
 198   source_extensions = r'\.(cc|cpp|cxx|mm)$'
 199   file_inclusion_pattern = r'.+%s' % source_extensions
 200   file_exclusion_patterns = (
 201       r'.*[/\\](test_|mock_).+%s' % source_extensions,
 202       r'.+_test_(base|support|util)%s' % source_extensions,
 203       r'.+_(api|browser|perf|unit|ui)?test%s%s' % (platform_specifiers,
 204                                                    source_extensions),
 205       r'.+profile_sync_service_harness%s' % source_extensions,
 206       )
 207   path_exclusion_patterns = (
 208       r'.*[/\\](test|tool(s)?)[/\\].*',
 209       # At request of folks maintaining this folder.
 210       r'chrome[/\\]browser[/\\]automation[/\\].*',
 211       )
 212
 213   base_function_pattern = r'ForTest(ing)?|for_test(ing)?'
 214   inclusion_pattern = input_api.re.compile(r'(%s)\s*\(' % base_function_pattern)
 215   exclusion_pattern = input_api.re.compile(
 216     r'::[A-Za-z0-9_]+(%s)|(%s)[^;]+\{' % (
 217       base_function_pattern, base_function_pattern))
 218
 219   def FilterFile(affected_file):
 220     black_list = (file_exclusion_patterns + path_exclusion_patterns +
 221                   _EXCLUDED_PATHS + input_api.DEFAULT_BLACK_LIST)
 222     return input_api.FilterSourceFile(
 223       affected_file,
 224       white_list=(file_inclusion_pattern, ),
 225       black_list=black_list)
 226
 227   problems = []
 228   for f in input_api.AffectedSourceFiles(FilterFile):
 229     local_path = f.LocalPath()
 230     lines = input_api.ReadFile(f).splitlines()
 231     line_number = 0
 232     for line in lines:
 233       if (inclusion_pattern.search(line) and
 234           not exclusion_pattern.search(line)):
 235         problems.append(
 236           '%s:%d\n    %s' % (local_path, line_number, line.strip()))
 237       line_number += 1
 238
 239   if problems:
 240     if not input_api.is_committing:
 241       return [output_api.PresubmitPromptWarning(_TEST_ONLY_WARNING, problems)]
 242     else:
 243       # We don't warn on commit, to avoid stopping commits going through CQ.
 244       return [output_api.PresubmitNotifyResult(_TEST_ONLY_WARNING, problems)]
 245   else:
 246     return []
 247
 248
 249 def _CheckNoIOStreamInHeaders(input_api, output_api):
 250   """Checks to make sure no .h files include <iostream>."""
 251   files = []
 252   pattern = input_api.re.compile(r'^#include\s*<iostream>',
 253                                  input_api.re.MULTILINE)
 254   for f in input_api.AffectedSourceFiles(input_api.FilterSourceFile):
 255     if not f.LocalPath().endswith('.h'):
 256       continue
 257     contents = input_api.ReadFile(f)
 258     if pattern.search(contents):
 259       files.append(f)
 260
 261   if len(files):
 262     return [ output_api.PresubmitError(
 263         'Do not #include <iostream> in header files, since it inserts static '
 264         'initialization into every file including the header. Instead, '
 265         '#include <ostream>. See http://crbug.com/94794',
 266         files) ]
 267   return []
 268
 269
 270 def _CheckNoUNIT_TESTInSourceFiles(input_api, output_api):
 271   """Checks to make sure no source files use UNIT_TEST"""
 272   problems = []
 273   for f in input_api.AffectedFiles():
 274     if (not f.LocalPath().endswith(('.cc', '.mm'))):
 275       continue
 276
 277     for line_num, line in f.ChangedContents():
 278       if 'UNIT_TEST' in line:
 279         problems.append('    %s:%d' % (f.LocalPath(), line_num))
 280
 281   if not problems:
 282     return []
 283   return [output_api.PresubmitPromptWarning('UNIT_TEST is only for headers.\n' +
 284       '\n'.join(problems))]
 285
 286
 287 def _CheckNoNewWStrings(input_api, output_api):
 288   """Checks to make sure we don't introduce use of wstrings."""
 289   problems = []
 290   for f in input_api.AffectedFiles():
 291     if (not f.LocalPath().endswith(('.cc', '.h')) or
 292         f.LocalPath().endswith('test.cc')):
 293       continue
 294
 295     allowWString = False
 296     for line_num, line in f.ChangedContents():
 297       if 'presubmit: allow wstring' in line:
 298         allowWString = True
 299       elif not allowWString and 'wstring' in line:
 300         problems.append('    %s:%d' % (f.LocalPath(), line_num))
 301         allowWString = False
 302       else:
 303         allowWString = False
 304
 305   if not problems:
 306     return []
 307   return [output_api.PresubmitPromptWarning('New code should not use wstrings.'
 308       '  If you are calling a cross-platform API that accepts a wstring, '
 309       'fix the API.\n' +
 310       '\n'.join(problems))]
 311
 312
 313 def _CheckNoDEPSGIT(input_api, output_api):
 314   """Make sure .DEPS.git is never modified manually."""
 315   if any(f.LocalPath().endswith('.DEPS.git') for f in
 316       input_api.AffectedFiles()):
 317     return [output_api.PresubmitError(
 318       'Never commit changes to .DEPS.git. This file is maintained by an\n'
 319       'automated system based on what\'s in DEPS and your changes will be\n'
 320       'overwritten.\n'
 321       'See http://code.google.com/p/chromium/wiki/UsingNewGit#Rolling_DEPS\n'
 322       'for more information')]
 323   return []
 324
 325
 326 def _CheckNoBannedFunctions(input_api, output_api):
 327   """Make sure that banned functions are not used."""
 328   warnings = []
 329   errors = []
 330
 331   file_filter = lambda f: f.LocalPath().endswith(('.mm', '.m', '.h'))
 332   for f in input_api.AffectedFiles(file_filter=file_filter):
 333     for line_num, line in f.ChangedContents():
 334       for func_name, message, error in _BANNED_OBJC_FUNCTIONS:
 335         if func_name in line:
 336           problems = warnings;
 337           if error:
 338             problems = errors;
 339           problems.append('    %s:%d:' % (f.LocalPath(), line_num))
 340           for message_line in message:
 341             problems.append('      %s' % message_line)
 342
 343   file_filter = lambda f: f.LocalPath().endswith(('.cc', '.mm', '.h'))
 344   for f in input_api.AffectedFiles(file_filter=file_filter):
 345     for line_num, line in f.ChangedContents():
 346       for func_name, message, error in _BANNED_CPP_FUNCTIONS:
 347         if func_name in line:
 348           problems = warnings;
 349           if error:
 350             problems = errors;
 351           problems.append('    %s:%d:' % (f.LocalPath(), line_num))
 352           for message_line in message:
 353             problems.append('      %s' % message_line)
 354
 355   result = []
 356   if (warnings):
 357     result.append(output_api.PresubmitPromptWarning(
 358         'Banned functions were used.\n' + '\n'.join(warnings)))
 359   if (errors):
 360     result.append(output_api.PresubmitError(
 361         'Banned functions were used.\n' + '\n'.join(errors)))
 362   return result
 363
 364
 365 def _CheckNoPragmaOnce(input_api, output_api):
 366   """Make sure that banned functions are not used."""
 367   files = []
 368   pattern = input_api.re.compile(r'^#pragma\s+once',
 369                                  input_api.re.MULTILINE)
 370   for f in input_api.AffectedSourceFiles(input_api.FilterSourceFile):
 371     if not f.LocalPath().endswith('.h'):
 372       continue
 373     contents = input_api.ReadFile(f)
 374     if pattern.search(contents):
 375       files.append(f)
 376
 377   if files:
 378     return [output_api.PresubmitError(
 379         'Do not use #pragma once in header files.\n'
 380         'See http://www.chromium.org/developers/coding-style#TOC-File-headers',
 381         files)]
 382   return []
 383
 384
 385 def _CheckUnwantedDependencies(input_api, output_api):
 386   """Runs checkdeps on #include statements added in this
 387   change. Breaking - rules is an error, breaking ! rules is a
 388   warning.
 389   """
 390   # We need to wait until we have an input_api object and use this
 391   # roundabout construct to import checkdeps because this file is
 392   # eval-ed and thus doesn't have __file__.
 393   original_sys_path = sys.path
 394   try:
 395     sys.path = sys.path + [input_api.os_path.join(
 396         input_api.PresubmitLocalPath(), 'tools', 'checkdeps')]
 397     import checkdeps
 398     from cpp_checker import CppChecker
 399     from rules import Rule
 400   finally:
 401     # Restore sys.path to what it was before.
 402     sys.path = original_sys_path
 403
 404   added_includes = []
 405   for f in input_api.AffectedFiles():
 406     if not CppChecker.IsCppFile(f.LocalPath()):
 407       continue
 408
 409     changed_lines = [line for line_num, line in f.ChangedContents()]
 410     added_includes.append([f.LocalPath(), changed_lines])
 411
 412   deps_checker = checkdeps.DepsChecker()
 413
 414   error_descriptions = []
 415   warning_descriptions = []
 416   for path, rule_type, rule_description in deps_checker.CheckAddedCppIncludes(
 417       added_includes):
 418     description_with_path = '%s\n    %s' % (path, rule_description)
 419     if rule_type == Rule.DISALLOW:
 420       error_descriptions.append(description_with_path)
 421     else:
 422       warning_descriptions.append(description_with_path)
 423
 424   results = []
 425   if error_descriptions:
 426     results.append(output_api.PresubmitError(
 427         'You added one or more #includes that violate checkdeps rules.',
 428         error_descriptions))
 429   if warning_descriptions:
 430     results.append(output_api.PresubmitPromptWarning(
 431         'You added one or more #includes of files that are temporarily\n'
 432         'allowed but being removed. Can you avoid introducing the\n'
 433         '#include? See relevant DEPS file(s) for details and contacts.',
 434         warning_descriptions))
 435   return results
 436
 437
 438 def _CommonChecks(input_api, output_api):
 439   """Checks common to both upload and commit."""
 440   results = []
 441   results.extend(input_api.canned_checks.PanProjectChecks(
 442       input_api, output_api, excluded_paths=_EXCLUDED_PATHS))
 443   results.extend(_CheckAuthorizedAuthor(input_api, output_api))
 444   results.extend(
 445     _CheckNoProductionCodeUsingTestOnlyFunctions(input_api, output_api))
 446   results.extend(_CheckNoIOStreamInHeaders(input_api, output_api))
 447   results.extend(_CheckNoUNIT_TESTInSourceFiles(input_api, output_api))
 448   results.extend(_CheckNoNewWStrings(input_api, output_api))
 449   results.extend(_CheckNoDEPSGIT(input_api, output_api))
 450   results.extend(_CheckNoBannedFunctions(input_api, output_api))
 451   results.extend(_CheckNoPragmaOnce(input_api, output_api))
 452   results.extend(_CheckUnwantedDependencies(input_api, output_api))
 453   return results
 454
 455
 456 def _CheckSubversionConfig(input_api, output_api):
 457   """Verifies the subversion config file is correctly setup.
 458
 459   Checks that autoprops are enabled, returns an error otherwise.
 460   """
 461   join = input_api.os_path.join
 462   if input_api.platform == 'win32':
 463     appdata = input_api.environ.get('APPDATA', '')
 464     if not appdata:
 465       return [output_api.PresubmitError('%APPDATA% is not configured.')]
 466     path = join(appdata, 'Subversion', 'config')
 467   else:
 468     home = input_api.environ.get('HOME', '')
 469     if not home:
 470       return [output_api.PresubmitError('$HOME is not configured.')]
 471     path = join(home, '.subversion', 'config')
 472
 473   error_msg = (
 474       'Please look at http://dev.chromium.org/developers/coding-style to\n'
 475       'configure your subversion configuration file. This enables automatic\n'
 476       'properties to simplify the project maintenance.\n'
 477       'Pro-tip: just download and install\n'
 478       'http://src.chromium.org/viewvc/chrome/trunk/tools/build/slave/config\n')
 479
 480   try:
 481     lines = open(path, 'r').read().splitlines()
 482     # Make sure auto-props is enabled and check for 2 Chromium standard
 483     # auto-prop.
 484     if (not '*.cc = svn:eol-style=LF' in lines or
 485         not '*.pdf = svn:mime-type=application/pdf' in lines or
 486         not 'enable-auto-props = yes' in lines):
 487       return [
 488           output_api.PresubmitNotifyResult(
 489               'It looks like you have not configured your subversion config '
 490               'file or it is not up-to-date.\n' + error_msg)
 491       ]
 492   except (OSError, IOError):
 493     return [
 494         output_api.PresubmitNotifyResult(
 495             'Can\'t find your subversion config file.\n' + error_msg)
 496     ]
 497   return []
 498
 499
 500 def _CheckAuthorizedAuthor(input_api, output_api):
 501   """For non-googler/chromites committers, verify the author's email address is
 502   in AUTHORS.
 503   """
 504   # TODO(maruel): Add it to input_api?
 505   import fnmatch
 506
 507   author = input_api.change.author_email
 508   if not author:
 509     input_api.logging.info('No author, skipping AUTHOR check')
 510     return []
 511   authors_path = input_api.os_path.join(
 512       input_api.PresubmitLocalPath(), 'AUTHORS')
 513   valid_authors = (
 514       input_api.re.match(r'[^#]+\s+\<(.+?)\>\s*$', line)
 515       for line in open(authors_path))
 516   valid_authors = [item.group(1).lower() for item in valid_authors if item]
 517   if input_api.verbose:
 518     print 'Valid authors are %s' % ', '.join(valid_authors)
 519   if not any(fnmatch.fnmatch(author.lower(), valid) for valid in valid_authors):
 520     return [output_api.PresubmitPromptWarning(
 521         ('%s is not in AUTHORS file. If you are a new contributor, please visit'
 522         '\n'
 523         'http://www.chromium.org/developers/contributing-code and read the '
 524         '"Legal" section\n'
 525         'If you are a chromite, verify the contributor signed the CLA.') %
 526         author)]
 527   return []
 528
 529
 530 def CheckChangeOnUpload(input_api, output_api):
 531   results = []
 532   results.extend(_CommonChecks(input_api, output_api))
 533   return results
 534
 535
 536 def CheckChangeOnCommit(input_api, output_api):
 537   results = []
 538   results.extend(_CommonChecks(input_api, output_api))
 539   # TODO(thestig) temporarily disabled, doesn't work in third_party/
 540   #results.extend(input_api.canned_checks.CheckSvnModifiedDirectories(
 541   #    input_api, output_api, sources))
 542   # Make sure the tree is 'open'.
 543   results.extend(input_api.canned_checks.CheckTreeIsOpen(
 544       input_api,
 545       output_api,
 546       json_url='http://chromium-status.appspot.com/current?format=json'))
 547   results.extend(input_api.canned_checks.CheckRietveldTryJobExecution(input_api,
 548       output_api, 'http://codereview.chromium.org',
 549       ('win_rel', 'linux_rel', 'mac_rel, win:compile'),
 550       'tryserver@chromium.org'))
 551
 552   results.extend(input_api.canned_checks.CheckChangeHasBugField(
 553       input_api, output_api))
 554   results.extend(input_api.canned_checks.CheckChangeHasDescription(
 555       input_api, output_api))
 556   results.extend(_CheckSubversionConfig(input_api, output_api))
 557   return results
 558
 559
 560 def GetPreferredTrySlaves(project, change):
 561   files = change.LocalPaths()
 562
 563   if not files:
 564     return []
 565
 566   if all(re.search('\.(m|mm)$|[/_]mac[/_.]', f) for f in files):
 567     return ['mac_rel']
 568   if all(re.search('[/_]win[/_.]', f) for f in files):
 569     return ['win_rel']
 570   if all(re.search('[/_]android[/_.]', f) for f in files):
 571     return ['android']
 572
 573   trybots = ['win_rel', 'linux_rel', 'mac_rel', 'linux_clang:compile',
 574              'android']
 575
 576   # Match things like path/aura/file.cc and path/file_aura.cc.
 577   # Same for chromeos.
 578   if any(re.search('[/_](aura|chromeos)', f) for f in files):
 579     trybots += ['linux_chromeos', 'linux_chromeos_clang:compile']
 580
 581   return trybots