OLD | NEW |
1 # Copyright 2014 The Chromium Authors. All rights reserved. | 1 # Copyright 2014 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 """Chromium presubmit script for src/extensions/browser. | 5 """Chromium presubmit script for src/extensions/browser. |
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 on the presubmit API built into gcl. | 8 for more details on the presubmit API built into gcl. |
9 """ | 9 """ |
10 | 10 |
| 11 import sys |
| 12 |
11 def GetPreferredTryMasters(project, change): | 13 def GetPreferredTryMasters(project, change): |
12 return { | 14 return { |
13 'tryserver.chromium': { | 15 'tryserver.chromium': { |
14 'linux_chromium_chromeos_rel': set(['defaulttests']), | 16 'linux_chromium_chromeos_rel': set(['defaulttests']), |
15 } | 17 } |
16 } | 18 } |
17 | 19 |
18 class HistogramValueChecker(object): | 20 def _CreateHistogramValueChecker(input_api, output_api): |
19 """Verify that changes to "extension_function_histogram_value.h" are valid. | 21 original_sys_path = sys.path |
20 | 22 |
21 See comments at the top of the "extension_function_histogram_value.h" file | 23 try: |
22 for what are considered valid changes. There are situations where this script | 24 sys.path.append(input_api.os_path.join( |
23 gives false positive warnings, i.e. it warns even though the edit is | 25 input_api.PresubmitLocalPath(), '..', '..', 'tools', |
24 legitimate. Since the script warns using prompt warnings, the user can always | 26 'strict_enum_value_checker')) |
25 choose to continue. The main point is to attract the attention to all | 27 from strict_enum_value_checker import StrictEnumValueChecker |
26 (potentially or not) invalid edits. | 28 finally: |
| 29 sys.path = original_sys_path |
27 | 30 |
28 """ | 31 return StrictEnumValueChecker(input_api, output_api, |
29 | 32 start_marker='enum HistogramValue {', end_marker=' // Last entry:', |
30 # The name of the file we want to check against | 33 path='extensions/browser/extension_function_histogram_value.h') |
31 LOCAL_PATH = "extensions/browser/extension_function_histogram_value.h" | |
32 | |
33 # The markers we look for in the above source file as delimiters of the enum | |
34 # definition. | |
35 ENUM_START_MARKER = "enum HistogramValue {" | |
36 ENUM_END_MARKER = " // Last entry:" | |
37 | |
38 def __init__(self, input_api, output_api): | |
39 self.input_api = input_api | |
40 self.output_api = output_api | |
41 self.results = [] | |
42 | |
43 class EnumRange(object): | |
44 """Represents a range of line numbers (1-based)""" | |
45 def __init__(self, first_line, last_line): | |
46 self.first_line = first_line | |
47 self.last_line = last_line | |
48 | |
49 def Count(self): | |
50 return self.last_line - self.first_line + 1 | |
51 | |
52 def Contains(self, line_num): | |
53 return self.first_line <= line_num and line_num <= self.last_line | |
54 | |
55 def LogInfo(self, message): | |
56 self.input_api.logging.info(message) | |
57 return | |
58 | |
59 def LogDebug(self, message): | |
60 self.input_api.logging.debug(message) | |
61 return | |
62 | |
63 def ComputeEnumRangeInContents(self, contents): | |
64 """Returns an |EnumRange| object representing the line extent of the | |
65 HistogramValue enum members in |contents|. The line numbers are 1-based, | |
66 compatible with line numbers returned by AffectedFile.ChangeContents(). | |
67 |contents| is a list of strings reprenting the lines of a text file. | |
68 | |
69 If either ENUM_START_MARKER or ENUM_END_MARKER cannot be found in | |
70 |contents|, returns None and emits detailed warnings about the problem. | |
71 | |
72 """ | |
73 first_enum_line = 0 | |
74 last_enum_line = 0 | |
75 line_num = 1 # Line numbers are 1-based | |
76 for line in contents: | |
77 if line.startswith(self.ENUM_START_MARKER): | |
78 first_enum_line = line_num + 1 | |
79 elif line.startswith(self.ENUM_END_MARKER): | |
80 last_enum_line = line_num | |
81 line_num += 1 | |
82 | |
83 if first_enum_line == 0: | |
84 self.EmitWarning("The presubmit script could not find the start of the " | |
85 "enum definition (\"%s\"). Did the enum definition " | |
86 "change?" % self.ENUM_START_MARKER) | |
87 return None | |
88 | |
89 if last_enum_line == 0: | |
90 self.EmitWarning("The presubmit script could not find the end of the " | |
91 "enum definition (\"%s\"). Did the enum definition " | |
92 "change?" % self.ENUM_END_MARKER) | |
93 return None | |
94 | |
95 if first_enum_line >= last_enum_line: | |
96 self.EmitWarning("The presubmit script located the start of the enum " | |
97 "definition (\"%s\" at line %d) *after* its end " | |
98 "(\"%s\" at line %d). Something is not quite right." | |
99 % (self.ENUM_START_MARKER, first_enum_line, | |
100 self.ENUM_END_MARKER, last_enum_line)) | |
101 return None | |
102 | |
103 self.LogInfo("Line extent of |HistogramValue| enum definition: " | |
104 "first_line=%d, last_line=%d." | |
105 % (first_enum_line, last_enum_line)) | |
106 return self.EnumRange(first_enum_line, last_enum_line) | |
107 | |
108 def ComputeEnumRangeInNewFile(self, affected_file): | |
109 return self.ComputeEnumRangeInContents(affected_file.NewContents()) | |
110 | |
111 def GetLongMessage(self): | |
112 return str("The file \"%s\" contains the definition of the " | |
113 "|HistogramValue| enum which should be edited in specific ways " | |
114 "only - *** read the comments at the top of the header file ***" | |
115 ". There are changes to the file that may be incorrect and " | |
116 "warrant manual confirmation after review. Note that this " | |
117 "presubmit script can not reliably report the nature of all " | |
118 "types of invalid changes, especially when the diffs are " | |
119 "complex. For example, an invalid deletion may be reported " | |
120 "whereas the change contains a valid rename." | |
121 % self.LOCAL_PATH) | |
122 | |
123 def EmitWarning(self, message, line_number=None, line_text=None): | |
124 """Emits a presubmit prompt warning containing the short message | |
125 |message|. |item| is |LOCAL_PATH| with optional |line_number| and | |
126 |line_text|. | |
127 | |
128 """ | |
129 if line_number is not None and line_text is not None: | |
130 item = "%s(%d): %s" % (self.LOCAL_PATH, line_number, line_text) | |
131 elif line_number is not None: | |
132 item = "%s(%d)" % (self.LOCAL_PATH, line_number) | |
133 else: | |
134 item = self.LOCAL_PATH | |
135 long_message = self.GetLongMessage() | |
136 self.LogInfo(message) | |
137 self.results.append( | |
138 self.output_api.PresubmitPromptWarning(message, [item], long_message)) | |
139 | |
140 def CollectRangesInsideEnumDefinition(self, affected_file, | |
141 first_line, last_line): | |
142 """Returns a list of triplet (line_start, line_end, line_text) of ranges of | |
143 edits changes. The |line_text| part is the text at line |line_start|. | |
144 Since it used only for reporting purposes, we do not need all the text | |
145 lines in the range. | |
146 | |
147 """ | |
148 results = [] | |
149 previous_line_number = 0 | |
150 previous_range_start_line_number = 0 | |
151 previous_range_start_text = "" | |
152 | |
153 def addRange(): | |
154 tuple = (previous_range_start_line_number, | |
155 previous_line_number, | |
156 previous_range_start_text) | |
157 results.append(tuple) | |
158 | |
159 for line_number, line_text in affected_file.ChangedContents(): | |
160 if first_line <= line_number and line_number <= last_line: | |
161 self.LogDebug("Line change at line number " + str(line_number) + ": " + | |
162 line_text) | |
163 # Start a new interval if none started | |
164 if previous_range_start_line_number == 0: | |
165 previous_range_start_line_number = line_number | |
166 previous_range_start_text = line_text | |
167 # Add new interval if we reached past the previous one | |
168 elif line_number != previous_line_number + 1: | |
169 addRange() | |
170 previous_range_start_line_number = line_number | |
171 previous_range_start_text = line_text | |
172 previous_line_number = line_number | |
173 | |
174 # Add a last interval if needed | |
175 if previous_range_start_line_number != 0: | |
176 addRange() | |
177 return results | |
178 | |
179 def CheckForFileDeletion(self, affected_file): | |
180 """Emits a warning notification if file has been deleted """ | |
181 if not affected_file.NewContents(): | |
182 self.EmitWarning("The file seems to be deleted in the changelist. If " | |
183 "your intent is to really delete the file, the code in " | |
184 "PRESUBMIT.py should be updated to remove the " | |
185 "|HistogramValueChecker| class."); | |
186 return False | |
187 return True | |
188 | |
189 def GetDeletedLinesFromScmDiff(self, affected_file): | |
190 """Return a list of of line numbers (1-based) corresponding to lines | |
191 deleted from the new source file (if they had been present in it). Note | |
192 that if multiple contiguous lines have been deleted, the returned list will | |
193 contain contiguous line number entries. To prevent false positives, we | |
194 return deleted line numbers *only* from diff chunks which decrease the size | |
195 of the new file. | |
196 | |
197 Note: We need this method because we have access to neither the old file | |
198 content nor the list of "delete" changes from the current presubmit script | |
199 API. | |
200 | |
201 """ | |
202 results = [] | |
203 line_num = 0 | |
204 deleting_lines = False | |
205 for line in affected_file.GenerateScmDiff().splitlines(): | |
206 # Parse the unified diff chunk optional section heading, which looks like | |
207 # @@ -l,s +l,s @@ optional section heading | |
208 m = self.input_api.re.match( | |
209 r'^@@ \-([0-9]+)\,([0-9]+) \+([0-9]+)\,([0-9]+) @@', line) | |
210 if m: | |
211 old_line_num = int(m.group(1)) | |
212 old_size = int(m.group(2)) | |
213 new_line_num = int(m.group(3)) | |
214 new_size = int(m.group(4)) | |
215 line_num = new_line_num | |
216 # Return line numbers only from diff chunks decreasing the size of the | |
217 # new file | |
218 deleting_lines = old_size > new_size | |
219 continue | |
220 if not line.startswith('-'): | |
221 line_num += 1 | |
222 if deleting_lines and line.startswith('-') and not line.startswith('--'): | |
223 results.append(line_num) | |
224 return results | |
225 | |
226 def CheckForEnumEntryDeletions(self, affected_file): | |
227 """Look for deletions inside the enum definition. We currently use a | |
228 simple heuristics (not 100% accurate): if there are deleted lines inside | |
229 the enum definition, this might be a deletion. | |
230 | |
231 """ | |
232 range_new = self.ComputeEnumRangeInNewFile(affected_file) | |
233 if not range_new: | |
234 return False | |
235 | |
236 is_ok = True | |
237 for line_num in self.GetDeletedLinesFromScmDiff(affected_file): | |
238 if range_new.Contains(line_num): | |
239 self.EmitWarning("It looks like you are deleting line(s) from the " | |
240 "enum definition. This should never happen.", | |
241 line_num) | |
242 is_ok = False | |
243 return is_ok | |
244 | |
245 def CheckForEnumEntryInsertions(self, affected_file): | |
246 range = self.ComputeEnumRangeInNewFile(affected_file) | |
247 if not range: | |
248 return False | |
249 | |
250 first_line = range.first_line | |
251 last_line = range.last_line | |
252 | |
253 # Collect the range of changes inside the enum definition range. | |
254 is_ok = True | |
255 for line_start, line_end, line_text in \ | |
256 self.CollectRangesInsideEnumDefinition(affected_file, | |
257 first_line, | |
258 last_line): | |
259 # The only edit we consider valid is adding 1 or more entries *exactly* | |
260 # at the end of the enum definition. Every other edit inside the enum | |
261 # definition will result in a "warning confirmation" message. | |
262 # | |
263 # TODO(rpaquay): We currently cannot detect "renames" of existing entries | |
264 # vs invalid insertions, so we sometimes will warn for valid edits. | |
265 is_valid_edit = (line_end == last_line - 1) | |
266 | |
267 self.LogDebug("Edit range in new file at starting at line number %d and " | |
268 "ending at line number %d: valid=%s" | |
269 % (line_start, line_end, is_valid_edit)) | |
270 | |
271 if not is_valid_edit: | |
272 self.EmitWarning("The change starting at line %d and ending at line " | |
273 "%d is *not* located *exactly* at the end of the " | |
274 "enum definition. Unless you are renaming an " | |
275 "existing entry, this is not a valid changes, as new " | |
276 "entries should *always* be added at the end of the " | |
277 "enum definition, right before the 'ENUM_BOUNDARY' " | |
278 "entry." % (line_start, line_end), | |
279 line_start, | |
280 line_text) | |
281 is_ok = False | |
282 return is_ok | |
283 | |
284 def PerformChecks(self, affected_file): | |
285 if not self.CheckForFileDeletion(affected_file): | |
286 return | |
287 if not self.CheckForEnumEntryDeletions(affected_file): | |
288 return | |
289 if not self.CheckForEnumEntryInsertions(affected_file): | |
290 return | |
291 | |
292 def ProcessHistogramValueFile(self, affected_file): | |
293 self.LogInfo("Start processing file \"%s\"" % affected_file.LocalPath()) | |
294 self.PerformChecks(affected_file) | |
295 self.LogInfo("Done processing file \"%s\"" % affected_file.LocalPath()) | |
296 | |
297 def Run(self): | |
298 for file in self.input_api.AffectedFiles(include_deletes=True): | |
299 if file.LocalPath() == self.LOCAL_PATH: | |
300 self.ProcessHistogramValueFile(file) | |
301 return self.results | |
302 | 34 |
303 def CheckChangeOnUpload(input_api, output_api): | 35 def CheckChangeOnUpload(input_api, output_api): |
304 results = [] | 36 results = [] |
305 results += HistogramValueChecker(input_api, output_api).Run() | 37 results += _CreateHistogramValueChecker(input_api, output_api).Run() |
306 results += input_api.canned_checks.CheckPatchFormatted(input_api, output_api) | 38 results += input_api.canned_checks.CheckPatchFormatted(input_api, output_api) |
307 return results | 39 return results |
308 | 40 |
| 41 def CheckChangeOnCommit(input_api, output_api): |
| 42 return _CreateHistogramValueChecker(input_api, output_api).Run() |
OLD | NEW |