Repository: theacodes/nox
Pull request #290 (2020-03-04 00:45:53)

Convert function decorators to class decorators
Hey, sorry for the delay.  
I have improved typing as discussed in #287. I have:

-   Updated the `noxfile.py` to include the `--warn-unused-ignores` flag.
-   Made a `_decorators` file, as the code started to call and inherit from multiple different files. This was causing some cyclical dependencies.
-   Changed [`_parametrize.generate_calls`](https://github.com/theacodes/nox/blob/1bd08bc7d85f7276b07a72b720c673a5df384188/nox/_parametrize.py#L172) to a class decorator, `Call`.
-   Changed [`registry.session_decorator`](https://github.com/theacodes/nox/blob/1bd08bc7d85f7276b07a72b720c673a5df384188/nox/registry.py#L24) to delegate to a class decorator, `Func`.
-   Moved [`manifest._copy_func`](https://github.com/theacodes/nox/blob/1bd08bc7d85f7276b07a72b720c673a5df384188/nox/manifest.py#L26) into `_decorators` and call it in `Func`.
-   Changed [`manifest.Manifest.make_session`](https://github.com/theacodes/nox/blob/1bd08bc7d85f7276b07a72b720c673a5df384188/nox/manifest.py#L186) to use `Func.copy` rather than `manifest._copy_func`.
-   Made `Call` subclass `Func`.
-   Removed a large amount of `# type: ignore`.
-   Updated a few `Callable` to `Func`.
-   Updated tests.

Given the size of the existing PR I have decided to leave the typing of the attributes as is.



Patch:
diff --git a/nox/_decorators.py b/nox/_decorators.py
new file mode 100644
index 0000000..32773f8
--- /dev/null
+++ b/nox/_decorators.py
@@ -0,0 +1,83 @@
+import copy
+import functools
+import types
+from typing import Any, Callable, Iterable, List, Optional, cast
+
+from . import _typing
+
+if _typing.TYPE_CHECKING:
+    from ._parametrize import Param
+
+
+class FunctionDecorator:
+    def __new__(
+        cls, func: Callable[..., Any], *args: Any, **kwargs: Any
+    ) -> "FunctionDecorator":
+        obj = super().__new__(cls)
+        return cast("FunctionDecorator", functools.wraps(func)(obj))
+
+
+def _copy_func(src: Callable, name: str = None) -> Callable:
+    dst = types.FunctionType(
+        src.__code__,
+        src.__globals__,  # type: ignore
+        name=name or src.__name__,
+        argdefs=src.__defaults__,  # type: ignore
+        closure=src.__closure__,  # type: ignore
+    )
+    dst.__dict__.update(copy.deepcopy(src.__dict__))
+    dst = functools.update_wrapper(dst, src)  # type: ignore
+    dst.__kwdefaults__ = src.__kwdefaults__  # type: ignore
+    return dst
+
+
+class Func(FunctionDecorator):
+    def __init__(
+        self,
+        func: Callable,
+        python: _typing.Python = None,
+        reuse_venv: Optional[bool] = None,
+        name: Optional[str] = None,
+        venv_backend: Any = None,
+        venv_params: Any = None,
+    ):
+        self.func = func
+        self.python = python
+        self.reuse_venv = reuse_venv
+        self.venv_backend = venv_backend
+        self.venv_params = venv_params
+
+    def __call__(self, *args: Any, **kwargs: Any) -> Any:
+        return self.func(*args, **kwargs)
+
+    def copy(self, name: str = None) -> "Func":
+        return Func(
+            _copy_func(self.func, name),
+            self.python,
+            self.reuse_venv,
+            name,
+            self.venv_backend,
+            self.venv_params,
+        )
+
+
+class Call(Func):
+    def __init__(self, func: Func, param_spec: "Param") -> None:
+        super().__init__(
+            func,
+            func.python,
+            func.reuse_venv,
+            None,
+            func.venv_backend,
+            func.venv_params,
+        )
+        self.param_spec = param_spec
+        self.session_signature = "({})".format(param_spec)
+
+    def __call__(self, *args: Any, **kwargs: Any) -> Any:
+        kwargs.update(self.param_spec.call_spec)
+        return super().__call__(*args, **kwargs)
+
+    @classmethod
+    def generate_calls(cls, func: Func, param_specs: "Iterable[Param]") -> "List[Call]":
+        return [cls(func, param_spec) for param_spec in param_specs]
diff --git a/nox/_parametrize.py b/nox/_parametrize.py
index 888899f..cd124d9 100644
--- a/nox/_parametrize.py
+++ b/nox/_parametrize.py
@@ -167,25 +167,3 @@ def update_param_specs(
             spec.update(new_spec)
             combined_specs.append(spec)
     return combined_specs
-
-
-def generate_calls(
-    func: Callable[..., Any], param_specs: Iterable[Param]
-) -> List[Callable[..., Any]]:
-    calls = []
-    for param_spec in param_specs:
-
-        def make_call_wrapper(param_spec: Param) -> Callable[..., Any]:
-            @functools.wraps(func)
-            def call_wrapper(*args: Any, **kwargs: Any) -> Any:
-                kwargs.update(param_spec.call_spec)
-                return func(*args, **kwargs)
-
-            return call_wrapper
-
-        call = make_call_wrapper(param_spec)
-        call.session_signature = "({})".format(param_spec)  # type: ignore
-        call.param_spec = param_spec  # type: ignore
-        calls.append(call)
-
-    return calls
diff --git a/nox/_typing.py b/nox/_typing.py
index 1b83f6a..46671ac 100644
--- a/nox/_typing.py
+++ b/nox/_typing.py
@@ -1,4 +1,6 @@
-__all__ = ["TYPE_CHECKING", "NoReturn"]
+__all__ = ["TYPE_CHECKING", "NoReturn", "Python"]
+
+import typing as _typing
 
 try:
     from typing import TYPE_CHECKING
@@ -15,3 +17,5 @@ except ImportError:
         from typing_extensions import NoReturn
     except ImportError:
         pass
+
+Python = _typing.Optional[_typing.Union[str, _typing.Sequence[str], bool]]
diff --git a/nox/manifest.py b/nox/manifest.py
index 31d6cf9..0e8994b 100644
--- a/nox/manifest.py
+++ b/nox/manifest.py
@@ -13,30 +13,13 @@
 # limitations under the License.
 
 import argparse
-import copy
-import functools
 import itertools
-import types
-from typing import Any, Callable, Iterable, Iterator, List, Mapping, Set, Tuple, Union
+from typing import Any, Iterable, Iterator, List, Mapping, Set, Tuple, Union
 
-from nox._parametrize import generate_calls
+from nox._decorators import Call, Func
 from nox.sessions import Session, SessionRunner
 
 
-def _copy_func(src: Callable, name: str = None) -> Callable:
-    dst = types.FunctionType(
-        src.__code__,
-        src.__globals__,  # type: ignore
-        name=name or src.__name__,  # type: ignore
-        argdefs=src.__defaults__,  # type: ignore
-        closure=src.__closure__,  # type: ignore
-    )
-    dst.__dict__.update(copy.deepcopy(src.__dict__))
-    dst = functools.update_wrapper(dst, src)  # type: ignore
-    dst.__kwdefaults__ = src.__kwdefaults__  # type: ignore
-    return dst
-
-
 class Manifest:
     """Session manifest.
 
@@ -54,9 +37,7 @@ class Manifest:
     """
 
     def __init__(
-        self,
-        session_functions: Mapping[str, Callable],
-        global_config: argparse.Namespace,
+        self, session_functions: Mapping[str, "Func"], global_config: argparse.Namespace
     ) -> None:
         self._all_sessions = []  # type: List[SessionRunner]
         self._queue = []  # type: List[SessionRunner]
@@ -162,7 +143,7 @@ class Manifest:
         ]
 
     def make_session(
-        self, name: str, func: Callable, multi: bool = False
+        self, name: str, func: "Func", multi: bool = False
     ) -> List[SessionRunner]:
         """Create a session object from the session function.
 
@@ -180,13 +161,12 @@ class Manifest:
 
         # If the func has the python attribute set to a list, we'll need
         # to expand them.
-        if isinstance(func.python, (list, tuple, set)):  # type: ignore
+        if isinstance(func.python, (list, tuple, set)):
 
-            for python in func.python:  # type: ignore
-                single_func = _copy_func(func)
-                single_func.python = python  # type: ignore
-                session = self.make_session(name, single_func, multi=True)
-                sessions.extend(session)
+            for python in func.python:
+                single_func = func.copy()
+                single_func.python = python
+                sessions.extend(self.make_session(name, single_func, multi=True))
 
             return sessions
 
@@ -196,31 +176,25 @@ class Manifest:
             long_names = []
             if not multi:
                 long_names.append(name)
-            if func.python:  # type: ignore
-                long_names.append("{}-{}".format(name, func.python))  # type: ignore
+            if func.python:
+                long_names.append("{}-{}".format(name, func.python))
 
-            session = SessionRunner(  # type: ignore
-                name, long_names, func, self._config, self
-            )
-            return [session]  # type: ignore
+            return [SessionRunner(name, long_names, func, self._config, self)]
 
         # Since this function is parametrized, we need to add a distinct
         # session for each permutation.
-        calls = generate_calls(func, func.parametrize)  # type: ignore
+        parametrize = func.parametrize  # type: ignore
+        calls = Call.generate_calls(func, parametrize)
         for call in calls:
             long_names = []
             if not multi:
+                long_names.append("{}{}".format(name, call.session_signature))
+            if func.python:
                 long_names.append(
-                    "{}{}".format(name, call.session_signature)  # type: ignore
-                )
-            if func.python:  # type: ignore
-                long_names.append(
-                    "{}-{}{}".format(
-                        name, func.python, call.session_signature  # type: ignore
-                    )
+                    "{}-{}{}".format(name, func.python, call.session_signature)
                 )
                 # Ensure that specifying session-python will run all parameterizations.
-                long_names.append("{}-{}".format(name, func.python))  # type: ignore
+                long_names.append("{}-{}".format(name, func.python))
 
             sessions.append(SessionRunner(name, long_names, call, self._config, self))
 
@@ -294,9 +268,9 @@ def keyword_match(expression: str, keywords: Iterable[str]) -> Any:
     return eval(expression, {}, locals)  # type: ignore
 
 
-def _null_session_func(session: Session) -> None:
+def _null_session_func_(session: Session) -> None:
     """A no-op session for patemetrized sessions with no available params."""
     session.skip("This session had no parameters available.")
 
 
-_null_session_func.python = False  # type: ignore
+_null_session_func = Func(_null_session_func_, python=False)
diff --git a/nox/registry.py b/nox/registry.py
index e1396ed..3a3d0a5 100644
--- a/nox/registry.py
+++ b/nox/registry.py
@@ -15,10 +15,12 @@
 import collections
 import copy
 import functools
-from typing import Any, Callable, Optional, Sequence, Union
+from typing import Any, Callable, Optional
 
-_REGISTRY = collections.OrderedDict()  # type: collections.OrderedDict[str, Callable]
-Python = Optional[Union[str, Sequence[str], bool]]
+from ._decorators import Func
+from ._typing import Python
+
+_REGISTRY = collections.OrderedDict()  # type: collections.OrderedDict[str, Func]
 
 
 def session_decorator(
@@ -58,16 +60,12 @@ def session_decorator(
     if python is None:
         python = py
 
-    func.python = python  # type: ignore
-    func.reuse_venv = reuse_venv  # type: ignore
-    func.venv_backend = venv_backend  # type: ignore
-    func.venv_params = venv_params  # type: ignore
-    _REGISTRY[name or func.__name__] = func
-
-    return func
+    fn = Func(func, python, reuse_venv, name, venv_backend, venv_params)
+    _REGISTRY[name or func.__name__] = fn
+    return fn
 
 
-def get() -> "collections.OrderedDict[str, Callable]":
+def get() -> "collections.OrderedDict[str, Func]":
     """Return a shallow copy of the registry.
 
     This ensures that the registry is not accidentally modified by
diff --git a/nox/sessions.py b/nox/sessions.py
index 4f75cd7..eb523f0 100644
--- a/nox/sessions.py
+++ b/nox/sessions.py
@@ -34,6 +34,7 @@ from typing import (
 import nox.command
 import py  # type: ignore
 from nox import _typing
+from nox._decorators import Func
 from nox.logger import logger
 from nox.virtualenv import CondaEnv, ProcessEnv, VirtualEnv
 
@@ -124,7 +125,7 @@ class Session:
     @property
     def python(self) -> Optional[Union[str, Sequence[str], bool]]:
         """The python version passed into ``@nox.session``."""
-        return self._runner.func.python  # type: ignore
+        return self._runner.func.python
 
     @property
     def bin(self) -> str:
@@ -356,7 +357,7 @@ class SessionRunner:
         self,
         name: str,
         signatures: List[str],
-        func: Callable,
+        func: Func,
         global_config: argparse.Namespace,
         manifest: "Optional[Manifest]" = None,
     ) -> None:
@@ -384,45 +385,41 @@ class SessionRunner:
         return self.signatures[0] if self.signatures else self.name
 
     def _create_venv(self) -> None:
-        if self.func.python is False:  # type: ignore
+        if self.func.python is False:
             self.venv = ProcessEnv()
             return
 
         path = _normalize_path(self.global_config.envdir, self.friendly_name)
         reuse_existing = (
-            self.func.reuse_venv  # type: ignore
-            or self.global_config.reuse_existing_virtualenvs
+            self.func.reuse_venv or self.global_config.reuse_existing_virtualenvs
         )
 
-        if (
-            not self.func.venv_backend  # type: ignore
-            or self.func.venv_backend == "virtualenv"  # type: ignore
-        ):
+        if not self.func.venv_backend or self.func.venv_backend == "virtualenv":
             self.venv = VirtualEnv(
                 path,
                 interpreter=self.func.python,  # type: ignore
                 reuse_existing=reuse_existing,
-                venv_params=self.func.venv_params,  # type: ignore
+                venv_params=self.func.venv_params,
             )
-        elif self.func.venv_backend == "conda":  # type: ignore
+        elif self.func.venv_backend == "conda":
             self.venv = CondaEnv(
                 path,
                 interpreter=self.func.python,  # type: ignore
                 reuse_existing=reuse_existing,
-                venv_params=self.func.venv_params,  # type: ignore
+                venv_params=self.func.venv_params,
             )
-        elif self.func.venv_backend == "venv":  # type: ignore
+        elif self.func.venv_backend == "venv":
             self.venv = VirtualEnv(
                 path,
                 interpreter=self.func.python,  # type: ignore
                 reuse_existing=reuse_existing,
                 venv=True,
-                venv_params=self.func.venv_params,  # type: ignore
+                venv_params=self.func.venv_params,
             )
         else:
             raise ValueError(
                 "Expected venv_backend one of ('virtualenv', 'conda', 'venv'), but got '{}'.".format(
-                    self.func.venv_backend  # type: ignore
+                    self.func.venv_backend
                 )
             )
 
diff --git a/noxfile.py b/noxfile.py
index 115084c..ec43aa7 100644
--- a/noxfile.py
+++ b/noxfile.py
@@ -67,7 +67,7 @@ def blacken(session):
 @nox.session(python="3.8")
 def lint(session):
     session.install("flake8==3.7.8", "black==19.3b0", "mypy==0.720")
-    session.run("mypy", "--disallow-untyped-defs", "nox")
+    session.run("mypy", "--disallow-untyped-defs", "--warn-unused-ignores", "nox")
     files = ["nox", "tests", "noxfile.py", "setup.py"]
     session.run("black", "--check", *files)
     session.run("flake8", "nox", *files)


Test patch:
diff --git a/tests/test__parametrize.py b/tests/test__parametrize.py
index ece85dc..9e2d9a7 100644
--- a/tests/test__parametrize.py
+++ b/tests/test__parametrize.py
@@ -15,7 +15,7 @@
 from unittest import mock
 
 import pytest
-from nox import _parametrize
+from nox import _decorators, _parametrize
 
 
 @pytest.mark.parametrize(
@@ -166,7 +166,7 @@ def test_generate_calls_simple():
         _parametrize.Param(3, arg_names=arg_names),
     ]
 
-    calls = _parametrize.generate_calls(f, call_specs)
+    calls = _decorators.Call.generate_calls(f, call_specs)
 
     assert len(calls) == 3
     assert calls[0].session_signature == "(abc=1)"
@@ -198,7 +198,7 @@ def test_generate_calls_multiple_args():
         _parametrize.Param(3, "c", arg_names=arg_names),
     ]
 
-    calls = _parametrize.generate_calls(f, call_specs)
+    calls = _decorators.Call.generate_calls(f, call_specs)
 
     assert len(calls) == 3
     assert calls[0].session_signature == "(abc=1, foo='a')"
@@ -223,7 +223,7 @@ def test_generate_calls_ids():
         _parametrize.Param(2, arg_names=arg_names, id="b"),
     ]
 
-    calls = _parametrize.generate_calls(f, call_specs)
+    calls = _decorators.Call.generate_calls(f, call_specs)
 
     assert len(calls) == 2
     assert calls[0].session_signature == "(a)"
diff --git a/tests/test_manifest.py b/tests/test_manifest.py
index e735e26..62b1664 100644
--- a/tests/test_manifest.py
+++ b/tests/test_manifest.py
@@ -17,6 +17,7 @@ from unittest import mock
 
 import nox
 import pytest
+from nox._decorators import Func
 from nox.manifest import Manifest, _null_session_func
 
 
@@ -173,9 +174,8 @@ def test_add_session_multiple_pythons():
     def session_func():
         pass
 
-    session_func.python = ["3.5", "3.6"]
-
-    for session in manifest.make_session("my_session", session_func):
+    func = Func(session_func, python=["3.5", "3.6"])
+    for session in manifest.make_session("my_session", func):
         manifest.add_session(session)
 
     assert len(manifest) == 2
@@ -189,10 +189,10 @@ def test_add_session_parametrized():
     def my_session(session, param):
         pass
 
-    my_session.python = None
+    func = Func(my_session, python=None)
 
     # Add the session to the manifest.
-    for session in manifest.make_session("my_session", my_session):
+    for session in manifest.make_session("my_session", func):
         manifest.add_session(session)
     assert len(manifest) == 3
 
@@ -205,10 +205,10 @@ def test_add_session_parametrized_multiple_pythons():
     def my_session(session, param):
         pass
 
-    my_session.python = ["2.7", "3.6"]
+    func = Func(my_session, python=["2.7", "3.6"])
 
     # Add the session to the manifest.
-    for session in manifest.make_session("my_session", my_session):
+    for session in manifest.make_session("my_session", func):
         manifest.add_session(session)
     assert len(manifest) == 4