Properly handle if-statements optimized away. #522

4 files changed, 198 insertions, 25 deletions

diff --git a/CHANGES.rst b/CHANGES.rst
index 90cb43d2..459f82ff 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -13,6 +13,11 @@ Unreleased
   would cause a "No data to report" error, as reported in `issue 549`_. This is
   now fixed; thanks, Loïc Dachary.
 
+- If-statements can be optimized away during compilation, for example, `if 0:`
+  or `if __debug__:`.  Coverage.py had problems properly understanding these
+  statements which existed in the source, but not in the compiled bytecode.
+  This problem, reported in `issue 522`_, is now fixed.
+
 - If you specified ``--source`` as a directory, then coverage.py would look for
   importable Python files in that directory, and could identify ones that had
   never been executed at all.  But if you specified it as a package name, that
@@ -41,6 +46,7 @@ Unreleased
 
 .. _issue 322: https://bitbucket.org/ned/coveragepy/issues/322/cannot-use-coverage-with-jython
 .. _issue 426: https://bitbucket.org/ned/coveragepy/issues/426/difference-between-coverage-results-with
+.. _issue 522: https://bitbucket.org/ned/coveragepy/issues/522/incorrect-branch-reporting-with-__debug__
 .. _issue 549: https://bitbucket.org/ned/coveragepy/issues/549/skip-covered-with-100-coverage-throws-a-no
 .. _issue 551: https://bitbucket.org/ned/coveragepy/issues/551/coveragepy-cannot-be-imported-in-jython27
 
diff --git a/coverage/parser.py b/coverage/parser.py
index 6bcda9ca..b9aa6c23 100644
--- a/coverage/parser.py
+++ b/coverage/parser.py
@@ -15,7 +15,7 @@ from coverage.backward import range    # pylint: disable=redefined-builtin
 from coverage.backward import bytes_to_ints, string_class
 from coverage.bytecode import CodeObjects
 from coverage.debug import short_stack
-from coverage.misc import contract, new_contract, nice_pair, join_regex
+from coverage.misc import contract, join_regex, new_contract, nice_pair, one_of
 from coverage.misc import NoSource, IncapablePython, NotPython
 from coverage.phystokens import compile_unicode, generate_tokens, neuter_encoding_declaration
 
@@ -492,6 +492,18 @@ new_contract('ArcStarts', lambda seq: all(isinstance(x, ArcStart) for x in seq))
 # Turn on AST dumps with an environment variable.
 AST_DUMP = bool(int(os.environ.get("COVERAGE_AST_DUMP", 0)))
 
+class NodeList(object):
+    """A synthetic fictitious node, containing a sequence of nodes.
+
+    This is used when collapsing optimized if-statements, to represent the
+    unconditional execution of one of the clauses.
+
+    """
+    def __init__(self, body):
+        self.body = body
+        self.lineno = body[0].lineno
+
+
 class AstArcAnalyzer(object):
     """Analyze source text with an AST to find executable code paths."""
 
@@ -586,7 +598,7 @@ class AstArcAnalyzer(object):
         if node.body:
             return self.line_for_node(node.body[0])
         else:
-            # Modules have no line number, they always start at 1.
+            # Empty 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.
@@ -599,7 +611,17 @@ class AstArcAnalyzer(object):
     def add_arcs(self, node):
         """Add the arcs for `node`.
 
-        Return a set of ArcStarts, exits from this node to the next.
+        Return a set of ArcStarts, exits from this node to the next. Because a
+        node represents an entire sub-tree (including its children), the exits
+        from a node can be arbitrarily complex::
+
+            if something(1):
+                if other(2):
+                    doit(3)
+                else:
+                    doit(5)
+
+        There are two exits from line 1: they start at line 3 and line 5.
 
         """
         node_name = node.__class__.__name__
@@ -610,12 +632,14 @@ class AstArcAnalyzer(object):
             # 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))
