From 35350e11cd227ff74fdfdcc856dad10b74fda80b Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Sat, 12 Sep 2020 22:22:29 +0300 Subject: [PATCH 1/2] assertion/rewrite: rewrite condition to be easier to follow --- src/_pytest/assertion/rewrite.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/_pytest/assertion/rewrite.py b/src/_pytest/assertion/rewrite.py index 48587b17e..3a7c5f6f2 100644 --- a/src/_pytest/assertion/rewrite.py +++ b/src/_pytest/assertion/rewrite.py @@ -687,15 +687,15 @@ class AssertionRewriter(ast.NodeVisitor): return expect_docstring = False elif ( - not isinstance(item, ast.ImportFrom) - or item.level > 0 - or item.module != "__future__" + isinstance(item, ast.ImportFrom) + and item.level == 0 + and item.module == "__future__" ): - lineno = item.lineno + pass + else: break pos += 1 - else: - lineno = item.lineno + lineno = item.lineno imports = [ ast.Import([alias], lineno=lineno, col_offset=0) for alias in aliases ] From d18cb961cfc57b9b433e67ece546c22f468a07fa Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Sat, 12 Sep 2020 22:28:19 +0300 Subject: [PATCH 2/2] assertion/rewrite: fix internal error on collection error due to decorated function For decorated functions, the lineno of the FunctionDef AST node points to the `def` line, not to the first decorator line. On the other hand, in code objects, the `co_firstlineno` points to the first decorator line. Assertion rewriting inserts some imports to code it rewrites. The imports are inserted at the lineno of the first statement in the AST. In turn, the code object compiled from the rewritten AST uses the lineno of the first statement (which is the first inserted import). This means that given a module like this, ```py @foo @bar def baz(): pass ``` the lineno of the code object without assertion rewriting (`--assertion=plain`) is 1, but with assertion rewriting it is 3. And *this* causes some issues for the exception repr when e.g. the decorator line is invalid and raises during collection. The code becomes confused and crashes with INTERNALERROR> File "_pytest/_code/code.py", line 638, in get_source INTERNALERROR> lines.append(space_prefix + source.lines[line_index].strip()) INTERNALERROR> IndexError: list index out of range Fix it by special casing decorators. Maybe there are other cases like this but off hand I can't think of another Python construct where the lineno of the item would be after its first line, and this is the only such issue we have had reported. --- changelog/4984.bugfix.rst | 3 +++ src/_pytest/assertion/rewrite.py | 7 ++++++- testing/test_collection.py | 14 ++++++++++++++ 3 files changed, 23 insertions(+), 1 deletion(-) create mode 100644 changelog/4984.bugfix.rst diff --git a/changelog/4984.bugfix.rst b/changelog/4984.bugfix.rst new file mode 100644 index 000000000..869a9f244 --- /dev/null +++ b/changelog/4984.bugfix.rst @@ -0,0 +1,3 @@ +Fixed an internal error crash with ``IndexError: list index out of range`` when +collecting a module which starts with a decorated function, the decorator +raises, and assertion rewriting is enabled. diff --git a/src/_pytest/assertion/rewrite.py b/src/_pytest/assertion/rewrite.py index 3a7c5f6f2..5ff578245 100644 --- a/src/_pytest/assertion/rewrite.py +++ b/src/_pytest/assertion/rewrite.py @@ -695,7 +695,12 @@ class AssertionRewriter(ast.NodeVisitor): else: break pos += 1 - lineno = item.lineno + # Special case: for a decorated function, set the lineno to that of the + # first decorator, not the `def`. Issue #4984. + if isinstance(item, ast.FunctionDef) and item.decorator_list: + lineno = item.decorator_list[0].lineno + else: + lineno = item.lineno imports = [ ast.Import([alias], lineno=lineno, col_offset=0) for alias in aliases ] diff --git a/testing/test_collection.py b/testing/test_collection.py index 12030e56e..3e1b816b7 100644 --- a/testing/test_collection.py +++ b/testing/test_collection.py @@ -1393,3 +1393,17 @@ class TestImportModeImportlib: "* 1 failed in *", ] ) + + +def test_does_not_crash_on_error_from_decorated_function(testdir: Testdir) -> None: + """Regression test for an issue around bad exception formatting due to + assertion rewriting mangling lineno's (#4984).""" + testdir.makepyfile( + """ + @pytest.fixture + def a(): return 4 + """ + ) + result = testdir.runpytest() + # Not INTERNAL_ERROR + assert result.ret == ExitCode.INTERRUPTED