diff --git a/changelog/14994.improvement.rst b/changelog/14994.improvement.rst new file mode 100644 index 00000000000..be392b14b63 --- /dev/null +++ b/changelog/14994.improvement.rst @@ -0,0 +1,4 @@ +The redundant ``File "...", line N`` / source / caret block is no longer shown +in ``SyntaxError`` output when the crash line already reports the error's own +``FILE:LINE:COLUMN`` location. The block is still shown as a fallback when the +error does not carry its own location, so no information is lost. diff --git a/changelog/2388.improvement.rst b/changelog/2388.improvement.rst new file mode 100644 index 00000000000..c257c7c72d0 --- /dev/null +++ b/changelog/2388.improvement.rst @@ -0,0 +1 @@ +:class:`SyntaxError` crash lines now report the error's own `FILE:LINE:COLUMN` location instead of the traceback entry that raised it, in the format most editors/IDEs understand. diff --git a/src/_pytest/_code/code.py b/src/_pytest/_code/code.py index e7712c48bf4..b867aeb64cf 100644 --- a/src/_pytest/_code/code.py +++ b/src/_pytest/_code/code.py @@ -496,6 +496,45 @@ def stringify_exception( E = TypeVar("E", bound=BaseException, covariant=True) +def _syntax_error_location(exc: BaseException) -> tuple[str, int, int, str] | None: + """Return (filename, lineno, offset, message) for a SyntaxError with location info, else None.""" + if ( + isinstance(exc, SyntaxError) + and exc.offset + and exc.lineno is not None + and exc.filename + ): + return ( + exc.filename, + exc.lineno, + exc.offset, + exc.msg or "", + ) + return None + + +def _strip_syntax_error_file_block(exlines: list[str], exc: BaseException) -> list[str]: + """Drop the leading ``File "...", line N`` / source / caret lines from + :func:`traceback.format_exception_only` output for :class:`SyntaxError`. + + The exception line itself (and any lines after it, such as notes) is kept + verbatim. If the exception line cannot be found, the lines are returned + unchanged. + """ + # Mirror the exception type name computation of + # traceback.format_exception_only(). + stype = type(exc).__qualname__ + smod = type(exc).__module__ + if smod not in ("__main__", "builtins"): + stype = f"{smod}.{stype}" + for i, line in enumerate(exlines): + # The location block lines are always indented; the exception line + # starts at column 0 with the exception type name. + if line.startswith(stype): + return exlines[i:] + return exlines + + @final @dataclasses.dataclass class ExceptionInfo(Generic[E]): @@ -689,6 +728,17 @@ def errisinstance(self, exc: EXCEPTION_OR_MORE) -> bool: return isinstance(self.value, exc) def _getreprcrash(self) -> ReprFileLocation | None: + # A SyntaxError carries its own location, which is more useful than + # the traceback entry where it was raised (#2388). + loc = _syntax_error_location(self.value) + if loc is not None: + filename, lineno, offset, message = loc + return ReprFileLocation( + filename, + lineno, + f"{self.typename}: {message}", + column=offset, + ) # Find last non-hidden traceback entry that led to the exception of the # traceback, or None if all hidden. for i in range(-1, -len(self.traceback) - 1, -1): @@ -1031,6 +1081,12 @@ def get_exconly( indentstr = " " * indent # Get the real exception information out. exlines = excinfo.exconly(tryshort=True).split("\n") + if _syntax_error_location(excinfo.value) is not None: + # The crash line already reports the error's own file:line:column + # location (see ExceptionInfo._getreprcrash), so the File/source/ + # caret block emitted by format_exception_only() would only + # repeat it (#14994). + exlines = _strip_syntax_error_file_block(exlines, excinfo.value) failindent = self.fail_marker + indentstr[1:] for line in exlines: lines.append(failindent + line) @@ -1495,7 +1551,7 @@ def __str__(self) -> str: @dataclasses.dataclass(eq=False) class ReprFileLocation(TerminalRepr): - """A message at a file location, using the `:: ` + """A message at a file location, using the `:[:]: ` format that most editors understand. Only the first line of the message is emitted. @@ -1504,6 +1560,7 @@ class ReprFileLocation(TerminalRepr): path: str lineno: int message: str + column: int | None = None def __post_init__(self) -> None: self.path = str(self.path) @@ -1514,7 +1571,8 @@ def toterminal(self, tw: TerminalWriter) -> None: if i != -1: msg = msg[:i] tw.write(self.path, bold=True, red=True) - tw.line(f":{self.lineno}: {msg}") + column = f":{self.column}" if self.column is not None else "" + tw.line(f":{self.lineno}{column}: {msg}") @dataclasses.dataclass(eq=False) diff --git a/src/_pytest/python.py b/src/_pytest/python.py index bc3d55243f7..b76006f12c8 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -532,9 +532,15 @@ def importtestmodule( consider_namespace_packages=config.getini("consider_namespace_packages"), ) except SyntaxError as e: - raise nodes.Collector.CollectError( - ExceptionInfo.from_current().getrepr(style="short") - ) from e + excinfo = ExceptionInfo.from_current() + repr_ = excinfo.getrepr(style="short") + reprcrash = excinfo._getreprcrash() + msg = str(repr_) + # A column is present only when the SyntaxError carried its own + # location; a fallback crash line would just repeat the last entry. + if reprcrash is not None and reprcrash.column is not None: + msg += "\n" + str(reprcrash) + raise nodes.Collector.CollectError(msg) from e except ImportPathMismatchError as e: raise nodes.Collector.CollectError( "import file mismatch:\n" diff --git a/testing/code/test_excinfo.py b/testing/code/test_excinfo.py index ec9f584dfba..c717e520132 100644 --- a/testing/code/test_excinfo.py +++ b/testing/code/test_excinfo.py @@ -15,6 +15,7 @@ from typing import TYPE_CHECKING import _pytest._code +from _pytest._code.code import _strip_syntax_error_file_block from _pytest._code.code import ExceptionChainRepr from _pytest._code.code import ExceptionInfo from _pytest._code.code import ExceptionInfoFormatter @@ -339,6 +340,94 @@ def f(): f() assert excinfo._getreprcrash() is None + def test_getreprcrash_syntax_error(self): + with pytest.raises(SyntaxError) as excinfo: + raise SyntaxError("bad syntax", ("file.py", 1, 5, "def foo(:", 1, 6)) + reprcrash = excinfo._getreprcrash() + assert reprcrash is not None + assert reprcrash.path == "file.py" + assert reprcrash.lineno == 1 + assert reprcrash.column == 5 + assert reprcrash.message == "SyntaxError: bad syntax" + assert str(reprcrash) == "file.py:1:5: SyntaxError: bad syntax" + + def test_getreprcrash_syntax_error_none_msg(self): + with pytest.raises(SyntaxError) as excinfo: + err = SyntaxError("bad syntax", ("file.py", 1, 5, "def foo(:", 1, 6)) + err.msg = cast(Any, None) + raise err + reprcrash = excinfo._getreprcrash() + assert reprcrash is not None + assert reprcrash.message == "SyntaxError: " + assert str(reprcrash) == "file.py:1:5: SyntaxError: " + + def test_getreprcrash_syntax_error_without_offset(self): + def f(): + raise SyntaxError("no location") + + with pytest.raises(SyntaxError) as excinfo: + f() + reprcrash = excinfo._getreprcrash() + assert reprcrash is not None + assert reprcrash.column is None + assert reprcrash.message == "SyntaxError: no location" + + def test_getreprcrash_syntax_error_without_filename(self): + def f(): + raise SyntaxError("bad syntax", (None, 1, 5, "def foo(:", 1, 6)) + + with pytest.raises(SyntaxError) as excinfo: + f() + reprcrash = excinfo._getreprcrash() + assert reprcrash is not None + co = _pytest._code.Code.from_function(f) + assert reprcrash.path == str(co.path) + assert reprcrash.lineno == co.firstlineno + 1 + 1 + assert reprcrash.column is None + assert reprcrash.message.endswith("SyntaxError: bad syntax") + + def test_getreprcrash_indentation_error(self): + with pytest.raises(IndentationError) as excinfo: + raise IndentationError( + "unexpected indent", ("file.py", 3, 5, " foo", 3, 6) + ) + reprcrash = excinfo._getreprcrash() + assert reprcrash is not None + assert reprcrash.path == "file.py" + assert reprcrash.lineno == 3 + assert reprcrash.column == 5 + assert reprcrash.message == "IndentationError: unexpected indent" + assert str(reprcrash) == "file.py:3:5: IndentationError: unexpected indent" + + def test_getreprcrash_syntax_error_without_lineno(self): + def f(): + raise SyntaxError( + "bad syntax", ("file.py", None, 5, "def foo(:", None, None) + ) + + with pytest.raises(SyntaxError) as excinfo: + f() + reprcrash = excinfo._getreprcrash() + assert reprcrash is not None + assert reprcrash.column is None + co = _pytest._code.Code.from_function(f) + assert reprcrash.path == str(co.path) + assert reprcrash.lineno == co.firstlineno + 1 + 1 + + def test_getreprcrash_syntax_error_zero_offset(self): + def f(): + raise SyntaxError("bad syntax", ("file.py", 1, 0, "def foo(:", 1, 0)) + + with pytest.raises(SyntaxError) as excinfo: + f() + reprcrash = excinfo._getreprcrash() + assert reprcrash is not None + assert reprcrash.column is None + co = _pytest._code.Code.from_function(f) + assert reprcrash.path == str(co.path) + assert reprcrash.lineno == co.firstlineno + 1 + 1 + assert ":0:" not in str(reprcrash) + def test_excinfo_exconly(): with pytest.raises(ValueError) as excinfo: @@ -782,7 +871,7 @@ def func1(m): reprfuncargs = p.repr_args(entry) assert reprfuncargs is not None assert reprfuncargs.args[0] == ("m", repr("m" * 500)) - assert "..." not in reprfuncargs.args[0][1] + assert "..." not in str(reprfuncargs.args[0][1]) def test_repr_tracebackentry_lines(self, importasmod) -> None: mod = importasmod( @@ -1116,6 +1205,138 @@ def entry(): assert repr.reprcrash.message == "ValueError" assert str(repr.reprcrash).endswith("mod.py:3: ValueError") + def test_repr_excinfo_reprcrash_syntax_error(self, importasmod) -> None: + mod = importasmod( + """ + def entry(): + raise SyntaxError("bad syntax", ("file.py", 1, 5, "def foo(:", 1, 6)) + """ + ) + with pytest.raises(SyntaxError) as excinfo: + mod.entry() + repr = excinfo.getrepr() + assert repr.reprcrash is not None + assert repr.reprcrash.path == "file.py" + assert repr.reprcrash.lineno == 1 + assert repr.reprcrash.column == 5 + assert repr.reprcrash.message == "SyntaxError: bad syntax" + assert str(repr.reprcrash) == "file.py:1:5: SyntaxError: bad syntax" + + def test_syntax_error_default_tb_long(self, pytester: Pytester) -> None: + pytester.makepyfile( + """ + def entry(): + raise SyntaxError("bad syntax", ("file.py", 1, 5, "def foo(:", 1, 6)) + def test_x(): + entry() + """ + ) + result = pytester.runpytest("--tb=long") + result.stdout.fnmatch_lines(["*.py:2: SyntaxError"]) + + def test_syntax_error_default_tb_short(self, pytester: Pytester) -> None: + pytester.makepyfile( + """ + def entry(): + raise SyntaxError("bad syntax", ("file.py", 1, 5, "def foo(:", 1, 6)) + def test_x(): + entry() + """ + ) + result = pytester.runpytest("--tb=short") + result.stdout.fnmatch_lines(["*.py:2: in entry"]) + result.stdout.fnmatch_lines(["*SyntaxError: bad syntax*"]) + + def test_syntax_error_tb_line(self, pytester: Pytester) -> None: + pytester.makepyfile( + """ + def entry(): + raise SyntaxError("bad syntax", ("file.py", 1, 5, "def foo(:", 1, 6)) + def test_x(): + entry() + """ + ) + result = pytester.runpytest("--tb=line") + result.stdout.fnmatch_lines(["file.py:1:5: SyntaxError: bad syntax"]) + + def test_syntax_error_no_offset_fallback(self, pytester: Pytester) -> None: + pytester.makepyfile( + """ + def entry(): + raise SyntaxError("no location") + def test_x(): + entry() + """ + ) + result = pytester.runpytest("--tb=long") + result.stdout.fnmatch_lines(["*SyntaxError: no location*"]) + + def test_syntax_error_collection(self, pytester: Pytester) -> None: + pytester.makepyfile("def broken(:\n pass\n") + result = pytester.runpytest() + result.stdout.fnmatch_lines(["*.py:1:*: SyntaxError*"]) + + def test_indentation_error_collection(self, pytester: Pytester) -> None: + pytester.makepyfile("def f():\n x = 1\n y = 2\n") + result = pytester.runpytest() + result.stdout.fnmatch_lines(["*.py:3:*: IndentationError*"]) + + def test_get_exconly_syntax_error_strips_file_block(self) -> None: + # The File/source/caret block is redundant when the crash line already + # carries the error's own file:line:column (#14994). + with pytest.raises(SyntaxError) as excinfo: + raise SyntaxError("bad syntax", ("file.py", 1, 5, "def foo(:", 1, 6)) + lines = ExceptionInfoFormatter().get_exconly(excinfo) + assert lines == ["E SyntaxError: bad syntax"] + + def test_get_exconly_syntax_error_keeps_file_block_on_fallback(self) -> None: + # Without column info the crash line falls back to the traceback entry, + # so the File block must be kept to not lose the location (#14994). + with pytest.raises(SyntaxError) as excinfo: + raise SyntaxError("bad syntax", ("file.py", 1, None, "def foo(:", 1, None)) + lines = ExceptionInfoFormatter().get_exconly(excinfo) + assert lines == [ + 'E File "file.py", line 1', + " def foo(:", + " SyntaxError: bad syntax", + ] + + def test_strip_syntax_error_file_block_module_prefixed_type(self) -> None: + # Mirrors traceback.format_exception_only(): the exception type name + # is module-prefixed for non-builtin exception types. + class PluginSyntaxError(SyntaxError): + pass + + PluginSyntaxError.__module__ = "some_plugin" + exc = PluginSyntaxError("bad", ("file.py", 1, 1, "def foo(:", 1, 2)) + type_name = f"{type(exc).__module__}.{type(exc).__qualname__}" + lines = [ + ' File "file.py", line 1', + " def foo(:", + " ^", + f"{type_name}: bad", + ] + assert _strip_syntax_error_file_block(lines, exc) == [f"{type_name}: bad"] + + def test_strip_syntax_error_file_block_returns_unchanged_without_match( + self, + ) -> None: + # If the exception line cannot be found, the lines are kept verbatim. + exc = SyntaxError("bad", ("file.py", 1, 1, "def foo(:", 1, 2)) + lines = ["something unexpected"] + assert _strip_syntax_error_file_block(lines, exc) == lines + + def test_syntax_error_collection_omits_redundant_file_block( + self, pytester: Pytester + ) -> None: + pytester.makepyfile("def broken(:\n pass\n") + result = pytester.runpytest() + # The crash line carries file:line:column ... + result.stdout.fnmatch_lines(["*.py:1:*: SyntaxError: invalid syntax"]) + # ... so the redundant File/source/caret E-block is omitted (#14994). + result.stdout.no_fnmatch_line('E*File "*", line *') + result.stdout.fnmatch_lines(["E SyntaxError: invalid syntax"]) + def test_repr_traceback_recursion(self, importasmod): mod = importasmod( """ diff --git a/testing/test_session.py b/testing/test_session.py index 94738837831..8975f94a0e3 100644 --- a/testing/test_session.py +++ b/testing/test_session.py @@ -78,7 +78,12 @@ def test_syntax_error_module(self, pytester: Pytester) -> None: values = reprec.getfailedcollections() assert len(values) == 1 out = str(values[0].longrepr) - assert out.find("not python") != -1 + # The crash line carries the SyntaxError's own file:line:column + # location (#2388); the redundant File/source/caret block is + # omitted (#14994). + assert "test_syntax_error_module.py:1:" in out + assert "SyntaxError: invalid syntax" in out + assert 'File "' not in out def test_exit_first_problem(self, pytester: Pytester) -> None: reprec = pytester.inline_runsource(