Skip to content

Commit

Permalink
chore: mypy: Disallow untyped definitions
Browse files Browse the repository at this point in the history
Be more strict and don't allow untyped definitions on the files we
check.

Also this adds type-hints for two of the decorators so that now
functions/methods decorated by them will have their types be revealed
correctly.
  • Loading branch information
JohnVillalovos committed Apr 28, 2021
1 parent a6b6cd4 commit 6aef2da
Show file tree
Hide file tree
Showing 8 changed files with 78 additions and 53 deletions.
3 changes: 3 additions & 0 deletions .mypy.ini
Expand Up @@ -4,3 +4,6 @@ files = gitlab/*.py
# disallow_incomplete_defs: This flag reports an error whenever it encounters a
# partly annotated function definition.
disallow_incomplete_defs = True
# disallow_untyped_defs: This flag reports an error whenever it encounters a
# function without type annotations or with incomplete type annotations.
disallow_untyped_defs = True
3 changes: 2 additions & 1 deletion gitlab/__main__.py
@@ -1,4 +1,5 @@
import gitlab.cli


__name__ == "__main__" and gitlab.cli.main()
if __name__ == "__main__":
gitlab.cli.main()
12 changes: 6 additions & 6 deletions gitlab/base.py
Expand Up @@ -17,7 +17,7 @@

import importlib
from types import ModuleType
from typing import Any, Dict, NamedTuple, Optional, Tuple, Type
from typing import Any, Dict, Iterable, NamedTuple, Optional, Tuple, Type

from .client import Gitlab, GitlabList
from gitlab import types as g_types
Expand Down Expand Up @@ -133,8 +133,8 @@ def __ne__(self, other: object) -> bool:
return self.get_id() != other.get_id()
return super(RESTObject, self) != other

def __dir__(self):
return super(RESTObject, self).__dir__() | self.attributes.keys()
def __dir__(self) -> Iterable[str]:
return set(self.attributes).union(super(RESTObject, self).__dir__())

def __hash__(self) -> int:
if not self.get_id():
Expand All @@ -155,7 +155,7 @@ def _update_attrs(self, new_attrs: Dict[str, Any]) -> None:
self.__dict__["_updated_attrs"] = {}
self.__dict__["_attrs"] = new_attrs

def get_id(self):
def get_id(self) -> Any:
"""Returns the id of the resource."""
if self._id_attr is None or not hasattr(self, self._id_attr):
return None
Expand Down Expand Up @@ -207,10 +207,10 @@ def __iter__(self) -> "RESTObjectList":
def __len__(self) -> int:
return len(self._list)

def __next__(self):
def __next__(self) -> RESTObject:
return self.next()

def next(self):
def next(self) -> RESTObject:
data = self._list.next()
return self._obj_cls(self.manager, data)

Expand Down
55 changes: 28 additions & 27 deletions gitlab/cli.py
Expand Up @@ -21,7 +21,7 @@
import functools
import re
import sys
from typing import Any, Callable, Dict, Optional, Tuple, Union
from typing import Any, Callable, cast, Dict, Optional, Tuple, TypeVar, Union

import gitlab.config # noqa: F401

Expand All @@ -35,14 +35,21 @@
custom_actions: Dict[str, Dict[str, Tuple[Tuple[str, ...], Tuple[str, ...], bool]]] = {}


# For an explanation of how these type-hints work see:
# https://mypy.readthedocs.io/en/stable/generics.html#declaring-decorators
#
# The goal here is that functions which get decorated will retain their types.
__F = TypeVar("__F", bound=Callable[..., Any])


def register_custom_action(
cls_names: Union[str, Tuple[str, ...]],
mandatory: Tuple[str, ...] = tuple(),
optional: Tuple[str, ...] = tuple(),
) -> Callable:
def wrap(f: Callable) -> Callable:
) -> Callable[[__F], __F]:
def wrap(f: __F) -> __F:
@functools.wraps(f)
def wrapped_f(*args, **kwargs):
def wrapped_f(*args: Any, **kwargs: Any) -> Any:
return f(*args, **kwargs)

# in_obj defines whether the method belongs to the obj or the manager
Expand All @@ -63,7 +70,7 @@ def wrapped_f(*args, **kwargs):
action = f.__name__.replace("_", "-")
custom_actions[final_name][action] = (mandatory, optional, in_obj)

return wrapped_f
return cast(__F, wrapped_f)

return wrap

Expand Down Expand Up @@ -135,12 +142,16 @@ def _get_base_parser(add_help: bool = True) -> argparse.ArgumentParser:
return parser


def _get_parser(cli_module):
def _get_parser() -> argparse.ArgumentParser:
# NOTE: We must delay import of gitlab.v4.cli until now or
# otherwise it will cause circular import errors
import gitlab.v4.cli

parser = _get_base_parser()
return cli_module.extend_parser(parser)
return gitlab.v4.cli.extend_parser(parser)


def _parse_value(v):
def _parse_value(v: Any) -> Any:
if isinstance(v, str) and v.startswith("@"):
# If the user-provided value starts with @, we try to read the file
# path provided after @ as the real value. Exit on any error.
Expand All @@ -162,18 +173,10 @@ def docs() -> argparse.ArgumentParser:
if "sphinx" not in sys.modules:
sys.exit("Docs parser is only intended for build_sphinx")

# NOTE: We must delay import of gitlab.v4.cli until now or
# otherwise it will cause circular import errors
import gitlab.v4.cli

return _get_parser(gitlab.v4.cli)

return _get_parser()

def main():
# NOTE: We must delay import of gitlab.v4.cli until now or
# otherwise it will cause circular import errors
import gitlab.v4.cli

def main() -> None:
if "--version" in sys.argv:
print(gitlab.__version__)
sys.exit(0)
Expand All @@ -183,7 +186,7 @@ def main():
# This first parsing step is used to find the gitlab config to use, and
# load the propermodule (v3 or v4) accordingly. At that point we don't have
# any subparser setup
(options, args) = parser.parse_known_args(sys.argv)
(options, _) = parser.parse_known_args(sys.argv)
try:
config = gitlab.config.GitlabConfigParser(options.gitlab, options.config_file)
except gitlab.config.ConfigError as e:
Expand All @@ -196,14 +199,14 @@ def main():
raise ModuleNotFoundError(name="gitlab.v%s.cli" % config.api_version)

# Now we build the entire set of subcommands and do the complete parsing
parser = _get_parser(gitlab.v4.cli)
parser = _get_parser()
try:
import argcomplete # type: ignore

argcomplete.autocomplete(parser)
except Exception:
pass
args = parser.parse_args(sys.argv[1:])
args = parser.parse_args()

config_files = args.config_file
gitlab_id = args.gitlab
Expand All @@ -216,7 +219,7 @@ def main():
action = args.whaction
what = args.what

args = args.__dict__
args_dict = vars(args)
# Remove CLI behavior-related args
for item in (
"gitlab",
Expand All @@ -228,8 +231,8 @@ def main():
"version",
"output",
):
args.pop(item)
args = {k: _parse_value(v) for k, v in args.items() if v is not None}
args_dict.pop(item)
args_dict = {k: _parse_value(v) for k, v in args_dict.items() if v is not None}

try:
gl = gitlab.Gitlab.from_config(gitlab_id, config_files)
Expand All @@ -241,6 +244,4 @@ def main():
if debug:
gl.enable_debug()

gitlab.v4.cli.run(gl, what, action, args, verbose, output, fields)

sys.exit(0)
gitlab.v4.cli.run(gl, what, action, args_dict, verbose, output, fields)
2 changes: 1 addition & 1 deletion gitlab/config.py
Expand Up @@ -206,7 +206,7 @@ def __init__(
except Exception:
pass

def _get_values_from_helper(self):
def _get_values_from_helper(self) -> None:
"""Update attributes that may get values from an external helper program"""
for attr in HELPER_ATTRIBUTES:
value = getattr(self, attr)
Expand Down
29 changes: 23 additions & 6 deletions gitlab/exceptions.py
Expand Up @@ -16,10 +16,16 @@
# along with this program. If not, see <http://www.gnu.org/licenses/>.

import functools
from typing import Any, Callable, cast, Optional, Type, TypeVar, TYPE_CHECKING, Union


class GitlabError(Exception):
def __init__(self, error_message="", response_code=None, response_body=None):
def __init__(
self,
error_message: Union[str, bytes] = "",
response_code: Optional[int] = None,
response_body: Optional[bytes] = None,
) -> None:

Exception.__init__(self, error_message)
# Http status code
Expand All @@ -30,11 +36,15 @@ def __init__(self, error_message="", response_code=None, response_body=None):
try:
# if we receive str/bytes we try to convert to unicode/str to have
# consistent message types (see #616)
if TYPE_CHECKING:
assert isinstance(error_message, bytes)
self.error_message = error_message.decode()
except Exception:
if TYPE_CHECKING:
assert isinstance(error_message, str)
self.error_message = error_message

def __str__(self):
def __str__(self) -> str:
if self.response_code is not None:
return "{0}: {1}".format(self.response_code, self.error_message)
else:
Expand Down Expand Up @@ -269,7 +279,14 @@ class GitlabUnfollowError(GitlabOperationError):
pass


def on_http_error(error):
# For an explanation of how these type-hints work see:
# https://mypy.readthedocs.io/en/stable/generics.html#declaring-decorators
#
# The goal here is that functions which get decorated will retain their types.
__F = TypeVar("__F", bound=Callable[..., Any])


def on_http_error(error: Type[Exception]) -> Callable[[__F], __F]:
"""Manage GitlabHttpError exceptions.
This decorator function can be used to catch GitlabHttpError exceptions
Expand All @@ -280,14 +297,14 @@ def on_http_error(error):
GitlabError
"""

def wrap(f):
def wrap(f: __F) -> __F:
@functools.wraps(f)
def wrapped_f(*args, **kwargs):
def wrapped_f(*args: Any, **kwargs: Any) -> Any:
try:
return f(*args, **kwargs)
except GitlabHttpError as e:
raise error(e.error_message, e.response_code, e.response_body) from e

return wrapped_f
return cast(__F, wrapped_f)

return wrap
5 changes: 2 additions & 3 deletions gitlab/tests/test_cli.py
Expand Up @@ -26,7 +26,6 @@
import pytest

from gitlab import cli
import gitlab.v4.cli


def test_what_to_cls():
Expand Down Expand Up @@ -94,14 +93,14 @@ def test_base_parser():


def test_v4_parse_args():
parser = cli._get_parser(gitlab.v4.cli)
parser = cli._get_parser()
args = parser.parse_args(["project", "list"])
assert args.what == "project"
assert args.whaction == "list"


def test_v4_parser():
parser = cli._get_parser(gitlab.v4.cli)
parser = cli._get_parser()
subparsers = next(
action
for action in parser._actions
Expand Down
22 changes: 13 additions & 9 deletions gitlab/types.py
Expand Up @@ -15,46 +15,50 @@
# You should have received a copy of the GNU Lesser General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.

from typing import Any, Optional, TYPE_CHECKING


class GitlabAttribute(object):
def __init__(self, value=None):
def __init__(self, value: Any = None) -> None:
self._value = value

def get(self):
def get(self) -> Any:
return self._value

def set_from_cli(self, cli_value):
def set_from_cli(self, cli_value: Any) -> None:
self._value = cli_value

def get_for_api(self):
def get_for_api(self) -> Any:
return self._value


class ListAttribute(GitlabAttribute):
def set_from_cli(self, cli_value):
def set_from_cli(self, cli_value: str) -> None:
if not cli_value.strip():
self._value = []
else:
self._value = [item.strip() for item in cli_value.split(",")]

def get_for_api(self):
def get_for_api(self) -> str:
# Do not comma-split single value passed as string
if isinstance(self._value, str):
return self._value

if TYPE_CHECKING:
assert isinstance(self._value, list)
return ",".join([str(x) for x in self._value])


class LowercaseStringAttribute(GitlabAttribute):
def get_for_api(self):
def get_for_api(self) -> str:
return str(self._value).lower()


class FileAttribute(GitlabAttribute):
def get_file_name(self, attr_name=None):
def get_file_name(self, attr_name: Optional[str] = None) -> Optional[str]:
return attr_name


class ImageAttribute(FileAttribute):
def get_file_name(self, attr_name=None):
def get_file_name(self, attr_name: Optional[str] = None) -> str:
return "%s.png" % attr_name if attr_name else "image.png"

0 comments on commit 6aef2da

Please sign in to comment.