From f46e46efa2f91cf4b162eb64c0ce685150588939 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Wed, 12 Jul 2023 16:56:47 -0500 Subject: [PATCH] fix(types): improve typing (#720) * fix(types): improve typing This fixes an issue with os.PathLike[str] being accepted but not present in the typing. It also uses collections.abc wherever possible instead of typing (can't fully change all of them until 3.9+ is supported, due to runtime requirements). It also uses Sequence/Mapping/Iterable for input parameters (you should be generic in input, specific in output) - doing so adds a few copy functions and improves safety a bit, fewer arguments are allowed to be mutated later this way). A few TypeVars were added - they can be replaced with native syntax in Python 3.12+. Signed-off-by: Henry Schreiner * fix missing annotation * fix coverage in _clean_env --------- Signed-off-by: Henry Schreiner Co-authored-by: Stargirl Flowers --- nox/_decorators.py | 11 ++++++----- nox/_option_set.py | 5 +++-- nox/_parametrize.py | 13 +++++++------ nox/command.py | 34 +++++++++++++++++++--------------- nox/manifest.py | 11 ++++++----- nox/popen.py | 3 ++- nox/registry.py | 5 +++-- nox/sessions.py | 30 ++++++++++++++++-------------- nox/tasks.py | 12 ++++++++++-- nox/tox_to_nox.py | 3 ++- nox/virtualenv.py | 3 ++- nox/workflow.py | 3 ++- tests/test__parametrize.py | 6 +++--- tests/test_manifest.py | 3 ++- 14 files changed, 83 insertions(+), 59 deletions(-) diff --git a/nox/_decorators.py b/nox/_decorators.py index 1d4c4eaa..648d96aa 100644 --- a/nox/_decorators.py +++ b/nox/_decorators.py @@ -18,7 +18,8 @@ import functools import inspect import types -from typing import TYPE_CHECKING, Any, Callable, Iterable, TypeVar, cast +from collections.abc import Iterable, Mapping, Sequence +from typing import TYPE_CHECKING, Any, Callable, TypeVar, cast from . import _typing @@ -60,8 +61,8 @@ def __init__( name: str | None = None, venv_backend: Any = None, venv_params: Any = None, - should_warn: dict[str, Any] | None = None, - tags: list[str] | None = None, + should_warn: Mapping[str, Any] | None = None, + tags: Sequence[str] | None = None, ): self.func = func self.python = python @@ -69,8 +70,8 @@ def __init__( self.name = name self.venv_backend = venv_backend self.venv_params = venv_params - self.should_warn = should_warn or {} - self.tags = tags or [] + self.should_warn = dict(should_warn or {}) + self.tags = list(tags or []) def __call__(self, *args: Any, **kwargs: Any) -> Any: return self.func(*args, **kwargs) diff --git a/nox/_option_set.py b/nox/_option_set.py index 929c3a32..39a84f6e 100644 --- a/nox/_option_set.py +++ b/nox/_option_set.py @@ -23,7 +23,8 @@ import functools from argparse import ArgumentError as ArgumentError from argparse import ArgumentParser, Namespace -from typing import Any, Callable +from collections.abc import Callable, Sequence +from typing import Any import argcomplete @@ -91,7 +92,7 @@ def __init__( | list[str] | Callable[[], bool | str | None | list[str]] = None, hidden: bool = False, - completer: Callable[..., list[str]] | None = None, + completer: Callable[..., Sequence[str]] | None = None, **kwargs: Any, ) -> None: self.name = name diff --git a/nox/_parametrize.py b/nox/_parametrize.py index 967a261c..8b5d55c3 100644 --- a/nox/_parametrize.py +++ b/nox/_parametrize.py @@ -16,7 +16,8 @@ import functools import itertools -from typing import Any, Callable, Iterable, Sequence, Union +from collections.abc import Callable, Sequence +from typing import Any, Iterable, Union class Param: @@ -80,7 +81,7 @@ def __eq__(self, other: object) -> bool: raise NotImplementedError -def _apply_param_specs(param_specs: list[Param], f: Any) -> Any: +def _apply_param_specs(param_specs: Iterable[Param], f: Any) -> Any: previous_param_specs = getattr(f, "parametrize", None) new_param_specs = update_param_specs(previous_param_specs, param_specs) f.parametrize = new_param_specs @@ -91,7 +92,7 @@ def _apply_param_specs(param_specs: list[Param], f: Any) -> Any: def parametrize_decorator( - arg_names: str | list[str] | tuple[str], + arg_names: str | Sequence[str], arg_values_list: Iterable[ArgValue] | ArgValue, ids: Iterable[str | None] | None = None, ) -> Callable[[Any], Any]: @@ -116,7 +117,7 @@ def parametrize_decorator( """ # Allow args names to be specified as any of 'arg', 'arg,arg2' or ('arg', 'arg2') - if not isinstance(arg_names, (list, tuple)): + if isinstance(arg_names, str): arg_names = list(filter(None, [arg.strip() for arg in arg_names.split(",")])) # If there's only one arg_name, arg_values_list should be a single item @@ -157,11 +158,11 @@ def parametrize_decorator( def update_param_specs( - param_specs: Iterable[Param] | None, new_specs: list[Param] + param_specs: Iterable[Param] | None, new_specs: Iterable[Param] ) -> list[Param]: """Produces all combinations of the given sets of specs.""" if not param_specs: - return new_specs + return list(new_specs) # New specs must be combined with old specs by *multiplying* them. combined_specs = [] diff --git a/nox/command.py b/nox/command.py index 9398a8d0..9280f8e6 100644 --- a/nox/command.py +++ b/nox/command.py @@ -18,7 +18,8 @@ import shlex import shutil import sys -from typing import Any, Iterable, Sequence +from collections.abc import Iterable, Mapping, Sequence +from typing import Any from nox.logger import logger from nox.popen import popen @@ -37,44 +38,46 @@ def __init__(self, reason: str | None = None) -> None: self.reason = reason -def which(program: str, paths: list[str] | None) -> str: +def which(program: str | os.PathLike[str], paths: Sequence[str] | None) -> str: """Finds the full path to an executable.""" if paths is not None: full_path = shutil.which(program, path=os.pathsep.join(paths)) if full_path: - return full_path + return os.fspath(full_path) full_path = shutil.which(program) if full_path: - return full_path + return os.fspath(full_path) logger.error(f"Program {program} not found.") raise CommandFailed(f"Program {program} not found") -def _clean_env(env: dict[str, str] | None) -> dict[str, str]: - clean_env = {} +def _clean_env(env: Mapping[str, str] | None) -> dict[str, str] | None: + if env is None: + return None + + clean_env: dict[str, str] = {} # Ensure systemroot is passed down, otherwise Windows will explode. - if sys.platform == "win32": + if sys.platform == "win32": # pragma: no cover clean_env["SYSTEMROOT"] = os.environ.get("SYSTEMROOT", "") - if env is not None: - clean_env.update(env) + clean_env.update(env) return clean_env -def _shlex_join(args: Sequence[str]) -> str: +def _shlex_join(args: Sequence[str | os.PathLike[str]]) -> str: return " ".join(shlex.quote(os.fspath(arg)) for arg in args) def run( - args: Sequence[str], + args: Sequence[str | os.PathLike[str]], *, - env: dict[str, str] | None = None, + env: Mapping[str, str] | None = None, silent: bool = False, - paths: list[str] | None = None, + paths: Sequence[str] | None = None, success_codes: Iterable[int] | None = None, log: bool = True, external: Literal["error"] | bool = False, @@ -88,7 +91,8 @@ def run( cmd, args = args[0], args[1:] full_cmd = f"{cmd} {_shlex_join(args)}" - cmd_path = which(cmd, paths) + cmd_path = which(os.fspath(cmd), paths) + str_args = [os.fspath(arg) for arg in args] if log: logger.info(full_cmd) @@ -115,7 +119,7 @@ def run( try: return_code, output = popen( - [cmd_path, *list(args)], silent=silent, env=env, **popen_kws + [cmd_path, *str_args], silent=silent, env=env, **popen_kws ) if return_code not in success_codes: diff --git a/nox/manifest.py b/nox/manifest.py index 006fd72f..1eff9528 100644 --- a/nox/manifest.py +++ b/nox/manifest.py @@ -18,7 +18,8 @@ import ast import itertools from collections import OrderedDict -from typing import Any, Iterable, Iterator, Mapping, Sequence +from collections.abc import Iterable, Iterator, Sequence +from typing import Any, Mapping from nox._decorators import Call, Func from nox.sessions import Session, SessionRunner @@ -174,10 +175,10 @@ def filter_by_keywords(self, keywords: str) -> None: self._queue = [ x for x in self._queue - if keyword_match(keywords, x.signatures + x.tags + [x.name]) + if keyword_match(keywords, [*x.signatures, *x.tags, x.name]) ] - def filter_by_tags(self, tags: list[str]) -> None: + def filter_by_tags(self, tags: Iterable[str]) -> None: """Filter sessions by their tags. Args: @@ -280,7 +281,7 @@ def next(self) -> SessionRunner: return self.__next__() def notify( - self, session: str | SessionRunner, posargs: list[str] | None = None + self, session: str | SessionRunner, posargs: Iterable[str] | None = None ) -> bool: """Enqueue the specified session in the queue. @@ -311,7 +312,7 @@ def notify( for s in self._all_sessions: if s == session or s.name == session or session in s.signatures: if posargs is not None: - s.posargs = posargs + s.posargs = list(posargs) self._queue.append(s) return True diff --git a/nox/popen.py b/nox/popen.py index 2b52134e..2d37b551 100644 --- a/nox/popen.py +++ b/nox/popen.py @@ -18,7 +18,8 @@ import locale import subprocess import sys -from typing import IO, Mapping, Sequence +from collections.abc import Mapping, Sequence +from typing import IO def shutdown_process( diff --git a/nox/registry.py b/nox/registry.py index 35cb3890..2f6e5990 100644 --- a/nox/registry.py +++ b/nox/registry.py @@ -17,6 +17,7 @@ import collections import copy import functools +from collections.abc import Sequence from typing import Any, Callable, TypeVar, overload from ._decorators import Func @@ -41,7 +42,7 @@ def session_decorator( name: str | None = ..., venv_backend: Any = ..., venv_params: Any = ..., - tags: list[str] | None = ..., + tags: Sequence[str] | None = ..., ) -> Callable[[F], F]: ... @@ -54,7 +55,7 @@ def session_decorator( name: str | None = None, venv_backend: Any = None, venv_params: Any = None, - tags: list[str] | None = None, + tags: Sequence[str] | None = None, ) -> F | Callable[[F], F]: """Designate the decorated function as a session.""" # If `func` is provided, then this is the decorator call with the function diff --git a/nox/sessions.py b/nox/sessions.py index dc1aa7e8..bab59d5b 100644 --- a/nox/sessions.py +++ b/nox/sessions.py @@ -23,17 +23,19 @@ import re import sys import unicodedata -from types import TracebackType -from typing import ( - TYPE_CHECKING, - Any, +from collections.abc import ( Callable, Generator, Iterable, Mapping, - NoReturn, Sequence, ) +from types import TracebackType +from typing import ( + TYPE_CHECKING, + Any, + NoReturn, +) import nox.command from nox._decorators import Func @@ -85,7 +87,7 @@ def _normalize_path(envdir: str, path: str | bytes) -> str: return full_path -def _dblquote_pkg_install_args(args: tuple[str, ...]) -> tuple[str, ...]: +def _dblquote_pkg_install_args(args: Iterable[str]) -> tuple[str, ...]: """Double-quote package install arguments in case they contain '>' or '<' symbols""" # routine used to handle a single arg @@ -271,8 +273,8 @@ def _run_func( def run( self, - *args: str, - env: dict[str, str] | None = None, + *args: str | os.PathLike[str], + env: Mapping[str, str] | None = None, include_outer_env: bool = True, **kwargs: Any, ) -> Any | None: @@ -390,8 +392,8 @@ def run( def run_always( self, - *args: str, - env: dict[str, str] | None = None, + *args: str | os.PathLike[str], + env: Mapping[str, str] | None = None, include_outer_env: bool = True, **kwargs: Any, ) -> Any | None: @@ -456,15 +458,15 @@ def run_always( def _run( self, - *args: str, - env: dict[str, str] | None = None, + *args: str | os.PathLike[str], + env: Mapping[str, str] | None = None, include_outer_env: bool = True, **kwargs: Any, ) -> Any: """Like run(), except that it runs even if --install-only is provided.""" # Legacy support - run a function given. if callable(args[0]): - return self._run_func(args[0], args[1:], kwargs) + return self._run_func(args[0], args[1:], kwargs) # type: ignore[unreachable] # Combine the env argument with our virtualenv's env vars. if include_outer_env: @@ -694,7 +696,7 @@ class SessionRunner: def __init__( self, name: str, - signatures: list[str], + signatures: Sequence[str], func: Func, global_config: argparse.Namespace, manifest: Manifest, diff --git a/nox/tasks.py b/nox/tasks.py index a72d570e..595a4056 100644 --- a/nox/tasks.py +++ b/nox/tasks.py @@ -21,6 +21,7 @@ import sys import types from argparse import Namespace +from typing import Sequence, TypeVar from colorlog.escape_codes import parse_colors @@ -350,7 +351,12 @@ def run_manifest(manifest: Manifest, global_config: Namespace) -> list[Result]: return results -def print_summary(results: list[Result], global_config: Namespace) -> list[Result]: +Sequence_Results_T = TypeVar("Sequence_Results_T", bound=Sequence[Result]) + + +def print_summary( + results: Sequence_Results_T, global_config: Namespace +) -> Sequence_Results_T: """Print a summary of the results. Args: @@ -377,7 +383,9 @@ def print_summary(results: list[Result], global_config: Namespace) -> list[Resul return results -def create_report(results: list[Result], global_config: Namespace) -> list[Result]: +def create_report( + results: Sequence_Results_T, global_config: Namespace +) -> Sequence_Results_T: """Write a report to the location designated in the config, if any. Args: diff --git a/nox/tox_to_nox.py b/nox/tox_to_nox.py index a6591b4b..c68b13a5 100644 --- a/nox/tox_to_nox.py +++ b/nox/tox_to_nox.py @@ -18,7 +18,8 @@ import argparse import pkgutil -from typing import Any, Iterator +from collections.abc import Iterator +from typing import Any import jinja2 import tox.config diff --git a/nox/virtualenv.py b/nox/virtualenv.py index 8964f19b..431e8f27 100644 --- a/nox/virtualenv.py +++ b/nox/virtualenv.py @@ -21,8 +21,9 @@ import shutil import subprocess import sys +from collections.abc import Mapping from socket import gethostbyname -from typing import Any, ClassVar, Mapping +from typing import Any, ClassVar import nox import nox.command diff --git a/nox/workflow.py b/nox/workflow.py index 9d81e7eb..c50bfbce 100644 --- a/nox/workflow.py +++ b/nox/workflow.py @@ -15,7 +15,8 @@ from __future__ import annotations import argparse -from typing import Any, Callable, Iterable +from collections.abc import Callable, Iterable +from typing import Any def execute( diff --git a/tests/test__parametrize.py b/tests/test__parametrize.py index f21fddd8..1364087d 100644 --- a/tests/test__parametrize.py +++ b/tests/test__parametrize.py @@ -166,7 +166,7 @@ def f(): def test_generate_calls_simple(): - f = mock.Mock() + f = mock.Mock(should_warn={}, tags=[]) f.__name__ = "f" f.some_prop = 42 @@ -198,7 +198,7 @@ def test_generate_calls_simple(): def test_generate_calls_multiple_args(): - f = mock.Mock() + f = mock.Mock(should_warn=None, tags=None) f.__name__ = "f" arg_names = ("foo", "abc") @@ -224,7 +224,7 @@ def test_generate_calls_multiple_args(): def test_generate_calls_ids(): - f = mock.Mock() + f = mock.Mock(should_warn={}, tags=[]) f.__name__ = "f" arg_names = ("foo",) diff --git a/tests/test_manifest.py b/tests/test_manifest.py index 4df7ab55..5d87bfd4 100644 --- a/tests/test_manifest.py +++ b/tests/test_manifest.py @@ -15,6 +15,7 @@ from __future__ import annotations import collections +from collections.abc import Sequence from unittest import mock import pytest @@ -210,7 +211,7 @@ def test_filter_by_keyword(): (["baz", "missing"], 2), ], ) -def test_filter_by_tags(tags: list[str], session_count: int): +def test_filter_by_tags(tags: Sequence[str], session_count: int): sessions = create_mock_sessions() manifest = Manifest(sessions, create_mock_config()) assert len(manifest) == 2