fix: avoid measuring generated code #1160

CHANGES.rst

@@ -30,6 +30,12 @@ Unreleased
 - Feature: The HTML report pages for Python source files now have a sticky
   header so the file name and controls are always visible.
 
+- Fix: more generated code is now excluded from measurement.  Code such as
+  `attrs`_ boilerplate, or doctest code, was being measured though the
+  synthetic line numbers meant they were never reported.  Once Cython was
+  involved though, the generated .so files were parsed as Python, raising
+  syntax errors, as reported in `issue 1160`_.  This is now fixed.
+
 - Fix: When sorting human-readable names, numeric components are sorted
   correctly: file10.py will appear after file9.py.  This applies to file names,
   module names, environment variables, and test contexts.
@@ -41,6 +47,8 @@ Unreleased
 
 .. _issue 553: https://github.com/nedbat/coveragepy/issues/553
 .. _issue 840: https://github.com/nedbat/coveragepy/issues/840
+.. _issue 1160: https://github.com/nedbat/coveragepy/issues/1160
+.. _attrs: https://www.attrs.org/
 
 
 .. _changes_602:

coverage/ctracer/stats.h

@@ -17,10 +17,8 @@ typedef struct Stats {
 #if COLLECT_STATS
     unsigned int lines;
     unsigned int returns;
-    unsigned int exceptions;
     unsigned int others;
     unsigned int files;
-    unsigned int missed_returns;
     unsigned int stack_reallocs;
     unsigned int errors;
     unsigned int pycalls;

coverage/ctracer/tracer.c

@@ -295,48 +295,6 @@ CTracer_set_pdata_stack(CTracer *self)
  * Parts of the trace function.
  */
 
-static int
-CTracer_check_missing_return(CTracer *self, PyFrameObject *frame)
-{
-    int ret = RET_ERROR;
-
-    if (self->last_exc_back) {
-        if (frame == self->last_exc_back) {
-            /* Looks like someone forgot to send a return event. We'll clear
-               the exception state and do the RETURN code here.  Notice that the
-               frame we have in hand here is not the correct frame for the RETURN,
-               that frame is gone.  Our handling for RETURN doesn't need the
-               actual frame, but we do log it, so that will look a little off if
-               you're looking at the detailed log.
-
-               If someday we need to examine the frame when doing RETURN, then
-               we'll need to keep more of the missed frame's state.
-            */
-            STATS( self->stats.missed_returns++; )
-            if (CTracer_set_pdata_stack(self) < 0) {
-                goto error;
-            }
-            if (self->pdata_stack->depth >= 0) {
-                if (self->tracing_arcs && self->pcur_entry->file_data) {
-                    if (CTracer_record_pair(self, self->pcur_entry->last_line, -self->last_exc_firstlineno) < 0) {
-                        goto error;
-                    }
-                }
-                SHOWLOG(PyFrame_GetLineNumber(frame), MyFrame_GetCode(frame)->co_filename, "missedreturn");
-                self->pdata_stack->depth--;
-                self->pcur_entry = &self->pdata_stack->stack[self->pdata_stack->depth];
-            }
-        }
-        self->last_exc_back = NULL;
-    }
-
-    ret = RET_OK;
-
-error:
-
-    return ret;
-}
-
 static int
 CTracer_handle_call(CTracer *self, PyFrameObject *frame)
 {
@@ -773,30 +731,6 @@ CTracer_handle_return(CTracer *self, PyFrameObject *frame)
     return ret;
 }
 
-static int
-CTracer_handle_exception(CTracer *self, PyFrameObject *frame)
-{
-    /* Some code (Python 2.3, and pyexpat anywhere) fires an exception event
-        without a return event.  To detect that, we'll keep a copy of the
-        parent frame for an exception event.  If the next event is in that
-        frame, then we must have returned without a return event.  We can
-        synthesize the missing event then.
-
-        Python itself fixed this problem in 2.4.  Pyexpat still has the bug.
-        I've reported the problem with pyexpat as http://bugs.python.org/issue6359 .
-        If it gets fixed, this code should still work properly.  Maybe some day
-        the bug will be fixed everywhere coverage.py is supported, and we can
-        remove this missing-return detection.
-
-        More about this fix: https://nedbatchelder.com/blog/200907/a_nasty_little_bug.html
-    */
-    STATS( self->stats.exceptions++; )
-    self->last_exc_back = frame->f_back;
-    self->last_exc_firstlineno = MyFrame_GetCode(frame)->co_firstlineno;
-
-    return RET_OK;
-}
-
 /*
  * The Trace Function
  */
@@ -837,11 +771,6 @@ CTracer_trace(CTracer *self, PyFrameObject *frame, int what, PyObject *arg_unuse
     Py_DECREF(ascii);
     #endif
 
-    /* See below for details on missing-return detection. */
-    if (CTracer_check_missing_return(self, frame) < 0) {
-        goto error;
-    }
-
     self->activity = TRUE;
 
     switch (what) {
@@ -863,12 +792,6 @@ CTracer_trace(CTracer *self, PyFrameObject *frame, int what, PyObject *arg_unuse
         }
         break;
 
-    case PyTrace_EXCEPTION:
-        if (CTracer_handle_exception(self, frame) < 0) {
-            goto error;
-        }
-        break;
-
     default:
         STATS( self->stats.others++; )
         break;
@@ -1050,10 +973,8 @@ CTracer_get_stats(CTracer *self, PyObject *args_unused)
         "calls", self->stats.calls,
         "lines", self->stats.lines,
         "returns", self->stats.returns,
-        "exceptions", self->stats.exceptions,
         "others", self->stats.others,
         "files", self->stats.files,
-        "missed_returns", self->stats.missed_returns,
         "stack_reallocs", self->stats.stack_reallocs,
         "stack_alloc", self->pdata_stack->alloc,
         "errors", self->stats.errors,

coverage/ctracer/tracer.h

@@ -60,10 +60,6 @@ typedef struct CTracer {
     /* The current file's data stack entry. */
     DataStackEntry * pcur_entry;
 
-    /* The parent frame for the last exception event, to fix missing returns. */
-    PyFrameObject * last_exc_back;
-    int last_exc_firstlineno;
-
     Stats stats;
 } CTracer;
 

coverage/disposition.py

@@ -30,6 +30,8 @@ def disposition_debug_msg(disp):
     """Make a nice debug message of what the FileDisposition is doing."""
     if disp.trace:
         msg = f"Tracing {disp.original_filename!r}"
+        if disp.original_filename != disp.source_filename:
+            msg += f" as {disp.source_filename!r}"
         if disp.file_tracer:
             msg += ": will be traced by %r" % disp.file_tracer
     else:

coverage/inorout.py

@@ -317,6 +317,9 @@ def nope(disp, reason):
             disp.reason = reason
             return disp
 
+        if original_filename.startswith('<'):
+            return nope(disp, "not a real original file name")
+
         if frame is not None:
             # Compiled Python files have two file names: frame.f_code.co_filename is
             # the file name at the time the .pyc was compiled.  The second name is
@@ -349,11 +352,6 @@ def nope(disp, reason):
             # can't do anything with the data later anyway.
             return nope(disp, "not a real file name")
 
-        # pyexpat does a dumb thing, calling the trace function explicitly from
-        # C code with a C file name.
-        if re.search(r"[/\\]Modules[/\\]pyexpat.c", filename):
-            return nope(disp, "pyexpat lies about itself")
-
         # Jython reports the .class file to the tracer, use the source file.
         if filename.endswith("$py.class"):
             filename = filename[:-9] + ".py"

coverage/pytracer.py

@@ -55,8 +55,6 @@ def __init__(self):
         self.started_context = False
 
         self.data_stack = []
-        self.last_exc_back = None
-        self.last_exc_firstlineno = 0
         self.thread = None
         self.stopped = False
         self._activity = False
@@ -118,17 +116,6 @@ def _trace(self, frame, event, arg_unused):
             )
             return None
 
-        if self.last_exc_back:
-            if frame == self.last_exc_back:
-                # Someone forgot a return event.
-                if self.trace_arcs and self.cur_file_data:
-                    pair = (self.last_line, -self.last_exc_firstlineno)
-                    self.cur_file_data.add(pair)
-                self.cur_file_data, self.cur_file_name, self.last_line, self.started_context = (
-                    self.data_stack.pop()
-                )
-            self.last_exc_back = None
-
         # if event != 'call' and frame.f_code.co_filename != self.cur_file_name:
         #     self.log("---\n*", frame.f_code.co_filename, self.cur_file_name, frame.f_lineno)
 
@@ -204,9 +191,6 @@ def _trace(self, frame, event, arg_unused):
             if self.started_context:
                 self.context = None
                 self.switch_context(None)
-        elif event == 'exception':
-            self.last_exc_back = frame.f_back
-            self.last_exc_firstlineno = frame.f_code.co_firstlineno
         return self._trace
 
     def start(self):

tests/test_oddball.py

@@ -393,7 +393,13 @@ class DoctestTest(CoverageTest):
     """Tests invoked with doctest should measure properly."""
 
     def test_doctest(self):
-        self.check_coverage('''\
+        # Doctests used to be traced, with their line numbers credited to the
+        # file they were in.  Below, one of the doctests has four lines (1-4),
+        # which would incorrectly claim that lines 1-4 of the file were
+        # executed.  In this file, line 2 is not executed.
+        self.make_file("the_doctest.py", '''\
+            if "x" in "abc":
+                print("hello")
             def return_arg_or_void(arg):
                 """If <arg> is None, return "Void"; otherwise return <arg>
 
@@ -403,6 +409,11 @@ def return_arg_or_void(arg):
                 'arg'
                 >>> return_arg_or_void("None")
                 'None'
+                >>> if "x" in "xyz":                # line 1
+                ...   if "a" in "aswed":            # line 2
+                ...      if "a" in "abc":           # line 3
+                ...         return_arg_or_void(12)  # line 4
+                12
                 """
                 if arg is None:
                     return "Void"
@@ -411,8 +422,13 @@ def return_arg_or_void(arg):
 
             import doctest, sys
             doctest.testmod(sys.modules[__name__])  # we're not __main__ :(
-            ''',
-            [1, 11, 12, 14, 16, 17], "")
+            ''')
+        cov = coverage.Coverage()
+        self.start_import_stop(cov, "the_doctest")
+        data = cov.get_data()
+        assert len(data.measured_files()) == 1
+        lines = data.lines(data.measured_files().pop())
+        assert lines == [1, 3, 18, 19, 21, 23, 24]
 
 
 class GettraceTest(CoverageTest):