From ae9f90be4926877954fb00b4ae36c523476fc8f5 Mon Sep 17 00:00:00 2001 From: mdevolde Date: Sat, 4 Jul 2026 01:03:12 +0300 Subject: [PATCH] fix: fix warns catching and warns stacklevels --- src/language_tool_python/__main__.py | 81 +++++++++++--------- src/language_tool_python/_internals/utils.py | 34 +++++++- src/language_tool_python/download_lt.py | 35 +-------- src/language_tool_python/server.py | 5 +- tests/unit/test_download.py | 21 +++-- tests/unit/test_internals_utils.py | 39 +++++++++- 6 files changed, 135 insertions(+), 80 deletions(-) diff --git a/src/language_tool_python/__main__.py b/src/language_tool_python/__main__.py index 8b5cc27..93313c5 100644 --- a/src/language_tool_python/__main__.py +++ b/src/language_tool_python/__main__.py @@ -8,6 +8,7 @@ import re import sys import traceback +import warnings from importlib.metadata import PackageNotFoundError, version from logging.config import dictConfig from pathlib import Path @@ -374,55 +375,61 @@ def process_file( print(filename, file=sys.stderr) try: - with LanguageTool( - language=args.language, - mother_tongue=args.mother_tongue, - remote_server=remote_server, - ) as lang_tool: - try: - text = get_input_text(filename, args) - except (UnicodeError, FileNotFoundError) as exception: - print_exception(exception, args.verbose) - return 0 + with warnings.catch_warnings(): + warnings.filterwarnings( + "ignore", + message="No scheme was specified in the URL", + category=RuntimeWarning, + ) + with LanguageTool( + language=args.language, + mother_tongue=args.mother_tongue, + remote_server=remote_server, + ) as lang_tool: + try: + text = get_input_text(filename, args) + except (UnicodeError, FileNotFoundError) as exception: + print_exception(exception, args.verbose) + return 0 - if not args.spell_check: - lang_tool.disable_spellchecking() + if not args.spell_check: + lang_tool.disable_spellchecking() - lang_tool.disabled_rules.update(args.disable) - lang_tool.enabled_rules.update(args.enable) - lang_tool.disabled_categories.update(args.disable_categories) - lang_tool.enabled_categories.update(args.enable_categories) - lang_tool.enabled_rules_only = args.enabled_only + lang_tool.disabled_rules.update(args.disable) + lang_tool.enabled_rules.update(args.enable) + lang_tool.disabled_categories.update(args.disable_categories) + lang_tool.enabled_categories.update(args.enable_categories) + lang_tool.enabled_rules_only = args.enabled_only - if args.picky: - lang_tool.picky = True + if args.picky: + lang_tool.picky = True - if args.apply: - print(lang_tool.correct(text)) - return 0 + if args.apply: + print(lang_tool.correct(text)) + return 0 - status = 0 - for match in lang_tool.check(text): - rule_id = match.rule_id + status = 0 + for match in lang_tool.check(text): + rule_id = match.rule_id - replacement_text = ", ".join( - f"'{word}'" for word in match.replacements - ).strip() + replacement_text = ", ".join( + f"'{word}'" for word in match.replacements + ).strip() - message = match.message + message = match.message - # Messages that end with punctuation already include the - # suggestion. - if replacement_text and not message.endswith("?"): - message += " Suggestions: " + replacement_text + # Messages that end with punctuation already include the + # suggestion. + if replacement_text and not message.endswith("?"): + message += " Suggestions: " + replacement_text - line, column = match.get_line_and_column(text) + line, column = match.get_line_and_column(text) - print(f"{filename}:{line}:{column}: {rule_id}: {message}") + print(f"{filename}:{line}:{column}: {rule_id}: {message}") - status = 2 + status = 2 - return status + return status except LanguageToolError as exception: print_exception(exception, args.verbose) return 0 diff --git a/src/language_tool_python/_internals/utils.py b/src/language_tool_python/_internals/utils.py index 170ff1a..32d4e2e 100644 --- a/src/language_tool_python/_internals/utils.py +++ b/src/language_tool_python/_internals/utils.py @@ -1,4 +1,5 @@ import contextlib +import inspect import locale import logging import math @@ -15,6 +16,7 @@ __all__ = [ "SupportsBool", + "external_stacklevel", "get_env_float", "get_env_int", "get_language_tool_download_path", @@ -39,6 +41,36 @@ # (RFC 3986's scheme:opaque form, e.g. "ftp:example.com" or "mailto:a@b.com"). _EXPLICIT_SCHEME_NO_SLASH_RE = re.compile(r"^([a-zA-Z][a-zA-Z0-9+.-]*):(?!\d)") +_PACKAGE_DIR = Path(__file__).resolve().parent.parent + + +def external_stacklevel() -> int: + """Compute the stacklevel of the first caller located outside this package. + + The call chain leading into a warning here varies in depth depending on how it is + reached (e.g. through ``LanguageTool.__init__()`` versus calling + ``LocalLanguageTool.download()`` directly), so a hardcoded stacklevel would point + to the wrong line, or nothing at all, depending on the caller. Walking the stack + until leaving this package keeps the warning attributed to the caller's code. + + :return: The stacklevel to pass to :func:`warnings.warn`, from the perspective of + the caller of this function. + :rtype: int + """ + frame = inspect.currentframe() + try: + frame = frame.f_back if frame is not None else None + stacklevel = 1 + while frame is not None: + frame_dir = Path(frame.f_code.co_filename).resolve().parent + if not frame_dir.is_relative_to(_PACKAGE_DIR): + break + frame = frame.f_back + stacklevel += 1 + return stacklevel + finally: + del frame # Avoid reference cycles + def _check_supported_scheme(scheme: str) -> None: """Raise ValueError if ``scheme`` is not one of the supported URL schemes.""" @@ -85,7 +117,7 @@ def parse_url(url_str: str) -> str: "scheme was used by default. It is recommended to explicitly set the " "protocol ('http://' or 'https://') yourself in the URL.", RuntimeWarning, - stacklevel=2, + stacklevel=external_stacklevel(), ) parsed = urllib.parse.urlparse(stripped) diff --git a/src/language_tool_python/download_lt.py b/src/language_tool_python/download_lt.py index 432bc93..d1e2df6 100644 --- a/src/language_tool_python/download_lt.py +++ b/src/language_tool_python/download_lt.py @@ -5,7 +5,6 @@ import contextlib import hashlib import importlib.resources -import inspect import logging import os import re @@ -27,6 +26,7 @@ from ._internals.compat import toml_loads from ._internals.safe_zip import SafeZipExtractor from ._internals.utils import ( + external_stacklevel, get_env_int, get_language_tool_download_path, version_tuple, @@ -84,35 +84,6 @@ _LTP_JAR_DIR_PATH_ENV_VAR = "LTP_JAR_DIR_PATH" _DOWNLOAD_CHUNK_BYTES = 1024 * 1024 _SAFE_ZIP_EXTRACTOR = SafeZipExtractor() -_PACKAGE_DIR = Path(__file__).resolve().parent - - -def _external_stacklevel() -> int: - """Compute the stacklevel of the first caller located outside this package. - - The call chain leading into a warning here varies in depth depending on how it is - reached (e.g. through ``LanguageTool.__init__()`` versus calling - ``LocalLanguageTool.download()`` directly), so a hardcoded stacklevel would point - to the wrong line, or nothing at all, depending on the caller. Walking the stack - until leaving this package keeps the warning attributed to the caller's code. - - :return: The stacklevel to pass to :func:`warnings.warn`, from the perspective of - the caller of this function. - :rtype: int - """ - frame = inspect.currentframe() - try: - frame = frame.f_back if frame is not None else None - stacklevel = 1 - while frame is not None: - frame_dir = Path(frame.f_code.co_filename).resolve().parent - if frame_dir != _PACKAGE_DIR: - break - frame = frame.f_back - stacklevel += 1 - return stacklevel - finally: - del frame # Avoid reference cycles def _loads_manifest(raw_manifest: str) -> object: @@ -205,7 +176,7 @@ def _get_zip_hash(version_name: str) -> str | None: f"{_LTP_BYPASS_VERIFIED_DOWNLOADS_ENV_VAR}=" f"false to re-enable verification." ) - warn(err, RuntimeWarning, stacklevel=_external_stacklevel()) + warn(err, RuntimeWarning, stacklevel=external_stacklevel()) return None suffix = re.sub(r"[^A-Za-z0-9]+", "_", version_name).strip("_").upper() version_env_var = f"LTP_DOWNLOAD_SHA256_{suffix}" @@ -226,7 +197,7 @@ def _get_zip_hash(version_name: str) -> str | None: f"Integrity will not be verified for this download. " f"You can provide one via the {version_env_var} environment variable." ) - warn(err, RuntimeWarning, stacklevel=_external_stacklevel()) + warn(err, RuntimeWarning, stacklevel=external_stacklevel()) return None diff --git a/src/language_tool_python/server.py b/src/language_tool_python/server.py index 5c4fc80..65059d6 100644 --- a/src/language_tool_python/server.py +++ b/src/language_tool_python/server.py @@ -26,6 +26,7 @@ ) from ._internals.utils import ( FAILSAFE_LANGUAGE, + external_stacklevel, get_locale_language, kill_process_force, parse_url, @@ -349,7 +350,9 @@ def __del__(self) -> None: necessary cleanup. """ if self._server_is_alive(): - warnings.warn("unclosed server", ResourceWarning, stacklevel=2) + warnings.warn( + "unclosed server", ResourceWarning, stacklevel=external_stacklevel() + ) logger.warning( "Unclosed server (server still running at %s). Closing it now.", getattr(self, "_url", "unknown"), diff --git a/tests/unit/test_download.py b/tests/unit/test_download.py index abf4a01..9de9d4b 100644 --- a/tests/unit/test_download.py +++ b/tests/unit/test_download.py @@ -430,11 +430,16 @@ def test_snapshot_download_renames_archive_root_to_requested_date( lambda: tmp_path, ) - with patch( - "language_tool_python.download_lt.requests.get", - return_value=MockDownloadResponse(payload), + with ( + patch( + "language_tool_python.download_lt.requests.get", + return_value=MockDownloadResponse(payload), + ), ): - local_language_tool.download() + with pytest.warns( + RuntimeWarning, match="No SHA-256 checksum available for LanguageTool" + ): + local_language_tool.download() expected_dir = tmp_path / f"LanguageTool-{requested_snapshot}" assert (expected_dir / "languagetool-server.jar").read_bytes() == b"jar" @@ -505,6 +510,9 @@ def test_snapshot_download_raises_when_archive_has_multiple_root_dirs( return_value=MockDownloadResponse(payload), ), pytest.raises(PathError, match="Expected snapshot archive"), + pytest.warns( + RuntimeWarning, match="No SHA-256 checksum available for LanguageTool" + ), ): local_language_tool.download() @@ -538,7 +546,10 @@ def test_latest_snapshot_download_renames_archive_root_to_current_date( "language_tool_python.download_lt.requests.get", return_value=MockDownloadResponse(payload), ): - local_language_tool.download() + with pytest.warns( + RuntimeWarning, match="No SHA-256 checksum available for LanguageTool" + ): + local_language_tool.download() expected_dir = tmp_path / f"LanguageTool-{current_snapshot_date}" assert (expected_dir / "languagetool-server.jar").read_bytes() == b"jar" diff --git a/tests/unit/test_internals_utils.py b/tests/unit/test_internals_utils.py index fa3df61..e1155a5 100644 --- a/tests/unit/test_internals_utils.py +++ b/tests/unit/test_internals_utils.py @@ -3,13 +3,14 @@ from __future__ import annotations import locale -from typing import TYPE_CHECKING +from pathlib import Path import psutil import pytest from language_tool_python._internals.utils import ( FAILSAFE_LANGUAGE, + external_stacklevel, get_env_float, get_env_int, get_language_tool_download_path, @@ -20,9 +21,6 @@ ) from language_tool_python.exceptions import PathError -if TYPE_CHECKING: - from pathlib import Path - _DEFAULT_INT = 42 _ENV_INT_VALUE = 100 _DEFAULT_FLOAT = 1.5 @@ -159,6 +157,39 @@ def test_ws_scheme_without_slashes_raises_unsupported(self) -> None: with pytest.raises(ValueError, match="Unsupported URL scheme 'ws'"): parse_url("ws:example.com") + def test_warning_is_attributed_to_the_external_caller(self) -> None: + """The 'no scheme' warning points at the caller's file, not internal code.""" + with pytest.warns(RuntimeWarning, match="No scheme was specified") as record: + parse_url("example.com") + assert len(record) == 1 + assert record[0].filename == __file__ + + +class TestExternalStacklevel: + """Tests for external_stacklevel()'s package-boundary detection.""" + + def test_walks_past_frames_in_a_subdirectory_of_the_package_root( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """A frame in a subdirectory of the package root still counts as internal.""" + monkeypatch.setattr( + "language_tool_python._internals.utils._PACKAGE_DIR", + Path(__file__).resolve().parent.parent, + ) + + def _wrapper() -> int: + return external_stacklevel() + + stacklevel_from_here = external_stacklevel() + stacklevel_from_wrapper = _wrapper() + + # The wrapper adds exactly one extra internal frame compared to calling + # external_stacklevel() directly, so the reported stacklevel must be one + # higher. Under the old strict-equality bug, both calls would stop at the + # first frame (this test module isn't *exactly* the patched package root) + # and return the same value regardless of the extra wrapper frame. + assert stacklevel_from_wrapper == stacklevel_from_here + 1 + class TestGetEnvInt: """Tests for get_env_int() environment variable reader."""