chore: add metthod definition order plugin to pylint

Also reorder some methods to fix errors
This commit is contained in:
Evgenii Alekseev 2023-11-04 16:16:14 +02:00
parent eec94521a7
commit df787657aa
17 changed files with 323 additions and 150 deletions

View File

@ -67,7 +67,7 @@ ignored-modules=
# Python code to execute, usually for sys.path manipulation such as
# pygtk.require().
#init-hook=
init-hook='import sys; sys.path.append("pylint_plugins")'
# Use multiple processes to speed up Pylint. Specifying 0 will auto-detect the
# number of processors available to use, and will cap the count on Windows to
@ -81,7 +81,8 @@ limit-inference-results=100
# List of plugins (as comma separated values of python module names) to load,
# usually to register additional checkers.
load-plugins=
load-plugins=pylint.extensions.docparams,
definition_order,
# Pickle collected data for later comparisons.
persistent=yes
@ -227,7 +228,7 @@ name-group=
# Regular expression which should only match function or class names that do
# not require a docstring.
no-docstring-rgx=^_
no-docstring-rgx=
# List of decorators that produce properties, such as abc.abstractproperty. Add
# to this list to register other decorators that produce valid properties.

View File

@ -0,0 +1,171 @@
#
# Copyright (c) 2021-2023 ahriman team.
#
# This file is part of ahriman
# (see https://github.com/arcan1s/ahriman).
#
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation, either version 3 of the License, or
# (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
#
from astroid import nodes
from collections.abc import Iterable
from enum import Enum
from pylint.checkers import BaseRawFileChecker
from pylint.lint import PyLinter
from typing import Any
class MethodTypeOrder(int, Enum):
"""
method type enumeration
Attributes:
New(MethodTypeOrder): (class attribute) constructor method
Init(MethodTypeOrder): (class attribute) initialization method
Property(MethodTypeOrder): (class attribute) property method
Class(MethodTypeOrder): (class attribute) class method
Static(MethodTypeOrder): (class attribute) static method
Normal(MethodTypeOrder): (class attribute) usual method
Magic(MethodTypeOrder): (class attribute) other magical methods
"""
New = 0
Init = 1
Property = 2
Class = 3
Static = 4
Normal = 5
Magic = 6
class DefinitionOrder(BaseRawFileChecker):
"""
check if methods are defined in recommended order
Attributes:
DECORATED_METHODS_ORDER(list[str]): (class attribute) predefined list of known function decorators
"""
DECORATED_METHODS_ORDER = {
"cached_property": MethodTypeOrder.Property,
"classmethod": MethodTypeOrder.Class,
"property": MethodTypeOrder.Property,
"staticmethod": MethodTypeOrder.Static,
}
name = "method-ordering"
msgs = {
"W0001": (
"Invalid method order %s, expected %s",
"methods-out-of-order",
"Methods are defined out of recommended order.",
)
}
options = ()
@staticmethod
def comparator(function: nodes.FunctionDef) -> tuple[int, str]:
"""
compare key for function node
Args:
function(nodes.FunctionDef): function definition
Returns:
tuple[int, str]: comparison key
"""
# init methods
if function.name in ("__new__",):
return MethodTypeOrder.New, function.name
if function.name in ("__init__", "__post_init__"):
return MethodTypeOrder.Init, function.name
# decorated methods
decorators = []
if function.decorators is not None:
decorators = [getattr(decorator, "name", None) for decorator in function.decorators.get_children()]
for decorator in decorators:
if decorator in DefinitionOrder.DECORATED_METHODS_ORDER:
return DefinitionOrder.DECORATED_METHODS_ORDER[decorator], function.name
# magic methods
if function.name.startswith("__") and function.name.endswith("__"):
return MethodTypeOrder.Magic, function.name
# normal method
return MethodTypeOrder.Normal, function.name
@staticmethod
def methods(source: Iterable[Any], start_lineno: int = 0) -> list[nodes.FunctionDef]:
"""
extract function nodes from list of raw nodes
Args:
source(Iterable[Any]): all available nodes
start_lineno(int, optional): minimal allowed line number (Default value = 0)
Returns:
list[nodes.FunctionDef]: list of function nodes
"""
def is_defined_function(function: Any) -> bool:
return isinstance(function, nodes.FunctionDef) \
and function.lineno is not None \
and function.lineno >= start_lineno
return list(filter(is_defined_function, source))
def check_class(self, clazz: nodes.ClassDef) -> None:
"""
check class functions ordering
Args:
clazz(nodes.ClassDef): class definition
"""
methods = DefinitionOrder.methods(clazz.values(), clazz.lineno)
self.check_functions(methods)
def check_functions(self, functions: list[nodes.FunctionDef]) -> None:
"""
check global functions ordering
Args:
functions(list[nodes.FunctionDef]): list of functions in their defined order
"""
for real, expected in zip(functions, sorted(functions, key=DefinitionOrder.comparator)):
if real == expected:
continue
self.add_message("methods-out-of-order", line=real.lineno, args=(real.name, expected.name))
break
def process_module(self, node: nodes.Module) -> None:
"""
process module
Args:
node(nodes.Module): module node to check
"""
# check global methods
self.check_functions(DefinitionOrder.methods(node.values()))
# check class definitions
for clazz in filter(lambda method: isinstance(method, nodes.ClassDef), node.values()):
self.check_class(clazz)
def register(linter: PyLinter) -> None:
"""
register custom checker
Args:
linter(PyLinter): linter in which checker should be registered
"""
linter.register_checker(DefinitionOrder(linter))

