style(mypy): Remove most “type: ignore” pragmas

Description

Abstract

Be more precise about typing to be able to remove many or our pragmas telling mypy to ignore some parts of the code, especially in the isshub.domain.utils.entity module.

Motivation

Simply calling Repository(name=name, namespace=namespace) made mypy fail. So it was necessary to let mypy understand our layer above the attr package. Also, having all these # type: ignore pragmas was not really what we wanted, it was temporary until having time to fix it.

Rationale

• Add a mypy plugin to let mypy know about our own functions to define attr classes and fields

• Add typing in the isshub.domain.utils.entity to be able to remove all our # type: ignore pragmas when defining classes and fields

During this last step, we had to change the field_validator decorator from a function to a class because of an error from mypy that kept telling us that the functions decorated by this decorated were made untyped because the decorator was untyped… but it was not.

Info

Hash

76638c3d09c47d58febbc9e2e2cc80e84c98ac33

Date

2020-10-04 20:36:50 +0200

Parents

• docs(domain): Add diagram of entities for each domain context [bb5e73eb] — 2020-10-04 11:50:37 +0200

Children

• style(entity): Change the world “model” by “entity” [c2dd0fc6] — 2020-10-04 21:07:00 +0200

Branches

• develop

Tags

(No tags)

Changes

isshub/domain/contexts/code_repository/entities/namespace/__init__.py

Note

View last source and history

Type

Modified

Stats

+6 -8

@@ -20,7 +20,7 @@ class NamespaceKind(enum.Enum):
     GROUP = "Group"


-@validated()  # type: ignore
+@validated()
 class Namespace(BaseModelWithId):
     """A namespace can contain namespaces and repositories.

@@ -39,16 +39,14 @@ class Namespace(BaseModelWithId):

     """

