Skip to content

Commit

Permalink
fix(types): improve typing (#720)
Browse files Browse the repository at this point in the history
* 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 <[email protected]>

* fix missing annotation

* fix coverage in _clean_env

---------

Signed-off-by: Henry Schreiner <[email protected]>
Co-authored-by: Stargirl Flowers <[email protected]>
  • Loading branch information
henryiii and theacodes authored Jul 12, 2023
1 parent c1120f2 commit f46e46e
Show file tree
Hide file tree
Showing 14 changed files with 83 additions and 59 deletions.
11 changes: 6 additions & 5 deletions nox/_decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -60,17 +61,17 @@ 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
self.reuse_venv = reuse_venv
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)
Expand Down
5 changes: 3 additions & 2 deletions nox/_option_set.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
13 changes: 7 additions & 6 deletions nox/_parametrize.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand All @@ -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]:
Expand All @@ -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
Expand Down Expand Up @@ -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 = []
Expand Down
34 changes: 19 additions & 15 deletions nox/command.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand All @@ -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)
Expand All @@ -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:
Expand Down
11 changes: 6 additions & 5 deletions nox/manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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

Expand Down
3 changes: 2 additions & 1 deletion nox/popen.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
5 changes: 3 additions & 2 deletions nox/registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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]:
...

Expand All @@ -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
Expand Down
30 changes: 16 additions & 14 deletions nox/sessions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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,
Expand Down
12 changes: 10 additions & 2 deletions nox/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import sys
import types
from argparse import Namespace
from typing import Sequence, TypeVar

from colorlog.escape_codes import parse_colors

Expand Down Expand Up @@ -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:
Expand All @@ -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:
Expand Down
3 changes: 2 additions & 1 deletion nox/tox_to_nox.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading

0 comments on commit f46e46e

Please sign in to comment.