fix: correct codeql warnings (jxmorris12#196)

* `__eq__` not overridden when adding attributes
* Module is imported with 'import' and 'import from'
* Should use a 'with' statement
* Statement has no effect
* Unused global variable

Author: mdevolde

src/language_tool_python/_internals/utils.py

@@ -24,17 +24,10 @@
 
 logger = logging.getLogger(__name__)
 
-JAR_NAMES = [
-    "languagetool-server.jar",
-    "LanguageTool.jar",
-]
 FAILSAFE_LANGUAGE = "en"
 
 LTP_PATH_ENV_VAR = "LTP_PATH"  # LanguageTool download path
 
-# Directory containing the LanguageTool jar file:
-LTP_JAR_DIR_PATH_ENV_VAR = "LTP_JAR_DIR_PATH"
-
 
 def parse_url(url_str: str) -> str:
     """Parse the given URL string and ensure it has a scheme.
@@ -189,7 +182,6 @@ class SupportsBool(Protocol):
 
     def __bool__(self) -> bool:
         """Define the interface for types that can be evaluated in a boolean context."""
-        ...
 
 
 def version_tuple(v: str) -> tuple[int, int]:

src/language_tool_python/download_lt.py

@@ -26,7 +26,6 @@
 from ._internals.compat import toml_loads
 from ._internals.safe_zip import SafeZipExtractor
 from ._internals.utils import (
-    LTP_JAR_DIR_PATH_ENV_VAR,
     get_env_int,
     get_language_tool_download_path,
     version_tuple,
@@ -80,6 +79,7 @@
 _LTP_DOWNLOAD_SHA256_ENV_VAR = "LTP_DOWNLOAD_SHA256"
 _LTP_BYPASS_VERIFIED_DOWNLOADS_ENV_VAR = "LTP_BYPASS_VERIFIED_DOWNLOADS"
 _LTP_MAX_DOWNLOAD_BYTES_ENV_VAR = "LTP_MAX_DOWNLOAD_BYTES"
+_LTP_JAR_DIR_PATH_ENV_VAR = "LTP_JAR_DIR_PATH"
 _DOWNLOAD_CHUNK_BYTES = 1024 * 1024
 _SAFE_ZIP_EXTRACTOR = SafeZipExtractor()
 
@@ -746,7 +746,7 @@ def download(self) -> None:
 
         # Use the env var to the jar directory if it is defined
         # otherwise look in the download directory
-        if os.environ.get(LTP_JAR_DIR_PATH_ENV_VAR):
+        if os.environ.get(_LTP_JAR_DIR_PATH_ENV_VAR):
             return
 
         if self not in self.get_installed_versions():
@@ -815,6 +815,28 @@ def download_url(self) -> str:
         # Versions < 6.0 from archive
         return urljoin(_BASE_URL_ARCHIVE, filename)
 
+    def __eq__(self, other: object) -> bool:
+        """Check equality between two ReleaseLocalLanguageTool instances.
+
+        Two instances are considered equal if they have the same version name.
+
+        :param other: The object to compare with.
+        :type other: object
+        :return: True if the instances are equal, False otherwise.
+        :rtype: bool
+        """
+        return super().__eq__(other)
+
+    def __hash__(self) -> int:
+        """Return the hash of the ReleaseLocalLanguageTool instance.
+
+        If the version name is modified, the hash will change.
+
+        :return: The hash of the version name.
+        :rtype: int
+        """
+        return super().__hash__()
+
 
 class SnapshotLocalLanguageTool(LocalLanguageTool):
     """Represents a snapshot (development) version of LanguageTool.
@@ -855,7 +877,7 @@ def download(self) -> None:
 
         # Use the env var to the jar directory if it is defined
         # otherwise look in the download directory
-        if os.environ.get(LTP_JAR_DIR_PATH_ENV_VAR):
+        if os.environ.get(_LTP_JAR_DIR_PATH_ENV_VAR):
             return
 
         if self not in self.get_installed_versions():
@@ -932,3 +954,25 @@ def download_url(self) -> str:
         """
         filename = _FILENAME_SNAPSHOT.format(version=self._version_name)
         return urljoin(_BASE_URL_SNAPSHOT, filename)
+
+    def __eq__(self, other: object) -> bool:
+        """Check equality between two SnapshotLocalLanguageTool instances.
+
+        Two instances are considered equal if they have the same version name.
+
+        :param other: The object to compare with.
+        :type other: object
+        :return: True if the instances are equal, False otherwise.
+        :rtype: bool
+        """
+        return super().__eq__(other)
+
+    def __hash__(self) -> int:
+        """Return the hash of the SnapshotLocalLanguageTool instance.
+
+        If the version name is modified, the hash will change.
+
+        :return: The hash of the version name.
+        :rtype: int
+        """
+        return super().__hash__()

tests/test_cli.py

@@ -74,14 +74,10 @@ def remote_server() -> Generator[tuple[str, int], None, None]:
     :return: A tuple containing the server host and port (host, port).
     :rtype: Generator[Tuple[str, int], None, None]
     """
-    tool = language_tool_python.LanguageTool("en-US")
-    host = tool._host
-    port = tool._port
-
-    try:
+    with language_tool_python.LanguageTool("en-US") as tool:
+        host = tool._host
+        port = tool._port
         yield host, port
-    finally:
-        tool.close()
 
 
 def test_cli_remote_ok(remote_server: tuple[str, int]) -> None:

tests/test_download.py

@@ -17,7 +17,6 @@
 import pytest
 
 import language_tool_python
-from language_tool_python import download_lt
 from language_tool_python.download_lt import (
     _LTP_BYPASS_VERIFIED_DOWNLOADS_ENV_VAR,
     _LTP_DOWNLOAD_SHA256_ENV_VAR,
@@ -186,7 +185,7 @@ def test_http_get_rejects_oversized_content_length(
     )
     response = MockDownloadResponse(payload)
     response.headers["Content-Length"] = "2"
-    monkeypatch.setattr(download_lt, "_MAX_DOWNLOAD_BYTES", 1)
+    monkeypatch.setattr(language_tool_python.download_lt, "_MAX_DOWNLOAD_BYTES", 1)
 
     with (
         patch(
@@ -207,11 +206,14 @@ def test_max_download_bytes_uses_env_override(
             env.setenv(
                 _LTP_MAX_DOWNLOAD_BYTES_ENV_VAR, str(EXPECTED_DOWNLOAD_BYTES_OVERRIDE)
             )
-            importlib.reload(download_lt)
+            importlib.reload(language_tool_python.download_lt)
 
-            assert download_lt._MAX_DOWNLOAD_BYTES == EXPECTED_DOWNLOAD_BYTES_OVERRIDE
+            assert (
+                language_tool_python.download_lt._MAX_DOWNLOAD_BYTES
+                == EXPECTED_DOWNLOAD_BYTES_OVERRIDE
+            )
     finally:
-        importlib.reload(download_lt)
+        importlib.reload(language_tool_python.download_lt)
 
 
 def test_http_get_rejects_oversized_stream(
@@ -223,7 +225,9 @@ def test_http_get_rejects_oversized_stream(
     )
     response = MockDownloadResponse(payload)
     response.headers = {}
-    monkeypatch.setattr(download_lt, "_MAX_DOWNLOAD_BYTES", len(payload) - 1)
+    monkeypatch.setattr(
+        language_tool_python.download_lt, "_MAX_DOWNLOAD_BYTES", len(payload) - 1
+    )
 
     with (
         patch(
@@ -244,7 +248,9 @@ def test_http_get_rejects_oversized_stream_with_small_content_length(
     )
     response = MockDownloadResponse(payload)
     response.headers["Content-Length"] = "1"
-    monkeypatch.setattr(download_lt, "_MAX_DOWNLOAD_BYTES", len(payload) - 1)
+    monkeypatch.setattr(
+        language_tool_python.download_lt, "_MAX_DOWNLOAD_BYTES", len(payload) - 1
+    )
 
     with (
         patch(
@@ -279,13 +285,13 @@ def test_latest_snapshot_uses_latest_download_url_and_current_date(
 ) -> None:
     """Test that latest remains a snapshot alias installed under the current date."""
     monkeypatch.setattr(
-        download_lt,
+        language_tool_python.download_lt,
         "_BASE_URL_SNAPSHOT",
         "https://example.test/snapshots/",
     )
 
     FixedDatetime.current_datetime = datetime(2024, 5, 14, tzinfo=timezone.utc)
-    monkeypatch.setattr(download_lt, "datetime", FixedDatetime)
+    monkeypatch.setattr(language_tool_python.download_lt, "datetime", FixedDatetime)
     local_language_tool = LocalLanguageTool.from_version_name("latest")
 
     assert local_language_tool.version_name == "20240514"
@@ -302,7 +308,7 @@ def test_release_download_url_uses_new_release_base_from_6_7(
 ) -> None:
     """Test that releases 6.7 and newer include the version in the base URL."""
     monkeypatch.setattr(
-        download_lt,
+        language_tool_python.download_lt,
         "_BASE_URL_NEW_RELEASES",
         "https://example.test/releases/LanguageTool-{version}/",
     )
@@ -322,7 +328,7 @@ def test_release_download_url_keeps_main_release_base_for_6_6(
 ) -> None:
     """Test that release 6.6 keeps using the versioned filename."""
     monkeypatch.setattr(
-        download_lt,
+        language_tool_python.download_lt,
         "_BASE_URL_RELEASE",
         "https://example.test/download/",
     )
@@ -340,7 +346,7 @@ def test_release_download_url_keeps_main_release_base_before_6_7(
 ) -> None:
     """Test that earlier 6.x releases keep using the versioned filename."""
     monkeypatch.setattr(
-        download_lt,
+        language_tool_python.download_lt,
         "_BASE_URL_RELEASE",
         "https://example.test/download/",
     )
@@ -358,7 +364,7 @@ def test_release_download_url_keeps_archive_base_before_6_0(
 ) -> None:
     """Test that older supported releases keep using the archive base URL."""
     monkeypatch.setattr(
-        download_lt,
+        language_tool_python.download_lt,
         "_BASE_URL_ARCHIVE",
         "https://example.test/archive/",
     )
@@ -486,7 +492,7 @@ def test_snapshot_download_renames_archive_root_to_requested_date(
     )
     local_language_tool = LocalLanguageTool.from_version_name(requested_snapshot)
     monkeypatch.setattr(
-        download_lt,
+        language_tool_python.download_lt,
         "_confirm_java_compatibility",
         skip_java_compatibility_check,
     )
@@ -499,7 +505,7 @@ def test_snapshot_download_renames_archive_root_to_requested_date(
         ),
     ):
         monkeypatch.setattr(
-            download_lt,
+            language_tool_python.download_lt,
             "get_language_tool_download_path",
             lambda: temp_dir,
         )
@@ -525,10 +531,10 @@ def test_latest_snapshot_download_renames_archive_root_to_current_date(
         {"LanguageTool-6.9-SNAPSHOT/languagetool-server.jar": b"jar"},
     )
     FixedDatetime.current_datetime = datetime(2024, 5, 14, tzinfo=timezone.utc)
-    monkeypatch.setattr(download_lt, "datetime", FixedDatetime)
+    monkeypatch.setattr(language_tool_python.download_lt, "datetime", FixedDatetime)
     local_language_tool = LocalLanguageTool.from_version_name("latest")
     monkeypatch.setattr(
-        download_lt,
+        language_tool_python.download_lt,
         "_confirm_java_compatibility",
         skip_java_compatibility_check,
     )
@@ -541,7 +547,7 @@ def test_latest_snapshot_download_renames_archive_root_to_current_date(
         ),
     ):
         monkeypatch.setattr(
-            download_lt,
+            language_tool_python.download_lt,
             "get_language_tool_download_path",
             lambda: temp_dir,
         )