Skip to content
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

Add type hints, assertions, and minor refactorings #196

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
16 changes: 16 additions & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,22 @@ jobs:
cd /review
/hyperstyle/bin/pytest

type-checking:
name: Type checking
runs-on: [ self-hosted, small ]
needs:
- build_image
container:
image: hyperskill.azurecr.io/hyperstyle:${{ github.sha }}
credentials:
username: ${{ secrets.REGISTRY_USER }}
password: ${{ secrets.REGISTRY_PASSWORD }}
steps:
- name: Lint
run: |
cd /review
/hyperstyle/bin/mypy .

build:
runs-on: [ self-hosted, small ]
needs:
Expand Down
12 changes: 6 additions & 6 deletions hyperstyle/src/python/common/tool_arguments.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

from dataclasses import dataclass
from enum import Enum, unique
from typing import ClassVar

from hyperstyle.src.python.review.common.language import Language
from hyperstyle.src.python.review.common.language_version import LanguageVersion
Expand Down Expand Up @@ -30,6 +29,10 @@ class ArgumentsInfo:
description: str


INSPECTORS = [inspector.lower() for inspector in InspectorType.available_values()]
DISABLED_INSPECTORS_EXAMPLE = f"-d {INSPECTORS[0].lower()},{INSPECTORS[1].lower()}"


@unique
class RunToolArgument(Enum):
VERBOSITY = ArgumentsInfo(
Expand All @@ -43,15 +46,12 @@ class RunToolArgument(Enum):
"default is 0",
)

inspectors: ClassVar = [inspector.lower() for inspector in InspectorType.available_values()]
disabled_inspectors_example = f"-d {inspectors[0].lower()},{inspectors[1].lower()}"

DISABLE = ArgumentsInfo(
"-d",
"--disable",
'Disable inspectors. '
f'Available values: {", ".join(inspectors)}. '
f'Example: {disabled_inspectors_example}',
f'Available values: {", ".join(INSPECTORS)}. '
f'Example: {DISABLED_INSPECTORS_EXAMPLE}',
)

