Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 44 additions & 37 deletions src/language_tool_python/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
34 changes: 33 additions & 1 deletion src/language_tool_python/_internals/utils.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import contextlib
import inspect
import locale
import logging
import math
Expand All @@ -15,6 +16,7 @@

__all__ = [
"SupportsBool",
"external_stacklevel",
"get_env_float",
"get_env_int",
"get_language_tool_download_path",
Expand All @@ -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

Check warning

Code scanning / CodeQL

Unnecessary delete statement in function Warning

Unnecessary deletion of local variable
frame
in function
external_stacklevel
.
Comment thread
mdevolde marked this conversation as resolved.
Dismissed


def _check_supported_scheme(scheme: str) -> None:
"""Raise ValueError if ``scheme`` is not one of the supported URL schemes."""
Expand Down Expand Up @@ -85,7 +117,7 @@
"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)
Expand Down
35 changes: 3 additions & 32 deletions src/language_tool_python/download_lt.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import contextlib
import hashlib
import importlib.resources
import inspect
import logging
import os
import re
Expand All @@ -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,
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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}"
Expand All @@ -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


Expand Down
5 changes: 4 additions & 1 deletion src/language_tool_python/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
)
from ._internals.utils import (
FAILSAFE_LANGUAGE,
external_stacklevel,
get_locale_language,
kill_process_force,
parse_url,
Expand Down Expand Up @@ -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"),
Expand Down
21 changes: 16 additions & 5 deletions tests/unit/test_download.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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()

Expand Down Expand Up @@ -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"
Expand Down
39 changes: 35 additions & 4 deletions tests/unit/test_internals_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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."""
Expand Down
Loading