Skip to content

Build: Initial mypy integration #1395

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Jun 1, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions requirements-test.txt
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,5 @@ coverage
black
isort
pylint
mypy
bandit
7 changes: 7 additions & 0 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,10 @@ ignore =
requirements-dev.txt
.travis.yml
.coveragerc

[mypy]
warn_unused_configs = True
files = tuf/api/

[mypy-securesystemslib.*]
ignore_missing_imports = True
13 changes: 8 additions & 5 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,18 @@ commands =
python -m coverage report -m

[testenv:lint]
changedir = {toxinidir}
commands =
# Use different configs for new (tuf/api/*) and legacy code
# TODO: configure black and isort args in pyproject.toml (see #1161)
black --check --diff --line-length 80 {toxinidir}/tuf/api
isort --check --diff --line-length 80 --profile black -p tuf {toxinidir}/tuf/api
pylint {toxinidir}/tuf/api --rcfile={toxinidir}/tuf/api/pylintrc
black --check --diff --line-length 80 tuf/api
isort --check --diff --line-length 80 --profile black -p tuf tuf/api
pylint -j 0 tuf/api --rcfile=tuf/api/pylintrc

# NOTE: Contrary to what the pylint docs suggest, ignoring full paths does
# work, unfortunately each subdirectory has to be ignored explicitly.
pylint {toxinidir}/tuf --ignore={toxinidir}/tuf/api,{toxinidir}/tuf/api/serialization
pylint -j 0 tuf --ignore=tuf/api,tuf/api/serialization

bandit -r {toxinidir}/tuf
mypy

bandit -r tuf
73 changes: 45 additions & 28 deletions tuf/api/metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,10 @@
available in the class model.

"""
import abc
import tempfile
from datetime import datetime, timedelta
from typing import Any, Dict, List, Mapping, Optional
from typing import Any, ClassVar, Dict, List, Mapping, Optional, Tuple, Type

from securesystemslib.keys import verify_signature
from securesystemslib.signer import Signature, Signer
Expand Down Expand Up @@ -78,7 +79,7 @@ def from_dict(cls, metadata: Dict[str, Any]) -> "Metadata":
_type = metadata["signed"]["_type"]

if _type == "targets":
inner_cls = Targets
inner_cls: Type[Signed] = Targets
elif _type == "snapshot":
inner_cls = Snapshot
elif _type == "timestamp":
Expand Down Expand Up @@ -304,7 +305,7 @@ def verify(
)


class Signed:
class Signed(metaclass=abc.ABCMeta):
"""A base class for the signed part of TUF metadata.

Objects with base class Signed are usually included in a Metadata object
Expand All @@ -321,7 +322,7 @@ class Signed:
"""

# Signed implementations are expected to override this
_signed_type = None
_signed_type: ClassVar[str] = "signed"

# _type and type are identical: 1st replicates file format, 2nd passes lint
@property
Expand Down Expand Up @@ -351,8 +352,21 @@ def __init__(
self.version = version
self.unrecognized_fields: Mapping[str, Any] = unrecognized_fields or {}

@abc.abstractmethod
def to_dict(self) -> Dict[str, Any]:
"""Serialization helper that returns dict representation of self"""
raise NotImplementedError

@classmethod
@abc.abstractmethod
def from_dict(cls, signed_dict: Dict[str, Any]) -> "Signed":
"""Deserialization helper, creates object from dict representation"""
raise NotImplementedError

@classmethod
def _common_fields_from_dict(cls, signed_dict: Dict[str, Any]) -> List[Any]:
def _common_fields_from_dict(
cls, signed_dict: Dict[str, Any]
) -> Tuple[int, str, datetime]:
"""Returns common fields of 'Signed' instances from the passed dict
representation, and returns an ordered list to be passed as leading
positional arguments to a subclass constructor.
Expand All @@ -371,7 +385,7 @@ def _common_fields_from_dict(cls, signed_dict: Dict[str, Any]) -> List[Any]:
# what the constructor expects and what we store. The inverse operation
# is implemented in '_common_fields_to_dict'.
expires = formats.expiry_string_to_datetime(expires_str)
return [version, spec_version, expires]
return version, spec_version, expires

