feat: add rule enforces Self types on constructors (#20)

Author: jack-tutor

README.md

@@ -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]()`
@@ -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`)

poetry.lock

@@ -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"
 

pyproject.toml

@@ -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
@@ -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,
         )
 
 

tutor_flake/common.py

@@ -3,6 +3,8 @@
 from enum import Enum
 from typing import ClassVar, Final, List, NamedTuple, Protocol, TypeVar
 
+from typing_extensions import Self
+
 
 class DummyClass:
 
@@ -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

tutor_flake/example_code/test_classvar_rules.py

@@ -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

tutor_flake/example_code/test_constructor.py

@@ -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
@@ -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
@@ -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,
@@ -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
@@ -210,6 +213,7 @@ def visit_AsyncFunctionDef(
             ),
             ConsecutiveSameTypedPositionalArgs.check(node),
             AsyncFunctionsAreAsynchronous.check(node),
+            ConstructorIsWellTyped.check(node, self.parents),
         )
 
     @visitor_decorator

tutor_flake/plugin.py

@@ -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