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 |
- |