def _common_fields_to_dict(self) -> Dict[str, Any]:
"""Returns dict representation of common fields of 'Signed' instances.
Expand Down Expand Up @@ -550,20 +564,20 @@ def __init__(
self.roles = roles

@classmethod
def from_dict(cls, root_dict: Dict[str, Any]) -> "Root":
def from_dict(cls, signed_dict: Dict[str, Any]) -> "Root":
"""Creates Root object from its dict representation."""
common_args = cls._common_fields_from_dict(root_dict)
consistent_snapshot = root_dict.pop("consistent_snapshot", None)
keys = root_dict.pop("keys")
roles = root_dict.pop("roles")
common_args = cls._common_fields_from_dict(signed_dict)
consistent_snapshot = signed_dict.pop("consistent_snapshot", None)
keys = signed_dict.pop("keys")
roles = signed_dict.pop("roles")

for keyid, key_dict in keys.items():
keys[keyid] = Key.from_dict(key_dict)
for role_name, role_dict in roles.items():
roles[role_name] = Role.from_dict(role_dict)

# All fields left in the root_dict are unrecognized.
return cls(*common_args, keys, roles, consistent_snapshot, root_dict)
# All fields left in the signed_dict are unrecognized.
return cls(*common_args, keys, roles, consistent_snapshot, signed_dict)

def to_dict(self) -> Dict[str, Any]:
"""Returns the dict representation of self."""
Expand Down Expand Up @@ -646,7 +660,10 @@ def from_dict(cls, meta_dict: Dict[str, Any]) -> "MetaFile":

def to_dict(self) -> Dict[str, Any]:
"""Returns the dictionary representation of self."""
res_dict = {"version": self.version, **self.unrecognized_fields}
res_dict: Dict[str, Any] = {
"version": self.version,
**self.unrecognized_fields,
}

if self.length is not None:
res_dict["length"] = self.length
Expand Down Expand Up @@ -683,13 +700,13 @@ def __init__(
self.meta = meta

@classmethod
def from_dict(cls, timestamp_dict: Dict[str, Any]) -> "Timestamp":
def from_dict(cls, signed_dict: Dict[str, Any]) -> "Timestamp":
"""Creates Timestamp object from its dict representation."""
common_args = cls._common_fields_from_dict(timestamp_dict)
meta_dict = timestamp_dict.pop("meta")
common_args = cls._common_fields_from_dict(signed_dict)
meta_dict = signed_dict.pop("meta")
meta = {"snapshot.json": MetaFile.from_dict(meta_dict["snapshot.json"])}
# All fields left in the timestamp_dict are unrecognized.
return cls(*common_args, meta, timestamp_dict)
return cls(*common_args, meta, signed_dict)

def to_dict(self) -> Dict[str, Any]:
"""Returns the dict representation of self."""
Expand Down Expand Up @@ -733,15 +750,15 @@ def __init__(
self.meta = meta

@classmethod
def from_dict(cls, snapshot_dict: Dict[str, Any]) -> "Snapshot":
def from_dict(cls, signed_dict: Dict[str, Any]) -> "Snapshot":
"""Creates Snapshot object from its dict representation."""
common_args = cls._common_fields_from_dict(snapshot_dict)
meta_dicts = snapshot_dict.pop("meta")
common_args = cls._common_fields_from_dict(signed_dict)
meta_dicts = signed_dict.pop("meta")
meta = {}
for meta_path, meta_dict in meta_dicts.items():
meta[meta_path] = MetaFile.from_dict(meta_dict)
# All fields left in the snapshot_dict are unrecognized.
return cls(*common_args, meta, snapshot_dict)
return cls(*common_args, meta, signed_dict)

def to_dict(self) -> Dict[str, Any]:
"""Returns the dict representation of self."""
Expand Down Expand Up @@ -801,7 +818,7 @@ def __init__(
self.path_hash_prefixes = path_hash_prefixes

@classmethod
def from_dict(cls, role_dict: Mapping[str, Any]) -> "Role":
def from_dict(cls, role_dict: Dict[str, Any]) -> "DelegatedRole":
"""Creates DelegatedRole object from its dict representation."""
name = role_dict.pop("name")
keyids = role_dict.pop("keyids")
Expand Down Expand Up @@ -971,12 +988,12 @@ def __init__(
self.delegations = delegations

@classmethod
def from_dict(cls, targets_dict: Dict[str, Any]) -> "Targets":
def from_dict(cls, signed_dict: Dict[str, Any]) -> "Targets":
"""Creates Targets object from its dict representation."""
common_args = cls._common_fields_from_dict(targets_dict)
targets = targets_dict.pop("targets")
common_args = cls._common_fields_from_dict(signed_dict)
targets = signed_dict.pop("targets")
try:
delegations_dict = targets_dict.pop("delegations")
delegations_dict = signed_dict.pop("delegations")
except KeyError:
delegations = None
else:
Expand All @@ -985,7 +1002,7 @@ def from_dict(cls, targets_dict: Dict[str, Any]) -> "Targets":
for target_path, target_info in targets.items():
res_targets[target_path] = TargetFile.from_dict(target_info)
# All fields left in the targets_dict are unrecognized.
return cls(*common_args, res_targets, delegations, targets_dict)
return cls(*common_args, res_targets, delegations, signed_dict)

def to_dict(self) -> Dict[str, Any]:
"""Returns the dict representation of self."""
Expand Down
5 changes: 5 additions & 0 deletions tuf/api/serialization/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@

"""
import abc
from typing import TYPE_CHECKING

if TYPE_CHECKING:
# pylint: disable=cyclic-import
from tuf.api.metadata import Metadata, Signed
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So mypy recognizes that the quoted strings "Metadata" and "Signed" are the classes in this import?
Cool trick!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, AFAIK for mypy the type-names-as-string are just as valid.



# TODO: Should these be in tuf.exceptions or inherit from tuf.exceptions.Error?
Expand Down