+            if 0:
+                if node_name not in self.OK_TO_DEFAULT:
+                    print("*** Unhandled: {0}".format(node))
 
             # Default for simple statements: one exit from this node.
             return set([ArcStart(self.line_for_node(node))])
 
+    @one_of("from_start, prev_starts")
     @contract(returns='ArcStarts')
     def add_body_arcs(self, body, from_start=None, prev_starts=None):
         """Add arcs for the body of a compound statement.
@@ -634,19 +658,73 @@ class AstArcAnalyzer(object):
             lineno = self.line_for_node(body_node)
             first_line = self.multiline.get(lineno, lineno)
             if first_line not in self.statements:
-                continue
+                body_node = self.find_non_missing_node(body_node)
+                if body_node is None:
+                    continue
+                lineno = self.line_for_node(body_node)
             for prev_start in prev_starts:
                 self.add_arc(prev_start.lineno, lineno, prev_start.cause)
             prev_starts = self.add_arcs(body_node)
         return prev_starts
 
+    def find_non_missing_node(self, node):
+        """Search `node` looking for a child that has not been optimized away.
+
+        This might return the node you started with, or it will work recursively
+        to find a child node in self.statements.
+
+        Returns a node, or None if none of the node remains.
+
+        """
+        # This repeats work just done in add_body_arcs, but this duplication
+        # means we can avoid a function call in the 99.9999% case of not
+        # optimizing away statements.
+        lineno = self.line_for_node(node)
+        first_line = self.multiline.get(lineno, lineno)
+        if first_line in self.statements:
+            return node
+
+        missing_fn = getattr(self, "_missing__" + node.__class__.__name__, None)
+        if missing_fn:
+            node = missing_fn(node)
+        else:
+            node = None
+        return node
+
+    def _missing__If(self, node):
+        # If the if-node is missing, then one of its children might still be
+        # here, but not both. So return the first of the two that isn't missing.
+        # Use a NodeList to hold the clauses as a single node.
+        non_missing = self.find_non_missing_node(NodeList(node.body))
+        if non_missing:
+            return non_missing
+        if node.orelse:
+            return self.find_non_missing_node(NodeList(node.orelse))
+        return None
+
+    def _missing__NodeList(self, node):
+        # A NodeList might be a mixture of missing and present nodes. Find the
+        # ones that are present.
+        non_missing_children = []
+        for child in node.body:
+            child = self.find_non_missing_node(child)
+            if child is not None:
+                non_missing_children.append(child)
+
+        # Return the simplest representation of the present children.
+        if not non_missing_children:
+            return None
+        if len(non_missing_children) == 1:
+            return non_missing_children[0]
+        return NodeList(non_missing_children)
+
     def is_constant_expr(self, node):
         """Is this a compile-time constant?"""
         node_name = node.__class__.__name__
         if node_name in ["NameConstant", "Num"]:
             return "Num"
         elif node_name == "Name":
-            if node.id in ["True", "False", "None"]:
+            if node.id in ["True", "False", "None", "__debug__"]:
                 return "Name"
         return None
 
@@ -728,8 +806,10 @@ class AstArcAnalyzer(object):
     # 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.
+    # add_arcs.  Each deals with a particular kind of node type, and returns
+    # the set of exits from that node. These functions mirror the Python
+    # semantics of each syntactic construct.  See the docstring for add_arcs to
+    # understand the concept of exits from a node.
 
     @contract(returns='ArcStarts')
     def _handle__Break(self, node):
@@ -805,6 +885,12 @@ class AstArcAnalyzer(object):
         return exits
 
     @contract(returns='ArcStarts')
+    def _handle__NodeList(self, node):
+        start = self.line_for_node(node)
+        exits = self.add_body_arcs(node.body, from_start=ArcStart(start))
+        return exits
+
+    @contract(returns='ArcStarts')
     def _handle__Raise(self, node):
         here = self.line_for_node(node)
         raise_start = ArcStart(here, cause="the raise on line {lineno} wasn't executed")
diff --git a/tests/test_arcs.py b/tests/test_arcs.py
index 505cab73..1388d506 100644
--- a/tests/test_arcs.py
+++ b/tests/test_arcs.py
@@ -1032,6 +1032,102 @@ class YieldTest(CoverageTest):
         )
 
 
+class OptimizedIfTest(CoverageTest):
+    """Tests of if statements being optimized away."""
+
+    def test_optimized_away_if_0(self):
+        self.check_coverage("""\
+            a = 1
+            if len([2]):
+                c = 3
+            if 0:               # this line isn't in the compiled code.
+                if len([5]):
+                    d = 6
+            else:
+                e = 8
+            f = 9
+            """,
+            lines=[1, 2, 3, 8, 9],
+            arcz=".1 12 23 28 38 89 9.",
+            arcz_missing="28",
+        )
+
+    def test_optimized_away_if_1(self):
+        self.check_coverage("""\
+            a = 1
+            if len([2]):
+                c = 3
+            if 1:               # this line isn't in the compiled code,
+                if len([5]):    # but these are.
+                    d = 6
+            else:
+                e = 8
+            f = 9
+            """,
+            lines=[1, 2, 3, 5, 6, 9],
+            arcz=".1 12 23 25 35 56 69 59 9.",
+            arcz_missing="25 59",
+        )
+        self.check_coverage("""\
+            a = 1
+            if 1:
+                b = 3
+                c = 4
+            d = 5
+            """,
+            lines=[1, 3, 4, 5],
+            arcz=".1 13 34 45 5.",
+        )
+
+    def test_optimized_nested(self):
+        self.check_coverage("""\
+            a = 1
+            if 0:
+                if 0:
+                    b = 4
+                else:
+                    c = 6
+            else:
+                if 0:
+                    d = 9
+                else:
+                    if 0: e = 11
+                    f = 12
+                    if 0: g = 13
+            h = 14
+            """,
+            lines=[1, 12, 14],
+            arcz=".1 1C CE E.",
+        )
+
+    def test_constant_if(self):
+        if env.PYPY:
+            self.skipTest("PyPy doesn't optimize away 'if __debug__:'")
+        # CPython optimizes away "if __debug__:"
+        self.check_coverage("""\
+            for value in [True, False]:
+                if value:
+                    if __debug__:
+                        x = 4
+                else:
+                    x = 6
+            """,
+            arcz=".1 12 24 41 26 61 1.",
+        )
+        # No Python optimizes away "if not __debug__:"
+        self.check_coverage("""\
+            for value in [True, False]:
+                if value:
+                    if not __debug__:
+                        x = 4
+                else:
+                    x = 6
+            """,
+            arcz=".1 12 23 31 34 41 26 61 1.",
+            arcz_missing="34 41",
+        )
+
+
 class MiscArcTest(CoverageTest):
     """Miscellaneous arc-measuring tests."""
 
@@ -1127,21 +1223,6 @@ class MiscArcTest(CoverageTest):
             arcs_missing=[], arcs_unpredicted=[],
             )
 
-    def test_optimized_away_lines(self):
-        self.check_coverage("""\
-            a = 1
-            if len([2]):
-                c = 3
-            if 0:               # this line isn't in the compiled code.
-                if len([5]):
-                    d = 6
-            e = 7
-            """,
-            lines=[1, 2, 3, 7],
-            arcz=".1 12 23 27 37 7.",
-            arcz_missing="27",
-        )
-
     def test_partial_generators(self):
         # https://bitbucket.org/ned/coveragepy/issues/475/generator-expression-is-marked-as-not
         # Line 2 is executed completely.
diff --git a/tests/test_parser.py b/tests/test_parser.py
index 838d7814..0053e932 100644
--- a/tests/test_parser.py
+++ b/tests/test_parser.py
@@ -193,7 +193,7 @@ class ParserMissingArcDescriptionTest(CoverageTest):
 
     def parse_text(self, source):
         """Parse Python source, and return the parser object."""
-        parser = PythonParser(textwrap.dedent(source))
+        parser = PythonParser(text=textwrap.dedent(source))
         parser.parse_source()
         return parser