[Findit] Reorganizing findit_for_*.py

appengine/findit/crash/results.py

 # Copyright 2016 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.
 
 
+from collections import namedtuple
+
 # TODO(http://crbug.com/644476): this class needs a better name.
+class AnalysisInfo(namedtuple('AnalysisInfo',
+    ['min_distance', 'min_distance_frame'])):
+  __slots__ = ()
+
+
+# TODO(http://crbug.com/644476): this class needs a better name.
+# TODO(wrengr): subclass namedtuple, so most things are immutable.
 class Result(object):
   """Represents findit culprit result."""
 
   def __init__(self, changelog, dep_path,
                confidence=None, reasons=None, changed_files=None):
     self.changelog = changelog
     self.dep_path = dep_path
     self.confidence = confidence
     self.reasons = reasons
     self.changed_files = changed_files
 
+    # TODO(wrengr): (a) make these two fields private/readonly
+    # TODO(wrengr): (b) zip them together.
+    # TODO(wrengr): replace "stack_info" pair with a namedtuple.
     self.file_to_stack_infos = {}
+    # TODO(wrengr): replace "analysis_info" dict with a namedtuple.
     self.file_to_analysis_info = {}
 
   def ToDict(self):
     return {
         'url': self.changelog.commit_url,
         'review_url': self.changelog.code_review_url,
         'revision': self.changelog.revision,
         'project_path': self.dep_path,
         'author': self.changelog.author_email,
         'time': str(self.changelog.author_time),
         'reasons': self.reasons,
         'changed_files': self.changed_files,
         'confidence': self.confidence,
     }
 
+  # TODO(katesonia): This is unusable for logging because in all the
+  # cases that need logging it returns the empty string! We should print
+  # this out in a more useful way (e.g., how CrashConfig is printed)
+  # so that callers don't have to use |str(result.ToDict())| instead. If
+  # we want a method that does what this one does, we should give it a
+  # different name that indicates what it's actually printing out.
   def ToString(self):
     if not self.file_to_stack_infos:
       return ''
 
     lines = []
     for file_path, stack_infos in self.file_to_stack_infos.iteritems():
       line_parts = []
       for frame, _ in stack_infos:
         line_parts.append('frame #%d' % frame.index)
 
@@ skipping 41 matching lines @@
       region_end = region_start + region.count - 1
       for frame, _ in stack_infos:
         frame_start = frame.crashed_line_numbers[0]
         frame_end = frame.crashed_line_numbers[-1]
         distance = _DistanceBetweenLineRanges((frame_start, frame_end),
                                               (region_start, region_end))
         if distance < min_distance:
           min_distance = distance
           min_distance_frame = frame
 
-    self.file_to_analysis_info[file_path] = {
-        'min_distance': min_distance,
-        'min_distance_frame': min_distance_frame,
-    }
+    self.file_to_analysis_info[file_path] = AnalysisInfo(
+        min_distance = min_distance,
+        min_distance_frame = min_distance_frame,
+    )
 
 
 def _DistanceBetweenLineRanges((start1, end1), (start2, end2)):
   """Given two ranges, compute the (unsigned) distance between them.
 
   Args:
     start1: the start of the first range
     end1: the end of the first range. Must be greater than start1.
     start2: the start of the second range
     end2: the end of the second range. Must be greater than start2.
 
   Returns:
     If the end of the earlier range comes before the start of the later
     range, then the difference between those points. Otherwise, returns
     zero (because the ranges overlap)."""
   assert end1 >= start1
   assert end2 >= start2
   # There are six possible cases, but in all the cases where the two
   # ranges overlap, the latter two differences will be negative.
   return max(0, start2 - end1, start1 - end2)
 
 
 class MatchResults(dict):
   """A dict indexing MatchResult with its revision."""
 
   def __init__(self, ignore_cls=None):
     super(MatchResults, self).__init__()
-    self.ignore_cls = ignore_cls
+    self._ignore_cls = ignore_cls
 
   def GenerateMatchResults(self, file_path, dep_path,
                            stack_infos, changelogs, blame):
     """Generates match results.
 
     Match results are generated based on newly found file path, its stack_infos,
     and all the changelogs that touched this file in the dep in regression
     ranges, those reverted changelogs should be ignored.
 
     Args:
       file_path (str): File path of the crashed file.
       dep_path (str): Path of the dependency of the file.
       stack_infos (list): List of stack_info dicts, represents frames of this
         file and the callstack priorities of those frames.
       changelogs (list): List of Changelog objects in the dep in regression
         range which touched the file.
       blame (Blame): Blame of the file.
     """
     for changelog in changelogs:
-      if self.ignore_cls and changelog.revision in self.ignore_cls:
+      if self._ignore_cls and changelog.revision in self._ignore_cls:
         continue
 
       if changelog.revision not in self:
         self[changelog.revision] = MatchResult(changelog, dep_path)
 
       match_result = self[changelog.revision]
       match_result.Update(file_path, stack_infos, blame)