Chromium Code Reviews| OLD | NEW |
|---|---|
| 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 """Presubmit script for Chromium WebUI resources. | 5 """Presubmit script for Chromium WebUI resources. |
| 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/git cl, and see | 8 for more details about the presubmit API built into gcl/git cl, and see |
| 9 http://www.chromium.org/developers/web-development-style-guide for the rules | 9 http://www.chromium.org/developers/web-development-style-guide for the rules |
| 10 we're checking against here. | 10 we're checking against here. |
| 11 """ | 11 """ |
| 12 | 12 |
| 13 # TODO(dbeam): Real CSS parser? pycss? http://code.google.com/p/pycss/ | 13 # TODO(dbeam): Real CSS parser? https://github.com/danbeam/css-py/tree/css3 |
| 14 | 14 |
| 15 class CSSChecker(object): | 15 class CSSChecker(object): |
| 16 def __init__(self, input_api, output_api, file_filter=None): | 16 def __init__(self, input_api, output_api, file_filter=None): |
| 17 self.input_api = input_api | 17 self.input_api = input_api |
| 18 self.output_api = output_api | 18 self.output_api = output_api |
| 19 self.file_filter = file_filter | 19 self.file_filter = file_filter |
| 20 | 20 |
| 21 def RunChecks(self): | 21 def RunChecks(self): |
| 22 # We use this a lot, so make a nick name variable. | 22 # We use this a lot, so make a nick name variable. |
| 23 re = self.input_api.re | 23 re = self.input_api.re |
| (...skipping 17 matching lines...) Expand all Loading... | |
| 41 grit_reg = r'<if[^>]+>.*?<\s*/\s*if[^>]*>|<include[^>]+>' | 41 grit_reg = r'<if[^>]+>.*?<\s*/\s*if[^>]*>|<include[^>]+>' |
| 42 return re.sub(re.compile(grit_reg, re.DOTALL), '', s) | 42 return re.sub(re.compile(grit_reg, re.DOTALL), '', s) |
| 43 | 43 |
| 44 def _rgb_from_hex(s): | 44 def _rgb_from_hex(s): |
| 45 if len(s) == 3: | 45 if len(s) == 3: |
| 46 r, g, b = s[0] + s[0], s[1] + s[1], s[2] + s[2] | 46 r, g, b = s[0] + s[0], s[1] + s[1], s[2] + s[2] |
| 47 else: | 47 else: |
| 48 r, g, b = s[0:2], s[2:4], s[4:6] | 48 r, g, b = s[0:2], s[2:4], s[4:6] |
| 49 return int(r, base=16), int(g, base=16), int(b, base=16) | 49 return int(r, base=16), int(g, base=16), int(b, base=16) |
| 50 | 50 |
| 51 def _strip_prefix(s): | |
| 52 return re.sub(r'^-(?:o|ms|moz|khtml|webkit)-', '', s) | |
| 53 | |
| 51 def alphabetize_props(contents): | 54 def alphabetize_props(contents): |
| 52 errors = [] | 55 errors = [] |
| 53 for rule in re.finditer(r'{(.*?)}', contents, re.DOTALL): | 56 for rule in re.finditer(r'{(.*?)}', contents, re.DOTALL): |
| 54 semis = map(lambda t: t.strip(), rule.group(1).split(';'))[:-1] | 57 semis = map(lambda t: t.strip(), rule.group(1).split(';'))[:-1] |
| 55 rules = filter(lambda r: ': ' in r, semis) | 58 rules = filter(lambda r: ': ' in r, semis) |
| 56 props = map(lambda r: r[0:r.find(':')], rules) | 59 props = map(lambda r: r[0:r.find(':')], rules) |
| 57 if props != sorted(props): | 60 if props != sorted(props): |
| 58 errors.append(' %s;\n' % (';\n '.join(rules))) | 61 errors.append(' %s;\n' % (';\n '.join(rules))) |
| 59 return errors | 62 return errors |
| 60 | 63 |
| (...skipping 29 matching lines...) Expand all Loading... | |
| 90 small_seconds = r'(?:^|[^_a-zA-Z0-9-])(0?\.[0-9]+)s(?!-?[_a-zA-Z0-9-])' | 93 small_seconds = r'(?:^|[^_a-zA-Z0-9-])(0?\.[0-9]+)s(?!-?[_a-zA-Z0-9-])' |
| 91 def milliseconds_for_small_times(line): | 94 def milliseconds_for_small_times(line): |
| 92 return re.search(small_seconds, line) | 95 return re.search(small_seconds, line) |
| 93 | 96 |
| 94 def no_data_uris_in_source_files(line): | 97 def no_data_uris_in_source_files(line): |
| 95 return re.search(r'\(\s*\'?\s*data:', line) | 98 return re.search(r'\(\s*\'?\s*data:', line) |
| 96 | 99 |
| 97 def one_rule_per_line(line): | 100 def one_rule_per_line(line): |
| 98 return re.search(r'[_a-zA-Z0-9-](?<!data):(?!//)[^;]+;\s*[^ }]\s*', line) | 101 return re.search(r'[_a-zA-Z0-9-](?<!data):(?!//)[^;]+;\s*[^ }]\s*', line) |
| 99 | 102 |
| 103 pseudo_elements = ['after', | |
|
Tyler Breisacher (Chromium)
2012/12/05 18:59:57
Presumably more things will be added to this list
Dan Beam
2012/12/05 20:36:29
Very few are actually official (just before, after
| |
| 104 'before', | |
| 105 'calendar-picker-indicator', | |
| 106 'color-swatch', | |
| 107 'color-swatch-wrapper', | |
| 108 'date-and-time-container', | |
| 109 'date-and-time-value', | |
| 110 'datetime-edit', | |
| 111 'datetime-edit-ampm-field', | |
| 112 'datetime-edit-day-field', | |
| 113 'datetime-edit-hour-field', | |
| 114 'datetime-edit-millisecond-field', | |
| 115 'datetime-edit-minute-field', | |
| 116 'datetime-edit-month-field', | |
| 117 'datetime-edit-second-field', | |
| 118 'datetime-edit-text', | |
| 119 'datetime-edit-week-field', | |
| 120 'datetime-edit-year-field', | |
| 121 'details-marker', | |
| 122 'file-upload-button', | |
| 123 'first-letter', | |
| 124 'first-line', | |
| 125 'inner-spin-button', | |
| 126 'input-placeholder', | |
| 127 'input-speech-button', | |
| 128 'keygen-select', | |
| 129 'media-slider-container', | |
| 130 'media-slider-thumb', | |
| 131 'meter-bar', | |
| 132 'meter-even-less-good-value', | |
| 133 'meter-inner-element', | |
| 134 'meter-optimum-value', | |
| 135 'meter-suboptimum-value', | |
| 136 'progress-bar', | |
| 137 'progress-inner-element', | |
| 138 'progress-value', | |
| 139 'resizer', | |
| 140 'scrollbar', | |
| 141 'scrollbar-button', | |
| 142 'scrollbar-corner', | |
| 143 'scrollbar-thumb', | |
| 144 'scrollbar-track', | |
| 145 'scrollbar-track-piece', | |
| 146 'search-cancel-button', | |
| 147 'search-decoration', | |
| 148 'search-results-button', | |
| 149 'search-results-decoration', | |
| 150 'selection', | |
| 151 'slider-container', | |
| 152 'slider-runnable-track', | |
| 153 'slider-thumb', | |
| 154 'textfield-decoration-container', | |
| 155 'validation-bubble', | |
| 156 'validation-bubble-arrow', | |
| 157 'validation-bubble-arrow-clipper', | |
| 158 'validation-bubble-heading', | |
| 159 'validation-bubble-message', | |
| 160 'validation-bubble-text-block'] | |
| 161 pseudo_reg = r'(?<!:):([a-z-]+)(?=[^{}]+?{)' | |
|
Tyler Breisacher (Chromium)
2012/12/05 18:59:57
Please consider using Python's verbose regular exp
| |
| 162 def pseudo_elements_double_colon(contents): | |
| 163 errors = [] | |
| 164 for p in re.finditer(re.compile(pseudo_reg, re.MULTILINE), contents): | |
|
Tyler Breisacher (Chromium)
2012/12/05 20:45:14
optional nit: Do the compilation when you first wr
| |
| 165 pseudo = p.group(1).strip().splitlines()[0] | |
| 166 if _strip_prefix(pseudo) in pseudo_elements: | |
| 167 errors.append(' :%s (should be ::%s)' % (pseudo, pseudo)) | |
| 168 return errors | |
| 169 | |
| 100 any_reg = re.compile(r':(?:-webkit-)?any\(.*?\)', re.DOTALL) | 170 any_reg = re.compile(r':(?:-webkit-)?any\(.*?\)', re.DOTALL) |
| 101 multi_sels = re.compile(r'(?:}[\n\s]*)?([^,]+,(?=[^{}]+?{).*[,{])\s*$', | 171 multi_sels = re.compile(r'(?:}[\n\s]*)?([^,]+,(?=[^{}]+?{).*[,{])\s*$', |
| 102 re.MULTILINE) | 172 re.MULTILINE) |
| 103 def one_selector_per_line(contents): | 173 def one_selector_per_line(contents): |
| 104 errors = [] | 174 errors = [] |
| 105 for b in re.finditer(multi_sels, re.sub(any_reg, '', contents)): | 175 for b in re.finditer(multi_sels, re.sub(any_reg, '', contents)): |
| 106 errors.append(' ' + b.group(1).strip().splitlines()[-1:][0]) | 176 errors.append(' ' + b.group(1).strip().splitlines()[-1:][0]) |
| 107 return errors | 177 return errors |
| 108 | 178 |
| 109 def rgb_if_not_gray(line): | 179 def rgb_if_not_gray(line): |
| (...skipping 18 matching lines...) Expand all Loading... | |
| 128 r'(?:\.0|0(?:\.0?|px|em|%|in|cm|mm|pc|pt|ex|deg|g?rad|m?s|k?hz))' | 198 r'(?:\.0|0(?:\.0?|px|em|%|in|cm|mm|pc|pt|ex|deg|g?rad|m?s|k?hz))' |
| 129 r'(?:\D|$)(?=[^{}]+?}).*$') | 199 r'(?:\D|$)(?=[^{}]+?}).*$') |
| 130 def zero_length_values(contents): | 200 def zero_length_values(contents): |
| 131 errors = [] | 201 errors = [] |
| 132 for z in re.finditer(re.compile(zeros, re.MULTILINE), contents): | 202 for z in re.finditer(re.compile(zeros, re.MULTILINE), contents): |
| 133 first_line = z.group(0).strip().splitlines()[0] | 203 first_line = z.group(0).strip().splitlines()[0] |
| 134 if not re.search(hsl, first_line): | 204 if not re.search(hsl, first_line): |
| 135 errors.append(' ' + first_line) | 205 errors.append(' ' + first_line) |
| 136 return errors | 206 return errors |
| 137 | 207 |
| 208 # NOTE: Currently multi-line checks don't support 'after'. Instead, add | |
| 209 # suggestions while parsing the file so another pass isn't necessary. | |
| 138 added_or_modified_files_checks = [ | 210 added_or_modified_files_checks = [ |
| 139 { 'desc': 'Alphabetize properties and list vendor specific (i.e. ' | 211 { 'desc': 'Alphabetize properties and list vendor specific (i.e. ' |
| 140 '-webkit) above standard.', | 212 '-webkit) above standard.', |
| 141 'test': alphabetize_props, | 213 'test': alphabetize_props, |
| 142 'multiline': True, | 214 'multiline': True, |
| 143 }, | 215 }, |
| 144 { 'desc': 'Start braces ({) end a selector, have a space before them ' | 216 { 'desc': 'Start braces ({) end a selector, have a space before them ' |
| 145 'and no rules after.', | 217 'and no rules after.', |
| 146 'test': braces_have_space_before_and_nothing_after, | 218 'test': braces_have_space_before_and_nothing_after, |
| 147 }, | 219 }, |
| (...skipping 11 matching lines...) Expand all Loading... | |
| 159 'test': favor_single_quotes, | 231 'test': favor_single_quotes, |
| 160 }, | 232 }, |
| 161 { 'desc': 'Use abbreviated hex (#rgb) when in form #rrggbb.', | 233 { 'desc': 'Use abbreviated hex (#rgb) when in form #rrggbb.', |
| 162 'test': hex_could_be_shorter, | 234 'test': hex_could_be_shorter, |
| 163 'after': suggest_short_hex, | 235 'after': suggest_short_hex, |
| 164 }, | 236 }, |
| 165 { 'desc': 'Use milliseconds for time measurements under 1 second.', | 237 { 'desc': 'Use milliseconds for time measurements under 1 second.', |
| 166 'test': milliseconds_for_small_times, | 238 'test': milliseconds_for_small_times, |
| 167 'after': suggest_ms_from_s, | 239 'after': suggest_ms_from_s, |
| 168 }, | 240 }, |
| 169 { 'desc': 'Don\'t use data URIs in source files. Use grit instead.', | 241 { 'desc': "Don't use data URIs in source files. Use grit instead.", |
| 170 'test': no_data_uris_in_source_files, | 242 'test': no_data_uris_in_source_files, |
| 171 }, | 243 }, |
| 172 { 'desc': 'One rule per line (what not to do: color: red; margin: 0;).', | 244 { 'desc': 'One rule per line (what not to do: color: red; margin: 0;).', |
| 173 'test': one_rule_per_line, | 245 'test': one_rule_per_line, |
| 174 }, | 246 }, |
| 175 { 'desc': 'One selector per line (what not to do: a, b {}).', | 247 { 'desc': 'One selector per line (what not to do: a, b {}).', |
| 176 'test': one_selector_per_line, | 248 'test': one_selector_per_line, |
| 177 'multiline': True, | 249 'multiline': True, |
| 178 }, | 250 }, |
| 251 { 'desc': 'Pseudo-elements should use double colon (i.e. ::after).', | |
| 252 'test': pseudo_elements_double_colon, | |
| 253 'multiline': True, | |
| 254 }, | |
| 179 { 'desc': 'Use rgb() over #hex when not a shade of gray (like #333).', | 255 { 'desc': 'Use rgb() over #hex when not a shade of gray (like #333).', |
| 180 'test': rgb_if_not_gray, | 256 'test': rgb_if_not_gray, |
| 181 'after': suggest_rgb_from_hex, | 257 'after': suggest_rgb_from_hex, |
| 182 }, | 258 }, |
| 183 { 'desc': 'Make all zero length terms (i.e. 0px) 0 unless inside of ' | 259 { 'desc': 'Make all zero length terms (i.e. 0px) 0 unless inside of ' |
| 184 'hsl() or part of @keyframe.', | 260 'hsl() or part of @keyframe.', |
| 185 'test': zero_length_values, | 261 'test': zero_length_values, |
| 186 'multiline': True, | 262 'multiline': True, |
| 187 }, | 263 }, |
| 188 ] | 264 ] |
| 189 | 265 |
| 190 results = [] | 266 results = [] |
| 191 affected_files = self.input_api.AffectedFiles(include_deletes=False, | 267 affected_files = self.input_api.AffectedFiles(include_deletes=False, |
| 192 file_filter=self.file_filter) | 268 file_filter=self.file_filter) |
| 193 files = [] | 269 files = [] |
| 194 for f in affected_files: | 270 for f in affected_files: |
| 195 # Remove all /*comments*/, @at-keywords, and grit <if|include> tags; we're | 271 # Remove all /*comments*/, @at-keywords, and grit <if|include> tags; we're |
| 196 # not using a real parser. TODO(dbeam): Check alpha in <if> blocks. | 272 # not using a real parser. TODO(dbeam): Check alpha in <if> blocks. |
| 197 file_contents = _remove_all('\n'.join(f.NewContents())) | 273 file_contents = _remove_all('\n'.join(f.NewContents())) |
| 198 files.append((f.LocalPath(), file_contents)) | 274 files.append((f.LocalPath(), file_contents)) |
| 199 | 275 |
| 200 # Only look at CSS files for now. | 276 # Only look at CSS files for now. |
| 201 for f in filter(lambda f: f[0].endswith('.css'), files): | 277 for f in filter(lambda f: f[0].endswith('.css'), files): |
| 202 file_errors = [] | 278 file_errors = [] |
| 203 for check in added_or_modified_files_checks: | 279 for check in added_or_modified_files_checks: |
| 204 # If the check is multiline, it receieves the whole file and gives us | 280 # If the check is multiline, it receieves the whole file and gives us |
| 205 # back a list of things wrong. If the check isn't multiline, we pass it | 281 # back a list of things wrong. If the check isn't multiline, we pass it |
| 206 # each line and the check returns something truthy if there's an issue. | 282 # each line and the check returns something truthy if there's an issue. |
| 207 if ('multiline' in check and check['multiline']): | 283 if ('multiline' in check and check['multiline']): |
| 284 assert not 'after' in check | |
| 208 check_errors = check['test'](f[1]) | 285 check_errors = check['test'](f[1]) |
| 209 if len(check_errors) > 0: | 286 if len(check_errors) > 0: |
| 210 # There are currently no multiline checks with ['after']. | |
| 211 file_errors.append('- %s\n%s' % | 287 file_errors.append('- %s\n%s' % |
| 212 (check['desc'], '\n'.join(check_errors).rstrip())) | 288 (check['desc'], '\n'.join(check_errors).rstrip())) |
| 213 else: | 289 else: |
| 214 check_errors = [] | 290 check_errors = [] |
| 215 lines = f[1].splitlines() | 291 lines = f[1].splitlines() |
| 216 for lnum in range(0, len(lines)): | 292 for lnum in range(0, len(lines)): |
| 217 line = lines[lnum] | 293 line = lines[lnum] |
| 218 if check['test'](line): | 294 if check['test'](line): |
| 219 error = ' ' + line.strip() | 295 error = ' ' + line.strip() |
| 220 if 'after' in check: | 296 if 'after' in check: |
| 221 error += check['after'](line) | 297 error += check['after'](line) |
| 222 check_errors.append(error) | 298 check_errors.append(error) |
| 223 if len(check_errors) > 0: | 299 if len(check_errors) > 0: |
| 224 file_errors.append('- %s\n%s' % | 300 file_errors.append('- %s\n%s' % |
| 225 (check['desc'], '\n'.join(check_errors))) | 301 (check['desc'], '\n'.join(check_errors))) |
| 226 if file_errors: | 302 if file_errors: |
| 227 results.append(self.output_api.PresubmitPromptWarning( | 303 results.append(self.output_api.PresubmitPromptWarning( |
| 228 '%s:\n%s' % (f[0], '\n\n'.join(file_errors)))) | 304 '%s:\n%s' % (f[0], '\n\n'.join(file_errors)))) |
| 229 | 305 |
| 230 if results: | 306 if results: |
| 231 # Add your name if you're here often mucking around in the code. | 307 # Add your name if you're here often mucking around in the code. |
| 232 authors = ['dbeam@chromium.org'] | 308 authors = ['dbeam@chromium.org'] |
| 233 results.append(self.output_api.PresubmitNotifyResult( | 309 results.append(self.output_api.PresubmitNotifyResult( |
| 234 'Was the CSS checker useful? Send feedback or hate mail to %s.' % | 310 'Was the CSS checker useful? Send feedback or hate mail to %s.' % |
| 235 ', '.join(authors))) | 311 ', '.join(authors))) |
| 236 | 312 |
| 237 return results | 313 return results |
| OLD | NEW |