Merged in dachary/coverage.py/issue-493-2 (pull request #108)

finally happens before return in a try #493

1 files changed, 52 insertions, 14 deletions

diff --git a/coverage/parser.py b/coverage/parser.py
index 3d46bfa..e75694f 100644
--- a/coverage/parser.py
+++ b/coverage/parser.py
@@ -433,23 +433,35 @@ class ByteParser(object):
 
 class LoopBlock(object):
     """A block on the block stack representing a `for` or `while` loop."""
+    @contract(start=int)
     def __init__(self, start):
+        # The line number where the loop starts.
         self.start = start
+        # A set of ArcStarts, the arcs from break statements exiting this loop.
         self.break_exits = set()
 
 
 class FunctionBlock(object):
     """A block on the block stack representing a function definition."""
+    @contract(start=int, name=str)
     def __init__(self, start, name):
+        # The line number where the function starts.
         self.start = start
+        # The name of the function.
         self.name = name
 
 
 class TryBlock(object):
     """A block on the block stack representing a `try` block."""
-    def __init__(self, handler_start=None, final_start=None):
+    @contract(handler_start='int|None', final_start='int|None')
+    def __init__(self, handler_start, final_start):
+        # The line number of the first "except" handler, if any.
         self.handler_start = handler_start
+        # The line number of the "finally:" clause, if any.
         self.final_start = final_start
+
+        # The ArcStarts for breaks/continues/returns/raises inside the "try:"
+        # that need to route through the "finally:" clause.
         self.break_from = set()
         self.continue_from = set()
         self.return_from = set()
@@ -459,8 +471,13 @@ class TryBlock(object):
 class ArcStart(collections.namedtuple("Arc", "lineno, cause")):
     """The information needed to start an arc.
 
-    `lineno` is the line number the arc starts from.  `cause` is a fragment
-    used as the startmsg for AstArcAnalyzer.missing_arc_fragments.
+    `lineno` is the line number the arc starts from.
+
+    `cause` is an English text fragment used as the `startmsg` for
+    AstArcAnalyzer.missing_arc_fragments.  It will be used to describe why an
+    arc wasn't executed, so should fit well into a sentence of the form,
+    "Line 17 didn't run because {cause}."  The fragment can include "{lineno}"
+    to have `lineno` interpolated into it.
 
     """
     def __new__(cls, lineno, cause=None):
@@ -493,7 +510,9 @@ class AstArcAnalyzer(object):
 
         self.arcs = set()
 
-        # A map from arc pairs to a pair of sentence fragments: (startmsg, endmsg).
+        # A map from arc pairs to a list of pairs of sentence fragments:
+        #   { (start, end): [(startmsg, endmsg), ...], }
+        #
         # For an arc from line 17, they should be usable like:
         #    "Line 17 {endmsg}, because {startmsg}"
         self.missing_arc_fragments = collections.defaultdict(list)
@@ -570,6 +589,7 @@ class AstArcAnalyzer(object):
             # Modules have no line number, they always start at 1.
             return 1
 
+    # The node types that just flow to the next node with no complications.
     OK_TO_DEFAULT = set([
         "Assign", "Assert", "AugAssign", "Delete", "Exec", "Expr", "Global",
         "Import", "ImportFrom", "Nonlocal", "Pass", "Print",
@@ -586,12 +606,15 @@ class AstArcAnalyzer(object):
         handler = getattr(self, "_handle__" + node_name, None)
         if handler is not None:
             return handler(node)
-
-        if 0:
-            node_name = node.__class__.__name__
-            if node_name not in self.OK_TO_DEFAULT:
+        else:
+            # No handler: either it's something that's ok to default (a simple
+            # statement), or it's something we overlooked. Change this 0 to 1
+            # to see if it's overlooked.
+            if 0 and node_name not in self.OK_TO_DEFAULT:
                 print("*** Unhandled: {0}".format(node))
-        return set([ArcStart(self.line_for_node(node), cause=None)])
+
+            # Default for simple statements: one exit from this node.
+            return set([ArcStart(self.line_for_node(node))])
 
     @contract(returns='ArcStarts')
     def add_body_arcs(self, body, from_start=None, prev_starts=None):
@@ -634,6 +657,15 @@ class AstArcAnalyzer(object):
     #   listcomps hidden in lists: x = [[i for i in range(10)]]
     #   nested function definitions
 
+
+    # Exit processing: process_*_exits
+    #
+    # These functions process the four kinds of jump exits: break, continue,
+    # raise, and return.  To figure out where an exit goes, we have to look at
+    # the block stack context.  For example, a break will jump to the nearest
+    # enclosing loop block, or the nearest enclosing finally block, whichever
+    # is nearer.
+
     @contract(exits='ArcStarts')
     def process_break_exits(self, exits):
         """Add arcs due to jumps from `exits` being breaks."""
@@ -692,7 +724,12 @@ class AstArcAnalyzer(object):
                     )
                 break
 
-    ## Handlers
+
+    # Handlers: _handle__*
+    #
+    # Each handler deals with a specific AST node type, dispatched from
+    # add_arcs.  These functions mirror the Python semantics of each syntactic
+    # construct.
 
     @contract(returns='ArcStarts')
     def _handle__Break(self, node):
@@ -722,7 +759,7 @@ class AstArcAnalyzer(object):
                     self.add_arc(last, lineno)
                     last = lineno
         # The body is handled in collect_arcs.
-        return set([ArcStart(last, cause=None)])
+        return set([ArcStart(last)])
 
     _handle__ClassDef = _handle_decorated
 
@@ -749,7 +786,7 @@ class AstArcAnalyzer(object):
             else_exits = self.add_body_arcs(node.orelse, from_start=from_start)
             exits |= else_exits
         else:
-            # no else clause: exit from the for line.
+            # No else clause: exit from the for line.
             exits.add(from_start)
         return exits
 
@@ -795,11 +832,11 @@ class AstArcAnalyzer(object):
         else:
             final_start = None
 
-        try_block = TryBlock(handler_start=handler_start, final_start=final_start)
+        try_block = TryBlock(handler_start, final_start)
         self.block_stack.append(try_block)
 
         start = self.line_for_node(node)
-        exits = self.add_body_arcs(node.body, from_start=ArcStart(start, cause=None))
+        exits = self.add_body_arcs(node.body, from_start=ArcStart(start))
 
         # We're done with the `try` body, so this block no longer handles
         # exceptions. We keep the block so the `finally` clause can pick up
@@ -860,6 +897,7 @@ class AstArcAnalyzer(object):
 
         return exits
 
+    @contract(returns='ArcStarts')
     def _combine_finally_starts(self, starts, exits):
         """Helper for building the cause of `finally` branches."""
         causes = []