OLD | NEW |
| (Empty) |
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 """Presubmit script for Chromium JS resources. | |
6 | |
7 See chrome/browser/resources/PRESUBMIT.py | |
8 """ | |
9 | |
10 class JSChecker(object): | |
11 def __init__(self, input_api, output_api, file_filter=None): | |
12 self.input_api = input_api | |
13 self.output_api = output_api | |
14 self.file_filter = file_filter | |
15 | |
16 def RegexCheck(self, line_number, line, regex, message): | |
17 """Searches for |regex| in |line| to check for a particular style | |
18 violation, returning a message like the one below if the regex matches. | |
19 The |regex| must have exactly one capturing group so that the relevant | |
20 part of |line| can be highlighted. If more groups are needed, use | |
21 "(?:...)" to make a non-capturing group. Sample message: | |
22 | |
23 line 6: Use var instead of const. | |
24 const foo = bar(); | |
25 ^^^^^ | |
26 """ | |
27 match = self.input_api.re.search(regex, line) | |
28 if match: | |
29 assert len(match.groups()) == 1 | |
30 start = match.start(1) | |
31 length = match.end(1) - start | |
32 return ' line %d: %s\n%s\n%s' % ( | |
33 line_number, | |
34 message, | |
35 line, | |
36 self.error_highlight(start, length)) | |
37 return '' | |
38 | |
39 def ChromeSendCheck(self, i, line): | |
40 """Checks for a particular misuse of 'chrome.send'.""" | |
41 return self.RegexCheck(i, line, r"chrome\.send\('[^']+'\s*(, \[\])\)", | |
42 'Passing an empty array to chrome.send is unnecessary') | |
43 | |
44 def ConstCheck(self, i, line): | |
45 """Check for use of the 'const' keyword.""" | |
46 if self.input_api.re.search(r'\*\s+@const', line): | |
47 # Probably a JsDoc line | |
48 return '' | |
49 | |
50 return self.RegexCheck(i, line, r'(?:^|\s|\()(const)\s', | |
51 'Use /** @const */ var varName; instead of const varName;') | |
52 | |
53 def EndJsDocCommentCheck(self, i, line): | |
54 msg = 'End JSDoc comments with */ instead of **/' | |
55 def _check(regex): | |
56 return self.RegexCheck(i, line, regex, msg) | |
57 return _check(r'^\s*(\*\*/)\s*$') or _check(r'/\*\* @[a-zA-Z]+.* (\*\*/)') | |
58 | |
59 def GetElementByIdCheck(self, i, line): | |
60 """Checks for use of 'document.getElementById' instead of '$'.""" | |
61 return self.RegexCheck(i, line, r"(document\.getElementById)\('", | |
62 "Use $('id'), from chrome://resources/js/util.js, instead of " | |
63 "document.getElementById('id')") | |
64 | |
65 def InheritDocCheck(self, i, line): | |
66 """Checks for use of '@inheritDoc' instead of '@override'.""" | |
67 return self.RegexCheck(i, line, r"\* (@inheritDoc)", | |
68 "@inheritDoc is deprecated, use @override instead") | |
69 | |
70 def WrapperTypeCheck(self, i, line): | |
71 """Check for wrappers (new String()) instead of builtins (string).""" | |
72 return self.RegexCheck(i, line, | |
73 r"(?:/\*)?\*.*?@(?:param|return|type) ?" # /** @param/@return/@type | |
74 r"{[^}]*\b(String|Boolean|Number)\b[^}]*}", # {(Boolean|Number|String)} | |
75 "Don't use wrapper types (i.e. new String() or @type {String})") | |
76 | |
77 def VarNameCheck(self, i, line): | |
78 """See the style guide. http://goo.gl/uKir6""" | |
79 return self.RegexCheck(i, line, | |
80 r"var (?!g_\w+)([a-z]*[_$][\w_$]*)(?<! \$)", | |
81 "Please use var namesLikeThis <http://goo.gl/uKir6>") | |
82 | |
83 def error_highlight(self, start, length): | |
84 """Takes a start position and a length, and produces a row of '^'s to | |
85 highlight the corresponding part of a string. | |
86 """ | |
87 return start * ' ' + length * '^' | |
88 | |
89 def _makeErrorOrWarning(self, error_text, filename): | |
90 """Takes a few lines of text indicating a style violation and turns it into | |
91 a PresubmitError (if |filename| is in a directory where we've already | |
92 taken out all the style guide violations) or a PresubmitPromptWarning | |
93 (if it's in a directory where we haven't done that yet). | |
94 """ | |
95 # TODO(tbreisacher): Once we've cleaned up the style nits in all of | |
96 # resources/ we can get rid of this function. | |
97 path = self.input_api.os_path | |
98 resources = self.input_api.PresubmitLocalPath() | |
99 dirs = ( | |
100 path.join(resources, 'bookmark_manager'), | |
101 path.join(resources, 'extensions'), | |
102 path.join(resources, 'file_manager'), | |
103 path.join(resources, 'help'), | |
104 path.join(resources, 'history'), | |
105 path.join(resources, 'memory_internals'), | |
106 path.join(resources, 'net_export'), | |
107 path.join(resources, 'net_internals'), | |
108 path.join(resources, 'network_action_predictor'), | |
109 path.join(resources, 'ntp4'), | |
110 path.join(resources, 'options'), | |
111 path.join(resources, 'password_manager_internals'), | |
112 path.join(resources, 'print_preview'), | |
113 path.join(resources, 'profiler'), | |
114 path.join(resources, 'sync_promo'), | |
115 path.join(resources, 'tracing'), | |
116 path.join(resources, 'uber'), | |
117 ) | |
118 if filename.startswith(dirs): | |
119 return self.output_api.PresubmitError(error_text) | |
120 else: | |
121 return self.output_api.PresubmitPromptWarning(error_text) | |
122 | |
123 def RunChecks(self): | |
124 """Check for violations of the Chromium JavaScript style guide. See | |
125 http://chromium.org/developers/web-development-style-guide#TOC-JavaScript | |
126 """ | |
127 | |
128 import sys | |
129 import warnings | |
130 old_path = sys.path | |
131 old_filters = warnings.filters | |
132 | |
133 try: | |
134 closure_linter_path = self.input_api.os_path.join( | |
135 self.input_api.change.RepositoryRoot(), | |
136 "third_party", | |
137 "closure_linter") | |
138 gflags_path = self.input_api.os_path.join( | |
139 self.input_api.change.RepositoryRoot(), | |
140 "third_party", | |
141 "python_gflags") | |
142 | |
143 sys.path.insert(0, closure_linter_path) | |
144 sys.path.insert(0, gflags_path) | |
145 | |
146 warnings.filterwarnings('ignore', category=DeprecationWarning) | |
147 | |
148 from closure_linter import checker, errors | |
149 from closure_linter.common import errorhandler | |
150 | |
151 finally: | |
152 sys.path = old_path | |
153 warnings.filters = old_filters | |
154 | |
155 class ErrorHandlerImpl(errorhandler.ErrorHandler): | |
156 """Filters out errors that don't apply to Chromium JavaScript code.""" | |
157 | |
158 def __init__(self, re): | |
159 self._errors = [] | |
160 self.re = re | |
161 | |
162 def HandleFile(self, filename, first_token): | |
163 self._filename = filename | |
164 | |
165 def HandleError(self, error): | |
166 if (self._valid(error)): | |
167 error.filename = self._filename | |
168 self._errors.append(error) | |
169 | |
170 def GetErrors(self): | |
171 return self._errors | |
172 | |
173 def HasErrors(self): | |
174 return bool(self._errors) | |
175 | |
176 def _valid(self, error): | |
177 """Check whether an error is valid. Most errors are valid, with a few | |
178 exceptions which are listed here. | |
179 """ | |
180 | |
181 is_grit_statement = bool( | |
182 self.re.search("</?(include|if)", error.token.line)) | |
183 | |
184 # Ignore missing spaces before "(" until Promise#catch issue is solved. | |
185 # http://crbug.com/338301 | |
186 if (error.code == errors.MISSING_SPACE and error.token.string == '(' and | |
187 'catch(' in error.token.line): | |
188 return False | |
189 | |
190 return not is_grit_statement and error.code not in [ | |
191 errors.COMMA_AT_END_OF_LITERAL, | |
192 errors.JSDOC_ILLEGAL_QUESTION_WITH_PIPE, | |
193 errors.JSDOC_TAG_DESCRIPTION_ENDS_WITH_INVALID_CHARACTER, | |
194 errors.LINE_TOO_LONG, | |
195 errors.MISSING_JSDOC_TAG_THIS, | |
196 ] | |
197 | |
198 results = [] | |
199 | |
200 affected_files = self.input_api.change.AffectedFiles( | |
201 file_filter=self.file_filter, | |
202 include_deletes=False) | |
203 affected_js_files = filter(lambda f: f.LocalPath().endswith('.js'), | |
204 affected_files) | |
205 for f in affected_js_files: | |
206 error_lines = [] | |
207 | |
208 # Check for the following: | |
209 # * document.getElementById() | |
210 # * the 'const' keyword | |
211 # * Passing an empty array to 'chrome.send()' | |
212 for i, line in enumerate(f.NewContents(), start=1): | |
213 error_lines += filter(None, [ | |
214 self.ChromeSendCheck(i, line), | |
215 self.ConstCheck(i, line), | |
216 self.GetElementByIdCheck(i, line), | |
217 self.InheritDocCheck(i, line), | |
218 self.WrapperTypeCheck(i, line), | |
219 self.VarNameCheck(i, line), | |
220 ]) | |
221 | |
222 # Use closure_linter to check for several different errors | |
223 error_handler = ErrorHandlerImpl(self.input_api.re) | |
224 js_checker = checker.JavaScriptStyleChecker(error_handler) | |
225 js_checker.Check(self.input_api.os_path.join( | |
226 self.input_api.change.RepositoryRoot(), | |
227 f.LocalPath())) | |
228 | |
229 for error in error_handler.GetErrors(): | |
230 highlight = self.error_highlight( | |
231 error.token.start_index, error.token.length) | |
232 error_msg = ' line %d: E%04d: %s\n%s\n%s' % ( | |
233 error.token.line_number, | |
234 error.code, | |
235 error.message, | |
236 error.token.line.rstrip(), | |
237 highlight) | |
238 error_lines.append(error_msg) | |
239 | |
240 if error_lines: | |
241 error_lines = [ | |
242 'Found JavaScript style violations in %s:' % | |
243 f.LocalPath()] + error_lines | |
244 results.append(self._makeErrorOrWarning( | |
245 '\n'.join(error_lines), f.AbsoluteLocalPath())) | |
246 | |
247 if results: | |
248 results.append(self.output_api.PresubmitNotifyResult( | |
249 'See the JavaScript style guide at ' | |
250 'http://www.chromium.org/developers/web-development-style-guide' | |
251 '#TOC-JavaScript and if you have any feedback about the JavaScript ' | |
252 'PRESUBMIT check, contact tbreisacher@chromium.org or ' | |
253 'dbeam@chromium.org')) | |
254 | |
255 return results | |
OLD | NEW |