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