-    name: str = required_field(str)  # type: ignore
-    kind: NamespaceKind = required_field(  # type: ignore
-        NamespaceKind, relation_verbose_name="is a"
-    )
-    namespace: Optional["Namespace"] = optional_field(  # type: ignore
+    name: str = required_field(str)
+    kind: NamespaceKind = required_field(NamespaceKind, relation_verbose_name="is a")
+    namespace: Optional["Namespace"] = optional_field(
         "self", relation_verbose_name="may belongs to"
     )
-    description: Optional[str] = optional_field(str)  # type: ignore
+    description: Optional[str] = optional_field(str)

-    @field_validator(namespace)  # type: ignore
+    @field_validator(namespace)
     def validate_namespace_is_not_in_a_loop(  # noqa  # pylint: disable=unused-argument
         self, field: Any, value: Any
     ) -> None:

isshub/domain/contexts/code_repository/entities/repository/__init__.py

Note

View last source and history

Type

Modified

Stats

+3 -5

@@ -4,7 +4,7 @@ from isshub.domain.contexts.code_repository.entities.namespace import Namespace
 from isshub.domain.utils.entity import BaseModelWithId, required_field, validated


-@validated()  # type: ignore
+@validated()
 class Repository(BaseModelWithId):
     """A repository holds code, issues, code requests...

@@ -19,7 +19,5 @@ class Repository(BaseModelWithId):

     """

-    name: str = required_field(str)  # type: ignore
-    namespace: Namespace = required_field(  # type: ignore
-        Namespace, relation_verbose_name="belongs to"
-    )
+    name: str = required_field(str)
+    namespace: Namespace = required_field(Namespace, relation_verbose_name="belongs to")

isshub/domain/utils/entity.py

Note

View last source and history

Type

Modified

Stats

+86 -21

@@ -3,18 +3,37 @@
 It is an adapter over the ``attrs`` external dependency.

 """
-
-# type: ignore
+from typing import (
+    TYPE_CHECKING,
+    Any,
+    Callable,
+    Generic,
+    Optional,
+    Type,
+    TypeVar,
+    Union,
+    cast,
+)

 import attr


+_T = TypeVar("_T")
+
+if TYPE_CHECKING:
+    from attr.__init__ import Attribute  # isort:skip
+else:
+
+    class Attribute(Generic[_T]):
+        """Class for typing when not using mypy, for example when using ``get_type_hints``."""
+
+
 class _InstanceOfSelfValidator(
-    attr.validators._InstanceOfValidator  # pylint: disable=protected-access
+    attr.validators._InstanceOfValidator  # type: ignore  # pylint: disable=protected-access
 ):
     """Validator checking that the field holds an instance of its own model."""

-    def __call__(self, inst, attr, value):  # pylint: disable=redefined-outer-name
+    def __call__(self, inst, attr, value):  # type: ignore  # pylint: disable=redefined-outer-name
         """Validate that the `value` is an instance of the class of `inst`.

         For the parameters, see ``attr.validators._InstanceOfValidator``
@@ -34,7 +53,9 @@ def instance_of_self() -> _InstanceOfSelfValidator:
     return _InstanceOfSelfValidator(type=None)


-def optional_field(field_type, relation_verbose_name=None):
+def optional_field(
+    field_type: Union[Type[_T], str], relation_verbose_name: Optional[str] = None
+) -> Optional[_T]:
     """Define an optional field of the specified `field_type`.

     Parameters
@@ -51,6 +72,11 @@ def optional_field(field_type, relation_verbose_name=None):
         An ``attrs`` attribute, with a default value set to ``None``, and a validator checking
         that this field is optional and, if set, of the correct type.

+    Raises
+    ------
+    AssertionError
+        If `field_type` is a string and this string is not "self"
+
     Examples
     --------
     >>> from isshub.domain.utils.entity import optional_field, validated, BaseModel
@@ -67,18 +93,24 @@ def optional_field(field_type, relation_verbose_name=None):
     if relation_verbose_name:
         metadata["relation_verbose_name"] = relation_verbose_name

+    assert not isinstance(field_type, str) or field_type == "self"
+
     return attr.ib(
         default=None,
         validator=attr.validators.optional(
             instance_of_self()
-            if field_type == "self"
+            if isinstance(field_type, str)
             else attr.validators.instance_of(field_type)
         ),
         metadata=metadata,
     )


-def required_field(field_type, frozen=False, relation_verbose_name=None):
+def required_field(
+    field_type: Union[Type[_T], str],
+    frozen: bool = False,
+    relation_verbose_name: Optional[str] = None,
+) -> _T:
     """Define a required field of the specified `field_type`.

     Parameters
@@ -98,6 +130,11 @@ def required_field(field_type, frozen=False, relation_verbose_name=None):
     Any
         An ``attrs`` attribute, and a validator checking that this field is of the correct type.

+    Raises
+    ------
+    AssertionError
+        If `field_type` is a string and this string is not "self"
+
     Examples
     --------
     >>> from isshub.domain.utils.entity import required_field, validated, BaseModel
@@ -114,19 +151,21 @@ def required_field(field_type, frozen=False, relation_verbose_name=None):
     if relation_verbose_name:
         metadata["relation_verbose_name"] = relation_verbose_name

+    assert not isinstance(field_type, str) or field_type == "self"
+
     kwargs = {
         "validator": instance_of_self()
-        if field_type == "self"
+        if isinstance(field_type, str)
         else attr.validators.instance_of(field_type),
         "metadata": metadata,
     }
     if frozen:
         kwargs["on_setattr"] = attr.setters.frozen

-    return attr.ib(**kwargs)
+    return attr.ib(**kwargs)  # type: ignore


-def validated():
+def validated() -> Any:
     """Decorate an entity to handle validation.

     This will let ``attrs`` manage the class, using slots for fields, and forcing attributes to
@@ -167,19 +206,24 @@ def validated():
     return attr.s(slots=True, kw_only=True)


-def field_validator(field):
+TValidateMethod = TypeVar(
+    "TValidateMethod", bound=Callable[[Any, "Attribute[_T]", _T], None]
+)
+
+
+class field_validator:  # pylint: disable=invalid-name
     """Decorate an entity method to make it a validator of the given `field`.

+    Notes
+    -----
+    It's easier to implement as a function but we couldn't make mypy work with it.
+    Thanks to https://github.com/python/mypy/issues/1551#issuecomment-253978622
+
     Parameters
     ----------
     field : Any
         The field to validate.

-    Returns
-    -------
-    Callable
-        The decorated method.
-
     Examples
     --------
     >>> from isshub.domain.utils.entity import field_validator, required_field, BaseModel
@@ -211,10 +255,29 @@ def field_validator(field):
     'foo'

     """
-    return field.validator
+
+    def __init__(self, field: "Attribute[_T]") -> None:
+        """Save the given field."""
+        self.field = field
+
+    def __call__(self, func: TValidateMethod) -> TValidateMethod:
+        """Decorate the given function.
+
+        Parameters
+        ----------
+        func: Callable
+            The validation method to decorate
+
+        Returns
+        -------
+        Callable
+            The decorated method.
+
+        """
+        return cast(TValidateMethod, self.field.validator(func))


-def validate_instance(instance):
+def validate_instance(instance: Any) -> Any:
     """Validate a whole instance.

     Parameters
@@ -247,7 +310,9 @@ def validate_instance(instance):
     attr.validate(instance)


-def validate_positive_integer(value, none_allowed, display_name):
+def validate_positive_integer(
+    value: Any, none_allowed: bool, display_name: str
+) -> None:
     """Validate that the given `value` is a positive integer (``None`` accepted if `none_allowed`).

     Parameters
@@ -346,8 +411,8 @@ class BaseModelWithId(BaseModel):

     @field_validator(id)
     def validate_id_is_positive_integer(  # noqa  # pylint: disable=unused-argument
-        self, field, value
-    ):
+        self, field: "Attribute[_T]", value: _T
+    ) -> None:
         """Validate that the ``id`` field is a positive integer.

         Parameters

isshub/domain/utils/mypy_plugin.py

Note

View last source and history

Type

Added

Stats

+45 -0

@@ -0,0 +1,45 @@
+"""Mypy plugin to declare our own functions to create ``attr`` classes and fields.
+
+Inspired by https://github.com/python/mypy/issues/5406#issuecomment-490547091
+
+"""
+
+# type: ignore
+
+# pylint: disable=no-name-in-module
+from mypy.plugin import Plugin
+from mypy.plugins.attrs import attr_attrib_makers, attr_class_makers
+
+
+# pylint: enable=no-name-in-module
+
+
+attr_class_makers.add("isshub.domain.utils.entity.validated")
+attr_attrib_makers.add("isshub.domain.utils.entity.optional_field")
+attr_attrib_makers.add("isshub.domain.utils.entity.required_field")
+
+
+class MypyPlugin(Plugin):
+    """Plugin class for mypy.
+
+    Notes
+    -----
+    Our plugin does nothing but it has to exist so this file gets loaded.
+    """
+
+
+def plugin(version: str):  # pylint: disable=unused-argument
+    """Define the plugin to use.
+
+    Parameters
+    ----------
+    version : str
+        The version for which to return a plugin class. Ignored in our case.
+
+    Returns
+    -------
+    Type[Plugin]
+        The plugin class to be used by mypy
+
+    """
+    return MypyPlugin

setup.cfg

Note

View last source and history

Type

Modified

Stats

+3 -1

@@ -81,13 +81,15 @@ ignore_missing_imports = true
 python_version = 3.6
 strict_optional = true
 warn_incomplete_stub = true
+plugins = isshub.domain.utils.mypy_plugin

 [mypy-isshub.*.tests.*]
 ignore_errors = True

-[mypy-isshub.domain.utils.entity]
+[mypy-isshub.domain.utils.mypy_plugin]
 ignore_errors = True

+
 [flake8]
 ignore =
     # Line too long: we let black manage it