View File

@ -96,8 +96,8 @@ def _parser() -> argparse.ArgumentParser:
subparsers = parser.add_subparsers(title="command", help="command to run", dest="command")
_set_aur_search_parser(subparsers)
_set_help_parser(subparsers)
_set_help_commands_unsafe_parser(subparsers)
_set_help_parser(subparsers)
_set_help_updates_parser(subparsers)
_set_help_version_parser(subparsers)
_set_package_add_parser(subparsers)
@ -166,25 +166,6 @@ def _set_aur_search_parser(root: SubParserAction) -> argparse.ArgumentParser:
return parser
def _set_help_parser(root: SubParserAction) -> argparse.ArgumentParser:
"""
add parser for listing help subcommand
Args:
root(SubParserAction): subparsers for the commands
Returns:
argparse.ArgumentParser: created argument parser
"""
parser = root.add_parser("help", help="show help message",
description="show help message for application or command and exit",
formatter_class=_formatter)
parser.add_argument("command", help="show help message for specific command", nargs="?")
parser.set_defaults(handler=handlers.Help, architecture="", lock=None, quiet=True, report=False, repository="",
unsafe=True, parser=_parser)
return parser
def _set_help_commands_unsafe_parser(root: SubParserAction) -> argparse.ArgumentParser:
"""
add parser for listing unsafe commands
@ -204,6 +185,25 @@ def _set_help_commands_unsafe_parser(root: SubParserAction) -> argparse.Argument
return parser
def _set_help_parser(root: SubParserAction) -> argparse.ArgumentParser:
"""
add parser for listing help subcommand
Args:
root(SubParserAction): subparsers for the commands
Returns:
argparse.ArgumentParser: created argument parser
"""
parser = root.add_parser("help", help="show help message",
description="show help message for application or command and exit",
formatter_class=_formatter)
parser.add_argument("command", help="show help message for specific command", nargs="?")
parser.set_defaults(handler=handlers.Help, architecture="", lock=None, quiet=True, report=False, repository="",
unsafe=True, parser=_parser)
return parser
def _set_help_updates_parser(root: SubParserAction) -> argparse.ArgumentParser:
"""
add parser for service update check subcommand

View File