DUPLICATES = ArgumentsInfo(
Expand Down
2 changes: 1 addition & 1 deletion hyperstyle/src/python/review/application_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class ApplicationConfig:
disabled_inspectors: set[InspectorType]
allow_duplicates: bool
n_cpu: int
inspectors_config: dict
inspectors_config: dict[str, object]
with_all_categories: bool
start_line: int = 1
language: Language | None = None
Expand Down
4 changes: 0 additions & 4 deletions hyperstyle/src/python/review/common/file_system.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,6 @@ def new_temp_dir() -> Iterator[Path]:
yield Path(temp_dir)


def new_temp_file(suffix: Extension = Extension.EMPTY) -> Iterator[tuple[str, str]]:
yield tempfile.mkstemp(suffix=suffix.value)


def get_file_line(path: Path, line_number: int) -> str:
return linecache.getline(
str(path),
Expand Down
12 changes: 7 additions & 5 deletions hyperstyle/src/python/review/common/parallel_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,11 @@ def inspect_in_parallel(
return issues

with multiprocessing.Pool(config.n_cpu) as pool:
issues = pool.map(
functools.partial(inspector_runner, data, config),
inspectors_to_run,
return list(
itertools.chain(
*pool.map(
functools.partial(inspector_runner, data, config),
inspectors_to_run,
)
)
)

return list(itertools.chain(*issues))
Original file line number Diff line number Diff line change
Expand Up @@ -138,10 +138,6 @@ def convert_to_base_issues(
return base_issues

def _get_inspection_result(self, code_text: str, file_path: Path) -> list[BaseIssue]:
if self.host is None or self.port is None:
msg = "Connection parameters is not set up."
raise Exception(msg)

client = IJClient(self.host, self.port)

code = model_pb2.Code()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@


class IJClient:
stub: model_pb2_grpc.CodeInspectionServiceStub

def __init__(self, host: str = "localhost", port: int = 8080) -> None:
self.host = host
self.port = port
Expand All @@ -22,10 +24,10 @@ def __init__(self, host: str = "localhost", port: int = 8080) -> None:
msg = "Failed to connect to ij code server"
raise Exception(msg) from e
else:
self.stub = model_pb2_grpc.CodeInspectionServiceStub(self.channel)
self.stub = model_pb2_grpc.CodeInspectionServiceStub(self.channel) # type: ignore[no-untyped-call]

def inspect(self, code: model_pb2.Code) -> model_pb2.InspectionResult:
return self.stub.inspect(code, timeout=TIMEOUT)

def init(self, service: model_pb2.Service) -> model_pb2.InitResult:
return self.stub.init(service, timeout=TIMEOUT)
return self.stub.init(service, timeout=TIMEOUT) # type: ignore[attr-defined]
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
get_issue_class_by_issue_type,
get_measure_name_by_measurable_issue_type,
IssueData,
Measurable,
MeasurableIssue,
)

if TYPE_CHECKING:
Expand Down Expand Up @@ -36,7 +36,7 @@ def convert_base_issue(base_issue: BaseIssue, issue_configs_handler: IssueConfig
issue_data[IssueData.DESCRIPTION.value] = issue_configs_handler.get_description(origin_class, description)

issue_class = get_issue_class_by_issue_type(issue_type)
if issubclass(issue_class, Measurable):
if issubclass(issue_class, MeasurableIssue):
measure = issue_configs_handler.parse_measure(origin_class, description)
if measure is None:
logger.error(f"{inspector_type.value}: Unable to parse measure.")
Expand Down
34 changes: 16 additions & 18 deletions hyperstyle/src/python/review/inspectors/common/issue/issue.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
from __future__ import annotations

import abc
import logging
from collections import defaultdict
from dataclasses import dataclass
from enum import Enum, unique
from typing import Any, TYPE_CHECKING
from typing import Any, Protocol, runtime_checkable, TYPE_CHECKING

if TYPE_CHECKING:
from pathlib import Path
Expand Down Expand Up @@ -206,10 +205,9 @@ class BaseIssue(ShortIssue):
difficulty: IssueDifficulty


class Measurable(abc.ABC):
@abc.abstractmethod
def measure(self) -> int:
pass
@runtime_checkable
class MeasurableIssue(Protocol):
def measure(self) -> int: ...


@dataclass(frozen=True)
Expand All @@ -218,7 +216,7 @@ class CodeIssue(BaseIssue):


@dataclass(frozen=True)
class BoolExprLenIssue(BaseIssue, Measurable):
class BoolExprLenIssue(BaseIssue, MeasurableIssue):
bool_expr_len: int
type = IssueType.BOOL_EXPR_LEN

Expand All @@ -227,7 +225,7 @@ def measure(self) -> int:


@dataclass(frozen=True)
class FuncLenIssue(BaseIssue, Measurable):
class FuncLenIssue(BaseIssue, MeasurableIssue):
func_len: int
type = IssueType.FUNC_LEN

Expand All @@ -236,7 +234,7 @@ def measure(self) -> int:


@dataclass(frozen=True)
class LineLenIssue(BaseIssue, Measurable):
class LineLenIssue(BaseIssue, MeasurableIssue):
line_len: int
type = IssueType.LINE_LEN

Expand All @@ -245,7 +243,7 @@ def measure(self) -> int:


@dataclass(frozen=True)
class CyclomaticComplexityIssue(BaseIssue, Measurable):
class CyclomaticComplexityIssue(BaseIssue, MeasurableIssue):
cc_value: int
type = IssueType.CYCLOMATIC_COMPLEXITY

Expand All @@ -254,7 +252,7 @@ def measure(self) -> int:


@dataclass(frozen=True)
class InheritanceIssue(BaseIssue, Measurable):
class InheritanceIssue(BaseIssue, MeasurableIssue):
inheritance_tree_depth: int
type = IssueType.INHERITANCE_DEPTH

Expand All @@ -263,7 +261,7 @@ def measure(self) -> int:


@dataclass(frozen=True)
class ChildrenNumberIssue(BaseIssue, Measurable):
class ChildrenNumberIssue(BaseIssue, MeasurableIssue):
children_number: int
type = IssueType.CHILDREN_NUMBER

Expand All @@ -272,7 +270,7 @@ def measure(self) -> int:


@dataclass(frozen=True)
class WeightedMethodIssue(BaseIssue, Measurable):
class WeightedMethodIssue(BaseIssue, MeasurableIssue):
weighted_method: int
type = IssueType.WEIGHTED_METHOD

Expand All @@ -281,7 +279,7 @@ def measure(self) -> int:


@dataclass(frozen=True)
class CouplingIssue(BaseIssue, Measurable):
class CouplingIssue(BaseIssue, MeasurableIssue):
class_objects_coupling: int
type = IssueType.COUPLING

Expand All @@ -290,7 +288,7 @@ def measure(self) -> int:


@dataclass(frozen=True)
class CohesionIssue(BaseIssue, Measurable):
class CohesionIssue(BaseIssue, MeasurableIssue):
cohesion_lack: int
type = IssueType.COHESION

Expand All @@ -299,7 +297,7 @@ def measure(self) -> int:


@dataclass(frozen=True)
class ClassResponseIssue(BaseIssue, Measurable):
class ClassResponseIssue(BaseIssue, MeasurableIssue):
class_response: int
type = IssueType.CLASS_RESPONSE

Expand All @@ -308,7 +306,7 @@ def measure(self) -> int:


@dataclass(frozen=True)
class MethodNumberIssue(BaseIssue, Measurable):
class MethodNumberIssue(BaseIssue, MeasurableIssue):
method_number: int
type = IssueType.METHOD_NUMBER

Expand All @@ -317,7 +315,7 @@ def measure(self) -> int:


@dataclass(frozen=True)
class MaintainabilityLackIssue(BaseIssue, Measurable):
class MaintainabilityLackIssue(BaseIssue, MeasurableIssue):
maintainability_lack: int
type = IssueType.MAINTAINABILITY

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import logging
from dataclasses import dataclass, field
from typing import Optional, TYPE_CHECKING
from typing import TYPE_CHECKING

from hyperstyle.src.python.review.inspectors.common.utils import (
contains_format_fields,
Expand Down Expand Up @@ -136,7 +136,7 @@ def __init__(self, *issue_configs: IssueConfig) -> None:
issue_config.origin_class: issue_config for issue_config in issue_configs
}

def _parse_description(self, origin_class: str, description: str) -> tuple | None:
def _parse_description(self, origin_class: str, description: str) -> tuple[object, ...] | None:
"""Parse a description.

:param origin_class: An origin class of issue.
Expand All @@ -158,7 +158,7 @@ def _parse_description(self, origin_class: str, description: str) -> tuple | Non

return args

def parse_measure(self, origin_class: str, description: str) -> Optional:
def parse_measure(self, origin_class: str, description: str) -> object | None:
"""Parse a measure from a description.

:param origin_class: An origin class of issue.
Expand Down
4 changes: 2 additions & 2 deletions hyperstyle/src/python/review/inspectors/common/xml_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@
logger = logging.getLogger(__name__)


def __should_handle_element(element: ET) -> bool:
def __should_handle_element(element: Element) -> bool:
"""Checks if a tree element is a file."""
return element.tag == "file"


def __is_error(element: ET) -> bool:
def __is_error(element: Element) -> bool:
"""Checks if a tree element is an error."""
return element.tag == "error"

Expand Down
1 change: 1 addition & 0 deletions hyperstyle/src/python/review/inspectors/flake8/flake8.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ def choose_issue_type(code: str) -> IssueType:
return CODE_TO_ISSUE_TYPE[code]

regex_match = re.match(r"^([A-Z]+)(\d)\d*$", code, re.IGNORECASE)
assert regex_match is not None, f"flake8: {code} - unexpected error code format"
code_prefix = regex_match.group(1)
first_code_number = regex_match.group(2)

Expand Down
2 changes: 1 addition & 1 deletion hyperstyle/src/python/review/inspectors/ij_java/ij_java.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

class JavaIJInspector(BaseIJInspector):
inspector_type = InspectorType.IJ_JAVA
language_id = model_pb2.LanguageId.Java
language_id = model_pb2.LanguageId.Java # type: ignore[attr-defined]
issue_configs = ISSUE_CONFIGS
ij_inspection_to_issue_type = IJ_INSPECTION_TO_ISSUE_TYPE
ij_message_to_issue_type = IJ_MESSAGE_TO_ISSUE_TYPE
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
from __future__ import annotations

IJ_INSPECTION_TO_ISSUE_TYPE = {}
from typing import TYPE_CHECKING

IJ_MESSAGE_TO_ISSUE_TYPE = {}
if TYPE_CHECKING:
from hyperstyle.src.python.review.inspectors.common.issue.issue import IssueType

IJ_INSPECTION_TO_ISSUE_TYPE: dict[str, IssueType] = {}

IJ_MESSAGE_TO_ISSUE_TYPE: dict[str, dict[str, IssueType]] = {}
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

class KotlinIJInspector(BaseIJInspector):
inspector_type = InspectorType.IJ_KOTLIN
language_id = model_pb2.LanguageId.kotlin
language_id = model_pb2.LanguageId.kotlin # type: ignore[attr-defined]
issue_configs = ISSUE_CONFIGS
ij_inspection_to_issue_type = IJ_INSPECTION_TO_ISSUE_TYPE
ij_message_to_issue_type = IJ_MESSAGE_TO_ISSUE_TYPE
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
from __future__ import annotations

IJ_INSPECTION_TO_ISSUE_TYPE = {}
from typing import TYPE_CHECKING

IJ_MESSAGE_TO_ISSUE_TYPE = {}
if TYPE_CHECKING:
from hyperstyle.src.python.review.inspectors.common.issue.issue import IssueType

IJ_INSPECTION_TO_ISSUE_TYPE: dict[str, IssueType] = {}

IJ_MESSAGE_TO_ISSUE_TYPE: dict[str, dict[str, IssueType]] = {}
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

class PythonIJInspector(BaseIJInspector):
inspector_type = InspectorType.IJ_PYTHON
language_id = model_pb2.LanguageId.Python
language_id = model_pb2.LanguageId.Python # type: ignore[attr-defined]
issue_configs = ISSUE_CONFIGS
ij_inspection_to_issue_type = IJ_INSPECTION_TO_ISSUE_TYPE
ij_message_to_issue_type = IJ_MESSAGE_TO_ISSUE_TYPE
Loading
Loading