Add PRESUBMIT check if modified UMA histogram name can be found (2)

PRESUBMIT.py

Index: PRESUBMIT.py
diff --git a/PRESUBMIT.py b/PRESUBMIT.py
index 33008be52923f9a0fb57c56b30139857bb76d6b4..c67158272e00b83e93c7594c07e5c0552ddccfca 100644
--- a/PRESUBMIT.py
+++ b/PRESUBMIT.py
@@ -358,6 +358,65 @@ def _CheckNoUNIT_TESTInSourceFiles(input_api, output_api):
   return [output_api.PresubmitPromptWarning('UNIT_TEST is only for headers.\n' +
       '\n'.join(problems))]
 
+# Helper function to smartly try to find a histogram name or prefix in a line.

[Alexei Svitkine (slow), 2015/01/30 15:15:36]

Make this a """ comment inside the function.

[mcasas, 2015/01/30 23:45:48]

Done.

+def _FindHistogramNameInLine(histogram_name, line):
+  if not "affected-histogram" in line:
+    return histogram_name in line;

[Alexei Svitkine (slow), 2015/01/30 15:15:37]

Nit: No need for trailing ;

[mcasas, 2015/01/30 23:45:48]

Done.

+  # A histogram_suffixes tag type has an affected-histogram name as a prefix of
+  # the histogram_name.
+  if not '"' in line:
+    return False;
+  histogram_prefix = line.split('\"')[1];
+  return histogram_prefix in histogram_name;
+

[Alexei Svitkine (slow), 2015/01/30 15:15:36]

Nit: Two empty lines between functions.

[mcasas, 2015/01/30 23:45:48]

Done.

+def _CheckUmaHistogramChanges(input_api, output_api):
+  """Check that UMA histogram names in touched lines can still be found in other
+  lines of the patch or in histograms.xml. Note that this check would not catch
+  the reverse: changes in histograms.xml not matched in the code itself."""
+

[Alexei Svitkine (slow), 2015/01/30 15:15:37]

Nit: No blank line.

[mcasas, 2015/01/30 23:45:48]

Done.

+  touched_histograms = []
+  histograms_xml_modifications = []
+  pattern = input_api.re.compile('UMA_HISTOGRAM.*\("(.*)"')
+  for f in input_api.AffectedFiles():
+    # If histograms.xml itself is modified, keep the modified lines for later.
+    if (f.LocalPath().endswith(('histograms.xml'))):

[Alexei Svitkine (slow), 2015/01/30 15:15:36]

Nit: No need for outer parens in Python. Same for the if below.

[mcasas, 2015/01/30 23:45:48]

Done.

+      histograms_xml_modifications = f.ChangedContents()
+      continue
+    if (not f.LocalPath().endswith(('cc', 'mm', 'cpp'))):
+      continue
+    for line_num, line in f.ChangedContents():
+      found = pattern.search(line)
+      if found:
+        touched_histograms.append([found.group(1), f, line_num])
+
+  # Search for the touched histogram names in the local modifications to
+  # histograms.xml, and, if not found, on the base file.
+  problems = []
+  for histogram_name, f, line_num in touched_histograms:
+    histogram_name_found = False
+    for line_num, line in histograms_xml_modifications:
+      histogram_name_found = _FindHistogramNameInLine(histogram_name, line);
+      if histogram_name_found:
+        break;
+    if histogram_name_found:
+      continue
+
+    with open('tools/metrics/histograms/histograms.xml') as histograms_xml:

[Alexei Svitkine (slow), 2015/01/30 15:15:37]

It seems inefficient to open the file and iterate through all of its lines
potentially multiple times.

Can you collect the list of histograms that weren't found and look for them all
at once with a single iteration over histograms.xml lines?

[mcasas, 2015/01/30 23:45:48]

Done.

+      for line in histograms_xml:
+        histogram_name_found = _FindHistogramNameInLine(histogram_name, line);
+        if histogram_name_found:
+          break;
+    if histogram_name_found:
+      continue
+
+    problems.append(' [%s:%d] %s' % (f.LocalPath(), line_num, histogram_name))
+
+  if not problems:
+    return []
+  return [output_api.PresubmitPromptWarning('Some UMA_HISTOGRAM lines have '
+    'been modified and the associated histogram name has no match in either '
+    'metrics/histograms.xml or the modifications of it:',  problems)]
+
 
 def _CheckNoNewWStrings(input_api, output_api):
   """Checks to make sure we don't introduce use of wstrings."""
@@ -1585,6 +1644,7 @@ def CheckChangeOnUpload(input_api, output_api):
   results.extend(_CheckJavaStyle(input_api, output_api))
   results.extend(
       input_api.canned_checks.CheckGNFormatted(input_api, output_api))
+  results.extend(_CheckUmaHistogramChanges(input_api, output_api))
   return results