@ -67,8 +67,8 @@ class Versions(Handler):
Args:
root(str): root package name
Returns:
Generator[tuple[str, str], None, None]: map of installed dependency to its version
Yields:
tuple[str, str]: map of installed dependency to its version
"""
def dependencies_by_key(key: str) -> Generator[str, None, None]:
# in importlib it returns requires in the following format

View File

@ -76,8 +76,8 @@ class Web(Handler):
args(argparse.Namespace): command line args
configuration(Configuration): configuration instance
Returns:
Generator[str, None, None]: command line arguments which were used for this specific command
Yields:
str: command line arguments which were used for this specific command
"""
# read configuration path from current settings
if (configuration_path := configuration.path) is not None:

View File

@ -80,6 +80,13 @@ class Lock(LazyLogging):
self.paths = configuration.repository_paths
self.reporter = Client.load(repository_id, configuration, report=args.report)
def check_user(self) -> None:
"""
check if current user is actually owner of ahriman root
"""
check_user(self.paths, unsafe=self.unsafe)
self.paths.tree_create()
def check_version(self) -> None:
"""
check web server version
@ -89,13 +96,6 @@ class Lock(LazyLogging):
self.logger.warning("status watcher version mismatch, our %s, their %s",
__version__, status.version)
def check_user(self) -> None:
"""
check if current user is actually owner of ahriman root
"""
check_user(self.paths, unsafe=self.unsafe)
self.paths.tree_create()
def clear(self) -> None:
"""
remove lock file

View File

@ -57,6 +57,7 @@ class Printer:
"""
return []
# pylint: disable=redundant-returns-doc
def title(self) -> str | None:
"""
generate entry title from content

View File

@ -67,7 +67,7 @@ class FilteredAccessLogger(AccessLogger):
Args:
request(BaseRequest): http reqeust descriptor
response(StreamResponse): streaming response object
time(float):
time(float): log record timestamp
"""
if self.is_logs_post(request) \
or self.is_process_get(request):

View File

@ -95,21 +95,6 @@ class GitHub(Upload, HttpUpload):
with path.open("rb") as archive:
self.make_request("POST", url, params=[("name", path.name)], data=archive, headers=headers)
def get_local_files(self, path: Path) -> dict[Path, str]:
"""
get all local files and their calculated checksums
Args:
path(Path): local path to sync
Returns:
dict[Path, str]: map of path objects to its checksum
"""
return {
local_file: self.calculate_hash(local_file)
for local_file in walk(path)
}
def files_remove(self, release: dict[str, Any], local_files: dict[Path, str], remote_files: dict[str, str]) -> None:
"""
remove files from GitHub
@ -140,6 +125,21 @@ class GitHub(Upload, HttpUpload):
continue
self.asset_upload(release, local_file)
def get_local_files(self, path: Path) -> dict[Path, str]:
"""
get all local files and their calculated checksums
Args:
path(Path): local path to sync
Returns:
dict[Path, str]: map of path objects to its checksum
"""
return {
local_file: self.calculate_hash(local_file)
for local_file in walk(path)
}
def release_create(self) -> dict[str, Any]:
"""
create empty release

View File

@ -87,6 +87,20 @@ class S3(Upload):
suffix = f"-{len(md5s)}" if len(md5s) > 1 else ""
return f"{checksum.hexdigest()}{suffix}"
@staticmethod
def files_remove(local_files: dict[Path, str], remote_objects: dict[Path, Any]) -> None:
"""
remove files which have been removed locally
Args:
local_files(dict[Path, str]): map of local path object to its checksum
remote_objects(dict[Path, Any]): map of remote path object to the remote s3 object
"""
for local_file, remote_object in remote_objects.items():
if local_file in local_files:
continue
remote_object.delete()
@staticmethod
def get_bucket(configuration: Configuration, section: str) -> Any:
"""
@ -105,20 +119,6 @@ class S3(Upload):
aws_secret_access_key=configuration.get(section, "secret_key"))
return client.Bucket(configuration.get(section, "bucket"))
@staticmethod
def files_remove(local_files: dict[Path, str], remote_objects: dict[Path, Any]) -> None:
"""
remove files which have been removed locally
Args:
local_files(dict[Path, str]): map of local path object to its checksum
remote_objects(dict[Path, Any]): map of remote path object to the remote s3 object
"""
for local_file, remote_object in remote_objects.items():
if local_file in local_files:
continue
remote_object.delete()
def files_upload(self, path: Path, local_files: dict[Path, str], remote_objects: dict[Path, Any]) -> None:
"""
upload changed files to s3

View File

@ -354,9 +354,9 @@ class Package(LazyLogging):
Args:
path(Path): path to package sources directory
Returns:
Generator[Path, None, None]: list of paths of files which belong to the package and distributed together
with this tarball. All paths are relative to the ``path``
Yields:
Path: list of paths of files which belong to the package and distributed together with this tarball.
All paths are relative to the ``path``
Raises:
PackageInfoError: if there are parsing errors

View File

@ -34,16 +34,6 @@ class RepositoryId:
architecture: str
name: str
@property
def is_empty(self) -> bool:
"""
check if all data is supplied for the loading
Returns:
bool: True in case if architecture or name are not set and False otherwise
"""
return not self.architecture or not self.name
@property
def id(self) -> str:
"""
@ -56,6 +46,16 @@ class RepositoryId:
return ""
return f"{self.architecture}-{self.name}" # basically the same as used for command line
@property
def is_empty(self) -> bool:
"""
check if all data is supplied for the loading
Returns:
bool: True in case if architecture or name are not set and False otherwise
"""
return not self.architecture or not self.name
def query(self) -> list[tuple[str, str]]:
"""
generate query parameters

View File

@ -41,8 +41,8 @@ def _dynamic_routes(module_root: Path) -> dict[str, Type[View]]:
Returns:
dict[str, Type[View]]: map of the route to its view
"""
def is_base_view(clz: Any) -> TypeGuard[Type[BaseView]]:
return isinstance(clz, type) and issubclass(clz, BaseView)
def is_base_view(clazz: Any) -> TypeGuard[Type[BaseView]]:
return isinstance(clazz, type) and issubclass(clazz, BaseView)
routes: dict[str, Type[View]] = {}
for module_info in _modules(module_root):

