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