Index: tools/about_flags_switches_histogram_ids_checker/about_flags_switches_histogram_ids_checker.py |
diff --git a/tools/about_flags_switches_histogram_ids_checker/about_flags_switches_histogram_ids_checker.py b/tools/about_flags_switches_histogram_ids_checker/about_flags_switches_histogram_ids_checker.py |
new file mode 100644 |
index 0000000000000000000000000000000000000000..f69800073aa8c0d2854d4f41c5cb56a0fd5c936b |
--- /dev/null |
+++ b/tools/about_flags_switches_histogram_ids_checker/about_flags_switches_histogram_ids_checker.py |
@@ -0,0 +1,164 @@ |
+# Copyright 2014 The Chromium Authors. All rights reserved. |
+# Use of this source code is governed by a BSD-style license that can be |
+# found in the LICENSE file. |
+ |
+class AboutFlagsSwitchesHistogramIDsChecker(object): |
+ """Verify that changes to SwitchesUmaHistogramIds enum are valid. |
+ |
+ This class is used to check SwitchesUmaHistogramIds enum is never shrinked or |
+ reordered. This enum is used to report UMA histograms, so IDs must never |
+ change. See comments on the top of |
+ chrome/browser/about_flags_switches_histogram_ids.h . |
+ |
+ """ |
+ def __init__(self, input_api, output_api, path): |
+ self.input_api = input_api |
+ self.output_api = output_api |
+ self.path = path |
+ self.warnings_exist = False |
+ self.enum_found = False |
+ self.results = [] |
+ |
+ def LogInfo(self, message): |
+ self.input_api.logging.info(message) |
+ return |
+ |
+ def LogDebug(self, message): |
+ self.input_api.logging.debug(message) |
+ return |
+ |
+ def GetLongMessage(self, local_path): |
+ return str("The file \"%s\"\n" |
+ "contains the definition of the 'SwitchesUmaHistogramId' enum,\n" |
+ "which should be edited in specific ways only.\n" |
+ "*** read the comments at the top of the header file ***." |
+ % (local_path)) |
+ |
+ def EmitWarning(self, message, line_number=None, line_text=None): |
+ """Emits a presubmit prompt warning containing the short message |
+ |message|. |item| is |LOCAL_PATH| with optional |line_number| and |
+ |line_text|. |
+ |
+ """ |
+ if line_number is not None and line_text is not None: |
+ item = "%s(%d): %s" % (self.path, line_number, line_text) |
+ elif line_number is not None: |
+ item = "%s(%d)" % (self.path, line_number) |
+ else: |
+ item = self.path |
+ if self.warnings_exist : |
+ long_message = '' |
+ else : |
+ long_message = self.GetLongMessage(self.path) |
+ self.warnings_exist = True |
+ |
+ self.LogInfo(message) |
+ self.results.append( |
+ self.output_api.PresubmitPromptWarning(message, [item], long_message)) |
+ |
+ def ReportDeletedLines(self, expected_new_deprecated, prefix=''): |
+ message = prefix |
+ message += str("It looks like you are deleting line(s) from the " |
+ "enum definition. This should never happen.\n") |
+ message += "The following line(s) have been deleted:\n" |
+ for value in expected_new_deprecated : |
+ message += "\t" + value + "\n" |
+ message += str("Remember to add 'DEPRECATED_' prefix to the no more " |
+ "needed entries") |
+ self.EmitWarning(message) |
+ |
+ def VerifyNewItemname(self, item) : |
+ m = self.input_api.re.match(r"^UMA_HISTOGRAM_ID_", item) |
+ if not m : |
+ self.EmitWarning("New enum item '" + item + "' doesn't start with " + |
+ "'UMA_HISTOGRAM_ID_'.") |
+ return False |
+ return True |
+ |
+ def CheckDiff(self, affected_file): |
+ """Check diff is valid. |
+ |
+ """ |
+ expected_new_deprecated = [] |
+ result = True |
+ end_flag = False # End of enum found (only addition is allowed). |
+ for line in affected_file.GenerateScmDiff().splitlines(): |
+ m = self.input_api.re.match( |
+ r"^([+ -]) *([A-Za-z0-9_]+) *,", line) |
+ if not m : |
+ # Check the last line (probably without ','). |
+ if not self.enum_found : |
+ continue |
+ m = self.input_api.re.match( |
+ r"^([+ -]) *([A-Za-z0-9_]+)", line) |
+ if m : |
+ self.enum_found = True |
+ self.LogDebug("MATCHED '" + line + "'") |
+ if end_flag : |
+ # We are at the end of enum. |
+ # Only new items are allowed here. |
+ if m.group(1) == "+" : |
+ if not self.VerifyNewItemname(m.group(2)) : |
+ result = False |
+ continue |
+ result = False |
+ message = str("It looks like you are inserting lines in the " |
+ "middle of the enum definition. This should never " |
+ "happen.\n") |
+ if m.group(1) == "-" : |
+ message += str("Found deleted item '" + m.group(2) + |
+ "' after some new lines added.") |
+ elif m.group(1) == " " : |
+ message += str("A new lines have been added before item '" + |
+ m.group(2) + "'.") |
+ else : |
+ raise Exception("Unknown SCM diff op '" + m.group(1) + |
+ "' found in diff line '" + line + "'") |
+ self.EmitWarning(message) |
+ else : # end_flag is False |
+ if m.group(1) == "-" : |
+ expected_new_deprecated.append(m.group(2)) |
+ continue |
+ |
+ # All items must be renamed before any unchanged line. |
+ if m.group(1) == " " and len(expected_new_deprecated) == 0 : |
+ continue |
+ |
+ if m.group(1) == "+" : |
+ if len(expected_new_deprecated) : |
+ if "DEPRECATED_" + expected_new_deprecated[0] == m.group(2) : |
+ expected_new_deprecated.pop(0) |
+ continue; |
+ else : |
+ if not self.VerifyNewItemname(m.group(2)) : |
+ result = False |
+ # New lines must be added to the end, so we've reached enum end. |
+ end_flag = True; |
+ continue; |
+ |
+ # Format error: |
+ result = False |
+ if len(expected_new_deprecated) : |
+ self.ReportDeletedLines(expected_new_deprecated, |
+ str("The unexpected line adds new value '" + m.group(2) + |
+ "', but 'DEPRECATED_" + expected_new_deprecated[0] + |
+ "' is expected.\n")) |
+ expected_new_deprecated = [] |
+ continue |
+ |
+ if len(expected_new_deprecated) : |
+ self.ReportDeletedLines(expected_new_deprecated) |
+ |
+ return result |
+ |
+ def PerformChecks(self, affected_file): |
+ if not self.CheckDiff(affected_file): |
+ return |
+ |
+ def Run(self): |
+ for file in self.input_api.AffectedFiles(include_deletes=True): |
+ if file.LocalPath() == self.path: |
+ self.LogInfo("Start processing file.") |
+ self.PerformChecks(file) |
+ self.LogInfo("Done processing file.") |
+ return self.results |
Ilya Sherman
2014/07/21 22:12:59
Can you reuse some of the existing code, rather th
Alexander Alekseev
2014/07/22 14:25:06
Hmm. That means splitting strict_enum_value_checke
|