Chromium Code Reviews
[email protected] (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(293)

Side by Side Diff: PRESUBMIT.py

Issue 11364156: PRESUBMIT.py: Check #include file order. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: code review (maruel) Created 8 years, 1 month 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 | no next file » | 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) 2012 The Chromium Authors. All rights reserved. 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 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 """Top-level presubmit script for Chromium. 5 """Top-level presubmit script for Chromium.
6 6
7 See http://dev.chromium.org/developers/how-tos/depottools/presubmit-scripts 7 See http://dev.chromium.org/developers/how-tos/depottools/presubmit-scripts
8 for more details about the presubmit API built into gcl. 8 for more details about the presubmit API built into gcl.
9 """ 9 """
10 10
(...skipping 19 matching lines...) Expand all
30 30
31 31
32 _TEST_ONLY_WARNING = ( 32 _TEST_ONLY_WARNING = (
33 'You might be calling functions intended only for testing from\n' 33 'You might be calling functions intended only for testing from\n'
34 'production code. It is OK to ignore this warning if you know what\n' 34 'production code. It is OK to ignore this warning if you know what\n'
35 'you are doing, as the heuristics used to detect the situation are\n' 35 'you are doing, as the heuristics used to detect the situation are\n'
36 'not perfect. The commit queue will not block on this warning.\n' 36 'not perfect. The commit queue will not block on this warning.\n'
37 'Email [email protected] if you have questions.') 37 'Email [email protected] if you have questions.')
38 38
39 39
40 _INCLUDE_ORDER_WARNING = (
41 'Your #include order seems to be broken. Send mail to\n'
42 '[email protected] if this is not the case.')
43
44
40 _BANNED_OBJC_FUNCTIONS = ( 45 _BANNED_OBJC_FUNCTIONS = (
41 ( 46 (
42 'addTrackingRect:', 47 'addTrackingRect:',
43 ( 48 (
44 'The use of -[NSView addTrackingRect:owner:userData:assumeInside:] is' 49 'The use of -[NSView addTrackingRect:owner:userData:assumeInside:] is'
45 'prohibited. Please use CrTrackingArea instead.', 50 'prohibited. Please use CrTrackingArea instead.',
46 'http://dev.chromium.org/developers/coding-style/cocoa-dos-and-donts', 51 'http://dev.chromium.org/developers/coding-style/cocoa-dos-and-donts',
47 ), 52 ),
48 False, 53 False,
49 ), 54 ),
(...skipping 443 matching lines...) Expand 10 before | Expand all | Expand 10 after
493 if pattern.match(line): 498 if pattern.match(line):
494 errors.append(' %s:%d' % (f.LocalPath(), line_num)) 499 errors.append(' %s:%d' % (f.LocalPath(), line_num))
495 500
496 results = [] 501 results = []
497 if errors: 502 if errors:
498 results.append(output_api.PresubmitError( 503 results.append(output_api.PresubmitError(
499 'Header files should not include ui/aura/window_property.h', errors)) 504 'Header files should not include ui/aura/window_property.h', errors))
500 return results 505 return results
501 506
502 507
508 def _CheckIncludeOrderForScope(scope, input_api, file_path, changed_linenums):
509 """Checks that the lines in scope occur in the right order.
510
511 1. C system files in alphabetical order
512 2. C++ system files in alphabetical order
513 3. Project's .h files
514 """
515
516 c_system_include_pattern = input_api.re.compile(r'\s*#include <.*\.h>')
517 cpp_system_include_pattern = input_api.re.compile(r'\s*#include <.*>')
518 custom_include_pattern = input_api.re.compile(r'\s*#include ".*')
519
520 C_SYSTEM_INCLUDES, CPP_SYSTEM_INCLUDES, CUSTOM_INCLUDES = range(3)
521
522 state = C_SYSTEM_INCLUDES
523
524 previous_line = ''
525 problem_linenums = []
526 for line_num, line in scope:
527 if c_system_include_pattern.match(line):
528 if state != C_SYSTEM_INCLUDES:
529 problem_linenums.append(line_num)
530 elif previous_line and previous_line > line:
531 problem_linenums.append(line_num)
532 elif cpp_system_include_pattern.match(line):
533 if state == C_SYSTEM_INCLUDES:
534 state = CPP_SYSTEM_INCLUDES
535 elif state == CUSTOM_INCLUDES:
536 problem_linenums.append(line_num)
537 elif previous_line and previous_line > line:
538 problem_linenums.append(line_num)
539 elif custom_include_pattern.match(line):
540 if state != CUSTOM_INCLUDES:
541 state = CUSTOM_INCLUDES
542 elif previous_line and previous_line > line:
543 problem_linenums.append(line_num)
544 else:
545 problem_linenums.append(line_num)
546 previous_line = line
547
548 warnings = []
549 for line_num in problem_linenums:
550 if line_num in changed_linenums or line_num - 1 in changed_linenums:
551 warnings.append(' %s:%d' % (file_path, line_num))
552 return warnings
553
554
555 def _CheckIncludeOrderInFile(input_api, output_api, f, is_source,
556 changed_linenums):
557 """Checks the #include order for the given file f."""
558
559 include_pattern = input_api.re.compile(r'\s*#include.*')
560 if_pattern = input_api.re.compile(r'\s*#if.*')
561 endif_pattern = input_api.re.compile(r'\s*#endif.*')
562
563 contents = f.NewContents()
564 warnings = []
565 line_num = 0
566
567 # Handle the special first include for source files.
568 if is_source:
569 for line in contents:
570 line_num += 1
571 if include_pattern.match(line):
572 expected = '#include "%s"' % f.LocalPath().replace('.cc', '.h')
573 if line != expected:
574 # Maybe there was no special first include, and that's fine. Process
575 # the line again along with the normal includes.
576 line_num -= 1
577 break
578
579 # Split into scopes: Each region between #if and #endif is its own scope.
580 scopes = []
581 current_scope = []
582 for line in contents[line_num:]:
583 line_num += 1
584 if if_pattern.match(line) or endif_pattern.match(line):
585 scopes.append(current_scope)
586 current_scope = []
587 elif include_pattern.match(line):
588 current_scope.append((line_num, line))
589 scopes.append(current_scope)
590
591 for scope in scopes:
592 warnings.extend(_CheckIncludeOrderForScope(scope, input_api, f.LocalPath(),
593 changed_linenums))
594 return warnings
595
596
597 def _CheckIncludeOrder(input_api, output_api):
598 """Checks that the #include order is correct.
599
600 1. The corresponding header for source files.
601 2. C system files in alphabetical order
602 3. C++ system files in alphabetical order
603 4. Project's .h files in alphabetical order
604
605 Each region between #if and #endif follows these rules separately.
606 """
607
608 warnings = []
609 for f in input_api.AffectedFiles():
610 changed_linenums = set([line_num for line_num, _ in f.ChangedContents()])
611 if f.LocalPath().endswith('.cc'):
612 warnings = _CheckIncludeOrderInFile(input_api, output_api, f, True,
613 changed_linenums)
614 elif f.LocalPath().endswith('.h'):
615 warnings = _CheckIncludeOrderInFile(input_api, output_api, f, False,
616 changed_linenums)
617
618 results = []
619 if warnings:
620 results.append(output_api.PresubmitPromptWarning(_INCLUDE_ORDER_WARNING,
621 warnings))
622 return results
623
624
503 def _CommonChecks(input_api, output_api): 625 def _CommonChecks(input_api, output_api):
504 """Checks common to both upload and commit.""" 626 """Checks common to both upload and commit."""
505 results = [] 627 results = []
506 results.extend(input_api.canned_checks.PanProjectChecks( 628 results.extend(input_api.canned_checks.PanProjectChecks(
507 input_api, output_api, excluded_paths=_EXCLUDED_PATHS)) 629 input_api, output_api, excluded_paths=_EXCLUDED_PATHS))
508 results.extend(_CheckAuthorizedAuthor(input_api, output_api)) 630 results.extend(_CheckAuthorizedAuthor(input_api, output_api))
509 results.extend( 631 results.extend(
510 _CheckNoProductionCodeUsingTestOnlyFunctions(input_api, output_api)) 632 _CheckNoProductionCodeUsingTestOnlyFunctions(input_api, output_api))
511 results.extend(_CheckNoIOStreamInHeaders(input_api, output_api)) 633 results.extend(_CheckNoIOStreamInHeaders(input_api, output_api))
512 results.extend(_CheckNoUNIT_TESTInSourceFiles(input_api, output_api)) 634 results.extend(_CheckNoUNIT_TESTInSourceFiles(input_api, output_api))
513 results.extend(_CheckNoNewWStrings(input_api, output_api)) 635 results.extend(_CheckNoNewWStrings(input_api, output_api))
514 results.extend(_CheckNoDEPSGIT(input_api, output_api)) 636 results.extend(_CheckNoDEPSGIT(input_api, output_api))
515 results.extend(_CheckNoBannedFunctions(input_api, output_api)) 637 results.extend(_CheckNoBannedFunctions(input_api, output_api))
516 results.extend(_CheckNoPragmaOnce(input_api, output_api)) 638 results.extend(_CheckNoPragmaOnce(input_api, output_api))
517 results.extend(_CheckNoTrinaryTrueFalse(input_api, output_api)) 639 results.extend(_CheckNoTrinaryTrueFalse(input_api, output_api))
tfarina 2012/11/13 19:22:42 unrelated: s/Trinary/Ternary, no?
518 results.extend(_CheckUnwantedDependencies(input_api, output_api)) 640 results.extend(_CheckUnwantedDependencies(input_api, output_api))
519 results.extend(_CheckFilePermissions(input_api, output_api)) 641 results.extend(_CheckFilePermissions(input_api, output_api))
520 results.extend(_CheckNoAuraWindowPropertyHInHeaders(input_api, output_api)) 642 results.extend(_CheckNoAuraWindowPropertyHInHeaders(input_api, output_api))
643 results.extend(_CheckIncludeOrder(input_api, output_api))
521 return results 644 return results
522 645
523 646
524 def _CheckSubversionConfig(input_api, output_api): 647 def _CheckSubversionConfig(input_api, output_api):
525 """Verifies the subversion config file is correctly setup. 648 """Verifies the subversion config file is correctly setup.
526 649
527 Checks that autoprops are enabled, returns an error otherwise. 650 Checks that autoprops are enabled, returns an error otherwise.
528 """ 651 """
529 join = input_api.os_path.join 652 join = input_api.os_path.join
530 if input_api.platform == 'win32': 653 if input_api.platform == 'win32':
(...skipping 127 matching lines...) Expand 10 before | Expand all | Expand 10 after
658 781
659 # Match things like path/aura/file.cc and path/file_aura.cc. 782 # Match things like path/aura/file.cc and path/file_aura.cc.
660 # Same for ash and chromeos. 783 # Same for ash and chromeos.
661 if any(re.search('[/_](ash|aura)', f) for f in files): 784 if any(re.search('[/_](ash|aura)', f) for f in files):
662 trybots += ['linux_chromeos_clang:compile', 'win_aura', 785 trybots += ['linux_chromeos_clang:compile', 'win_aura',
663 'linux_chromeos_asan'] 786 'linux_chromeos_asan']
664 elif any(re.search('[/_]chromeos', f) for f in files): 787 elif any(re.search('[/_]chromeos', f) for f in files):
665 trybots += ['linux_chromeos_clang:compile', 'linux_chromeos_asan'] 788 trybots += ['linux_chromeos_clang:compile', 'linux_chromeos_asan']
666 789
667 return trybots 790 return trybots
OLDNEW
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698