tools/strict_enum_value_checker/strict_enum_value_checker.py

   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