Skip to content
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
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ Custom flake8 rules
* TUT300 - no expressions in the main body, unless under __name__ == "__main__", prevents global side effects
* TUT4:
* TUT400 - detect strings that were likely meant to be f-strings
* TODO: we should exclude regex-es
* TUT410 - detect redundant type annotations
* TUT411 - detect redundant type annotations for generic assignment
* Don't do `x: Foo[bar] = Foo()` ; do `x = Foo[bar]()`
Expand All @@ -45,6 +46,9 @@ Custom flake8 rules
* All the same exceptions as 511
* TUT520 - `NotImplemented` is only allowed within a dunder method on a class
* Any other usage is very likely incorrect
* TUT530 - a constructor must return `Self` type rather than the class type
* This encourages good use of constructors that play well with inheritence
* We detect methods whose return type includes the class type without having any input of the type
* TUT6
* TUT610 - a function definition allows too many positional arguments (configurable with `max-definition-positional-args`)
* TUT620 - a function invocation uses too many positional arguments (configurable with `max-invocation-positional-args`)
Expand Down
365 changes: 173 additions & 192 deletions poetry.lock

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ python = "^3.10"
flake8 = "^6.0.0"
isort = "^5.10.1"
black = "^22.3.0"
mypy = "^0.961"
mypy = "^1.11.0"
astpretty = "^3.0.0"
pre-commit = "^2.19.0"

Expand Down
11 changes: 8 additions & 3 deletions tutor_flake/common.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import ast
from typing import List, NamedTuple, Optional, Type, Union

from typing_extensions import Self


class Flake8Error(NamedTuple):
line_number: int
Expand All @@ -15,9 +17,12 @@ def construct( # noqa: TUT610
code: int,
description: str,
rule_cls: type,
) -> "Flake8Error":
return Flake8Error(
node.lineno, node.col_offset, f"TUT{code} {description}", rule_cls
) -> Self:
return cls(
node.lineno, # type: ignore
node.col_offset, # type: ignore
f"TUT{code} {description}",
rule_cls,
)


Expand Down
4 changes: 3 additions & 1 deletion tutor_flake/example_code/test_classvar_rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
from enum import Enum
from typing import ClassVar, Final, List, NamedTuple, Protocol, TypeVar

from typing_extensions import Self


class DummyClass:

Expand Down Expand Up @@ -63,7 +65,7 @@ def __str__(self) -> str:
return super().__str__()

@classmethod
def construct(cls) -> "ExampleDataclass":
def construct(cls) -> Self:
raise NotImplementedError

# other inline comment
Expand Down
46 changes: 46 additions & 0 deletions tutor_flake/example_code/test_constructor.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
from contextlib import asynccontextmanager
from typing import AsyncGenerator, List, Tuple

from typing_extensions import Self


class FooBar:
@classmethod
def bad_cls_construct(cls) -> "FooBar": # noqa: TUT530
raise NotImplementedError

@staticmethod
def bad_static_struct() -> "FooBar": # noqa: TUT530
raise NotImplementedError

def bad_self_method(self) -> "FooBar": # noqa: TUT530
raise NotImplementedError

def bad_complicated_return(self) -> Tuple[None, "FooBar"]: # noqa: TUT530
raise NotImplementedError

@asynccontextmanager
async def bad_complicated_return_2( # noqa: TUT530 TUT210
self,
) -> AsyncGenerator["FooBar", None]:
yield self

async def return_allowed_because_of_args(
self, arg_1: List["FooBar"]
) -> List["FooBar"]:
raise NotImplementedError

def return_allowed_because_of_positional_only_arg(
self,
arg_1: "FooBar",
) -> "FooBar":
raise NotImplementedError

def return_allowed_because_of_keyword_only_arg(
self, *, arg_1: "FooBar"
) -> "FooBar":
raise NotImplementedError

@classmethod
def good_cls_construct(cls) -> Self:
raise NotImplementedError
8 changes: 6 additions & 2 deletions tutor_flake/plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from typing import Any, Callable, ClassVar, Generator, Iterable, List, Optional, TypeVar

from flake8.options.manager import OptionManager
from typing_extensions import Self

from tutor_flake import __version__
from tutor_flake.common import Flake8Error
Expand All @@ -20,6 +21,7 @@
)
from tutor_flake.rules.classvar import ClassvarCheck
from tutor_flake.rules.compact_generic import CompactGeneric
from tutor_flake.rules.constructor import ConstructorIsWellTyped
from tutor_flake.rules.dataclass import DataclassRenamed
from tutor_flake.rules.no_sideeffects import NoSideeffects
from tutor_flake.rules.not_implemented import NotImplementedCheck
Expand Down Expand Up @@ -75,8 +77,8 @@ def add_options(option_manager: OptionManager) -> None:
)

@classmethod
def parse_options(cls, options: Namespace) -> "TutorFlakeConfig":
return TutorFlakeConfig(
def parse_options(cls, options: Namespace) -> Self:
return cls(
options.max_definition_positional_args,
options.max_invocation_positional_args,
options.non_init_classes,
Expand Down Expand Up @@ -187,6 +189,7 @@ def visit_FunctionDef(self, node: ast.FunctionDef) -> Iterable[Flake8Error]:
ChildClassCallsSuperMethods.check(
node, self.parents, self.config.non_init_classes
),
ConstructorIsWellTyped.check(node, self.parents),
)

@visitor_decorator
Expand All @@ -210,6 +213,7 @@ def visit_AsyncFunctionDef(
),
ConsecutiveSameTypedPositionalArgs.check(node),
AsyncFunctionsAreAsynchronous.check(node),
ConstructorIsWellTyped.check(node, self.parents),
)

@visitor_decorator
Expand Down
50 changes: 50 additions & 0 deletions tutor_flake/rules/constructor.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import ast
import itertools
from ast import Constant, expr, walk
from typing import Generator, List

from tutor_flake.common import Flake8Error


class ConstructorIsWellTyped:
@classmethod
def check(
cls, func: ast.FunctionDef | ast.AsyncFunctionDef, parents: List[ast.AST]
) -> Generator[Flake8Error, None, None]:
class_definition = next(
(
parent
for parent in reversed(parents)
if isinstance(parent, ast.ClassDef)
),
None,
)
if class_definition is None:
return
class_name = class_definition.name
for func_arg in itertools.chain(
func.args.posonlyargs, func.args.args, func.args.kwonlyargs
):
if (
annotation := func_arg.annotation
) is not None and does_type_annotation_include_type(annotation, class_name):
# we allow the return type if an argument includes the class name
return
if (
func_return := func.returns
) is not None and does_type_annotation_include_type(func_return, class_name):
yield Flake8Error.construct(
func,
530,
f"Funciton `{func.name}`'s return value includes the class type: `{class_name}`."
" Likely you want to return the Self type imported from `typing` or `typing_extensions`"
" as child types likely should return their own type.",
cls,
)


def does_type_annotation_include_type(annotation: expr, check_type: str) -> bool:
for child_node in walk(annotation):
if isinstance(child_node, Constant) and child_node.value == check_type:
return True
return False