View File

@ -104,36 +104,6 @@ def test_subparsers_aur_search_option_repository(parser: argparse.ArgumentParser
assert args.repository == ""
def test_subparsers_help(parser: argparse.ArgumentParser) -> None:
"""
help command must imply architecture list, lock, quiet, report, repository, unsafe and parser
"""
args = parser.parse_args(["help"])
assert args.architecture == ""
assert args.lock is None
assert args.quiet
assert not args.report
assert args.repository == ""
assert args.unsafe
assert args.parser is not None and args.parser()
def test_subparsers_help_option_architecture(parser: argparse.ArgumentParser) -> None:
"""
help command must correctly parse architecture list
"""
args = parser.parse_args(["-a", "x86_64", "help"])
assert args.architecture == ""
def test_subparsers_help_option_repository(parser: argparse.ArgumentParser) -> None:
"""
help command must correctly parse repository list
"""
args = parser.parse_args(["-r", "repo", "help"])
assert args.repository == ""
def test_subparsers_help_commands_unsafe(parser: argparse.ArgumentParser) -> None:
"""
help-commands-unsafe command must imply architecture list, lock, quiet, report, repository, unsafe and parser
@ -164,6 +134,36 @@ def test_subparsers_help_commands_unsafe_option_repository(parser: argparse.Argu
assert args.repository == ""
def test_subparsers_help(parser: argparse.ArgumentParser) -> None:
"""
help command must imply architecture list, lock, quiet, report, repository, unsafe and parser
"""
args = parser.parse_args(["help"])
assert args.architecture == ""
assert args.lock is None
assert args.quiet
assert not args.report
assert args.repository == ""
assert args.unsafe
assert args.parser is not None and args.parser()
def test_subparsers_help_option_architecture(parser: argparse.ArgumentParser) -> None:
"""
help command must correctly parse architecture list
"""
args = parser.parse_args(["-a", "x86_64", "help"])
assert args.architecture == ""
def test_subparsers_help_option_repository(parser: argparse.ArgumentParser) -> None:
"""
help command must correctly parse repository list
"""
args = parser.parse_args(["-r", "repo", "help"])
assert args.repository == ""
def test_subparsers_help_updates(parser: argparse.ArgumentParser) -> None:
"""
help-updates command must imply architecture list, lock, quiet, report, repository, and unsafe

View File

@ -30,30 +30,6 @@ def test_path(args: argparse.Namespace, configuration: Configuration) -> None:
Lock(args, repository_id, configuration).path # special case
def test_check_version(lock: Lock, mocker: MockerFixture) -> None:
"""
must check version correctly
"""
mocker.patch("ahriman.core.status.client.Client.status_get",
return_value=InternalStatus(status=BuildStatus(), version=__version__))
logging_mock = mocker.patch("logging.Logger.warning")
lock.check_version()
logging_mock.assert_not_called()
def test_check_version_mismatch(lock: Lock, mocker: MockerFixture) -> None:
"""
must check mismatched version correctly
"""
mocker.patch("ahriman.core.status.client.Client.status_get",
return_value=InternalStatus(status=BuildStatus(), version="version"))
logging_mock = mocker.patch("logging.Logger.warning")
lock.check_version()
logging_mock.assert_called_once() # we do not check logging arguments
def test_check_user(lock: Lock, mocker: MockerFixture) -> None:
"""
must check user correctly
@ -84,6 +60,30 @@ def test_check_user_unsafe(lock: Lock, mocker: MockerFixture) -> None:
lock.check_user()
def test_check_version(lock: Lock, mocker: MockerFixture) -> None:
"""
must check version correctly
"""
mocker.patch("ahriman.core.status.client.Client.status_get",
return_value=InternalStatus(status=BuildStatus(), version=__version__))
logging_mock = mocker.patch("logging.Logger.warning")
lock.check_version()
logging_mock.assert_not_called()
def test_check_version_mismatch(lock: Lock, mocker: MockerFixture) -> None:
"""
must check mismatched version correctly
"""
mocker.patch("ahriman.core.status.client.Client.status_get",
return_value=InternalStatus(status=BuildStatus(), version="version"))
logging_mock = mocker.patch("logging.Logger.warning")
lock.check_version()
logging_mock.assert_called_once() # we do not check logging arguments
def test_clear(lock: Lock) -> None:
"""
must remove lock file

View File

@ -89,15 +89,6 @@ def test_asset_upload_empty_mimetype(github: GitHub, github_release: dict[str, A
headers={"Content-Type": "application/octet-stream"})
def test_get_local_files(github: GitHub, resource_path_root: Path, mocker: MockerFixture) -> None:
"""
must get all local files recursively
"""
walk_mock = mocker.patch("ahriman.core.util.walk")
github.get_local_files(resource_path_root)
walk_mock.assert_called()
def test_files_remove(github: GitHub, github_release: dict[str, Any], mocker: MockerFixture) -> None:
"""
must remove files from the remote
@ -137,6 +128,15 @@ def test_files_upload_empty(github: GitHub, github_release: dict[str, Any], mock
upload_mock.assert_not_called()
def test_get_local_files(github: GitHub, resource_path_root: Path, mocker: MockerFixture) -> None:
"""
must get all local files recursively
"""
walk_mock = mocker.patch("ahriman.core.util.walk")
github.get_local_files(resource_path_root)
walk_mock.assert_called()
def test_release_create(github: GitHub, mocker: MockerFixture) -> None:
"""
must create release

View File

@ -3,6 +3,14 @@ import pytest
from ahriman.models.repository_id import RepositoryId
def test_id() -> None:
"""
must correctly generate id
"""
assert RepositoryId("", "").id == ""
assert RepositoryId("arch", "repo").id == "arch-repo"
def test_is_empty() -> None:
"""
must check if repository id is empty or not
@ -13,14 +21,6 @@ def test_is_empty() -> None:
assert not RepositoryId("arch", "repo").is_empty
def test_id() -> None:
"""
must correctly generate id
"""
assert RepositoryId("", "").id == ""
assert RepositoryId("arch", "repo").id == "arch-repo"
def test_query() -> None:
"""
must generate query request parameters