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

Side by Side Diff: presubmit_canned_checks.py

Issue 9323016: [WebUI] Add some presubmit checks (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: fixes Created 8 years, 10 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) 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 """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 import re
M-A Ruel 2012/02/03 16:41:35 No need, input_api.re
Dan Beam 2012/02/04 07:26:22 Ah, OK, will do this in new file.
7 8
8 ### Description checks 9 ### Description checks
9 10
10 def CheckChangeHasTestField(input_api, output_api): 11 def CheckChangeHasTestField(input_api, output_api):
11 """Requires that the changelist have a TEST= field.""" 12 """Requires that the changelist have a TEST= field."""
12 if input_api.change.TEST: 13 if input_api.change.TEST:
13 return [] 14 return []
14 else: 15 else:
15 return [output_api.PresubmitNotifyResult( 16 return [output_api.PresubmitNotifyResult(
16 'If this change requires manual test instructions to QA team, add ' 17 'If this change requires manual test instructions to QA team, add '
(...skipping 831 matching lines...) Expand 10 before | Expand all | Expand 10 after
848 text_files = tuple(text_files or ( 849 text_files = tuple(text_files or (
849 r'.+\.txt$', 850 r'.+\.txt$',
850 r'.+\.json$', 851 r'.+\.json$',
851 )) 852 ))
852 project_name = project_name or 'Chromium' 853 project_name = project_name or 'Chromium'
853 license_header = license_header or ( 854 license_header = license_header or (
854 r'.*? Copyright \(c\) %(year)s The %(project)s Authors\. ' 855 r'.*? Copyright \(c\) %(year)s The %(project)s Authors\. '
855 r'All rights reserved\.\n' 856 r'All rights reserved\.\n'
856 r'.*? Use of this source code is governed by a BSD-style license that ' 857 r'.*? Use of this source code is governed by a BSD-style license that '
857 r'can be\n' 858 r'can be\n'
858 r'.*? found in the LICENSE file\.( \*/)?\n' 859 r'.*? found in the LICENSE file\.(?: \*/)?\n'
859 ) % { 860 ) % {
860 'year': input_api.time.strftime('%Y'), 861 'year': input_api.time.strftime('%Y'),
861 'project': project_name, 862 'project': project_name,
862 } 863 }
863 864
864 results = [] 865 results = []
865 # This code loads the default black list (e.g. third_party, experimental, etc) 866 # This code loads the default black list (e.g. third_party, experimental, etc)
866 # and add our black list (breakpad, skia and v8 are still not following 867 # and add our black list (breakpad, skia and v8 are still not following
867 # google style and are not really living this repository). 868 # google style and are not really living this repository).
868 # See presubmit_support.py InputApi.FilterSourceFile for the (simple) usage. 869 # See presubmit_support.py InputApi.FilterSourceFile for the (simple) usage.
869 black_list = input_api.DEFAULT_BLACK_LIST + excluded_paths 870 black_list = input_api.DEFAULT_BLACK_LIST + excluded_paths
870 white_list = input_api.DEFAULT_WHITE_LIST + text_files 871 white_list = input_api.DEFAULT_WHITE_LIST + text_files
871 sources = lambda x: input_api.FilterSourceFile(x, black_list=black_list) 872 sources = lambda x: input_api.FilterSourceFile(x, black_list=black_list)
872 text_files = lambda x: input_api.FilterSourceFile( 873 text_files = lambda x: input_api.FilterSourceFile(
873 x, black_list=black_list, white_list=white_list) 874 x, black_list=black_list, white_list=white_list)
875 resources = lambda x: input_api.FilterSourceFile(x, black_list=black_list,
Evan Stade 2012/02/03 04:13:15 # TODO(dbeam): add more folders to this list
Dan Beam 2012/02/03 04:29:18 Will do soon, this is kind of a hacky version (sho
876 white_list='.*chrome/browser/resources/(?:options2|ntp4).*\.(?j:cs)s$')
M-A Ruel 2012/02/03 16:41:35 This doesn't belong to canned checks. Then if shou
Dan Beam 2012/02/04 07:26:22 Yeah, I figured this yesterday (that this wasn't t
874 877
875 snapshot_memory = [] 878 snapshot_memory = []
876 def snapshot(msg): 879 def snapshot(msg):
877 """Measures & prints performance warning if a rule is running slow.""" 880 """Measures & prints performance warning if a rule is running slow."""
878 dt2 = input_api.time.clock() 881 dt2 = input_api.time.clock()
879 if snapshot_memory: 882 if snapshot_memory:
880 delta_ms = int(1000*(dt2 - snapshot_memory[0])) 883 delta_ms = int(1000*(dt2 - snapshot_memory[0]))
881 if delta_ms > 500: 884 if delta_ms > 500:
882 print " %s took a long time: %dms" % (snapshot_memory[1], delta_ms) 885 print " %s took a long time: %dms" % (snapshot_memory[1], delta_ms)
883 snapshot_memory[:] = (dt2, msg) 886 snapshot_memory[:] = (dt2, msg)
(...skipping 11 matching lines...) Expand all
895 input_api, output_api, source_file_filter=sources)) 898 input_api, output_api, source_file_filter=sources))
896 snapshot( "checking stray whitespace") 899 snapshot( "checking stray whitespace")
897 results.extend(input_api.canned_checks.CheckChangeHasNoStrayWhitespace( 900 results.extend(input_api.canned_checks.CheckChangeHasNoStrayWhitespace(
898 input_api, output_api, source_file_filter=sources)) 901 input_api, output_api, source_file_filter=sources))
899 snapshot("checking nsobjects") 902 snapshot("checking nsobjects")
900 results.extend(_CheckConstNSObject( 903 results.extend(_CheckConstNSObject(
901 input_api, output_api, source_file_filter=sources)) 904 input_api, output_api, source_file_filter=sources))
902 snapshot("checking singletons") 905 snapshot("checking singletons")
903 results.extend(_CheckSingletonInHeaders( 906 results.extend(_CheckSingletonInHeaders(
904 input_api, output_api, source_file_filter=sources)) 907 input_api, output_api, source_file_filter=sources))
908 snapshot("checking resouces")
Evan Stade 2012/02/03 04:13:15 sp
Dan Beam 2012/02/03 04:29:18 Thank you.
909 results.extend(_CheckWebDevStyleGuide(
910 input_api, output_api, source_file_filter=resources))
905 911
906 # The following checks are only done on commit, since the commit bot will 912 # The following checks are only done on commit, since the commit bot will
907 # auto-fix most of these. 913 # auto-fix most of these.
908 if input_api.is_committing: 914 if input_api.is_committing:
909 snapshot("checking eol style") 915 snapshot("checking eol style")
910 results.extend(input_api.canned_checks.CheckChangeSvnEolStyle( 916 results.extend(input_api.canned_checks.CheckChangeSvnEolStyle(
911 input_api, output_api, source_file_filter=text_files)) 917 input_api, output_api, source_file_filter=text_files))
912 snapshot("checking svn mime types") 918 snapshot("checking svn mime types")
913 results.extend(input_api.canned_checks.CheckSvnForCommonMimeTypes( 919 results.extend(input_api.canned_checks.CheckSvnForCommonMimeTypes(
914 input_api, output_api)) 920 input_api, output_api))
915 snapshot("checking license") 921 snapshot("checking license")
916 results.extend(input_api.canned_checks.CheckLicense( 922 results.extend(input_api.canned_checks.CheckLicense(
917 input_api, output_api, license_header, source_file_filter=sources)) 923 input_api, output_api, license_header, source_file_filter=sources))
918 snapshot("checking was uploaded") 924 snapshot("checking was uploaded")
919 results.extend(input_api.canned_checks.CheckChangeWasUploaded( 925 results.extend(input_api.canned_checks.CheckChangeWasUploaded(
920 input_api, output_api)) 926 input_api, output_api))
921 snapshot("done") 927 snapshot("done")
922 return results 928 return results
929
930 def _CheckWebDevStyleGuide(input_api, output_api, source_file_filter):
M-A Ruel 2012/02/03 16:41:35 why the underscore?
Dan Beam 2012/02/04 07:26:22 It was just to match local style (all the other fu
931 def _collapseable_hex(s):
932 return (len(s) == 6 and s[0] == s[1] and s[2] == s[3] and s[4] == s[5])
933
934 def _is_gray(s):
935 return s[0] == s[1] == s[2] if len(s) == 3 else s[0:2] == s[2:4] == s[4:6]
936
937 def _remove_ats(s):
938 return re.sub(re.compile(r'@\w+.*?{(.*{.*?})+.*?}', re.DOTALL), '\\1', s)
939
940 def _remove_comments(s):
941 return re.sub(re.compile(r'/\*.*?\*/', re.DOTALL), '', s)
942
943 def _rgb_from_hex(s):
944 if len(s) == 3:
945 r, g, b = s[0] + s[0], s[1] + s[1], s[2] + s[2]
946 else:
947 r, g, b = s[0:2], s[2:4], s[4:6]
948 return 'rgb(%d, %d, %d)' % (int(r, base=16),
949 int(g, base=16),
950 int(b, base=16))
951
952 # Shared between hex_could_be_shorter and rgb_if_not_gray.
953 hex_reg = r'#([a-fA-F0-9]{3}|[a-fA-F0-9]{6})(?=[^_a-zA-Z0-9-]|$)(?!.+[{,]$)'
954
955 def alphabetize_props(f):
956 errors = []
957 lines = _remove_ats(_remove_comments(input_api.ReadFile(f, 'rb')))
958 for block in re.finditer(r'{(.*?)}', lines, re.DOTALL):
959 rules = filter(lambda l: l.find(': ') >= 0,
960 map(lambda l: l.strip(), block.group(1).split(';'))[0:-1])
961 if rules != sorted(rules):
962 errors.append('%s:\n %s' % (f, '\n '.join(rules)))
963 return errors
964
965 def braces_and_colons_have_space_after(f):
966 errors = []
967 lines = _remove_comments(input_api.ReadFile(f, 'rb')).splitlines()
968 for l in range(0, len(lines)):
969 if re.search(r'(^\s*{)|(?:{|:(?!\/\/|.*[{,]))\S', lines[l]):
970 errors.append(' %s:%d:%s' % (f, l, lines[l]))
971 return errors
972
973 def classes_use_dashes(f):
974 errors = []
975 lines = _remove_comments(input_api.ReadFile(f, 'rb')).splitlines()
976 for l in range(0, len(lines)):
977 # Intentionally dumbed down version of CSS2.1 grammar for class without
978 # non-ASCII, escape chars, or whitespace.
979 m = re.search(r'(?:^| )\.(-?[_a-zA-Z0-9-]+)', lines[l])
980 if m:
981 c = m.group(1)
982 if (c.lower() != c or c.find('_') >= 0):
983 errors.append(' %s:%d:%s' % (f, l, lines[l]))
984 return errors
985
986 def favor_single_quotes(f):
987 errors = []
988 lines = _remove_comments(input_api.ReadFile(f, 'rb')).splitlines()
989 for l in range(0, len(lines)):
990 if lines[l].find('"') >= 0:
991 errors.append(' %s:%d:%s' % (f, l, lines[l]))
992 return errors
993
994 def hex_could_be_shorter(f):
995 errors = []
996 lines = input_api.ReadFile(f, 'rb').splitlines()
997 for l in range(0, len(lines)):
998 m = re.search(hex_reg, lines[l])
999 if (m and _collapseable_hex(m.group(1))):
1000 errors.append(' %s:%d:%s' % (f, l, lines[l]))
1001 return errors
1002
1003 def one_selector_per_line(f):
1004 errors = []
1005 lines = _remove_ats(_remove_comments(input_api.ReadFile(f, 'rb'))
1006 ).splitlines()
1007 for l in range(0, len(lines)):
1008 if re.search(r'[^,]+,[^,]+{', lines[l]):
1009 errors.append(' %s:%d:%s' % (f, l, lines[l]))
1010 return errors
1011
1012 def rgb_if_not_gray(f):
1013 errors = []
1014 lines = _remove_comments(input_api.ReadFile(f, 'rb')).splitlines()
1015 for l in range(0, len(lines)):
1016 m = re.search(hex_reg, lines[l])
1017 if (m and not _is_gray(m.group(1))):
1018 rgb = _rgb_from_hex(m.group(1))
1019 errors.append(' %s:%d:%s /* = %s */ ' % (f, l, lines[l], rgb))
1020 return errors
1021
1022 def zero_length_values(f):
1023 errors = []
1024 zeros = ''.join((r'[^0-9](?:0?\.)?0',
1025 r'(?:px|em|%|in|cm|mm|pc|pt|ex|deg|g?rad|m?s|k?hz)',
1026 r'(?!\s*\{)'))
1027 hsl = r'hsl\([^\)]*(?:[, ]|(?<=\())(?:0?\.?)?0%'
1028 lines = input_api.ReadFile(f, 'rb').splitlines()
1029 for l in range(0, len(lines)):
1030 if (re.search(zeros, lines[l]) and not re.search(hsl, lines[l])):
1031 errors.append(' %s:%d:%s' % (f, l, lines[l]))
1032 return errors
1033
1034 added_or_modified_files_checks = [
1035 { 'desc': 'Alphabetize properties, but list vendor specific (i.e. -webkit) '
1036 'above standard.',
1037 'test': alphabetize_props
1038 },
1039 { 'desc': 'Braces ({} and colons (:) should have a space after them and no '
1040 'newlines before them.',
1041 'test': braces_and_colons_have_space_after,
1042 },
1043 { 'desc': 'Classes use .dash-form.',
1044 'test': classes_use_dashes,
1045 },
1046 { 'desc': 'Use single quotes (\') instead of double quotes (") in strings.',
1047 'test': favor_single_quotes,
1048 },
1049 { 'desc': 'Favor short hex form (#rgb) when #rrggbb.',
1050 'test': hex_could_be_shorter,
1051 },
1052 { 'desc': 'One selector per line (what not to do: a, b {}).',
1053 'test': one_selector_per_line,
1054 },
1055 { 'desc': 'Use rgb() over #hex when not a shade of gray (like #333).',
1056 'test': rgb_if_not_gray,
1057 },
1058 { 'desc': 'Make all zero length terms (i.e. 0px) 0 unless inside of hsl()'
1059 'or @keyframe.',
1060 'test': zero_length_values,
1061 },
1062 ]
1063
1064 errors = []
1065 files = [f.AbsoluteLocalPath() for f in
1066 input_api.AffectedFiles(include_deletes=False,
1067 file_filter=source_file_filter)]
1068
1069 # Only look at CSS files for now.
1070 for f in filter(lambda l: l.endswith('.css'), files):
1071 for check in added_or_modified_files_checks:
1072 errs = check['test'](f)
1073 if errs:
1074 msg = '%s\n%s' % (check['desc'], '\n'.join(errs))
1075 errors.append(output_api.PresubmitNotifyResult(msg))
1076
1077 return errors
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