Merge pull request #39 from cachedjdk/type-hints

Use type checking

Author: marktsuchida

.github/workflows/ci.yml

@@ -22,6 +22,8 @@ jobs:
         with:
           python-version: "3.x"
       - uses: pre-commit/[email protected]
+      - uses: astral-sh/setup-uv@v5
+      - run: uv run ty check  # Until they add a pre-commit hook
 
   test:
     strategy:

docs/development.md

@@ -21,10 +21,18 @@ uv tool install pre-commit
 pre-commit install
 ```
 
+We use [ty](https://docs.astral.sh/ty/) for type checking. This will be added
+to the pre-commit hook in the future (when an official ty hook is available),
+but for now, you can run it manually:
+
+```sh
+uv run ty check
+```
+
 To run the tests:
 
 ```sh
-uv run test pytest
+uv run pytest
 ```
 
 To build the documentation with [Jupyter Book](https://jupyterbook.org/):

pyproject.toml

@@ -33,6 +33,7 @@ dependencies = [
     "click >= 8.0",
     "progressbar2 >= 4.0",
     "requests >= 2.24",
+    "typing-extensions>=4.15.0",
 ]
 
 [project.urls]
@@ -50,6 +51,7 @@ test = [
 ]
 dev = [
     {include-group = "test"},
+    "ty >= 0.0.9",
 ]
 docs = [
     "jgo >= 1.0",

src/cjdk/__main__.py

@@ -1,6 +1,7 @@
 # This file is part of cjdk.
 # Copyright 2022-25 Board of Regents of the University of Wisconsin System
 # SPDX-License-Identifier: MIT
+from __future__ import annotations
 
 import os
 import subprocess
@@ -44,7 +45,16 @@
     help="Show or do not show progress bars.",
 )
 @click.version_option(version=__version__)
-def _cli(ctx, jdk, cache_dir, index_url, index_ttl, os, arch, progress):
+def _cli(
+    ctx: click.Context,
+    jdk: str | None,
+    cache_dir: str | None,
+    index_url: str | None,
+    index_ttl: int | None,
+    os: str | None,
+    arch: str | None,
+    progress: bool,
+) -> None:
     """
     Download, cache, and run JDK or JRE distributions.
 
@@ -67,7 +77,7 @@ def _cli(ctx, jdk, cache_dir, index_url, index_ttl, os, arch, progress):
 
 @click.command(short_help="List available JDK vendors.")
 @click.pass_context
-def ls_vendors(ctx):
+def ls_vendors(ctx: click.Context) -> None:
     """
     Print the list of available JDK vendors.
     """
@@ -83,7 +93,7 @@ def ls_vendors(ctx):
     default=True,
     help="Show only already-cached JDKs, or show all available JDKs from the index (default cached only).",
 )
-def ls(ctx, cached: bool = False):
+def ls(ctx: click.Context, cached: bool) -> None:
     """
     Print the list of JDKs matching the given criteria.
 
@@ -96,7 +106,7 @@ def ls(ctx, cached: bool = False):
 
 @click.command(short_help="Ensure the requested JDK is cached.")
 @click.pass_context
-def cache(ctx):
+def cache(ctx: click.Context) -> None:
     """
     Download and extract the requested JDK if it is not already cached.
 
@@ -112,7 +122,7 @@ def cache(ctx):
 
 @click.command(hidden=True)
 @click.pass_context
-def cache_jdk(ctx):
+def cache_jdk(ctx: click.Context) -> None:
     """
     Deprecated. Use cache function instead.
     """
@@ -123,7 +133,7 @@ def cache_jdk(ctx):
     short_help="Print the Java home directory for the requested JDK."
 )
 @click.pass_context
-def java_home(ctx):
+def java_home(ctx: click.Context) -> None:
     """
     Print the path that is suitable as the value of JAVA_HOME for the requested
     JDK.
@@ -143,7 +153,7 @@ def java_home(ctx):
 @click.pass_context
 @click.argument("prog", nargs=1)
 @click.argument("args", nargs=-1, type=click.UNPROCESSED)
-def exec(ctx, prog, args):
+def exec(ctx: click.Context, prog: str, args: tuple[str, ...]) -> None:
     """
     Run PROG with the environment variables set for the requested JDK.
 
@@ -191,7 +201,16 @@ def exec(ctx, prog, args):
     metavar="HASH",
     help="Check the downloaded file against the given SHA-512 hash.",
 )
-def cache_file(ctx, url, filename, name, ttl, sha1, sha256, sha512):
+def cache_file(
+    ctx: click.Context,
+    url: str,
+    filename: str,
+    name: str | None,
+    ttl: int | None,
+    sha1: str | None,
+    sha256: str | None,
+    sha512: str | None,
+) -> None:
     """
     Download and store an arbitrary file if it is not already cached.
 
@@ -236,7 +255,14 @@ def cache_file(ctx, url, filename, name, ttl, sha1, sha256, sha512):
     metavar="HASH",
     help="Check the downloaded file against the given SHA-512 hash.",
 )
-def cache_package(ctx, url, name, sha1, sha256, sha512):
+def cache_package(
+    ctx: click.Context,
+    url: str,
+    name: str | None,
+    sha1: str | None,
+    sha256: str | None,
+    sha512: str | None,
+) -> None:
     """
     Download, extract, and store an arbitrary .zip or .tar.gz package if it is
     not already cached.
@@ -262,7 +288,7 @@ def cache_package(ctx, url, name, sha1, sha256, sha512):
 
 @click.command(short_help="Remove all cached files.")
 @click.pass_context
-def clear_cache(ctx):
+def clear_cache(ctx: click.Context) -> None:
     """
     Remove all cached JDKs, files, and packages from the cache directory.
 
@@ -292,7 +318,7 @@ def clear_cache(ctx):
 _cli.add_command(cache_jdk)
 
 
-def main():
+def main() -> None:
     try:
         _cli()
     except CjdkError as e:

src/cjdk/_api.py

@@ -20,7 +20,8 @@
 if TYPE_CHECKING:
     from collections.abc import Callable, Iterator
     from pathlib import Path
-    from typing import Any, Unpack
+
+    from typing_extensions import Unpack
 
     from ._conf import ConfigKwargs
 
@@ -46,6 +47,8 @@ def list_vendors(**kwargs: Unpack[ConfigKwargs]) -> list[str]:
 
     Other Parameters
     ----------------
+    cache_dir : pathlib.Path or str, optional
+        Override the root cache directory.
     index_url : str, optional
         Alternative URL for the JDK index.
 
@@ -122,8 +125,8 @@ def clear_cache(**kwargs: Unpack[ConfigKwargs]) -> Path:
     This should not be called when other processes may be using cjdk or the
     JDKs and files installed by cjdk.
 
-    Parameters
-    ----------
+    Other Parameters
+    ----------------
     cache_dir : pathlib.Path or str, optional
         Override the root cache directory.
 
@@ -280,7 +283,7 @@ def cache_file(
     name: str,
     url: str,
     filename: str,
-    ttl: int | None = None,
+    ttl: float | None = None,
     *,
     sha1: str | None = None,
     sha256: str | None = None,
@@ -301,13 +304,13 @@ def cache_file(
         The URL of the file resource. The scheme must be https.
     filename : str
         The filename under which the file will be stored.
-    ttl : int
+    ttl : int or float, optional
         Time to live (in seconds) for the cached file resource.
-    sha1 : str
+    sha1 : str, optional
         SHA-1 hash that the downloaded file must match.
-    sha256 : str
+    sha256 : str, optional
         SHA-256 hash that the downloaded file must match.
-    sha512 : str
+    sha512 : str, optional
         SHA-512 hash that the downloaded file must match.
 
     Returns
@@ -328,6 +331,9 @@ def cache_file(
     The check for SHA-1/SHA-256/SHA-512 hashes is only performed after a
     download; it is not performed if the file already exists in the cache.
     """
+    _conf.check_str("name", name)
+    _conf.check_str("url", url, allow_empty=False)
+    _conf.check_str("filename", filename, allow_empty=False)
     if ttl is None:
         ttl = 2**63
     check_hashes = _make_hash_checker(
@@ -369,11 +375,11 @@ def cache_package(
     url : str
         The URL of the file resource. The scheme must be tgz+https or
         zip+https.
-    sha1 : str
+    sha1 : str, optional
         SHA-1 hash that the downloaded file must match.
-    sha256 : str
+    sha256 : str, optional
         SHA-256 hash that the downloaded file must match.
-    sha512 : str
+    sha512 : str, optional
         SHA-512 hash that the downloaded file must match.
 
     Returns
@@ -394,6 +400,8 @@ def cache_package(
     unextracted archive) after a download; it is not performed if the directory
     already exists in the cache.
     """
+    _conf.check_str("name", name)
+    _conf.check_str("url", url, allow_empty=False)
     check_hashes = _make_hash_checker(
         dict(sha1=sha1, sha256=sha256, sha512=sha512)
     )
@@ -428,34 +436,41 @@ def _get_vendors(**kwargs: Unpack[ConfigKwargs]) -> set[str]:
     }
 
 
-def _get_jdks(*, vendor=None, version=None, cached_only=True, **kwargs):
-    conf = _conf.configure(
-        vendor=vendor,
-        version=version,
-        fallback_to_default_vendor=False,
-        **kwargs,
-    )
-    if conf.vendor is None:
-        # Search across all vendors.
-        kwargs.pop("jdk", None)  # It was already parsed.
+def _get_jdks(
+    *,
+    vendor: str | None = None,
+    version: str | None = None,
+    cached_only: bool = True,
+    **kwargs: Unpack[ConfigKwargs],
+) -> list[str]:
+    jdk = kwargs.pop("jdk", None)
+    if jdk:
+        parsed_vendor, parsed_version = _conf.parse_vendor_version(jdk)
+        vendor = vendor or parsed_vendor or None
+        version = version or parsed_version or None
+
+    # Handle "all vendors" before creating Configuration.
+    if vendor is None:
         return [
             jdk
             for v in sorted(_get_vendors())
             for jdk in _get_jdks(
                 vendor=v,
-                version=conf.version,
+                version=version,
                 cached_only=cached_only,
                 **kwargs,
             )
         ]
+
+    conf = _conf.configure(vendor=vendor, version=version, **kwargs)
     index = _index.jdk_index(conf)
     jdks = _index.available_jdks(index, conf)
     versions = _index._get_versions(jdks, conf)
     matched = _index._match_versions(conf.vendor, versions, conf.version)
 
     if cached_only:
         # Filter matches by existing key directories.
-        def is_cached(v):
+        def is_cached(v: str) -> bool:
             url = _index.jdk_url(index, conf, v)
             key = (_jdk._JDK_KEY_PREFIX, _cache._key_for_url(url))
             keydir = _cache._key_directory(conf.cache_dir, key)
@@ -464,21 +479,22 @@ def is_cached(v):
         matched = {k: v for k, v in matched.items() if is_cached(v)}
 
     class VersionElement:
-        def __init__(self, value):
+        def __init__(self, value: int | str) -> None:
             self.value = value
-            self.is_int = isinstance(value, int)
 
-        def __eq__(self, other):
-            if self.is_int and other.is_int:
+        def __eq__(self, other: VersionElement) -> bool:  # type: ignore[override]
+            if isinstance(self.value, int) and isinstance(other.value, int):
                 return self.value == other.value
             return str(self.value) == str(other.value)
 
-        def __lt__(self, other):
-            if self.is_int and other.is_int:
+        def __lt__(self, other: VersionElement) -> bool:
+            if isinstance(self.value, int) and isinstance(other.value, int):
                 return self.value < other.value
             return str(self.value) < str(other.value)
 
-    def version_key(version_tuple):
+    def version_key(
+        version_tuple: tuple[tuple[int | str, ...], str],
+    ) -> tuple[VersionElement, ...]:
         return tuple(VersionElement(elem) for elem in version_tuple[0])
 
     return [
@@ -487,24 +503,26 @@ def version_key(version_tuple):
     ]
 
 
-def _make_hash_checker(hashes: dict) -> Callable[[Any], None]:
+def _make_hash_checker(
+    hashes: dict[str, str | None],
+) -> Callable[[Path], None]:
     checks = [
         (hashes.pop("sha1", None), hashlib.sha1),
         (hashes.pop("sha256", None), hashlib.sha256),
         (hashes.pop("sha512", None), hashlib.sha512),
     ]
 
-    def check(filepath: Any) -> None:
+    def check(filepath: Path) -> None:
         for hash, hasher in checks:
             if hash:
                 _hasher = hasher()
                 try:
                     with open(filepath, "rb") as infile:
                         while True:
-                            bytes = infile.read(16384)
-                            if not len(bytes):
+                            chunk = infile.read(16384)
+                            if not len(chunk):
                                 break
-                            _hasher.update(bytes)
+                            _hasher.update(chunk)
                 except OSError as e:
                     raise InstallError(
                         f"Failed to read file for hash verification: {e}"
@@ -516,7 +534,7 @@ def check(filepath: Any) -> None:
 
 
 @contextmanager
-def _env_var_set(name, value):
+def _env_var_set(name: str, value: str) -> Iterator[None]:
     old_value = os.environ.get(name, None)
     os.environ[name] = value
     try: