fix: fix warns catching and warns stacklevels (#218)

Author: mdevolde

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

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)

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
 
 

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"),

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"

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."""