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

Unified Diff: tools/strict_enum_value_checker/strict_enum_value_checker.py

Issue 170233008: Add presubmit check and automatic update script for ExtensionPermission enum. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 6 years, 9 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 side-by-side diff with in-line comments
Download patch
Index: tools/strict_enum_value_checker/strict_enum_value_checker.py
diff --git a/extensions/browser/PRESUBMIT.py b/tools/strict_enum_value_checker/strict_enum_value_checker.py
similarity index 77%
copy from extensions/browser/PRESUBMIT.py
copy to tools/strict_enum_value_checker/strict_enum_value_checker.py
index 29a709b5c807ff63d08b90bba2efb5c61c888067..65c4a9ecbf1181e894f8d1821d4875fe0348e616 100644
--- a/extensions/browser/PRESUBMIT.py
+++ b/tools/strict_enum_value_checker/strict_enum_value_checker.py
@@ -2,42 +2,25 @@
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
-"""Chromium presubmit script for src/extensions/browser.
-
-See http://dev.chromium.org/developers/how-tos/depottools/presubmit-scripts
-for more details on the presubmit API built into gcl.
-"""
-
-def GetPreferredTryMasters(project, change):
- return {
- 'tryserver.chromium': {
- 'linux_chromium_chromeos_rel': set(['defaulttests']),
- }
- }
-
-class HistogramValueChecker(object):
- """Verify that changes to "extension_function_histogram_value.h" are valid.
-
- See comments at the top of the "extension_function_histogram_value.h" file
- for what are considered valid changes. There are situations where this script
- gives false positive warnings, i.e. it warns even though the edit is
- legitimate. Since the script warns using prompt warnings, the user can always
- choose to continue. The main point is to attract the attention to all
- (potentially or not) invalid edits.
+class StrictEnumValueChecker(object):
+ """Verify that changes to enums are valid.
+
+ This class is used to check enums where reordering or deletion is not allowed,
+ and additions must be at the end of the enum, just prior to some "boundary"
+ entry. See comments at the top of the "extension_function_histogram_value.h"
+ file in chrome/browser/extensions for an example what are considered valid
+ changes. There are situations where this class gives false positive warnings,
+ i.e. it warns even though the edit is legitimate. Since the class warns using
+ prompt warnings, the user can always choose to continue. The main point is to
+ attract the attention to all (potentially or not) invalid edits.
"""
-
- # The name of the file we want to check against
- LOCAL_PATH = "extensions/browser/extension_function_histogram_value.h"
-
- # The markers we look for in the above source file as delimiters of the enum
- # definition.
- ENUM_START_MARKER = "enum HistogramValue {"
- ENUM_END_MARKER = " // Last entry:"
-
- def __init__(self, input_api, output_api):
+ def __init__(self, input_api, output_api, start_marker, end_marker, path):
self.input_api = input_api
self.output_api = output_api
+ self.start_marker = start_marker
+ self.end_marker = end_marker
+ self.path = path
self.results = []
class EnumRange(object):
@@ -62,11 +45,11 @@ class HistogramValueChecker(object):
def ComputeEnumRangeInContents(self, contents):
"""Returns an |EnumRange| object representing the line extent of the
- HistogramValue enum members in |contents|. The line numbers are 1-based,
+ enum members in |contents|. The line numbers are 1-based,
compatible with line numbers returned by AffectedFile.ChangeContents().
|contents| is a list of strings reprenting the lines of a text file.
- If either ENUM_START_MARKER or ENUM_END_MARKER cannot be found in
+ If either start_marker or end_marker cannot be found in
|contents|, returns None and emits detailed warnings about the problem.
"""
@@ -74,43 +57,43 @@ class HistogramValueChecker(object):
last_enum_line = 0
line_num = 1 # Line numbers are 1-based
for line in contents:
- if line.startswith(self.ENUM_START_MARKER):
+ if line.startswith(self.start_marker):
first_enum_line = line_num + 1
- elif line.startswith(self.ENUM_END_MARKER):
+ elif line.startswith(self.end_marker):
last_enum_line = line_num
line_num += 1
if first_enum_line == 0:
self.EmitWarning("The presubmit script could not find the start of the "
"enum definition (\"%s\"). Did the enum definition "
- "change?" % self.ENUM_START_MARKER)
+ "change?" % self.start_marker)
return None
if last_enum_line == 0:
self.EmitWarning("The presubmit script could not find the end of the "
"enum definition (\"%s\"). Did the enum definition "
- "change?" % self.ENUM_END_MARKER)
+ "change?" % self.end_marker)
return None
if first_enum_line >= last_enum_line:
self.EmitWarning("The presubmit script located the start of the enum "
"definition (\"%s\" at line %d) *after* its end "
"(\"%s\" at line %d). Something is not quite right."
- % (self.ENUM_START_MARKER, first_enum_line,
- self.ENUM_END_MARKER, last_enum_line))
+ % (self.start_marker, first_enum_line,
+ self.end_marker, last_enum_line))
return None
- self.LogInfo("Line extent of |HistogramValue| enum definition: "
+ self.LogInfo("Line extent of (\"%s\") enum definition: "
"first_line=%d, last_line=%d."
- % (first_enum_line, last_enum_line))
+ % (self.start_marker, first_enum_line, last_enum_line))
return self.EnumRange(first_enum_line, last_enum_line)
def ComputeEnumRangeInNewFile(self, affected_file):
return self.ComputeEnumRangeInContents(affected_file.NewContents())
- def GetLongMessage(self):
+ def GetLongMessage(self, local_path):
return str("The file \"%s\" contains the definition of the "
- "|HistogramValue| enum which should be edited in specific ways "
+ "(\"%s\") enum which should be edited in specific ways "
"only - *** read the comments at the top of the header file ***"
". There are changes to the file that may be incorrect and "
"warrant manual confirmation after review. Note that this "
@@ -118,7 +101,7 @@ class HistogramValueChecker(object):
"types of invalid changes, especially when the diffs are "
"complex. For example, an invalid deletion may be reported "
"whereas the change contains a valid rename."
- % self.LOCAL_PATH)
+ % local_path, self.start_marker)
def EmitWarning(self, message, line_number=None, line_text=None):
"""Emits a presubmit prompt warning containing the short message
@@ -127,12 +110,12 @@ class HistogramValueChecker(object):
"""
if line_number is not None and line_text is not None:
- item = "%s(%d): %s" % (self.LOCAL_PATH, line_number, line_text)
+ item = "%s(%d): %s" % (self.path, line_number, line_text)
elif line_number is not None:
- item = "%s(%d)" % (self.LOCAL_PATH, line_number)
+ item = "%s(%d)" % (self.path, line_number)
else:
- item = self.LOCAL_PATH
- long_message = self.GetLongMessage()
+ item = self.path
+ long_message = self.GetLongMessage(self.path)
self.LogInfo(message)
self.results.append(
self.output_api.PresubmitPromptWarning(message, [item], long_message))
@@ -182,7 +165,7 @@ class HistogramValueChecker(object):
self.EmitWarning("The file seems to be deleted in the changelist. If "
"your intent is to really delete the file, the code in "
"PRESUBMIT.py should be updated to remove the "
- "|HistogramValueChecker| class.");
+ "|StrictEnumValueChecker| class.");
return False
return True
@@ -206,7 +189,7 @@ class HistogramValueChecker(object):
# Parse the unified diff chunk optional section heading, which looks like
# @@ -l,s +l,s @@ optional section heading
m = self.input_api.re.match(
- r'^@@ \-([0-9]+)\,([0-9]+) \+([0-9]+)\,([0-9]+) @@', line)
+ r"^@@ \-([0-9]+)\,([0-9]+) \+([0-9]+)\,([0-9]+) @@", line)
if m:
old_line_num = int(m.group(1))
old_size = int(m.group(2))
@@ -217,9 +200,9 @@ class HistogramValueChecker(object):
# new file
deleting_lines = old_size > new_size
continue
- if not line.startswith('-'):
+ if not line.startswith("-"):
line_num += 1
- if deleting_lines and line.startswith('-') and not line.startswith('--'):
+ if deleting_lines and line.startswith("-") and not line.startswith("--"):
results.append(line_num)
return results
@@ -272,10 +255,10 @@ class HistogramValueChecker(object):
self.EmitWarning("The change starting at line %d and ending at line "
"%d is *not* located *exactly* at the end of the "
"enum definition. Unless you are renaming an "
- "existing entry, this is not a valid changes, as new "
+ "existing entry, this is not a valid change, as new "
"entries should *always* be added at the end of the "
- "enum definition, right before the 'ENUM_BOUNDARY' "
- "entry." % (line_start, line_end),
+ "enum definition, right before the \"%s\" "
+ "entry." % (line_start, line_end, self.end_marker),
line_start,
line_text)
is_ok = False
@@ -296,13 +279,6 @@ class HistogramValueChecker(object):
def Run(self):
for file in self.input_api.AffectedFiles(include_deletes=True):
- if file.LocalPath() == self.LOCAL_PATH:
+ if file.LocalPath() == self.path:
self.ProcessHistogramValueFile(file)
return self.results
-
-def CheckChangeOnUpload(input_api, output_api):
- results = []
- results += HistogramValueChecker(input_api, output_api).Run()
- results += input_api.canned_checks.CheckPatchFormatted(input_api, output_api)
- return results
-
« no previous file with comments | « tools/strict_enum_value_checker/mock_enum.h ('k') | tools/strict_enum_value_checker/strict_enum_value_checker_test.py » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698