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

Side by Side Diff: chrome/browser/resources/web_dev_style/js_checker.py

Issue 253543002: web_dev_style: check webui browser tests as well. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: merge Created 6 years, 8 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
OLDNEW
(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
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698