strict_enum_value_checker.py 12 KB

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